Skip to content

Commit 982ec2e

Browse files
Sbussisoclaude
andcommitted
fix(notifications): hide dead "Mark all read" button + add motion toggle
Two related fixes on the notification inbox UX: 1. "Mark all read" button was always a no-op Opening the bell panel already calls markAllViewed() to clear the badge (see togglePanel), so by the time the button was visible the work was done. Clicking it re-ran the same call with no visible effect — users thought it was broken. Gate the button on unreadCount > 0 so it only appears when there's actually unread content (new items arriving via SSE while the panel is open). 2. Motion notifications were un-silenceable Add a per-org preferences gate (motion / camera transition / node transition) stored via the existing Setting key/value table. The gate lives inside create_notification() so every emitter respects it without duplicating the check. Defaults match previous behaviour ("all on") so existing orgs see no change until an admin flips a toggle off. Motion events are still recorded to the database for incidents and analytics — the gate only suppresses the bell-inbox row. Motion SSE (dashboard toasts) is intentionally NOT gated: if we ever want to mute those too it's a separate toggle. UI: Settings > Notifications > "Motion detection" toggle. Backend also exposes camera/node transition toggles (same pattern) ready for a future UI addition. Tests: 9 new backend tests covering the gate helper, the routes (GET, POST, admin-only, round-trip), and the /api/settings aggregate. Full suite: 164 passed (was 155). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 48afd48 commit 982ec2e

8 files changed

Lines changed: 370 additions & 12 deletions

File tree

backend/app/api/cameras.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,20 @@
1010
from app.models.models import Camera, CameraGroup, Setting, AuditLog
1111
from app.schemas.schemas import (
1212
CameraGroupCreate,
13+
NotificationSettings,
1314
RecordingSettings,
1415
)
1516

17+
18+
# Shared defaults for notification toggles — used by the GET handler and
19+
# the /api/settings aggregate. Pydantic's NotificationSettings holds the
20+
# canonical values; this dict re-expresses them as strings for Setting.get_many.
21+
_NOTIFICATION_SETTING_DEFAULTS = {
22+
"motion_notifications": "true",
23+
"camera_transition_notifications": "true",
24+
"node_transition_notifications": "true",
25+
}
26+
1627
router = APIRouter(prefix="/api", tags=["api"])
1728
logger = logging.getLogger(__name__)
1829

@@ -233,6 +244,7 @@ async def get_all_settings(
233244
"scheduled_start": "06:00",
234245
"scheduled_end": "17:00",
235246
"continuous_24_7": "false",
247+
**_NOTIFICATION_SETTING_DEFAULTS,
236248
})
237249
return {
238250
"recording": {
@@ -241,6 +253,11 @@ async def get_all_settings(
241253
"scheduled_end": vals["scheduled_end"],
242254
"continuous_24_7": vals["continuous_24_7"] == "true",
243255
},
256+
"notifications": {
257+
"motion_notifications": vals["motion_notifications"] == "true",
258+
"camera_transition_notifications": vals["camera_transition_notifications"] == "true",
259+
"node_transition_notifications": vals["node_transition_notifications"] == "true",
260+
},
244261
}
245262

246263

@@ -295,6 +312,67 @@ async def update_recording_settings(
295312
return {"success": True}
296313

297314

315+
# Notification preferences — parallel to the recording settings pair.
316+
# GET is view-level (every member needs to know what's on), POST is
317+
# admin-only (same audit-worthy gate as the other per-org toggles).
318+
319+
@router.get("/settings/notifications")
320+
async def get_notification_settings(
321+
user: AuthUser = Depends(require_view), db: Session = Depends(get_db)
322+
):
323+
"""Return the org's notification preferences.
324+
325+
Defaults to "all on" for backward compat with orgs that existed
326+
before the settings UI landed — the gate only starts filtering
327+
after an admin explicitly flips a toggle off.
328+
"""
329+
vals = Setting.get_many(db, user.org_id, _NOTIFICATION_SETTING_DEFAULTS)
330+
return {
331+
"motion_notifications": vals["motion_notifications"] == "true",
332+
"camera_transition_notifications": vals["camera_transition_notifications"] == "true",
333+
"node_transition_notifications": vals["node_transition_notifications"] == "true",
334+
}
335+
336+
337+
@router.post("/settings/notifications")
338+
@limiter.limit("30/minute")
339+
async def update_notification_settings(
340+
data: NotificationSettings,
341+
request: Request,
342+
user: AuthUser = Depends(require_admin),
343+
db: Session = Depends(get_db),
344+
):
345+
"""Update notification preferences. Requires admin.
346+
347+
Persists each toggle as a stringified bool so the existing Setting
348+
key/value table can store it without a schema change — same
349+
convention the recording toggles use.
350+
"""
351+
Setting.set(db, user.org_id, "motion_notifications", str(data.motion_notifications).lower())
352+
Setting.set(
353+
db, user.org_id, "camera_transition_notifications",
354+
str(data.camera_transition_notifications).lower(),
355+
)
356+
Setting.set(
357+
db, user.org_id, "node_transition_notifications",
358+
str(data.node_transition_notifications).lower(),
359+
)
360+
write_audit(
361+
db,
362+
org_id=user.org_id,
363+
event="notification_settings_updated",
364+
user_id=user.user_id,
365+
username=audit_label(user),
366+
details={
367+
"motion_notifications": bool(data.motion_notifications),
368+
"camera_transition_notifications": bool(data.camera_transition_notifications),
369+
"node_transition_notifications": bool(data.node_transition_notifications),
370+
},
371+
request=request,
372+
)
373+
return {"success": True}
374+
375+
298376
# Audit Logs
299377
@router.get("/audit-logs")
300378
async def list_audit_logs(

backend/app/api/notifications.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,41 @@
2929

3030
from app.core.auth import AuthUser, require_view
3131
from app.core.database import SessionLocal, get_db
32-
from app.models.models import Notification, UserNotificationState
32+
from app.models.models import Notification, Setting, UserNotificationState
33+
34+
35+
# ── Per-org preference gate ────────────────────────────────────────
36+
# Which notification kinds the org wants delivered to the inbox.
37+
# Defaults intentionally match "everything on" so behaviour before the
38+
# settings UI shipped is unchanged — operators only ever see less noise
39+
# than before, never more. Keys map kind → (setting_key, default).
40+
41+
_NOTIFICATION_KIND_TO_SETTING: dict[str, tuple[str, bool]] = {
42+
"motion": ("motion_notifications", True),
43+
"camera_online": ("camera_transition_notifications", True),
44+
"camera_offline": ("camera_transition_notifications", True),
45+
"node_online": ("node_transition_notifications", True),
46+
"node_offline": ("node_transition_notifications", True),
47+
}
48+
49+
50+
def notifications_enabled(db: Session, org_id: str, kind: str) -> bool:
51+
"""Return True if notifications of ``kind`` should be delivered for
52+
``org_id``. Unknown kinds default to enabled so new notification
53+
types don't silently disappear when added without a settings migration.
54+
55+
Reads from the ``Setting`` table using the same "string value" pattern
56+
the recording toggles use. Keeping it here (not in the motion path)
57+
means every emitter can gate without duplicating the lookup logic.
58+
"""
59+
cfg = _NOTIFICATION_KIND_TO_SETTING.get(kind)
60+
if cfg is None:
61+
return True
62+
key, default = cfg
63+
val = Setting.get(db, org_id, key, None)
64+
if val is None:
65+
return default
66+
return val == "true"
3367

3468
logger = logging.getLogger(__name__)
3569

@@ -124,6 +158,20 @@ def create_notification(
124158
if severity not in ("info", "warning", "error", "critical"):
125159
severity = "info"
126160

161+
# Per-org preference gate — an operator who turned off "motion
162+
# notifications" in Settings should stop seeing new motion rows in
163+
# the inbox without affecting the motion-event pipeline itself.
164+
# Done here (vs. at each call site) so every emitter, current and
165+
# future, automatically respects the toggle.
166+
try:
167+
if not notifications_enabled(session, org_id, kind):
168+
return None
169+
except Exception:
170+
# Gate failures must not block notifications — fall through to
171+
# the normal create path so we're no worse off than before the
172+
# gate existed.
173+
logger.exception("[Notifications] preference lookup failed; emitting anyway")
174+
127175
try:
128176
notif = Notification(
129177
org_id=org_id,

backend/app/schemas/schemas.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,19 @@ class RecordingSettings(BaseModel):
1515
continuous_24_7: bool = False
1616

1717

18+
class NotificationSettings(BaseModel):
19+
"""Per-org toggles controlling which kinds of notifications show up in
20+
the bell inbox. Camera/node transitions stay on by default because
21+
those are operator-critical; motion can get noisy so we let operators
22+
silence it per-org without disabling the motion-event pipeline itself
23+
(motion still records to the DB for incidents and analytics).
24+
"""
25+
26+
motion_notifications: bool = True
27+
camera_transition_notifications: bool = True
28+
node_transition_notifications: bool = True
29+
30+
1831
class CameraReport(BaseModel):
1932
camera_id: Optional[str] = Field(None, max_length=150)
2033
device_path: Optional[str] = Field(None, max_length=255)

backend/tests/test_cameras.py

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ def test_list_cameras_empty(admin_client):
1616

1717

1818
def test_get_settings(admin_client):
19-
"""Settings endpoint returns recording settings without fake detection fields."""
19+
"""Settings endpoint returns recording + notification sections."""
2020
from app.core.auth import require_view
2121
from tests.conftest import _make_admin_user
2222
app = admin_client.app
@@ -32,14 +32,21 @@ def test_get_settings(admin_client):
3232
assert "scheduled_recording" in recording
3333
assert "continuous_24_7" in recording
3434

35-
# Should NOT have fake detection fields
35+
# Should NOT have fake detection fields — these were removed because
36+
# the backend never actually did motion/face/object recording.
3637
assert "motion_recording" not in recording
3738
assert "face_recording" not in recording
3839
assert "object_recording" not in recording
3940
assert "post_buffer" not in recording
4041

41-
# Should NOT have notifications section
42-
assert "notifications" not in data
42+
# Should have notifications section with all three toggles defaulted on
43+
# (backwards compat: orgs that pre-date the toggle default to everything
44+
# enabled, same behaviour they had before).
45+
assert "notifications" in data
46+
notifications = data["notifications"]
47+
assert notifications["motion_notifications"] is True
48+
assert notifications["camera_transition_notifications"] is True
49+
assert notifications["node_transition_notifications"] is True
4350

4451

4552
def test_update_recording_settings(admin_client):
@@ -54,11 +61,58 @@ def test_update_recording_settings(admin_client):
5461
assert resp.json()["success"] is True
5562

5663

57-
def test_notification_endpoints_removed(admin_client):
58-
"""Fake notification endpoints should no longer exist."""
64+
def test_get_notification_settings_defaults_on(admin_client):
65+
"""New orgs see all notification toggles enabled by default."""
5966
resp = admin_client.get("/api/settings/notifications")
60-
# Should return 404 (or be caught by SPA middleware)
61-
assert resp.status_code in (404, 405, 200) # 200 if SPA middleware serves index.html
67+
assert resp.status_code == 200
68+
body = resp.json()
69+
assert body["motion_notifications"] is True
70+
assert body["camera_transition_notifications"] is True
71+
assert body["node_transition_notifications"] is True
72+
73+
74+
def test_update_notification_settings(admin_client):
75+
"""Admin can flip motion notifications off and the change persists."""
76+
resp = admin_client.post("/api/settings/notifications", json={
77+
"motion_notifications": False,
78+
"camera_transition_notifications": True,
79+
"node_transition_notifications": True,
80+
})
81+
assert resp.status_code == 200
82+
assert resp.json()["success"] is True
83+
84+
# Round-trip: GET must reflect the change
85+
follow = admin_client.get("/api/settings/notifications")
86+
assert follow.status_code == 200
87+
body = follow.json()
88+
assert body["motion_notifications"] is False
89+
assert body["camera_transition_notifications"] is True
90+
assert body["node_transition_notifications"] is True
91+
92+
93+
def test_notification_settings_reflected_in_all_settings(admin_client):
94+
"""After toggling motion off, /api/settings aggregate shows the change."""
95+
admin_client.post("/api/settings/notifications", json={
96+
"motion_notifications": False,
97+
"camera_transition_notifications": True,
98+
"node_transition_notifications": True,
99+
})
100+
resp = admin_client.get("/api/settings")
101+
assert resp.status_code == 200
102+
assert resp.json()["notifications"]["motion_notifications"] is False
103+
104+
105+
def test_update_notification_settings_requires_admin(viewer_client):
106+
"""Non-admin members can't flip the toggles."""
107+
resp = viewer_client.post("/api/settings/notifications", json={
108+
"motion_notifications": False,
109+
"camera_transition_notifications": True,
110+
"node_transition_notifications": True,
111+
})
112+
# require_admin yields 401/403/404 depending on the auth layer's
113+
# rejection path — viewer_client doesn't override require_admin so
114+
# the real dependency runs and rejects.
115+
assert resp.status_code in (401, 403)
62116

63117

64118
def test_camera_groups_crud(admin_client):

backend/tests/test_notifications.py

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@
2323
emit_camera_transition,
2424
emit_node_transition,
2525
notification_broadcaster,
26+
notifications_enabled,
2627
)
2728
from app.main import run_offline_sweep
28-
from app.models.models import Camera, CameraNode, Notification, UserNotificationState
29+
from app.models.models import Camera, CameraNode, Notification, Setting, UserNotificationState
2930
from tests.conftest import TestSession
3031

3132

@@ -473,3 +474,85 @@ def test_offline_sweep_mixed_fresh_and_stale(db):
473474
notifs = {n.kind: n for n in db.query(Notification).all()}
474475
assert notifs["camera_offline"].node_id == "node_1" # linked to parent node
475476
assert notifs["node_offline"].audience == "admin"
477+
478+
479+
# ── Preference gate ────────────────────────────────────────────────
480+
# The Settings UI lets operators silence specific notification kinds
481+
# without disabling the underlying event pipeline. These tests pin the
482+
# gate's behaviour so a refactor can't silently stop respecting the
483+
# toggle (which would be noisy in the worst way — spam returns).
484+
485+
def test_notifications_enabled_defaults_true(db):
486+
# No Setting row exists → gate returns the per-kind default, which is
487+
# True for every kind we ship today. Legacy orgs shouldn't lose
488+
# notifications just because they never visited the settings page.
489+
assert notifications_enabled(db, "org_test123", "motion") is True
490+
assert notifications_enabled(db, "org_test123", "camera_offline") is True
491+
assert notifications_enabled(db, "org_test123", "node_online") is True
492+
493+
494+
def test_notifications_enabled_honors_false_flag(db):
495+
Setting.set(db, "org_test123", "motion_notifications", "false")
496+
assert notifications_enabled(db, "org_test123", "motion") is False
497+
# Other kinds unaffected — each toggle is scoped to its setting key.
498+
assert notifications_enabled(db, "org_test123", "camera_offline") is True
499+
500+
501+
def test_notifications_enabled_unknown_kind_defaults_true(db):
502+
# A future notification kind added without a settings migration
503+
# should default to delivered, not silently dropped.
504+
assert notifications_enabled(db, "org_test123", "brand_new_kind") is True
505+
506+
507+
def test_create_notification_respects_motion_toggle(db):
508+
Setting.set(db, "org_test123", "motion_notifications", "false")
509+
510+
result = create_notification(
511+
org_id="org_test123",
512+
kind="motion",
513+
title="Motion on cam_abc",
514+
body="Scene change detected at 42% intensity.",
515+
audience="all",
516+
camera_id="cam_abc",
517+
db=db,
518+
)
519+
520+
# Gate returns None when disabled — no row persisted, no broadcast.
521+
assert result is None
522+
assert db.query(Notification).filter_by(kind="motion").count() == 0
523+
524+
525+
def test_create_notification_motion_allowed_when_toggle_on(db):
526+
# Explicitly enable (belt-and-suspenders; default is also True).
527+
Setting.set(db, "org_test123", "motion_notifications", "true")
528+
529+
result = create_notification(
530+
org_id="org_test123",
531+
kind="motion",
532+
title="Motion on cam_abc",
533+
audience="all",
534+
camera_id="cam_abc",
535+
db=db,
536+
)
537+
538+
assert result is not None
539+
assert db.query(Notification).filter_by(kind="motion").count() == 1
540+
541+
542+
def test_create_notification_camera_transition_gated(db):
543+
# Camera online/offline share one toggle. Turning it off suppresses
544+
# BOTH directions — the user either wants camera transition alerts or
545+
# doesn't.
546+
Setting.set(db, "org_test123", "camera_transition_notifications", "false")
547+
548+
offline = create_notification(
549+
org_id="org_test123", kind="camera_offline",
550+
title="cam_abc offline", audience="all", db=db,
551+
)
552+
online = create_notification(
553+
org_id="org_test123", kind="camera_online",
554+
title="cam_abc online", audience="all", db=db,
555+
)
556+
assert offline is None
557+
assert online is None
558+
assert db.query(Notification).count() == 0

0 commit comments

Comments
 (0)