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

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.
 
-        response = await async_client.patch(f"/api/v1/notifications/{provider.id}", json={field: True})
+        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: 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