ソースを参照

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

maziggy 3 日 前
親
コミット
461c3f045d

+ 3 - 0
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.

+ 16 - 1
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()

+ 25 - 1
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
 # =============================================================================

+ 2 - 1
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

+ 15 - 0
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.
 

+ 123 - 0
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