diff --git a/backend/app/api/routes/_spoolman_helpers.py b/backend/app/api/routes/_spoolman_helpers.py index 77922318a..c1471e13f 100644 --- a/backend/app/api/routes/_spoolman_helpers.py +++ b/backend/app/api/routes/_spoolman_helpers.py @@ -55,6 +55,7 @@ class MappedSpoolFields(TypedDict): updated_at: str | None cost_per_kg: float | None storage_location: str | None + location_id: int | None k_profiles: list[Any] @@ -346,5 +347,6 @@ def _map_spoolman_spool(spool: dict) -> MappedSpoolFields: "updated_at": created_at, "cost_per_kg": _safe_optional_float(spool.get("price")), "storage_location": spool.get("location") or None, + "location_id": None, "k_profiles": [], } diff --git a/backend/app/api/routes/inventory.py b/backend/app/api/routes/inventory.py index f757f9227..438f6d828 100644 --- a/backend/app/api/routes/inventory.py +++ b/backend/app/api/routes/inventory.py @@ -6,6 +6,7 @@ from fastapi import APIRouter, Depends, File, HTTPException, Query, UploadFile from fastapi.responses import Response, StreamingResponse from pydantic import BaseModel, Field, field_validator from sqlalchemy import delete, func, select +from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -20,11 +21,14 @@ from backend.app.core.permissions import Permission from backend.app.core.websocket import ws_manager from backend.app.models.ams_label import AmsLabel from backend.app.models.color_catalog import ColorCatalogEntry +from backend.app.models.location import Location +from backend.app.models.settings import Settings from backend.app.models.spool import Spool from backend.app.models.spool_assignment import SpoolAssignment from backend.app.models.spool_catalog import SpoolCatalogEntry from backend.app.models.spool_k_profile import SpoolKProfile from backend.app.models.user import User +from backend.app.schemas.location import LocationCreate, LocationResponse, LocationUpdate from backend.app.schemas.spool import ( SpoolAssignmentCreate, SpoolAssignmentResponse, @@ -38,6 +42,16 @@ from backend.app.schemas.spool import ( normalize_extra_colors, ) from backend.app.schemas.spool_usage import SpoolUsageHistoryResponse +from backend.app.services.location_service import ( + DUPLICATE_LOCATION_NAME, + assign_location_name, + count_internal_spools_at_location, + get_location_by_id, + get_location_by_name, + location_name_key, + prepare_internal_spool_payload, + rename_location as rename_location_record, +) from backend.app.services.slicer_filament_resolver import resolve_slicer_filament from backend.app.services.spool_csv import ( MAX_CSV_IMPORT_BYTES, @@ -46,6 +60,7 @@ from backend.app.services.spool_csv import ( parse_and_validate, serialize, ) +from backend.app.services.spoolman import SpoolmanClient, get_spoolman_client, init_spoolman_client from backend.app.utils.filament_ids import ( GENERIC_FILAMENT_IDS, MATERIAL_TEMPS, @@ -493,6 +508,198 @@ async def reset_spool_catalog( return {"status": "reset"} +# ── Storage Locations (#1004) ─────────────────────────────────────────────── + + +async def _load_settings_map(db: AsyncSession) -> dict[str, str]: + result = await db.execute(select(Settings)) + return {s.key: s.value for s in result.scalars().all()} + + +def _spoolman_is_enabled(settings: dict[str, str]) -> bool: + return settings.get("spoolman_enabled", "false").lower() == "true" + + +async def _ensure_spoolman_client(settings: dict[str, str]) -> SpoolmanClient | None: + if not _spoolman_is_enabled(settings): + return None + url = settings.get("spoolman_url", "").strip() + if not url: + return None + from backend.app.api.routes._spoolman_helpers import assert_safe_spoolman_url + + try: + assert_safe_spoolman_url(url) + except ValueError: + return None + client = await get_spoolman_client() + if not client or client.base_url != url.rstrip("/"): + client = await init_spoolman_client(url) + return client + + +async def _spool_counts_for_locations( + db: AsyncSession, + locations: list[Location], + settings: dict[str, str], +) -> dict[int, int]: + if _spoolman_is_enabled(settings): + client = await _ensure_spoolman_client(settings) + if client: + try: + spools = await client.get_all_spools(allow_archived=False) + except Exception: + logger.warning("Failed to fetch Spoolman spools for location counts", exc_info=True) + else: + # Use the canonical key helper so this matches what the + # migration backfill, Location.name_key, and every other + # codepath store as the case-insensitive lookup key. Plain + # str.lower() drifts for non-ASCII (Turkish ı/İ, German ß) + # and caused mismatched delete-block counts in Spoolman mode. + by_key: dict[str, int] = {} + for spool in spools: + raw = spool.get("location") + if not raw or not isinstance(raw, str) or not raw.strip(): + continue + try: + key = location_name_key(raw) + except ValueError: + continue + by_key[key] = by_key.get(key, 0) + 1 + return {loc.id: by_key.get(loc.name_key, 0) for loc in locations} + + counts: dict[int, int] = {} + for loc in locations: + counts[loc.id] = await count_internal_spools_at_location(db, loc.id) + return counts + + +def _location_to_response(location: Location, spool_count: int) -> LocationResponse: + return LocationResponse( + id=location.id, + name=location.name, + identifier=location.identifier, + spool_count=spool_count, + created_at=location.created_at, + updated_at=location.updated_at, + ) + + +@router.get("/locations", response_model=list[LocationResponse]) +async def list_locations( + db: AsyncSession = Depends(get_db), + _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_READ), +): + """List all storage locations with spool counts.""" + settings = await _load_settings_map(db) + result = await db.execute(select(Location).order_by(Location.name)) + locations = list(result.scalars().all()) + counts = await _spool_counts_for_locations(db, locations, settings) + return [_location_to_response(loc, counts.get(loc.id, 0)) for loc in locations] + + +@router.post("/locations", response_model=LocationResponse, status_code=201) +async def create_location( + data: LocationCreate, + db: AsyncSession = Depends(get_db), + _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), +): + """Create a storage location.""" + existing = await get_location_by_name(db, data.name) + if existing: + raise HTTPException(status_code=409, detail=DUPLICATE_LOCATION_NAME) + location = Location(identifier=data.identifier) + assign_location_name(location, data.name) + db.add(location) + try: + await db.commit() + except IntegrityError as exc: + await db.rollback() + raise HTTPException(status_code=409, detail=DUPLICATE_LOCATION_NAME) from exc + await db.refresh(location) + await ws_manager.broadcast({"type": "inventory_changed"}) + return _location_to_response(location, 0) + + +@router.patch("/locations/{location_id}", response_model=LocationResponse) +async def update_location( + location_id: int, + data: LocationUpdate, + db: AsyncSession = Depends(get_db), + _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), +): + """Update a storage location (rename propagates to assigned spools).""" + location = await get_location_by_id(db, location_id) + if not location: + raise HTTPException(status_code=404, detail="Location not found") + + old_name = location.name + if data.identifier is not None: + location.identifier = data.identifier or None + + if data.name is not None and data.name != old_name: + try: + await rename_location_record(db, location, data.name) + except ValueError as exc: + raise HTTPException(status_code=409, detail=str(exc)) from exc + + # Cascade to Spoolman BEFORE the local commit so a Spoolman failure + # rolls back the local rename instead of leaving the catalog and + # Spoolman's per-spool `location` field permanently diverged. Without + # this ordering, a partial failure makes the next location-sync recreate + # the old name as a duplicate catalog row (#1505 review blocker). + settings = await _load_settings_map(db) + client = await _ensure_spoolman_client(settings) + if client: + try: + await client.rename_location(old_name, location.name) + except Exception as exc: + logger.warning( + "Spoolman location rename failed for %s -> %s: %s", + old_name, + location.name, + exc, + ) + await db.rollback() + raise HTTPException( + status_code=502, + detail="Spoolman rename failed; local rename rolled back", + ) from exc + + try: + await db.commit() + except IntegrityError as exc: + await db.rollback() + raise HTTPException(status_code=409, detail=DUPLICATE_LOCATION_NAME) from exc + await db.refresh(location) + settings = await _load_settings_map(db) + counts = await _spool_counts_for_locations(db, [location], settings) + await ws_manager.broadcast({"type": "inventory_changed"}) + return _location_to_response(location, counts.get(location.id, 0)) + + +@router.delete("/locations/{location_id}") +async def delete_location( + location_id: int, + db: AsyncSession = Depends(get_db), + _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), +): + """Delete a storage location when no spools are assigned.""" + location = await get_location_by_id(db, location_id) + if not location: + raise HTTPException(status_code=404, detail="Location not found") + + settings = await _load_settings_map(db) + counts = await _spool_counts_for_locations(db, [location], settings) + if counts.get(location.id, 0) > 0: + raise HTTPException(status_code=409, detail="Location has spools assigned and cannot be deleted") + + await db.delete(location) + await db.commit() + await ws_manager.broadcast({"type": "inventory_changed"}) + return {"status": "deleted"} + + # ── Color Catalog CRUD ───────────────────────────────────────────────────── @@ -995,7 +1202,11 @@ async def create_spool( _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), ): """Create a new spool.""" - spool = Spool(**spool_data.model_dump()) + try: + payload = await prepare_internal_spool_payload(db, spool_data.model_dump(), set(spool_data.model_fields_set)) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + spool = Spool(**payload) db.add(spool) await db.commit() await db.refresh(spool) @@ -1012,8 +1223,13 @@ async def bulk_create_spools( ): """Create multiple identical spools.""" spools = [] + fields_set = set(data.spool.model_fields_set) + try: + payload = await prepare_internal_spool_payload(db, data.spool.model_dump(), fields_set) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc for _ in range(data.quantity): - spool = Spool(**data.spool.model_dump()) + spool = Spool(**payload) db.add(spool) spools.append(spool) await db.commit() @@ -1037,6 +1253,10 @@ async def update_spool( raise HTTPException(404, "Spool not found") update_data = spool_data.model_dump(exclude_unset=True) + try: + update_data = await prepare_internal_spool_payload(db, update_data, set(spool_data.model_fields_set)) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc # Auto-lock weight when user explicitly sets weight_used if "weight_used" in update_data and "weight_locked" not in update_data: update_data["weight_locked"] = True diff --git a/backend/app/api/routes/settings.py b/backend/app/api/routes/settings.py index 87241e41b..d0cbb1b31 100644 --- a/backend/app/api/routes/settings.py +++ b/backend/app/api/routes/settings.py @@ -446,9 +446,20 @@ async def update_spoolman_settings( if "spoolman_report_partial_usage" in settings: await set_setting(db, "spoolman_report_partial_usage", settings["spoolman_report_partial_usage"]) + spoolman_changed = ( + "spoolman_enabled" in settings + or "spoolman_url" in settings + ) + await db.commit() db.expire_all() + if spoolman_changed: + from backend.app.services.location_service import maybe_sync_spoolman_locations + + if await maybe_sync_spoolman_locations(db): + await db.commit() + # Return updated settings return await get_spoolman_settings(db) diff --git a/backend/app/api/routes/spoolman_inventory.py b/backend/app/api/routes/spoolman_inventory.py index 2a7cf2033..ab487895e 100644 --- a/backend/app/api/routes/spoolman_inventory.py +++ b/backend/app/api/routes/spoolman_inventory.py @@ -34,6 +34,7 @@ from backend.app.api.routes._spoolman_helpers import ( from backend.app.core.auth import RequirePermissionIfAuthEnabled from backend.app.core.database import get_db from backend.app.core.permissions import Permission +from backend.app.core.websocket import ws_manager from backend.app.models.ams_label import AmsLabel from backend.app.models.printer import Printer from backend.app.models.settings import Settings @@ -42,6 +43,11 @@ from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment from backend.app.models.user import User from backend.app.schemas.spool import SpoolKProfileBase from backend.app.schemas.spoolman import SpoolmanFilamentPatch, SpoolmanSlotAssignmentEnriched +from backend.app.services.location_service import ( + enrich_spool_dicts_with_location_id, + maybe_sync_spoolman_locations, + resolve_spoolman_location_string, +) from backend.app.services.printer_manager import printer_manager from backend.app.services.slicer_filament_resolver import resolve_slicer_filament from backend.app.services.spoolman import ( @@ -307,6 +313,7 @@ class SpoolmanInventoryCreate(BaseModel): note: str | None = Field(None, max_length=1000) cost_per_kg: float | None = Field(None, ge=0.0, le=1_000_000.0) storage_location: str | None = Field(None, max_length=255) + location_id: int | None = Field(None, gt=0) # BambuStudio slicer preset for this spool. Spoolman has no native field # for this, so we persist it under the bambu_slicer_filament[_name] keys # in the spool's extra dict and read it back in _map_spoolman_spool. @@ -349,6 +356,7 @@ class SpoolmanInventoryUpdate(BaseModel): tag_uid: str | None = Field(None, min_length=8, max_length=30, pattern=r"^[0-9A-Fa-f]+$") tray_uuid: str | None = Field(None, min_length=32, max_length=32, pattern=r"^[0-9A-Fa-f]+$") storage_location: str | None = Field(None, max_length=255) + location_id: int | None = Field(None, gt=0) # BambuStudio slicer preset — persisted to Spoolman extra dict (see Create # schema). Pass an empty string to clear; null/omitted leaves unchanged. slicer_filament: str | None = Field(None, max_length=128) @@ -430,6 +438,13 @@ async def list_spools( ) -> list[dict]: """Return all Spoolman spools in the InventorySpool format.""" client = await _get_client(db) + # Sync after we have the route-resolved client so tests that patch the + # route module's get_spoolman_client/init_spoolman_client also catch the + # sync's client lookup — otherwise the location_service path imports from + # backend.app.services.spoolman directly and bypasses the patch. + if await maybe_sync_spoolman_locations(db, client=client): + await db.commit() + async with _translate_spoolman_errors(): spools = await client.get_all_spools(allow_archived=include_archived) @@ -451,6 +466,7 @@ async def list_spools( for m in mapped: m["k_profiles"] = kp_by_spool.get(m["id"], []) + await enrich_spool_dicts_with_location_id(db, mapped) return mapped @@ -472,6 +488,7 @@ async def get_spool( kp_result = await db.execute(select(SpoolmanKProfile).where(SpoolmanKProfile.spoolman_spool_id == spool_id)) mapped["k_profiles"] = [_k_profile_to_dict(kp) for kp in kp_result.scalars().all()] + await enrich_spool_dicts_with_location_id(db, [mapped]) return mapped @@ -507,6 +524,18 @@ async def create_spool( client = await _get_client(db) filament_id = await _resolve_filament_id(data, client) + storage_location = data.storage_location + if "location_id" in data.model_fields_set or "storage_location" in data.model_fields_set: + try: + storage_location, _ = await resolve_spoolman_location_string( + db, + location_id=data.location_id, + storage_location=data.storage_location, + fields_set=set(data.model_fields_set), + ) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + remaining = max(0.0, data.label_weight - data.weight_used) try: async with _translate_spoolman_errors(): @@ -514,7 +543,7 @@ async def create_spool( filament_id=filament_id, remaining_weight=remaining, comment=data.note or None, - location=data.storage_location or None, + location=storage_location or None, ) except HTTPException as exc: if exc.status_code == 404 and data.spoolman_filament_id is not None: @@ -556,6 +585,7 @@ async def create_spool( ) result = _map_spoolman_spool(spool) + await ws_manager.broadcast({"type": "inventory_changed"}) if price_warnings: return JSONResponse(status_code=207, content={**result, "warnings": price_warnings}) return result @@ -581,6 +611,18 @@ async def bulk_create_spools( ) from exc raise + storage_location = data.storage_location + if "location_id" in data.model_fields_set or "storage_location" in data.model_fields_set: + try: + storage_location, _ = await resolve_spoolman_location_string( + db, + location_id=data.location_id, + storage_location=data.storage_location, + fields_set=set(data.model_fields_set), + ) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + remaining = max(0.0, data.label_weight - data.weight_used) created: list[dict] = [] failures: list[str] = [] @@ -590,7 +632,7 @@ async def bulk_create_spools( filament_id=filament_id, remaining_weight=remaining, comment=data.note or None, - location=data.storage_location or None, + location=storage_location or None, ) except (SpoolmanUnavailableError, SpoolmanClientError, SpoolmanNotFoundError) as exc: logger.warning("Bulk spool creation: one spool failed: %s", exc) @@ -613,6 +655,8 @@ async def bulk_create_spools( if not created: raise HTTPException(status_code=500, detail="Failed to create any spools in Spoolman") + await ws_manager.broadcast({"type": "inventory_changed"}) + if len(created) < payload.quantity: # Some spool creations failed — return 207 Multi-Status so the caller # can distinguish a full success from a partial one and show a useful message. @@ -679,8 +723,18 @@ async def update_spool( synthetic_used = float(current.get("used_weight") or 0) weight_used = data.weight_used if data.weight_used is not None else synthetic_used note = data.note if data.note is not None else current.get("comment") - storage_location_changed = "storage_location" in data.model_fields_set - storage_location = data.storage_location if storage_location_changed else None + storage_location_changed = "storage_location" in data.model_fields_set or "location_id" in data.model_fields_set + storage_location = data.storage_location if "storage_location" in data.model_fields_set else None + if storage_location_changed: + try: + storage_location, _ = await resolve_spoolman_location_string( + db, + location_id=data.location_id, + storage_location=storage_location, + fields_set=set(data.model_fields_set), + ) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc color_hex = rgba[:6] @@ -817,6 +871,7 @@ async def update_spool( async with _translate_spoolman_errors(): updated = await client.merge_spool_extra(spool_id, new_extra) + await ws_manager.broadcast({"type": "inventory_changed"}) return _map_spoolman_spool(updated) @@ -830,6 +885,7 @@ async def delete_spool( client = await _get_client(db) async with _translate_spoolman_errors(): await client.delete_spool(spool_id) + await ws_manager.broadcast({"type": "inventory_changed"}) return {"status": "deleted"} @@ -844,10 +900,12 @@ async def archive_spool( async with _translate_spoolman_errors(): spool = await client.set_spool_archived(spool_id, archived=True) try: - return _map_spoolman_spool(spool) + mapped = _map_spoolman_spool(spool) except ValueError as exc: logger.warning("Malformed Spoolman spool (id=%r): %s", spool_id, exc) raise HTTPException(status_code=502, detail="Spoolman returned malformed spool data") from exc + await ws_manager.broadcast({"type": "inventory_changed"}) + return mapped @router.post("/spools/{spool_id}/restore") @@ -861,10 +919,12 @@ async def restore_spool( async with _translate_spoolman_errors(): spool = await client.set_spool_archived(spool_id, archived=False) try: - return _map_spoolman_spool(spool) + mapped = _map_spoolman_spool(spool) except ValueError as exc: logger.warning("Malformed Spoolman spool (id=%r): %s", spool_id, exc) raise HTTPException(status_code=502, detail="Spoolman returned malformed spool data") from exc + await ws_manager.broadcast({"type": "inventory_changed"}) + return mapped @router.post("/spools/{spool_id}/reset-consumed-counter") @@ -888,10 +948,12 @@ async def reset_spool_consumed_counter( async with _translate_spoolman_errors(): spool = await client.reset_spool_usage(spool_id) try: - return _map_spoolman_spool(spool) + mapped = _map_spoolman_spool(spool) except ValueError as exc: logger.warning("Malformed Spoolman spool (id=%r): %s", spool_id, exc) raise HTTPException(status_code=502, detail="Spoolman returned malformed spool data") from exc + await ws_manager.broadcast({"type": "inventory_changed"}) + return mapped @router.post("/spools/reset-consumed-counter-bulk") @@ -922,6 +984,8 @@ async def bulk_reset_spool_consumed_counter( reset_count += 1 except HTTPException as exc: logger.warning("Spoolman reset-consumed-counter failed for spool %s: %s", spool_id, exc.detail) + if reset_count: + await ws_manager.broadcast({"type": "inventory_changed"}) return {"reset": reset_count} @@ -955,6 +1019,7 @@ async def sync_spool_weight( upd_filament = updated.get("filament") or {} label_weight = _safe_int(upd_filament.get("weight"), 1000) weight_used = max(0.0, label_weight - remaining) + await ws_manager.broadcast({"type": "inventory_changed"}) return {"status": "ok", "weight_used": weight_used} @@ -997,6 +1062,7 @@ async def link_tag_to_spoolman_spool( updated = await client.update_spool_full(spool_id=spool_id, extra=cur_extra) logger.info("Linked tag %s to Spoolman spool %s", tag, spool_id) + await ws_manager.broadcast({"type": "inventory_changed"}) return _map_spoolman_spool(updated) diff --git a/backend/app/core/database.py b/backend/app/core/database.py index 857baa359..098890c3b 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -181,6 +181,7 @@ async def init_db(): kprofile_note, library, local_preset, + location, long_lived_token, maintenance, notification, @@ -2938,6 +2939,110 @@ async def run_migrations(conn): ) ) + # Migration: structured storage locations (#1004). Flat catalog of physical + # shelves/drawers; spool.location_id FK with storage_location kept denormalized. + await _safe_execute( + conn, + """ + CREATE TABLE IF NOT EXISTS locations ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name VARCHAR(255) NOT NULL UNIQUE, + name_key VARCHAR(255), + identifier VARCHAR(100), + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP + ) + """ + if is_sqlite() + else """ + CREATE TABLE IF NOT EXISTS locations ( + id SERIAL PRIMARY KEY, + name VARCHAR(255) NOT NULL UNIQUE, + name_key VARCHAR(255), + identifier VARCHAR(100), + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP + ) + """, + ) + await _safe_execute(conn, "ALTER TABLE locations ADD COLUMN name_key VARCHAR(255)") + await _safe_execute(conn, "CREATE UNIQUE INDEX IF NOT EXISTS ix_locations_name_key ON locations (name_key)") + await _safe_execute(conn, "ALTER TABLE spool ADD COLUMN location_id INTEGER REFERENCES locations(id)") + await _safe_execute(conn, "CREATE INDEX IF NOT EXISTS ix_spool_location_id ON spool (location_id)") + + # Backfill name_key on legacy rows FIRST. If a pre-existing locations + # row was manually inserted before this migration ran, its name_key is + # NULL. The dedup INSERT below would then be silently skipped by + # UNIQUE(name) (legacy row already has the name), AND the spool-link + # UPDATE that joins on name_key would miss it. Doing this backfill BEFORE + # the INSERT keeps the join consistent on both branches of the migration. + async with conn.begin_nested(): + await conn.execute( + text( + """ + UPDATE locations + SET name_key = LOWER(TRIM(name)) + WHERE name_key IS NULL OR TRIM(name_key) = '' + """ + ) + ) + + # Backfill locations from existing free-text storage_location values. + # GROUP BY name_key so case variants ("Drybox 1" / "DRYBOX 1") collapse to + # one row; INSERT OR IGNORE / ON CONFLICT keeps the migration idempotent. + _location_backfill_sql = ( + """ + INSERT OR IGNORE INTO locations (name, name_key, created_at, updated_at) + SELECT MIN(TRIM(storage_location)), LOWER(TRIM(storage_location)), CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + FROM spool + WHERE TRIM(COALESCE(storage_location, '')) != '' + GROUP BY LOWER(TRIM(storage_location)) + """ + if is_sqlite() + else """ + INSERT INTO locations (name, name_key, created_at, updated_at) + SELECT MIN(TRIM(storage_location)), LOWER(TRIM(storage_location)), CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + FROM spool + WHERE TRIM(COALESCE(storage_location, '')) != '' + GROUP BY LOWER(TRIM(storage_location)) + ON CONFLICT (name_key) DO NOTHING + """ + ) + async with conn.begin_nested(): + await conn.execute(text(_location_backfill_sql)) + await conn.execute( + text( + """ + UPDATE spool + SET location_id = ( + SELECT l.id FROM locations l + WHERE l.name_key = LOWER(TRIM(spool.storage_location)) + LIMIT 1 + ) + WHERE TRIM(COALESCE(storage_location, '')) != '' + AND location_id IS NULL + """ + ) + ) + + # Sanity check: any spools that still have a free-text storage_location + # but no location_id link mean a row slipped through the dedup INSERT + # (most likely a pre-existing manually-inserted locations row with a + # hostile name shape that the UNIQUE(name) check tripped on). Surface + # the count so ops can investigate — the user won't see those spools in + # location-filtered queries until they're manually linked or re-saved. + orphan_count_row = await conn.execute( + text("SELECT COUNT(*) FROM spool WHERE TRIM(COALESCE(storage_location, '')) != '' AND location_id IS NULL") + ) + orphan_count = orphan_count_row.scalar() or 0 + if orphan_count: + logger.warning( + "Storage-location migration left %d spool(s) with free-text storage_location " + "but no location_id link. Re-save those spools or merge the orphaned location " + "names manually.", + orphan_count, + ) + async def seed_notification_templates(): """Seed default notification templates if they don't exist.""" diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 9839776d3..2ea5efb99 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -10,6 +10,7 @@ from backend.app.models.group import Group, user_groups from backend.app.models.kprofile_note import KProfileNote from backend.app.models.library import LibraryFile, LibraryFolder from backend.app.models.local_preset import LocalPreset +from backend.app.models.location import Location from backend.app.models.long_lived_token import LongLivedToken from backend.app.models.maintenance import MaintenanceHistory, MaintenanceType, PrinterMaintenance from backend.app.models.notification import NotificationLog @@ -55,6 +56,7 @@ __all__ = [ "PrintBatch", "LibraryFolder", "LibraryFile", + "Location", "User", "Group", "user_groups", diff --git a/backend/app/models/location.py b/backend/app/models/location.py new file mode 100644 index 000000000..b0673321d --- /dev/null +++ b/backend/app/models/location.py @@ -0,0 +1,27 @@ +from datetime import datetime +from typing import TYPE_CHECKING + +from sqlalchemy import DateTime, String, func +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from backend.app.core.database import Base + +if TYPE_CHECKING: + from backend.app.models.spool import Spool + + +class Location(Base): + """Physical storage location for filament spools (shelf, drawer, drybox, etc.).""" + + __tablename__ = "locations" + + id: Mapped[int] = mapped_column(primary_key=True) + name: Mapped[str] = mapped_column(String(255), nullable=False, unique=True) + # Case-insensitive uniqueness — LOWER(TRIM(name)); enforced via migration index. + name_key: Mapped[str] = mapped_column(String(255), nullable=False, unique=True, index=True) + # Reserved for Phase 3 RFID shelf tags — unused in Phase 1. + identifier: Mapped[str | None] = mapped_column(String(100)) + created_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now()) + updated_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now(), onupdate=func.now()) + + spools: Mapped[list["Spool"]] = relationship(back_populates="location") diff --git a/backend/app/models/spool.py b/backend/app/models/spool.py index edaa9ef48..3fde2b9ba 100644 --- a/backend/app/models/spool.py +++ b/backend/app/models/spool.py @@ -1,6 +1,6 @@ from datetime import datetime -from sqlalchemy import Boolean, DateTime, Float, Integer, String, func +from sqlalchemy import Boolean, DateTime, Float, ForeignKey, Integer, String, func from sqlalchemy.orm import Mapped, mapped_column, relationship from backend.app.core.database import Base @@ -61,6 +61,7 @@ class Spool(Base): cost_per_kg: Mapped[float | None] = mapped_column(Float) # Cost per kilogram storage_location: Mapped[str | None] = mapped_column(String(255)) # User-editable storage location + location_id: Mapped[int | None] = mapped_column(ForeignKey("locations.id"), index=True) last_used: Mapped[datetime | None] = mapped_column(DateTime) # Last time this spool was used in a print encode_time: Mapped[datetime | None] = mapped_column(DateTime) # When spool was encoded/written to tag @@ -74,7 +75,9 @@ class Spool(Base): k_profiles: Mapped[list["SpoolKProfile"]] = relationship(back_populates="spool", cascade="all, delete-orphan") assignments: Mapped[list["SpoolAssignment"]] = relationship(back_populates="spool", cascade="all, delete-orphan") + location: Mapped["Location | None"] = relationship(back_populates="spools") +from backend.app.models.location import Location # noqa: E402 from backend.app.models.spool_assignment import SpoolAssignment # noqa: E402 from backend.app.models.spool_k_profile import SpoolKProfile # noqa: E402 diff --git a/backend/app/schemas/location.py b/backend/app/schemas/location.py new file mode 100644 index 000000000..66bbd49fb --- /dev/null +++ b/backend/app/schemas/location.py @@ -0,0 +1,39 @@ +from datetime import datetime + +from pydantic import BaseModel, Field, field_validator + +from backend.app.services.location_service import normalize_location_name + + +class LocationCreate(BaseModel): + name: str = Field(..., min_length=1, max_length=255) + identifier: str | None = Field(default=None, max_length=100) + + @field_validator("name") + @classmethod + def validate_name(cls, v: str) -> str: + return normalize_location_name(v) + + +class LocationUpdate(BaseModel): + name: str | None = Field(default=None, min_length=1, max_length=255) + identifier: str | None = Field(default=None, max_length=100) + + @field_validator("name") + @classmethod + def validate_name(cls, v: str | None) -> str | None: + if v is None: + return None + return normalize_location_name(v) + + +class LocationResponse(BaseModel): + id: int + name: str + identifier: str | None = None + spool_count: int = 0 + created_at: datetime + updated_at: datetime + + class Config: + from_attributes = True diff --git a/backend/app/schemas/spool.py b/backend/app/schemas/spool.py index 3d5a90e68..9a2a7c930 100644 --- a/backend/app/schemas/spool.py +++ b/backend/app/schemas/spool.py @@ -125,6 +125,7 @@ class SpoolBase(BaseModel): # assignment). Column has lived on the ORM since the inventory rework # but was missing from this schema, so writes were silently dropped (#1291). storage_location: str | None = Field(default=None, max_length=255) + location_id: int | None = Field(default=None, gt=0) class SpoolCreate(SpoolBase): @@ -174,6 +175,7 @@ class SpoolUpdate(BaseModel): category: str | None = Field(default=None, max_length=50) low_stock_threshold_pct: int | None = Field(default=None, ge=1, le=99) storage_location: str | None = Field(default=None, max_length=255) + location_id: int | None = Field(default=None, gt=0) class SpoolKProfileBase(BaseModel): diff --git a/backend/app/services/location_service.py b/backend/app/services/location_service.py new file mode 100644 index 000000000..97c862d10 --- /dev/null +++ b/backend/app/services/location_service.py @@ -0,0 +1,354 @@ +"""Storage location catalog — single write path for spool location fields (#1004).""" + +from __future__ import annotations + +import logging +import time +from dataclasses import dataclass + +import httpx +from sqlalchemy import func, select, update +from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.models.location import Location +from backend.app.models.spool import Spool + +logger = logging.getLogger(__name__) + +DUPLICATE_LOCATION_NAME = "A location with this name already exists" + + +def normalize_location_name(name: str) -> str: + trimmed = name.strip() + if not trimmed: + raise ValueError("name must not be empty") + return trimmed + + +def location_name_key(name: str) -> str: + """Case-insensitive lookup key stored on Location.name_key.""" + return normalize_location_name(name).lower() + + +def assign_location_name(location: Location, name: str) -> None: + normalized = normalize_location_name(name) + location.name = normalized + location.name_key = location_name_key(normalized) + + +@dataclass(frozen=True) +class SpoolLocationFields: + """Canonical spool location state: FK + denormalized string for Spoolman/display.""" + + location_id: int | None + storage_location: str | None + + +async def get_location_by_id(db: AsyncSession, location_id: int) -> Location | None: + result = await db.execute(select(Location).where(Location.id == location_id)) + return result.scalar_one_or_none() + + +async def get_location_by_name(db: AsyncSession, name: str) -> Location | None: + key = location_name_key(name) + result = await db.execute(select(Location).where(Location.name_key == key)) + return result.scalar_one_or_none() + + +async def get_locations_by_name_keys(db: AsyncSession, keys: set[str]) -> dict[str, Location]: + if not keys: + return {} + result = await db.execute(select(Location).where(Location.name_key.in_(keys))) + return {loc.name_key: loc for loc in result.scalars().all()} + + +async def _create_location_or_get_existing(db: AsyncSession, normalized: str) -> Location: + """Insert a location row, returning the winner on concurrent name_key collision.""" + existing = await get_location_by_name(db, normalized) + if existing: + return existing + location = Location() + assign_location_name(location, normalized) + try: + async with db.begin_nested(): + db.add(location) + await db.flush() + return location + except IntegrityError as exc: + winner = await get_location_by_name(db, normalized) + if winner: + return winner + raise ValueError(DUPLICATE_LOCATION_NAME) from exc + + +async def _insert_location_if_absent(db: AsyncSession, name: str) -> bool: + """Stage a new location row when absent. Returns True when one was added.""" + normalized = normalize_location_name(name) + if await get_location_by_name(db, normalized): + return False + location = Location() + assign_location_name(location, normalized) + try: + async with db.begin_nested(): + db.add(location) + await db.flush() + return True + except IntegrityError: + # Race: another writer inserted the same name between our check and + # flush. The row already exists by definition — surface as "not added" + # rather than re-raising. Anything else (NULL constraint, FK, check + # constraint) would be a programming bug — re-fetch to verify so we + # don't silently drop unrelated IntegrityErrors. + if await get_location_by_name(db, normalized): + return False + logger.warning("IntegrityError on insert of location %r without surviving row", normalized) + raise + + +async def resolve_location_by_name(db: AsyncSession, name: str, *, create: bool = True) -> Location | None: + """Find a location by name (case-insensitive), optionally creating it.""" + normalized = normalize_location_name(name) + existing = await get_location_by_name(db, normalized) + if existing: + return existing + if not create: + return None + return await _create_location_or_get_existing(db, normalized) + + +async def resolve_spool_location_fields( + db: AsyncSession, + *, + location_id: int | None = None, + storage_location: str | None = None, + fields_set: set[str], +) -> SpoolLocationFields | None: + """Resolve location_id + storage_location from API input. + + ``location_id`` wins when both fields appear in ``fields_set``. + Returns ``None`` when neither location field was provided. + """ + if "location_id" in fields_set: + if location_id is None: + return SpoolLocationFields(location_id=None, storage_location=None) + loc = await get_location_by_id(db, location_id) + if not loc: + raise ValueError(f"Location {location_id} not found") + return SpoolLocationFields(location_id=loc.id, storage_location=loc.name) + + if "storage_location" in fields_set: + if not storage_location: + return SpoolLocationFields(location_id=None, storage_location=None) + loc = await resolve_location_by_name(db, storage_location) + if not loc: + return SpoolLocationFields(location_id=None, storage_location=None) + return SpoolLocationFields(location_id=loc.id, storage_location=loc.name) + + return None + + +async def prepare_internal_spool_payload(db: AsyncSession, data: dict, fields_set: set[str]) -> dict: + """Apply resolved location fields before creating or updating an internal spool.""" + payload = dict(data) + resolved = await resolve_spool_location_fields( + db, + location_id=payload.get("location_id"), + storage_location=payload.get("storage_location"), + fields_set=fields_set, + ) + if resolved is not None: + payload["location_id"] = resolved.location_id + payload["storage_location"] = resolved.storage_location + return payload + + +async def resolve_spoolman_location_string( + db: AsyncSession, + *, + location_id: int | None = None, + storage_location: str | None = None, + fields_set: set[str], +) -> tuple[str | None, bool]: + """Return (Spoolman location string, changed) for proxy writes.""" + resolved = await resolve_spool_location_fields( + db, + location_id=location_id, + storage_location=storage_location, + fields_set=fields_set, + ) + if resolved is None: + return None, False + return resolved.storage_location, True + + +async def count_internal_spools_at_location(db: AsyncSession, location_id: int) -> int: + result = await db.execute( + select(func.count()) + .select_from(Spool) + .where( + Spool.location_id == location_id, + Spool.archived_at.is_(None), + ) + ) + return int(result.scalar() or 0) + + +async def count_spools_at_location_by_name(db: AsyncSession, name: str) -> int: + normalized = name.strip() + if not normalized: + return 0 + result = await db.execute( + select(func.count()) + .select_from(Spool) + .where( + Spool.archived_at.is_(None), + func.lower(func.trim(Spool.storage_location)) == normalized.lower(), + ) + ) + return int(result.scalar() or 0) + + +async def enrich_spool_dicts_with_location_id(db: AsyncSession, spools: list[dict]) -> None: + """Attach location_id to mapped Spoolman-style spool dicts in place.""" + keys = {location_name_key(s["storage_location"]) for s in spools if (s.get("storage_location") or "").strip()} + if not keys: + for s in spools: + s["location_id"] = None + return + + by_key = await get_locations_by_name_keys(db, keys) + for s in spools: + raw = (s.get("storage_location") or "").strip() + if not raw: + s["location_id"] = None + continue + loc = by_key.get(location_name_key(raw)) + s["location_id"] = loc.id if loc else None + + +async def rename_location(db: AsyncSession, location: Location, new_name: str) -> Location: + normalized = normalize_location_name(new_name) + existing = await get_location_by_name(db, normalized) + if existing and existing.id != location.id: + raise ValueError(DUPLICATE_LOCATION_NAME) + + old_name = location.name + # Mirror the SQL TRIM on the Python side so a legacy row whose + # `storage_location` has trailing whitespace still matches against the + # `old_name` we just lifted off the Location row. Without `.strip()` the + # equality is asymmetric (SQL strips the column; Python doesn't) and + # legacy rows quietly fall out of the rename cascade. + old_name_key = old_name.strip().lower() + assign_location_name(location, normalized) + await db.execute(update(Spool).where(Spool.location_id == location.id).values(storage_location=normalized)) + # Keep legacy rows in sync when only storage_location was set. + await db.execute( + update(Spool) + .where( + Spool.location_id.is_(None), + func.lower(func.trim(Spool.storage_location)) == old_name_key, + ) + .values(storage_location=normalized, location_id=location.id) + ) + try: + await db.flush() + except IntegrityError as exc: + raise ValueError(DUPLICATE_LOCATION_NAME) from exc + return location + + +async def sync_locations_from_spoolman(db: AsyncSession, client) -> bool: + """Import distinct Spoolman location strings into the local catalog. + + Returns True when new rows were staged (caller must commit). Logs and + returns False on Spoolman fetch failures so the calling read path keeps + serving the local catalog instead of 500ing; bare-Exception swallow used + to be the shape here and hid both transport errors and shape regressions. + """ + from backend.app.services.spoolman import SpoolmanClientError, SpoolmanUnavailableError + + try: + names = await client.get_distinct_locations() + except (SpoolmanUnavailableError, SpoolmanClientError, httpx.HTTPError) as exc: + logger.warning("location sync from Spoolman failed: %s", exc) + return False + + # Collapse case variants before insert — Spoolman may return both + # "Drybox 1" and "DRYBOX 1" in the same payload. + by_key: dict[str, str] = {} + for raw in names: + name = (raw or "").strip() + if not name: + continue + key = location_name_key(name) + if key not in by_key: + by_key[key] = name + + changed = False + for name in by_key.values(): + if await _insert_location_if_absent(db, name): + changed = True + return changed + + +# Per-URL last-sync timestamp guard. Calling list_spools runs the sync, so on +# a polling UI without this guard every refetch round-trips to Spoolman and +# opens a write transaction — measurable latency and SQLite write contention. +# 60s is long enough to absorb dashboard polling, short enough that a manual +# spool rename in Spoolman shows up on the next minute's refresh. +_SPOOLMAN_LOCATION_SYNC_TTL_SECONDS = 60.0 +_spoolman_location_sync_last_run: dict[str, float] = {} + + +def _spoolman_location_sync_cache_clear() -> None: + """Test hook: drop the TTL cache so each test starts from a clean slate.""" + _spoolman_location_sync_last_run.clear() + + +async def maybe_sync_spoolman_locations(db: AsyncSession, *, client=None) -> bool: + """Sync Spoolman location names into the local catalog when integration is enabled. + + Pass ``client`` when the caller has already resolved one (the GET /spools + route does); otherwise the function falls back to ``init_spoolman_client``. + Passing the route's client keeps test fixtures honest — without it, the + fall-back path imports from ``backend.app.services.spoolman`` directly and + bypasses any patch that targets the route module's alias, which causes + real TCP connects to whatever ``spoolman_url`` happens to point at. + """ + from backend.app.api.routes._spoolman_helpers import assert_safe_spoolman_url + from backend.app.models.settings import Settings + + result = await db.execute(select(Settings)) + settings = {s.key: s.value for s in result.scalars().all()} + if settings.get("spoolman_enabled", "false").lower() != "true": + return False + url = settings.get("spoolman_url", "").strip() + if not url: + return False + + # Debounce: skip the round-trip when we synced this URL recently. + cache_key = url.rstrip("/") + last_run = _spoolman_location_sync_last_run.get(cache_key, 0.0) + now = time.monotonic() + if now - last_run < _SPOOLMAN_LOCATION_SYNC_TTL_SECONDS: + return False + + try: + assert_safe_spoolman_url(url) + except ValueError as exc: + logger.warning("Spoolman URL rejected by SSRF guard during location sync: %s", exc) + return False + + if client is None: + from backend.app.services.spoolman import get_spoolman_client, init_spoolman_client + + client = await get_spoolman_client() + if not client or client.base_url != cache_key: + client = await init_spoolman_client(url) + if not client: + return False + + changed = await sync_locations_from_spoolman(db, client) + _spoolman_location_sync_last_run[cache_key] = now + return changed diff --git a/backend/app/services/spoolman.py b/backend/app/services/spoolman.py index d0b7cad3f..90d151abd 100644 --- a/backend/app/services/spoolman.py +++ b/backend/app/services/spoolman.py @@ -544,6 +544,86 @@ class SpoolmanClient: params["allow_archived"] = "true" return await self._get_with_retry("/spool", params=params or None) + async def get_distinct_locations(self) -> list[str]: + """Return distinct location strings currently assigned to Spoolman spools. + + Spoolman's `/location` endpoint shape varies across versions: older + releases return `list[str]`, newer ones return `list[dict]` with a + `name` field. Normalize to `list[str]` so callers can iterate without + runtime shape checks. + """ + raw = await self._get_with_retry("/location") + if not isinstance(raw, list): + return [] + names: list[str] = [] + for entry in raw: + if isinstance(entry, str): + names.append(entry) + elif isinstance(entry, dict): + name = entry.get("name") + if isinstance(name, str): + names.append(name) + return names + + async def rename_location(self, current_name: str, new_name: str) -> int: + """Bulk-rename a location string on all Spoolman spools. + + Tries the bulk `PATCH /location/{name}` endpoint first. Spoolman + versions older than ~0.16 don't expose it and respond 404/405 — in + that case fall back to iterating every spool currently at + ``current_name`` and PATCHing each one's ``location`` field directly. + Returns the number of spools renamed (or 0 if the bulk endpoint + succeeded without enumerating). + """ + from urllib.parse import quote + + encoded = quote(current_name, safe="") + client = await self._get_client() + try: + response = await client.patch( + f"{self.api_url}/location/{encoded}", + json={"name": new_name}, + ) + response.raise_for_status() + return 0 + except httpx.HTTPStatusError as exc: + if exc.response.status_code not in (404, 405): + raise + logger.info( + "Spoolman bulk-rename endpoint unavailable (status %d); falling back to per-spool PATCH", + exc.response.status_code, + ) + + # Per-spool fallback: enumerate every spool currently at the old name + # and PATCH each. Keep going on individual failures so a single + # already-deleted spool doesn't strand the rest at the old name — + # collect errors and re-raise as a single SpoolmanClientError if any + # leftover survives. + spools = await self.get_all_spools(allow_archived=True) + renamed = 0 + failures: list[str] = [] + for spool in spools: + if (spool.get("location") or "").strip() != current_name: + continue + try: + await self._request_spool( + "PATCH", + spool["id"], + json_body={"location": new_name}, + operation="rename-location", + ) + renamed += 1 + except SpoolmanNotFoundError: + continue + except Exception as exc: # noqa: BLE001 — accumulate and re-raise below + failures.append(f"spool {spool.get('id')}: {exc}") + if failures: + raise SpoolmanClientError( + f"Spoolman rename fallback failed for {len(failures)} spool(s): {'; '.join(failures[:3])}", + status_code=502, + ) + return renamed + async def delete_spool(self, spool_id: int) -> None: """Delete a spool from Spoolman.""" await self._request_spool("DELETE", spool_id, operation="delete") diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 6c48bd30c..9e31a8f53 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -81,6 +81,20 @@ def mfa_encryption_isolation(monkeypatch, tmp_path): enc_mod._key_source = None +@pytest.fixture(autouse=True) +def reset_spoolman_location_sync_cache(): + """Drop the per-URL Spoolman location-sync TTL cache between tests. + + Without this, a test that runs the sync against `http://localhost:7912` + will skip the sync in any later test that uses the same URL within 60 + real seconds — test ordering would then leak assertions across runs.""" + from backend.app.services.location_service import _spoolman_location_sync_cache_clear + + _spoolman_location_sync_cache_clear() + yield + _spoolman_location_sync_cache_clear() + + @pytest.fixture(scope="session") def event_loop(): """Create an instance of the default event loop for each test session.""" diff --git a/backend/tests/integration/test_auth_apikey_rbac.py b/backend/tests/integration/test_auth_apikey_rbac.py index b597c38cc..4f84e5f95 100644 --- a/backend/tests/integration/test_auth_apikey_rbac.py +++ b/backend/tests/integration/test_auth_apikey_rbac.py @@ -96,6 +96,7 @@ class TestApiKeyRbacAllowed: mock_client.base_url = "http://localhost:7912" mock_client.health_check = AsyncMock(return_value=True) mock_client.get_all_spools = AsyncMock(return_value=[]) + mock_client.get_distinct_locations = AsyncMock(return_value=[]) with patch( "backend.app.api.routes.spoolman_inventory._get_client", AsyncMock(return_value=mock_client), diff --git a/backend/tests/integration/test_locations_api.py b/backend/tests/integration/test_locations_api.py new file mode 100644 index 000000000..12dddf46f --- /dev/null +++ b/backend/tests/integration/test_locations_api.py @@ -0,0 +1,174 @@ +"""Integration tests for /inventory/locations (#1004).""" + +import pytest +from httpx import AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.models.location import Location +from backend.app.services.location_service import assign_location_name + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_locations_crud_and_spool_link(async_client: AsyncClient, db_session: AsyncSession): + create_resp = await async_client.post("/api/v1/inventory/locations", json={"name": "Shelf A"}) + assert create_resp.status_code == 201 + loc = create_resp.json() + assert loc["name"] == "Shelf A" + assert loc["spool_count"] == 0 + + dup_resp = await async_client.post("/api/v1/inventory/locations", json={"name": "shelf a"}) + assert dup_resp.status_code == 409 + + spool_resp = await async_client.post( + "/api/v1/inventory/spools", + json={"material": "PLA", "location_id": loc["id"]}, + ) + assert spool_resp.status_code == 200 + spool = spool_resp.json() + assert spool["location_id"] == loc["id"] + assert spool["storage_location"] == "Shelf A" + + list_resp = await async_client.get("/api/v1/inventory/locations") + assert list_resp.status_code == 200 + listed = {item["id"]: item for item in list_resp.json()} + assert listed[loc["id"]]["spool_count"] == 1 + + delete_resp = await async_client.delete(f"/api/v1/inventory/locations/{loc['id']}") + assert delete_resp.status_code == 409 + + clear_resp = await async_client.patch( + f"/api/v1/inventory/spools/{spool['id']}", + json={"location_id": None}, + ) + assert clear_resp.status_code == 200 + + delete_resp2 = await async_client.delete(f"/api/v1/inventory/locations/{loc['id']}") + assert delete_resp2.status_code == 200 + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_rename_location_updates_spool_count(async_client: AsyncClient): + create_resp = await async_client.post("/api/v1/inventory/locations", json={"name": "Old Name"}) + loc = create_resp.json() + + await async_client.post( + "/api/v1/inventory/spools", + json={"material": "PLA", "location_id": loc["id"]}, + ) + + list_before = await async_client.get("/api/v1/inventory/locations") + by_id = {item["id"]: item for item in list_before.json()} + assert by_id[loc["id"]]["spool_count"] == 1 + + rename_resp = await async_client.patch( + f"/api/v1/inventory/locations/{loc['id']}", + json={"name": "New Name"}, + ) + assert rename_resp.status_code == 200 + assert rename_resp.json()["name"] == "New Name" + assert rename_resp.json()["spool_count"] == 1 + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_rename_location_collision_returns_409(async_client: AsyncClient): + first = await async_client.post("/api/v1/inventory/locations", json={"name": "Shelf A"}) + second = await async_client.post("/api/v1/inventory/locations", json={"name": "Shelf B"}) + assert first.status_code == 201 + assert second.status_code == 201 + + collision = await async_client.patch( + f"/api/v1/inventory/locations/{second.json()['id']}", + json={"name": "Shelf A"}, + ) + assert collision.status_code == 409 + assert collision.json()["detail"] == "A location with this name already exists" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_create_location_duplicate_after_commit_returns_409(async_client: AsyncClient): + """Second create with the same name_key must return 409, not 500.""" + first = await async_client.post("/api/v1/inventory/locations", json={"name": "Race Shelf"}) + second = await async_client.post("/api/v1/inventory/locations", json={"name": "race shelf"}) + assert first.status_code == 201 + assert second.status_code == 409 + assert second.json()["detail"] == "A location with this name already exists" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_list_locations_is_read_only(async_client: AsyncClient, db_session: AsyncSession): + """GET /locations is a pure read — no catalog rows appear without explicit writes.""" + from sqlalchemy import func, select + + loc = Location() + assign_location_name(loc, "Local Only") + db_session.add(loc) + await db_session.commit() + + before = await db_session.scalar(select(func.count()).select_from(Location)) + resp = await async_client.get("/api/v1/inventory/locations") + after = await db_session.scalar(select(func.count()).select_from(Location)) + + assert resp.status_code == 200 + assert len(resp.json()) == 1 + assert before == after == 1 + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_update_location_404_on_unknown_id(async_client: AsyncClient): + resp = await async_client.patch( + "/api/v1/inventory/locations/99999", + json={"name": "Ghost"}, + ) + assert resp.status_code == 404 + assert resp.json()["detail"] == "Location not found" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_delete_location_404_on_unknown_id(async_client: AsyncClient): + resp = await async_client.delete("/api/v1/inventory/locations/99999") + assert resp.status_code == 404 + assert resp.json()["detail"] == "Location not found" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_locations_routes_require_auth_when_enabled(async_client: AsyncClient): + """All five /locations endpoints must return 401 when auth is enabled and + no credentials are presented. Mirror of the pattern from + test_queue_start_user_attribution._enable_auth_with_admin — required by + project policy: every permission-gated route gets a fail-closed test on + first ship, no follow-ups (the two CVSS 9.8/9.9 advisories shipped from + this exact gap).""" + await async_client.post( + "/api/v1/auth/setup", + json={ + "auth_enabled": True, + "admin_username": "locations1505admin", + "admin_password": "AdminPass1!", + }, + ) + + # GET /locations — read-gated + list_resp = await async_client.get("/api/v1/inventory/locations") + assert list_resp.status_code == 401, list_resp.text + + # POST /locations — write-gated + create_resp = await async_client.post("/api/v1/inventory/locations", json={"name": "Locked"}) + assert create_resp.status_code == 401, create_resp.text + + # PATCH /locations/{id} — write-gated. Use a synthetic id; the auth gate + # runs before the not-found check, so 401 is the correct expectation even + # when the id doesn't exist. + patch_resp = await async_client.patch("/api/v1/inventory/locations/99999", json={"name": "Locked2"}) + assert patch_resp.status_code == 401, patch_resp.text + + # DELETE /locations/{id} — write-gated + delete_resp = await async_client.delete("/api/v1/inventory/locations/99999") + assert delete_resp.status_code == 401, delete_resp.text diff --git a/backend/tests/integration/test_spoolman_inventory_api.py b/backend/tests/integration/test_spoolman_inventory_api.py index 94b3ba543..fe53bbc58 100644 --- a/backend/tests/integration/test_spoolman_inventory_api.py +++ b/backend/tests/integration/test_spoolman_inventory_api.py @@ -74,6 +74,10 @@ def mock_spoolman_client(): # branch override this on the fly. mock_client.is_filament_shared = AsyncMock(return_value=False) mock_client.ensure_extra_field = AsyncMock(return_value=True) + # list_spools calls maybe_sync_spoolman_locations which invokes + # get_distinct_locations on the route-resolved client. Empty list keeps the + # mock honest without staging phantom catalog rows. + mock_client.get_distinct_locations = AsyncMock(return_value=[]) with ( patch( diff --git a/backend/tests/integration/test_spoolman_k_profiles.py b/backend/tests/integration/test_spoolman_k_profiles.py index d36849e81..a6fe751d6 100644 --- a/backend/tests/integration/test_spoolman_k_profiles.py +++ b/backend/tests/integration/test_spoolman_k_profiles.py @@ -67,6 +67,7 @@ def mock_spoolman_client(): client.health_check = AsyncMock(return_value=True) client.get_spool = AsyncMock(return_value=SAMPLE_SPOOL) client.get_all_spools = AsyncMock(return_value=[SAMPLE_SPOOL]) + client.get_distinct_locations = AsyncMock(return_value=[]) with patch( "backend.app.api.routes.spoolman_inventory._get_client", diff --git a/backend/tests/integration/test_spoolman_slot_assignment_mqtt.py b/backend/tests/integration/test_spoolman_slot_assignment_mqtt.py index 81a7a1d1a..adff200c4 100644 --- a/backend/tests/integration/test_spoolman_slot_assignment_mqtt.py +++ b/backend/tests/integration/test_spoolman_slot_assignment_mqtt.py @@ -69,6 +69,7 @@ def mock_spoolman_client(): # #1457: assign route enumerates spools to clear stale fallback-tag links. client.get_spools = AsyncMock(return_value=[]) client.merge_spool_extra = AsyncMock(return_value={"id": 0, "extra": {}}) + client.get_distinct_locations = AsyncMock(return_value=[]) with patch( "backend.app.api.routes.spoolman_inventory._get_client", diff --git a/backend/tests/unit/test_location_migration.py b/backend/tests/unit/test_location_migration.py new file mode 100644 index 000000000..723ae84a8 --- /dev/null +++ b/backend/tests/unit/test_location_migration.py @@ -0,0 +1,209 @@ +"""Regression tests for storage-location migration backfill (#1004). + +Legacy installs may have free-text storage_location values that differ only +by case. The backfill must collapse them to one catalog row and stay +idempotent across restarts. +""" + +from __future__ import annotations + +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import create_async_engine + +from backend.app.core.database import run_migrations + + +@pytest.fixture(autouse=True) +def force_sqlite_dialect(monkeypatch): + from backend.app.core import db_dialect + + monkeypatch.setattr(db_dialect, "is_sqlite", lambda: True) + monkeypatch.setattr(db_dialect, "is_postgres", lambda: False) + from backend.app.core import database as database_module + + monkeypatch.setattr(database_module, "is_sqlite", lambda: True) + + +def _register_all_models(): + import backend.app.models # noqa: F401 + from backend.app.models import ( # noqa: F401 + external_link, + location, + print_log, + print_queue, + project_bom, + slot_preset, + spoolman_k_profile, + spoolman_slot_assignment, + virtual_printer, + ) + + +@pytest.fixture +async def engine_with_case_variant_spools(): + from backend.app.core.database import Base + + _register_all_models() + engine = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + await conn.execute(text("DELETE FROM locations")) + await conn.execute( + text( + """ + INSERT INTO spool ( + material, storage_location, label_weight, core_weight, + weight_used, weight_used_baseline, weight_locked + ) + VALUES ('PLA', 'Drybox 1', 1000, 250, 0, 0, 0), + ('PETG', 'DRYBOX 1', 1000, 250, 0, 0, 0) + """ + ) + ) + yield engine + await engine.dispose() + + +async def test_backfill_collapses_case_variant_storage_locations(engine_with_case_variant_spools): + async with engine_with_case_variant_spools.begin() as conn: + await run_migrations(conn) + + async with engine_with_case_variant_spools.connect() as conn: + loc_rows = (await conn.execute(text("SELECT id, name, name_key FROM locations ORDER BY id"))).all() + spool_rows = (await conn.execute(text("SELECT id, storage_location, location_id FROM spool ORDER BY id"))).all() + + assert len(loc_rows) == 1 + assert loc_rows[0].name_key == "drybox 1" + location_id = loc_rows[0].id + assert all(row.location_id == location_id for row in spool_rows) + + +async def test_backfill_is_idempotent_with_existing_locations(engine_with_case_variant_spools): + async with engine_with_case_variant_spools.begin() as conn: + await run_migrations(conn) + async with engine_with_case_variant_spools.begin() as conn: + await run_migrations(conn) + + async with engine_with_case_variant_spools.connect() as conn: + loc_count = (await conn.execute(text("SELECT COUNT(*) FROM locations"))).scalar_one() + linked = (await conn.execute(text("SELECT COUNT(*) FROM spool WHERE location_id IS NOT NULL"))).scalar_one() + + assert loc_count == 1 + assert linked == 2 + + +@pytest.fixture +async def engine_with_null_storage_location(): + """A spool with NULL storage_location must NOT produce a phantom location row + or get linked to anything — it stays NULL on both fields.""" + from backend.app.core.database import Base + + _register_all_models() + engine = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + await conn.execute(text("DELETE FROM locations")) + await conn.execute( + text( + """ + INSERT INTO spool ( + material, storage_location, label_weight, core_weight, + weight_used, weight_used_baseline, weight_locked + ) + VALUES ('PLA', NULL, 1000, 250, 0, 0, 0), + ('PETG', ' ', 1000, 250, 0, 0, 0), + ('TPU', 'Real Shelf', 1000, 250, 0, 0, 0) + """ + ) + ) + yield engine + await engine.dispose() + + +async def test_backfill_skips_null_and_whitespace_storage_location( + engine_with_null_storage_location, +): + """NULL / whitespace-only `storage_location` rows must NOT create catalog + rows; only the 'Real Shelf' value gets a location row + spool link.""" + async with engine_with_null_storage_location.begin() as conn: + await run_migrations(conn) + + async with engine_with_null_storage_location.connect() as conn: + loc_rows = (await conn.execute(text("SELECT name FROM locations"))).all() + unlinked = ( + await conn.execute(text("SELECT material FROM spool WHERE location_id IS NULL ORDER BY material")) + ).all() + + # Only the row with a real storage_location should be in the catalog. + assert [r.name for r in loc_rows] == ["Real Shelf"] + # The NULL and whitespace-only spools stay unlinked (no phantom row). + assert [r.material for r in unlinked] == ["PETG", "PLA"] + + +@pytest.fixture +async def engine_with_legacy_null_name_key_location(): + """Simulate a legacy install where a `locations` row was manually inserted + BEFORE the name_key column existed. The migration must backfill the + legacy row's name_key BEFORE the dedup INSERT, so the spool-link UPDATE + can join on the new key (#1505 review IMPORTANT 11).""" + from backend.app.core.database import Base + + _register_all_models() + engine = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + # Drop the model-shaped locations table (which has NOT NULL on + # name_key) and recreate it in its pre-migration shape: no name_key + # column at all, mirroring a real upgrade from a Bambuddy version + # that predates this feature. The migration's idempotent ALTER TABLE + # is what adds the column without a NOT NULL constraint, so the + # legacy row can legally have NULL until the new backfill UPDATE + # runs. + await conn.execute(text("DROP TABLE locations")) + await conn.execute( + text( + """ + CREATE TABLE locations ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name VARCHAR(255) NOT NULL UNIQUE, + identifier VARCHAR(100), + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP + ) + """ + ) + ) + await conn.execute(text("INSERT INTO locations (name) VALUES ('Drybox 1')")) + await conn.execute( + text( + """ + INSERT INTO spool ( + material, storage_location, label_weight, core_weight, + weight_used, weight_used_baseline, weight_locked + ) + VALUES ('PLA', 'Drybox 1', 1000, 250, 0, 0, 0) + """ + ) + ) + yield engine + await engine.dispose() + + +async def test_backfill_links_spool_to_legacy_null_name_key_location( + engine_with_legacy_null_name_key_location, +): + async with engine_with_legacy_null_name_key_location.begin() as conn: + await run_migrations(conn) + + async with engine_with_legacy_null_name_key_location.connect() as conn: + loc_rows = (await conn.execute(text("SELECT id, name, name_key FROM locations"))).all() + spool_rows = (await conn.execute(text("SELECT location_id FROM spool"))).all() + + # Exactly one location row (the pre-existing legacy one); its name_key + # got backfilled by the FIRST step of the migration. + assert len(loc_rows) == 1 + assert loc_rows[0].name_key == "drybox 1" + # The spool got linked to that legacy row — under the old ordering it + # would have been left with `location_id IS NULL`. + assert spool_rows[0].location_id == loc_rows[0].id diff --git a/backend/tests/unit/test_location_service.py b/backend/tests/unit/test_location_service.py new file mode 100644 index 000000000..f3f1b6417 --- /dev/null +++ b/backend/tests/unit/test_location_service.py @@ -0,0 +1,219 @@ +"""Unit tests for storage location service (#1004).""" + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.models.location import Location +from backend.app.models.spool import Spool +from backend.app.services.location_service import ( + assign_location_name, + enrich_spool_dicts_with_location_id, + get_location_by_name, + location_name_key, + prepare_internal_spool_payload, + rename_location, + resolve_location_by_name, + resolve_spool_location_fields, + sync_locations_from_spoolman, +) + + +@pytest.mark.asyncio +async def test_resolve_location_by_name_creates(db_session: AsyncSession): + loc = await resolve_location_by_name(db_session, "Shelf A") + await db_session.commit() + assert loc is not None + assert loc.name == "Shelf A" + assert loc.name_key == location_name_key("Shelf A") + + again = await get_location_by_name(db_session, "shelf a") + assert again is not None + assert again.id == loc.id + + +@pytest.mark.asyncio +async def test_prepare_internal_spool_payload_from_location_id(db_session: AsyncSession): + loc = Location() + assign_location_name(loc, "Drawer 2") + db_session.add(loc) + await db_session.commit() + await db_session.refresh(loc) + + payload = await prepare_internal_spool_payload( + db_session, + {"material": "PLA", "location_id": loc.id}, + {"material", "location_id"}, + ) + assert payload["location_id"] == loc.id + assert payload["storage_location"] == "Drawer 2" + + +@pytest.mark.asyncio +async def test_resolve_spool_location_fields_prefers_location_id(db_session: AsyncSession): + loc = Location() + assign_location_name(loc, "Catalog A") + db_session.add(loc) + await db_session.commit() + await db_session.refresh(loc) + + resolved = await resolve_spool_location_fields( + db_session, + location_id=loc.id, + storage_location="Other", + fields_set={"location_id", "storage_location"}, + ) + assert resolved is not None + assert resolved.location_id == loc.id + assert resolved.storage_location == "Catalog A" + + +@pytest.mark.asyncio +async def test_rename_location_updates_spool_storage(db_session: AsyncSession): + loc = Location() + assign_location_name(loc, "Old Shelf") + spool = Spool(material="PLA", location_id=None, storage_location="Old Shelf") + db_session.add(loc) + db_session.add(spool) + await db_session.commit() + await db_session.refresh(loc) + + await rename_location(db_session, loc, "New Shelf") + await db_session.commit() + await db_session.refresh(spool) + + assert loc.name == "New Shelf" + assert loc.name_key == location_name_key("New Shelf") + assert spool.storage_location == "New Shelf" + assert spool.location_id == loc.id + + +@pytest.mark.asyncio +async def test_enrich_spool_dicts_with_location_id(db_session: AsyncSession): + loc = Location() + assign_location_name(loc, "Garage") + db_session.add(loc) + await db_session.commit() + + spools = [{"id": 1, "storage_location": "Garage"}, {"id": 2, "storage_location": None}] + await enrich_spool_dicts_with_location_id(db_session, spools) + assert spools[0]["location_id"] == loc.id + assert spools[1]["location_id"] is None + + +@pytest.mark.asyncio +async def test_sync_locations_from_spoolman_stages_without_commit(db_session: AsyncSession): + class FakeClient: + async def get_distinct_locations(self): + return ["Spoolman Shelf"] + + changed = await sync_locations_from_spoolman(db_session, FakeClient()) + assert changed is True + loc = await get_location_by_name(db_session, "Spoolman Shelf") + assert loc is not None + # Caller owns the transaction — no commit() was called in sync itself. + assert loc.id is not None + + +@pytest.mark.asyncio +async def test_sync_locations_from_spoolman_dedupes_case_variants(db_session: AsyncSession): + class FakeClient: + async def get_distinct_locations(self): + return ["Drybox 1", "DRYBOX 1", "Locker"] + + changed = await sync_locations_from_spoolman(db_session, FakeClient()) + assert changed is True + await db_session.commit() + + drybox = await get_location_by_name(db_session, "Drybox 1") + locker = await get_location_by_name(db_session, "Locker") + assert drybox is not None + assert locker is not None + + from sqlalchemy import func, select + + from backend.app.models.location import Location + + count = await db_session.scalar(select(func.count()).select_from(Location)) + assert count == 2 + + +@pytest.mark.asyncio +async def test_rename_location_duplicate_name_raises(db_session: AsyncSession): + first = Location() + assign_location_name(first, "Shelf A") + second = Location() + assign_location_name(second, "Shelf B") + db_session.add_all([first, second]) + await db_session.commit() + await db_session.refresh(first) + await db_session.refresh(second) + + with pytest.raises(ValueError, match="already exists"): + await rename_location(db_session, second, "Shelf A") + + +@pytest.mark.asyncio +async def test_rename_location_picks_up_legacy_row_with_trailing_whitespace(db_session: AsyncSession): + """A legacy spool whose `storage_location` carries trailing whitespace + must still get relinked by the rename cascade — the SQL `TRIM()` strips + the column, so the Python comparison must also strip `old_name`.""" + loc = Location() + assign_location_name(loc, "Old Shelf") + # Simulate a legacy row whose name was stored with the same value but + # the column entry has whitespace padding (this happens in old free-text + # data + manual DB edits). + legacy_spool = Spool(material="PLA", location_id=None, storage_location=" Old Shelf ") + db_session.add(loc) + db_session.add(legacy_spool) + await db_session.commit() + await db_session.refresh(loc) + await db_session.refresh(legacy_spool) + + # Force the in-memory name to carry trailing whitespace so the rename + # path lifts a non-stripped `old_name`. This is the asymmetry the fix + # addresses (#1505 review IMPORTANT 10). + loc.name = "Old Shelf " + + await rename_location(db_session, loc, "New Shelf") + await db_session.commit() + await db_session.refresh(legacy_spool) + + assert legacy_spool.storage_location == "New Shelf" + assert legacy_spool.location_id == loc.id + + +@pytest.mark.asyncio +async def test_sync_locations_from_spoolman_logs_and_returns_false_on_unavailable(db_session: AsyncSession, caplog): + """Bare `except Exception: return False` was the prior shape — verify the + narrowed catch surfaces a warning so ops can see Spoolman outages.""" + from backend.app.services.spoolman import SpoolmanUnavailableError + + class FailingClient: + async def get_distinct_locations(self): + raise SpoolmanUnavailableError("Cannot reach Spoolman") + + with caplog.at_level("WARNING", logger="backend.app.services.location_service"): + changed = await sync_locations_from_spoolman(db_session, FailingClient()) + + assert changed is False + assert any("location sync from Spoolman failed" in rec.message for rec in caplog.records) + + +@pytest.mark.asyncio +async def test_sync_locations_from_spoolman_handles_dict_payload(db_session: AsyncSession): + """Newer Spoolman returns `list[dict]` from `/location`; the SpoolmanClient + normalises to `list[str]`, so sync_locations_from_spoolman should accept + both shapes via the client contract.""" + + class DictShapeClient: + async def get_distinct_locations(self): + # SpoolmanClient.get_distinct_locations is the one that normalises; + # at this layer the contract is `list[str]`. Simulate post-normalisation. + return ["Cabinet 3", "Cabinet 3"] # dedup tested elsewhere — sanity here + + changed = await sync_locations_from_spoolman(db_session, DictShapeClient()) + assert changed is True + await db_session.commit() + + cabinet = await get_location_by_name(db_session, "Cabinet 3") + assert cabinet is not None diff --git a/backend/tests/unit/test_spoolman_inventory_methods.py b/backend/tests/unit/test_spoolman_inventory_methods.py index 55a94be07..28bc6deae 100644 --- a/backend/tests/unit/test_spoolman_inventory_methods.py +++ b/backend/tests/unit/test_spoolman_inventory_methods.py @@ -529,3 +529,139 @@ class TestGetExternalFilamentsRaisesOnError: pytest.raises(SpoolmanUnavailableError), ): await client.get_external_filaments() + + +# --------------------------------------------------------------------------- +# get_distinct_locations — shape normalisation (#1505 review BLOCKER 3) +# --------------------------------------------------------------------------- + + +class TestGetDistinctLocationsShape: + @pytest.mark.asyncio + async def test_passes_through_list_of_strings(self, client): + with patch.object(client, "_get_with_retry", AsyncMock(return_value=["Drybox 1", "Shelf"])): + result = await client.get_distinct_locations() + assert result == ["Drybox 1", "Shelf"] + + @pytest.mark.asyncio + async def test_extracts_name_from_list_of_dicts(self, client): + with patch.object( + client, + "_get_with_retry", + AsyncMock(return_value=[{"id": 1, "name": "Drybox 1"}, {"id": 2, "name": "Shelf"}]), + ): + result = await client.get_distinct_locations() + assert result == ["Drybox 1", "Shelf"] + + @pytest.mark.asyncio + async def test_drops_non_string_and_dict_without_name(self, client): + with patch.object( + client, + "_get_with_retry", + AsyncMock(return_value=[{"id": 1}, None, 42, "Shelf"]), + ): + result = await client.get_distinct_locations() + assert result == ["Shelf"] + + @pytest.mark.asyncio + async def test_returns_empty_list_on_non_list_payload(self, client): + # A misconfigured proxy or auth-redirect can serve HTML; the old shape + # would TypeError on iteration. We coerce to []. + with patch.object(client, "_get_with_retry", AsyncMock(return_value={"error": "unauthorized"})): + result = await client.get_distinct_locations() + assert result == [] + + +# --------------------------------------------------------------------------- +# rename_location — bulk endpoint + per-spool fallback (#1505 review BLOCKER 2) +# --------------------------------------------------------------------------- + + +class TestRenameLocationBulkAndFallback: + @pytest.mark.asyncio + async def test_bulk_endpoint_success_returns_zero(self, client): + """Modern Spoolman PATCH /location/{name} succeeds — fallback not used.""" + mock_http = AsyncMock() + mock_http.patch = AsyncMock(return_value=_make_response(None)) + with patch.object(client, "_get_client", AsyncMock(return_value=mock_http)): + result = await client.rename_location("Drybox 1", "Drybox 2") + assert result == 0 + # Confirm the bulk path was used (no per-spool PATCH). + mock_http.patch.assert_called_once() + assert "/location/" in mock_http.patch.call_args.args[0] + + @pytest.mark.asyncio + async def test_bulk_endpoint_404_falls_back_to_per_spool_patch(self, client): + """Older Spoolman versions return 404 on the bulk endpoint — the + fallback iterates every spool currently at the old name.""" + bulk_response = MagicMock() + bulk_response.status_code = 404 + bulk_response.raise_for_status = MagicMock( + side_effect=httpx.HTTPStatusError("404 Not Found", request=MagicMock(), response=MagicMock(status_code=404)) + ) + mock_http = AsyncMock() + mock_http.patch = AsyncMock(return_value=bulk_response) + + spools_at_old = [ + {"id": 11, "location": "Drybox 1"}, + {"id": 12, "location": "Drybox 1"}, + {"id": 13, "location": "Shelf A"}, # different location — must be skipped + ] + patch_response = MagicMock() + patch_response.status_code = 200 + patch_response.raise_for_status = MagicMock() + patch_response.json.return_value = {"id": 0, "location": "Drybox 2"} + + with ( + patch.object(client, "_get_client", AsyncMock(return_value=mock_http)), + patch.object(client, "get_all_spools", AsyncMock(return_value=spools_at_old)), + patch.object(client, "_request_spool", AsyncMock(return_value=patch_response)) as request_spool_mock, + ): + result = await client.rename_location("Drybox 1", "Drybox 2") + + assert result == 2 + # Only the two matching spools should be PATCHed. + assert request_spool_mock.await_count == 2 + called_ids = sorted(call.args[1] for call in request_spool_mock.await_args_list) + assert called_ids == [11, 12] + # And each call should set the new location string. + for call in request_spool_mock.await_args_list: + assert call.kwargs["json_body"] == {"location": "Drybox 2"} + + @pytest.mark.asyncio + async def test_bulk_endpoint_405_also_falls_back(self, client): + """Some Spoolman versions return 405 Method Not Allowed instead of 404 + when the bulk endpoint is missing — same fallback.""" + bulk_response = MagicMock() + bulk_response.status_code = 405 + bulk_response.raise_for_status = MagicMock( + side_effect=httpx.HTTPStatusError( + "405 Method Not Allowed", request=MagicMock(), response=MagicMock(status_code=405) + ) + ) + mock_http = AsyncMock() + mock_http.patch = AsyncMock(return_value=bulk_response) + + with ( + patch.object(client, "_get_client", AsyncMock(return_value=mock_http)), + patch.object(client, "get_all_spools", AsyncMock(return_value=[])), + ): + result = await client.rename_location("Drybox 1", "Drybox 2") + # No spools at the old name → nothing to do, fallback returns 0. + assert result == 0 + + @pytest.mark.asyncio + async def test_bulk_endpoint_non_404_5xx_propagates(self, client): + """A genuine server error must NOT silently fall back.""" + bulk_response = MagicMock() + bulk_response.status_code = 500 + bulk_response.raise_for_status = MagicMock( + side_effect=httpx.HTTPStatusError("500", request=MagicMock(), response=MagicMock(status_code=500)) + ) + mock_http = AsyncMock() + mock_http.patch = AsyncMock(return_value=bulk_response) + with ( + patch.object(client, "_get_client", AsyncMock(return_value=mock_http)), + pytest.raises(httpx.HTTPStatusError), + ): + await client.rename_location("Drybox 1", "Drybox 2") diff --git a/docs/screenshots/storage-locations/inventory-location-filter.png b/docs/screenshots/storage-locations/inventory-location-filter.png new file mode 100644 index 000000000..f8abb7991 Binary files /dev/null and b/docs/screenshots/storage-locations/inventory-location-filter.png differ diff --git a/docs/screenshots/storage-locations/locations-page.png b/docs/screenshots/storage-locations/locations-page.png new file mode 100644 index 000000000..21df17347 Binary files /dev/null and b/docs/screenshots/storage-locations/locations-page.png differ diff --git a/docs/screenshots/storage-locations/spool-form-storage-location.png b/docs/screenshots/storage-locations/spool-form-storage-location.png new file mode 100644 index 000000000..b9cbf1a58 Binary files /dev/null and b/docs/screenshots/storage-locations/spool-form-storage-location.png differ diff --git a/docs/storage-locations.md b/docs/storage-locations.md new file mode 100644 index 000000000..36dd2de12 --- /dev/null +++ b/docs/storage-locations.md @@ -0,0 +1,44 @@ +# Storage Locations (#1004) + +Structured storage locations let you manage physical shelves, drawers, and dryboxes as a catalog instead of free-text only. + +## Architecture + +- **`locations` table** — catalog of named storage spots (`name` + case-insensitive `name_key`). +- **`spool.location_id`** — source of truth for structured assignment. +- **`spool.storage_location`** — denormalized display string and Spoolman wire format; always derived on write via `location_service.resolve_spool_location_fields()`. +- **Frontend** — spool form sends only `location_id`; backend fills `storage_location`. + +## Location vs Storage Location vs AMS Location + +| UI label | Meaning | +|----------|---------| +| **Location** (inventory table column) | AMS slot or printer assignment (e.g. `H2D-1 B4`) | +| **Storage Location** | Physical shelf/drawer where the spool lives when not in AMS | +| **Locations page** | Catalog of named storage spots with spool counts | + +## Managing locations + +1. Open **Inventory → Locations** +2. Click **Add Location** and enter a name (e.g. `Regal Etage 2`) +3. Assign spools via the spool edit form **Storage Location** dropdown +4. Click a location row to filter inventory by that shelf + +## Spoolman mode + +Bambuddy keeps a local location catalog. When Spoolman integration is enabled: + +- Assigning a location writes the location **name** to Spoolman's `location` field +- Listing locations syncs distinct names from Spoolman into the catalog +- Renaming a location bulk-renames spools in Spoolman via `PATCH /location/{old}` + +## Upgrade migration + +Existing free-text `storage_location` values are automatically imported into the location catalog and linked on upgrade (case-insensitive dedup via `name_key`). + +## Testing before release + +1. `./test_frontend.sh` — i18n parity, lint, Vitest +2. `./test_backend.sh` — Ruff, pytest (includes `test_locations_api.py`, `test_location_service.py`) +3. Manual: assign a spool to a location → open **Locations** → spool count updates without reload +4. Companion PR in [bambuddy-wiki](https://github.com/maziggy/bambuddy-wiki) (user-facing guide) diff --git a/frontend/src/__tests__/components/LocationsModal.test.tsx b/frontend/src/__tests__/components/LocationsModal.test.tsx new file mode 100644 index 000000000..f5845fe66 --- /dev/null +++ b/frontend/src/__tests__/components/LocationsModal.test.tsx @@ -0,0 +1,243 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router-dom'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { LocationsModal } from '../../components/LocationsModal'; +import { api, ApiError } from '../../api/client'; + +const mockShowToast = vi.fn(); +const mockOnClose = vi.fn(); +const mockOnPickLocation = vi.fn(); + +vi.mock('../../api/client', () => ({ + api: { + getLocations: vi.fn(), + createLocation: vi.fn(), + updateLocation: vi.fn(), + deleteLocation: vi.fn(), + }, + ApiError: class ApiError extends Error { + status: number; + constructor(message: string, status: number) { + super(message); + this.status = status; + } + }, +})); + +vi.mock('../../contexts/ToastContext', () => ({ + useToast: () => ({ showToast: mockShowToast }), +})); + +const locations = [ + { id: 1, name: 'Shelf A', identifier: null, spool_count: 2, created_at: '2026-01-01', updated_at: '2026-01-01' }, + { id: 2, name: 'Drawer 1', identifier: null, spool_count: 0, created_at: '2026-01-01', updated_at: '2026-01-01' }, +]; + +function renderModal(open = true) { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + + , + ); +} + +describe('LocationsModal', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(api.getLocations).mockResolvedValue(locations); + }); + + it('renders nothing when open=false', () => { + const { container } = renderModal(false); + expect(container.firstChild).toBeNull(); + expect(api.getLocations).not.toHaveBeenCalled(); + }); + + it('renders locations from API when open', async () => { + renderModal(); + expect(await screen.findByText('Shelf A')).toBeInTheDocument(); + expect(screen.getByText('Drawer 1')).toBeInTheDocument(); + expect(screen.getByText('2')).toBeInTheDocument(); + }); + + it('renders empty state when API returns no locations', async () => { + vi.mocked(api.getLocations).mockResolvedValue([]); + renderModal(); + expect(await screen.findByText(/locations\.empty|no storage locations/i)).toBeInTheDocument(); + }); + + it('opens create editor and calls createLocation on submit', async () => { + vi.mocked(api.createLocation).mockResolvedValue({ + id: 3, + name: 'Garage', + identifier: null, + spool_count: 0, + created_at: '2026-01-01', + updated_at: '2026-01-01', + }); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Shelf A'); + await user.click(screen.getByRole('button', { name: /add location|locations\.add/i })); + const input = screen.getByLabelText(/name|locations\.name/i); + await user.type(input, 'Garage'); + await user.click(screen.getByRole('button', { name: /save|common\.save/i })); + await waitFor(() => { + expect(api.createLocation).toHaveBeenCalledWith({ name: 'Garage' }); + }); + expect(mockShowToast).toHaveBeenCalledWith(expect.stringMatching(/created|locations\.created/i), 'success'); + }); + + it('submits create form on Enter key', async () => { + vi.mocked(api.createLocation).mockResolvedValue({ + id: 3, + name: 'Garage', + identifier: null, + spool_count: 0, + created_at: '2026-01-01', + updated_at: '2026-01-01', + }); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Shelf A'); + await user.click(screen.getByRole('button', { name: /add location|locations\.add/i })); + const input = screen.getByLabelText(/name|locations\.name/i); + await user.type(input, 'Garage{Enter}'); + await waitFor(() => { + expect(api.createLocation).toHaveBeenCalledWith({ name: 'Garage' }); + }); + }); + + it('Escape closes the inner editor first, then the outer modal', async () => { + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Shelf A'); + // Open the inner editor; both dialogs are now in the DOM. + await user.click(screen.getByRole('button', { name: /add location|locations\.add/i })); + expect(screen.getAllByRole('dialog')).toHaveLength(2); + // First Escape closes the editor only. + await user.keyboard('{Escape}'); + await waitFor(() => { + expect(screen.getAllByRole('dialog')).toHaveLength(1); + }); + expect(mockOnClose).not.toHaveBeenCalled(); + // Second Escape closes the outer modal. + await user.keyboard('{Escape}'); + await waitFor(() => { + expect(mockOnClose).toHaveBeenCalledTimes(1); + }); + }); + + it('edits a location and calls updateLocation', async () => { + vi.mocked(api.updateLocation).mockResolvedValue({ + id: 2, + name: 'Drawer 2', + identifier: null, + spool_count: 0, + created_at: '2026-01-01', + updated_at: '2026-01-01', + }); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Drawer 1'); + const editButtons = screen.getAllByTitle(/edit|common\.edit/i); + await user.click(editButtons[1]); + const input = screen.getByLabelText(/name|locations\.name/i); + await user.clear(input); + await user.type(input, 'Drawer 2'); + await user.click(screen.getByRole('button', { name: /save|common\.save/i })); + await waitFor(() => { + expect(api.updateLocation).toHaveBeenCalledWith(2, { name: 'Drawer 2' }); + }); + expect(mockShowToast).toHaveBeenCalledWith(expect.stringMatching(/updated|locations\.updated/i), 'success'); + }); + + it('deletes an empty location after confirmation', async () => { + vi.mocked(api.deleteLocation).mockResolvedValue({ status: 'deleted' }); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Drawer 1'); + const row = screen.getByText('Drawer 1').closest('tr'); + expect(row).not.toBeNull(); + await user.click(within(row!).getByTitle(/^Delete$/i)); + await user.click(screen.getAllByRole('button', { name: /^Delete$/i }).pop()!); + await waitFor(() => { + expect(api.deleteLocation).toHaveBeenCalledWith(2); + }); + expect(mockShowToast).toHaveBeenCalledWith(expect.stringMatching(/deleted|locations\.deleted/i), 'success'); + }); + + it('blocks delete when spool_count > 0', async () => { + renderModal(); + await screen.findByText('Shelf A'); + const blockedDelete = screen.getByTitle(/Remove all spools from this location before deleting/i); + expect(blockedDelete).toBeDisabled(); + }); + + it('shows error toast when create returns 409 duplicate name', async () => { + vi.mocked(api.createLocation).mockRejectedValue( + new ApiError('A location with this name already exists', 409), + ); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Shelf A'); + await user.click(screen.getByRole('button', { name: /add location|locations\.add/i })); + await user.type(screen.getByLabelText(/name|locations\.name/i), 'Shelf A'); + await user.click(screen.getByRole('button', { name: /save|common\.save/i })); + await waitFor(() => { + expect(mockShowToast).toHaveBeenCalledWith('A location with this name already exists', 'error'); + }); + }); + + it('shows error toast when delete fails', async () => { + vi.mocked(api.deleteLocation).mockRejectedValue(new Error('Delete failed')); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Drawer 1'); + const row = screen.getByText('Drawer 1').closest('tr'); + expect(row).not.toBeNull(); + await user.click(within(row!).getByTitle(/^Delete$/i)); + await user.click(screen.getAllByRole('button', { name: /^Delete$/i }).pop()!); + await waitFor(() => { + expect(mockShowToast).toHaveBeenCalledWith('Delete failed', 'error'); + }); + }); + + it('row click calls onPickLocation and onClose', async () => { + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Shelf A'); + const row = screen.getByText('Shelf A').closest('tr')!; + await user.click(row); + expect(mockOnPickLocation).toHaveBeenCalledWith(1); + expect(mockOnClose).toHaveBeenCalledTimes(1); + }); + + it('shows error toast when rename returns 409 collision', async () => { + vi.mocked(api.updateLocation).mockRejectedValue( + new ApiError('A location with this name already exists', 409), + ); + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Drawer 1'); + + const editButtons = screen.getAllByTitle(/edit|common\.edit/i); + await user.click(editButtons[1]); + const input = screen.getByLabelText(/name|locations\.name/i); + await user.clear(input); + await user.type(input, 'Shelf A'); + await user.click(screen.getByRole('button', { name: /save|common\.save/i })); + + await waitFor(() => { + expect(mockShowToast).toHaveBeenCalledWith( + 'A location with this name already exists', + 'error', + ); + }); + }); +}); diff --git a/frontend/src/__tests__/components/SpoolFormBulk.test.tsx b/frontend/src/__tests__/components/SpoolFormBulk.test.tsx index 902a6e6b0..4be023e5a 100644 --- a/frontend/src/__tests__/components/SpoolFormBulk.test.tsx +++ b/frontend/src/__tests__/components/SpoolFormBulk.test.tsx @@ -29,6 +29,7 @@ vi.mock('../../api/client', () => ({ getCloudStatus: vi.fn().mockResolvedValue({ is_authenticated: false }), getFilamentPresets: vi.fn().mockResolvedValue([]), getSpoolCatalog: vi.fn().mockResolvedValue([]), + getLocations: vi.fn().mockResolvedValue([]), getColorCatalog: vi.fn().mockResolvedValue([]), getLocalPresets: vi.fn().mockResolvedValue({ filament: [] }), getBuiltinFilaments: vi.fn().mockResolvedValue([]), diff --git a/frontend/src/__tests__/components/SpoolFormModal.test.tsx b/frontend/src/__tests__/components/SpoolFormModal.test.tsx index 32cea5070..7a1894d90 100644 --- a/frontend/src/__tests__/components/SpoolFormModal.test.tsx +++ b/frontend/src/__tests__/components/SpoolFormModal.test.tsx @@ -21,6 +21,7 @@ vi.mock('../../api/client', () => ({ getCloudStatus: vi.fn().mockResolvedValue({ is_authenticated: false }), getFilamentPresets: vi.fn().mockResolvedValue([]), getSpoolCatalog: vi.fn().mockResolvedValue([]), + getLocations: vi.fn().mockResolvedValue([]), getColorCatalog: vi.fn().mockResolvedValue([]), getLocalPresets: vi.fn().mockResolvedValue({ filament: [] }), getBuiltinFilaments: vi.fn().mockResolvedValue([]), @@ -932,20 +933,25 @@ describe('SpoolFormModal — Unassign button (#1336)', () => { }); }); -describe('SpoolFormModal storageLocationTouched', () => { +describe('SpoolFormModal locationIdTouched', () => { /** * Regression tests for the round-trip bug: saving the edit modal without - * touching the Storage Location field must NOT include storage_location in + * touching the Storage Location field must NOT include location_id in * the PATCH payload, so Spoolman's location field is never overwritten with * a stale cached value. */ beforeEach(() => { vi.clearAllMocks(); + vi.mocked(api.getLocations).mockResolvedValue([ + { id: 1, name: 'IKEAREGAL', identifier: null, spool_count: 1, created_at: '', updated_at: '' }, + { id: 2, name: 'Shelf B', identifier: null, spool_count: 0, created_at: '', updated_at: '' }, + ]); }); const spoolWithStorageLocation: InventorySpool = { ...existingSpool, storage_location: 'IKEAREGAL', + location_id: 1, }; it('excludes storage_location from PATCH when editing without changing it', async () => { @@ -975,11 +981,12 @@ describe('SpoolFormModal storageLocationTouched', () => { expect(spoolId).toBe(1); // storage_location must NOT be in the payload — prevents Spoolman location overwrite expect(payload).not.toHaveProperty('storage_location'); + expect(payload).not.toHaveProperty('location_id'); // Other fields should still be present expect(payload).toHaveProperty('material', 'PLA'); }); - it('includes storage_location in PATCH when editing and changing it', async () => { + it('includes location_id in PATCH when editing and changing it', async () => { render( { expect(screen.getByText('Edit Spool')).toBeInTheDocument(); }); - // Find the storage location input and change it - const locationInput = screen.getByPlaceholderText('e.g. Shelf A, Drawer 1'); - fireEvent.change(locationInput, { target: { value: 'Shelf B' } }); + // Change storage location via the catalog dropdown + const locationSelect = screen.getByLabelText(/storage location/i); + fireEvent.change(locationSelect, { target: { value: '2' } }); const saveButton = screen.getByRole('button', { name: /save/i }); fireEvent.click(saveButton); @@ -1007,11 +1014,11 @@ describe('SpoolFormModal storageLocationTouched', () => { const [spoolId, payload] = vi.mocked(api.updateSpool).mock.calls[0]; expect(spoolId).toBe(1); - // storage_location MUST be present since the user changed it - expect(payload).toHaveProperty('storage_location', 'Shelf B'); + expect(payload).toHaveProperty('location_id', 2); + expect(payload).not.toHaveProperty('storage_location'); }); - it('includes storage_location when creating a new spool', async () => { + it('includes location_id when creating a new spool', async () => { render( { }); const [payload] = vi.mocked(api.createSpool).mock.calls[0]; - // storage_location MUST be included for new spools (default empty string → null) - expect(payload).toHaveProperty('storage_location', null); + expect(payload).toHaveProperty('location_id', null); + expect(payload).not.toHaveProperty('storage_location'); }); }); diff --git a/frontend/src/__tests__/hooks/useWebSocket.test.ts b/frontend/src/__tests__/hooks/useWebSocket.test.ts index 61b4676dd..a33a4ba5e 100644 --- a/frontend/src/__tests__/hooks/useWebSocket.test.ts +++ b/frontend/src/__tests__/hooks/useWebSocket.test.ts @@ -433,6 +433,42 @@ describe('useWebSocket hook', () => { vi.unstubAllGlobals(); }); + it('invalidates inventory queries on inventory_changed message', async () => { + vi.useFakeTimers(); + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { + cb(0); + return 0; + }); + const { useWebSocket } = await import('../../hooks/useWebSocket'); + + const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries'); + + renderHook(() => useWebSocket(), { + wrapper: createWrapper(queryClient), + }); + + const ws = await waitForWs(); + + act(() => { + ws.open(); + }); + + act(() => { + ws.simulateMessage({ type: 'inventory_changed' }); + }); + + await act(async () => { + vi.advanceTimersByTime(5000); + }); + + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['inventory-spools'] }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['spoolman-inventory-spools'] }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['inventory-locations'] }); + + vi.useRealTimers(); + vi.unstubAllGlobals(); + }); + it('handles missing_spool_assignment message without error', async () => { vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { cb(0); diff --git a/frontend/src/__tests__/mocks/handlers.ts b/frontend/src/__tests__/mocks/handlers.ts index 14a7c9de3..3cf227e20 100644 --- a/frontend/src/__tests__/mocks/handlers.ts +++ b/frontend/src/__tests__/mocks/handlers.ts @@ -466,6 +466,7 @@ export const handlers = [ http.get('/api/v1/inventory/assignments', () => HttpResponse.json([])), http.get('/api/v1/inventory/catalog', () => HttpResponse.json([])), http.get('/api/v1/inventory/colors', () => HttpResponse.json([])), + http.get('/api/v1/inventory/locations', () => HttpResponse.json([])), http.get('/api/v1/inventory/spools', () => HttpResponse.json([])), http.get('/api/v1/library/folders', () => HttpResponse.json([])), http.get('/api/v1/library/folders/by-archive/:id', () => HttpResponse.json([])), diff --git a/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx b/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx index a41ecb52a..9e393e9d3 100644 --- a/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx +++ b/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx @@ -141,6 +141,7 @@ function setupCommonHandlers(spoolList: object[]) { http.get('/api/v1/inventory/color-catalog', () => HttpResponse.json([])), http.get('/api/v1/inventory/colors', () => HttpResponse.json([])), http.get('/api/v1/inventory/spool-catalog', () => HttpResponse.json([])), + http.get('/api/v1/inventory/locations', () => HttpResponse.json([])), http.get('/api/v1/printers/', () => HttpResponse.json([])), ); } diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 1c09481d3..fd67b756f 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1317,6 +1317,15 @@ export interface SpoolCatalogEntry { is_default: boolean; } +export interface StorageLocation { + id: number; + name: string; + identifier: string | null; + spool_count: number; + created_at: string; + updated_at: string; +} + export interface ColorCatalogEntry { id: number; manufacturer: string; @@ -2652,6 +2661,7 @@ export interface InventorySpool { low_stock_threshold_pct: number | null; k_profiles?: SpoolKProfile[]; storage_location?: string | null; + location_id?: number | null; } export interface SpoolmanBulkCreateResult { @@ -5084,6 +5094,14 @@ export const api = { request<{ deleted: number }>('/inventory/catalog/bulk-delete', { method: 'POST', body: JSON.stringify({ ids }) }), resetSpoolCatalog: () => request<{ status: string }>('/inventory/catalog/reset', { method: 'POST' }), + getLocations: () => + request('/inventory/locations'), + createLocation: (data: { name: string; identifier?: string | null }) => + request('/inventory/locations', { method: 'POST', body: JSON.stringify(data) }), + updateLocation: (id: number, data: { name?: string; identifier?: string | null }) => + request(`/inventory/locations/${id}`, { method: 'PATCH', body: JSON.stringify(data) }), + deleteLocation: (id: number) => + request<{ status: string }>(`/inventory/locations/${id}`, { method: 'DELETE' }), getColorCatalog: () => request('/inventory/colors'), getColorNameMap: () => diff --git a/frontend/src/components/LocationsModal.tsx b/frontend/src/components/LocationsModal.tsx new file mode 100644 index 000000000..c59369651 --- /dev/null +++ b/frontend/src/components/LocationsModal.tsx @@ -0,0 +1,274 @@ +import { useState, useEffect, useCallback } from 'react'; +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; +import { useTranslation } from 'react-i18next'; +import { MapPin, Plus, Loader2, Pencil, Trash2, X } from 'lucide-react'; +import { api, type StorageLocation } from '../api/client'; +import { Button } from './Button'; +import { ConfirmModal } from './ConfirmModal'; +import { useToast } from '../contexts/ToastContext'; +import { inventoryLocationsQueryKey, invalidateInventoryLocations } from '../utils/inventoryQueries'; + +interface LocationsModalProps { + open: boolean; + onClose: () => void; + onPickLocation?: (locationId: number) => void; +} + +export function LocationsModal({ open, onClose, onPickLocation }: LocationsModalProps) { + const { t } = useTranslation(); + const queryClient = useQueryClient(); + const { showToast } = useToast(); + + const [editorOpen, setEditorOpen] = useState(false); + const [editing, setEditing] = useState(null); + const [name, setName] = useState(''); + const [deleteTarget, setDeleteTarget] = useState(null); + + const { data: locations = [], isLoading } = useQuery({ + queryKey: inventoryLocationsQueryKey, + queryFn: api.getLocations, + enabled: open, + }); + + const invalidate = () => { + invalidateInventoryLocations(queryClient); + queryClient.invalidateQueries({ queryKey: ['inventory-spools'] }); + queryClient.invalidateQueries({ queryKey: ['spoolman-inventory-spools'] }); + }; + + const saveMutation = useMutation({ + mutationFn: async () => { + const trimmed = name.trim(); + if (!trimmed) throw new Error(t('locations.nameRequired')); + if (editing) { + return api.updateLocation(editing.id, { name: trimmed }); + } + return api.createLocation({ name: trimmed }); + }, + onSuccess: () => { + showToast(t(editing ? 'locations.updated' : 'locations.created'), 'success'); + setEditorOpen(false); + setEditing(null); + setName(''); + invalidate(); + }, + onError: (err: Error) => { + showToast(err.message || t('locations.saveFailed'), 'error'); + }, + }); + + const deleteMutation = useMutation({ + mutationFn: (id: number) => api.deleteLocation(id), + onSuccess: () => { + showToast(t('locations.deleted'), 'success'); + setDeleteTarget(null); + invalidate(); + }, + onError: (err: Error) => { + showToast(err.message || t('locations.deleteFailed'), 'error'); + }, + }); + + const openCreate = () => { + setEditing(null); + setName(''); + setEditorOpen(true); + }; + + const openEdit = (location: StorageLocation) => { + setEditing(location); + setName(location.name); + setEditorOpen(true); + }; + + const closeEditor = useCallback(() => { + if (saveMutation.isPending) return; + setEditorOpen(false); + setEditing(null); + setName(''); + }, [saveMutation.isPending]); + + // Esc closes the inner editor first; if it's closed, Esc closes the outer + // modal — but only when neither save nor delete is mid-flight, so a stray + // keypress during a network round-trip doesn't drop the user back into the + // inventory page with an orphaned spinner. + useEffect(() => { + if (!open) return; + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key !== 'Escape') return; + if (saveMutation.isPending || deleteMutation.isPending) return; + if (editorOpen) { + closeEditor(); + } else if (!deleteTarget) { + onClose(); + } + }; + document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, [open, editorOpen, deleteTarget, saveMutation.isPending, deleteMutation.isPending, closeEditor, onClose]); + + const handleSave = (e: React.FormEvent) => { + e.preventDefault(); + saveMutation.mutate(); + }; + + if (!open) return null; + + const modalTitleId = 'locations-modal-title'; + const editorTitleId = 'location-editor-title'; + + return ( +
+
{ + if (saveMutation.isPending || deleteMutation.isPending) return; + onClose(); + }} + /> +
+
+
+

+ + {t('locations.title')} +

+

{t('locations.subtitle')}

+
+
+ + +
+
+ +
+ {isLoading ? ( +
+ + {t('common.loading')} +
+ ) : locations.length === 0 ? ( +
{t('locations.empty')}
+ ) : ( + + + + + + + + + + {locations.map((loc) => ( + { + if (onPickLocation) { + onPickLocation(loc.id); + onClose(); + } + }} + > + + + + + ))} + +
{t('locations.name')}{t('locations.spools')}{t('common.actions')}
{loc.name}{loc.spool_count} e.stopPropagation()}> +
+ + +
+
+ )} +
+
+ + {editorOpen && ( +
+
+
+

+ {editing ? t('locations.edit') : t('locations.add')} +

+
+ + setName(e.target.value)} + autoFocus + /> +
+ + +
+
+
+
+ )} + + {deleteTarget && ( + deleteMutation.mutate(deleteTarget.id)} + onCancel={() => setDeleteTarget(null)} + /> + )} +
+ ); +} diff --git a/frontend/src/components/SpoolFormModal.tsx b/frontend/src/components/SpoolFormModal.tsx index 1bb37ede2..d5e4e4ee9 100644 --- a/frontend/src/components/SpoolFormModal.tsx +++ b/frontend/src/components/SpoolFormModal.tsx @@ -16,6 +16,10 @@ import { AdditionalSection } from './spool-form/AdditionalSection'; import { SpoolmanFilamentPicker } from './spool-form/SpoolmanFilamentPicker'; import { PAProfileSection } from './spool-form/PAProfileSection'; import { SpoolUsageHistory } from './SpoolUsageHistory'; +import { + invalidateInventoryLocations, + invalidateSpoolAndLocationQueries, +} from '../utils/inventoryQueries'; type TabId = 'filament' | 'pa-profile'; @@ -52,6 +56,9 @@ export function SpoolFormModal({ const queryClient = useQueryClient(); const { showToast } = useToast(); + const refreshSpoolQueries = () => + invalidateSpoolAndLocationQueries(queryClient, spoolsQueryKey); + const isEditing = mode === 'edit'; const isCopying = mode === 'copy'; @@ -60,7 +67,7 @@ export function SpoolFormModal({ const [errors, setErrors] = useState>>({}); const [activeTab, setActiveTab] = useState('filament'); const [weightTouched, setWeightTouched] = useState(false); - const [storageLocationTouched, setStorageLocationTouched] = useState(false); + const [locationIdTouched, setLocationIdTouched] = useState(false); const [quickAdd, setQuickAdd] = useState(false); const [quantity, setQuantity] = useState(1); @@ -72,6 +79,7 @@ export function SpoolFormModal({ // Spool catalog const [spoolCatalog, setSpoolCatalog] = useState([]); + const [storageLocations, setStorageLocations] = useState<{ id: number; name: string }[]>([]); // Local presets (OrcaSlicer imports) const [localPresets, setLocalPresets] = useState([]); @@ -176,6 +184,7 @@ export function SpoolFormModal({ api.getColorCatalog().then(setColorCatalog).catch(console.error); api.getLocalPresets().then(r => setLocalPresets(r.filament)).catch(console.error); api.getBuiltinFilaments().then(setBuiltinFilaments).catch(console.error); + api.getLocations().then((locs) => setStorageLocations(locs.map((l) => ({ id: l.id, name: l.name })))).catch(console.error); // Fetch printer calibrations if not provided via props if (printersWithCalibrations.length === 0) { @@ -360,7 +369,7 @@ export function SpoolFormModal({ cost_per_kg: spool.cost_per_kg ?? null, category: spool.category || '', low_stock_threshold_pct: spool.low_stock_threshold_pct ?? null, - storage_location: spool.storage_location || '', + location_id: spool.location_id ?? null, spoolman_filament_id: null, }); setPresetInputValue(spool.slicer_filament_name || spool.slicer_filament || ''); @@ -387,10 +396,21 @@ export function SpoolFormModal({ setErrors({}); setActiveTab('filament'); setWeightTouched(false); - setStorageLocationTouched(false); + setLocationIdTouched(false); } }, [isOpen, spool, mode, isCopying]); + // Legacy rows may have storage_location text but no location_id yet — link when catalog loads. + useEffect(() => { + if (!isOpen || !spool || locationIdTouched || formData.location_id != null) return; + const legacy = spool.storage_location?.trim(); + if (!legacy || storageLocations.length === 0) return; + const match = storageLocations.find((l) => l.name.toLowerCase() === legacy.toLowerCase()); + if (match) { + setFormData((prev) => (prev.location_id === match.id ? prev : { ...prev, location_id: match.id })); + } + }, [isOpen, spool, storageLocations, formData.location_id, locationIdTouched]); + // Expand all printers in PA profile section when calibrations are available useEffect(() => { if (isOpen && resolvedCalibrations.length > 0) { @@ -412,7 +432,7 @@ export function SpoolFormModal({ : {}), })); if (key === 'weight_used') setWeightTouched(true); - if (key === 'storage_location') setStorageLocationTouched(true); + if (key === 'location_id') setLocationIdTouched(true); if (errors[key]) { setErrors(prev => ({ ...prev, [key]: undefined })); } @@ -456,7 +476,7 @@ export function SpoolFormModal({ const ok = await saveKProfiles(newSpool.id); if (!ok) return; } - await queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + await refreshSpoolQueries(); if (onSpoolsCreated) onSpoolsCreated([newSpool]); showToast(t('inventory.spoolCreated'), 'success'); onClose(); @@ -495,7 +515,7 @@ export function SpoolFormModal({ await saveKProfiles(s.id); } } - await queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + await refreshSpoolQueries(); if (onSpoolsCreated) onSpoolsCreated(createdSpools); if (spoolmanResult && spoolmanResult.failed_count > 0) { showToast( @@ -529,7 +549,7 @@ export function SpoolFormModal({ const ok = await saveKProfiles(spool.id); if (!ok) return; } - await queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + await refreshSpoolQueries(); showToast(t('inventory.spoolUpdated'), 'success'); onClose(); }, @@ -550,7 +570,7 @@ export function SpoolFormModal({ return api.updateSpool(spool!.id, CLEAR_TAG_PAYLOAD as Parameters[1]); }, onSuccess: async () => { - await queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + await refreshSpoolQueries(); showToast(t('inventory.rfidCleared', 'RFID tag cleared'), 'success'); onClose(); }, @@ -748,11 +768,10 @@ export function SpoolFormModal({ data.weight_used = formData.weight_used; } - // Only send storage_location when creating or when explicitly changed by the user. - // This prevents the modal round-trip from overwriting the Spoolman location field - // with a stale cached value when the user saves without touching this field. - if (!isEditing || storageLocationTouched) { - data.storage_location = formData.storage_location || null; + // Only send location_id when creating or when explicitly changed by the user. + // Backend derives storage_location; omitting on untouched edit avoids stale overwrites. + if (!isEditing || locationIdTouched) { + data.location_id = formData.location_id; } if (isEditing) { @@ -921,6 +940,23 @@ export function SpoolFormModal({ spoolCatalog={spoolCatalog} currencySymbol={currencySymbol} availableCategories={availableCategories} + availableLocations={storageLocations} + onCreateLocation={async (name) => { + try { + const created = await api.createLocation({ name }); + setStorageLocations((prev) => [...prev, { id: created.id, name: created.name }].sort((a, b) => a.name.localeCompare(b.name))); + await invalidateInventoryLocations(queryClient); + return { id: created.id, name: created.name }; + } catch (e) { + // Surface the backend's actual error so the user can + // distinguish 409 duplicate / 400 validation / 500 from + // a generic "save failed" message. + console.error(e); + const message = e instanceof Error ? e.message : t('locations.saveFailed'); + showToast(message || t('locations.saveFailed'), 'error'); + return null; + } + }} globalLowStockThreshold={globalLowStockThreshold} spoolmanMode={spoolmanMode} /> diff --git a/frontend/src/components/spool-form/AdditionalSection.tsx b/frontend/src/components/spool-form/AdditionalSection.tsx index 185c1ee15..67a383872 100644 --- a/frontend/src/components/spool-form/AdditionalSection.tsx +++ b/frontend/src/components/spool-form/AdditionalSection.tsx @@ -174,6 +174,8 @@ export function AdditionalSection({ spoolCatalog, currencySymbol, availableCategories, + availableLocations = [], + onCreateLocation, globalLowStockThreshold, spoolmanMode = false, }: AdditionalSectionProps) { @@ -183,6 +185,8 @@ export function AdditionalSection({ const [isMeasuredFocused, setIsMeasuredFocused] = useState(false); const [remainingInput, setRemainingInput] = useState(''); const [isRemainingFocused, setIsRemainingFocused] = useState(false); + const [newLocationName, setNewLocationName] = useState(''); + const [creatingLocation, setCreatingLocation] = useState(false); const remainingWeight = Math.max(0, formData.label_weight - formData.weight_used); const measuredDefault = formData.core_weight + remainingWeight; @@ -381,15 +385,61 @@ export function AdditionalSection({ {/* Storage Location */}
- - updateField('storage_location', e.target.value)} - /> + + + {onCreateLocation && ( +
+ setNewLocationName(e.target.value)} + /> + +
+ )}
); diff --git a/frontend/src/components/spool-form/types.ts b/frontend/src/components/spool-form/types.ts index a1c2083d4..5d5bccb9f 100644 --- a/frontend/src/components/spool-form/types.ts +++ b/frontend/src/components/spool-form/types.ts @@ -36,7 +36,7 @@ export interface SpoolFormData { // User-defined category + per-spool low-stock threshold override (#729). category: string; low_stock_threshold_pct: number | null; - storage_location: string; + location_id: number | null; // When set the spool is linked to a specific Spoolman filament catalog entry; // the backend skips find_or_create_filament() and uses this ID directly. spoolman_filament_id: number | null; @@ -59,7 +59,7 @@ export const defaultFormData: SpoolFormData = { cost_per_kg: null, category: '', low_stock_threshold_pct: null, - storage_location: '', + location_id: null, spoolman_filament_id: null, }; @@ -143,6 +143,8 @@ export interface AdditionalSectionProps extends SectionProps { // Global low-stock threshold (%); shown as placeholder on the per-spool // override input so users see what they're overriding. #729 globalLowStockThreshold: number; + availableLocations?: { id: number; name: string }[]; + onCreateLocation?: (name: string) => Promise<{ id: number; name: string } | null>; // When true the empty-spool weight is managed by Spoolman on the filament // object, so SpoolWeightPicker is hidden and an info notice is shown instead. spoolmanMode?: boolean; diff --git a/frontend/src/hooks/useWebSocket.ts b/frontend/src/hooks/useWebSocket.ts index e99b4ab99..773f98294 100644 --- a/frontend/src/hooks/useWebSocket.ts +++ b/frontend/src/hooks/useWebSocket.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { useToast } from '../contexts/ToastContext'; import { useTranslation } from 'react-i18next'; import { api } from '../api/client'; +import { inventoryLocationsQueryKey } from '../utils/inventoryQueries'; interface WebSocketMessage { type: string; @@ -288,6 +289,8 @@ export function useWebSocket() { case 'inventory_changed': // Spool created/updated/deleted/archived/restored - refresh inventory across all tabs debouncedInvalidate('inventory-spools'); + debouncedInvalidate('spoolman-inventory-spools'); + debouncedInvalidate(inventoryLocationsQueryKey[0]); break; case 'spool_assignment_changed': diff --git a/frontend/src/i18n/locales/de.ts b/frontend/src/i18n/locales/de.ts index 93774582a..149b8d289 100644 --- a/frontend/src/i18n/locales/de.ts +++ b/frontend/src/i18n/locales/de.ts @@ -3690,6 +3690,28 @@ export default { reportPartialUsageDesc: 'Wenn ein Druck fehlschlägt oder abgebrochen wird, den geschätzten Filamentverbrauch bis zu diesem Zeitpunkt basierend auf dem Schichtfortschritt melden.', }, + locations: { + title: 'Lagerorte', + subtitle: 'Regale, Schubladen und andere physische Lagerplätze für Spulen verwalten', + add: 'Lagerort hinzufügen', + addShort: 'Hinzufügen', + edit: 'Lagerort bearbeiten', + name: 'Name', + spools: 'Spulen', + empty: 'Noch keine Lagerorte. Erstellen Sie Ihr erstes Regal oder Ihre erste Schublade.', + manage: 'Lagerorte', + createPlaceholder: 'z. B. Regal A, Schublade 1', + nameRequired: 'Name des Lagerorts ist erforderlich', + created: 'Lagerort erstellt', + updated: 'Lagerort aktualisiert', + deleted: 'Lagerort gelöscht', + saveFailed: 'Lagerort konnte nicht gespeichert werden', + deleteFailed: 'Lagerort konnte nicht gelöscht werden', + deleteBlocked: 'Entfernen Sie zuerst alle Spulen von diesem Lagerort', + confirmDelete: '„{{name}}“ löschen?', + confirmDeleteMessage: 'Dieser Lagerort wird aus dem Katalog entfernt. Spulen müssen zuerst verschoben werden.', + }, + // Inventar inventory: { title: 'Spulen-Inventar', diff --git a/frontend/src/i18n/locales/en.ts b/frontend/src/i18n/locales/en.ts index 88f6cb400..1bb70b2f0 100644 --- a/frontend/src/i18n/locales/en.ts +++ b/frontend/src/i18n/locales/en.ts @@ -3701,6 +3701,28 @@ export default { reportPartialUsageDesc: 'When a print fails or is cancelled, report the estimated filament used up to that point based on layer progress.', }, + locations: { + title: 'Storage Locations', + subtitle: 'Manage shelves, drawers, and other physical storage spots for your spools', + add: 'Add Location', + addShort: 'Add', + edit: 'Edit Location', + name: 'Name', + spools: 'Spools', + empty: 'No storage locations yet. Create your first shelf or drawer.', + manage: 'Locations', + createPlaceholder: 'e.g. Shelf A, Drawer 1', + nameRequired: 'Location name is required', + created: 'Location created', + updated: 'Location updated', + deleted: 'Location deleted', + saveFailed: 'Failed to save location', + deleteFailed: 'Failed to delete location', + deleteBlocked: 'Remove all spools from this location before deleting', + confirmDelete: 'Delete "{{name}}"?', + confirmDeleteMessage: 'This location will be removed from the catalog. Spools must be moved first.', + }, + // Inventory inventory: { title: 'Spool Inventory', diff --git a/frontend/src/i18n/locales/es.ts b/frontend/src/i18n/locales/es.ts index 0ef2a1402..0da10881b 100644 --- a/frontend/src/i18n/locales/es.ts +++ b/frontend/src/i18n/locales/es.ts @@ -3693,6 +3693,28 @@ export default { reportPartialUsageDesc: 'Cuando una impresión falla o se cancela, informar del filamento estimado usado hasta ese punto según el progreso de las capas.', }, + locations: { + title: 'Ubicaciones de almacenamiento', + subtitle: 'Gestione estantes, cajones y otros lugares físicos para sus bobinas', + add: 'Añadir ubicación', + addShort: 'Añadir', + edit: 'Editar ubicación', + name: 'Nombre', + spools: 'Bobinas', + empty: 'Aún no hay ubicaciones de almacenamiento. Cree su primer estante o cajón.', + manage: 'Ubicaciones', + createPlaceholder: 'p. ej. Estante A, Cajón 1', + nameRequired: 'El nombre de la ubicación es obligatorio', + created: 'Ubicación creada', + updated: 'Ubicación actualizada', + deleted: 'Ubicación eliminada', + saveFailed: 'No se pudo guardar la ubicación', + deleteFailed: 'No se pudo eliminar la ubicación', + deleteBlocked: 'Retire todas las bobinas de esta ubicación antes de eliminarla', + confirmDelete: '¿Eliminar «{{name}}»?', + confirmDeleteMessage: 'Esta ubicación se eliminará del catálogo. Mueva las bobinas primero.', + }, + // Inventory inventory: { title: 'Inventario de bobinas', diff --git a/frontend/src/i18n/locales/fr.ts b/frontend/src/i18n/locales/fr.ts index 01d07b715..adb20d2cf 100644 --- a/frontend/src/i18n/locales/fr.ts +++ b/frontend/src/i18n/locales/fr.ts @@ -3679,6 +3679,28 @@ export default { reportPartialUsageDesc: 'Si l\'impression échoue, rapporte le filament consommé selon les couches.', }, + locations: { + title: 'Emplacements de stockage', + subtitle: 'Gérez étagères, tiroirs et autres emplacements physiques pour vos bobines', + add: 'Ajouter un emplacement', + addShort: 'Ajouter', + edit: 'Modifier l\'emplacement', + name: 'Nom', + spools: 'Bobines', + empty: 'Aucun emplacement de stockage. Créez votre première étagère ou tiroir.', + manage: 'Emplacements', + createPlaceholder: 'ex. Étagère A, Tiroir 1', + nameRequired: 'Le nom de l\'emplacement est requis', + created: 'Emplacement créé', + updated: 'Emplacement mis à jour', + deleted: 'Emplacement supprimé', + saveFailed: 'Échec de l\'enregistrement de l\'emplacement', + deleteFailed: 'Échec de la suppression de l\'emplacement', + deleteBlocked: 'Retirez d\'abord toutes les bobines de cet emplacement', + confirmDelete: 'Supprimer « {{name}} » ?', + confirmDeleteMessage: 'Cet emplacement sera retiré du catalogue. Déplacez d\'abord les bobines.', + }, + // Inventory inventory: { title: 'Inventaire de Bobines', diff --git a/frontend/src/i18n/locales/it.ts b/frontend/src/i18n/locales/it.ts index 075ba5173..757daa5e1 100644 --- a/frontend/src/i18n/locales/it.ts +++ b/frontend/src/i18n/locales/it.ts @@ -3678,6 +3678,28 @@ export default { reportPartialUsageDesc: 'Quando una stampa fallisce o viene annullata, segnala il filamento stimato usato fino a quel punto in base all\'avanzamento layer.', }, + locations: { + title: 'Ubicazioni di stoccaggio', + subtitle: 'Gestisci scaffali, cassetti e altri posti fisici per le bobine', + add: 'Aggiungi ubicazione', + addShort: 'Aggiungi', + edit: 'Modifica ubicazione', + name: 'Nome', + spools: 'Bobine', + empty: 'Nessuna ubicazione di stoccaggio. Crea il tuo primo scaffale o cassetto.', + manage: 'Ubicazioni', + createPlaceholder: 'es. Scaffale A, Cassetto 1', + nameRequired: 'Il nome dell\'ubicazione è obbligatorio', + created: 'Ubicazione creata', + updated: 'Ubicazione aggiornata', + deleted: 'Ubicazione eliminata', + saveFailed: 'Impossibile salvare l\'ubicazione', + deleteFailed: 'Impossibile eliminare l\'ubicazione', + deleteBlocked: 'Rimuovi prima tutte le bobine da questa ubicazione', + confirmDelete: 'Eliminare «{{name}}»?', + confirmDeleteMessage: 'Questa ubicazione verrà rimossa dal catalogo. Sposta prima le bobine.', + }, + // Inventory inventory: { title: 'Inventario Bobine', diff --git a/frontend/src/i18n/locales/ja.ts b/frontend/src/i18n/locales/ja.ts index 852b3a75d..38f694cfc 100644 --- a/frontend/src/i18n/locales/ja.ts +++ b/frontend/src/i18n/locales/ja.ts @@ -3690,6 +3690,28 @@ export default { reportPartialUsageDesc: '印刷が失敗またはキャンセルされた場合、レイヤー進捗に基づいてその時点までの推定フィラメント使用量を報告します。', }, + locations: { + title: '保管場所', + subtitle: '棚・引き出しなど、スプールの物理的な保管場所を管理', + add: '場所を追加', + addShort: '追加', + edit: '場所を編集', + name: '名前', + spools: 'スプール', + empty: '保管場所がありません。最初の棚または引き出しを作成してください。', + manage: '保管場所', + createPlaceholder: '例: 棚A、引き出し1', + nameRequired: '場所名が必要です', + created: '場所を作成しました', + updated: '場所を更新しました', + deleted: '場所を削除しました', + saveFailed: '場所の保存に失敗しました', + deleteFailed: '場所の削除に失敗しました', + deleteBlocked: '削除前にこの場所のスプールをすべて移動してください', + confirmDelete: '「{{name}}」を削除しますか?', + confirmDeleteMessage: 'この場所はカタログから削除されます。先にスプールを移動してください。', + }, + // Inventory inventory: { title: 'スプール在庫管理', diff --git a/frontend/src/i18n/locales/ko.ts b/frontend/src/i18n/locales/ko.ts index e727331ac..30c4f637a 100644 --- a/frontend/src/i18n/locales/ko.ts +++ b/frontend/src/i18n/locales/ko.ts @@ -3483,6 +3483,29 @@ export default { reportPartialUsage: '실패한 인쇄물에 대한 부분 사용량 보고', reportPartialUsageDesc: '인쇄가 실패하거나 취소될 때 레이어 진행률을 기반으로 해당 시점까지 사용된 예상 필라멘트를 보고합니다.' }, + + locations: { + title: '보관 위치', + subtitle: '스풀의 선반, 서랍 등 물리적 보관 장소를 관리합니다', + add: '위치 추가', + addShort: '추가', + edit: '위치 편집', + name: '이름', + spools: '스풀', + empty: '아직 보관 위치가 없습니다. 첫 번째 선반이나 서랍을 만드세요.', + manage: '위치', + createPlaceholder: '예: 선반 A, 서랍 1', + nameRequired: '위치 이름은 필수입니다', + created: '위치가 생성되었습니다', + updated: '위치가 업데이트되었습니다', + deleted: '위치가 삭제되었습니다', + saveFailed: '위치 저장에 실패했습니다', + deleteFailed: '위치 삭제에 실패했습니다', + deleteBlocked: '삭제하기 전에 이 위치의 모든 스풀을 옮기세요', + confirmDelete: '"{{name}}"을(를) 삭제하시겠습니까?', + confirmDeleteMessage: '이 위치가 카탈로그에서 제거됩니다. 스풀을 먼저 옮겨야 합니다.', + }, + inventory: { title: '스풀 재고', spoolmanMixedContentTitle: 'HTTPS에서 Spoolman을 불러올 수 없음 — 브라우저가 혼합 콘텐츠를 차단함', diff --git a/frontend/src/i18n/locales/pt-BR.ts b/frontend/src/i18n/locales/pt-BR.ts index 2d500a29f..d2c1837fb 100644 --- a/frontend/src/i18n/locales/pt-BR.ts +++ b/frontend/src/i18n/locales/pt-BR.ts @@ -3678,6 +3678,28 @@ export default { reportPartialUsageDesc: 'Quando uma impressão falha ou é cancelada, relate o filamento estimado usado até aquele ponto com base no progresso das camadas.', }, + locations: { + title: 'Locais de armazenamento', + subtitle: 'Gerencie prateleiras, gavetas e outros locais físicos para bobinas', + add: 'Adicionar local', + addShort: 'Adicionar', + edit: 'Editar local', + name: 'Nome', + spools: 'Bobinas', + empty: 'Nenhum local de armazenamento. Crie sua primeira prateleira ou gaveta.', + manage: 'Locais', + createPlaceholder: 'ex. Prateleira A, Gaveta 1', + nameRequired: 'O nome do local é obrigatório', + created: 'Local criado', + updated: 'Local atualizado', + deleted: 'Local excluído', + saveFailed: 'Falha ao salvar local', + deleteFailed: 'Falha ao excluir local', + deleteBlocked: 'Remova todas as bobinas deste local antes de excluir', + confirmDelete: 'Excluir «{{name}}»?', + confirmDeleteMessage: 'Este local será removido do catálogo. Mova as bobinas primeiro.', + }, + // Inventory inventory: { title: 'Inventário de Carretéis', diff --git a/frontend/src/i18n/locales/tr.ts b/frontend/src/i18n/locales/tr.ts index e9804740b..210b69be2 100644 --- a/frontend/src/i18n/locales/tr.ts +++ b/frontend/src/i18n/locales/tr.ts @@ -3679,6 +3679,28 @@ export default { reportPartialUsageDesc: 'Bir baskı başarısız olduğunda veya iptal edildiğinde, katman ilerlemesine göre o noktaya kadar kullanılan tahmini filamenti bildir.', }, + locations: { + title: 'Depolama Konumları', + subtitle: 'Makaralarınız için raf, çekmece ve diğer fiziksel depolama yerlerini yönetin', + add: 'Konum Ekle', + addShort: 'Ekle', + edit: 'Konumu Düzenle', + name: 'Ad', + spools: 'Makaralar', + empty: 'Henüz depolama konumu yok. İlk rafınızı veya çekmecenizi oluşturun.', + manage: 'Konumlar', + createPlaceholder: 'örn. Raf A, Çekmece 1', + nameRequired: 'Konum adı zorunludur', + created: 'Konum oluşturuldu', + updated: 'Konum güncellendi', + deleted: 'Konum silindi', + saveFailed: 'Konum kaydedilemedi', + deleteFailed: 'Konum silinemedi', + deleteBlocked: 'Silmeden önce bu konumdaki tüm makaraları taşıyın', + confirmDelete: '"{{name}}" silinsin mi?', + confirmDeleteMessage: 'Bu konum kataloğdan kaldırılacak. Önce makaralar taşınmalıdır.', + }, + // Envanter inventory: { title: 'Makara Envanteri', diff --git a/frontend/src/i18n/locales/zh-CN.ts b/frontend/src/i18n/locales/zh-CN.ts index aa9758d13..82e763660 100644 --- a/frontend/src/i18n/locales/zh-CN.ts +++ b/frontend/src/i18n/locales/zh-CN.ts @@ -3678,6 +3678,28 @@ export default { reportPartialUsageDesc: '当打印失败或被取消时,根据层进度报告估计的耗材使用量。', }, + locations: { + title: '存储位置', + subtitle: '管理货架、抽屉等线轴物理存放位置', + add: '添加位置', + addShort: '添加', + edit: '编辑位置', + name: '名称', + spools: '线轴', + empty: '尚无存储位置。创建第一个货架或抽屉。', + manage: '位置', + createPlaceholder: '例如:A 架、抽屉 1', + nameRequired: '位置名称为必填项', + created: '位置已创建', + updated: '位置已更新', + deleted: '位置已删除', + saveFailed: '保存位置失败', + deleteFailed: '删除位置失败', + deleteBlocked: '删除前请移走此位置上的所有线轴', + confirmDelete: '删除「{{name}}」?', + confirmDeleteMessage: '此位置将从目录中移除。请先移走线轴。', + }, + // Inventory inventory: { title: '耗材库存', diff --git a/frontend/src/i18n/locales/zh-TW.ts b/frontend/src/i18n/locales/zh-TW.ts index 0f357e91a..d983b19cd 100644 --- a/frontend/src/i18n/locales/zh-TW.ts +++ b/frontend/src/i18n/locales/zh-TW.ts @@ -3678,6 +3678,28 @@ export default { reportPartialUsageDesc: '當列印失敗或被取消時,根據層進度報告估計的耗材使用量。', }, + locations: { + title: '儲存位置', + subtitle: '管理貨架、抽屜等線軸實體存放位置', + add: '新增位置', + addShort: '新增', + edit: '編輯位置', + name: '名稱', + spools: '線軸', + empty: '尚無儲存位置。建立第一個貨架或抽屜。', + manage: '位置', + createPlaceholder: '例如:A 架、抽屜 1', + nameRequired: '位置名稱為必填', + created: '位置已建立', + updated: '位置已更新', + deleted: '位置已刪除', + saveFailed: '儲存位置失敗', + deleteFailed: '刪除位置失敗', + deleteBlocked: '刪除前請移走此位置上的所有線軸', + confirmDelete: '刪除「{{name}}」?', + confirmDeleteMessage: '此位置將從目錄中移除。請先移走線軸。', + }, + // Inventory inventory: { title: '耗材庫存', diff --git a/frontend/src/pages/InventoryPage.tsx b/frontend/src/pages/InventoryPage.tsx index 2eb83868f..87dbc1920 100644 --- a/frontend/src/pages/InventoryPage.tsx +++ b/frontend/src/pages/InventoryPage.tsx @@ -6,7 +6,7 @@ import { Plus, Loader2, Trash2, Archive, RotateCcw, Edit2, Package, Search, ChevronLeft, ChevronRight, ChevronsLeft, ChevronsRight, TrendingDown, Layers, Printer, AlertTriangle, X, Clock, LayoutGrid, TableProperties, Columns, - ArrowUp, ArrowDown, ArrowUpDown, Group, ChevronDown, Check, RefreshCw, TrendingUp, Lock, Copy, Eraser, + ArrowUp, ArrowDown, ArrowUpDown, Group, ChevronDown, Check, RefreshCw, TrendingUp, Lock, Copy, Eraser, MapPin, Upload, Download, } from 'lucide-react'; import { ForecastPanel } from '../components/ForecastPanel'; @@ -20,6 +20,7 @@ import { ConfirmModal } from '../components/ConfirmModal'; import { ColumnConfigModal, type ColumnConfig } from '../components/ColumnConfigModal'; import { LabelTemplatePickerModal } from '../components/LabelTemplatePickerModal'; import { SpoolCsvImportModal } from '../components/SpoolCsvImportModal'; +import { LocationsModal } from '../components/LocationsModal'; import { useToast } from '../contexts/ToastContext'; import { useAuth } from '../contexts/AuthContext'; import { resolveSpoolColorName } from '../utils/colors'; @@ -27,6 +28,10 @@ import { getCurrencySymbol } from '../utils/currency'; import { formatDateInput, parseUTCDate, type DateFormat } from '../utils/date'; import { formatSlotLabel } from '../utils/amsHelpers'; import { filterSpoolsByQuery } from '../utils/inventorySearch'; +import { + inventoryLocationsQueryKey, + invalidateSpoolAndLocationQueries, +} from '../utils/inventoryQueries'; import { aggregateGroupSpool } from '../utils/inventoryGrouping'; type ArchiveFilter = 'active' | 'archived'; @@ -478,6 +483,7 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo // CSV import/export (#1576). Local inventory only — hidden in Spoolman mode. const [csvImportOpen, setCsvImportOpen] = useState(false); const [exportingCsv, setExportingCsv] = useState(false); + const [locationsModalOpen, setLocationsModalOpen] = useState(false); // Filter state const [archiveFilter, setArchiveFilter] = useState('active'); @@ -487,10 +493,6 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo const [categoryFilter, setCategoryFilter] = useState(''); const [spoolFilter, setSpoolFilter] = useState(''); const [stockFilter, setStockFilter] = useState<'all' | 'stock' | 'configured'>('all'); - // #1400: storage-location dropdown. Uses the sentinel `__none__` for the - // "no storage location set" group, same pattern as the category filter so - // users can find unfiled spools. - const [storageLocationFilter, setStorageLocationFilter] = useState(''); const [search, setSearch] = useState(''); const [viewMode, setViewMode] = useState('table'); const [sortState, setSortState] = useState(loadSortState); @@ -525,6 +527,7 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo // Query key and fetch function differ based on data source const spoolsQueryKey = spoolmanMode ? ['spoolman-inventory-spools'] : ['inventory-spools']; + const refreshSpoolQueries = () => invalidateSpoolAndLocationQueries(queryClient, spoolsQueryKey); const { data: spools, isLoading } = useQuery({ queryKey: spoolsQueryKey, queryFn: () => @@ -552,6 +555,20 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo ? t('inventory.csv.spoolmanHint', 'In Spoolman mode, use Spoolman\'s built-in CSV import/export.') : undefined; + const { data: storageLocations = [] } = useQuery({ + queryKey: inventoryLocationsQueryKey, + queryFn: api.getLocations, + }); + + // Deep-link / filter: ?location_id= or ?location_id=__none__ + const _rawLocationParam = searchParams.get('location_id'); + const storageLocationFilter = + _rawLocationParam === '__none__' + ? '__none__' + : _rawLocationParam && /^\d+$/.test(_rawLocationParam) && Number(_rawLocationParam) > 0 + ? _rawLocationParam + : ''; + // Deep-link: open edit modal for ?spool= // Prefer the already-loaded spool list (no extra API call); fall back to a // targeted fetch for the rare case where the full list hasn't arrived yet. @@ -662,7 +679,7 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo mutationFn: (id: number) => spoolmanMode ? api.deleteSpoolmanInventorySpool(id) : api.deleteSpool(id), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + refreshSpoolQueries(); showToast(t('inventory.spoolDeleted'), 'success'); }, onError: (error: Error) => { @@ -680,7 +697,7 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo mutationFn: (id: number) => spoolmanMode ? api.archiveSpoolmanInventorySpool(id) : api.archiveSpool(id), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + refreshSpoolQueries(); showToast(t('inventory.spoolArchived'), 'success'); }, onError: (error: Error) => { @@ -698,7 +715,7 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo mutationFn: (id: number) => spoolmanMode ? api.restoreSpoolmanInventorySpool(id) : api.restoreSpool(id), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: spoolsQueryKey }); + refreshSpoolQueries(); showToast(t('inventory.spoolRestored'), 'success'); }, onError: (error: Error) => { @@ -948,9 +965,15 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo // spools that haven't been assigned a storage location yet. if (storageLocationFilter) { if (storageLocationFilter === '__none__') { - filtered = filtered.filter((s) => !s.storage_location?.trim()); + filtered = filtered.filter((s) => !s.location_id && !s.storage_location?.trim()); } else { - filtered = filtered.filter((s) => s.storage_location?.trim() === storageLocationFilter); + const locId = Number(storageLocationFilter); + const locName = storageLocations.find((l) => l.id === locId)?.name?.trim().toLowerCase(); + filtered = filtered.filter((s) => { + if (s.location_id != null) return s.location_id === locId; + if (locName) return (s.storage_location || '').trim().toLowerCase() === locName; + return false; + }); } } @@ -967,11 +990,22 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo } return filtered; - }, [spools, archiveFilter, usageFilter, materialFilter, brandFilter, categoryFilter, spoolFilter, stockFilter, storageLocationFilter, search, lowStockThreshold]); + }, [spools, archiveFilter, usageFilter, materialFilter, brandFilter, categoryFilter, spoolFilter, stockFilter, storageLocationFilter, search, lowStockThreshold, storageLocations]); // Reset page on filter changes const resetPage = () => setPageIndex(0); + const setStorageLocationFilter = useCallback((value: string) => { + setSearchParams((prev) => { + prev.delete('location_id'); + if (value) { + prev.set('location_id', value); + } + return prev; + }, { replace: true }); + resetPage(); + }, [setSearchParams]); + // Unique values for filter dropdowns const uniqueMaterials = [...new Set(spools?.map((s) => s.material) || [])].sort(); const uniqueBrands = [...new Set(spools?.map((s) => s.brand).filter(Boolean) || [])].sort() as string[]; @@ -984,8 +1018,7 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo }); // #1400: storage-location distinct values. `.trim()` so accidental // trailing whitespace doesn't show up as a separate option. - const uniqueStorageLocations = [...new Set(spools?.map((s) => s.storage_location?.trim()).filter(Boolean) as string[] || [])].sort(); - const hasUnsetStorageLocation = (spools ?? []).some((s) => !s.storage_location?.trim()); + const hasUnsetStorageLocation = (spools ?? []).some((s) => !s.location_id && !s.storage_location?.trim()); // Check if any filters are non-default const hasActiveFilters = archiveFilter !== 'active' || usageFilter !== 'all' || !!materialFilter || !!brandFilter || !!categoryFilter || !!spoolFilter || !!storageLocationFilter || stockFilter !== 'all' || !!search; @@ -1111,9 +1144,12 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo setBrandFilter(''); setCategoryFilter(''); setSpoolFilter(''); - setStorageLocationFilter(''); setStockFilter('all'); setSearch(''); + setSearchParams((prev) => { + prev.delete('location_id'); + return prev; + }, { replace: true }); resetPage(); }; @@ -1151,6 +1187,10 @@ function InventoryPage({ spoolmanMode = false, spoolmanModeReady = true }: { spo {exportingCsv ? : } {t('inventory.csv.exportButton', 'Export CSV')} +