Browse Source

Fix: SpoolBuddy Write-Tag page honours Spoolman mode + complete ID surface (#1439)

  Two bugs surfaced by @flom89 in the same thread:

  (1) The Write-Tag page hardcoded api.getSpools and friends regardless of
  inventory backend. Users in Spoolman mode saw internal spools they never
  created and a successful tag write would have bound the NFC tag to the
  wrong backend (the backend write-tag route is mode-aware; the frontend
  was driving it with the wrong IDs). Fix follows the InventoryPageRouter
  pattern: detect spoolmanMode from getSpoolmanSettings, gate the spool
  fetch on spoolmanModeReady to avoid the initial wrong-backend request,
  and branch all 6 API call sites (list, autocomplete, untag, K-profile
  save, single create, bulk create — the bulk variant returns a different
  envelope shape so the duck-typed check from SpoolFormModal is mirrored).

  (2) The spool ID was missing from three more SpoolBuddy components
  beyond the first round of #1439 work — SpoolInfoCard (the dashboard's
  right-side found-tag panel — the "main screen" view the reporter named),
  InventorySpoolInfoCard, and SpoolBuddyAmsPage's assigned-spool block.
  Same #1385 pattern (#<id> in muted small monospace with shrink-0).
maziggy 1 week ago
parent
commit
a4e8ae3ab4

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


+ 98 - 1
frontend/src/__tests__/pages/SpoolBuddyWriteTagPage.test.tsx

@@ -17,11 +17,19 @@ import { ToastProvider } from '../../contexts/ToastContext';
 import { SpoolBuddyWriteTagPage } from '../../pages/spoolbuddy/SpoolBuddyWriteTagPage';
 import { api as mockedApi, spoolbuddyApi as mockedSpoolbuddyApi } from '../../api/client';
 
-// Mock the API modules
+// Mock the API modules. spoolman-* mocks default to "spoolman disabled" so
+// existing tests run in internal-inventory mode — the new spoolman parity
+// test below overrides getSpoolmanSettings before rendering.
 vi.mock('../../api/client', () => ({
   api: {
     getSpools: vi.fn().mockResolvedValue([]),
     createSpool: vi.fn().mockResolvedValue({ id: 1, material: 'PLA' }),
+    getSpoolmanSettings: vi.fn().mockResolvedValue({
+      spoolman_enabled: 'false',
+      spoolman_url: '',
+    }),
+    getSpoolmanInventorySpools: vi.fn().mockResolvedValue([]),
+    createSpoolmanInventorySpool: vi.fn().mockResolvedValue({ id: 1, material: 'PLA' }),
   },
   spoolbuddyApi: {
     getDevices: vi.fn().mockResolvedValue([]),
@@ -86,6 +94,16 @@ function renderPage() {
 describe('SpoolBuddyWriteTagPage', () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    // Re-pin the default Spoolman-mode settings since previous tests may
+    // have overridden the resolved value (clearAllMocks doesn't reset
+    // implementations set via mockResolvedValue). Default = Spoolman OFF.
+    vi.mocked(mockedApi.getSpoolmanSettings).mockResolvedValue({
+      spoolman_enabled: 'false',
+      spoolman_url: '',
+      spoolman_sync_mode: '',
+      spoolman_disable_weight_sync: '',
+      spoolman_report_partial_usage: '',
+    });
   });
 
   it('renders three workflow tabs', () => {
@@ -183,4 +201,83 @@ describe('SpoolBuddyWriteTagPage', () => {
 
     mockOutletContext.sbState.deviceOnline = false; // reset
   });
+
+  it('reads from internal inventory when Spoolman mode is OFF (#1439 parity baseline)', async () => {
+    vi.mocked(mockedApi.getSpoolmanSettings).mockResolvedValue({
+      spoolman_enabled: 'false',
+      spoolman_url: '',
+      spoolman_sync_mode: '',
+      spoolman_disable_weight_sync: '',
+      spoolman_report_partial_usage: '',
+    });
+    renderPage();
+
+    // Internal API is hit at least once; the Spoolman list endpoint must NOT
+    // be called when the user hasn't enabled Spoolman.
+    await waitFor(() => {
+      expect(vi.mocked(mockedApi.getSpools)).toHaveBeenCalled();
+    });
+    expect(vi.mocked(mockedApi.getSpoolmanInventorySpools)).not.toHaveBeenCalled();
+  });
+
+  it('reads from Spoolman when Spoolman mode is ON (#1439 — the actual bug)', async () => {
+    // The bug report: write-tag page hard-coded api.getSpools(false) regardless
+    // of inventory backend, so Spoolman-mode users saw internal spools they
+    // never created and the write path would have bound the tag to the wrong
+    // backend. After the fix, the page must hit the spoolman variant.
+    vi.mocked(mockedApi.getSpoolmanSettings).mockResolvedValue({
+      spoolman_enabled: 'true',
+      spoolman_url: 'http://spoolman.test',
+      spoolman_sync_mode: '',
+      spoolman_disable_weight_sync: '',
+      spoolman_report_partial_usage: '',
+    });
+    renderPage();
+
+    await waitFor(() => {
+      expect(vi.mocked(mockedApi.getSpoolmanInventorySpools)).toHaveBeenCalled();
+    });
+    // Critical: the internal endpoint must NOT be called in Spoolman mode.
+    // A future refactor that re-hardcodes api.getSpools breaks here.
+    expect(vi.mocked(mockedApi.getSpools)).not.toHaveBeenCalled();
+  });
+
+  it('shows the spool ID next to each spool so multiple identical rolls can be disambiguated (#1439)', async () => {
+    const spools = [
+      {
+        id: 42,
+        material: 'PLA',
+        label_weight: 1000,
+        weight_used: 0,
+        tag_uid: null,
+        tray_uuid: null,
+        archived_at: null,
+        rgba: 'FF0000FF',
+      },
+      {
+        id: 43,
+        material: 'PLA',
+        label_weight: 1000,
+        weight_used: 0,
+        tag_uid: null,
+        tray_uuid: null,
+        archived_at: null,
+        rgba: 'FF0000FF',
+      },
+    ];
+    vi.mocked(mockedApi.getSpools).mockResolvedValue(spools as never);
+    mockOutletContext.sbState.deviceOnline = true;
+    renderPage();
+
+    // Both rolls are identical PLA Red but the IDs must let the user tell
+    // them apart at the picker. Without #1439 the two rows were visually
+    // indistinguishable and an NFC write could bind the tag to the wrong
+    // physical spool.
+    await waitFor(() => {
+      expect(screen.getByText('#42')).toBeDefined();
+      expect(screen.getByText('#43')).toBeDefined();
+    });
+
+    mockOutletContext.sbState.deviceOnline = false;
+  });
 });

+ 1 - 0
frontend/src/components/spoolbuddy/AssignToAmsModal.tsx

@@ -367,6 +367,7 @@ export function AssignToAmsModal({ isOpen, onClose, spool, printerId, spoolmanMo
               <span className="font-normal text-zinc-500 ml-2">
                 {spool.color_name || 'Unknown'} &bull; {spool.brand} {spool.material}{spool.subtype && ` ${spool.subtype}`}
               </span>
+              <span className="text-[10px] font-mono text-zinc-500 ml-2 shrink-0">#{spool.id}</span>
             </h2>
           </div>
         </div>

+ 6 - 3
frontend/src/components/spoolbuddy/InventorySpoolInfoCard.tsx

@@ -149,9 +149,12 @@ export function InventorySpoolInfoCard({
         </div>
 
         <div className="flex-1 min-w-0 pt-1">
-          <h3 className="text-lg font-semibold text-zinc-100">
-            {spool.color_name || 'Unknown color'}
-          </h3>
+          <div className="flex items-center gap-2">
+            <h3 className="text-lg font-semibold text-zinc-100">
+              {spool.color_name || 'Unknown color'}
+            </h3>
+            <span className="text-xs font-mono text-zinc-500 shrink-0">#{spool.id}</span>
+          </div>
           <p className="text-sm text-zinc-400">
             {spool.brand} &bull; {spool.material}
             {spool.subtype && ` ${spool.subtype}`}

+ 5 - 2
frontend/src/components/spoolbuddy/LinkSpoolModal.tsx

@@ -105,8 +105,11 @@ export function LinkSpoolModal({
                     size={40}
                   />
                   <div className="flex-1 min-w-0">
-                    <div className="font-medium text-zinc-100 truncate">
-                      {spool.color_name || 'Unknown color'}
+                    <div className="flex items-center gap-2">
+                      <div className="font-medium text-zinc-100 truncate">
+                        {spool.color_name || 'Unknown color'}
+                      </div>
+                      <span className="text-[10px] font-mono text-zinc-500 shrink-0">#{spool.id}</span>
                     </div>
                     <div className="text-sm text-zinc-400 truncate">
                       {spool.brand} &bull; {spool.material}

+ 6 - 3
frontend/src/components/spoolbuddy/SpoolInfoCard.tsx

@@ -101,9 +101,12 @@ export function SpoolInfoCard({ spool, scaleWeight, onClose, onSyncWeight, onAss
 
         {/* Main info */}
         <div className="flex-1 min-w-0 pt-1">
-          <h3 className="text-lg font-semibold text-zinc-100">
-            {spool.color_name || 'Unknown color'}
-          </h3>
+          <div className="flex items-center gap-2">
+            <h3 className="text-lg font-semibold text-zinc-100">
+              {spool.color_name || 'Unknown color'}
+            </h3>
+            <span className="text-xs font-mono text-zinc-500 shrink-0">#{spool.id}</span>
+          </div>
           <p className="text-sm text-zinc-400">
             {spool.brand} &bull; {spool.material}
             {spool.subtype && ` ${spool.subtype}`}

+ 6 - 3
frontend/src/components/spoolbuddy/TagDetectedModal.tsx

@@ -187,9 +187,12 @@ function KnownSpoolView({ spool, scaleWeight, weightStable, syncing, synced, onS
         </div>
 
         <div className="flex-1 min-w-0 pt-1">
-          <h3 className="text-lg font-semibold text-zinc-100">
-            {spool.color_name || 'Unknown color'}
-          </h3>
+          <div className="flex items-center gap-2">
+            <h3 className="text-lg font-semibold text-zinc-100">
+              {spool.color_name || 'Unknown color'}
+            </h3>
+            <span className="text-xs font-mono text-zinc-500 shrink-0">#{spool.id}</span>
+          </div>
           <p className="text-sm text-zinc-400">
             {spool.brand} &bull; {spool.material}
             {spool.subtype && ` ${spool.subtype}`}

+ 1 - 0
frontend/src/pages/spoolbuddy/SpoolBuddyAmsPage.tsx

@@ -753,6 +753,7 @@ export function SpoolBuddyAmsPage() {
                         {assignment.spool.brand ? `${assignment.spool.brand} ` : ''}{assignment.spool.material}
                         {assignment.spool.color_name ? ` - ${assignment.spool.color_name}` : ''}
                       </span>
+                      <span className="text-[10px] font-mono text-zinc-500 shrink-0 ml-auto">#{assignment.spool.id}</span>
                     </div>
                   </div>
                 )}

+ 58 - 14
frontend/src/pages/spoolbuddy/SpoolBuddyWriteTagPage.tsx

@@ -48,11 +48,30 @@ export function SpoolBuddyWriteTagPage() {
   const [tagOnReader, setTagOnReader] = useState(false);
   const [tagUid, setTagUid] = useState<string | null>(null);
 
+  // Detect Spoolman mode — when the user has Spoolman as their inventory
+  // backend, every read/write on this page must go to /spoolman/inventory/*
+  // routes instead of /inventory/*. Without this branching, the picker
+  // shows internal spools (which the user never created in Spoolman mode)
+  // and the write-tag write path lands at the wrong backend (#1439).
+  const { data: spoolmanSettings } = useQuery({
+    queryKey: ['spoolman-settings'],
+    queryFn: api.getSpoolmanSettings,
+    staleTime: 5 * 60 * 1000,
+  });
+  const spoolmanModeReady = spoolmanSettings !== undefined;
+  const spoolmanMode =
+    spoolmanSettings?.spoolman_enabled === 'true' && !!spoolmanSettings?.spoolman_url;
 
   const { data: spools = [], refetch: refetchSpools } = useQuery({
-    queryKey: ['inventory-spools'],
-    queryFn: () => api.getSpools(false),
+    queryKey: [spoolmanMode ? 'spoolman-inventory-spools' : 'inventory-spools', 'write-tag'],
+    queryFn: () =>
+      spoolmanMode ? api.getSpoolmanInventorySpools(false) : api.getSpools(false),
     refetchInterval: 10000,
+    // Wait until we know which backend to talk to — otherwise the first
+    // render fires getSpools (default spoolmanMode=false) and getSpoolman*
+    // (after settings resolve), wasting one request and briefly showing
+    // the wrong inventory.
+    enabled: spoolmanModeReady,
   });
 
   const { data: devices = [] } = useQuery({
@@ -193,11 +212,19 @@ export function SpoolBuddyWriteTagPage() {
     setWriteStatus('idle');
     setWriteMessage('');
     try {
-      await api.linkTagToSpool(selectedSpool.id, {
-        tag_uid: '',
-        tray_uuid: '',
-        data_origin: 'manual',
-      });
+      if (spoolmanMode) {
+        // Spoolman variant doesn't accept data_origin (managed Spoolman-side).
+        await api.linkTagToSpoolmanSpool(selectedSpool.id, {
+          tag_uid: '',
+          tray_uuid: '',
+        });
+      } else {
+        await api.linkTagToSpool(selectedSpool.id, {
+          tag_uid: '',
+          tray_uuid: '',
+          data_origin: 'manual',
+        });
+      }
       await refetchSpools();
       setSelectedSpool(null);
       setWriteStatus('success');
@@ -255,6 +282,7 @@ export function SpoolBuddyWriteTagPage() {
               currencySymbol={currencySymbol}
               onCreated={handleSpoolCreated}
               selectedSpool={selectedSpool}
+              spoolmanMode={spoolmanMode}
               t={t}
             />
           ) : (
@@ -359,6 +387,7 @@ function SpoolListItem({ spool, selected, showTag, onClick }: {
           <span className="text-sm font-medium text-white truncate">
             {spool.brand ? `${spool.brand} ` : ''}{spool.material}{spool.subtype ? ` ${spool.subtype}` : ''}
           </span>
+          <span className="text-[10px] font-mono text-zinc-500 shrink-0">#{spool.id}</span>
         </div>
         <div className="flex items-center gap-2 text-xs text-zinc-400">
           {spool.color_name && <span>{spool.color_name}</span>}
@@ -383,17 +412,20 @@ type NewSpoolSubTab = 'filament' | 'pa-profile';
 type NewSpoolViewMode = 'simple' | 'full';
 
 // --- New spool touch form (mirrors Add Spool fields/options in kiosk-friendly layout) ---
-function NewSpoolTouchForm({ currencySymbol, onCreated, selectedSpool, t }: {
+function NewSpoolTouchForm({ currencySymbol, onCreated, selectedSpool, spoolmanMode, t }: {
   currencySymbol: string;
   onCreated: (spool: InventorySpool) => void;
   selectedSpool: InventorySpool | null;
+  spoolmanMode: boolean;
   t: (key: string, fallback: string) => string;
 }) {
   // Read inventory + settings from the shared react-query cache to drive the
   // category autocomplete and low-stock-threshold placeholder. #729
+  // Spoolman-mode branched: see comment on the parent page's spools query.
   const { data: allSpoolsForForm = [] } = useQuery({
-    queryKey: ['inventory-spools'],
-    queryFn: () => api.getSpools(true),
+    queryKey: [spoolmanMode ? 'spoolman-inventory-spools' : 'inventory-spools', 'write-tag-form'],
+    queryFn: () =>
+      spoolmanMode ? api.getSpoolmanInventorySpools(true) : api.getSpools(true),
   });
   const { data: settingsForForm } = useQuery({
     queryKey: ['settings'],
@@ -593,9 +625,10 @@ function NewSpoolTouchForm({ currencySymbol, onCreated, selectedSpool, t }: {
   };
 
   const saveKProfiles = async (spoolId: number) => {
+    const save = spoolmanMode ? api.saveSpoolmanKProfiles : api.saveSpoolKProfiles;
     if (selectedProfiles.size === 0) {
       try {
-        await api.saveSpoolKProfiles(spoolId, []);
+        await save(spoolId, []);
       } catch {
         // ignore
       }
@@ -627,7 +660,7 @@ function NewSpoolTouchForm({ currencySymbol, onCreated, selectedSpool, t }: {
     }
 
     if (profiles.length > 0) {
-      await api.saveSpoolKProfiles(spoolId, profiles);
+      await save(spoolId, profiles);
     }
   };
 
@@ -675,13 +708,24 @@ function NewSpoolTouchForm({ currencySymbol, onCreated, selectedSpool, t }: {
     setCreating(true);
     try {
       if (quantity > 1) {
-        const created = await api.bulkCreateSpools(payload, quantity);
+        // Spoolman bulk returns SpoolmanBulkCreateResult (207-style envelope);
+        // internal bulk returns InventorySpool[]. Mirrors SpoolFormModal's
+        // duck-typed handling so partial failures surface as a warning toast.
+        const raw = spoolmanMode
+          ? await api.bulkCreateSpoolmanInventorySpools(payload, quantity)
+          : await api.bulkCreateSpools(payload, quantity);
+        const created: InventorySpool[] =
+          spoolmanMode && raw && typeof raw === 'object' && 'created' in raw
+            ? (raw as { created: InventorySpool[] }).created
+            : (raw as InventorySpool[]);
         for (const spool of created) {
           await saveKProfiles(spool.id);
         }
         if (created.length > 0) onCreated(created[0]);
       } else {
-        const created = await api.createSpool(payload);
+        const created = spoolmanMode
+          ? await api.createSpoolmanInventorySpool(payload)
+          : await api.createSpool(payload);
         await saveKProfiles(created.id);
         onCreated(created);
       }

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-HpdkPzCi.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-BRJQNS0m.js"></script>
+    <script type="module" crossorigin src="/assets/index-HpdkPzCi.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-KYwGxnG9.css">
   </head>
   <body>

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