Explorar o código

fix(projects): refuse an unknown priority on create, keep imported statuses valid, repair stored ones

Creating a project stored any priority string, and the JSON and file imports
stored any status. The project editor sends both back on every save and
refuses such values, so a project holding one could not be edited at all,
not even its name. A value longer than 20 characters failed with a server
error on PostgreSQL.

Create now answers 400 "Invalid priority" for a priority outside
low/normal/high/urgent, as PATCH already did. Both imports keep a status of
active/completed/archived and fall back to "active", as they already did for
the priority. PROJECT_PRIORITIES and PROJECT_STATUSES now live in
models/project.py.

A startup repair gives existing projects with an unknown or missing priority
"normal" and an unknown status "active". It is not gated behind a flag: it
only matches values nothing accepts, so a second run changes nothing, and a
restored older database is repaired on its next start.

------------------

fix(smart-plugs): check printer id 0 like any other printer id

Smart plug create and edit, and virtual printer create, tested the printer id
for truthiness, so 0 skipped the "Printer not found" check. On PostgreSQL
the foreign key then failed with a server error; on SQLite the plug or
virtual printer was saved linked to a printer that does not exist.

The checks now use "is not None", as queue create and virtual printer update
already did, so 0 is refused with "Printer not found".
maziggy hai 1 semana
pai
achega
26a8a09937

+ 2 - 1
CHANGELOG.md

@@ -162,10 +162,11 @@ 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
+- **A smart plug or virtual printer can no longer be linked to printer 0** — Adding or editing a smart plug and adding a virtual printer skipped the "Printer not found" check when the printer id was `0`. On PostgreSQL the save then failed with a server error; on SQLite the plug or virtual printer was saved linked to a printer that does not exist. Such a request is now refused with "Printer not found", as for any other unknown printer.
 - **No more firmware-check errors in the log while the internet is down** — Without an internet connection, every firmware check of every printer logged a full error for each Bambu Lab page it tried (measured: 12 errors and 9 warnings for three printers checked three times). The outage is now logged once per Bambu Lab site as a warning, the checks keep retrying quietly, and a line in the log says when the site is reachable again.
 - **Bambu Cloud being unreachable is reported as such** — When Bambu Cloud failed (no internet, a blocked or failing connection, an error or an error page such as a Cloudflare challenge), the cloud preset, device and firmware views answered with a server error (500), and pasting an access token answered as if the token were wrong (401). Signing in or entering the email code without a connection to Bambu Cloud also answered 401, as if the password or code were wrong. These now answer 502 (Bambu Cloud unreachable) with the cause, as the preset lookups already did; a pasted token counts as invalid only when Bambu Cloud itself refuses it. An expired session is answered as such (401, sign in again) instead of with a server error.
 - **Editing a smart plug can no longer save a plug that cannot be read back** — Adding a plug checks its values: a Tasmota plug needs an IPv4 address, a Home Assistant plug an entity of the right kind, an MQTT plug a topic, a REST plug an ON or OFF URL, and the name must not be empty. Editing one did not, so clearing such a field, switching the type without filling in the new one, or typing a hostname as a Tasmota address saved a plug that the smart plug list, the plug itself and the edit all failed on with a server error until it was fixed in the database. Such an edit is now refused with the same message as when adding a plug, and the plug stays as it was. Switching a plug to Tasmota on a printer that already has a Tasmota plug is refused as well (adding one already was); before, it gave the printer a second one, and adding a plug to that printer then failed with a server error.
-- **Importing a project from a file without a priority** — The import gave such a project the priority `0`. On PostgreSQL the import failed with a server error; on SQLite the project got a priority the project editor itself refuses. An imported project now keeps a valid priority (low, normal, high or urgent) and gets **normal** otherwise, from a file and from the JSON import alike.
+- **Creating or importing a project gives it a priority and status the project editor accepts** — Importing a project from a file without a priority gave it the priority `0`: on PostgreSQL the import failed with a server error, on SQLite the project got a priority the project editor itself refuses. Creating a project through the API stored any priority, and importing one any status, the same way (a value longer than 20 characters failed with a server error on PostgreSQL). An imported project now keeps a valid priority (low, normal, high or urgent) and status (active, completed or archived) and gets **normal** and **active** otherwise, from a file and from the JSON import alike. Creating a project with an unknown priority is refused with "Invalid priority", as editing one already was. Such projects could not be edited at all (every save, even of the name alone, was refused as an invalid priority or status); existing ones get **normal** and **active** at the next start.
 - **The "Password Reset" email template is used again** — Since password resets send a link instead of a new password, the reset email (self-service and admin) was always the built-in one: editing the "Password Reset" template under Notifications changed nothing, and its default text still promised a new password. The reset email is now built from that template, and Bambuddy adds the reset link at the end of it (and as a button); the template itself cannot place or contain the link. `{password}` and `{login_url}` are gone from this template. The default text is updated on upgrade unless you edited it; if you did, reword it for a link instead of a new password.
 - **A cost centre is charged for a print whose user was deleted while it ran** — When a user was deleted during their print, the print was never charged: the cost centre's ledger missed its cost, and its budget reservation stayed active for good, holding back that much of the budget. The print is now charged to its cost centre, also to the deleted user's own private one, which their personal prints go through, and its reservation is settled. A queue print is charged under the user who queued it, never under the owner of the archive it reprints. Ledger entries without a user now read "No user" instead of "Deleted user": with login turned off, prints on a cost centre are now charged too and never had a user. Deleting a user together with their items now also releases the budget their queued prints still held; a print of theirs still running is kept until it ends (without a user) and billed to its cost centre like any other. Only a print with no cost centre at all (started while billing was off) has nobody left to charge.
 - **Deleting a printer no longer deletes an archive of a print starting at that moment (PostgreSQL)** — If a queue or reprint print started on the printer while it was being deleted, the delete could take the print's archive with it, even with "keep archives" and even when the archive belonged to another printer or a virtual printer queue. The delete now locks the printer while it runs, so such a print start waits and is turned away, and its archive stays as it was; this is logged as a print start for a deleted printer, not as an error, and sends no start notification. An archive that could not be written no longer leaves its copied 3MF behind in the archive folder.

+ 11 - 7
backend/app/api/routes/projects.py

@@ -25,7 +25,7 @@ from backend.app.models.archive import PrintArchive
 from backend.app.models.library import LibraryFile, LibraryFolder
 from backend.app.models.print_log import PrintLogEntry
 from backend.app.models.print_queue import PrintQueueItem, PrintQueueVariant
-from backend.app.models.project import Project
+from backend.app.models.project import PROJECT_PRIORITIES, PROJECT_STATUSES, Project
 from backend.app.models.project_bom import ProjectBOMItem
 from backend.app.models.user import User
 from backend.app.schemas.project import (
@@ -52,15 +52,17 @@ logger = logging.getLogger(__name__)
 
 router = APIRouter(prefix="/projects", tags=["projects"])
 
-# The priorities the project editor accepts.
-PROJECT_PRIORITIES = ("low", "normal", "high", "urgent")
-
 
 def _imported_priority(value: object) -> str:
     """An imported project's priority: one the editor accepts, else "normal"."""
     return value if isinstance(value, str) and value in PROJECT_PRIORITIES else "normal"
 
 
+def _imported_status(value: object) -> str:
+    """An imported project's status: one the editor accepts, else "active"."""
+    return value if isinstance(value, str) and value in PROJECT_STATUSES else "active"
+
+
 _FAILURE_STATUSES = ("failed", "aborted", "cancelled", "stopped")
 
 # A completed run whose user verdict is 'reject' (#1898) finished on the
@@ -544,6 +546,8 @@ async def create_project(
     _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_CREATE),
 ):
     """Create a new project."""
+    if data.priority not in PROJECT_PRIORITIES:
+        raise HTTPException(status_code=400, detail="Invalid priority")
     # Verify parent exists if specified
     parent_name = None
     if data.parent_id:
@@ -814,7 +818,7 @@ async def update_project(
     if data.color is not None:
         project.color = data.color
     if data.status is not None:
-        if data.status not in ["active", "completed", "archived"]:
+        if data.status not in PROJECT_STATUSES:
             raise HTTPException(status_code=400, detail="Invalid status")
         project.status = data.status
     if data.target_count is not None:
@@ -2072,7 +2076,7 @@ async def import_project(
         name=data.name,
         description=data.description,
         color=data.color,
-        status=data.status,
+        status=_imported_status(data.status),
         target_count=data.target_count,
         target_parts_count=data.target_parts_count,
         target_sets=data.target_sets,
@@ -2202,7 +2206,7 @@ async def import_project_file(
         name=data.get("name", "Imported Project"),
         description=data.get("description"),
         color=data.get("color"),
-        status=data.get("status", "active"),
+        status=_imported_status(data.get("status")),
         target_count=data.get("target_count"),
         target_parts_count=data.get("target_parts_count"),
         target_sets=data.get("target_sets"),

+ 3 - 3
backend/app/api/routes/smart_plugs.py

@@ -120,8 +120,8 @@ async def create_smart_plug(
         reject_placeholder({field: getattr(data, field) for field in SMART_PLUG_SECRETS})
     except SecretConfigError as exc:
         raise HTTPException(400, str(exc)) from None
-    # Validate printer_id if provided
-    if data.printer_id:
+    # Validate printer_id if provided (0 included: it names no printer either)
+    if data.printer_id is not None:
         printer_scope.ensure(data.printer_id)
         result = await db.execute(select(Printer).where(Printer.id == data.printer_id))
         if not result.scalar_one_or_none():
@@ -618,7 +618,7 @@ async def update_smart_plug(
         raise HTTPException(400, str(exc)) from None
 
     # Validate new printer_id if being changed
-    if "printer_id" in update_data and update_data["printer_id"]:
+    if "printer_id" in update_data and update_data["printer_id"] is not None:
         new_printer_id = update_data["printer_id"]
         printer_scope.ensure(new_printer_id)
 

+ 1 - 1
backend/app/api/routes/virtual_printers.py

@@ -193,7 +193,7 @@ async def create_virtual_printer(
 
     # Validate proxy target printer exists
     target_printer = None
-    if body.target_printer_id:
+    if body.target_printer_id is not None:
         from backend.app.models.printer import Printer
 
         result = await db.execute(select(Printer).where(Printer.id == body.target_printer_id))

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

@@ -2191,6 +2191,49 @@ async def _migrate_failure_reason_vocabulary(conn):
         logger.info("[#2974] converted %d failure_reason value(s) to the canonical vocabulary", total)
 
 
+async def _migrate_project_priority_status(conn) -> None:
+    """Give projects stored with an unknown priority or status a valid one.
+
+    Older builds stored what they were given: a file import without a priority
+    wrote ``0`` (SQLite), create accepted any priority and both imports any
+    status. The project editor sends both back on every save and refuses such
+    values, so the project could no longer be edited at all. An unknown
+    priority becomes ``normal``, an unknown status ``active`` -- the same
+    fallbacks the imports use now.
+
+    Not gated behind a flag, like the failure_reason vocabulary: it only matches
+    values nothing accepts, so a second run changes nothing, and a restored
+    older database is repaired on its next start.
+    """
+    from sqlalchemy import bindparam, text
+
+    from backend.app.models.project import PROJECT_PRIORITIES, PROJECT_STATUSES
+
+    repaired: dict[str, int] = {}
+    async with conn.begin_nested():
+        # nosec B608 -- `column` and `fallback` come from the literal tuple below;
+        # the accepted values are bound parameters.
+        for column, accepted, fallback in (
+            ("priority", PROJECT_PRIORITIES, "normal"),
+            ("status", PROJECT_STATUSES, "active"),
+        ):
+            result = await conn.execute(
+                text(
+                    f"UPDATE projects SET {column} = :fallback "  # noqa: S608  # nosec B608
+                    f"WHERE {column} IS NULL OR {column} NOT IN :accepted"
+                ).bindparams(bindparam("fallback"), bindparam("accepted", expanding=True)),
+                {"fallback": fallback, "accepted": list(accepted)},
+            )
+            repaired[column] = result.rowcount or 0
+
+    if any(repaired.values()):
+        logger.info(
+            "Repaired projects: %d unknown priority -> normal, %d unknown status -> active",
+            repaired["priority"],
+            repaired["status"],
+        )
+
+
 def _pg_ddl_if_not_exists(conn, cursor, statement, parameters, context, executemany):
     """before_cursor_execute hook: the _pg_if_not_exists rewrite for every statement.
 
@@ -2573,6 +2616,10 @@ async def _run_migrations(conn):
     except (OperationalError, ProgrammingError):
         pass  # Already applied
 
+    # Data migration: unknown project priority/status -> the editor's fallbacks.
+    # After the priority column exists (above).
+    await _migrate_project_priority_status(conn)
+
     # Migration: Rename quantity_printed to quantity_acquired in project_bom_items
     await _safe_execute(conn, "ALTER TABLE project_bom_items RENAME COLUMN quantity_printed TO quantity_acquired")
 

+ 4 - 0
backend/app/models/project.py

@@ -5,6 +5,10 @@ from sqlalchemy.orm import Mapped, mapped_column, relationship
 
 from backend.app.core.database import Base
 
+# The priorities and statuses the project editor accepts.
+PROJECT_PRIORITIES = ("low", "normal", "high", "urgent")
+PROJECT_STATUSES = ("active", "completed", "archived")
+
 
 class Project(Base):
     """Project to group related prints (e.g., 'Voron Build' with multiple parts)."""

+ 49 - 0
backend/tests/integration/test_printer_id_zero.py

@@ -0,0 +1,49 @@
+"""A printer id of 0 is checked like any other printer id.
+
+Smart plug create and edit and virtual printer create tested the id for
+truthiness, so 0 skipped the "Printer not found" check: PostgreSQL then failed
+on the foreign key with a server error, SQLite stored a plug or virtual
+printer pointing at a printer that does not exist.
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import AsyncClient
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+
+async def test_creating_a_plug_for_printer_0_is_refused(async_client: AsyncClient):
+    response = await async_client.post(
+        "/api/v1/smart-plugs/", json={"name": "Plug", "ip_address": "192.168.1.100", "printer_id": 0}
+    )
+
+    assert response.status_code == 400, response.text
+    assert response.json()["detail"] == "Printer not found"
+    assert (await async_client.get("/api/v1/smart-plugs/")).json() == []
+
+
+async def test_moving_a_plug_to_printer_0_is_refused(async_client: AsyncClient):
+    created = await async_client.post("/api/v1/smart-plugs/", json={"name": "Plug", "ip_address": "192.168.1.100"})
+    assert created.status_code == 200, created.text
+    plug_id = created.json()["id"]
+
+    response = await async_client.patch(f"/api/v1/smart-plugs/{plug_id}", json={"printer_id": 0})
+
+    assert response.status_code == 400, response.text
+    assert response.json()["detail"] == "Printer not found"
+    assert (await async_client.get(f"/api/v1/smart-plugs/{plug_id}")).json()["printer_id"] is None
+
+
+async def test_creating_a_virtual_printer_for_printer_0_is_refused(async_client: AsyncClient):
+    response = await async_client.post(
+        "/api/v1/virtual-printers",
+        json={"name": "VP", "mode": "queue", "access_code": "12345678", "target_printer_id": 0},
+    )
+
+    assert response.status_code == 400, response.text
+    assert response.json()["detail"] == "Printer with ID 0 not found"
+    listed = (await async_client.get("/api/v1/virtual-printers")).json()
+    printers = listed if isinstance(listed, list) else listed.get("printers", [])
+    assert printers == []

+ 49 - 3
backend/tests/integration/test_project_import_priority.py

@@ -1,7 +1,9 @@
-"""An imported project gets a priority the project editor accepts.
+"""A new project gets a priority and status the project editor accepts.
 
-A file without one used to get the number 0: refused by PostgreSQL (a text
-column) and stored as a priority the editor itself refuses on SQLite.
+A file without a priority used to get the number 0: refused by PostgreSQL (a
+text column) and stored as a priority the editor itself refuses on SQLite.
+Creating a project stored any priority, and importing one any status (an
+over-long value failed on PostgreSQL, any other one the editor refuses).
 """
 
 from __future__ import annotations
@@ -39,3 +41,47 @@ async def test_a_json_import_gets_a_valid_priority(async_client: AsyncClient, gi
 
     assert response.status_code == 200, response.text
     assert response.json()["priority"] == expected
+
+
+@pytest.mark.parametrize(
+    ("given", "expected"),
+    [(None, "active"), ("completed", "completed"), ("archived", "archived"), ("done", "active"), (0, "active")],
+)
+async def test_a_file_import_gets_a_valid_status(async_client: AsyncClient, given, expected):
+    response = await _import_file(async_client, {"name": "Imported", "status": given})
+
+    assert response.status_code == 200, response.text
+    assert response.json()["status"] == expected
+
+
+async def test_a_file_import_without_a_status_is_active(async_client: AsyncClient):
+    response = await _import_file(async_client, {"name": "Imported"})
+
+    assert response.status_code == 200, response.text
+    assert response.json()["status"] == "active"
+
+
+@pytest.mark.parametrize(("given", "expected"), [("archived", "archived"), ("x" * 40, "active")])
+async def test_a_json_import_gets_a_valid_status(async_client: AsyncClient, given, expected):
+    response = await async_client.post("/api/v1/projects/import", json={"name": "Imported", "status": given})
+
+    assert response.status_code == 200, response.text
+    assert response.json()["status"] == expected
+
+
+@pytest.mark.parametrize("priority", ["whenever", "x" * 40, ""])
+async def test_creating_a_project_refuses_an_unknown_priority(async_client: AsyncClient, priority):
+    response = await async_client.post("/api/v1/projects/", json={"name": "New", "priority": priority})
+
+    assert response.status_code == 400, response.text
+    assert response.json()["detail"] == "Invalid priority"
+    listed = await async_client.get("/api/v1/projects/")
+    assert [p["name"] for p in listed.json()] == []
+
+
+@pytest.mark.parametrize("priority", ["low", "normal", "high", "urgent"])
+async def test_creating_a_project_keeps_a_known_priority(async_client: AsyncClient, priority):
+    response = await async_client.post("/api/v1/projects/", json={"name": "New", "priority": priority})
+
+    assert response.status_code == 200, response.text
+    assert response.json()["priority"] == priority

+ 90 - 0
backend/tests/integration/test_project_priority_status_repair.py

@@ -0,0 +1,90 @@
+"""Projects stored with an unknown priority or status get a valid one at start.
+
+Older builds stored a file-imported project without a priority as ``0`` and
+took any priority on create and any status on import. The project editor sends
+both back on every save and refused them, so such a project could not be
+edited at all, not even its name.
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import select, text
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from backend.app.core.database import _migrate_project_priority_status
+from backend.app.models.project import Project
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+ROWS = [
+    # (priority, status) as stored -> as repaired
+    (("0", "active"), ("normal", "active")),
+    (("whenever", "done"), ("normal", "active")),
+    (("urgent", "archived"), ("urgent", "archived")),
+    (("low", "completed"), ("low", "completed")),
+]
+
+
+async def _seed(session: AsyncSession) -> list[int]:
+    ids = []
+    for (priority, status), _ in ROWS:
+        project = Project(name=f"{priority}/{status}")
+        session.add(project)
+        await session.flush()
+        # Raw UPDATE: the API no longer stores such values.
+        await session.execute(
+            text("UPDATE projects SET priority = :p, status = :s WHERE id = :id"),
+            {"p": priority, "s": status, "id": project.id},
+        )
+        ids.append(project.id)
+    await session.commit()
+    return ids
+
+
+async def _repair(session: AsyncSession) -> None:
+    await _migrate_project_priority_status(await session.connection())
+    await session.commit()
+
+
+async def _stored(session: AsyncSession, ids: list[int]) -> list[tuple[str, str]]:
+    session.expire_all()
+    rows = (
+        await session.execute(select(Project.id, Project.priority, Project.status).where(Project.id.in_(ids)))
+    ).all()
+    by_id = {row[0]: (row[1], row[2]) for row in rows}
+    return [by_id[i] for i in ids]
+
+
+async def test_unknown_values_are_repaired_and_valid_ones_kept(db_session: AsyncSession):
+    ids = await _seed(db_session)
+
+    await _repair(db_session)
+
+    assert await _stored(db_session, ids) == [expected for _, expected in ROWS]
+
+
+async def test_a_second_run_changes_nothing(db_session: AsyncSession):
+    ids = await _seed(db_session)
+    await _repair(db_session)
+
+    await _repair(db_session)
+
+    assert await _stored(db_session, ids) == [expected for _, expected in ROWS]
+
+
+async def test_a_repaired_project_can_be_edited_again(async_client: AsyncClient, db_session: AsyncSession):
+    project_id = (await _seed(db_session))[0]
+    project = (await async_client.get(f"/api/v1/projects/{project_id}")).json()
+    # What the editor sends back on save: the stored priority and status.
+    edit = {"name": "Renamed", "priority": project["priority"], "status": project["status"]}
+    assert (await async_client.patch(f"/api/v1/projects/{project_id}", json=edit)).status_code == 400
+
+    await _repair(db_session)
+    project = (await async_client.get(f"/api/v1/projects/{project_id}")).json()
+    edit = {"name": "Renamed", "priority": project["priority"], "status": project["status"]}
+    response = await async_client.patch(f"/api/v1/projects/{project_id}", json=edit)
+
+    assert response.status_code == 200, response.text
+    assert response.json()["name"] == "Renamed"