mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 21:47:39 +02:00
fix(inventory): restore Spoolman spool ID search + Unassign button (#1336)
Two regressions reported in #1336: 1. spoolMatchesQuery did not include spool.id in the predicate, so typing a numeric Spoolman ID into the Assign Spool dialog or the Inventory page search returned no matches. Predicate now also tests String(spool.id).includes(q). 2. The Unassign button in the spool edit modal was permanently disabled for Spoolman-mode spools. The modal only ever queried the legacy spool_assignments table (keyed by spool_id), but in Spoolman mode the assignment lives in spoolman_slot_assignments (keyed by spoolman_spool_id). Both the lookup query and the unassign mutation now branch on the spoolmanMode prop and call the Spoolman-flavored endpoints.
This commit is contained in:
@@ -14,6 +14,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Spoolman weight tracking now uses per-print grams for all spools, matching the internal Filament Inventory** ([#1119](https://github.com/maziggy/bambuddy/issues/1119), reported by @Moskito99) — Spoolman previously had two mutually-exclusive weight paths: AMS remain%×tray_weight auto-sync (default; only worked for Bambu Lab spools with valid RFID tray_weight) and per-print 3MF-grams tracking (only enabled when "Disable AMS Weight Sync" was toggled on). Non-BL spools without RFID fell through both paths — AMS auto-sync had no tray_weight to multiply, and the inventory_remaining fallback was wiped because activating Spoolman deletes the internal `spool_assignment` table — so Spoolman never saw a weight update for them. The internal Filament Inventory has no such gap: it always uses per-print 3MF grams as the primary path with AMS-remain% delta as fallback, and it works for every spool type. Spoolman now does the same: per-print tracking runs whenever Spoolman is enabled and is the only writer of `remaining_weight`. AMS auto-sync continues to maintain spool metadata and slot assignments but no longer touches weight (eliminating the double-count that would otherwise occur for BL spools with both paths active). `store_print_data` ([`spoolman_tracking.py:159`](backend/app/services/spoolman_tracking.py)) had its `disable_weight_sync` early-return removed; the three `sync_ams_tray` callsites (`main.py:1450` auto-sync, `spoolman.py:318` per-printer manual, `spoolman.py:517` sync-all) now hard-code `disable_weight_sync=True`. The `spoolman_disable_weight_sync` setting is now deprecated and a no-op — kept in the DB/UI for backwards compat. Behavioral consequence for existing users on the default flag (False): live AMS-based remaining_weight updates between prints stop happening; weight updates now arrive once per print completion with 3MF gram precision. Regression test in `test_spoolman_tracking.py::test_stores_tracking_when_disable_weight_sync_is_false` proves the early-return is gone.
|
||||
|
||||
### Fixed
|
||||
- **Assign Spool / Inventory search: numeric spool ID lookup is back, and Unassign in Spoolman mode no longer stays permanently disabled** ([#1336](https://github.com/maziggy/bambuddy/issues/1336), reported by @S0liter) — Two independent regressions surfaced from the same report. **(1) Numeric ID search:** typing a Spoolman spool's numeric ID into the search box on the "Assign Spool" dialog (or on the Inventory page) returned no results. The shared search helper `spoolMatchesQuery` at [`frontend/src/utils/inventorySearch.ts:7`](frontend/src/utils/inventorySearch.ts) only checked the text fields (`material`, `brand`, `color_name`, `subtype`, `note`, `slicer_filament_name`, `storage_location`) — the spool's `id` was not part of the predicate, so a query like `12` only matched when "12" happened to be a substring of one of the text fields. One-line fix: the predicate now also tests `String(spool.id).includes(q)`, mirroring the case-insensitive substring semantics of the other fields. Covers both call sites: the Assign Spool dialog ([`AssignSpoolModal.tsx:255`](frontend/src/components/AssignSpoolModal.tsx) for local inventory + `:446` for Spoolman) and the main Inventory page ([`InventoryPage.tsx:871`](frontend/src/pages/InventoryPage.tsx)). New regression test in [`__tests__/utils/inventorySearch.test.ts`](frontend/src/__tests__/utils/inventorySearch.test.ts) pins exact-match (`'42'` → id 42), substring (`'4'` → id 42), and non-match (`'99'` → id 42 rejected) so the predicate can't drift back into "text only" silently. **(2) Unassign button stuck disabled in Spoolman mode:** opening the edit modal on a Spoolman spool that was assigned to an AMS slot left the Unassign button greyed out — the user had no way to release the spool back to "available". The modal at [`SpoolFormModal.tsx:526`](frontend/src/components/SpoolFormModal.tsx) only ever queried `api.getAssignments()` (the legacy local `spool_assignments` table) and looked up by `a.spool_id === spool.id`. In Spoolman mode the slot assignment lives in the separate `spoolman_slot_assignments` table, keyed by `spoolman_spool_id` — so the lookup always returned `undefined`, the button's `disabled={isPending || !spoolAssignment}` predicate stayed true forever, and `unassignMutation` was also pointing at the wrong API (`unassignSpool` instead of `unassignSpoolmanSlot`). Both the query and the mutation now branch on the existing `spoolmanMode` prop: Spoolman mode uses `getSpoolmanSlotAssignments()` + lookup by `spoolman_spool_id` + `unassignSpoolmanSlot(spool.id)` and invalidates the `spoolman-slot-assignments-all` / `spoolman-slot-assignments` query keys; local mode keeps the existing path unchanged. Two new regression tests in [`__tests__/components/SpoolFormModal.test.tsx`](frontend/src/__tests__/components/SpoolFormModal.test.tsx) (`SpoolFormModal — Unassign button (#1336)`): the button is enabled and clicking it calls `unassignSpoolmanSlot(42)` when a matching `spoolman_slot_assignment` exists, and the button stays disabled (no `unassignSpool` fallback) when no assignment exists. All 12 search-helper tests + 13 InventoryPage search tests + 27 SpoolFormModal tests pass; frontend build clean.
|
||||
|
||||
- **Spoolman auto-create no longer labels Bambu Lab RFID spools with competitor names like "3DXTECH™ Black"** ([#1309](https://github.com/maziggy/bambuddy/issues/1309), [PR #1330](https://github.com/maziggy/bambuddy/pull/1330) by @ojimpo) — When Bambuddy auto-created a Spoolman filament entry for a Bambu Lab RFID spool, the second-stage lookup against Spoolman's external library (`GET /api/v1/external/filament`, served from [SpoolmanDB](https://donkie.github.io/SpoolmanDB/filaments.json)) matched on `material + color_hex` only — there was no `manufacturer` / `vendor` filter. **The catalog is multi-vendor and roughly ID-sorted**: for PLA + `#000000` (black) it contains 64 entries, with the first hit being `3djake_pla_black_1000_175_n` (`3DJAKE`), the third being `3dxtech_pla_carbonxcarbonfiberblack_500_175_p` (`3DXTECH`, name `CarbonX™ Carbon Fiber Black`), and the actual `bambulab_pla_black_1000_175_n` not surfacing until position 15. Bambuddy therefore created the filament under the Bambu Lab vendor but labeled it with a competitor's product name. Real-world observations in production: Bambu Lab ABS Black created as `3DXTECH™ Black`, Bambu Lab PLA Support picked the adjacent / wrong variant instead of `bambulab_pla_supportforpla/petgblack_500_175_n`, and PLA Basic Black created as `PLA` (material, not `PLA Basic`). A **secondary issue compounded this**: `_create_filament_from_external` dropped the external entry's `density` field, so even when the correct entry was eventually picked the density got overwritten by `create_filament`'s built-in PLA-default 1.24 fallback instead of the catalog's actual value (1.26 for PLA Basic, 1.31 for PETG, etc.). **Fix** in [`backend/app/services/spoolman.py::_find_or_create_filament`](backend/app/services/spoolman.py): (1) the external-library loop now filters by `manufacturer == "Bambu Lab"` (case-insensitive, whitespace-trimmed), with a defensive `id.startswith("bambulab_")` fallback that handles entries where the `manufacturer` field is missing or has drifted in a future SpoolmanDB schema. (2) When multiple Bambu Lab candidates match the same `material + color_hex`, the function prefers the entry whose `name` equals the AMS `tray_sub_brands` (lowercase+strip comparison) so the more specific variant wins — `PLA Basic` over generic `Black`, `Support for PLA/PETG Black` over generic `Black`, etc. (3) `_create_filament_from_external` now propagates `external.get("density")` through to `create_filament`; when the catalog entry has no density set, the existing material-table fallback inside `create_filament` still kicks in via the `if density is None` branch at line 321 — no path lost. **Behavioural caveat the user needs to know**: previously-created mis-named filaments are NOT auto-renamed by this fix. Step 1 of `_find_or_create_filament` is the internal-Spoolman-filament loop that short-circuits on `(vendor == "Bambu Lab", material, color_hex)` — and that path is unchanged. Any Bambu Lab filament created by an older Bambuddy build (or hand-edited by the user) will continue to be matched and reused on subsequent AMS reads, regardless of how wrong its name is. To pick up the corrected name, the user has to **delete the mis-named filament in Spoolman once** — then the next AMS read for the same material+color falls through to the external-library step and creates a new entry with the correct Bambu Lab name. This is deliberate: some users may have intentionally renamed Bambu Lab filaments (e.g. to follow their own naming convention or to merge variants) and a silent auto-rename would undo that. **Regression tests** in [`test_spoolman_service.py::TestFindOrCreateFilament`](backend/tests/unit/services/test_spoolman_service.py) (6 new): internal short-circuit preserves the existing match without touching the external library, non-Bambu-Lab external entries are skipped even when they sort first in SpoolmanDB, `PLA Basic` wins over generic `Black` via the `tray_sub_brands` tiebreaker (per maintainer request on #1309), no-match-anywhere falls back to `tray_sub_brands or tray_type` instead of leaking a competitor name into the create call, `id.startswith("bambulab_")` accepts entries with absent `manufacturer` field, and density propagates end-to-end through the public method instead of getting clobbered by the material-default. All 44 tests in `test_spoolman_service.py` pass; ruff clean. **Reported and patched by @ojimpo.**
|
||||
|
||||
- **Safety: bed-jog Z direction was inverted on A1 / A1 Mini — "Up" rammed the nozzle into the bed** ([#1334](https://github.com/maziggy/bambuddy/issues/1334), reported by william.filipcic@gmail.com) — On A1 / A1 Mini, clicking the "Up" arrow on the printer-card bed-jog control would send the nozzle straight into the build plate. Reporter triggered it with the 50 mm step and crashed their nozzle. **Root cause:** the bed-jog UI was designed against the X1 / P1 / H2 family's bed-on-Z convention. On those printers the bed is the Z-axis, Bambu's firmware homes Z=0 at the top, and ``G1 Z-`` raises the bed toward the toolhead (decreases the nozzle-bed gap). The frontend maps "Up" to negative distance with that convention in mind. **A1 / A1 Mini are bed-slingers**: the bed moves on Y, the toolhead moves on X+Z, and the firmware uses standard cartesian Z (Z+ = toolhead up). On those models ``G1 Z-10`` drives the *toolhead* down 10 mm — straight through any clearance the user had — which is exactly what the reporter saw. There was no model classification at the bed-jog code path; every printer got the same X1-convention G-code. **Fix:** new ``is_bed_slinger(model)`` helper at [`backend/app/services/printer_manager.py`](backend/app/services/printer_manager.py) (sibling to existing ``supports_chamber_temp`` / ``has_stg_cur_idle_bug``, reuses the already-defined ``A1_MODELS`` frozenset which covers display names and internal codes ``N1`` / ``N2S``). The bed-jog route at [`backend/app/api/routes/printers.py:2710`](backend/app/api/routes/printers.py) now inverts the signed distance before emitting the G-code when the printer model is in that set, so the UI "Up" semantics ("decrease nozzle-bed gap") stay consistent regardless of which physical part moves on the printer. Frontend stays untouched — single source of truth for the direction logic lives in the backend, keyed off the printer's ``model`` column, so any future bed-slinger Bambu model only needs one frozenset update. The route's ``Query`` description and docstring now state the new contract explicitly: distance is the *gap adjustment*, not the raw Z value, and the backend translates per model. **Regression tests:** 13 in [`test_bed_jog.py::TestBedJogAPI`](backend/tests/unit/test_bed_jog.py) — 6 parametrised cases prove bed-on-Z models (X1C / P1S / H2D / H2S / H2C / P2S) still emit ``G1 Z-10.00`` for a UI "Up" click (pass-through), 6 parametrised cases prove A1 / A1 Mini / A1MINI / A1-MINI / N1 / N2S emit ``G1 Z10.00`` instead (inverted, toolhead up), plus 1 symmetric "Down arrow drops the toolhead via ``G1 Z-``" case. 5 in [`test_printer_manager.py::TestIsBedSlinger`](backend/tests/unit/services/test_printer_manager.py) pin the helper's classification contract — A1 family true, every bed-on-Z model false, None / empty-string safe, case-insensitive. **Safety note:** if you own an A1 or A1 Mini and were running any 0.2.x build before this release, do not use the printer-card bed-jog buttons — they will move the toolhead in the wrong direction. The Z controls in Bambu Studio / Bambu Handy are unaffected (they generate their own model-aware G-code).
|
||||
|
||||
@@ -38,6 +38,10 @@ vi.mock('../../api/client', () => ({
|
||||
failed_count: 0,
|
||||
}),
|
||||
getSpoolmanInventoryFilaments: vi.fn().mockResolvedValue([]),
|
||||
getAssignments: vi.fn().mockResolvedValue([]),
|
||||
getSpoolmanSlotAssignments: vi.fn().mockResolvedValue([]),
|
||||
unassignSpool: vi.fn().mockResolvedValue({}),
|
||||
unassignSpoolmanSlot: vi.fn().mockResolvedValue({}),
|
||||
},
|
||||
ApiError: class ApiError extends Error {
|
||||
status: number;
|
||||
@@ -828,6 +832,106 @@ describe('SpoolFormModal — SpoolmanFilamentPicker integration (T2)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('SpoolFormModal — Unassign button (#1336)', () => {
|
||||
const spoolmanSpool: InventorySpool = {
|
||||
id: 42,
|
||||
material: 'PLA',
|
||||
subtype: 'Basic',
|
||||
brand: 'BrandX',
|
||||
color_name: 'Black',
|
||||
rgba: '000000FF',
|
||||
extra_colors: null,
|
||||
effect_type: null,
|
||||
label_weight: 1000,
|
||||
core_weight: 250,
|
||||
core_weight_catalog_id: null,
|
||||
weight_used: 200,
|
||||
slicer_filament: '',
|
||||
slicer_filament_name: '',
|
||||
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: 'spoolman',
|
||||
tag_type: 'spoolman',
|
||||
archived_at: null,
|
||||
created_at: '2025-01-01T00:00:00Z',
|
||||
updated_at: '2025-01-01T00:00:00Z',
|
||||
cost_per_kg: null,
|
||||
last_scale_weight: null,
|
||||
last_weighed_at: null,
|
||||
category: null,
|
||||
low_stock_threshold_pct: null,
|
||||
k_profiles: [],
|
||||
} as InventorySpool;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('enables Unassign in Spoolman mode when a spoolman_slot_assignment exists for the spool', async () => {
|
||||
vi.mocked(api.getSpoolmanSlotAssignments).mockResolvedValueOnce([
|
||||
{
|
||||
printer_id: 1,
|
||||
printer_name: 'Test Printer',
|
||||
ams_id: 0,
|
||||
tray_id: 2,
|
||||
spoolman_spool_id: 42,
|
||||
ams_label: 'AMS 1',
|
||||
},
|
||||
]);
|
||||
|
||||
render(
|
||||
<SpoolFormModal
|
||||
isOpen={true}
|
||||
onClose={vi.fn()}
|
||||
spool={spoolmanSpool}
|
||||
mode="edit"
|
||||
currencySymbol="$"
|
||||
spoolmanMode={true}
|
||||
/>
|
||||
);
|
||||
|
||||
const unassignBtn = await screen.findByRole('button', { name: /unassign/i });
|
||||
await waitFor(() => {
|
||||
expect(unassignBtn).not.toBeDisabled();
|
||||
});
|
||||
|
||||
fireEvent.click(unassignBtn);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(api.unassignSpoolmanSlot).toHaveBeenCalledWith(42);
|
||||
});
|
||||
expect(api.unassignSpool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps Unassign disabled in Spoolman mode when no slot assignment exists', async () => {
|
||||
vi.mocked(api.getSpoolmanSlotAssignments).mockResolvedValueOnce([]);
|
||||
|
||||
render(
|
||||
<SpoolFormModal
|
||||
isOpen={true}
|
||||
onClose={vi.fn()}
|
||||
spool={spoolmanSpool}
|
||||
mode="edit"
|
||||
currencySymbol="$"
|
||||
spoolmanMode={true}
|
||||
/>
|
||||
);
|
||||
|
||||
const unassignBtn = await screen.findByRole('button', { name: /unassign/i });
|
||||
// Wait one tick for the (empty) query result to settle so the disabled state is final.
|
||||
await waitFor(() => {
|
||||
expect(api.getSpoolmanSlotAssignments).toHaveBeenCalled();
|
||||
});
|
||||
expect(unassignBtn).toBeDisabled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('SpoolFormModal storageLocationTouched', () => {
|
||||
/**
|
||||
* Regression tests for the round-trip bug: saving the edit modal without
|
||||
|
||||
@@ -73,6 +73,13 @@ describe('spoolMatchesQuery', () => {
|
||||
const spool = makeSpool({ brand: null, color_name: null, subtype: null });
|
||||
expect(spoolMatchesQuery(spool, 'bambu')).toBe(false);
|
||||
});
|
||||
|
||||
it('matches on numeric id (#1336)', () => {
|
||||
const spool = makeSpool({ id: 42, material: 'PLA', brand: null, color_name: null, subtype: null, note: null });
|
||||
expect(spoolMatchesQuery(spool, '42')).toBe(true);
|
||||
expect(spoolMatchesQuery(spool, '4')).toBe(true);
|
||||
expect(spoolMatchesQuery(spool, '99')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('filterSpoolsByQuery', () => {
|
||||
|
||||
@@ -523,13 +523,27 @@ export function SpoolFormModal({
|
||||
},
|
||||
});
|
||||
|
||||
// Fetch assignment for this spool (to show Unassign button)
|
||||
// Fetch assignment for this spool (to show Unassign button). In Spoolman mode
|
||||
// the slot assignment lives in the spoolman_slot_assignments table keyed by
|
||||
// spoolman_spool_id, not in the legacy spool_assignments table — #1336 was the
|
||||
// resulting "Unassign button is always disabled" report.
|
||||
const { data: assignments } = useQuery({
|
||||
queryKey: ['spool-assignments'],
|
||||
queryFn: () => api.getAssignments(),
|
||||
enabled: isOpen && isEditing,
|
||||
enabled: isOpen && isEditing && !spoolmanMode,
|
||||
});
|
||||
const spoolAssignment = spool ? assignments?.find(a => a.spool_id === spool.id) : undefined;
|
||||
const { data: spoolmanSlotAssignments } = useQuery({
|
||||
queryKey: ['spoolman-slot-assignments-all'],
|
||||
queryFn: () => api.getSpoolmanSlotAssignments(),
|
||||
enabled: isOpen && isEditing && spoolmanMode,
|
||||
});
|
||||
const spoolAssignment = (() => {
|
||||
if (!spool) return undefined;
|
||||
if (spoolmanMode) {
|
||||
return spoolmanSlotAssignments?.find(a => a.spoolman_spool_id === spool.id);
|
||||
}
|
||||
return assignments?.find(a => a.spool_id === spool.id);
|
||||
})();
|
||||
|
||||
// Read inventory + settings caches (already populated by InventoryPage) to
|
||||
// drive the category autocomplete and low-stock-threshold placeholder. #729
|
||||
@@ -554,12 +568,22 @@ export function SpoolFormModal({
|
||||
const globalLowStockThreshold = settingsForForm?.low_stock_threshold ?? 20;
|
||||
|
||||
const unassignMutation = useMutation({
|
||||
mutationFn: () => {
|
||||
mutationFn: async () => {
|
||||
if (!spoolAssignment) throw new Error('No assignment');
|
||||
return api.unassignSpool(spoolAssignment.printer_id, spoolAssignment.ams_id, spoolAssignment.tray_id);
|
||||
if (spoolmanMode) {
|
||||
if (!spool) throw new Error('No spool');
|
||||
await api.unassignSpoolmanSlot(spool.id);
|
||||
return;
|
||||
}
|
||||
await api.unassignSpool(spoolAssignment.printer_id, spoolAssignment.ams_id, spoolAssignment.tray_id);
|
||||
},
|
||||
onSuccess: async () => {
|
||||
await queryClient.invalidateQueries({ queryKey: ['spool-assignments'] });
|
||||
if (spoolmanMode) {
|
||||
await queryClient.invalidateQueries({ queryKey: ['spoolman-slot-assignments-all'] });
|
||||
await queryClient.invalidateQueries({ queryKey: ['spoolman-slot-assignments'] });
|
||||
} else {
|
||||
await queryClient.invalidateQueries({ queryKey: ['spool-assignments'] });
|
||||
}
|
||||
showToast(t('inventory.unassignSuccess', 'Spool unassigned'), 'success');
|
||||
onClose();
|
||||
},
|
||||
|
||||
@@ -8,6 +8,7 @@ export function spoolMatchesQuery(spool: InventorySpool, query: string): boolean
|
||||
if (!query) return true;
|
||||
const q = query.toLowerCase();
|
||||
return (
|
||||
String(spool.id).includes(q) ||
|
||||
spool.material.toLowerCase().includes(q) ||
|
||||
(spool.brand?.toLowerCase().includes(q) ?? false) ||
|
||||
(spool.color_name?.toLowerCase().includes(q) ?? false) ||
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -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-D5ddq2Sh.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-D3mBzx5f.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-BkYu3kLs.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user