Просмотр исходного кода

Remember the Printers page's status and location filters (#2833)

Pick a location, navigate away, come back, and every printer was showing
again. Both filters were plain useState, and the only preferences on that
page that were not remembered -- sort order, card size, view mode,
collapsed sections and hide-disconnected all persist, each with the same
initializer-plus-setItem shape. Give these two the same treatment.

A saved filter needs a way out, though. The location dropdown is only
rendered while at least one printer has a location, so a saved location
that was later renamed or removed would match nothing and take its own
dropdown off screen with it -- an empty page and no control to undo it.
A location that is not among the available ones now resets to all, and
the same for a status the dropdown does not offer.

That check waits for the printers query to resolve. The list is undefined
while it is in flight, so the available locations start out empty, and
acting on that would throw the saved filter away on every page load.

Search stays unpersisted: a box that silently refills itself on return is
a surprise rather than a convenience.
maziggy 3 недель назад
Родитель
Сommit
8fe93169ca

Разница между файлами не показана из-за своего большого размера
+ 1 - 0
CHANGELOG.md


+ 229 - 0
frontend/src/__tests__/pages/PrintersPageFilterPersistence.test.tsx

@@ -0,0 +1,229 @@
+/**
+ * The Printers page's status and location filters survive navigation (#2833).
+ *
+ * Every other preference on that page already persisted -- sort order, card
+ * size, view mode, collapsed sections, hide-disconnected -- while these two
+ * were plain useState, so picking a location and coming back showed everything
+ * again.
+ *
+ * Persisting a filter needs a way out, though: the location dropdown is only
+ * rendered while some printer has a location, so a saved value that no longer
+ * matches anything would hide every printer *and* the control that undoes it.
+ */
+
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { render } from '../utils';
+import { PrintersPage } from '../../pages/PrintersPage';
+import { http, HttpResponse } from 'msw';
+import { server } from '../mocks/server';
+
+const basePrinter = {
+  ip_address: '192.168.1.100',
+  access_code: '12345678',
+  enabled: true,
+  is_active: true,
+  nozzle_diameter: 0.4,
+  nozzle_type: 'hardened_steel',
+  auto_archive: true,
+  created_at: '2024-01-01T00:00:00Z',
+  updated_at: '2024-01-01T00:00:00Z',
+};
+
+const workshopPrinter = {
+  ...basePrinter,
+  id: 1,
+  name: 'X1 Carbon',
+  serial_number: '00M09A350100001',
+  model: 'X1C',
+  location: 'Workshop',
+};
+
+const officePrinter = {
+  ...basePrinter,
+  id: 2,
+  name: 'P1S Backup',
+  serial_number: '00W00A123456789',
+  model: 'P1S',
+  location: 'Office',
+};
+
+const mockStatus = {
+  connected: true,
+  state: 'IDLE',
+  awaiting_plate_clear: false,
+  progress: 0,
+  layer_num: 0,
+  total_layers: 0,
+  temperatures: { nozzle: 25, bed: 25, chamber: 25 },
+  remaining_time: 0,
+  filename: null,
+  wifi_signal: -50,
+  vt_tray: [],
+};
+
+const mockApi = (printers: unknown[]) => {
+  server.use(
+    http.get('/api/v1/printers/', () => HttpResponse.json(printers)),
+    http.get('/api/v1/printers/:id/status', () => HttpResponse.json(mockStatus)),
+    http.get('/api/v1/settings/ui-preferences', () => HttpResponse.json({})),
+    http.get('/api/v1/queue/', () => HttpResponse.json([]))
+  );
+};
+
+/**
+ * The shared setup replaces localStorage with bare vi.fn()s that store nothing,
+ * so a test written against the real API asserts against a stub that always
+ * answers undefined -- and every "it remembered" assertion passes for the wrong
+ * reason. Give this file a store that actually holds what it is given.
+ */
+const store = new Map<string, string>();
+
+const installMemoryLocalStorage = () => {
+  store.clear();
+  vi.mocked(localStorage.getItem).mockImplementation((key: string) => store.get(key) ?? null);
+  vi.mocked(localStorage.setItem).mockImplementation((key: string, value: string) => {
+    store.set(key, String(value));
+  });
+  vi.mocked(localStorage.removeItem).mockImplementation((key: string) => {
+    store.delete(key);
+  });
+};
+
+const pickFromDropdown = async (current: RegExp, option: RegExp) => {
+  const user = userEvent.setup();
+  await user.click(await screen.findByRole('button', { name: current }));
+  await user.click(await screen.findByRole('button', { name: option }));
+};
+
+describe('PrintersPage filter persistence', () => {
+  beforeEach(() => {
+    installMemoryLocalStorage();
+  });
+
+  afterEach(() => {
+    store.clear();
+    vi.mocked(localStorage.getItem).mockReset();
+    vi.mocked(localStorage.setItem).mockReset();
+    vi.mocked(localStorage.removeItem).mockReset();
+  });
+
+  describe('location', () => {
+    it('remembers the chosen location', async () => {
+      mockApi([workshopPrinter, officePrinter]);
+      render(<PrintersPage />);
+      await screen.findByText('X1 Carbon');
+
+      await pickFromDropdown(/All Locations/i, /^Workshop$/);
+
+      await waitFor(() => {
+        expect(localStorage.getItem('printerLocationFilter')).toBe('Workshop');
+      });
+    });
+
+    it('applies the remembered location on a fresh mount', async () => {
+      localStorage.setItem('printerLocationFilter', 'Workshop');
+      mockApi([workshopPrinter, officePrinter]);
+
+      render(<PrintersPage />);
+
+      await screen.findByText('X1 Carbon');
+      expect(screen.queryByText('P1S Backup')).not.toBeInTheDocument();
+    });
+
+    it('survives the printers query resolving', async () => {
+      // The list is undefined while the query is in flight, so the available
+      // locations start out empty. Reacting to that would drop the saved
+      // filter on every single page load.
+      localStorage.setItem('printerLocationFilter', 'Workshop');
+      mockApi([workshopPrinter, officePrinter]);
+
+      render(<PrintersPage />);
+      await screen.findByText('X1 Carbon');
+
+      await waitFor(() => {
+        expect(localStorage.getItem('printerLocationFilter')).toBe('Workshop');
+      });
+    });
+
+    it('falls back to all when the saved location no longer exists', async () => {
+      // Renamed, cleared, or its last printer deleted. The dropdown is gone
+      // with it, so leaving the filter set would empty the page for good.
+      localStorage.setItem('printerLocationFilter', 'Workshop');
+      mockApi([{ ...officePrinter, id: 3, name: 'Only Printer' }]);
+
+      render(<PrintersPage />);
+
+      await screen.findByText('Only Printer');
+      await waitFor(() => {
+        expect(localStorage.getItem('printerLocationFilter')).toBe('all');
+      });
+    });
+
+    it('falls back to all when no printer has a location at all', async () => {
+      localStorage.setItem('printerLocationFilter', 'Workshop');
+      mockApi([{ ...workshopPrinter, location: null }]);
+
+      render(<PrintersPage />);
+
+      await screen.findByText('X1 Carbon');
+      await waitFor(() => {
+        expect(localStorage.getItem('printerLocationFilter')).toBe('all');
+      });
+    });
+  });
+
+  describe('status', () => {
+    it('remembers the chosen status', async () => {
+      mockApi([workshopPrinter, officePrinter]);
+      render(<PrintersPage />);
+      await screen.findByText('X1 Carbon');
+
+      await pickFromDropdown(/All Statuses/i, /^Idle$/i);
+
+      await waitFor(() => {
+        expect(localStorage.getItem('printerStatusFilter')).toBe('idle');
+      });
+    });
+
+    it('applies the remembered status on a fresh mount', async () => {
+      localStorage.setItem('printerStatusFilter', 'printing');
+      mockApi([workshopPrinter]);
+
+      render(<PrintersPage />);
+
+      // Both printers report IDLE, so a printing filter matches none of them.
+      await waitFor(() => {
+        expect(screen.queryByText('X1 Carbon')).not.toBeInTheDocument();
+      });
+    });
+
+    it('ignores a saved status the dropdown does not offer', async () => {
+      // A value from an older or newer build would otherwise match nothing,
+      // with nothing on screen to explain it.
+      localStorage.setItem('printerStatusFilter', 'teleporting');
+      mockApi([workshopPrinter]);
+
+      render(<PrintersPage />);
+
+      await screen.findByText('X1 Carbon');
+    });
+  });
+
+  describe('search is deliberately not persisted', () => {
+    it('does not save what was typed', async () => {
+      mockApi([workshopPrinter, officePrinter]);
+      render(<PrintersPage />);
+      await screen.findByText('X1 Carbon');
+
+      const user = userEvent.setup();
+      await user.type(screen.getByPlaceholderText(/search/i), 'Carbon');
+
+      await waitFor(() => {
+        expect(screen.queryByText('P1S Backup')).not.toBeInTheDocument();
+      });
+      expect([...store.keys()].some(key => key.toLowerCase().includes('search'))).toBe(false);
+    });
+  });
+});

+ 55 - 13
frontend/src/pages/PrintersPage.tsx

@@ -134,6 +134,23 @@ import { Collapsible } from '../components/Collapsible';
 import { ConnectionDiagnosticModal, DiagnosticChecklist } from '../components/ConnectionDiagnostic';
 import { getColorName, parseFilamentColor, isLightColor } from '../utils/colors';
 
+// The status filter's options, and the only values it may hold. One list so a
+// saved filter cannot be validated against a set the dropdown has since moved
+// on from (#2833). Labels stay literal so they remain greppable.
+const STATUS_FILTER_OPTIONS = [
+  { value: 'all', labelKey: 'printers.filter.allStatuses' },
+  { value: 'printing', labelKey: 'printers.status.printing' },
+  { value: 'paused', labelKey: 'printers.status.paused' },
+  { value: 'idle', labelKey: 'printers.status.idle' },
+  { value: 'finished', labelKey: 'printers.status.finished' },
+  { value: 'error', labelKey: 'printers.status.error' },
+  { value: 'offline', labelKey: 'printers.status.offline' },
+] as const;
+
+function isKnownStatusFilter(value: string | null): boolean {
+  return STATUS_FILTER_OPTIONS.some(option => option.value === value);
+}
+
 export interface SpoolmanSlotAssignmentRow {
   printer_id: number;
   ams_id: number;
@@ -8126,8 +8143,18 @@ export function PrintersPage() {
     }
   }, [compactDrilldownPrinterId, scrollPrinterIntoView]);
   const [search, setSearch] = useState('');
-  const [statusFilter, setStatusFilter] = useState<string>('all');
-  const [locationFilter, setLocationFilter] = useState<string>('all');
+  // Both filters persist like every other preference on this page (#2833).
+  // `search` deliberately does not: a box that silently refills itself on
+  // return is more surprising than a dropdown that remembers.
+  const [statusFilter, setStatusFilter] = useState<string>(() => {
+    // Validated on read: a value the dropdown no longer offers would filter
+    // every printer out, and nothing would explain why.
+    const saved = localStorage.getItem('printerStatusFilter');
+    return isKnownStatusFilter(saved) ? (saved as string) : 'all';
+  });
+  const [locationFilter, setLocationFilter] = useState<string>(
+    () => localStorage.getItem('printerLocationFilter') || 'all'
+  );
   const [statusCacheVersion, setStatusCacheVersion] = useState(0);
   const [collapsedSections, setCollapsedSections] = useState<Record<string, boolean>>(() => {
     try {
@@ -8486,6 +8513,16 @@ export function PrintersPage() {
     localStorage.setItem('printerSortBy', newSort);
   };
 
+  const handleStatusFilterChange = (value: string) => {
+    setStatusFilter(value);
+    localStorage.setItem('printerStatusFilter', value);
+  };
+
+  const handleLocationFilterChange = (value: string) => {
+    setLocationFilter(value);
+    localStorage.setItem('printerLocationFilter', value);
+  };
+
   const toggleSortDirection = () => {
     const newAsc = !sortAsc;
     setSortAsc(newAsc);
@@ -8569,6 +8606,19 @@ export function PrintersPage() {
     return [...new Set(printers.map(p => p.location || '').filter(Boolean))].sort();
   }, [printers]);
 
+  // A saved location can outlive what it matched -- renamed, cleared, or the
+  // only printer that had it deleted. The dropdown is rendered only while some
+  // printer has a location, so a stale one would hide every printer *and* the
+  // control to undo it, leaving an empty page and no way back (#2833).
+  // Deliberately waits for `printers`: it is undefined while the query is in
+  // flight, and acting on the empty list that produces would throw the saved
+  // filter away every time the page loads.
+  useEffect(() => {
+    if (!printers || locationFilter === 'all' || availableLocations.includes(locationFilter)) return;
+    setLocationFilter('all');
+    localStorage.setItem('printerLocationFilter', 'all');
+  }, [printers, availableLocations, locationFilter]);
+
   // Sort printers based on selected option
   const sortedPrinters = useMemo(() => {
     const sorted = [...filteredPrinters];
@@ -8767,17 +8817,9 @@ export function PrintersPage() {
       {printers && printers.length > 0 && (
         <ToolbarDropdown
           value={statusFilter}
-          onChange={setStatusFilter}
+          onChange={handleStatusFilterChange}
           fullWidth={inMenu}
-          options={[
-            { value: 'all', label: t('printers.filter.allStatuses') },
-            { value: 'printing', label: t('printers.status.printing') },
-            { value: 'paused', label: t('printers.status.paused') },
-            { value: 'idle', label: t('printers.status.idle') },
-            { value: 'finished', label: t('printers.status.finished') },
-            { value: 'error', label: t('printers.status.error') },
-            { value: 'offline', label: t('printers.status.offline') },
-          ]}
+          options={STATUS_FILTER_OPTIONS.map(option => ({ value: option.value, label: t(option.labelKey) }))}
         />
       )}
 
@@ -8785,7 +8827,7 @@ export function PrintersPage() {
       {printers && printers.length > 0 && availableLocations.length > 0 && (
         <ToolbarDropdown
           value={locationFilter}
-          onChange={setLocationFilter}
+          onChange={handleLocationFilterChange}
           fullWidth={inMenu}
           options={[
             { value: 'all', label: t('printers.filter.allLocations') },

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
static/assets/index-xpKc9yRt.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-Cin_PtM9.js"></script>
+    <script type="module" crossorigin src="/assets/index-xpKc9yRt.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-1Ya6fAmN.css">
   </head>
   <body>

Некоторые файлы не были показаны из-за большого количества измененных файлов