Review Berry verwerken

tad/api/routes/tasks.py

@@ -1,4 +1,4 @@
-from typing import Annotated, Any
+from typing import Annotated
 
 from fastapi import APIRouter, Depends, Request, status
 from fastapi.responses import HTMLResponse
@@ -22,26 +22,20 @@ async def move_task(
     :return: a HTMLResponse object, in this case the html code of the card that was moved
     """
     try:
+        # because htmx form always sends a value and siblings are optional, we use -1 for None and convert it here
+        if moved_task.next_sibling_id is -1:
+            moved_task.next_sibling_id = None
+        if moved_task.previous_sibling_id is -1:
+            moved_task.previous_sibling_id = None
         task = tasks_service.move_task(
-            convert_to_int_if_is_int(moved_task.id),
-            convert_to_int_if_is_int(moved_task.status_id),
-            convert_to_int_if_is_int(moved_task.previous_sibling_id),
-            convert_to_int_if_is_int(moved_task.next_sibling_id),
+            moved_task.id,
+            moved_task.status_id,
+            moved_task.previous_sibling_id,
+            moved_task.next_sibling_id,
         )
         # todo(Robbert) add error handling for input error or task error handling
         return templates.TemplateResponse(request=request, name="task.jinja", context={"task": task})
     except Exception:
         return templates.TemplateResponse(
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, request=request, name="error.jinja"
         )
-
-
-def convert_to_int_if_is_int(value: Any) -> int | Any:
-    """
-    If the given value is of type int, return it as int, otherwise return the input value as is.
-    :param value: the value to convert
-    :return: the value as int or the original type
-    """
-    if value is not None and isinstance(value, str) and value.isdigit():
-        return int(value)
-    return value

tad/schema/task.py

@@ -1,19 +1,9 @@
-from pydantic import BaseModel, field_validator
+from pydantic import BaseModel
 from pydantic import Field as PydanticField
-from pydantic_core.core_schema import ValidationInfo
 
 
 class MovedTask(BaseModel):
-    # todo(robbert) values from htmx json are all strings, using type int does not work for
-    #  sibling variables (they are optional)
-    id: str = PydanticField(None, alias="taskId", strict=False)
-    status_id: str = PydanticField(None, alias="statusId", strict=False)
-    previous_sibling_id: str | None = PydanticField(None, alias="previousSiblingId", strict=False)
-    next_sibling_id: str | None = PydanticField(None, alias="nextSiblingId", strict=False)
-
-    @field_validator("id", "status_id", "previous_sibling_id", "next_sibling_id")
-    @classmethod
-    def check_is_int(cls, value: str, info: ValidationInfo) -> str:
-        if isinstance(value, str) and value.isdigit():
-            assert value.isdigit(), f"{info.field_name} must be an integer"  # noqa: S101
-        return value
+    id: int = PydanticField(None, alias="taskId", strict=False)
+    status_id: int = PydanticField(None, alias="statusId", strict=False)
+    previous_sibling_id: int | None = PydanticField(None, alias="previousSiblingId", strict=False)
+    next_sibling_id: int | None = PydanticField(None, alias="nextSiblingId", strict=False)

tad/site/static/js/tad.js

@@ -6,8 +6,8 @@ window.onload = function () {
       group: 'shared', // set both lists to same group
       animation: 150,
       onEnd: function (evt) {
-        let previousSiblingId = evt.item.previousElementSibling ? evt.item.previousElementSibling.getAttribute("data-id") : "";
-        let nextSiblingId = evt.item.nextElementSibling ? evt.item.nextElementSibling.getAttribute("data-id") : "";
+        let previousSiblingId = evt.item.previousElementSibling ? evt.item.previousElementSibling.getAttribute("data-id") : "-1";
+        let nextSiblingId = evt.item.nextElementSibling ? evt.item.nextElementSibling.getAttribute("data-id") : "-1";
         let targetId = "#" + evt.item.getAttribute("id");
         let form = document.getElementById("cardMovedForm");
         document.getElementsByName("taskId")[0].value = evt.item.getAttribute("data-id");

tests/api/routes/test_status.py

@@ -13,7 +13,7 @@ def test_post_move_task(client: TestClient, db: DatabaseTestUtils) -> None:
             {"table": "task", "id": 3, "status_id": 2},
         ]
     )
-    move_task: MovedTask = MovedTask(taskId="2", statusId="2", previousSiblingId="1", nextSiblingId="3")
+    move_task: MovedTask = MovedTask(taskId=2, statusId=2, previousSiblingId=1, nextSiblingId=3)
     response = client.patch("/tasks/", json=move_task.model_dump(by_alias=True))
     assert response.status_code == 200
     assert response.headers["content-type"] == "text/html; charset=utf-8"

tests/api/routes/test_tasks_move.py

@@ -12,7 +12,7 @@ def test_post_task_move(client: TestClient, db: DatabaseTestUtils) -> None:
         ]
     )
     response = client.patch(
-        "/tasks/", json={"taskId": "1", "statusId": "1", "previousSiblingId": "2", "nextSiblingId": ""}
+        "/tasks/", json={"taskId": "1", "statusId": "1", "previousSiblingId": "2", "nextSiblingId": "-1"}
     )
     assert response.status_code == 200
     assert response.headers["content-type"] == "text/html; charset=utf-8"
@@ -22,7 +22,7 @@ def test_post_task_move(client: TestClient, db: DatabaseTestUtils) -> None:
 def test_task_move_error(client: TestClient, db: DatabaseTestUtils) -> None:
     db.init()
     response = client.patch(
-        "/tasks/", json={"taskId": "1", "statusId": "1", "previousSiblingId": "2", "nextSiblingId": ""}
+        "/tasks/", json={"taskId": "1", "statusId": "1", "previousSiblingId": "2", "nextSiblingId": "-1"}
     )
     assert response.status_code == 500
     assert response.headers["content-type"] == "text/html; charset=utf-8"