Kaynağa Gözat

fix(archives): archives a spool was used for can be deleted on PostgreSQL

spool_usage_history.archive_id referenced the archive without a delete rule,
so on PostgreSQL every hard delete of an archive with spool usage failed on
the foreign key with a server error: deleting with purge from statistics,
the automatic purge with that option, and deleting a printer with its
archives. The usage record is the spool's consumption log and now outlives
the archive: ON DELETE SET NULL in the model, a PostgreSQL migration, and
the SQLite rule list, whose start-up sweep also clears links left dangling
by earlier deletes.

Recalculate costs no longer charges usage without an archive link to
archives by print name. With deleted archives' usage now unlinked, that
fallback would have charged one print's spool cost to every archive of the
same name; such archives get the filament-price estimate instead.
maziggy 1 hafta önce
ebeveyn
işleme
9613d65e0d

+ 1 - 0
CHANGELOG.md

@@ -162,6 +162,7 @@ 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
+- **Archives a spool was used for can be deleted on PostgreSQL** — With PostgreSQL, deleting an archive for good (deleting it with **purge from statistics**, the automatic archive purge with that option, or deleting a printer together with its archives) failed with a server error when a spool from the inventory had been used for that print. The spool's usage record now stays and only loses its link to the deleted archive, on both databases; on SQLite, usage records still pointing at an archive deleted earlier are cleared of that link at the next start. **Recalculate costs** no longer charges usage records without an archive link to archives by their print name: that added the spool cost of a deleted print, of a print made with archiving off, or of every earlier run to each archive with the same name. Archives without linked usage get the estimate from the filament price instead, as they already did when no such record existed (usage recorded before 0.2.1, or with archiving off).
 - **An id of 0 no longer slips past the checks when queueing, batching or creating a project** — Adding to the queue with an archive id of `0` next to a library file, creating a batch with an archive or library file id of `0`, and creating a project with parent `0` skipped the check that the archive, file or project exists and saved `0` as its id: a server error on PostgreSQL, a link to nothing on SQLite. `0` now means "none" there, as it already did when editing a project. Creating a batch also checks that its archive or library file exists, and refuses an unknown one.
 - **Archiving a virtual-printer upload into a project that does not exist is refused up front** — The project chosen when archiving a pending upload was saved without checking it. On PostgreSQL an unknown project failed with a server error after the file had already been archived; on SQLite the archive was saved with a broken project link. The project is now checked first and an unknown one refused with "Project not found", before anything is archived.
 - **A project's bill of materials no longer shows archives from printers outside the user's groups** — A BOM line can be linked to an archive through the API, and the BOM answered with that archive's name whichever printer it came from. A member of a group limited to some printers could read the name of any archive by linking a line to it, and everyone reading the project saw the names of archives from printers they have no access to. A link to an archive that does not exist was saved too (a server error on PostgreSQL). A BOM line now links only to an archive the user can see, and shows a linked archive only to users who can see it.

+ 5 - 11
backend/app/api/routes/archives.py

@@ -2181,17 +2181,11 @@ async def recalculate_all_costs(
                 total_cost += (untracked_grams / 1000.0) * default_cost_per_kg
             new_cost = round(total_cost, 2)
         else:
-            # Fallback: sum costs for old records by print_name
-            usage_result = await db.execute(
-                select(func.sum(SpoolUsageHistory.cost)).where(
-                    SpoolUsageHistory.print_name == archive.print_name,
-                    SpoolUsageHistory.archive_id.is_(None),
-                )
-            )
-            fallback_cost = usage_result.scalar()
-            if fallback_cost is not None and fallback_cost > 0:
-                new_cost = round(fallback_cost, 2)
-            elif spoolman_owns and archive.cost is not None:
+            # No usage linked to this archive. Usage without an archive link
+            # (recorded before usage was linked, with archiving off, or of a
+            # deleted archive) is not matched by print name: that charged one
+            # print's spool cost to every archive of the same name.
+            if spoolman_owns and archive.cost is not None:
                 # Priced from the linked Spoolman spools at completion; there is
                 # nothing better to recompute it from here (#2591).
                 new_cost = None

+ 12 - 0
backend/app/core/database.py

@@ -5803,6 +5803,7 @@ async def _run_migrations(conn):
     await _migrate_add_missing_set_null_fks(conn)
     await _migrate_wallet_transactions_keep_deleted_users(conn)
     await _migrate_notification_providers_go_with_their_printer(conn)
+    await _migrate_spool_usage_outlives_its_archive(conn)
     await _migrate_pg_column_types(conn)
     await _migrate_pg_columns_to_model(conn)
 
@@ -6056,6 +6057,17 @@ async def _migrate_notification_providers_go_with_their_printer(conn) -> None:
         await _pg_set_fk_ondelete(conn, "smart_plugs", "printer_id", "printers", "CASCADE")
 
 
+async def _migrate_spool_usage_outlives_its_archive(conn) -> None:
+    """spool_usage_history.archive_id: ON DELETE SET NULL, as the model says (PostgreSQL).
+
+    The key had no rule, so deleting an archive a spool was used for (hard
+    delete, or a printer delete with its archives) failed on the key. SQLite
+    doesn't enforce it; core/sqlite_fk applies the same rule there.
+    """
+    if not is_sqlite():
+        await _pg_set_fk_ondelete(conn, "spool_usage_history", "archive_id", "print_archives", "SET NULL")
+
+
 _PG_ON_DELETE_CODES = {"CASCADE": "c", "SET NULL": "n"}
 
 

+ 1 - 0
backend/app/core/sqlite_fk.py

@@ -50,6 +50,7 @@ APPLIED: frozenset[tuple[str, str]] = frozenset(
         ("print_log_entries", "archive_id"),
         ("print_queue", "archive_id"),
         ("project_bom_items", "archive_id"),
+        ("spool_usage_history", "archive_id"),
         ("telegram_pending_verdicts", "archive_id"),
         ("wallet_transactions", "print_archive_id"),
         # library_files. print_queue.library_file_id is CASCADE in the model, but

+ 3 - 1
backend/app/models/spool_usage_history.py

@@ -15,7 +15,9 @@ class SpoolUsageHistory(Base):
     spool_id: Mapped[int] = mapped_column(ForeignKey("spool.id", ondelete="CASCADE"))
     printer_id: Mapped[int | None] = mapped_column(ForeignKey("printers.id", ondelete="SET NULL"))
     print_name: Mapped[str | None] = mapped_column(String(500))
-    archive_id: Mapped[int | None] = mapped_column(ForeignKey("print_archives.id"), nullable=True)
+    # The spool's consumption outlives the archive: deleting the archive only
+    # drops the link.
+    archive_id: Mapped[int | None] = mapped_column(ForeignKey("print_archives.id", ondelete="SET NULL"), nullable=True)
     weight_used: Mapped[float] = mapped_column(Float, default=0)
     percent_used: Mapped[int] = mapped_column(Integer, default=0)
     status: Mapped[str] = mapped_column(String(20), default="completed")  # completed/failed/aborted

+ 9 - 3
backend/tests/integration/test_cost_statistics.py

@@ -324,7 +324,12 @@ class TestCostCalculationScenarios:
     async def test_archive_cost_with_archive_id_and_print_name(
         self, async_client, archive_factory, printer_factory, db_session
     ):
-        """Test archive cost recalculation using both archive_id and print_name fallback."""
+        """Recalculation uses usage linked by archive_id; unlinked usage is not matched by name.
+
+        Matching unlinked usage by print name charged a deleted print's (or every
+        earlier run's) spool cost to each archive of that name, so an archive
+        without linked usage gets the estimate from the filament price.
+        """
         from backend.app.models.spool import Spool
         from backend.app.models.spool_usage_history import SpoolUsageHistory
 
@@ -408,9 +413,10 @@ class TestCostCalculationScenarios:
         assert response_new.status_code == 200
         assert response_new.json()["cost"] == 0.50
 
-        # Verify archive_old cost from legacy print_name fallback
+        # archive_old: the unlinked 0.45 is not used; 30 g of PLA at the default
+        # 25.00 per kg instead.
         response_old = await async_client.get(f"/api/v1/archives/{archive_old.id}")
         assert response_old.status_code == 200
-        assert response_old.json()["cost"] == 0.45
+        assert response_old.json()["cost"] == 0.75
 
         await db_session.rollback()

+ 102 - 0
backend/tests/integration/test_spool_usage_outlives_archive.py

@@ -0,0 +1,102 @@
+"""Deleting an archive a spool was used for keeps the spool's usage record.
+
+The usage row pointed at the archive with no delete rule: on PostgreSQL every
+hard delete of such an archive (purge from statistics, or a printer delete
+with its archives) failed on the foreign key with a server error. The row is
+the spool's consumption record, so it stays and only loses the link.
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import select
+
+from backend.app.models.spool import Spool
+from backend.app.models.spool_usage_history import SpoolUsageHistory
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+
+async def _used_archive(db_session, archive_factory, printer_id: int) -> tuple[int, int]:
+    archive = await archive_factory(printer_id)
+    spool = Spool(material="PLA")
+    db_session.add(spool)
+    await db_session.flush()
+    usage = SpoolUsageHistory(spool_id=spool.id, printer_id=printer_id, archive_id=archive.id, weight_used=10, cost=0.5)
+    db_session.add(usage)
+    await db_session.commit()
+    return archive.id, usage.id
+
+
+async def _usage(db_session, usage_id: int):
+    db_session.expire_all()
+    return (
+        await db_session.execute(
+            select(SpoolUsageHistory.archive_id, SpoolUsageHistory.weight_used).where(SpoolUsageHistory.id == usage_id)
+        )
+    ).one_or_none()
+
+
+async def test_purging_an_archive_keeps_the_spool_usage(
+    async_client: AsyncClient, db_session, printer_factory, archive_factory
+):
+    printer = await printer_factory()
+    archive_id, usage_id = await _used_archive(db_session, archive_factory, printer.id)
+
+    response = await async_client.delete(f"/api/v1/archives/{archive_id}?purge_stats=true")
+
+    assert response.status_code == 200, response.text
+    assert tuple(await _usage(db_session, usage_id)) == (None, 10)
+
+
+async def test_deleting_a_printer_with_its_archives_keeps_the_spool_usage(
+    async_client: AsyncClient, db_session, printer_factory, archive_factory
+):
+    printer = await printer_factory()
+    _, usage_id = await _used_archive(db_session, archive_factory, printer.id)
+
+    response = await async_client.delete(f"/api/v1/printers/{printer.id}?delete_archives=true")
+
+    assert response.status_code == 200, response.text
+    row = await _usage(db_session, usage_id)
+    assert row is not None and row[0] is None
+
+
+async def _unlinked_usage(db_session, print_name: str, cost: float, created_at) -> None:
+    spool = Spool(material="PLA")
+    db_session.add(spool)
+    await db_session.flush()
+    db_session.add(
+        SpoolUsageHistory(
+            spool_id=spool.id, print_name=print_name, archive_id=None, weight_used=10, cost=cost, created_at=created_at
+        )
+    )
+    await db_session.commit()
+
+
+async def test_recalculating_costs_does_not_match_unlinked_usage_by_name(
+    async_client: AsyncClient, db_session, printer_factory, archive_factory
+):
+    # A deleted archive's usage keeps its print name but loses its link; usage
+    # without a link is not charged to an archive by name, so the archive gets
+    # its estimate from the catalog price.
+    from datetime import datetime, timedelta
+
+    from backend.app.models.archive import PrintArchive
+
+    printer = await printer_factory()
+    printed = datetime(2026, 9, 1, 12, 0)
+    archive = await archive_factory(
+        printer.id, print_name="Benchy", created_at=printed, filament_used_grams=100, filament_type="PLA"
+    )
+    archive_id = archive.id
+    await _unlinked_usage(db_session, "Benchy", 9.99, printed + timedelta(hours=1))
+
+    response = await async_client.post("/api/v1/archives/recalculate-costs")
+
+    assert response.status_code == 200, response.text
+    db_session.expire_all()
+    cost = (await db_session.execute(select(PrintArchive.cost).where(PrintArchive.id == archive_id))).scalar()
+    # 100 g at the default 25.00 per kg, not the unlinked 9.99.
+    assert cost == 2.5