mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 11:56:01 +02:00
fix(vp): show target printer's serial on proxy-mode VP card
The runtime services (SSDP, MQTT bind identity, cert subject) already advertise the target printer's serial via target_printer_serial or self.serial in proxy mode, but the API response that drives the VP settings card always returned the self-generated suffix-based serial. The card therefore displayed a serial that didn't match what slicers see, breaking the "one identity per VP" mental model. _vp_to_dict now resolves vp.target_printer_id -> Printer.serial_number when mode == VP_MODE_PROXY and substitutes the result into the response serial field. Archive / queue / review keep the self-generated serial (those modes never speak the target's identity). Orphaned target falls back to self-generated so the card still renders.
This commit is contained in:
@@ -4,6 +4,9 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
|
||||
## [0.2.5b1] - Unreleased
|
||||
|
||||
### Fixed
|
||||
- **VP settings card now shows the target printer's serial in proxy mode** — On a proxy-mode VP, the runtime services (SSDP advertisement, MQTT bind identity, certificate subject) all use the target printer's actual serial via `target_printer_serial or self.serial` (`manager.py:235, 941, 957`), but the `/api/v1/virtual-printers` response — which feeds the VP settings card — always returned the self-generated suffix-based serial from `_get_serial_for_model(model_code, vp.serial_suffix)`. The card therefore displayed a serial that didn't match what the bridge actually advertises and what the slicer sees, breaking the visual "one identity per VP" mental model. **Fix:** `_vp_to_dict` (`api/routes/virtual_printers.py:77`) is now async and accepts `db`; when `vp.mode == VP_MODE_PROXY and vp.target_printer_id`, it issues a single `SELECT serial_number FROM printers WHERE id = vp.target_printer_id` and substitutes the result into the response `serial` field. Archive / queue / review modes keep the self-generated serial — those modes synthesise their own identity and never speak the target's. **Defensive fallback** when the target row is missing (printer deleted mid-config, manual SQL tweak, race between delete-printer and read-VP): the response falls back to the self-generated serial so the card still renders and the user can fix the target, rather than the API 500-ing. All 4 `_vp_to_dict` call sites (list, create, get, update) updated to `await` with `db`. **Tests:** 3 new in `TestVirtualPrinterSerialSurface` — proxy VP returns target serial across all three response paths (create / get / list), non-proxy VP with a target still uses the self-generated serial, orphaned proxy VP falls back to self-generated. Full VP API suite stays green (34/34); VP unit suite stays green (126/126); ruff clean.
|
||||
|
||||
### Added
|
||||
- **Inventory page now supports native CSV import / export (#1576, PR #1659 by @samedyuksel)** — Bulk-add spools without manually clicking through the form, and back up / migrate the local inventory in a single round-trip. Export downloads `bambuddy-spools-YYYY-MM-DD.csv` (header + one row per active spool); Import shows a preview table that classifies each row as valid / error / skipped before anything hits the database, then a confirm click persists only the valid rows in one transaction (invalid rows are skipped, the user fixes them and re-uploads). Local inventory only — in Spoolman mode the buttons render disabled with a tooltip pointing at Spoolman's own CSV import/export, since the Spoolman backend has its own data store. **Schema**: fixed 18 columns, case- and whitespace-tolerant headers, includes `weight_used`, `last_used`, and the SpoolCreate fields `storage_location` / `category` / `low_stock_threshold_pct` so the round-trip preserves the per-spool location data from #1291. `remaining` is a derived, export-only column (`label_weight - weight_used`, clamped at 0) — it's written for human readability and ignored on import (weight_used is the source of truth, accepting both would let them contradict). **Colour resolution**: explicit `rgba` wins, otherwise `brand + color_name` resolves against the Color Catalog (case-insensitive, single in-memory pass — no N+1); a catalog entry with `material = NULL` is treated as the project's "matches any material" convention so a generic match counts as exact rather than firing the cross-material warning. Validation reuses `SpoolCreate` so every constraint that already protects manual adds (`weight_used >= 0`, `weight_used <= label_weight`, `low_stock_threshold_pct` range, etc.) protects bulk imports too. **Hardening**: 5 MB upload cap with a structured `csv_import_too_large` 413 response — Bambuddy doesn't have a global HTTP-level cap so the check lives on the route, and the implementation is a bounded 64 KB chunked read that bails the moment the accumulated body crosses the cap (file.size is `None` for chunked uploads so the loop is what actually prevents the OOM, not the pre-check). Spreadsheet formula-injection guard: every exported cell starting with `=` / `+` / `-` / `@` / tab / CR is prefixed with a single quote on export, and the inverse strip on import keeps the round-trip lossless instead of accumulating quotes on every cycle. Soft-warn surface in the preview: a `duplicate_of_existing` flag fires when an active spool with the same material + brand + color_name exists (single SELECT, no N+1) so a double-click or re-upload of the same CSV doesn't silently duplicate the inventory — the row still imports (Spool has no unique constraint, by design), but the preview renders a Copy icon + tooltip so the user knows. **Frontend**: new `SpoolCsvImportModal` (file pick → preview table with per-row status / colour swatch / warnings → confirm imports valid rows) wired to Import + Export buttons on the inventory header; swatch rendering uses the existing `getSwatchStyle` helper so alpha=00 shows the checkerboard underlay instead of rendering as solid black, matching the rest of the inventory surface. **i18n**: new `inventory.csv` namespace with full translations in all 11 locales (de/en/es/fr/it/ja/ko/pt-BR/tr/zh-CN/zh-TW). **Tests**: 25 backend integration cases pin every behaviour — export shape, import dry-run vs real, color resolution (catalog hit, explicit rgba wins, cross-material flagged, exact-material match, generic-material match not flagged), 5 MB rejection, weight_used bounds, formula-injection round-trip without quote accumulation, dated filename, extra-column round-trip, duplicate-warn flag. Plus 3 frontend modal tests. Full backend suite + ruff + ESLint + frontend build + i18n parity (5092 leaves × 11 locales) green. **Companion docs**: wiki PR maziggy/bambuddy-wiki#41 documents the schema, behaviour, and the Spoolman-mode disabled-with-tooltip semantics.
|
||||
|
||||
|
||||
@@ -74,13 +74,26 @@ def _resolve_printer_model(printer_model: str | None) -> str | None:
|
||||
return DISPLAY_NAME_TO_MODEL_CODE.get(printer_model)
|
||||
|
||||
|
||||
def _vp_to_dict(vp, status: dict | None = None) -> dict:
|
||||
"""Convert VirtualPrinter model to response dict."""
|
||||
async def _vp_to_dict(vp, db: AsyncSession, status: dict | None = None) -> dict:
|
||||
"""Convert VirtualPrinter model to response dict.
|
||||
|
||||
In proxy mode the surfaced serial is the target printer's actual serial
|
||||
(what the bridge advertises over SSDP / what slicers see), not the
|
||||
self-generated suffix. Archive / queue / review keep the self-generated
|
||||
serial since those modes never speak the target's identity.
|
||||
"""
|
||||
from backend.app.models.printer import Printer
|
||||
from backend.app.models.virtual_printer import VP_MODE_PROXY
|
||||
from backend.app.services.virtual_printer import VIRTUAL_PRINTER_MODELS
|
||||
from backend.app.services.virtual_printer.manager import DEFAULT_VIRTUAL_PRINTER_MODEL, _get_serial_for_model
|
||||
|
||||
model_code = vp.model or DEFAULT_VIRTUAL_PRINTER_MODEL
|
||||
serial = _get_serial_for_model(model_code, vp.serial_suffix)
|
||||
if vp.mode == VP_MODE_PROXY and vp.target_printer_id:
|
||||
result = await db.execute(select(Printer.serial_number).where(Printer.id == vp.target_printer_id))
|
||||
target_serial = result.scalar_one_or_none()
|
||||
if target_serial:
|
||||
serial = target_serial
|
||||
|
||||
return {
|
||||
"id": vp.id,
|
||||
@@ -118,7 +131,7 @@ async def list_virtual_printers(
|
||||
for vp in vps:
|
||||
instance = virtual_printer_manager.get_instance(vp.id)
|
||||
status = instance.get_status() if instance else {"running": False, "pending_files": 0}
|
||||
printers.append(_vp_to_dict(vp, status))
|
||||
printers.append(await _vp_to_dict(vp, db, status))
|
||||
|
||||
return {
|
||||
"printers": printers,
|
||||
@@ -247,7 +260,7 @@ async def create_virtual_printer(
|
||||
except Exception as e:
|
||||
logger.error("Failed to start virtual printer after create: %s", e)
|
||||
|
||||
return _vp_to_dict(vp)
|
||||
return await _vp_to_dict(vp, db)
|
||||
|
||||
|
||||
@router.get("/tailscale-status", response_model=TailscaleStatusResponse)
|
||||
@@ -331,7 +344,7 @@ async def get_virtual_printer(
|
||||
instance = virtual_printer_manager.get_instance(vp.id)
|
||||
status = instance.get_status() if instance else {"running": False, "pending_files": 0}
|
||||
|
||||
return _vp_to_dict(vp, status)
|
||||
return await _vp_to_dict(vp, db, status)
|
||||
|
||||
|
||||
@router.put("/{vp_id}")
|
||||
@@ -506,7 +519,7 @@ async def update_virtual_printer(
|
||||
instance = virtual_printer_manager.get_instance(vp.id)
|
||||
status = instance.get_status() if instance else {"running": False, "pending_files": 0}
|
||||
|
||||
return _vp_to_dict(vp, status)
|
||||
return await _vp_to_dict(vp, db, status)
|
||||
|
||||
|
||||
@router.delete("/{vp_id}")
|
||||
|
||||
@@ -599,3 +599,83 @@ class TestVirtualPrinterAccessCodeInheritance:
|
||||
|
||||
vp = (await db_session.execute(select(VirtualPrinter).where(VirtualPrinter.id == vp_id))).scalar_one()
|
||||
assert vp.access_code == "REALCODE"
|
||||
|
||||
|
||||
class TestVirtualPrinterSerialSurface:
|
||||
"""Proxy-mode VPs must surface the target printer's serial in API responses.
|
||||
|
||||
The bridge advertises the target's serial over SSDP and forwards the
|
||||
target's identity to the slicer; the VP-settings card should show the
|
||||
same serial so the user sees one consistent identity per VP. Archive /
|
||||
queue / review VPs keep the self-generated suffix-based serial since
|
||||
those modes never speak the target's identity.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_proxy_vp_response_uses_target_printer_serial(self, async_client: AsyncClient, printer_factory):
|
||||
target = await printer_factory(name="Real X1C", access_code="REALCODE", serial_number="00M09A123456789")
|
||||
|
||||
create_resp = await async_client.post(
|
||||
"/api/v1/virtual-printers",
|
||||
json={
|
||||
"name": "ProxyVP",
|
||||
"mode": "proxy",
|
||||
"target_printer_id": target.id,
|
||||
},
|
||||
)
|
||||
assert create_resp.status_code == 200
|
||||
vp_id = create_resp.json()["id"]
|
||||
assert create_resp.json()["serial"] == "00M09A123456789"
|
||||
|
||||
get_resp = await async_client.get(f"/api/v1/virtual-printers/{vp_id}")
|
||||
assert get_resp.status_code == 200
|
||||
assert get_resp.json()["serial"] == "00M09A123456789"
|
||||
|
||||
list_resp = await async_client.get("/api/v1/virtual-printers")
|
||||
assert list_resp.status_code == 200
|
||||
listed = next(p for p in list_resp.json()["printers"] if p["id"] == vp_id)
|
||||
assert listed["serial"] == "00M09A123456789"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_non_proxy_vp_keeps_self_generated_serial(self, async_client: AsyncClient, printer_factory):
|
||||
# Even with a target printer set (#1429 access-code inheritance flow),
|
||||
# archive / queue / review VPs are NOT bridging the target's identity
|
||||
# to the slicer — they synthesise their own. Self-generated serial.
|
||||
target = await printer_factory(name="Real X1C", access_code="REALCODE", serial_number="00M09A123456789")
|
||||
|
||||
create_resp = await async_client.post(
|
||||
"/api/v1/virtual-printers",
|
||||
json={
|
||||
"name": "QueueVP",
|
||||
"mode": "queue",
|
||||
"target_printer_id": target.id,
|
||||
},
|
||||
)
|
||||
assert create_resp.status_code == 200
|
||||
assert create_resp.json()["serial"] != "00M09A123456789"
|
||||
# The synthesised serial follows _get_serial_for_model's `<prefix><suffix>`
|
||||
# shape — model-specific prefix + 8-char hex suffix from `vp.serial_suffix`.
|
||||
assert len(create_resp.json()["serial"]) >= 8
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_proxy_vp_falls_back_to_self_generated_when_target_missing(
|
||||
self, async_client: AsyncClient, db_session
|
||||
):
|
||||
# Defensive fallback: a proxy VP whose target_printer_id points at a
|
||||
# row that no longer exists (printer deleted mid-config, manual SQL
|
||||
# tweak, race) must not 500 — it returns the self-generated serial
|
||||
# so the card still renders and the user can fix the target.
|
||||
from backend.app.models.virtual_printer import VirtualPrinter
|
||||
|
||||
vp = VirtualPrinter(name="OrphanProxy", mode="proxy", target_printer_id=99999, enabled=False)
|
||||
db_session.add(vp)
|
||||
await db_session.commit()
|
||||
|
||||
get_resp = await async_client.get(f"/api/v1/virtual-printers/{vp.id}")
|
||||
assert get_resp.status_code == 200
|
||||
body = get_resp.json()
|
||||
assert body["serial"] # non-empty
|
||||
assert len(body["serial"]) >= 8
|
||||
|
||||
Reference in New Issue
Block a user