Skip to content

Commit

Permalink
fix: delete non existing item should return 404
Browse files Browse the repository at this point in the history
  • Loading branch information
eoaksnes committed Nov 4, 2022
1 parent 0528e5b commit 4341416
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 13 deletions.
2 changes: 1 addition & 1 deletion api/src/data_providers/clients/ClientInterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion api/src/data_providers/repositories/TodoRepository.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions api/src/features/todo/use_cases/delete_todo_by_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
3 changes: 0 additions & 3 deletions api/src/features/todo/use_cases/get_todo_by_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from pydantic import BaseModel, Field

from common.exceptions import NotFoundException
from data_providers.repository_interfaces.TodoRepositoryInterface import (
TodoRepositoryInterface,
)
Expand All @@ -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))
18 changes: 17 additions & 1 deletion api/src/tests/integration/features/todo/test_todo_feature.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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": ""})

Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Original file line number Diff line number Diff line change
Expand Up @@ -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."

0 comments on commit 4341416

Please sign in to comment.