mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
fix(spoolbuddy): inventory search matches spool ID + storage location (#1738)
The SpoolBuddy inventory page reimplemented its filter inline and only
matched material/subtype/brand/color_name/note, while Bambuddy's main
inventory uses the shared filterSpoolsByQuery helper which also matches
spool ID, slicer_filament_name, and storage_location. Delegate to the
shared helper so both pages stay in lockstep.
- frontend/src/pages/spoolbuddy/SpoolBuddyInventoryPage.tsx: replace
inline filter with filterSpoolsByQuery
- frontend/src/__tests__/pages/SpoolBuddyInventorySearch.test.ts: lock
in ID / partial ID / pre-fix fields / parity-gain fields
This commit is contained in:
@@ -5,6 +5,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
## [0.2.4.7] - 2026-06-14
|
||||
|
||||
### Fixed
|
||||
- **SpoolBuddy inventory search now matches spool ID, slicer filament name, and storage location (#1738, reported by @shaddowlink)** — The reporter found that typing a numeric spool ID into SpoolBuddy → Inventory's search box returned no results, even though the same query in Bambuddy's main Inventory page worked. Root cause: `frontend/src/pages/spoolbuddy/SpoolBuddyInventoryPage.tsx:147-155` reimplemented the search filter inline and only matched `material`, `subtype`, `brand`, `color_name`, and `note`. The main Inventory page delegates to the shared `filterSpoolsByQuery` helper in `frontend/src/utils/inventorySearch.ts:7`, which additionally matches `String(spool.id)`, `slicer_filament_name`, and `storage_location`. SpoolBuddy had diverged. **Fix:** replace the inline filter with a single call to `filterSpoolsByQuery(list, searchQuery.trim())`. Both inventory modes (internal via `getSpools`, Spoolman via `getSpoolmanInventorySpools`) return the same `InventorySpool` shape, so this covers both paths in one drop. SpoolBuddy now matches Bambuddy's search behaviour across all eight fields. **Tests:** new `SpoolBuddyInventorySearch.test.ts` with 4 cases pinning the parity — exact spool ID match, partial spool ID match, the five pre-fix fields still match, and the three newly-included fields (storage_location, slicer_filament_name, plus implicit id) match. Existing `inventorySearch.test.ts` ID matching test (#1336) still green. ESLint clean; `npm run build` clean. No backend change, no i18n, no new permission.
|
||||
|
||||
- **Sidebar entries for Files / Archives / Queue no longer hide from non-admin users with granular read access (#1755, reported by @knifesk)** — The reporter noticed the **File Manager** sidebar entry was hidden for a default Operators user even though the same user could load `/files` directly and the backend API accepted their requests. Root cause is broader than reported: `frontend/src/components/Layout.tsx::navPermissions` mapped `files → 'library:read'`, `archives → 'archives:read'`, `queue → 'queue:read'` — the LEGACY permission flags — but the default Operators group at `backend/app/core/permissions.py:368-380` is seeded with the GRANULAR variants only (`ARCHIVES_READ_OWN.value`, `QUEUE_READ_OWN.value`, `LIBRARY_READ_OWN.value`). The migration path at `backend/app/core/database.py:3034-3041` also flips legacy `*:read` → `*:read_own` on existing non-admin groups. So a non-admin user never holds the legacy permission, `hasPermission('library:read')` returns false, sidebar entry is suppressed — for all three resources, not just Files. Admins get `ALL_PERMISSIONS` which includes the legacy variant, so the sidebar always renders for them, which is why this regression went unnoticed until a real non-admin Operator account landed in #1755. **Fix:** `navPermissions` now accepts `Permission | Permission[]` and the three affected resources list all three tiers (`*:read`, `*:read_own`, `*:read_all`). The `isHidden` check switches on the array type — `some(hasPermission)` for arrays, current behavior for single values. Nothing else in the gate logic changed. `frontend/src/api/client.ts` Permission type extended with the missing granular variants (`archives:read_own`, `archives:read_all`, `queue:read_own`, `queue:read_all`, `library:read_own`, `library:read_all`) — these existed in the backend enum and were already being shipped to the frontend in `/auth/me`, but the TS type didn't declare them so any new code wanting to gate on the granular tier would TypeScript-error. **What this also fixes downstream:** any future feature that needs to gate UI on `*:read_own` / `*:read_all` can now do so without re-adding the same type entries. **Tests:** 5 new cases in `Layout.test.tsx::'Sidebar gate accepts granular read tiers (#1755)'` — Files visible with only `library:read_own`, Files visible with only `library:read_all`, Archives visible with only `archives:read_own`, Queue visible with only `queue:read_own`, and the negative case (`printers:read` only — none of Files / Archives / Queue render). 22/22 Layout vitests green; ESLint clean; `npm run build` clean. No backend change, no DB migration, no new i18n keys. No new permission — just unmasks UI for users who already had backend access.
|
||||
|
||||
- **Push notification for "Printer offline" now actually fires (#1752, reported by @saint-hh)** — The notification provider's `on_printer_offline` toggle has shipped since the notifications feature landed: schema field, DB column, `notification_template.py` entry, and the dispatcher `NotificationService.on_printer_offline(printer_id, printer_name, db)` are all in place. What was missing was the caller — nothing in the codebase actually invoked the dispatcher when a printer went offline. The reporter (P2S, smart-plug-cuts-power scenario) confirmed turning the toggle on did nothing; only the print-failure notification fired when power was restored, via the firmware's `gcode_state=FAILED` report on MQTT reconnect. **Why the toggle was orphan:** every other provider event (`on_print_start`, `on_print_complete`, `on_print_progress`, `on_printer_error`, etc.) has a clear call site under `main.py::on_printer_status_change` or alongside the print-lifecycle hooks. The offline event was the only edge-triggered toggle without one — the dispatcher and template predated the wiring step and were silently shipped. Both upstream offline-trigger paths (`smart_plug_manager` → `printer_manager.mark_printer_offline()` and `bambu_mqtt.py::check_staleness` after the 30s STALE_RECONNECT_COOLDOWN) route through `_on_status_change` already and reach `on_printer_status_change`; the handler just didn't act on the disconnect edge. **Fix:** edge detection in `on_printer_status_change` watches `state.connected` against the previous observation per printer (`_printer_last_connected: dict[int, bool]`). On the True → False transition it schedules `_maybe_notify_printer_offline(printer_id)` as a background asyncio task; on the next True observation it cancels any pending task. The helper sleeps `_PRINTER_OFFLINE_NOTIFY_DEBOUNCE_SECONDS = 60.0` then re-checks `printer_manager.is_connected(printer_id)` — only fires the notification if the printer is still offline. **Why 60s debounce:** sized against `bambu_mqtt.py::STALE_RECONNECT_COOLDOWN = 30s` — a single stale-trigger + reconnect cycle isn't enough to fire, only a real outage that survives one full cooldown notifies. Transient MQTT blips (WiFi roam, broker reload, brief packet loss) recover within the window and the cancellation path kicks in. **Edge-case handling:** initial observation with no prior connected state doesn't fire (covers Bambuddy startup with an already-offline printer); a False → False repeat doesn't reschedule (the in-flight task stays in place rather than resetting the clock on every status callback, which would otherwise mean the notification never fires); the task entry pops from `_printer_offline_notify_tasks` in the finally block whether the notification fired, the printer reconnected, or the task was cancelled mid-await. **No symmetric `on_printer_online` event:** the reporter explicitly noted the "printer lost power and interrupted the print" notification already fires when power is restored — that's the print-failure notification, triggered by the firmware reporting `gcode_state=FAILED` for the interrupted print on MQTT reconnect. That covers the "printer is back" channel without a new toggle. If the user then resumes the print, no print_start notification fires (Bambuddy's `bambu_mqtt.py:3039` explicitly suppresses `is_new_print` for PAUSE → RUNNING to prevent duplicates when resuming from pause), but that's a separate scope from offline-detection. **Tests:** 9 new cases in `test_printer_offline_notification.py` split across two classes. `TestMaybeNotifyPrinterOffline` pins the debounced helper: fires notification when still offline at end of window, doesn't fire when printer reconnected during debounce, doesn't fire when the printer disappeared from the DB (uninstall mid-window), clears `_printer_offline_notify_tasks[printer_id]` after run. `TestOfflineEdgeDetection` pins the edge logic inside `on_printer_status_change`: first observation (connected) doesn't schedule, first observation (disconnected) doesn't schedule (the no-prior-True case — important for startup), True → False schedules a task, reconnect cancels the pending task, repeated False observations don't replace the in-flight task. Full backend suite still green; ruff clean.
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
/**
|
||||
* Regression test for #1738 — SpoolBuddy's inventory search must match by
|
||||
* numeric spool ID, just like Bambuddy's main InventoryPage. Both pages now
|
||||
* share `filterSpoolsByQuery` so behaviour stays in lockstep; this test fails
|
||||
* loudly if SpoolBuddyInventoryPage ever re-inlines its filter and drops
|
||||
* fields.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import type { InventorySpool } from '../../api/client';
|
||||
import { filterSpoolsByQuery } from '../../utils/inventorySearch';
|
||||
|
||||
function makeSpool(overrides: Partial<InventorySpool> & { id: number }): InventorySpool {
|
||||
return {
|
||||
material: 'PLA',
|
||||
subtype: 'Basic',
|
||||
brand: 'Bambu Lab',
|
||||
color_name: 'White',
|
||||
rgba: 'FFFFFFFF',
|
||||
label_weight: 1000,
|
||||
core_weight: 250,
|
||||
core_weight_catalog_id: null,
|
||||
weight_used: 0,
|
||||
weight_locked: false,
|
||||
slicer_filament: null,
|
||||
slicer_filament_name: null,
|
||||
nozzle_temp_min: null,
|
||||
nozzle_temp_max: null,
|
||||
note: null,
|
||||
added_full: null,
|
||||
last_used: null,
|
||||
encode_time: null,
|
||||
tag_uid: null,
|
||||
tray_uuid: null,
|
||||
data_origin: null,
|
||||
tag_type: null,
|
||||
archived_at: null,
|
||||
created_at: '2025-01-01T00:00:00Z',
|
||||
updated_at: '2025-01-01T00:00:00Z',
|
||||
k_profiles: [],
|
||||
cost_per_kg: null,
|
||||
last_scale_weight: null,
|
||||
last_weighed_at: null,
|
||||
storage_location: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('SpoolBuddyInventoryPage search filter (#1738)', () => {
|
||||
it('matches an exact spool ID', () => {
|
||||
const spools = [
|
||||
makeSpool({ id: 1336 }),
|
||||
makeSpool({ id: 1337 }),
|
||||
makeSpool({ id: 42 }),
|
||||
];
|
||||
const result = filterSpoolsByQuery(spools, '1336');
|
||||
expect(result.map((s) => s.id)).toEqual([1336]);
|
||||
});
|
||||
|
||||
it('matches a partial spool ID', () => {
|
||||
const spools = [
|
||||
makeSpool({ id: 100 }),
|
||||
makeSpool({ id: 200 }),
|
||||
makeSpool({ id: 1001 }),
|
||||
];
|
||||
const result = filterSpoolsByQuery(spools, '00');
|
||||
expect(result.map((s) => s.id).sort((a, b) => a - b)).toEqual([100, 200, 1001]);
|
||||
});
|
||||
|
||||
it('still matches by the existing fields SpoolBuddy supported pre-fix', () => {
|
||||
const spools = [
|
||||
makeSpool({ id: 1, material: 'PLA', brand: 'Bambu Lab', color_name: 'Red' }),
|
||||
makeSpool({ id: 2, material: 'PETG', brand: 'Polymaker', color_name: 'Blue' }),
|
||||
];
|
||||
expect(filterSpoolsByQuery(spools, 'polymaker').map((s) => s.id)).toEqual([2]);
|
||||
expect(filterSpoolsByQuery(spools, 'PLA').map((s) => s.id)).toEqual([1]);
|
||||
expect(filterSpoolsByQuery(spools, 'red').map((s) => s.id)).toEqual([1]);
|
||||
});
|
||||
|
||||
it('also matches by storage_location and slicer_filament_name (parity gain)', () => {
|
||||
const spools = [
|
||||
makeSpool({ id: 1, storage_location: 'IKEA Regal' }),
|
||||
makeSpool({ id: 2, slicer_filament_name: 'Generic PLA Matte' }),
|
||||
];
|
||||
expect(filterSpoolsByQuery(spools, 'IKEA').map((s) => s.id)).toEqual([1]);
|
||||
expect(filterSpoolsByQuery(spools, 'Matte').map((s) => s.id)).toEqual([2]);
|
||||
});
|
||||
});
|
||||
@@ -7,6 +7,7 @@ import { api } from '../../api/client';
|
||||
import type { InventorySpool } from '../../api/client';
|
||||
import { resolveSpoolColorName, getSwatchStyle, spoolColorString } from '../../utils/colors';
|
||||
import { formatSlotLabel } from '../../utils/amsHelpers';
|
||||
import { filterSpoolsByQuery } from '../../utils/inventorySearch';
|
||||
import { InventorySpoolInfoCard } from '../../components/spoolbuddy/InventorySpoolInfoCard';
|
||||
import { AssignToAmsModal } from '../../components/spoolbuddy/AssignToAmsModal';
|
||||
import type { SpoolBuddyOutletContext } from '../../components/spoolbuddy/SpoolBuddyLayout';
|
||||
@@ -144,16 +145,7 @@ export function SpoolBuddyInventoryPage() {
|
||||
list = list.filter(s => s.material === filterMode);
|
||||
}
|
||||
|
||||
if (searchQuery.trim()) {
|
||||
const q = searchQuery.toLowerCase().trim();
|
||||
list = list.filter(s =>
|
||||
s.material.toLowerCase().includes(q) ||
|
||||
(s.subtype && s.subtype.toLowerCase().includes(q)) ||
|
||||
(s.brand && s.brand.toLowerCase().includes(q)) ||
|
||||
(s.color_name && s.color_name.toLowerCase().includes(q)) ||
|
||||
(s.note && s.note.toLowerCase().includes(q))
|
||||
);
|
||||
}
|
||||
list = filterSpoolsByQuery(list, searchQuery.trim());
|
||||
|
||||
// Sort: assigned spools first (by slot label), then by most recently updated
|
||||
return [...list].sort((a, b) => {
|
||||
|
||||
Reference in New Issue
Block a user