PB-1108 Refine user deletion responses

app/access/api.py

@@ -55,10 +55,12 @@ def delete(request: HttpRequest, username: str) -> HttpResponse:
     Return HTTP status code
     - 204 (No Content) if the user has been deleted
     - 404 (Not Found) if there is no user with the given username
+    - 500 (Internal Server Error) if there is inconsistency with cognito
+    - 503 (Service Unavailable) if cognito cannot be reached
     """
     model = get_object_or_404(User, username=username)
     deleted = delete_user(model)
     if not deleted:
-        raise HttpError(503, "Service Unavailable. Please retry later.")
+        raise HttpError(500, "Internal Server Error")
     model.delete()
     return HttpResponse(status=204)

app/access/tests/test_api.py

@@ -4,6 +4,8 @@
 from access.api import user_to_response
 from access.models import User
 from access.schemas import UserSchema
+from botocore.exceptions import EndpointConnectionError
+from config.api import cognito_connection_error
 from ninja.testing import TestClient
 from provider.models import Provider
 
@@ -23,6 +25,13 @@ def setUp(self):
         }
         User.objects.create(**model_fields)
 
+        self.client = TestClient(router)
+        # Re-attach custom exception handlers https://github.com/vitalik/django-ninja/issues/1171
+        _ = self.client.urls
+        self.client.router_or_app.api.add_exception_handler(
+            EndpointConnectionError, cognito_connection_error
+        )
+
     def test_user_to_response_maps_fields_correctly(self):
 
         model = User.objects.last()
@@ -41,8 +50,7 @@ def test_user_to_response_maps_fields_correctly(self):
 
     def test_get_user_returns_existing_user(self):
 
-        client = TestClient(router)
-        response = client.get("users/dude")
+        response = self.client.get("users/dude")
 
         assert response.status_code == 200
         assert response.data == {
@@ -55,16 +63,14 @@ def test_get_user_returns_existing_user(self):
 
     def test_get_user_returns_404_if_nonexisting(self):
 
-        client = TestClient(router)
-        response = client.get("users/nihilist")
+        response = self.client.get("users/nihilist")
 
         assert response.status_code == 404
         assert response.data == {"detail": "Not Found"}
 
     def test_get_users_returns_single_user(self):
 
-        client = TestClient(router)
-        response = client.get("users")
+        response = self.client.get("users")
 
         assert response.status_code == 200
         assert response.data == {
@@ -88,8 +94,7 @@ def test_get_users_returns_users_ordered_by_id(self):
         }
         User.objects.create(**model_fields)
 
-        client = TestClient(router)
-        response = client.get("users")
+        response = self.client.get("users")
 
         assert response.status_code == 200
         assert response.data == {
@@ -111,31 +116,42 @@ def test_get_users_returns_users_ordered_by_id(self):
             ]
         }
 
-    @patch('access.api.delete_user', return_value=True)
-    def test_delete_user_deletes_user(self, mock):
-        client = TestClient(router)
-        response = client.delete("users/dude")
+    @patch('access.api.delete_user')
+    def test_delete_user_deletes_user(self, delete_user):
+        delete_user.return_value = True
+
+        response = self.client.delete("users/dude")
 
         assert response.status_code == 204
         assert response.content == b''
         assert User.objects.count() == 0
 
-    @patch('access.api.delete_user', return_value=False)
-    def test_delete_user_returns_404_if_nonexisting(self, mock):
+    @patch('access.api.delete_user')
+    def test_delete_user_returns_404_if_nonexisting(self, delete_user):
+        delete_user.return_value = False
 
-        client = TestClient(router)
-        response = client.delete("users/lebowski")
+        response = self.client.delete("users/lebowski")
 
         assert response.status_code == 404
         assert response.data == {"detail": "Not Found"}
         assert User.objects.count() == 1
 
-    @patch('access.api.delete_user', return_value=False)
-    def test_delete_user_returns_503_if_cognito_down(self, mock):
+    @patch('access.api.delete_user')
+    def test_delete_user_returns_500_if_cognito_inconsistent(self, delete_user):
+        delete_user.return_value = False
+
+        response = self.client.delete("users/dude")
+
+        assert response.status_code == 500
+        assert response.data == {"detail": "Internal Server Error"}
+        assert User.objects.count() == 1
+
+    @patch('access.api.delete_user')
+    def test_delete_user_returns_503_if_cognito_down(self, delete_user):
+        delete_user.side_effect = EndpointConnectionError(endpoint_url='http://localhost')
 
-        client = TestClient(router)
-        response = client.delete("users/dude")
+        response = self.client.delete("users/dude")
 
         assert response.status_code == 503
-        assert response.data == {'detail': 'Service Unavailable. Please retry later.'}
+        assert response.data == {"code": 503, "description": "Service Unavailable"}
         assert User.objects.count() == 1

app/config/api.py

@@ -1,4 +1,5 @@
 from access.api import router as access_router
+from botocore.exceptions import EndpointConnectionError
 from distributions.api import router as distributions_router
 from ninja import NinjaAPI
 from provider.api import router as provider_router
@@ -14,6 +15,17 @@
 root = NinjaAPI(urls_namespace="root")
 
 
+@api.exception_handler(EndpointConnectionError)
+def cognito_connection_error(request: HttpRequest, exception: EndpointConnectionError):
+    return api.create_response(
+        request,
+        {
+            "code": 503, "description": "Service Unavailable"
+        },
+        status=503,
+    )
+
+
 @root.get("/checker")
 def checker(request: HttpRequest) -> dict[str, bool | str]:
     return {"success": True, "message": "OK"}