From b51ef33472b8878b7985bbcd16424c89d0ebd8fb Mon Sep 17 00:00:00 2001 From: maziggy Date: Thu, 14 May 2026 14:52:18 +0200 Subject: [PATCH] fix(inventory): apply catalog color's gradient + effect, not just hex (#1340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a color preset from the catalog only copied color_name and rgba onto the spool — extra_colors (gradient stops) and effect_type (sparkle / wood / etc.) were silently dropped at three layers above the API: the SpoolFormModal state shape, the CatalogDisplayColor mapping in ColorSection, and the selectColor handler itself. All three widened to carry both fields through. Picking a catalog swatch now writes both fields from the entry, so solid presets cleanly replace previous gradients. Recent-colors and the hardcoded-fallback palette stay as plain hex pickers — they don't touch extras/effect since they aren't full presets. Also fixed the en-US `colour` → `color` drift in 8 locale files that the reporter flagged. --- CHANGELOG.md | 2 + .../ColorSectionCatalogExtras.test.tsx | 116 ++++++++++++++++++ frontend/src/components/SpoolFormModal.tsx | 11 +- .../components/spool-form/ColorSection.tsx | 31 ++++- frontend/src/components/spool-form/types.ts | 14 ++- frontend/src/i18n/locales/en.ts | 8 +- frontend/src/i18n/locales/fr.ts | 6 +- frontend/src/i18n/locales/it.ts | 6 +- frontend/src/i18n/locales/ja.ts | 6 +- frontend/src/i18n/locales/pt-BR.ts | 6 +- frontend/src/i18n/locales/zh-CN.ts | 2 +- frontend/src/i18n/locales/zh-TW.ts | 2 +- .../{index-D3mBzx5f.js => index-oQDjU99K.js} | 16 +-- static/index.html | 2 +- 14 files changed, 197 insertions(+), 31 deletions(-) create mode 100644 frontend/src/__tests__/components/spool-form/ColorSectionCatalogExtras.test.tsx rename static/assets/{index-D3mBzx5f.js => index-oQDjU99K.js} (96%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a20c8b1f..411d2ebce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 +- **Color catalog presets now apply `extra_colors` (gradient stops) and `effect_type` (sparkle / wood / marble / glow / matte) onto the spool, not just hex + name** ([#1340](https://github.com/maziggy/bambuddy/issues/1340), reported by @maugsburger) — Creating a catalog entry that pairs a base color with multi-color gradient stops and a visual effect, then clicking that swatch in the Edit Spool dialog, only copied `color_name` and `rgba` over — the `extra_colors` and `effect_type` fields were silently dropped. The data was flowing from the backend correctly (`GET /api/v1/inventory/color-catalog` returns both fields per the `ColorCatalogEntry` schema in [`frontend/src/api/client.ts`](frontend/src/api/client.ts)), but three layers above stripped them: (1) [`SpoolFormModal.tsx`](frontend/src/components/SpoolFormModal.tsx) typed its `colorCatalog` state with a narrower shape that omitted the two fields; (2) [`ColorSection.tsx`](frontend/src/components/spool-form/ColorSection.tsx) mapped catalog entries to `CatalogDisplayColor` (the typed-down shape rendered on swatches) without propagating them; (3) the `selectColor()` handler only set `rgba` + `color_name` on click. **Fix:** widened both types in [`spool-form/types.ts`](frontend/src/components/spool-form/types.ts) (`CatalogDisplayColor` + `ColorSectionProps.catalogColors`) to carry the optional `extra_colors` and `effect_type`, propagated them through the four `matchedCatalogColors` mapping callbacks (byBrand / exact full-material / normalized-trailing-`+` / base-material prefix), and extended `selectColor` to take optional `extraColors` / `effectType` parameters. **Semantic rule:** catalog swatches are complete presets — picking one writes BOTH gradient and effect from the entry (overwriting any existing values), so a gradient catalog entry applies its stops AND a solid catalog entry clears any old gradient that was on the spool. Recent-colors and the hardcoded-fallback palette are plain hex pickers — picking one keeps any existing `extra_colors` / `effect_type` untouched, since those swatches aren't presets, just color picks. **Bonus:** fixed the en-US spelling drift the reporter flagged in their nitpick — `'Extra colours'` and `'wrong colour loaded'` strings (which had been seeded into all 8 locale files as English fallbacks) standardized to `'Extra colors'` and `'wrong color loaded'`; matching comment blocks (`// Multi-colour ...`) normalized in the same pass. **Regression tests** in [`__tests__/components/spool-form/ColorSectionCatalogExtras.test.tsx`](frontend/src/__tests__/components/spool-form/ColorSectionCatalogExtras.test.tsx) (3 cases): catalog click with gradient + effect propagates all four fields to `updateField`, catalog click on a solid preset clears any pre-existing extras/effect (preset-replaces-look semantic), and fallback palette click leaves extras/effect untouched. All 23 spool-form tests + 8 i18n parity tests pass; build clean. + - **Assigning a spool to an unconfigured AMS slot no longer silently skips MQTT on A1 Mini / P1S firmware — and the "PETG over a PLA-configured slot won't reconfigure" symptom is fixed in the same change** ([#1322](https://github.com/maziggy/bambuddy/issues/1322), reported by @RosdasHH) — On the user's A1 Mini BMCU (firmware 01.07.02.00) and P1S Standard AMS (firmware 00.00.06.75), pressing "Assign Spool" on any slot left the slot unconfigured: the DB row was created with `pending_config=True`, the MQTT publish was skipped, and the log line `Pre-configured assignment: ... (slot empty, will configure on insert)` fired even though the spool was physically loaded. The same code path also blocked the "swap PLA to PETG in the same slot" flow — Bambuddy would keep treating the spool as PLA because the publish never reached the printer. **Root cause:** the empty-slot detection at [`backend/app/api/routes/inventory.py:1267`](backend/app/api/routes/inventory.py) preferred `tray.state == 11` ("filament fed to extruder") over `tray_type`, falling back to `tray_type` only when `state` was missing entirely. Reporter's AMS dumps showed `state == 3` on every slot — configured and unconfigured, on both printers — and `state` was never absent. So the state-only branch always fired, the result was always "empty", and MQTT was always skipped regardless of whether the slot was actually loaded. The "fingerprint_type empty → defer until insert" pre-config replay at [`backend/app/main.py:1026`](backend/app/main.py) had the same `cur_state == 11` gate, so even when the user manually configured the slot in Bambu Studio afterward (making `tray_type` go from `""` to `"PLA"`), the deferred MQTT publish never fired because state stayed at 3. **Fix:** both call sites now use a disjunction — the slot is treated as loaded when **either** `state == 11` **or** `tray_type` is non-empty. The "Reset slot" case (state=11 + tray_type="") that the original state-only check was protecting still works through the first clause; the configured-slot case (state=3 + tray_type="PLA") on firmwares that never set state=11 now works through the second; and truly empty unconfigured slots (state≠11 + tray_type="") still fall through to the pending-config path correctly. The on_ams_change replay's disjunction also fires the deferred publish when the user later configures the slot through Bambu Studio, since that flips `tray_type` non-empty even if state stays at 3. **Caveat:** for a truly empty slot with a 3rd-party non-RFID spool that the user physically inserted, neither signal points to "loaded" on these firmwares, so we still can't auto-fire the publish until the slot gets configured (manually or by another assign). The pending-config row persists in the DB and gets applied on the next AMS push that flips `tray_type` non-empty. **Regression tests:** 3 in [`test_inventory_assign.py`](backend/tests/integration/test_inventory_assign.py) — `test_state_never_eleven_firmware_with_loaded_tray_fires_mqtt` (state=3 + tray_type='PLA' → MQTT fires; pins the reporter's primary symptom and the PETG-over-PLA secondary symptom which goes through the same predicate), `test_state_never_eleven_firmware_with_empty_tray_marks_pending` (state=3 + tray_type='' still pending — confirms the disjunction didn't accidentally turn truly empty slots into the loaded branch), and `test_on_ams_change_fires_replay_when_tray_type_appears_without_state_11` (pre-existing SpoolBuddy-style assignment with empty fingerprint; tray_type going `''→'PLA'` on a state=3 firmware fires the deferred publish even though state never becomes 11). All 28 tests in the file pass; ruff clean. - **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. diff --git a/frontend/src/__tests__/components/spool-form/ColorSectionCatalogExtras.test.tsx b/frontend/src/__tests__/components/spool-form/ColorSectionCatalogExtras.test.tsx new file mode 100644 index 000000000..c4d6ad02e --- /dev/null +++ b/frontend/src/__tests__/components/spool-form/ColorSectionCatalogExtras.test.tsx @@ -0,0 +1,116 @@ +/** + * Regression test for #1340: clicking a catalog color must apply its + * extra_colors (gradient stops) and effect_type alongside hex + name. + * + * Previously only hex + name were copied onto the spool, so a catalog entry + * configured as a multi-color gradient with a visual effect would degrade to + * a flat solid swatch the moment the user picked it. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { I18nextProvider } from 'react-i18next'; +import i18n from '../../../i18n'; +import { ColorSection } from '../../../components/spool-form/ColorSection'; +import { defaultFormData } from '../../../components/spool-form/types'; + +function renderColorSection(opts: { + catalogColors: Parameters[0]['catalogColors']; + formData?: Partial; +}) { + const formData = { + ...defaultFormData, + brand: 'Bambu Lab', + material: 'PLA', + ...opts.formData, + }; + const updateField = vi.fn(); + render( + + + , + ); + return { updateField }; +} + +describe('ColorSection — catalog color picker (#1340)', () => { + it('applies extra_colors and effect_type from the catalog entry', () => { + const { updateField } = renderColorSection({ + catalogColors: [ + { + manufacturer: 'Bambu Lab', + color_name: 'Galaxy PLA', + hex_color: '#1a1a2e', + material: 'PLA', + extra_colors: 'ec984c,6cd4bc,a66eb9,d87694', + effect_type: 'sparkle', + }, + ], + }); + + // Catalog swatches are rendered as buttons with the hex as their background; + // the most reliable handle is the title text built from name + manufacturer. + const swatch = screen.getByTitle(/Galaxy PLA \(Bambu Lab/); + fireEvent.click(swatch); + + expect(updateField).toHaveBeenCalledWith('rgba', '1A1A2EFF'); + expect(updateField).toHaveBeenCalledWith('color_name', 'Galaxy PLA'); + expect(updateField).toHaveBeenCalledWith('extra_colors', 'ec984c,6cd4bc,a66eb9,d87694'); + expect(updateField).toHaveBeenCalledWith('effect_type', 'sparkle'); + }); + + it('clears existing extras when a catalog entry has none (preset replaces look)', () => { + const { updateField } = renderColorSection({ + catalogColors: [ + { + manufacturer: 'Bambu Lab', + color_name: 'Plain Red', + hex_color: '#ff0000', + material: 'PLA', + extra_colors: null, + effect_type: null, + }, + ], + formData: { extra_colors: 'aabbcc,ddeeff', effect_type: 'sparkle' }, + }); + + const swatch = screen.getByTitle(/Plain Red \(Bambu Lab/); + fireEvent.click(swatch); + + // The catalog entry is a complete preset — picking a solid preset must + // wipe the previously-set gradient and effect, not leave them clinging. + expect(updateField).toHaveBeenCalledWith('extra_colors', ''); + expect(updateField).toHaveBeenCalledWith('effect_type', ''); + }); + + it('leaves extras and effect untouched when a plain swatch is clicked', () => { + // The fallback hardcoded palette renders when no catalog colors match the + // brand/material. Those are plain hex pickers — they must NOT clobber an + // existing gradient on the spool. + const { updateField } = renderColorSection({ + catalogColors: [], + formData: { + brand: '', + material: '', + extra_colors: 'aabbcc,ddeeff', + effect_type: 'sparkle', + }, + }); + + // The QUICK_COLORS palette includes Black/White/etc. Pick any one. + const whiteSwatch = screen.getByTitle('White'); + fireEvent.click(whiteSwatch); + + expect(updateField).toHaveBeenCalledWith('rgba', 'FFFFFFFF'); + // No extra_colors / effect_type updates — those buttons aren't presets. + const calledKeys = updateField.mock.calls.map(c => c[0]); + expect(calledKeys).not.toContain('extra_colors'); + expect(calledKeys).not.toContain('effect_type'); + }); +}); diff --git a/frontend/src/components/SpoolFormModal.tsx b/frontend/src/components/SpoolFormModal.tsx index 7e72ba286..2d908769c 100644 --- a/frontend/src/components/SpoolFormModal.tsx +++ b/frontend/src/components/SpoolFormModal.tsx @@ -80,7 +80,16 @@ export function SpoolFormModal({ const [builtinFilaments, setBuiltinFilaments] = useState([]); // Color catalog - const [colorCatalog, setColorCatalog] = useState<{ manufacturer: string; color_name: string; hex_color: string; material: string | null }[]>([]); + const [colorCatalog, setColorCatalog] = useState<{ + manufacturer: string; + color_name: string; + hex_color: string; + material: string | null; + // #1340: gradient + effect carried from the catalog entry through to the + // color picker so they're applied alongside hex + name on selection. + extra_colors?: string | null; + effect_type?: string | null; + }[]>([]); // Color state const [recentColors, setRecentColors] = useState([]); diff --git a/frontend/src/components/spool-form/ColorSection.tsx b/frontend/src/components/spool-form/ColorSection.tsx index 1ac788fd4..4492a8684 100644 --- a/frontend/src/components/spool-form/ColorSection.tsx +++ b/frontend/src/components/spool-form/ColorSection.tsx @@ -44,10 +44,31 @@ export function ColorSection({ return currentHex.toUpperCase() === hex.toUpperCase(); }; - const selectColor = (hex: string, name: string) => { + const selectColor = ( + hex: string, + name: string, + // #1340: catalog entries carry an optional gradient + effect. Pass them in + // (even as empty strings) to overwrite the spool's existing values — the + // catalog entry is a complete preset, the user explicitly chose its look. + // Pass `undefined` (the default, used by recent/fallback swatches) to + // leave any existing gradient/effect untouched — those buttons are plain + // hex pickers, not full presets. + extraColors?: string | null, + effectType?: string | null, + ) => { // Store as RRGGBBAA (with FF alpha) updateField('rgba', hex.toUpperCase() + 'FF'); updateField('color_name', name); + if (extraColors !== undefined) { + const next = extraColors ?? ''; + setExtraColorsDraft(next); + setExtraColorsErrors([]); + lastCommittedExtraColorsRef.current = next; + updateField('extra_colors', next); + } + if (effectType !== undefined) { + updateField('effect_type', effectType ?? ''); + } onColorUsed({ name, hex }); }; @@ -82,6 +103,8 @@ export function ColorSection({ hex: c.hex_color.replace('#', '').substring(0, 6), manufacturer: c.manufacturer, material: typeof c.material === 'string' ? c.material : undefined, + extra_colors: c.extra_colors ?? null, + effect_type: c.effect_type ?? null, })); } } @@ -101,6 +124,8 @@ export function ColorSection({ hex: c.hex_color.replace('#', '').substring(0, 6), manufacturer: c.manufacturer, material: typeof c.material === 'string' ? c.material : undefined, + extra_colors: c.extra_colors ?? null, + effect_type: c.effect_type ?? null, })); } // Try without trailing "+" (e.g. "PLA Silk+" -> "PLA Silk") @@ -133,6 +158,8 @@ export function ColorSection({ hex: c.hex_color.replace('#', '').substring(0, 6), manufacturer: c.manufacturer, material: typeof c.material === 'string' ? c.material : undefined, + extra_colors: c.extra_colors ?? null, + effect_type: c.effect_type ?? null, })); } } @@ -271,7 +298,7 @@ export function ColorSection({