GH-37 | Fix selecting spool by ID (#40)

* GH-37 | Ensure we're always selecting spools by id, using proper types * GH-37 | onAfterTabChange cannot be async, so wrap async execution into IIFE * GH-37 | Document the race condition in onAfterTabChange
mdziekon · Jul 13, 2023 · b709b7e · b709b7e
1 parent af70e9c
commit b709b7e
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 29 deletions.
diff --git a/octoprint_SpoolManager/DatabaseManager.py b/octoprint_SpoolManager/DatabaseManager.py
@@ -881,7 +881,7 @@ def databaseCallMethode():
 
 	def loadSpool(self, databaseId, withReusedConnection=False):
 		def databaseCallMethode():
-			return SpoolModel.get_or_none(databaseId)
+			return SpoolModel.get_or_none(SpoolModel.databaseId == int(databaseId))
 
 		return self._handleReusableConnection(databaseCallMethode, withReusedConnection, "loadSpool")
 

diff --git a/octoprint_SpoolManager/static/js/SpoolManager.js b/octoprint_SpoolManager/static/js/SpoolManager.js
@@ -1217,7 +1217,7 @@ $(function() {
             }
         }
 
-        self.onAfterTabChange = async function(current, previous) {
+        self.onAfterTabChange = function(current, previous) {
             const tabHashCode = window.location.hash;
             // QR-Code-Call: We can only contain -spoolId on the very first page
             if (!tabHashCode.includes("#tab_plugin_SpoolManager-spoolId")) {
@@ -1228,6 +1228,13 @@ $(function() {
             selectedSpoolId = parseInt(selectedSpoolId);
             console.info('Loading spool: '+selectedSpoolId);
 
+            // Note: Due to OctoPrint initialization race condition
+            // (onAfterTabChange called before spools are loaded),
+            // in most cases this comparison does not work correctly.
+            // This code path is triggered only when selecting spool by QR Code via API,
+            // where the API call already selects the spool for us.
+            // TODO: Remove spool selection call from here, and instead simply show
+            // the spool edit modal. To achieve that, a "getSpoolById" API call should be added.
             const spoolCurrentToolId = self.getSpoolItemSelectedTool(selectedSpoolId);
             if (spoolCurrentToolId !== null) {
                 alert('This spool is already selected for tool ' + spoolCurrentToolId + '!');
@@ -1237,40 +1244,46 @@ $(function() {
                 // not doing this while printing
                 return;
             }
-            // - Load SpoolItem from Backend
-            // - Open SpoolItem
-            const commitCurrentSpoolValues = false;
 
-            const queryResult = await self.apiClient.callSelectSpool({
-                toolIndex: 0,
-                spoolDbId: selectedSpoolId,
-                shouldCommitCurrentSpoolProgress: commitCurrentSpoolValues,
-            });
+            // Note: ViewModel callbacks like `onAfterTabChange` cannot be async
+            // since OctoPrint does not recognize async function as callable functions.
+            // Therefore, wrap async execution into an IIFE to leverage async/await syntax.
+            (async () => {
+                // - Load SpoolItem from Backend
+                // - Open SpoolItem
+                const commitCurrentSpoolValues = false;
 
-            if (!queryResult.isSuccess) {
-                return self.showPopUp(
-                    "error",
-                    'Switch spool',
-                    'An unknown error occurred while requesting data',
-                    true,
-                );
-            }
+                const queryResult = await self.apiClient.callSelectSpool({
+                    toolIndex: 0,
+                    spoolDbId: selectedSpoolId,
+                    shouldCommitCurrentSpoolProgress: commitCurrentSpoolValues,
+                });
 
-            const responseData = queryResult.payload.response;
-            const spoolData = responseData.selectedSpool;
+                if (!queryResult.isSuccess) {
+                    return self.showPopUp(
+                        "error",
+                        'Switch spool',
+                        'An unknown error occurred while requesting data',
+                        true,
+                    );
+                }
 
-            // Select the SpoolManager tab
-            $('a[href="#tab_plugin_SpoolManager"]').tab('show');
+                const responseData = queryResult.payload.response;
+                const spoolData = responseData.selectedSpool;
 
-            if (spoolData == null) {
-                return;
-            }
+                // Select the SpoolManager tab
+                $('a[href="#tab_plugin_SpoolManager"]').tab('show');
+
+                if (spoolData == null) {
+                    return;
+                }
 
-            const spoolItem = self.spoolDialog.createSpoolItemForTable(spoolData);
+                const spoolItem = self.spoolDialog.createSpoolItemForTable(spoolData);
 
-            spoolItem.selectedFromQRCode(true);
-            self.selectedSpoolsForSidebar()[0](spoolItem);
-            self.showSpoolDialogAction(spoolItem);
+                spoolItem.selectedFromQRCode(true);
+                self.selectedSpoolsForSidebar()[0](spoolItem);
+                self.showSpoolDialogAction(spoolItem);
+            })();
         }
     }