From fb4cbde9155518ae6d927e66b48b8415cf6d7c56 Mon Sep 17 00:00:00 2001 From: saad-mhmd Date: Thu, 6 Aug 2026 20:00:47 +0200 Subject: [PATCH] fix(api): return 409 with blocker counts for PROTECT-blocked deletes (#160) --- apps/common/exceptions.py | 27 ++++++++++ .../tests/test_protected_error_handler.py | 54 +++++++++++++++++++ config/settings.py | 2 + 3 files changed, 83 insertions(+) create mode 100644 apps/common/exceptions.py create mode 100644 apps/common/tests/test_protected_error_handler.py diff --git a/apps/common/exceptions.py b/apps/common/exceptions.py new file mode 100644 index 0000000..6d9a1f5 --- /dev/null +++ b/apps/common/exceptions.py @@ -0,0 +1,27 @@ +"""Project-wide DRF exception handling (config.settings EXCEPTION_HANDLER).""" + +from collections import Counter + +from django.db.models.deletion import ProtectedError +from rest_framework import status +from rest_framework.response import Response +from rest_framework.views import exception_handler + + +def drf_exception_handler(exc, context) -> Response | None: + """Map Django's ProtectedError to 409 instead of an unhandled 500. + + A delete blocked by on_delete=PROTECT is a data conflict, not a server + fault: the request is valid, other rows just still reference the target. + The `detail` string names the blockers; the backoffice error toasts + already render `detail`, so no frontend change is needed. + """ + if isinstance(exc, ProtectedError): + counts = Counter(type(obj)._meta for obj in exc.protected_objects) + parts = [ + f"{count} {meta.verbose_name if count == 1 else meta.verbose_name_plural}" + for meta, count in sorted(counts.items(), key=lambda item: (-item[1], str(item[0].verbose_name))) + ] + detail = f"Cannot delete: still referenced by {', '.join(parts)}." + return Response({"detail": detail}, status=status.HTTP_409_CONFLICT) + return exception_handler(exc, context) diff --git a/apps/common/tests/test_protected_error_handler.py b/apps/common/tests/test_protected_error_handler.py new file mode 100644 index 0000000..103e688 --- /dev/null +++ b/apps/common/tests/test_protected_error_handler.py @@ -0,0 +1,54 @@ +"""ProtectedError → 409 via the project exception handler (#160). + +Deleting a record still referenced through on_delete=PROTECT used to escape as +an unhandled 500. It is a data conflict: the handler answers 409 with a +`detail` naming the blockers. +""" + +import pytest +import rest_framework + +from apps.annotations.tests.factories import GraphFactory +from apps.scribes.models import Hand +from apps.scribes.tests.factories import HandFactory + +HANDS_URL = "/api/v1/management/scribes/hands/" + + +@pytest.mark.django_db +def test_delete_blocked_by_protect_returns_409(management_client): + graph = GraphFactory() + + response = management_client.delete(f"{HANDS_URL}{graph.hand_id}/") + + assert response.status_code == rest_framework.status.HTTP_409_CONFLICT, response.data + assert response.data["detail"] == "Cannot delete: still referenced by 1 graph." + assert Hand.objects.filter(id=graph.hand_id).exists() + + +@pytest.mark.django_db +def test_409_detail_pluralizes_blocker_count(management_client): + graph = GraphFactory() + GraphFactory(item_image=graph.item_image, allograph=graph.allograph, hand=graph.hand) + + response = management_client.delete(f"{HANDS_URL}{graph.hand_id}/") + + assert response.status_code == rest_framework.status.HTTP_409_CONFLICT + assert response.data["detail"] == "Cannot delete: still referenced by 2 graphs." + + +@pytest.mark.django_db +def test_unreferenced_delete_still_works(management_client): + hand = HandFactory() + + response = management_client.delete(f"{HANDS_URL}{hand.id}/") + + assert response.status_code == rest_framework.status.HTTP_204_NO_CONTENT + assert not Hand.objects.filter(id=hand.id).exists() + + +@pytest.mark.django_db +def test_other_exceptions_still_handled_by_drf(management_client): + response = management_client.delete(f"{HANDS_URL}999999/") + + assert response.status_code == rest_framework.status.HTTP_404_NOT_FOUND diff --git a/config/settings.py b/config/settings.py index c995770..0ab5f18 100644 --- a/config/settings.py +++ b/config/settings.py @@ -242,6 +242,8 @@ }, "DEFAULT_PAGINATION_CLASS": "config.pagination.BoundedLimitOffsetPagination", "PAGE_SIZE": 20, + # ProtectedError → 409 (a PROTECT-blocked delete is a conflict, not a 500). + "EXCEPTION_HANDLER": "apps.common.exceptions.drf_exception_handler", } STATIC_URL = "static/"