Fix printer and print job ordering, add back 'move to top'

plugins/UM3NetworkPrinting/resources/qml/MonitorContextMenu.qml

@@ -46,9 +46,6 @@ Item
 
                 spacing: Math.floor(UM.Theme.getSize("default_margin").height / 2)
 
-                // Due to an issue with the ordering if print jobs caused by the Qt list models,
-                // we hide the 'move to top' feature for now as it's not displayed on the appropriate elements.
-                // Solving the ordering issue will cost more time than we currently have available.
                 PrintJobContextMenuItem {
                     onClicked: {
                         sendToTopConfirmationDialog.visible = true;
@@ -56,11 +53,11 @@ Item
                     }
                     text: catalog.i18nc("@label", "Move to top");
                     visible: {
-//                        if (printJob && (printJob.state == "queued" || printJob.state == "error") && !isAssigned(printJob)) {
+                        if (printJob && (printJob.state == "queued" || printJob.state == "error") && !isAssigned(printJob)) {
-//                            if (OutputDevice && OutputDevice.queuedPrintJobs[0]) {
+                            if (OutputDevice && OutputDevice.queuedPrintJobs[0]) {
-//                                return OutputDevice.queuedPrintJobs[0].key != printJob.key;
+                                return OutputDevice.queuedPrintJobs[0].key != printJob.key;
-//                            }
+                            }
-//                        }
+                        }
                         return false;
                     }
                 }

plugins/UM3NetworkPrinting/resources/qml/MonitorQueue.qml

@@ -95,6 +95,22 @@ Item
         }
         spacing: 18 * screenScaleFactor // TODO: Theme!
 
+        Label
+        {
+            text: catalog.i18nc("@label", "There are no print jobs in the queue. Slice and send a job to add one.")
+            color: UM.Theme.getColor("monitor_text_primary")
+            elide: Text.ElideRight
+            font: UM.Theme.getFont("medium") // 14pt, regular
+            anchors.verticalCenter: parent.verticalCenter
+            width: 600 * screenScaleFactor // TODO: Theme! (Should match column size)
+
+            // FIXED-LINE-HEIGHT:
+            height: 18 * screenScaleFactor // TODO: Theme!
+            verticalAlignment: Text.AlignVCenter
+            renderType: Text.NativeRendering
+            visible: printJobList.count === 0
+        }
+
         Label
         {
             text: catalog.i18nc("@label", "Print jobs")
@@ -108,6 +124,7 @@ Item
             height: 18 * screenScaleFactor // TODO: Theme!
             verticalAlignment: Text.AlignVCenter
             renderType: Text.NativeRendering
+            visible: printJobList.count > 0
         }
 
         Label
@@ -123,6 +140,7 @@ Item
             height: 18 * screenScaleFactor // TODO: Theme!
             verticalAlignment: Text.AlignVCenter
             renderType: Text.NativeRendering
+            visible: printJobList.count > 0
         }
 
         Label
@@ -138,6 +156,7 @@ Item
             height: 18 * screenScaleFactor // TODO: Theme!
             verticalAlignment: Text.AlignVCenter
             renderType: Text.NativeRendering
+            visible: printJobList.count > 0
         }
     }
 
@@ -181,88 +200,4 @@ Item
             spacing: 6  // TODO: Theme!
         }
     }
-
-    Rectangle
-    {
-        anchors
-        {
-            horizontalCenter: parent.horizontalCenter
-            top: printJobQueueHeadings.bottom
-            topMargin: 12 * screenScaleFactor // TODO: Theme!
-        }
-        height: 48 * screenScaleFactor // TODO: Theme!
-        width: parent.width
-        color: UM.Theme.getColor("monitor_card_background")
-        border.color: UM.Theme.getColor("monitor_card_border")
-        radius: 2 * screenScaleFactor // TODO: Theme!
-        visible: OutputDevice.printJobs.length == 0
-
-        Row
-        {
-            anchors
-            {
-                left: parent.left
-                leftMargin: 18 * screenScaleFactor // TODO: Theme!
-                verticalCenter: parent.verticalCenter
-            }
-            spacing: 18 * screenScaleFactor // TODO: Theme!
-            height: 18 * screenScaleFactor // TODO: Theme!
-
-            Label
-            {
-                text: i18n.i18nc("@info", "All jobs are printed.")
-                color: UM.Theme.getColor("monitor_text_primary")
-                font: UM.Theme.getFont("medium") // 14pt, regular
-                renderType: Text.NativeRendering
-            }
-
-            Item
-            {
-                id: viewPrintHistoryLabel
-                
-                height: 18 * screenScaleFactor // TODO: Theme!
-                width: childrenRect.width
-                visible: !cloudConnection
-
-                UM.RecolorImage
-                {
-                    id: printHistoryIcon
-                    anchors.verticalCenter: parent.verticalCenter
-                    color: UM.Theme.getColor("monitor_text_link")
-                    source: UM.Theme.getIcon("external_link")
-                    width: 16 * screenScaleFactor // TODO: Theme! (Y U NO USE 18 LIKE ALL OTHER ICONS?!)
-                    height: 16 * screenScaleFactor // TODO: Theme! (Y U NO USE 18 LIKE ALL OTHER ICONS?!)
-                }
-                Label
-                {
-                    id: viewPrintHistoryText
-                    anchors
-                    {
-                        left: printHistoryIcon.right
-                        leftMargin: 6 * screenScaleFactor // TODO: Theme!
-                        verticalCenter: printHistoryIcon.verticalCenter
-                    }
-                    color: UM.Theme.getColor("monitor_text_link")
-                    font: UM.Theme.getFont("medium") // 14pt, regular
-                    linkColor: UM.Theme.getColor("monitor_text_link")
-                    text: catalog.i18nc("@label link to connect manager", "Manage in browser")
-                    renderType: Text.NativeRendering
-                }
-                MouseArea
-                {
-                    anchors.fill: parent
-                    hoverEnabled: true
-                    onClicked: OutputDevice.openPrintJobControlPanel()
-                    onEntered:
-                    {
-                        viewPrintHistoryText.font.underline = true
-                    }
-                    onExited:
-                    {
-                        viewPrintHistoryText.font.underline = false
-                    }
-                }
-            }
-        }
-    }
 }

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py

@@ -35,8 +35,7 @@ from .Models.CloudPrintResponse import CloudPrintResponse
 from .Models.CloudPrintJobResponse import CloudPrintJobResponse
 from .Models.CloudClusterPrinterStatus import CloudClusterPrinterStatus
 from .Models.CloudClusterPrintJobStatus import CloudClusterPrintJobStatus
-from .Utils import findChanges, formatDateCompleted, formatTimeCompleted
+from .Utils import formatDateCompleted, formatTimeCompleted
-
 
 I18N_CATALOG = i18nCatalog("cura")
 
@@ -46,7 +45,6 @@ I18N_CATALOG = i18nCatalog("cura")
 #   As such, those methods have been implemented here.
 #   Note that this device represents a single remote cluster, not a list of multiple clusters.
 class CloudOutputDevice(NetworkedPrinterOutputDevice):
-
     # The interval with which the remote clusters are checked
     CHECK_CLUSTER_INTERVAL = 10.0  # seconds
 
@@ -80,8 +78,8 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
             b"cluster_size": b"1"  # cloud devices are always clusters of at least one
         }
 
-        super().__init__(device_id = cluster.cluster_id, address = "",
+        super().__init__(device_id=cluster.cluster_id, address="",
-                         connection_type = ConnectionType.CloudConnection, properties = properties, parent = parent)
+                         connection_type=ConnectionType.CloudConnection, properties=properties, parent=parent)
         self._api = api_client
         self._cluster = cluster
 
@@ -179,9 +177,10 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         # Show an error message if we're already sending a job.
         if self._progress.visible:
             message = Message(
-                text = I18N_CATALOG.i18nc("@info:status", "Sending new jobs (temporarily) blocked, still sending the previous print job."),
+                text=I18N_CATALOG.i18nc("@info:status",
-                title = I18N_CATALOG.i18nc("@info:title", "Cloud error"),
+                                        "Sending new jobs (temporarily) blocked, still sending the previous print job."),
-                lifetime = 10
+                title=I18N_CATALOG.i18nc("@info:title", "Cloud error"),
+                lifetime=10
             )
             message.show()
             return
@@ -203,9 +202,9 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
 
         self._tool_path = mesh
         request = CloudPrintJobUploadRequest(
-            job_name = file_name or mesh_format.file_extension,
+            job_name=file_name or mesh_format.file_extension,
-            file_size = len(mesh),
+            file_size=len(mesh),
-            content_type = mesh_format.mime_type,
+            content_type=mesh_format.mime_type,
         )
         self._api.requestUpload(request, self._onPrintJobCreated)
 
@@ -237,65 +236,74 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
             self._updatePrintJobs(status.print_jobs)
 
     ## Updates the local list of printers with the list received from the cloud.
-    #  \param jobs: The printers received from the cloud.
+    #  \param remote_printers: The printers received from the cloud.
-    def _updatePrinters(self, printers: List[CloudClusterPrinterStatus]) -> None:
+    def _updatePrinters(self, remote_printers: List[CloudClusterPrinterStatus]) -> None:
-        previous = {p.key: p for p in self._printers}  # type: Dict[str, PrinterOutputModel]
-        received = {p.uuid: p for p in printers}  # type: Dict[str, CloudClusterPrinterStatus]
-        removed_printers, added_printers, updated_printers = findChanges(previous, received)
 
+        # Keep track of the new printers to show.
+        # We create a new list instead of changing the existing one to get the correct order.
+        new_printers = []
+
+        # Check which printers need to be created or updated.
+        for index, printer_data in enumerate(remote_printers):
+            printer = next(iter(printer for printer in self._printers if printer.key == printer_data.uuid), None)
+            if not printer:
+                new_printers.append(printer_data.createOutputModel(CloudOutputController(self)))
+            else:
+                printer_data.updateOutputModel(printer)
+                new_printers.append(printer)
+
+        # Check which printers need to be removed (de-referenced).
+        remote_printers_keys = [printer_data.uuid for printer_data in remote_printers]
+        removed_printers = [printer for printer in self._printers if printer.key not in remote_printers_keys]
         for removed_printer in removed_printers:
-            if self._active_printer == removed_printer:
+            if self._active_printer.key == removed_printer.key:
                 self.setActivePrinter(None)
-            self._printers.remove(removed_printer)
 
-        for added_printer in added_printers:
+        self._printers = new_printers
-            self._printers.append(added_printer.createOutputModel(CloudOutputController(self)))
+        if self._printers and not self.activePrinter:
-
-        for model, printer in updated_printers:
-            printer.updateOutputModel(model)
-
-        # Always have an active printer
-        if self._printers and not self._active_printer:
             self.setActivePrinter(self._printers[0])
 
-        if added_printers or removed_printers:
         self.printersChanged.emit()
 
     ## Updates the local list of print jobs with the list received from the cloud.
-    #  \param jobs: The print jobs received from the cloud.
+    #  \param remote_jobs: The print jobs received from the cloud.
-    def _updatePrintJobs(self, jobs: List[CloudClusterPrintJobStatus]) -> None:
+    def _updatePrintJobs(self, remote_jobs: List[CloudClusterPrintJobStatus]) -> None:
-        received = {j.uuid: j for j in jobs}  # type: Dict[str, CloudClusterPrintJobStatus]
-        previous = {j.key: j for j in self._print_jobs}  # type: Dict[str, UM3PrintJobOutputModel]
 
-        removed_jobs, added_jobs, updated_jobs = findChanges(previous, received)
+        # Keep track of the new print jobs to show.
+        # We create a new list instead of changing the existing one to get the correct order.
+        new_print_jobs = []
 
+        # Check which print jobs need to be created or updated.
+        for index, print_job_data in enumerate(remote_jobs):
+            print_job = next(
+                iter(print_job for print_job in self._print_jobs if print_job.key == print_job_data.uuid), None)
+            if not print_job:
+                new_print_jobs.append(self._createPrintJobModel(print_job_data))
+            else:
+                print_job_data.updateOutputModel(print_job)
+                if print_job_data.printer_uuid:
+                    self._updateAssignedPrinter(print_job, print_job_data.printer_uuid)
+                new_print_jobs.append(print_job)
+
+        # Check which print job need to be removed (de-referenced).
+        remote_job_keys = [print_job_data.uuid for print_job_data in remote_jobs]
+        removed_jobs = [print_job for print_job in self._print_jobs if print_job.key not in remote_job_keys]
         for removed_job in removed_jobs:
             if removed_job.assignedPrinter:
                 removed_job.assignedPrinter.updateActivePrintJob(None)
                 removed_job.stateChanged.disconnect(self._onPrintJobStateChanged)
-            self._print_jobs.remove(removed_job)
 
-        for added_job in added_jobs:
+        self._print_jobs = new_print_jobs
-            self._addPrintJob(added_job)
-
-        for model, job in updated_jobs:
-            job.updateOutputModel(model)
-            if job.printer_uuid:
-                self._updateAssignedPrinter(model, job.printer_uuid)
-
-        # We only have to update when jobs are added or removed
-        # updated jobs push their changes via their output model
-        if added_jobs or removed_jobs:
         self.printJobsChanged.emit()
 
-    ## Registers a new print job received via the cloud API.
+    ## Create a new print job model based on the remote status of the job.
-    #  \param job: The print job received.
+    #  \param remote_job: The remote print job data.
-    def _addPrintJob(self, job: CloudClusterPrintJobStatus) -> None:
+    def _createPrintJobModel(self, remote_job: CloudClusterPrintJobStatus) -> UM3PrintJobOutputModel:
-        model = job.createOutputModel(CloudOutputController(self))
+        model = remote_job.createOutputModel(CloudOutputController(self))
         model.stateChanged.connect(self._onPrintJobStateChanged)
-        if job.printer_uuid:
+        if remote_job.printer_uuid:
-            self._updateAssignedPrinter(model, job.printer_uuid)
+            self._updateAssignedPrinter(model, remote_job.printer_uuid)
-        self._print_jobs.append(model)
+        return model
 
     ## Handles the event of a change in a print job state
     def _onPrintJobStateChanged(self) -> None:
@@ -305,13 +313,14 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
             if job.state == "wait_cleanup" and job.key not in self._finished_jobs and job.owner == user_name:
                 self._finished_jobs.add(job.key)
                 Message(
-                    title = I18N_CATALOG.i18nc("@info:status", "Print finished"),
+                    title=I18N_CATALOG.i18nc("@info:status", "Print finished"),
-                    text = (I18N_CATALOG.i18nc("@info:status", "Printer '{printer_name}' has finished printing '{job_name}'.").format(
+                    text=(I18N_CATALOG.i18nc("@info:status",
-                        printer_name = job.assignedPrinter.name,
+                                             "Printer '{printer_name}' has finished printing '{job_name}'.").format(
-                        job_name = job.name
+                        printer_name=job.assignedPrinter.name,
+                        job_name=job.name
                     ) if job.assignedPrinter else
                           I18N_CATALOG.i18nc("@info:status", "The print job '{job_name}' was finished.").format(
-                                job_name = job.name
+                              job_name=job.name
                           )),
                 ).show()
 
@@ -322,7 +331,6 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
             Logger.log("w", "Missing printer %s for job %s in %s", model.assignedPrinter, model.key,
                        [p.key for p in self._printers])
             return
-
         printer.updateActivePrintJob(model)
         model.updateAssignedPrinter(printer)
 
@@ -332,7 +340,8 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         self._progress.show()
         self._uploaded_print_job = job_response
         tool_path = cast(bytes, self._tool_path)
-        self._api.uploadToolPath(job_response, tool_path, self._onPrintJobUploaded, self._progress.update, self._onUploadError)
+        self._api.uploadToolPath(job_response, tool_path, self._onPrintJobUploaded, self._progress.update,
+                                 self._onUploadError)
 
     ## Requests the print to be sent to the printer when we finished uploading the mesh.
     def _onPrintJobUploaded(self) -> None:
@@ -346,9 +355,9 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         self._progress.hide()
         self._uploaded_print_job = None
         Message(
-            text = message or I18N_CATALOG.i18nc("@info:text", "Could not upload the data to the printer."),
+            text=message or I18N_CATALOG.i18nc("@info:text", "Could not upload the data to the printer."),
-            title = I18N_CATALOG.i18nc("@info:title", "Cloud error"),
+            title=I18N_CATALOG.i18nc("@info:title", "Cloud error"),
-            lifetime = 10
+            lifetime=10
         ).show()
         self.writeError.emit()
 
@@ -358,22 +367,24 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         Logger.log("d", "The cluster will be printing this print job with the ID %s", response.cluster_job_id)
         self._progress.hide()
         Message(
-            text = I18N_CATALOG.i18nc("@info:status", "Print job was successfully sent to the printer."),
+            text=I18N_CATALOG.i18nc("@info:status", "Print job was successfully sent to the printer."),
-            title = I18N_CATALOG.i18nc("@info:title", "Data Sent"),
+            title=I18N_CATALOG.i18nc("@info:title", "Data Sent"),
-            lifetime = 5
+            lifetime=5
         ).show()
         self.writeFinished.emit()
 
     ##  Whether the printer that this output device represents supports print job actions via the cloud.
-    @pyqtProperty(bool, notify = _clusterPrintersChanged)
+    @pyqtProperty(bool, notify=_clusterPrintersChanged)
     def supportsPrintJobActions(self) -> bool:
+        if not self._printers:
+            return False
         version_number = self.printers[0].firmwareVersion.split(".")
         firmware_version = Version([version_number[0], version_number[1], version_number[2]])
         return firmware_version >= self.PRINT_JOB_ACTIONS_MIN_VERSION
 
     ##  Gets the number of printers in the cluster.
     #   We use a minimum of 1 because cloud devices are always a cluster and printer discovery needs it.
-    @pyqtProperty(int, notify = _clusterPrintersChanged)
+    @pyqtProperty(int, notify=_clusterPrintersChanged)
     def clusterSize(self) -> int:
         return max(1, len(self._printers))
 
@@ -383,7 +394,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         return self._printers
 
     ##  Get the active printer in the UI (monitor page).
-    @pyqtProperty(QObject, notify = activePrinterChanged)
+    @pyqtProperty(QObject, notify=activePrinterChanged)
     def activePrinter(self) -> Optional[PrinterOutputModel]:
         return self._active_printer
 
@@ -395,18 +406,18 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
             self.activePrinterChanged.emit()
 
     ##  Get remote print jobs.
-    @pyqtProperty("QVariantList", notify = printJobsChanged)
+    @pyqtProperty("QVariantList", notify=printJobsChanged)
     def printJobs(self) -> List[UM3PrintJobOutputModel]:
         return self._print_jobs
 
     ##  Get remote print jobs that are still in the print queue.
-    @pyqtProperty("QVariantList", notify = printJobsChanged)
+    @pyqtProperty("QVariantList", notify=printJobsChanged)
     def queuedPrintJobs(self) -> List[UM3PrintJobOutputModel]:
         return [print_job for print_job in self._print_jobs
                 if print_job.state == "queued" or print_job.state == "error"]
 
     ##  Get remote print jobs that are assigned to a printer.
-    @pyqtProperty("QVariantList", notify = printJobsChanged)
+    @pyqtProperty("QVariantList", notify=printJobsChanged)
     def activePrintJobs(self) -> List[UM3PrintJobOutputModel]:
         return [print_job for print_job in self._print_jobs if
                 print_job.assignedPrinter is not None and print_job.state != "queued"]
@@ -427,15 +438,15 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     def forceSendJob(self, print_job_uuid: str) -> None:
         self._api.doPrintJobAction(self._cluster.cluster_id, print_job_uuid, "force")
 
-    @pyqtSlot(int, result = str)
+    @pyqtSlot(int, result=str)
     def formatDuration(self, seconds: int) -> str:
         return Duration(seconds).getDisplayString(DurationFormat.Format.Short)
 
-    @pyqtSlot(int, result = str)
+    @pyqtSlot(int, result=str)
     def getTimeCompleted(self, time_remaining: int) -> str:
         return formatTimeCompleted(time_remaining)
 
-    @pyqtSlot(int, result = str)
+    @pyqtSlot(int, result=str)
     def getDateCompleted(self, time_remaining: int) -> str:
         return formatDateCompleted(time_remaining)
 
@@ -454,7 +465,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     ##  TODO: The following methods are required by the monitor page QML, but are not actually available using cloud.
     #   TODO: We fake the methods here to not break the monitor page.
 
-    @pyqtProperty(QUrl, notify = _clusterPrintersChanged)
+    @pyqtProperty(QUrl, notify=_clusterPrintersChanged)
     def activeCameraUrl(self) -> "QUrl":
         return QUrl()
 
@@ -462,6 +473,6 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     def setActiveCameraUrl(self, camera_url: "QUrl") -> None:
         pass
 
-    @pyqtProperty("QVariantList", notify = _clusterPrintersChanged)
+    @pyqtProperty("QVariantList", notify=_clusterPrintersChanged)
     def connectedPrintersTypeCount(self) -> List[Dict[str, str]]:
         return []