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)
This commit is contained in:
@@ -135,6 +135,16 @@ async def mark_all_read(
|
|||||||
return MarkAllReadResponse(marked_count=marked)
|
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)
|
@router.delete("/{notification_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||||
async def dismiss_notification(
|
async def dismiss_notification(
|
||||||
notification_id: uuid.UUID,
|
notification_id: uuid.UUID,
|
||||||
@@ -149,13 +159,3 @@ async def dismiss_notification(
|
|||||||
status_code=status.HTTP_404_NOT_FOUND,
|
status_code=status.HTTP_404_NOT_FOUND,
|
||||||
detail="Notification not found",
|
detail="Notification not found",
|
||||||
) from exc
|
) 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)
|
|
||||||
|
|||||||
@@ -139,11 +139,7 @@ async def publish_lifecycle_event(
|
|||||||
if not _should_notify(event_type, effective_status):
|
if not _should_notify(event_type, effective_status):
|
||||||
return
|
return
|
||||||
|
|
||||||
severity = (
|
severity = "error" if event_type == "instance.error" else "success"
|
||||||
"error"
|
|
||||||
if event_type == "instance.error"
|
|
||||||
else "success"
|
|
||||||
)
|
|
||||||
title = _derive_title(event_type)
|
title = _derive_title(event_type)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -351,8 +351,12 @@ async def test_dismiss_all_affects_only_caller(
|
|||||||
cleared = await notification_service.dismiss_all(db_session, user_a.id)
|
cleared = await notification_service.dismiss_all(db_session, user_a.id)
|
||||||
|
|
||||||
assert cleared == 3
|
assert cleared == 3
|
||||||
items_a, total_a = await notification_service.list_notifications(db_session, user_a.id)
|
items_a, total_a = await notification_service.list_notifications(
|
||||||
items_b, total_b = await notification_service.list_notifications(db_session, user_b.id)
|
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_a == 0
|
||||||
assert total_b == 2
|
assert total_b == 2
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user