fix: delete non existing item should return 404

Author: eoaksnes

api/src/data_providers/clients/ClientInterface.py

@@ -14,7 +14,7 @@ def create(self, instance: M) -> M:
         pass
 
     @abstractmethod
-    def delete(self, id: K) -> None:
+    def delete(self, id: K) -> bool:
         pass
 
     @abstractmethod

api/src/data_providers/clients/mongodb/MongoDatabaseClient.py

@@ -51,7 +51,8 @@ def update(self, uid: str, document: Dict) -> Dict:
         return self.get(uid)
 
     def delete(self, uid: str) -> bool:
-        return self.collection.delete_one(filter={"_id": uid}).acknowledged
+        result = self.collection.delete_one(filter={"_id": uid})
+        return result.deleted_count > 0
 
     def find(self, filters: Dict) -> Cursor:
         return self.collection.find(filter=filters)

api/src/data_providers/repositories/TodoRepository.py

@@ -1,5 +1,6 @@
 from typing import Optional
 
+from common.exceptions import NotFoundException
 from data_providers.clients.ClientInterface import ClientInterface
 from data_providers.repository_interfaces.TodoRepositoryInterface import (
     TodoRepositoryInterface,
@@ -24,7 +25,9 @@ def update(self, todo_item: TodoItem) -> TodoItem:
         return TodoItem.from_dict(updated_todo_item)
 
     def delete(self, todo_item_id: str) -> None:
-        self.client.delete(todo_item_id)
+        is_deleted = self.client.delete(todo_item_id)
+        if not is_deleted:
+            raise NotFoundException
 
     def delete_all(self) -> None:
         self.client.delete_collection(self.client.collection_name)

api/src/features/todo/use_cases/delete_todo_by_id.py

@@ -10,7 +10,5 @@ class DeleteTodoByIdResponse(BaseModel):
 
 
 def delete_todo_use_case(id: str, todo_repository: TodoRepositoryInterface) -> DeleteTodoByIdResponse:
-    if not todo_repository.get(id):
-        return DeleteTodoByIdResponse(success=False)
     todo_repository.delete(id)
     return DeleteTodoByIdResponse(success=True)

api/src/features/todo/use_cases/get_todo_by_id.py

@@ -2,7 +2,6 @@
 
 from pydantic import BaseModel, Field
 
-from common.exceptions import NotFoundException
 from data_providers.repository_interfaces.TodoRepositoryInterface import (
     TodoRepositoryInterface,
 )
@@ -21,6 +20,4 @@ def from_entity(todo_item: TodoItem) -> "GetTodoByIdResponse":
 
 def get_todo_by_id_use_case(id: str, todo_repository: TodoRepositoryInterface) -> GetTodoByIdResponse:
     todo_item = todo_repository.get(id)
-    if not todo_item:
-        raise NotFoundException
     return GetTodoByIdResponse.from_entity(cast(TodoItem, todo_item))

api/src/tests/integration/features/todo/test_todo_feature.py

@@ -1,5 +1,9 @@
 import pytest
-from starlette.status import HTTP_200_OK, HTTP_422_UNPROCESSABLE_ENTITY
+from starlette.status import (
+    HTTP_200_OK,
+    HTTP_404_NOT_FOUND,
+    HTTP_422_UNPROCESSABLE_ENTITY,
+)
 from starlette.testclient import TestClient
 
 from data_providers.clients.ClientInterface import ClientInterface
@@ -35,6 +39,10 @@ def test_get_todo_by_id(self, test_app: TestClient):
         assert response.json()["id"] == "1"
         assert response.json()["title"] == "title 1"
 
+    def test_get_todo_should_return_not_found(self, test_app: TestClient):
+        response = test_app.get("/todos/unknown")
+        assert response.status_code == HTTP_404_NOT_FOUND
+
     def test_add_todo(self, test_app: TestClient):
         response = test_app.post("/todos", json={"title": "title 3"})
         item = response.json()
@@ -53,6 +61,10 @@ def test_update_todo(self, test_app):
         assert response.status_code == HTTP_200_OK
         assert response.json()["success"]
 
+    def test_update_todo_should_return_not_found(self, test_app):
+        response = test_app.put("/todos/unknown", json={"title": "something", "is_completed": False})
+        assert response.status_code == HTTP_404_NOT_FOUND
+
     def test_update_todo_should_return_unprocessable_when_invalid_entity(self, test_app: TestClient):
         response = test_app.put("/todos/1", json={"title": ""})
 
@@ -63,3 +75,7 @@ def test_delete_todo(self, test_app: TestClient):
 
         assert response.status_code == HTTP_200_OK
         assert response.json()["success"]
+
+    def test_delete_todo_should_return_not_found(self, test_app: TestClient):
+        response = test_app.delete("/todos/unknown")
+        assert response.status_code == HTTP_404_NOT_FOUND

api/src/tests/unit/features/todo/use_cases/test_delete_todo_by_id.py

@@ -18,6 +18,5 @@ def test_delete_todo_should_return_success(todo_repository: TodoRepositoryInterf
 
 def test_delete_todo_should_return_not_success(todo_repository: TodoRepositoryInterface):
     id = "unkown"
-    with pytest.raises(NotFoundException) as error:
+    with pytest.raises(NotFoundException):
         delete_todo_use_case(id=id, todo_repository=todo_repository)
-        assert error.value.message == "The todo item you specified does not exist."

api/src/tests/unit/features/todo/use_cases/test_get_todo_by_id.py

@@ -20,6 +20,5 @@ def test_get_todo_by_id_should_return_todo(todo_repository: TodoRepositoryInterf
 
 def test_get_todo_by_id_should_throw_todo_not_found_error(todo_repository: TodoRepositoryInterface):
     id = "unknown"
-    with pytest.raises(NotFoundException) as error:
+    with pytest.raises(NotFoundException):
         get_todo_by_id_use_case(id, todo_repository=todo_repository)
-        assert error.value.message == "The todo item you specified does not exist."