From d8bb4001cfb6ad756a104140349ccdc866719aac Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Tue, 29 Jul 2025 20:34:58 +0530 Subject: [PATCH 1/7] feat: enhance task status filtering and validation logic - Updated the TaskRepository to improve status filtering logic, allowing for more precise queries based on task status and deferred details. - Refactored the TaskService to adjust validation for deferring tasks, ensuring that deferred dates are correctly compared to due dates. - Modified integration and unit tests to reflect changes in task deferral logic and removed unused constants for cleaner code. These enhancements improve the accuracy of task management operations and ensure better validation during task deferral. --- todo/repositories/task_repository.py | 36 ++++++++++++++----- todo/services/task_service.py | 7 ++-- todo/tests/integration/test_task_defer_api.py | 11 +++--- .../unit/repositories/test_task_repository.py | 7 +++- todo/tests/unit/services/test_task_service.py | 2 +- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/todo/repositories/task_repository.py b/todo/repositories/task_repository.py index 10b97363..39e50de8 100644 --- a/todo/repositories/task_repository.py +++ b/todo/repositories/task_repository.py @@ -17,16 +17,36 @@ class TaskRepository(MongoRepository): @classmethod def _build_status_filter(cls, status_filter: str = None) -> dict: - """ - Build status filter for task queries. + now = datetime.now(timezone.utc) + + if status_filter == TaskStatus.DEFERRED.value: + return { + "$and": [ + {"deferredDetails": {"$ne": None}}, + {"deferredDetails.deferredTill": {"$gt": now}}, + ] + } + + elif status_filter == TaskStatus.DONE.value: + return { + "$or": [ + {"deferredDetails": None}, + {"deferredDetails.deferredTill": {"$lte": now}}, + ] + } - """ - if status_filter: - if status_filter == TaskStatus.DONE.value: - return {} # No status filtering, include all tasks - return {"status": status_filter} else: - return {"status": {"$ne": TaskStatus.DONE.value}} + return { + "$and": [ + {"status": {"$ne": TaskStatus.DONE.value}}, + { + "$or": [ + {"deferredDetails": None}, + {"deferredDetails.deferredTill": {"$lte": now}}, + ] + }, + ] + } @classmethod def list( diff --git a/todo/services/task_service.py b/todo/services/task_service.py index 3c79f6b4..27c9b677 100644 --- a/todo/services/task_service.py +++ b/todo/services/task_service.py @@ -3,7 +3,7 @@ from django.core.exceptions import ValidationError from django.urls import reverse_lazy from urllib.parse import urlencode -from datetime import datetime, timezone, timedelta +from datetime import datetime, timezone from todo.dto.deferred_details_dto import DeferredDetailsDTO from todo.dto.label_dto import LabelDTO from todo.dto.task_dto import TaskDTO, CreateTaskDTO @@ -28,7 +28,6 @@ from todo.constants.task import ( TaskStatus, TaskPriority, - MINIMUM_DEFERRAL_NOTICE_DAYS, ) from todo.constants.messages import ApiErrors, ValidationErrors from django.conf import settings @@ -547,9 +546,7 @@ def defer_task(cls, task_id: str, deferred_till: datetime, user_id: str) -> Task else current_task.dueAt.astimezone(timezone.utc) ) - defer_limit = due_at - timedelta(days=MINIMUM_DEFERRAL_NOTICE_DAYS) - - if deferred_till > defer_limit: + if deferred_till >= due_at: raise UnprocessableEntityException( ValidationErrors.CANNOT_DEFER_TOO_CLOSE_TO_DUE_DATE, source={ApiErrorSource.PARAMETER: "deferredTill"}, diff --git a/todo/tests/integration/test_task_defer_api.py b/todo/tests/integration/test_task_defer_api.py index c4ac2cc4..c20393be 100644 --- a/todo/tests/integration/test_task_defer_api.py +++ b/todo/tests/integration/test_task_defer_api.py @@ -3,7 +3,7 @@ from bson import ObjectId from django.urls import reverse from todo.constants.messages import ApiErrors, ValidationErrors -from todo.constants.task import MINIMUM_DEFERRAL_NOTICE_DAYS, TaskPriority, TaskStatus +from todo.constants.task import TaskPriority, TaskStatus from todo.tests.integration.base_mongo_test import AuthenticatedMongoTestCase from todo.tests.fixtures.task import tasks_db_data @@ -52,7 +52,7 @@ def _insert_task(self, *, status: str = TaskStatus.TODO.value, due_at: datetime def test_defer_task_success(self): now = datetime.now(timezone.utc) - due_at = now + timedelta(days=MINIMUM_DEFERRAL_NOTICE_DAYS + 30) + due_at = now + timedelta(days=30) task_id = self._insert_task(due_at=due_at) deferred_till = now + timedelta(days=10) @@ -76,11 +76,10 @@ def test_defer_task_success(self): def test_defer_task_too_close_to_due_date_returns_422(self): now = datetime.now(timezone.utc) - due_at = now + timedelta(days=MINIMUM_DEFERRAL_NOTICE_DAYS + 5) + due_at = now + timedelta(days=5) task_id = self._insert_task(due_at=due_at) - defer_limit = due_at - timedelta(days=MINIMUM_DEFERRAL_NOTICE_DAYS) - deferred_till = defer_limit + timedelta(days=1) + deferred_till = due_at + timedelta(days=1) url = reverse("task_detail", args=[task_id]) + "?action=defer" response = self.client.patch(url, data={"deferredTill": deferred_till.isoformat()}, format="json") @@ -129,7 +128,7 @@ def test_defer_task_with_missing_date_returns_400(self): def test_defer_task_unauthorized(self): now = datetime.now(timezone.utc) - due_at = now + timedelta(days=MINIMUM_DEFERRAL_NOTICE_DAYS + 30) + due_at = now + timedelta(days=30) task_id = self._insert_task(due_at=due_at) deferred_till = now + timedelta(days=10) url = reverse("task_detail", args=[task_id]) + "?action=defer" diff --git a/todo/tests/unit/repositories/test_task_repository.py b/todo/tests/unit/repositories/test_task_repository.py index 6a3e4fcb..e6a25969 100644 --- a/todo/tests/unit/repositories/test_task_repository.py +++ b/todo/tests/unit/repositories/test_task_repository.py @@ -97,7 +97,12 @@ def test_count_returns_total_task_count(self): result = TaskRepository.count() self.assertEqual(result, 42) - self.mock_collection.count_documents.assert_called_once_with({"status": {"$ne": "DONE"}}) + + self.mock_collection.count_documents.assert_called_once() + actual_filter = self.mock_collection.count_documents.call_args[0][0] + self.assertIn("$and", actual_filter) + self.assertIn("status", actual_filter["$and"][0]) + self.assertIn("$or", actual_filter["$and"][1]) def test_get_all_returns_all_tasks(self): self.mock_collection.find.return_value = self.task_data diff --git a/todo/tests/unit/services/test_task_service.py b/todo/tests/unit/services/test_task_service.py index 603929ab..4159e0cf 100644 --- a/todo/tests/unit/services/test_task_service.py +++ b/todo/tests/unit/services/test_task_service.py @@ -1024,7 +1024,7 @@ def test_defer_task_success(self, mock_prepare_dto, mock_repo_update, mock_repo_ @patch("todo.services.task_service.TaskRepository.get_by_id") def test_defer_task_too_close_to_due_date_raises_exception(self, mock_repo_get_by_id): mock_repo_get_by_id.return_value = self.task_model - deferred_till = self.due_at - timedelta(days=1) + deferred_till = self.due_at + timedelta(days=1) # AFTER due_at to trigger validation error with self.assertRaises(UnprocessableEntityException): TaskService.defer_task(self.task_id, deferred_till, self.user_id) From ba6934fe3bc37505bc449d140a79fa86bbdf7ed7 Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Tue, 29 Jul 2025 23:54:59 +0530 Subject: [PATCH 2/7] fix: failing teams unit test --- .../unit/repositories/test_task_repository.py | 2 +- todo/tests/unit/views/test_team.py | 31 ++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/todo/tests/unit/repositories/test_task_repository.py b/todo/tests/unit/repositories/test_task_repository.py index e6a25969..f24029a1 100644 --- a/todo/tests/unit/repositories/test_task_repository.py +++ b/todo/tests/unit/repositories/test_task_repository.py @@ -97,7 +97,7 @@ def test_count_returns_total_task_count(self): result = TaskRepository.count() self.assertEqual(result, 42) - + self.mock_collection.count_documents.assert_called_once() actual_filter = self.mock_collection.count_documents.call_args[0][0] self.assertIn("$and", actual_filter) diff --git a/todo/tests/unit/views/test_team.py b/todo/tests/unit/views/test_team.py index c78f83cd..b733e374 100644 --- a/todo/tests/unit/views/test_team.py +++ b/todo/tests/unit/views/test_team.py @@ -3,7 +3,7 @@ from rest_framework.test import APIClient from rest_framework import status -from todo.views.team import TeamListView, JoinTeamByInviteCodeView +from todo.views.team import TeamListView, JoinTeamByInviteCodeView, RemoveTeamMemberView from todo.dto.responses.get_user_teams_response import GetUserTeamsResponse from todo.dto.team_dto import TeamDTO from datetime import datetime, timezone @@ -138,30 +138,47 @@ def test_join_team_by_invite_code_validation_error(self): class RemoveTeamMemberViewTests(TestCase): def setUp(self): - self.client = APIClient() + self.view = RemoveTeamMemberView() self.team_id = "507f1f77bcf86cd799439012" self.user_id = "507f1f77bcf86cd799439011" - self.url = f"/teams/{self.team_id}/members/{self.user_id}/" + self.mock_user_id = "507f1f77bcf86cd799439013" @patch("todo.views.team.TeamService.remove_member_from_team") def test_remove_member_success(self, mock_remove): mock_remove.return_value = True - response = self.client.delete(self.url) + + mock_request = MagicMock() + mock_request.user_id = self.mock_user_id + + response = self.view.delete(mock_request, self.team_id, self.user_id) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - mock_remove.assert_called_once_with(user_id=self.user_id, team_id=self.team_id) + mock_remove.assert_called_once_with( + user_id=self.user_id, team_id=self.team_id, removed_by_user_id=self.mock_user_id + ) @patch("todo.views.team.TeamService.remove_member_from_team") def test_remove_member_not_found(self, mock_remove): from todo.services.team_service import TeamService mock_remove.side_effect = TeamService.TeamOrUserNotFound() - response = self.client.delete(self.url) + + mock_request = MagicMock() + mock_request.user_id = self.mock_user_id + + response = self.view.delete(mock_request, self.team_id, self.user_id) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.assertIn("not found", response.data["detail"]) @patch("todo.views.team.TeamService.remove_member_from_team") def test_remove_member_generic_error(self, mock_remove): mock_remove.side_effect = Exception("Something went wrong") - response = self.client.delete(self.url) + + mock_request = MagicMock() + mock_request.user_id = self.mock_user_id + + response = self.view.delete(mock_request, self.team_id, self.user_id) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertIn("Something went wrong", response.data["detail"]) From 268fdd191e34e54cc13c107628cb818dae2d5b6d Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Wed, 30 Jul 2025 01:35:21 +0530 Subject: [PATCH 3/7] nit: remove comment --- todo/tests/unit/services/test_task_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/todo/tests/unit/services/test_task_service.py b/todo/tests/unit/services/test_task_service.py index 4159e0cf..7fe1d1e8 100644 --- a/todo/tests/unit/services/test_task_service.py +++ b/todo/tests/unit/services/test_task_service.py @@ -1024,7 +1024,7 @@ def test_defer_task_success(self, mock_prepare_dto, mock_repo_update, mock_repo_ @patch("todo.services.task_service.TaskRepository.get_by_id") def test_defer_task_too_close_to_due_date_raises_exception(self, mock_repo_get_by_id): mock_repo_get_by_id.return_value = self.task_model - deferred_till = self.due_at + timedelta(days=1) # AFTER due_at to trigger validation error + deferred_till = self.due_at + timedelta(days=1) with self.assertRaises(UnprocessableEntityException): TaskService.defer_task(self.task_id, deferred_till, self.user_id) From c56552911562fe0311e0062c9dc6cdca03983a73 Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Wed, 30 Jul 2025 13:58:19 +0530 Subject: [PATCH 4/7] fix: update task status handling in TaskService - Adjusted task status assignment in TaskService to account for deferred tasks, ensuring that tasks with deferred details are correctly marked as DEFERRED. - Updated the return statement to reflect the new task status logic, improving the accuracy of task status representation. - Initialized task status to TODO in the update payload for task modifications, enhancing consistency in task state management. --- todo/services/task_service.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/todo/services/task_service.py b/todo/services/task_service.py index 27c9b677..0837befc 100644 --- a/todo/services/task_service.py +++ b/todo/services/task_service.py @@ -172,6 +172,11 @@ def prepare_task_dto(cls, task_model: TaskModel, user_id: str = None) -> TaskDTO if watchlist_entry: in_watchlist = watchlist_entry.isActive + task_status = task_model.status + + if task_model.deferredDetails and task_model.deferredDetails.deferredTill > datetime.now(timezone.utc): + task_status = TaskStatus.DEFERRED.value + return TaskDTO( id=str(task_model.id), displayId=task_model.displayId, @@ -182,7 +187,7 @@ def prepare_task_dto(cls, task_model: TaskModel, user_id: str = None) -> TaskDTO labels=label_dtos, startedAt=task_model.startedAt, dueAt=task_model.dueAt, - status=task_model.status, + status=task_status, priority=task_model.priority, deferredDetails=deferred_details, in_watchlist=in_watchlist, @@ -559,6 +564,7 @@ def defer_task(cls, task_id: str, deferred_till: datetime, user_id: str) -> Task ) update_payload = { + "status": TaskStatus.TODO.value, "deferredDetails": deferred_details.model_dump(), "updatedBy": user_id, } From df135a12a06d893d24a250310cb1d3c73d456ea1 Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Fri, 1 Aug 2025 12:47:55 +0530 Subject: [PATCH 5/7] fix: handle deferred details in task status updates - Added logic to clear deferred details when a task's status is updated, ensuring that tasks with deferred information are correctly managed during status changes. - This change improves the accuracy of task updates and maintains consistency in task state management. --- todo/services/task_service.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/todo/services/task_service.py b/todo/services/task_service.py index 0837befc..5dd7b147 100644 --- a/todo/services/task_service.py +++ b/todo/services/task_service.py @@ -424,6 +424,9 @@ def update_task_with_assignee_from_dict(cls, task_id: str, validated_data: dict, if validated_data.get("status") == TaskStatus.IN_PROGRESS and not current_task.startedAt: update_payload["startedAt"] = datetime.now(timezone.utc) + if validated_data.get("status") is not None and current_task.deferredDetails: + update_payload["deferredDetails"] = None + # Update task if there are changes if update_payload: update_payload["updatedBy"] = user_id From 6c1d84b70e606d6ac4686a3fd2f93f964bf083c1 Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Fri, 1 Aug 2025 13:32:56 +0530 Subject: [PATCH 6/7] fix: refine task status update logic in TaskService --- todo/services/task_service.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/todo/services/task_service.py b/todo/services/task_service.py index 5dd7b147..d73ab863 100644 --- a/todo/services/task_service.py +++ b/todo/services/task_service.py @@ -424,9 +424,16 @@ def update_task_with_assignee_from_dict(cls, task_id: str, validated_data: dict, if validated_data.get("status") == TaskStatus.IN_PROGRESS and not current_task.startedAt: update_payload["startedAt"] = datetime.now(timezone.utc) - if validated_data.get("status") is not None and current_task.deferredDetails: + if ( + validated_data.get("status") is not None + and validated_data.get("status") != TaskStatus.DEFERRED.value + and current_task.deferredDetails + ): update_payload["deferredDetails"] = None + if validated_data.get("status") == TaskStatus.DEFERRED.value: + update_payload["status"] = current_task.status + # Update task if there are changes if update_payload: update_payload["updatedBy"] = user_id From b0ff4108849ba8d4a7ce9d35d24974f3c8bcfd7e Mon Sep 17 00:00:00 2001 From: "anuj.k" Date: Sun, 3 Aug 2025 00:13:26 +0530 Subject: [PATCH 7/7] fix: correct comparison operator for deferred task details in TaskRepository --- todo/repositories/task_repository.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/todo/repositories/task_repository.py b/todo/repositories/task_repository.py index ca93dfd1..06f0fc4c 100644 --- a/todo/repositories/task_repository.py +++ b/todo/repositories/task_repository.py @@ -31,7 +31,7 @@ def _build_status_filter(cls, status_filter: str = None) -> dict: return { "$or": [ {"deferredDetails": None}, - {"deferredDetails.deferredTill": {"$lte": now}}, + {"deferredDetails.deferredTill": {"$lt": now}}, ] } @@ -42,7 +42,7 @@ def _build_status_filter(cls, status_filter: str = None) -> dict: { "$or": [ {"deferredDetails": None}, - {"deferredDetails.deferredTill": {"$lte": now}}, + {"deferredDetails.deferredTill": {"$lt": now}}, ] }, ]