Let the two stock alert toggles reach the database (issue #2945) (#2956)

CHANGELOG.md

@@ -44,6 +44,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
+- **"Reset usage to 0" on a Spoolman spool set it back to full (#2906, reported and contributed by @ojimpo in #2939)** — The reset set Spoolman's `used_weight` to 0. Spoolman calculates the remaining weight as initial minus used, so the spool jumped back to full and its measured remaining weight was lost: a roll with 737 g left read 1000 g, and later prints were charged against filament that wasn't there. The confirmation dialog promised the opposite, that the spool and its remaining weight would not change. The reset now stores the used weight at that moment in a Bambuddy field in the spool's custom fields (`bambu_weight_used_baseline`), the same way it has always worked without Spoolman, and Bambuddy subtracts it when it shows the consumed counter. Spoolman's initial, used and remaining weights are left untouched, and so is every other custom field. The value is stored as text because Spoolman sets a field's type on first use and never lets it change; a number would have been rejected on every install. The bulk reset works the same way. Spools already reset by the old version stay as they are: their remaining weight was overwritten in Spoolman, so they need re-weighing.
 - **Filament printed from the external spool was deducted from an AMS spool (#3166)** — The firmware doesn't accept the external spool in a print command's `ams_mapping` list. So BambuStudio and Bambuddy's own dispatch both write it there as -1, the same value as an unused slot, and put the real target in `ams_mapping2`. Bambuddy read only the first list, so an external-spool print looked unmapped, and usage tracking fell back to guessing by position: the first loaded AMS slot. The reporter's H2D charged every external-spool print that way, including 225 g of ABS taken from the PLA spool in AMS slot 1; the same was reported on H2S and P2S. The external spool is now read from `ams_mapping2` (left or right on dual-nozzle printers). A slot the mapping marks as unused is no longer given a tray by position. When a mapping names no tray at all, the tray the printer reports decides instead. The last change also stops an unused slot from taking over the external spool that another slot really used and dropping that slot's weight (#2880).
 - **Upgrading from Bambuddy 0.2.4.0 or older no longer crashes at startup** — Since 1.2.5.4, startup first converts old failure-reason labels to the new vocabulary (#2974), in both the archives and the print log. A database from before the print log had a failure-reason column (#1378) reached that conversion before the migration that adds the column, and Bambuddy stopped with `no such column: failure_reason` instead of starting. The conversion now skips a table that doesn't have the column yet; it can't hold old labels anyway. On SQLite it also rebuilds the archive search index before converting archives, because archives created before that index existed were never added to it, and updating one of them failed with `database disk image is malformed`. It only rebuilds when there is something to convert, and it skips both tables entirely when nothing needs converting, so a normal startup does less work than before.
 - **A connection check that overruns in the support bundle now says which step hung, and keeps what finished (#3164)** — The support bundle gives each printer's connection diagnostic 15 seconds and used to discard the whole result when it overran, recording only `timed_out`. A 14-printer farm's bundle came back with that marker on every printer and nothing else, so it could not show which check was slow or whether the printers were reachable at all. The diagnostic now records each check as it finishes and the step it is on; a timed-out entry carries `stalled_in` (the step that was still running), `elapsed_s`, and the checks that completed before it.

backend/tests/_fixtures/notification_toggles.py

@@ -0,0 +1,66 @@
+"""The per-event notification toggles, derived from the model's own columns (#2945).
+
+A hardcoded list only covers the toggles someone remembered to add to it, so it
+carries the same hazard as the field maps it is meant to police, one layer up.
+Derived, a toggle added tomorrow is exercised the day its column lands.
+
+This lives here rather than in either test module because the unit parity checks
+and the integration round-trips need the same two answers, and a derivation
+duplicated across files is a list with extra steps.
+"""
+
+from __future__ import annotations
+
+from sqlalchemy import Boolean
+from sqlalchemy.sql.schema import Column
+
+from backend.app.models.notification import NotificationProvider
+
+# The Boolean check is not decoration. An on_* column of some other type would
+# turn `{field: True}` into a 422 and break every test parametrised off this
+# list, in whichever unrelated PR happened to add that column -- and a spurious
+# failure is how a derivation gets edited until it passes.
+_EVENT_TOGGLES: list[Column] = [
+    column
+    for column in NotificationProvider.__table__.columns
+    if column.name.startswith("on_") and isinstance(column.type, Boolean)
+]
+
+EVENT_TOGGLE_COLUMNS: list[str] = sorted(column.name for column in _EVENT_TOGGLES)
+
+# Every column a client is meant to be able to set. The hazard #2945 describes is
+# per-column -- a field that exists everywhere except one of the maps it has to
+# cross -- and `enabled`, the quiet-hours and digest fields cross the same two
+# maps as the on_* toggles. The prefix is a naming convention, not the boundary.
+# What is left out is what the server owns: the key, the timestamps, and the
+# delivery status it records itself.
+_SERVER_OWNED = {"id", "created_at", "updated_at", "last_success", "last_error", "last_error_at"}
+
+SETTABLE_COLUMNS: list[str] = sorted(
+    column.name for column in NotificationProvider.__table__.columns if column.name not in _SERVER_OWNED
+)
+
+
+def _model_default(column: Column) -> bool:
+    """The value a row gets for this column when the caller sends nothing.
+
+    Reads the Python-side default only. That is exact while every toggle
+    declares one, which ``test_every_event_column_has_a_python_default`` holds
+    in place: a column added with only a ``server_default`` would read as False
+    here, get a target of True, and for a server-default-true column the round
+    trip would send True and assert True -- vacuous again, and silently.
+    """
+    if column.default is None:
+        return False
+    return bool(column.default.arg)
+
+
+# The value each toggle has to be driven to for the assertion to mean anything.
+#
+# Nine of these columns default to True on the model and on the response schema,
+# so a test that sends True and asserts True is answered by the default alone:
+# a field dropped by a hand-maintained map still reads back True, and the
+# mutation that would prove the map is load-bearing survives. Driving each
+# toggle to whatever its default is not makes the round trip the only thing that
+# can supply the answer.
+TOGGLE_TARGET: dict[str, bool] = {column.name: not _model_default(column) for column in _EVENT_TOGGLES}

backend/tests/integration/test_notifications_api.py

@@ -7,6 +7,8 @@ import pytest
 from httpx import AsyncClient
 from sqlalchemy import text
 
+from backend.tests._fixtures.notification_toggles import EVENT_TOGGLE_COLUMNS, TOGGLE_TARGET
+
 
 class TestNotificationsAPI:
     """Integration tests for /api/v1/notifications/ endpoints."""
@@ -500,7 +502,7 @@ class TestNotificationsAPI:
         response = await async_client.get(f"/api/v1/notifications/{provider.id}")
         assert response.json()["on_billing_charge_failed"] is False
 
-    # Per-event toggles that live only in these hand-maintained field maps.
+    # Every per-event toggle, across the hand-maintained field maps.
     #
     # These have to be exercised through the route, not the ORM: both
     # directions of notifications.py are hand-maintained field-by-field maps,
@@ -518,52 +520,61 @@ class TestNotificationsAPI:
     # the toggles could not be turned on at all.
     @pytest.mark.asyncio
     @pytest.mark.integration
-    @pytest.mark.parametrize(
-        "field",
-        ["on_ha_sensor_alert", "on_location_ha_sensor_alert", "on_stock_reorder_alert", "on_stock_break_alert"],
-    )
+    @pytest.mark.parametrize("field", EVENT_TOGGLE_COLUMNS)
     async def test_create_persists_and_returns_the_toggle(self, async_client: AsyncClient, field: str):
+        # Driven to whatever the column does not default to. Nine of these
+        # default to True on the model and on the response schema, so sending
+        # True and asserting True is answered by the default alone -- a field
+        # dropped by the create constructor still reads back True, and the
+        # mutation that would prove the constructor load-bearing survives.
+        target = TOGGLE_TARGET[field]
+
         response = await async_client.post(
             "/api/v1/notifications/",
             json={
                 "name": "Sensor Alert Test",
                 "provider_type": "ntfy",
                 "config": {"server": "https://ntfy.sh", "topic": "test"},
-                field: True,
+                field: target,
             },
         )
 
         assert response.status_code == 200
-        assert response.json()[field] is True
+        assert response.json()[field] is target
 
         # Re-read it: a value dropped by the create constructor but echoed
         # from the request body would still pass the assertion above.
         provider_id = response.json()["id"]
         response = await async_client.get(f"/api/v1/notifications/{provider_id}")
-        assert response.json()[field] is True
+        assert response.json()[field] is target
 
     @pytest.mark.asyncio
     @pytest.mark.integration
-    @pytest.mark.parametrize(
-        "field",
-        ["on_ha_sensor_alert", "on_location_ha_sensor_alert", "on_stock_reorder_alert", "on_stock_break_alert"],
-    )
+    @pytest.mark.parametrize("field", EVENT_TOGGLE_COLUMNS)
     async def test_patch_is_reflected_by_every_read_route(
         self, async_client: AsyncClient, notification_provider_factory, field: str
     ):
-        """PATCH already persisted (generic setattr loop) — the reads were the broken half."""
-        provider = await notification_provider_factory(**{field: False})
+        """PATCH already persisted (generic setattr loop) — the reads were the broken half.
+
+        Seeded at the target's opposite and driven to the target, so the value
+        asserted is never the one the column would have supplied on its own.
+        With ``True`` on both sides the nine True-default columns could not see
+        the half this test exists for: deleting a column from the read map left
+        them passing.
+        """
+        target = TOGGLE_TARGET[field]
+        provider = await notification_provider_factory(**{field: not target})
 
-        response = await async_client.patch(f"/api/v1/notifications/{provider.id}", json={field: True})
+        response = await async_client.patch(f"/api/v1/notifications/{provider.id}", json={field: target})
         assert response.status_code == 200
-        assert response.json()[field] is True
+        assert response.json()[field] is target
 
         response = await async_client.get(f"/api/v1/notifications/{provider.id}")
-        assert response.json()[field] is True
+        assert response.json()[field] is target
 
         response = await async_client.get("/api/v1/notifications/")
         listed = next(p for p in response.json() if p["id"] == provider.id)
-        assert listed[field] is True
+        assert listed[field] is target
 
 
 class TestNotificationTemplatesAPI:

backend/tests/unit/test_notification_provider_field_parity.py

@@ -0,0 +1,73 @@
+"""Every client-settable notification provider column must reach the API (#2945).
+
+The defect this pins is not a wrong value, it is a field that exists everywhere
+except the modules a toggle has to cross. `on_stock_reorder_alert` and
+`on_stock_break_alert` had model columns, service producers, templates, UI
+toggles and a frontend test asserting the PATCH — and no schema field, so
+Pydantic dropped them, the PATCH answered 200, and nothing was written. #1184
+introduced that gap and every layer it did touch worked, which is why it went
+unnoticed for months.
+
+These are structural checks of structural facts: a column that is absent from a
+schema cannot be sent at all, whatever the routes do with it afterwards. The
+behaviour behind them — create, both read routes and PATCH, over the same
+derived column list — is covered through the API in
+`backend/tests/integration/test_notifications_api.py`.
+
+The two schemas here are the whole of it. The update route needs nothing
+beyond `NotificationProviderUpdate`, because it applies changes with a generic
+`model_dump(exclude_unset=True)` + `setattr` loop rather than a third
+hand-maintained map: once the field survives the schema, it is written.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from backend.app.models.notification import NotificationProvider
+from backend.app.schemas.notification import (
+    NotificationProviderCreate,
+    NotificationProviderUpdate,
+)
+from backend.tests._fixtures.notification_toggles import (
+    EVENT_TOGGLE_COLUMNS,
+    SETTABLE_COLUMNS,
+    TOGGLE_TARGET,
+)
+
+
+def test_there_are_event_columns_to_check() -> None:
+    """Guard the guard: an empty enumeration would make every test below vacuous."""
+    assert len(EVENT_TOGGLE_COLUMNS) > 20
+
+
+def test_the_targets_cover_both_directions() -> None:
+    """Guard the other guard, the one the integration round-trips lean on.
+
+    ``TOGGLE_TARGET`` exists so a test drives each toggle to whatever its
+    default is not. If every column defaulted the same way -- or if the default
+    lookup quietly started returning one constant -- the targets would collapse
+    to a single value and the round-trips would be answered by the default again
+    without anything failing. Both values have to appear.
+    """
+    assert set(TOGGLE_TARGET.values()) == {True, False}
+
+
+@pytest.mark.parametrize("column", EVENT_TOGGLE_COLUMNS)
+def test_every_event_column_has_a_python_default(column: str) -> None:
+    """``TOGGLE_TARGET`` derives each target from the Python-side default, so a
+    toggle that only has a ``server_default`` would get the wrong target and
+    its round trip would pass on the default alone. Fail here instead."""
+    assert NotificationProvider.__table__.columns[column].default is not None
+
+
+@pytest.mark.parametrize("column", SETTABLE_COLUMNS)
+def test_every_settable_column_is_settable_on_create(column: str) -> None:
+    """Absent from the Create schema, the field is silently dropped from the POST."""
+    assert column in NotificationProviderCreate.model_fields
+
+
+@pytest.mark.parametrize("column", SETTABLE_COLUMNS)
+def test_every_settable_column_is_settable_on_update(column: str) -> None:
+    """This is the one the report hit: the PATCH succeeds and writes nothing."""
+    assert column in NotificationProviderUpdate.model_fields