Browse Source

fix(spool-assignments): union both assignment tables in the missing-spool check + symmetric mode-switch clear (#1473)

  notify_missing_spool_assignments_on_print_start queried only the legacy
  SpoolAssignment table. In Spoolman mode that table is empty -- bindings
  live in spoolman_slot_assignments -- so every used tray was flagged
  missing, firing a false-positive notification on every print.

  - spool_assignment_notifications.py: the assigned-tray set is now the
    union of SpoolAssignment + SpoolmanSlotAssignment rows. Union-only,
    so legacy-mode behavior cannot regress.
  - settings.py: the Spoolman toggle cleared SpoolAssignment on switch-on
    but never cleared SpoolmanSlotAssignment on switch-off. Added the
    symmetric clear so stale Spoolman rows can't leak into a later
    internal-mode session and mask a real missing-assignment warning.

  Adds 3 notification tests + 1 mode-switch integration test. An audit
  of the remaining SpoolAssignment consumers confirmed usage_tracker,
  spool_tag_matcher and routes/inventory are correctly internal-mode-only.
maziggy 6 days ago
parent
commit
b06f8f6951

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 10 - 0
backend/app/api/routes/settings.py

@@ -412,6 +412,16 @@ async def update_spoolman_settings(
 
             result = await db.execute(delete(SpoolAssignment))
             logger.info("Cleared %d spool assignments on switch to Spoolman mode", result.rowcount)
+        # Switching back to internal mode: clear Spoolman slot assignments — the
+        # symmetric counterpart of the clear above. Without this, stale
+        # spoolman_slot_assignments rows linger and would wrongly count as
+        # "assigned" in any mode-agnostic check (e.g. the missing-spool-
+        # assignment notification, which unions both tables — #1473).
+        elif old_val.lower() == "true" and new_val.lower() != "true":
+            from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
+
+            result = await db.execute(delete(SpoolmanSlotAssignment))
+            logger.info("Cleared %d Spoolman slot assignments on switch to internal mode", result.rowcount)
     if "spoolman_url" in settings:
         await set_setting(db, "spoolman_url", settings["spoolman_url"])
     if "spoolman_sync_mode" in settings:

+ 30 - 0
backend/tests/integration/test_spoolman_slot_assignments.py

@@ -577,3 +577,33 @@ class TestCascadeDeletePrinter:
             select(SpoolmanSlotAssignment).where(SpoolmanSlotAssignment.printer_id == test_printer.id)
         )
         assert post.scalars().all() == []
+
+
+class TestModeSwitchClearsAssignments:
+    """#1473 follow-up — the Spoolman mode toggle clears the other mode's
+    slot-assignment table so stale rows can't bleed across a mode switch."""
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_switch_to_internal_mode_clears_spoolman_slot_assignments(
+        self, async_client: AsyncClient, db_session, test_printer
+    ):
+        """Switching Spoolman OFF deletes spoolman_slot_assignments rows — the
+        symmetric counterpart of clearing legacy spool_assignment rows when
+        switching ON. Stale rows would otherwise wrongly count as 'assigned'
+        in mode-agnostic checks (e.g. the missing-spool-assignment notification,
+        which unions both tables)."""
+        from backend.app.models.settings import Settings
+        from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
+
+        db_session.add(Settings(key="spoolman_enabled", value="true"))
+        db_session.add(SpoolmanSlotAssignment(printer_id=test_printer.id, ams_id=0, tray_id=0, spoolman_spool_id=1))
+        await db_session.commit()
+
+        resp = await async_client.put("/api/v1/settings/spoolman", json={"spoolman_enabled": "false"})
+        assert resp.status_code == 200
+
+        rows = await db_session.execute(
+            select(SpoolmanSlotAssignment).where(SpoolmanSlotAssignment.printer_id == test_printer.id)
+        )
+        assert rows.scalars().all() == []

Some files were not shown because too many files changed in this diff