Skip to content

Commit 5bda8e4

Browse files
feat: Implement PATCH /tasks/{task_id} endpoint unit tests (#73)
* feat: Implement PATCH /tasks/{task_id} to update tasks - Added method to handle update requests. - Introduced for validating incoming update data, ensuring all fields are optional and is a future date. - Added method to to perform the MongoDB operation, ensuring is set. - Implemented to orchestrate the update logic: - Fetches the task or raises . - Processes validated data from the serializer. - Converts label string IDs to and validates their existence. - Converts priority and status string names to their enum values for database storage. - Sets (currently placeholder). - Calls the repository to persist changes. - Corrected an issue where enum objects (Priority, Status) were passed directly to the database layer, causing a serialization error. Now, their primitive values are stored. - Refined construction in to ensure all valid fields from the serializer are correctly prepared for update. * refactor(tasks): Standardize error handling in TaskService update - Removes redundant format validation for label ObjectIds, as this is already handled by . - For missing (but validly formatted) label IDs, now raises instead of a custom * rfactor: PATCH /tasks/{task_id} endpoint to have task title blank true * refactor: improve error handling for task updates and ID validation centralizes error handling for task update operations: - Removes specific ObjectId validation from . Invalid ID formats will now correctly raise , which is handled by the global DRF exception handler to return a 400 Bad Request. - Modifies to raise if the task to be updated is not found. This is also handled by the global exception handler, resulting in a 404 Not Found response * refactor: Enhance serializer validations and repository clarity - Implemented validation to ensure `startedAt` cannot be a future date. - Added `FUTURE_STARTED_AT` to `ValidationErrors` in `constants/messages.py` for the new validation message. - Added an explicit type check to ensure `labels` input is a list or tuple. - Modified to collect all invalid ObjectId formats within the `labels` list and report them in a single `ValidationError` for better client feedback. - Moved the "Labels must be a list or tuple..." message to `ValidationErrors.INVALID_LABELS_STRUCTURE` in `constants/messages.py` and updated the serializer to use this constant. - Deleted an unreachable `if not update_data_with_timestamp: pass` block, as the updatedAt field ensures the dictionary is never empty at that point. * Update todo/serializers/update_task_serializer.py Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> * Update todo/views/task.py Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> * fix: gramatical errors on the constants * fix: added missing imports * fix: used global exception handler for catching errors * Refactor(TaskRepository): modify update method for input validation and ObjectId handling * refactor(serializers): align validate_dueAt error handling pattern -refactors the method in to collect validation errors in a list and raise a single with this list. * refactor(services): reduce complexity of TaskService.update_task -Refactors the method in to improve readability and reduce cognitive complexity. The core logic for processing specific field types (labels, enums) has been extracted into dedicated helper methods: and * refactor(services): reduce complexity of TaskService.update_task -Refactors the method in to improve readability and reduce cognitive complexity. The core logic for processing specific field types (labels, enums) has been extracted into dedicated helper methods: and * refactor: refactor the code to improve maintainability * feat: Implement PATCH `/tasks/{task_id}` endpoint unit tests (#53) * feat: Implement PATCH /tasks/{task_id} endpoint unit tests - Developed a comprehensive suite of unit tests for: - (various valid/invalid payloads). - (success, task not found, invalid ID). - (success, task not found, label validation errors, repository update failures, enum handling). - (success, serializer errors, service-layer exceptions like TaskNotFound, BsonInvalidId, DRFValidationError) * refactor: improve error handling and update associated tests Centralizes error handling for task update operations: - Removes specific ObjectId validation from . Invalid ID formats now correctly raise , handled by the global DRF exception handler (400 Bad Request). - Modifies to raise if the task to be updated is not found by the repository. This is also handled by the global exception handler (404 Not Found). Updates unit tests to align with these changes: - now asserts is raised. - is renamed to and now asserts is raised. * refactor: adjust serializer tests for ListField behavior and add validation coverage * fix: import errors * refactor: harden TaskRepository.update and align tests * fix: test assertions * chore: remove test description * chore: update develop branch into this branch --------- Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
1 parent a10d480 commit 5bda8e4

File tree

4 files changed

+819
-2
lines changed

4 files changed

+819
-2
lines changed

todo/tests/unit/repositories/test_task_repository.py

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from pymongo import ReturnDocument
44
from pymongo.collection import Collection
55
from bson import ObjectId, errors as bson_errors
6-
from datetime import datetime, timezone
6+
from datetime import datetime, timezone, timedelta
77
import copy
88

99
from todo.models.task import TaskModel
@@ -208,6 +208,124 @@ def test_create_task_handles_exception(self, mock_create):
208208
mock_create.assert_called_once_with(task)
209209

210210

211+
class TaskRepositoryUpdateTests(TestCase):
212+
def setUp(self):
213+
self.patcher_get_collection = patch("todo.repositories.task_repository.TaskRepository.get_collection")
214+
self.mock_get_collection = self.patcher_get_collection.start()
215+
self.mock_collection = MagicMock(spec=Collection)
216+
self.mock_get_collection.return_value = self.mock_collection
217+
218+
self.task_id_str = str(ObjectId())
219+
self.task_id_obj = ObjectId(self.task_id_str)
220+
self.valid_update_data = {
221+
"title": "Updated Title",
222+
"description": "Updated description",
223+
"priority": TaskPriority.HIGH.value,
224+
"status": TaskStatus.IN_PROGRESS.value,
225+
}
226+
self.updated_doc_from_db = {
227+
"_id": self.task_id_obj,
228+
"displayId": "#123",
229+
"title": "Updated Title",
230+
"description": "Updated description",
231+
"priority": TaskPriority.HIGH.value,
232+
"status": TaskStatus.IN_PROGRESS.value,
233+
"assignee": "user1",
234+
"labels": [],
235+
"createdAt": datetime.now(timezone.utc) - timedelta(days=1),
236+
"updatedAt": datetime.now(timezone.utc),
237+
"createdBy": "system_user",
238+
"updatedBy": "patch_user",
239+
"isAcknowledged": False,
240+
"isDeleted": False,
241+
}
242+
243+
def tearDown(self):
244+
self.patcher_get_collection.stop()
245+
246+
def test_update_task_success(self):
247+
self.mock_collection.find_one_and_update.return_value = self.updated_doc_from_db
248+
249+
result_task = TaskRepository.update(self.task_id_str, self.valid_update_data)
250+
251+
self.assertIsNotNone(result_task)
252+
self.assertIsInstance(result_task, TaskModel)
253+
self.assertEqual(str(result_task.id), self.task_id_str)
254+
self.assertEqual(result_task.title, self.valid_update_data["title"])
255+
self.assertEqual(result_task.description, self.valid_update_data["description"])
256+
self.assertIsNotNone(result_task.updatedAt)
257+
258+
args, kwargs = self.mock_collection.find_one_and_update.call_args
259+
self.assertEqual(args[0], {"_id": self.task_id_obj})
260+
self.assertEqual(kwargs["return_document"], ReturnDocument.AFTER)
261+
262+
update_doc_arg = args[1]
263+
self.assertIn("$set", update_doc_arg)
264+
set_payload = update_doc_arg["$set"]
265+
self.assertIn("updatedAt", set_payload)
266+
self.assertIsInstance(set_payload["updatedAt"], datetime)
267+
268+
for key, value in self.valid_update_data.items():
269+
self.assertEqual(set_payload[key], value)
270+
271+
def test_update_task_returns_none_if_task_not_found(self):
272+
self.mock_collection.find_one_and_update.return_value = None
273+
274+
result_task = TaskRepository.update(self.task_id_str, self.valid_update_data)
275+
276+
self.assertIsNone(result_task)
277+
self.mock_collection.find_one_and_update.assert_called_once()
278+
279+
args, kwargs = self.mock_collection.find_one_and_update.call_args
280+
self.assertEqual(args[0], {"_id": self.task_id_obj})
281+
update_doc_arg = args[1]
282+
self.assertIn("updatedAt", update_doc_arg["$set"])
283+
284+
def test_update_task_returns_none_for_invalid_task_id_format(self):
285+
invalid_id_str = "not-an-object-id"
286+
287+
result_task = TaskRepository.update(invalid_id_str, self.valid_update_data)
288+
self.assertIsNone(result_task)
289+
290+
self.mock_collection.find_one_and_update.assert_not_called()
291+
292+
def test_update_task_raises_value_error_for_non_dict_update_data(self):
293+
with self.assertRaises(ValueError) as context:
294+
TaskRepository.update(self.task_id_str, "not-a-dict")
295+
self.assertEqual(str(context.exception), "update_data must be a dictionary.")
296+
self.mock_collection.find_one_and_update.assert_not_called()
297+
298+
def test_update_task_empty_update_data_still_calls_find_one_and_update(self):
299+
self.mock_collection.find_one_and_update.return_value = {**self.updated_doc_from_db, "title": "Original Title"}
300+
301+
result_task = TaskRepository.update(self.task_id_str, {})
302+
303+
self.assertIsNotNone(result_task)
304+
self.mock_collection.find_one_and_update.assert_called_once()
305+
args, kwargs = self.mock_collection.find_one_and_update.call_args
306+
self.assertEqual(args[0], {"_id": self.task_id_obj})
307+
update_doc_arg = args[1]["$set"]
308+
self.assertIn("updatedAt", update_doc_arg)
309+
self.assertEqual(len(update_doc_arg), 1)
310+
311+
def test_update_task_does_not_pass_id_or_underscore_id_in_update_payload(self):
312+
self.mock_collection.find_one_and_update.return_value = self.updated_doc_from_db
313+
314+
data_with_ids = {"_id": "some_other_id", "id": "yet_another_id", "title": "Title with IDs"}
315+
316+
TaskRepository.update(self.task_id_str, data_with_ids)
317+
318+
self.mock_collection.find_one_and_update.assert_called_once()
319+
args, _ = self.mock_collection.find_one_and_update.call_args
320+
set_payload = args[1]["$set"]
321+
322+
self.assertNotIn("_id", set_payload)
323+
self.assertNotIn("id", set_payload)
324+
self.assertIn("title", set_payload)
325+
self.assertEqual(set_payload["title"], "Title with IDs")
326+
self.assertIn("updatedAt", set_payload)
327+
328+
211329
class TestRepositoryDeleteTaskById(TestCase):
212330
def setUp(self):
213331
self.task_id = tasks_db_data[0]["id"]
Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
from unittest import TestCase
2+
from datetime import datetime, timezone, timedelta
3+
from bson import ObjectId
4+
5+
from todo.serializers.update_task_serializer import UpdateTaskSerializer
6+
from todo.constants.task import TaskPriority, TaskStatus
7+
from todo.constants.messages import ValidationErrors
8+
9+
10+
class UpdateTaskSerializerTests(TestCase):
11+
def setUp(self):
12+
self.valid_object_id_str = str(ObjectId())
13+
self.future_date = datetime.now(timezone.utc) + timedelta(days=1)
14+
self.past_date = datetime.now(timezone.utc) - timedelta(days=1)
15+
16+
def test_valid_full_payload(self):
17+
data = {
18+
"title": "Updated Test Task",
19+
"description": "This is an updated description.",
20+
"priority": TaskPriority.HIGH.name,
21+
"status": TaskStatus.IN_PROGRESS.name,
22+
"assignee": "user_assignee_id",
23+
"labels": [str(ObjectId()), str(ObjectId())],
24+
"dueAt": self.future_date.isoformat(),
25+
"startedAt": (datetime.now(timezone.utc) - timedelta(hours=1)).isoformat(),
26+
"isAcknowledged": True,
27+
}
28+
serializer = UpdateTaskSerializer(data=data)
29+
self.assertTrue(serializer.is_valid(), serializer.errors)
30+
validated_data = serializer.validated_data
31+
self.assertEqual(validated_data["title"], data["title"])
32+
self.assertEqual(validated_data["description"], data["description"])
33+
self.assertEqual(validated_data["priority"], data["priority"])
34+
self.assertEqual(validated_data["status"], data["status"])
35+
self.assertEqual(validated_data["assignee"], data["assignee"])
36+
self.assertEqual(validated_data["labels"], data["labels"])
37+
self.assertEqual(validated_data["dueAt"], datetime.fromisoformat(data["dueAt"]))
38+
self.assertEqual(validated_data["startedAt"], datetime.fromisoformat(data["startedAt"]))
39+
self.assertEqual(validated_data["isAcknowledged"], data["isAcknowledged"])
40+
41+
def test_partial_payload_title_only(self):
42+
data = {"title": "Only Title Update"}
43+
serializer = UpdateTaskSerializer(data=data, partial=True)
44+
self.assertTrue(serializer.is_valid(), serializer.errors)
45+
self.assertEqual(serializer.validated_data["title"], data["title"])
46+
self.assertEqual(len(serializer.validated_data), 1)
47+
48+
def test_all_fields_can_be_null_or_empty_if_allowed(self):
49+
data = {
50+
"description": None,
51+
"priority": None,
52+
"status": None,
53+
"assignee": None,
54+
"labels": None,
55+
"dueAt": None,
56+
"startedAt": None,
57+
}
58+
serializer = UpdateTaskSerializer(data=data, partial=True)
59+
self.assertTrue(serializer.is_valid(), serializer.errors)
60+
61+
self.assertNotIn("title", serializer.validated_data)
62+
self.assertIsNone(serializer.validated_data.get("description"))
63+
self.assertIsNone(serializer.validated_data.get("priority"))
64+
self.assertIsNone(serializer.validated_data.get("status"))
65+
self.assertIsNone(serializer.validated_data.get("assignee"))
66+
self.assertIsNone(serializer.validated_data.get("labels"))
67+
self.assertIsNone(serializer.validated_data.get("dueAt"))
68+
self.assertIsNone(serializer.validated_data.get("startedAt"))
69+
70+
def test_title_validation_blank(self):
71+
data = {"title": " "}
72+
serializer = UpdateTaskSerializer(data=data, partial=True)
73+
self.assertFalse(serializer.is_valid())
74+
self.assertIn("title", serializer.errors)
75+
self.assertEqual(str(serializer.errors["title"][0]), ValidationErrors.BLANK_TITLE)
76+
77+
def test_title_valid(self):
78+
data = {"title": "Valid Title"}
79+
serializer = UpdateTaskSerializer(data=data, partial=True)
80+
self.assertTrue(serializer.is_valid(), serializer.errors)
81+
self.assertEqual(serializer.validated_data["title"], "Valid Title")
82+
83+
def test_labels_validation_invalid_object_id(self):
84+
data = {"labels": [self.valid_object_id_str, "invalid-id"]}
85+
serializer = UpdateTaskSerializer(data=data, partial=True)
86+
self.assertFalse(serializer.is_valid())
87+
self.assertIn("labels", serializer.errors)
88+
self.assertIn(ValidationErrors.INVALID_OBJECT_ID.format("invalid-id"), str(serializer.errors["labels"]))
89+
90+
def test_labels_validation_valid_object_ids(self):
91+
valid_ids = [str(ObjectId()), str(ObjectId())]
92+
data = {"labels": valid_ids}
93+
serializer = UpdateTaskSerializer(data=data, partial=True)
94+
self.assertTrue(serializer.is_valid(), serializer.errors)
95+
self.assertEqual(serializer.validated_data["labels"], valid_ids)
96+
97+
def test_labels_can_be_empty_list(self):
98+
data = {"labels": []}
99+
serializer = UpdateTaskSerializer(data=data, partial=True)
100+
self.assertTrue(serializer.is_valid(), serializer.errors)
101+
self.assertEqual(serializer.validated_data["labels"], [])
102+
103+
def test_due_at_validation_past_date(self):
104+
data = {"dueAt": self.past_date.isoformat()}
105+
serializer = UpdateTaskSerializer(data=data, partial=True)
106+
self.assertFalse(serializer.is_valid())
107+
self.assertIn("dueAt", serializer.errors)
108+
self.assertEqual(str(serializer.errors["dueAt"][0]), ValidationErrors.PAST_DUE_DATE)
109+
110+
def test_due_at_validation_future_date(self):
111+
data = {"dueAt": self.future_date.isoformat()}
112+
serializer = UpdateTaskSerializer(data=data, partial=True)
113+
self.assertTrue(serializer.is_valid(), serializer.errors)
114+
self.assertEqual(serializer.validated_data["dueAt"], datetime.fromisoformat(data["dueAt"]))
115+
116+
def test_due_at_can_be_null(self):
117+
data = {"dueAt": None}
118+
serializer = UpdateTaskSerializer(data=data, partial=True)
119+
self.assertTrue(serializer.is_valid(), serializer.errors)
120+
self.assertIsNone(serializer.validated_data["dueAt"])
121+
122+
def test_assignee_validation_blank_string_becomes_none(self):
123+
data = {"assignee": " "}
124+
serializer = UpdateTaskSerializer(data=data, partial=True)
125+
self.assertTrue(serializer.is_valid(), serializer.errors)
126+
self.assertIsNone(serializer.validated_data["assignee"])
127+
128+
def test_assignee_valid_string(self):
129+
data = {"assignee": "user123"}
130+
serializer = UpdateTaskSerializer(data=data, partial=True)
131+
self.assertTrue(serializer.is_valid(), serializer.errors)
132+
self.assertEqual(serializer.validated_data["assignee"], "user123")
133+
134+
def test_assignee_can_be_null(self):
135+
data = {"assignee": None}
136+
serializer = UpdateTaskSerializer(data=data, partial=True)
137+
self.assertTrue(serializer.is_valid(), serializer.errors)
138+
self.assertIsNone(serializer.validated_data["assignee"])
139+
140+
def test_priority_invalid_choice(self):
141+
data = {"priority": "VERY_HIGH"}
142+
serializer = UpdateTaskSerializer(data=data, partial=True)
143+
self.assertFalse(serializer.is_valid())
144+
self.assertIn("priority", serializer.errors)
145+
self.assertIn("is not a valid choice.", str(serializer.errors["priority"][0]))
146+
147+
def test_status_invalid_choice(self):
148+
data = {"status": "PENDING_APPROVAL"}
149+
serializer = UpdateTaskSerializer(data=data, partial=True)
150+
self.assertFalse(serializer.is_valid())
151+
self.assertIn("status", serializer.errors)
152+
self.assertIn("is not a valid choice.", str(serializer.errors["status"][0]))
153+
154+
def test_is_acknowledged_valid(self):
155+
data = {"isAcknowledged": True}
156+
serializer = UpdateTaskSerializer(data=data, partial=True)
157+
self.assertTrue(serializer.is_valid(), serializer.errors)
158+
self.assertTrue(serializer.validated_data["isAcknowledged"])
159+
160+
data = {"isAcknowledged": False}
161+
serializer = UpdateTaskSerializer(data=data, partial=True)
162+
self.assertTrue(serializer.is_valid(), serializer.errors)
163+
self.assertFalse(serializer.validated_data["isAcknowledged"])
164+
165+
def test_description_can_be_null(self):
166+
data = {"description": None}
167+
serializer = UpdateTaskSerializer(data=data, partial=True)
168+
self.assertTrue(serializer.is_valid(), serializer.errors)
169+
self.assertIsNone(serializer.validated_data.get("description"))
170+
171+
def test_description_can_be_empty_string(self):
172+
data = {"description": ""}
173+
serializer = UpdateTaskSerializer(data=data, partial=True)
174+
self.assertTrue(serializer.is_valid(), serializer.errors)
175+
self.assertEqual(serializer.validated_data.get("description"), "")
176+
177+
def test_started_at_can_be_null(self):
178+
data = {"startedAt": None}
179+
serializer = UpdateTaskSerializer(data=data, partial=True)
180+
self.assertTrue(serializer.is_valid(), serializer.errors)
181+
self.assertIsNone(serializer.validated_data.get("startedAt"))
182+
183+
def test_started_at_valid_datetime(self):
184+
date_val = (datetime.now(timezone.utc) - timedelta(hours=1)).isoformat()
185+
data = {"startedAt": date_val}
186+
serializer = UpdateTaskSerializer(data=data, partial=True)
187+
self.assertTrue(serializer.is_valid(), serializer.errors)
188+
self.assertEqual(serializer.validated_data["startedAt"], datetime.fromisoformat(date_val))
189+
190+
def test_started_at_validation_future_date(self):
191+
future_started_at = (datetime.now(timezone.utc) + timedelta(days=1)).isoformat()
192+
data = {"startedAt": future_started_at}
193+
serializer = UpdateTaskSerializer(data=data, partial=True)
194+
self.assertFalse(serializer.is_valid())
195+
self.assertIn("startedAt", serializer.errors)
196+
self.assertEqual(str(serializer.errors["startedAt"][0]), ValidationErrors.FUTURE_STARTED_AT)
197+
198+
def test_labels_validation_not_list_or_tuple(self):
199+
data = {"labels": "not-a-list-or-tuple"}
200+
serializer = UpdateTaskSerializer(data=data, partial=True)
201+
self.assertFalse(serializer.is_valid())
202+
self.assertIn("labels", serializer.errors)
203+
self.assertEqual(str(serializer.errors["labels"][0]), 'Expected a list of items but got type "str".')
204+
205+
def test_labels_validation_multiple_invalid_object_ids(self):
206+
invalid_id_1 = "invalid-id-1"
207+
invalid_id_2 = "invalid-id-2"
208+
data = {"labels": [self.valid_object_id_str, invalid_id_1, invalid_id_2]}
209+
serializer = UpdateTaskSerializer(data=data, partial=True)
210+
self.assertFalse(serializer.is_valid())
211+
self.assertIn("labels", serializer.errors)
212+
213+
label_errors = serializer.errors["labels"]
214+
self.assertIsInstance(label_errors, list)
215+
216+
self.assertEqual(len(label_errors), 2)
217+
self.assertIn(ValidationErrors.INVALID_OBJECT_ID.format(invalid_id_1), label_errors)
218+
self.assertIn(ValidationErrors.INVALID_OBJECT_ID.format(invalid_id_2), label_errors)
219+
220+
def test_labels_validation_mixed_valid_and_multiple_invalid_ids(self):
221+
valid_id_1 = str(ObjectId())
222+
invalid_id_1 = "bad-id-format-1"
223+
valid_id_2 = str(ObjectId())
224+
invalid_id_2 = "another-invalid"
225+
226+
data = {"labels": [valid_id_1, invalid_id_1, valid_id_2, invalid_id_2]}
227+
serializer = UpdateTaskSerializer(data=data, partial=True)
228+
self.assertFalse(serializer.is_valid())
229+
self.assertIn("labels", serializer.errors)
230+
231+
label_errors = serializer.errors["labels"]
232+
self.assertIsInstance(label_errors, list)
233+
self.assertEqual(len(label_errors), 2)
234+
235+
expected_error_messages = [
236+
ValidationErrors.INVALID_OBJECT_ID.format(invalid_id_1),
237+
ValidationErrors.INVALID_OBJECT_ID.format(invalid_id_2),
238+
]
239+
240+
for msg in expected_error_messages:
241+
self.assertIn(msg, label_errors)

0 commit comments

Comments
 (0)