From 3d1f8d9cf7a52c3837739d9bae8e6770e42b235d Mon Sep 17 00:00:00 2001
From: Alex Blank <noreply@runfusion.ai>
Date: Fri, 29 May 2026 16:54:01 +0200
Subject: [PATCH] fix: reorder notification DELETE routes so bulk clear matches
 first

FastAPI matches routes in declaration order. The DELETE /notifications
endpoint (bulk clear) was registered AFTER DELETE /notifications/{id},
so the path parameter route intercepted all requests to the bulk route,
causing a 422 UUID validation error instead of hitting clear_all.

Moved clear_all_notifications above dismiss_notification in the router.
Added regression test to verify route order.

Quality gates: pytest (22 passed)
---
 apps/api/src/api/notifications.py             | 20 +++++------
 apps/api/src/services/lifecycle_hooks.py      |  6 +---
 .../tests/unit/test_notification_service.py   |  8 +++--
 .../unit/test_notifications_api_routes.py     | 34 +++++++++++++++++++
 4 files changed, 51 insertions(+), 17 deletions(-)
 create mode 100644 apps/api/tests/unit/test_notifications_api_routes.py

diff --git a/apps/api/src/api/notifications.py b/apps/api/src/api/notifications.py
index adaacb2..9dff52c 100644
--- a/apps/api/src/api/notifications.py
+++ b/apps/api/src/api/notifications.py
@@ -135,6 +135,16 @@ async def mark_all_read(
     return MarkAllReadResponse(marked_count=marked)
 
 
+@router.delete("", status_code=status.HTTP_200_OK)
+async def clear_all_notifications(
+    user: User = Depends(get_current_user),
+    session: AsyncSession = Depends(get_db_session),
+) -> ClearAllResponse:
+    """Dismiss all notifications for the authenticated user."""
+    cleared = await notification_service.dismiss_all(session, user.id)
+    return ClearAllResponse(cleared_count=cleared)
+
+
 @router.delete("/{notification_id}", status_code=status.HTTP_204_NO_CONTENT)
 async def dismiss_notification(
     notification_id: uuid.UUID,
@@ -149,13 +159,3 @@ async def dismiss_notification(
             status_code=status.HTTP_404_NOT_FOUND,
             detail="Notification not found",
         ) from exc
-
-
-@router.delete("", status_code=status.HTTP_200_OK)
-async def clear_all_notifications(
-    user: User = Depends(get_current_user),
-    session: AsyncSession = Depends(get_db_session),
-) -> ClearAllResponse:
-    """Dismiss all notifications for the authenticated user."""
-    cleared = await notification_service.dismiss_all(session, user.id)
-    return ClearAllResponse(cleared_count=cleared)
diff --git a/apps/api/src/services/lifecycle_hooks.py b/apps/api/src/services/lifecycle_hooks.py
index c3e802e..5af816e 100644
--- a/apps/api/src/services/lifecycle_hooks.py
+++ b/apps/api/src/services/lifecycle_hooks.py
@@ -139,11 +139,7 @@ async def publish_lifecycle_event(
     if not _should_notify(event_type, effective_status):
         return
 
-    severity = (
-        "error"
-        if event_type == "instance.error"
-        else "success"
-    )
+    severity = "error" if event_type == "instance.error" else "success"
     title = _derive_title(event_type)
 
     try:
diff --git a/apps/api/tests/unit/test_notification_service.py b/apps/api/tests/unit/test_notification_service.py
index d619e5e..6aca22b 100644
--- a/apps/api/tests/unit/test_notification_service.py
+++ b/apps/api/tests/unit/test_notification_service.py
@@ -351,8 +351,12 @@ async def test_dismiss_all_affects_only_caller(
     cleared = await notification_service.dismiss_all(db_session, user_a.id)
 
     assert cleared == 3
-    items_a, total_a = await notification_service.list_notifications(db_session, user_a.id)
-    items_b, total_b = await notification_service.list_notifications(db_session, user_b.id)
+    items_a, total_a = await notification_service.list_notifications(
+        db_session, user_a.id
+    )
+    items_b, total_b = await notification_service.list_notifications(
+        db_session, user_b.id
+    )
     assert total_a == 0
     assert total_b == 2
 
diff --git a/apps/api/tests/unit/test_notifications_api_routes.py b/apps/api/tests/unit/test_notifications_api_routes.py
new file mode 100644
index 0000000..e588456
--- /dev/null
+++ b/apps/api/tests/unit/test_notifications_api_routes.py
@@ -0,0 +1,34 @@
+"""Unit tests for notification API route ordering."""
+
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+from src.api.notifications import router as notifications_router
+
+
+def test_delete_notifications_route_order() -> None:
+    """DELETE /notifications must match before DELETE /notifications/{id}.
+
+    FastAPI matches routes in declaration order. The bulk clear endpoint
+    (DELETE /notifications) must be registered before the single dismiss
+    endpoint (DELETE /notifications/{notification_id}) or the path
+    parameter route will intercept the bulk route.
+    """
+    app = FastAPI()
+    app.include_router(notifications_router)
+    client = TestClient(app)
+
+    # Verify the bulk delete route exists and returns the expected schema
+    # (it will 401 without auth, but that's fine — we just need to confirm
+    # routing doesn't hit the UUID-parameter route first)
+    response = client.delete("/notifications")
+    # Should get 401 (unauthenticated), NOT 422 (UUID parse error)
+    assert response.status_code == 401, (
+        f"Expected 401 (auth required), got {response.status_code}. "
+        f"Route order may be wrong — DELETE /notifications matched "
+        f"DELETE /notifications/{{notification_id}} instead."
+    )
+
+    # Verify the single dismiss route still works (also 401 without auth)
+    response = client.delete("/notifications/12345678-1234-1234-1234-123456789abc")
+    assert response.status_code == 401