From 461c3f045dcf2b2e10fa07b54815f860a38837d8 Mon Sep 17 00:00:00 2001 From: maziggy Date: Mon, 19 Oct 2026 11:27:38 +0200 Subject: [PATCH] fix(archives): a cost set through the API or by Recalculate reaches the run that had it; edits during a reprint leave the previous run alone; archive cost bounded; a 422 survives Infinity/NaN --- CHANGELOG.md | 3 + backend/app/api/routes/archives.py | 17 ++- backend/app/main.py | 26 +++- backend/app/schemas/archive.py | 3 +- backend/app/services/print_log.py | 15 +++ .../test_archive_edit_cost_and_reprint.py | 123 ++++++++++++++++++ 6 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 backend/tests/integration/test_archive_edit_cost_and_reprint.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e3dde7905..2c9fb40ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,9 @@ All notable changes to Bambuddy will be documented in this file. - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows. ### Fixed +- **An archive cost set through the API, or by Recalculate costs, reaches the statistics** — Setting an archive's cost (`PATCH /api/v1/archives/{id}`) or **Recalculate costs** changed the archive only, while statistics, project totals and batch sums read the cost from the print log, so they kept the old figure. A completed print that had the archive's cost now gets the new one too; a print priced from its own spools, and a failed or cancelled one, keep theirs. **Recalculate costs** still recalculates a cost that was set by hand, as before. A negative cost, or one over 1,000,000, is refused. +- **A request with `Infinity` or `NaN` in a field that fails validation answers 422** — Such a request ended in a server error, because the error response repeats the value sent and JSON can't hold it. +- **Editing an archive while it is being reprinted leaves the previous print alone** — Its outcome, failure reason, verdict and filament weight went onto the previous print's log entry, rewriting that print's result in the print log and failure statistics. The edit now changes the archive only. - **Archives made without their 3MF show their colours and AMS-HT filament** — When a print's 3MF couldn't be downloaded at start, the archive took its filament from the printer's AMS report. Colours were stored as the printer reports them (`FF8800FF`), so their swatches on the Archives page stayed blank and the colour filter listed them apart from the same colour of other archives; they are now stored as `#FF8800`, like an archive with its 3MF, and existing archives and their print log entries are repaired on startup. Filament the print took from an AMS-HT was not found, so a print using only an AMS-HT got no filament. - **An archive whose 3MF arrives later is priced from its spools** — Such an archive started with the default filament price, having no 3MF to estimate from at print start, and kept it when the 3MF was found during the print. It is now estimated from its spools then, as at print start; a 3MF found only as the print ends leaves the pricing to the print's end, as before. - **An archive shows the nozzle temperature of the filament its plate prints** — The temperature was taken from the project's first filament, so a plate printing only PETG showed the PLA temperature of filament 1. It is now that of the plate's first used filament. diff --git a/backend/app/api/routes/archives.py b/backend/app/api/routes/archives.py index d19656835..ecd29343e 100644 --- a/backend/app/api/routes/archives.py +++ b/backend/app/api/routes/archives.py @@ -1939,6 +1939,8 @@ async def update_archive( # Read before the writes below: the mirror needs to know whether the run's # figure was inherited from this archive or measured on its own (#1820). previous_filament_grams = archive.filament_used_grams + previous_cost = archive.cost + previous_status = archive.status update_payload = update_data.model_dump(exclude_unset=True) # Later runs of the archive are charged from its cost and weight: a caller @@ -2006,9 +2008,11 @@ async def update_archive( # ENTRY's grams, not the archive's, so correcting only the archive would fix # the card and leave every aggregate reading the old figure -- or, for a # print that archived without its 3MF, no figure at all. + # While the archive is being reprinted its newest entry is the previous + # run's, whose outcome this edit is not about. mirror_fields = {"failure_reason", "status", "filament_used_grams", "user_verdict"} to_mirror = {k: v for k, v in update_payload.items() if k in mirror_fields} - if to_mirror: + if to_mirror and previous_status != "printing": from backend.app.models.print_log import PrintLogEntry latest_entry = await db.scalar( @@ -2036,6 +2040,13 @@ async def update_archive( for field, value in to_mirror.items(): setattr(latest_entry, field, value) + # A cost set by hand reaches the run that had the archive's (statistics, + # project totals and batch sums read the run's cost). + if "cost" in update_payload and previous_status != "printing": + from backend.app.services.print_log import mirror_archive_cost + + await mirror_archive_cost(db, archive, previous_cost) + await db.commit() # Re-fetch with relationships loaded after commit @@ -2218,6 +2229,7 @@ async def recalculate_all_costs( printer_scope: PrinterScope = RequestPrinterScope, ): """Recalculate costs for all archives based on filament usage and prices.""" + from backend.app.services.print_log import mirror_archive_cost # Only the archives of the caller's printers, as the archive list shows them. query = select(PrintArchive) @@ -2297,7 +2309,10 @@ async def recalculate_all_costs( else: new_cost = None if new_cost is not None and archive.cost != new_cost: + previous_cost = archive.cost archive.cost = new_cost + # Its run had the old figure: statistics read the run's cost. + await mirror_archive_cost(db, archive, previous_cost) updated += 1 await db.commit() diff --git a/backend/app/main.py b/backend/app/main.py index 9f75f0b9a..33254636e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -14,7 +14,9 @@ from pathlib import Path, PurePosixPath from urllib.parse import urlparse from fastapi import FastAPI -from fastapi.responses import FileResponse +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import RequestValidationError +from fastapi.responses import FileResponse, JSONResponse from fastapi.staticfiles import StaticFiles from sqlalchemy import delete, inspect as sa_inspect, or_, select, text, update from sqlalchemy.exc import IntegrityError @@ -11101,6 +11103,28 @@ app = FastAPI( app.add_middleware(RefusedHeaderResponseMask) +def _json_safe(value): + """The validation errors with every non-finite number written as text.""" + if isinstance(value, float) and not math.isfinite(value): + return str(value) + if isinstance(value, dict): + return {key: _json_safe(item) for key, item in value.items()} + if isinstance(value, list | tuple): + return [_json_safe(item) for item in value] + return value + + +@app.exception_handler(RequestValidationError) +async def _validation_error(_request, exc: RequestValidationError) -> JSONResponse: + """FastAPI's own 422, except that it survives a non-finite number. + + A body may carry ``Infinity`` or ``NaN`` (Python's JSON reader accepts + them), and the default handler echoes the input back in its errors, + which JSON cannot hold: the request ended in an error instead of a 422. + """ + return JSONResponse(status_code=422, content={"detail": _json_safe(jsonable_encoder(exc.errors()))}) + + # ============================================================================= # Authentication Middleware - Secures ALL API routes by default # ============================================================================= diff --git a/backend/app/schemas/archive.py b/backend/app/schemas/archive.py index 376bc7ab3..1449bb8a8 100644 --- a/backend/app/schemas/archive.py +++ b/backend/app/schemas/archive.py @@ -17,7 +17,8 @@ class ArchiveBase(BaseModel): is_favorite: bool | None = None tags: str | None = None notes: str | None = None - cost: float | None = None + # Added to every total: a negative (or infinite) cost would wreck them + cost: Annotated[float | None, Field(ge=0, le=1_000_000)] = None failure_reason: str | None = None # Number of items printed. 0 is a legal answer -- a plate that jammed and # came off ruined produced nothing, and the project's completed-items count diff --git a/backend/app/services/print_log.py b/backend/app/services/print_log.py index 53af32ce6..fd52fc193 100644 --- a/backend/app/services/print_log.py +++ b/backend/app/services/print_log.py @@ -111,6 +111,21 @@ async def write_log_entry( return entry +async def mirror_archive_cost(db: AsyncSession, archive, previous_cost: float | None) -> None: + """Give the archive's newest run the archive's new cost, if it had the old one. + + Statistics, project totals and batch sums read a run's cost. A completed + run without its own spool price took the archive's (or none), so a new + archive cost belongs to it too. A run its spools priced keeps that price, + and a failed or cancelled run keeps its own: it printed only part of it. + """ + entry = await db.scalar( + select(PrintLogEntry).where(PrintLogEntry.archive_id == archive.id).order_by(PrintLogEntry.id.desc()).limit(1) + ) + if entry is not None and entry.status == "completed" and entry.cost in (None, previous_cost): + entry.cost = archive.cost + + async def other_runs_of(db: AsyncSession, archive) -> int: """How many log entries *archive* has from runs other than its current one. diff --git a/backend/tests/integration/test_archive_edit_cost_and_reprint.py b/backend/tests/integration/test_archive_edit_cost_and_reprint.py new file mode 100644 index 000000000..dbc63ac6b --- /dev/null +++ b/backend/tests/integration/test_archive_edit_cost_and_reprint.py @@ -0,0 +1,123 @@ +"""Editing an archive: its cost reaches the run, a reprint's run is not touched. + +* A hand-set cost was saved on the archive only. Statistics, project totals + and batch sums read the run's log entry, so they kept the old figure (COST-5). + Like the grams, it reaches the run that inherited it, never a run that + measured its own. +* While the archive is being reprinted, its newest log entry is the previous + run's; an edit then rewrote that run's outcome (FG/I9). +* A negative cost was accepted and subtracted from every total (COST-6). +""" + +import pytest +from sqlalchemy import select + +from backend.app.models.print_log import PrintLogEntry + +pytestmark = [pytest.mark.asyncio, pytest.mark.integration] + + +async def _entry(db_session, archive_id): + query = select(PrintLogEntry).where(PrintLogEntry.archive_id == archive_id) + return (await db_session.scalars(query.execution_options(populate_existing=True))).one() + + +class TestAHandSetCost: + async def test_reaches_the_run_that_inherited_it(self, async_client, archive_factory, printer_factory, db_session): + printer = await printer_factory() + archive = await archive_factory(printer.id, cost=1.50) + + response = await async_client.patch(f"/api/v1/archives/{archive.id}", json={"cost": 5.00}) + + assert response.status_code == 200, response.text + assert (await _entry(db_session, archive.id)).cost == 5.00 + + async def test_leaves_a_run_that_measured_its_own(self, async_client, archive_factory, printer_factory, db_session): + printer = await printer_factory() + archive = await archive_factory(printer.id, cost=1.50) + entry = await _entry(db_session, archive.id) + entry.cost = 1.23 # the tracked spools' price for this run + await db_session.commit() + + await async_client.patch(f"/api/v1/archives/{archive.id}", json={"cost": 5.00}) + + assert (await _entry(db_session, archive.id)).cost == 1.23 + + async def test_a_negative_cost_is_refused(self, async_client, archive_factory, printer_factory, db_session): + printer = await printer_factory() + archive = await archive_factory(printer.id, cost=1.50) + + response = await async_client.patch(f"/api/v1/archives/{archive.id}", json={"cost": -100}) + + assert response.status_code == 422 + assert (await _entry(db_session, archive.id)).cost == 1.50 + + +class TestAnEditDuringAReprint: + async def test_leaves_the_previous_run_alone(self, async_client, archive_factory, printer_factory, db_session): + from backend.app.models.archive import PrintArchive + + printer = await printer_factory() + archive = await archive_factory(printer.id, status="completed", filament_used_grams=10.0, cost=1.50) + # The reprint is running: the archive row is its, the entry the last run's + row = await db_session.get(PrintArchive, archive.id) + row.status = "printing" + await db_session.commit() + + response = await async_client.patch( + f"/api/v1/archives/{archive.id}", + json={"failure_reason": "spaghetti", "user_verdict": "reject", "filament_used_grams": 12.0, "cost": 9.0}, + ) + + assert response.status_code == 200, response.text + entry = await _entry(db_session, archive.id) + assert (entry.status, entry.failure_reason, entry.user_verdict, entry.filament_used_grams, entry.cost) == ( + "completed", + None, + None, + 10.0, + 1.50, + ) + + +class TestWhichRunsTakeIt: + async def test_not_a_failed_run_that_printed_only_part( + self, async_client, archive_factory, printer_factory, db_session + ): + """A failed run without tracked spools has no cost on purpose.""" + printer = await printer_factory() + archive = await archive_factory(printer.id, cost=1.50, status="failed") + entry = await _entry(db_session, archive.id) + entry.cost = None + await db_session.commit() + + await async_client.patch(f"/api/v1/archives/{archive.id}", json={"cost": 5.00}) + + assert (await _entry(db_session, archive.id)).cost is None + + async def test_after_recalculate_costs_too(self, async_client, archive_factory, printer_factory, db_session): + """Recalculate gives the run the new figure as well, so a later hand + edit still finds the run carrying the archive's cost.""" + printer = await printer_factory() + archive = await archive_factory(printer.id, cost=1.50, filament_used_grams=100.0, filament_type="PLA") + + recalculated = await async_client.post("/api/v1/archives/recalculate-costs") + assert recalculated.json()["updated"] == 1 + recalculated_cost = (await _entry(db_session, archive.id)).cost + assert recalculated_cost not in (None, 1.50) + + await async_client.patch(f"/api/v1/archives/{archive.id}", json={"cost": 2.00}) + + assert (await _entry(db_session, archive.id)).cost == 2.00 + + async def test_an_infinite_cost_is_refused(self, async_client, archive_factory, printer_factory): + printer = await printer_factory() + archive = await archive_factory(printer.id, cost=1.50) + + response = await async_client.patch( + f"/api/v1/archives/{archive.id}", + content=b'{"cost": Infinity}', + headers={"Content-Type": "application/json"}, + ) + + assert response.status_code == 422