From 11f5f8638cb72cbec9247809e9e0d3883f6e6756 Mon Sep 17 00:00:00 2001 From: Achintya-Chatterjee Date: Wed, 10 Dec 2025 00:17:15 +0530 Subject: [PATCH 1/6] feat(tasks): support assignee filtering in team listings - extend fetch serializer and OpenAPI docs to accept repeated assigneeId parameters - validate assignee membership in TaskListView and forward normalized IDs to TaskService - teach TaskRepository list/count to intersect team and assignee scopes correctly while honoring creator visibility - add unit coverage across serializer, view, service, and repository for new filtering paths - document curl verification of /v1/tasks with team-scoped assignee filters --- todo/repositories/task_repository.py | 118 +++++++++-- todo/serializers/get_tasks_serializer.py | 28 +++ todo/services/task_service.py | 14 +- .../unit/repositories/test_task_repository.py | 193 ++++++++++++++++++ .../serializers/test_get_tasks_serializer.py | 19 +- todo/tests/unit/services/test_task_service.py | 111 +++++++++- todo/tests/unit/views/test_task.py | 54 +++++ todo/views/task.py | 24 +++ 8 files changed, 535 insertions(+), 26 deletions(-) diff --git a/todo/repositories/task_repository.py b/todo/repositories/task_repository.py index 50d5cc3c..a56dd7fd 100644 --- a/todo/repositories/task_repository.py +++ b/todo/repositories/task_repository.py @@ -29,6 +29,51 @@ def _get_team_task_ids(cls, team_id: str) -> List[ObjectId]: team_task_ids = [ObjectId(task["task_id"]) for task in team_tasks] return list(set(team_task_ids)) + @classmethod + def _get_task_ids_for_assignees(cls, assignee_ids: List[str], team_id: str | None = None) -> List[ObjectId]: + """ + Resolve active task IDs for the provided assignee IDs, optionally scoped to a team. + """ + if not assignee_ids: + return [] + + candidate_values = set() + for assignee_id in assignee_ids: + candidate_values.add(assignee_id) + if ObjectId.is_valid(assignee_id): + candidate_values.add(ObjectId(assignee_id)) + + if not candidate_values: + return [] + + assignment_collection = TaskAssignmentRepository.get_collection() + assignment_filter: dict = { + "assignee_id": {"$in": list(candidate_values)}, + "user_type": "user", + "is_active": True, + } + + if team_id: + team_candidates = {team_id} + if ObjectId.is_valid(team_id): + team_candidates.add(ObjectId(team_id)) + assignment_filter["team_id"] = {"$in": list(team_candidates)} + + assignments = assignment_collection.find( + assignment_filter, + {"task_id": 1}, + ) + + task_ids: set[ObjectId] = set() + for assignment in assignments: + task_identifier = assignment.get("task_id") + if isinstance(task_identifier, ObjectId): + task_ids.add(task_identifier) + elif isinstance(task_identifier, str) and ObjectId.is_valid(task_identifier): + task_ids.add(ObjectId(task_identifier)) + + return list(task_ids) + @classmethod def _build_status_filter(cls, status_filter: str = None) -> dict: now = datetime.now(timezone.utc) @@ -72,19 +117,41 @@ def list( user_id: str = None, team_id: str = None, status_filter: str = None, + assignee_ids: List[str] | None = None, ) -> List[TaskModel]: tasks_collection = cls.get_collection() base_filter = cls._build_status_filter(status_filter) - if team_id: + filters = [base_filter] + + team_scope_applied = False + + if assignee_ids: + assignee_task_ids = cls._get_task_ids_for_assignees(assignee_ids, team_id=team_id) + if not assignee_task_ids: + return [] + filters.append({"_id": {"$in": assignee_task_ids}}) + if team_id: + team_scope_applied = True + elif team_id: all_team_task_ids = cls._get_team_task_ids(team_id) - query_filter = {"$and": [base_filter, {"_id": {"$in": all_team_task_ids}}]} - elif user_id: + if not all_team_task_ids: + return [] + filters.append({"_id": {"$in": all_team_task_ids}}) + team_scope_applied = True + + if user_id and not team_scope_applied: assigned_task_ids = cls._get_assigned_task_ids_for_user(user_id) - query_filter = {"$and": [base_filter, {"_id": {"$in": assigned_task_ids}}]} + user_filters = [{"createdBy": user_id}] + if assigned_task_ids: + user_filters.append({"_id": {"$in": assigned_task_ids}}) + filters.append({"$or": user_filters}) + + if len(filters) == 1: + query_filter = filters[0] else: - query_filter = base_filter + query_filter = {"$and": filters} if sort_by == SORT_FIELD_UPDATED_AT: sort_direction = -1 if order == SORT_ORDER_DESC else 1 @@ -149,22 +216,47 @@ def _get_assigned_task_ids_for_user(cls, user_id: str) -> List[ObjectId]: return direct_task_ids + team_task_ids @classmethod - def count(cls, user_id: str = None, team_id: str = None, status_filter: str = None) -> int: + def count( + cls, + user_id: str = None, + team_id: str = None, + status_filter: str = None, + assignee_ids: List[str] | None = None, + ) -> int: tasks_collection = cls.get_collection() base_filter = cls._build_status_filter(status_filter) - if team_id: + filters = [base_filter] + + team_scope_applied = False + + if assignee_ids: + assignee_task_ids = cls._get_task_ids_for_assignees(assignee_ids, team_id=team_id) + if not assignee_task_ids: + return 0 + filters.append({"_id": {"$in": assignee_task_ids}}) + if team_id: + team_scope_applied = True + elif team_id: all_team_task_ids = cls._get_team_task_ids(team_id) - query_filter = {"$and": [base_filter, {"_id": {"$in": all_team_task_ids}}]} + if not all_team_task_ids: + return 0 + filters.append({"_id": {"$in": all_team_task_ids}}) + team_scope_applied = True - elif user_id: + if user_id and not team_scope_applied: assigned_task_ids = cls._get_assigned_task_ids_for_user(user_id) - query_filter = { - "$and": [base_filter, {"$or": [{"createdBy": user_id}, {"_id": {"$in": assigned_task_ids}}]}] - } + user_filters = [{"createdBy": user_id}] + if assigned_task_ids: + user_filters.append({"_id": {"$in": assigned_task_ids}}) + filters.append({"$or": user_filters}) + + if len(filters) == 1: + query_filter = filters[0] else: - query_filter = base_filter + query_filter = {"$and": filters} + return tasks_collection.count_documents(query_filter) @classmethod diff --git a/todo/serializers/get_tasks_serializer.py b/todo/serializers/get_tasks_serializer.py index 16f28f7e..6f1a0e18 100644 --- a/todo/serializers/get_tasks_serializer.py +++ b/todo/serializers/get_tasks_serializer.py @@ -11,6 +11,19 @@ def to_internal_value(self, data): return super().to_internal_value(data) +class QueryParameterListField(serializers.ListField): + """ + DRF list field that understands QueryDict inputs with repeated parameters. + """ + + def get_value(self, dictionary): + if hasattr(dictionary, "getlist") and self.field_name in dictionary: + values = dictionary.getlist(self.field_name) + if values: + return values + return super().get_value(dictionary) + + class GetTaskQueryParamsSerializer(serializers.Serializer): page = serializers.IntegerField( required=False, @@ -44,6 +57,11 @@ class GetTaskQueryParamsSerializer(serializers.Serializer): teamId = serializers.CharField(required=False, allow_blank=False, allow_null=True) + assigneeId = QueryParameterListField( + child=serializers.CharField(allow_blank=False), + required=False, + ) + status = CaseInsensitiveChoiceField( choices=[status.value for status in TaskStatus], required=False, @@ -57,4 +75,14 @@ def validate(self, attrs): sort_by = validated_data.get("sort_by", SORT_FIELD_UPDATED_AT) validated_data["order"] = SORT_FIELD_DEFAULT_ORDERS[sort_by] + assignee_ids = validated_data.pop("assigneeId", None) + if assignee_ids is not None: + seen = set() + normalized_ids = [] + for assignee_id in assignee_ids: + if assignee_id not in seen: + normalized_ids.append(assignee_id) + seen.add(assignee_id) + validated_data["assignee_ids"] = normalized_ids + return validated_data diff --git a/todo/services/task_service.py b/todo/services/task_service.py index 248a7e5d..1cc07413 100644 --- a/todo/services/task_service.py +++ b/todo/services/task_service.py @@ -74,6 +74,7 @@ def get_tasks( user_id: str, team_id: str = None, status_filter: str = None, + assignee_ids: List[str] | None = None, ) -> GetTasksResponse: try: cls._validate_pagination_params(page, limit) @@ -93,9 +94,18 @@ def get_tasks( ) tasks = TaskRepository.list( - page, limit, sort_by, order, user_id, team_id=team_id, status_filter=status_filter + page, + limit, + sort_by, + order, + user_id, + team_id=team_id, + status_filter=status_filter, + assignee_ids=assignee_ids, + ) + total_count = TaskRepository.count( + user_id, team_id=team_id, status_filter=status_filter, assignee_ids=assignee_ids ) - total_count = TaskRepository.count(user_id, team_id=team_id, status_filter=status_filter) if not tasks: return GetTasksResponse(tasks=[], links=None) diff --git a/todo/tests/unit/repositories/test_task_repository.py b/todo/tests/unit/repositories/test_task_repository.py index f24029a1..0758efa5 100644 --- a/todo/tests/unit/repositories/test_task_repository.py +++ b/todo/tests/unit/repositories/test_task_repository.py @@ -91,6 +91,117 @@ def test_list_returns_empty_list_for_no_tasks(self): self.mock_collection.find.return_value.sort.return_value.skip.assert_called_once_with(10) self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.assert_called_once_with(10) + @patch.object(TaskRepository, "_get_task_ids_for_assignees", return_value=[]) + def test_list_returns_empty_when_assignee_filter_has_no_matches(self, mock_get_task_ids): + result = TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id=None, + assignee_ids=["user1"], + ) + + self.assertEqual(result, []) + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.find.assert_not_called() + + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_list_filters_by_assignee_ids(self, mock_get_task_ids): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id=None, + assignee_ids=["user1"], + ) + + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.find.assert_called_once() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) + ) + + @patch.object(TaskRepository, "_get_assigned_task_ids_for_user", return_value=[]) + def test_list_includes_created_tasks_when_user_has_no_assignments(self, mock_get_assigned): + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + user_id = "user_creator" + TaskRepository.list(1, 10, sort_by="createdAt", order="desc", user_id=user_id) + + self.mock_collection.find.assert_called_once() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any("$or" in condition and {"createdBy": user_id} in condition["$or"] for condition in query_filter["$and"]) + ) + mock_get_assigned.assert_called_once_with(user_id) + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[]) + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_list_with_team_and_assignee_ids_relies_on_assignee_tasks(self, mock_get_task_ids, mock_get_team_tasks): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id=None, + team_id="team123", + assignee_ids=["user1"], + ) + + mock_get_team_tasks.assert_not_called() # team filtering is handled by assignee helper when both provided + mock_get_task_ids.assert_called_once_with(["user1"], team_id="team123") + self.mock_collection.find.assert_called_once() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) + ) + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[ObjectId()]) + @patch.object(TaskRepository, "_get_assigned_task_ids_for_user") + def test_list_team_filter_skips_user_filter(self, mock_get_assigned, mock_get_team_tasks): + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id="user123", + team_id="team123", + ) + + mock_get_team_tasks.assert_called_once() + mock_get_assigned.assert_not_called() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + # Ensure no user OR clause present + for condition in query_filter["$and"]: + self.assertFalse("$or" in condition and {"createdBy": "user123"} in condition["$or"]) + def test_count_returns_total_task_count(self): self.mock_collection.count_documents.return_value = 42 @@ -122,6 +233,88 @@ def test_get_all_returns_empty_list_for_no_tasks(self): self.assertEqual(result, []) self.mock_collection.find.assert_called_once() + @patch("todo.repositories.task_repository.TaskAssignmentRepository.get_collection") + def test_get_task_ids_for_assignees_returns_object_ids(self, mock_get_collection): + mock_collection = MagicMock() + task_id_obj = ObjectId() + task_id_str = str(ObjectId()) + mock_collection.find.return_value = [{"task_id": task_id_obj}, {"task_id": task_id_str}] + mock_get_collection.return_value = mock_collection + + result = TaskRepository._get_task_ids_for_assignees([str(ObjectId()), "not-an-object-id"]) + + self.assertEqual( + set(result), + {task_id_obj, ObjectId(task_id_str)}, + ) + mock_collection.find.assert_called_once() + + @patch("todo.repositories.task_repository.TaskAssignmentRepository.get_collection") + def test_get_task_ids_for_assignees_filters_by_team(self, mock_get_collection): + mock_collection = MagicMock() + mock_collection.find.return_value = [{"task_id": ObjectId()}] + mock_get_collection.return_value = mock_collection + + team_id = str(ObjectId()) + TaskRepository._get_task_ids_for_assignees(["user1"], team_id=team_id) + + assignment_filter = mock_collection.find.call_args[0][0] + self.assertIn("team_id", assignment_filter) + values = assignment_filter["team_id"]["$in"] + self.assertIn(team_id, values) + self.assertTrue(any(ObjectId.is_valid(v) for v in values)) + + @patch.object(TaskRepository, "_get_task_ids_for_assignees", return_value=[]) + def test_count_returns_zero_when_assignee_filter_has_no_matches(self, mock_get_task_ids): + result = TaskRepository.count(assignee_ids=["user1"]) + + self.assertEqual(result, 0) + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.count_documents.assert_not_called() + + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_count_filters_by_assignee_ids(self, mock_get_task_ids): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + self.mock_collection.count_documents.return_value = 5 + + TaskRepository.count(assignee_ids=["user1"]) + + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.count_documents.assert_called_once() + query_filter = self.mock_collection.count_documents.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) + ) + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[]) + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_count_with_team_and_assignee_ids_relies_on_assignee_tasks(self, mock_get_task_ids, mock_get_team_tasks): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + self.mock_collection.count_documents.return_value = 1 + + TaskRepository.count(team_id="team123", assignee_ids=["user1"]) + + mock_get_team_tasks.assert_not_called() + mock_get_task_ids.assert_called_once_with(["user1"], team_id="team123") + self.mock_collection.count_documents.assert_called_once() + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[ObjectId()]) + @patch.object(TaskRepository, "_get_assigned_task_ids_for_user") + def test_count_team_filter_skips_user_filter(self, mock_get_assigned, mock_get_team_tasks): + self.mock_collection.count_documents.return_value = 1 + + TaskRepository.count(team_id="team123", user_id="user123") + + mock_get_team_tasks.assert_called_once() + mock_get_assigned.assert_not_called() + query_filter = self.mock_collection.count_documents.call_args[0][0] + self.assertIn("$and", query_filter) + for condition in query_filter["$and"]: + self.assertFalse("$or" in condition and {"createdBy": "user123"} in condition["$or"]) + def test_get_by_id_returns_task_model_when_found(self): task_id_str = str(self.task_db_data_fixture["_id"]) self.mock_collection.find_one.return_value = self.task_db_data_fixture diff --git a/todo/tests/unit/serializers/test_get_tasks_serializer.py b/todo/tests/unit/serializers/test_get_tasks_serializer.py index c2423dbf..9afbce3f 100644 --- a/todo/tests/unit/serializers/test_get_tasks_serializer.py +++ b/todo/tests/unit/serializers/test_get_tasks_serializer.py @@ -1,6 +1,7 @@ from unittest import TestCase -from rest_framework.exceptions import ValidationError from django.conf import settings +from django.http import QueryDict +from rest_framework.exceptions import ValidationError from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer from todo.constants.task import ( @@ -71,6 +72,22 @@ def test_serializer_ignores_undefined_extra_fields(self): self.assertEqual(serializer.validated_data["limit"], 5) self.assertNotIn("extra_field", serializer.validated_data) + def test_serializer_collects_assignee_ids_from_querydict(self): + query_params = QueryDict(mutable=True) + query_params.setlist("assigneeId", ["user1", "user2"]) + + serializer = GetTaskQueryParamsSerializer(data=query_params) + self.assertTrue(serializer.is_valid()) + self.assertEqual(serializer.validated_data["assignee_ids"], ["user1", "user2"]) + + def test_serializer_deduplicates_assignee_ids(self): + query_params = QueryDict(mutable=True) + query_params.setlist("assigneeId", ["user1", "user1", "user2"]) + + serializer = GetTaskQueryParamsSerializer(data=query_params) + self.assertTrue(serializer.is_valid()) + self.assertEqual(serializer.validated_data["assignee_ids"], ["user1", "user2"]) + def test_serializer_uses_django_settings_values(self): """Test that the serializer correctly uses values from Django settings""" # Instead of mocking, we'll test against the actual settings values diff --git a/todo/tests/unit/services/test_task_service.py b/todo/tests/unit/services/test_task_service.py index e15c98be..49185758 100644 --- a/todo/tests/unit/services/test_task_service.py +++ b/todo/tests/unit/services/test_task_service.py @@ -72,9 +72,16 @@ def test_get_tasks_returns_paginated_response( ) mock_list.assert_called_once_with( - 2, 1, "createdAt", "desc", str(self.user_id), team_id=None, status_filter=None + 2, + 1, + "createdAt", + "desc", + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) - mock_count.assert_called_once() + mock_count.assert_called_once_with(str(self.user_id), team_id=None, status_filter=None, assignee_ids=None) @patch("todo.services.task_service.UserRepository.get_by_id") @patch("todo.services.task_service.TaskRepository.count") @@ -113,8 +120,50 @@ def test_get_tasks_returns_empty_response_if_no_tasks_present(self, mock_list: M self.assertEqual(len(response.tasks), 0) self.assertIsNone(response.links) - mock_list.assert_called_once_with(1, 10, "createdAt", "desc", "test_user", team_id=None, status_filter=None) - mock_count.assert_called_once() + mock_list.assert_called_once_with( + 1, + 10, + "createdAt", + "desc", + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, + ) + mock_count.assert_called_once_with("test_user", team_id=None, status_filter=None, assignee_ids=None) + + @patch("todo.services.task_service.TaskRepository.count") + @patch("todo.services.task_service.TaskRepository.list") + def test_get_tasks_passes_assignee_ids_to_repo(self, mock_list: Mock, mock_count: Mock): + mock_list.return_value = [] + mock_count.return_value = 0 + + TaskService.get_tasks( + page=1, + limit=10, + sort_by="createdAt", + order="desc", + user_id="request_user", + team_id="team123", + assignee_ids=["user1", "user2"], + ) + + mock_list.assert_called_once_with( + 1, + 10, + "createdAt", + "desc", + "request_user", + team_id="team123", + status_filter=None, + assignee_ids=["user1", "user2"], + ) + mock_count.assert_called_once_with( + "request_user", + team_id="team123", + status_filter=None, + assignee_ids=["user1", "user2"], + ) @patch("todo.services.task_service.TaskRepository.count") @patch("todo.services.task_service.TaskRepository.list") @@ -434,7 +483,14 @@ def test_get_tasks_default_sorting(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by="createdAt", order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_CREATED_AT, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -446,7 +502,14 @@ def test_get_tasks_explicit_sort_by_priority(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order=SORT_ORDER_DESC, user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_PRIORITY, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -458,7 +521,14 @@ def test_get_tasks_sort_by_due_at_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_DUE_AT, order="asc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_DUE_AT, + SORT_ORDER_ASC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -470,7 +540,14 @@ def test_get_tasks_sort_by_priority_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_PRIORITY, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -482,7 +559,14 @@ def test_get_tasks_sort_by_assignee_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_ASSIGNEE, order="asc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_ASSIGNEE, + SORT_ORDER_ASC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -494,7 +578,14 @@ def test_get_tasks_sort_by_created_at_default_order(self, mock_list, mock_count) TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_CREATED_AT, order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_CREATED_AT, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.reverse_lazy", return_value="/v1/tasks") diff --git a/todo/tests/unit/views/test_task.py b/todo/tests/unit/views/test_task.py index db780734..5a812ea2 100644 --- a/todo/tests/unit/views/test_task.py +++ b/todo/tests/unit/views/test_task.py @@ -53,6 +53,7 @@ def test_get_tasks_returns_200_for_valid_params(self, mock_get_tasks: Mock): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) self.assertEqual(response.status_code, status.HTTP_200_OK) expected_response = mock_get_tasks.return_value.model_dump(mode="json") @@ -72,6 +73,7 @@ def test_get_tasks_returns_200_without_params(self, mock_get_tasks: Mock): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -101,6 +103,49 @@ def test_get_tasks_returns_400_for_invalid_query_params(self): self.assertEqual(actual_error["source"]["parameter"], expected_error["source"]["parameter"]) self.assertEqual(actual_error["detail"], expected_error["detail"]) + def test_get_tasks_requires_team_for_assignee_filter(self): + response = self.client.get(self.url, {"assigneeId": str(ObjectId())}) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + error_sources = [error["source"].get("parameter") for error in response.data.get("errors", [])] + self.assertIn("teamId", error_sources) + + @patch("todo.views.task.UserTeamDetailsRepository.get_users_by_team_id", return_value=["user1"]) + def test_get_tasks_rejects_assignee_not_in_team(self, mock_get_team_members: Mock): + team_id = str(ObjectId()) + assignee_id = str(ObjectId()) + + response = self.client.get(self.url, {"teamId": team_id, "assigneeId": assignee_id}) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("errors", response.data) + self.assertTrue( + any(ValidationErrors.USER_NOT_TEAM_MEMBER in error.get("detail", "") for error in response.data["errors"]) + ) + mock_get_team_members.assert_called_once_with(team_id) + + @patch("todo.views.task.UserTeamDetailsRepository.get_users_by_team_id", return_value=["user1", "user2"]) + @patch("todo.services.task_service.TaskService.get_tasks") + def test_get_tasks_with_assignee_filter_passes_ids(self, mock_get_tasks: Mock, mock_get_team_members: Mock): + mock_get_tasks.return_value = GetTasksResponse(tasks=task_dtos) + team_id = str(ObjectId()) + params = {"teamId": team_id, "page": 1, "limit": 10, "assigneeId": ["user1", "user2"]} + + response = self.client.get(self.url, params) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_get_team_members.assert_called_once_with(team_id) + mock_get_tasks.assert_called_once_with( + page=1, + limit=10, + sort_by="updatedAt", + order="desc", + user_id=str(self.user_id), + team_id=team_id, + status_filter=None, + assignee_ids=["user1", "user2"], + ) + @patch("todo.services.task_service.TaskService.get_task_by_id") def test_get_single_task_success(self, mock_get_task_by_id: Mock): valid_task_id = str(ObjectId()) @@ -183,6 +228,7 @@ def test_get_tasks_with_default_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -201,6 +247,7 @@ def test_get_tasks_with_valid_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) def test_get_tasks_with_invalid_page(self): @@ -249,6 +296,7 @@ def test_get_tasks_with_sort_by_priority(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -266,6 +314,7 @@ def test_get_tasks_with_sort_by_and_order(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -295,6 +344,7 @@ def test_get_tasks_with_all_sort_fields(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -318,6 +368,7 @@ def test_get_tasks_with_all_order_values(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) def test_get_tasks_with_invalid_sort_by(self): @@ -353,6 +404,7 @@ def test_get_tasks_sorting_with_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -370,6 +422,7 @@ def test_get_tasks_default_behavior_unchanged(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) def test_get_tasks_edge_case_combinations(self): @@ -387,6 +440,7 @@ def test_get_tasks_edge_case_combinations(self): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) diff --git a/todo/views/task.py b/todo/views/task.py index 343e5cff..81d4ec48 100644 --- a/todo/views/task.py +++ b/todo/views/task.py @@ -28,6 +28,7 @@ from todo.dto.responses.create_task_assignment_response import CreateTaskAssignmentResponse from todo.dto.task_assignment_dto import CreateTaskAssignmentDTO from todo.exceptions.task_exceptions import TaskNotFoundException +from todo.repositories.team_repository import UserTeamDetailsRepository class TaskListView(APIView): @@ -63,6 +64,14 @@ class TaskListView(APIView): description="If provided, filters tasks by status (e.g., 'DONE', 'IN_PROGRESS', 'TODO', 'BLOCKED', 'DEFERRED').", required=False, ), + OpenApiParameter( + name="assigneeId", + type=OpenApiTypes.STR, + location=OpenApiParameter.QUERY, + description="Repeatable parameter that filters tasks assigned to the provided user IDs.", + required=False, + many=True, + ), ], responses={ 200: OpenApiResponse(response=GetTasksResponse, description="Successful response"), @@ -100,6 +109,20 @@ def get(self, request: Request): team_id = query.validated_data.get("teamId") status_filter = query.validated_data.get("status") + assignee_ids = query.validated_data.get("assignee_ids") + + if assignee_ids: + if not team_id: + raise ValidationError({"teamId": ["teamId is required when filtering by assigneeId."]}) + + team_members = set(UserTeamDetailsRepository.get_users_by_team_id(team_id)) + invalid_assignees = [assignee_id for assignee_id in assignee_ids if assignee_id not in team_members] + + if invalid_assignees: + raise ValidationError( + {"assigneeId": [f"{ValidationErrors.USER_NOT_TEAM_MEMBER}: {', '.join(invalid_assignees)}"]} + ) + response = TaskService.get_tasks( page=query.validated_data["page"], limit=query.validated_data["limit"], @@ -108,6 +131,7 @@ def get(self, request: Request): user_id=request.user_id, team_id=team_id, status_filter=status_filter, + assignee_ids=assignee_ids, ) if response.error and response.error.get("code") == "FORBIDDEN": From 2a1ee4adfb46bbb66a150c735942ce80a670a6b3 Mon Sep 17 00:00:00 2001 From: Achintya-Chatterjee Date: Wed, 10 Dec 2025 00:31:32 +0530 Subject: [PATCH 2/6] chore: remove testing codes from here as I have break the PR into two parts --- .../unit/repositories/test_task_repository.py | 193 ------------------ .../serializers/test_get_tasks_serializer.py | 19 +- todo/tests/unit/services/test_task_service.py | 111 +--------- todo/tests/unit/views/test_task.py | 54 ----- 4 files changed, 11 insertions(+), 366 deletions(-) diff --git a/todo/tests/unit/repositories/test_task_repository.py b/todo/tests/unit/repositories/test_task_repository.py index 0758efa5..f24029a1 100644 --- a/todo/tests/unit/repositories/test_task_repository.py +++ b/todo/tests/unit/repositories/test_task_repository.py @@ -91,117 +91,6 @@ def test_list_returns_empty_list_for_no_tasks(self): self.mock_collection.find.return_value.sort.return_value.skip.assert_called_once_with(10) self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.assert_called_once_with(10) - @patch.object(TaskRepository, "_get_task_ids_for_assignees", return_value=[]) - def test_list_returns_empty_when_assignee_filter_has_no_matches(self, mock_get_task_ids): - result = TaskRepository.list( - 1, - 10, - sort_by=SORT_FIELD_CREATED_AT, - order=SORT_ORDER_DESC, - user_id=None, - assignee_ids=["user1"], - ) - - self.assertEqual(result, []) - mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) - self.mock_collection.find.assert_not_called() - - @patch.object(TaskRepository, "_get_task_ids_for_assignees") - def test_list_filters_by_assignee_ids(self, mock_get_task_ids): - assignee_task_id = ObjectId() - mock_get_task_ids.return_value = [assignee_task_id] - - mock_cursor = MagicMock() - mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) - self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor - - TaskRepository.list( - 1, - 10, - sort_by=SORT_FIELD_CREATED_AT, - order=SORT_ORDER_DESC, - user_id=None, - assignee_ids=["user1"], - ) - - mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) - self.mock_collection.find.assert_called_once() - query_filter = self.mock_collection.find.call_args[0][0] - self.assertIn("$and", query_filter) - self.assertTrue( - any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) - ) - - @patch.object(TaskRepository, "_get_assigned_task_ids_for_user", return_value=[]) - def test_list_includes_created_tasks_when_user_has_no_assignments(self, mock_get_assigned): - mock_cursor = MagicMock() - mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) - self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor - - user_id = "user_creator" - TaskRepository.list(1, 10, sort_by="createdAt", order="desc", user_id=user_id) - - self.mock_collection.find.assert_called_once() - query_filter = self.mock_collection.find.call_args[0][0] - self.assertIn("$and", query_filter) - self.assertTrue( - any("$or" in condition and {"createdBy": user_id} in condition["$or"] for condition in query_filter["$and"]) - ) - mock_get_assigned.assert_called_once_with(user_id) - - @patch.object(TaskRepository, "_get_team_task_ids", return_value=[]) - @patch.object(TaskRepository, "_get_task_ids_for_assignees") - def test_list_with_team_and_assignee_ids_relies_on_assignee_tasks(self, mock_get_task_ids, mock_get_team_tasks): - assignee_task_id = ObjectId() - mock_get_task_ids.return_value = [assignee_task_id] - - mock_cursor = MagicMock() - mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) - self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor - - TaskRepository.list( - 1, - 10, - sort_by=SORT_FIELD_CREATED_AT, - order=SORT_ORDER_DESC, - user_id=None, - team_id="team123", - assignee_ids=["user1"], - ) - - mock_get_team_tasks.assert_not_called() # team filtering is handled by assignee helper when both provided - mock_get_task_ids.assert_called_once_with(["user1"], team_id="team123") - self.mock_collection.find.assert_called_once() - query_filter = self.mock_collection.find.call_args[0][0] - self.assertIn("$and", query_filter) - self.assertTrue( - any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) - ) - - @patch.object(TaskRepository, "_get_team_task_ids", return_value=[ObjectId()]) - @patch.object(TaskRepository, "_get_assigned_task_ids_for_user") - def test_list_team_filter_skips_user_filter(self, mock_get_assigned, mock_get_team_tasks): - mock_cursor = MagicMock() - mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) - self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor - - TaskRepository.list( - 1, - 10, - sort_by=SORT_FIELD_CREATED_AT, - order=SORT_ORDER_DESC, - user_id="user123", - team_id="team123", - ) - - mock_get_team_tasks.assert_called_once() - mock_get_assigned.assert_not_called() - query_filter = self.mock_collection.find.call_args[0][0] - self.assertIn("$and", query_filter) - # Ensure no user OR clause present - for condition in query_filter["$and"]: - self.assertFalse("$or" in condition and {"createdBy": "user123"} in condition["$or"]) - def test_count_returns_total_task_count(self): self.mock_collection.count_documents.return_value = 42 @@ -233,88 +122,6 @@ def test_get_all_returns_empty_list_for_no_tasks(self): self.assertEqual(result, []) self.mock_collection.find.assert_called_once() - @patch("todo.repositories.task_repository.TaskAssignmentRepository.get_collection") - def test_get_task_ids_for_assignees_returns_object_ids(self, mock_get_collection): - mock_collection = MagicMock() - task_id_obj = ObjectId() - task_id_str = str(ObjectId()) - mock_collection.find.return_value = [{"task_id": task_id_obj}, {"task_id": task_id_str}] - mock_get_collection.return_value = mock_collection - - result = TaskRepository._get_task_ids_for_assignees([str(ObjectId()), "not-an-object-id"]) - - self.assertEqual( - set(result), - {task_id_obj, ObjectId(task_id_str)}, - ) - mock_collection.find.assert_called_once() - - @patch("todo.repositories.task_repository.TaskAssignmentRepository.get_collection") - def test_get_task_ids_for_assignees_filters_by_team(self, mock_get_collection): - mock_collection = MagicMock() - mock_collection.find.return_value = [{"task_id": ObjectId()}] - mock_get_collection.return_value = mock_collection - - team_id = str(ObjectId()) - TaskRepository._get_task_ids_for_assignees(["user1"], team_id=team_id) - - assignment_filter = mock_collection.find.call_args[0][0] - self.assertIn("team_id", assignment_filter) - values = assignment_filter["team_id"]["$in"] - self.assertIn(team_id, values) - self.assertTrue(any(ObjectId.is_valid(v) for v in values)) - - @patch.object(TaskRepository, "_get_task_ids_for_assignees", return_value=[]) - def test_count_returns_zero_when_assignee_filter_has_no_matches(self, mock_get_task_ids): - result = TaskRepository.count(assignee_ids=["user1"]) - - self.assertEqual(result, 0) - mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) - self.mock_collection.count_documents.assert_not_called() - - @patch.object(TaskRepository, "_get_task_ids_for_assignees") - def test_count_filters_by_assignee_ids(self, mock_get_task_ids): - assignee_task_id = ObjectId() - mock_get_task_ids.return_value = [assignee_task_id] - self.mock_collection.count_documents.return_value = 5 - - TaskRepository.count(assignee_ids=["user1"]) - - mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) - self.mock_collection.count_documents.assert_called_once() - query_filter = self.mock_collection.count_documents.call_args[0][0] - self.assertIn("$and", query_filter) - self.assertTrue( - any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) - ) - - @patch.object(TaskRepository, "_get_team_task_ids", return_value=[]) - @patch.object(TaskRepository, "_get_task_ids_for_assignees") - def test_count_with_team_and_assignee_ids_relies_on_assignee_tasks(self, mock_get_task_ids, mock_get_team_tasks): - assignee_task_id = ObjectId() - mock_get_task_ids.return_value = [assignee_task_id] - self.mock_collection.count_documents.return_value = 1 - - TaskRepository.count(team_id="team123", assignee_ids=["user1"]) - - mock_get_team_tasks.assert_not_called() - mock_get_task_ids.assert_called_once_with(["user1"], team_id="team123") - self.mock_collection.count_documents.assert_called_once() - - @patch.object(TaskRepository, "_get_team_task_ids", return_value=[ObjectId()]) - @patch.object(TaskRepository, "_get_assigned_task_ids_for_user") - def test_count_team_filter_skips_user_filter(self, mock_get_assigned, mock_get_team_tasks): - self.mock_collection.count_documents.return_value = 1 - - TaskRepository.count(team_id="team123", user_id="user123") - - mock_get_team_tasks.assert_called_once() - mock_get_assigned.assert_not_called() - query_filter = self.mock_collection.count_documents.call_args[0][0] - self.assertIn("$and", query_filter) - for condition in query_filter["$and"]: - self.assertFalse("$or" in condition and {"createdBy": "user123"} in condition["$or"]) - def test_get_by_id_returns_task_model_when_found(self): task_id_str = str(self.task_db_data_fixture["_id"]) self.mock_collection.find_one.return_value = self.task_db_data_fixture diff --git a/todo/tests/unit/serializers/test_get_tasks_serializer.py b/todo/tests/unit/serializers/test_get_tasks_serializer.py index 9afbce3f..c2423dbf 100644 --- a/todo/tests/unit/serializers/test_get_tasks_serializer.py +++ b/todo/tests/unit/serializers/test_get_tasks_serializer.py @@ -1,7 +1,6 @@ from unittest import TestCase -from django.conf import settings -from django.http import QueryDict from rest_framework.exceptions import ValidationError +from django.conf import settings from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer from todo.constants.task import ( @@ -72,22 +71,6 @@ def test_serializer_ignores_undefined_extra_fields(self): self.assertEqual(serializer.validated_data["limit"], 5) self.assertNotIn("extra_field", serializer.validated_data) - def test_serializer_collects_assignee_ids_from_querydict(self): - query_params = QueryDict(mutable=True) - query_params.setlist("assigneeId", ["user1", "user2"]) - - serializer = GetTaskQueryParamsSerializer(data=query_params) - self.assertTrue(serializer.is_valid()) - self.assertEqual(serializer.validated_data["assignee_ids"], ["user1", "user2"]) - - def test_serializer_deduplicates_assignee_ids(self): - query_params = QueryDict(mutable=True) - query_params.setlist("assigneeId", ["user1", "user1", "user2"]) - - serializer = GetTaskQueryParamsSerializer(data=query_params) - self.assertTrue(serializer.is_valid()) - self.assertEqual(serializer.validated_data["assignee_ids"], ["user1", "user2"]) - def test_serializer_uses_django_settings_values(self): """Test that the serializer correctly uses values from Django settings""" # Instead of mocking, we'll test against the actual settings values diff --git a/todo/tests/unit/services/test_task_service.py b/todo/tests/unit/services/test_task_service.py index 49185758..e15c98be 100644 --- a/todo/tests/unit/services/test_task_service.py +++ b/todo/tests/unit/services/test_task_service.py @@ -72,16 +72,9 @@ def test_get_tasks_returns_paginated_response( ) mock_list.assert_called_once_with( - 2, - 1, - "createdAt", - "desc", - str(self.user_id), - team_id=None, - status_filter=None, - assignee_ids=None, + 2, 1, "createdAt", "desc", str(self.user_id), team_id=None, status_filter=None ) - mock_count.assert_called_once_with(str(self.user_id), team_id=None, status_filter=None, assignee_ids=None) + mock_count.assert_called_once() @patch("todo.services.task_service.UserRepository.get_by_id") @patch("todo.services.task_service.TaskRepository.count") @@ -120,50 +113,8 @@ def test_get_tasks_returns_empty_response_if_no_tasks_present(self, mock_list: M self.assertEqual(len(response.tasks), 0) self.assertIsNone(response.links) - mock_list.assert_called_once_with( - 1, - 10, - "createdAt", - "desc", - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, - ) - mock_count.assert_called_once_with("test_user", team_id=None, status_filter=None, assignee_ids=None) - - @patch("todo.services.task_service.TaskRepository.count") - @patch("todo.services.task_service.TaskRepository.list") - def test_get_tasks_passes_assignee_ids_to_repo(self, mock_list: Mock, mock_count: Mock): - mock_list.return_value = [] - mock_count.return_value = 0 - - TaskService.get_tasks( - page=1, - limit=10, - sort_by="createdAt", - order="desc", - user_id="request_user", - team_id="team123", - assignee_ids=["user1", "user2"], - ) - - mock_list.assert_called_once_with( - 1, - 10, - "createdAt", - "desc", - "request_user", - team_id="team123", - status_filter=None, - assignee_ids=["user1", "user2"], - ) - mock_count.assert_called_once_with( - "request_user", - team_id="team123", - status_filter=None, - assignee_ids=["user1", "user2"], - ) + mock_list.assert_called_once_with(1, 10, "createdAt", "desc", "test_user", team_id=None, status_filter=None) + mock_count.assert_called_once() @patch("todo.services.task_service.TaskRepository.count") @patch("todo.services.task_service.TaskRepository.list") @@ -483,14 +434,7 @@ def test_get_tasks_default_sorting(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by="createdAt", order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, - 20, - SORT_FIELD_CREATED_AT, - SORT_ORDER_DESC, - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, + 1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None ) @patch("todo.services.task_service.TaskRepository.count") @@ -502,14 +446,7 @@ def test_get_tasks_explicit_sort_by_priority(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order=SORT_ORDER_DESC, user_id="test_user") mock_list.assert_called_once_with( - 1, - 20, - SORT_FIELD_PRIORITY, - SORT_ORDER_DESC, - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, + 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None ) @patch("todo.services.task_service.TaskRepository.count") @@ -521,14 +458,7 @@ def test_get_tasks_sort_by_due_at_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_DUE_AT, order="asc", user_id="test_user") mock_list.assert_called_once_with( - 1, - 20, - SORT_FIELD_DUE_AT, - SORT_ORDER_ASC, - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, + 1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None ) @patch("todo.services.task_service.TaskRepository.count") @@ -540,14 +470,7 @@ def test_get_tasks_sort_by_priority_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, - 20, - SORT_FIELD_PRIORITY, - SORT_ORDER_DESC, - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, + 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None ) @patch("todo.services.task_service.TaskRepository.count") @@ -559,14 +482,7 @@ def test_get_tasks_sort_by_assignee_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_ASSIGNEE, order="asc", user_id="test_user") mock_list.assert_called_once_with( - 1, - 20, - SORT_FIELD_ASSIGNEE, - SORT_ORDER_ASC, - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, + 1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None ) @patch("todo.services.task_service.TaskRepository.count") @@ -578,14 +494,7 @@ def test_get_tasks_sort_by_created_at_default_order(self, mock_list, mock_count) TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_CREATED_AT, order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, - 20, - SORT_FIELD_CREATED_AT, - SORT_ORDER_DESC, - "test_user", - team_id=None, - status_filter=None, - assignee_ids=None, + 1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None ) @patch("todo.services.task_service.reverse_lazy", return_value="/v1/tasks") diff --git a/todo/tests/unit/views/test_task.py b/todo/tests/unit/views/test_task.py index 5a812ea2..db780734 100644 --- a/todo/tests/unit/views/test_task.py +++ b/todo/tests/unit/views/test_task.py @@ -53,7 +53,6 @@ def test_get_tasks_returns_200_for_valid_params(self, mock_get_tasks: Mock): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) self.assertEqual(response.status_code, status.HTTP_200_OK) expected_response = mock_get_tasks.return_value.model_dump(mode="json") @@ -73,7 +72,6 @@ def test_get_tasks_returns_200_without_params(self, mock_get_tasks: Mock): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -103,49 +101,6 @@ def test_get_tasks_returns_400_for_invalid_query_params(self): self.assertEqual(actual_error["source"]["parameter"], expected_error["source"]["parameter"]) self.assertEqual(actual_error["detail"], expected_error["detail"]) - def test_get_tasks_requires_team_for_assignee_filter(self): - response = self.client.get(self.url, {"assigneeId": str(ObjectId())}) - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - error_sources = [error["source"].get("parameter") for error in response.data.get("errors", [])] - self.assertIn("teamId", error_sources) - - @patch("todo.views.task.UserTeamDetailsRepository.get_users_by_team_id", return_value=["user1"]) - def test_get_tasks_rejects_assignee_not_in_team(self, mock_get_team_members: Mock): - team_id = str(ObjectId()) - assignee_id = str(ObjectId()) - - response = self.client.get(self.url, {"teamId": team_id, "assigneeId": assignee_id}) - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertIn("errors", response.data) - self.assertTrue( - any(ValidationErrors.USER_NOT_TEAM_MEMBER in error.get("detail", "") for error in response.data["errors"]) - ) - mock_get_team_members.assert_called_once_with(team_id) - - @patch("todo.views.task.UserTeamDetailsRepository.get_users_by_team_id", return_value=["user1", "user2"]) - @patch("todo.services.task_service.TaskService.get_tasks") - def test_get_tasks_with_assignee_filter_passes_ids(self, mock_get_tasks: Mock, mock_get_team_members: Mock): - mock_get_tasks.return_value = GetTasksResponse(tasks=task_dtos) - team_id = str(ObjectId()) - params = {"teamId": team_id, "page": 1, "limit": 10, "assigneeId": ["user1", "user2"]} - - response = self.client.get(self.url, params) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - mock_get_team_members.assert_called_once_with(team_id) - mock_get_tasks.assert_called_once_with( - page=1, - limit=10, - sort_by="updatedAt", - order="desc", - user_id=str(self.user_id), - team_id=team_id, - status_filter=None, - assignee_ids=["user1", "user2"], - ) - @patch("todo.services.task_service.TaskService.get_task_by_id") def test_get_single_task_success(self, mock_get_task_by_id: Mock): valid_task_id = str(ObjectId()) @@ -228,7 +183,6 @@ def test_get_tasks_with_default_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -247,7 +201,6 @@ def test_get_tasks_with_valid_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) def test_get_tasks_with_invalid_page(self): @@ -296,7 +249,6 @@ def test_get_tasks_with_sort_by_priority(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -314,7 +266,6 @@ def test_get_tasks_with_sort_by_and_order(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -344,7 +295,6 @@ def test_get_tasks_with_all_sort_fields(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -368,7 +318,6 @@ def test_get_tasks_with_all_order_values(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) def test_get_tasks_with_invalid_sort_by(self): @@ -404,7 +353,6 @@ def test_get_tasks_sorting_with_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -422,7 +370,6 @@ def test_get_tasks_default_behavior_unchanged(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) def test_get_tasks_edge_case_combinations(self): @@ -440,7 +387,6 @@ def test_get_tasks_edge_case_combinations(self): user_id=str(self.user_id), team_id=None, status_filter=None, - assignee_ids=None, ) From 697b2378595d7ac276374b3f33ea192d58ad0ab8 Mon Sep 17 00:00:00 2001 From: Achintya-Chatterjee Date: Wed, 10 Dec 2025 22:15:16 +0530 Subject: [PATCH 3/6] feat(tasks): support assignee filtering in team listings - accept repeated assigneeId query params and normalize them - require valid team membership before passing assignee filters downstream - thread assignee_ids through TaskService and TaskRepository list/count - add coverage for team-scoped filtering paths across view/service/repo - centralize the teamId missing validation message for consistency --- todo/constants/messages.py | 1 + todo/serializers/get_tasks_serializer.py | 8 +------- todo/views/task.py | 4 +++- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/todo/constants/messages.py b/todo/constants/messages.py index 226a3159..b0bf7693 100644 --- a/todo/constants/messages.py +++ b/todo/constants/messages.py @@ -81,6 +81,7 @@ class ValidationErrors: MISSING_EMAIL = "Email is required" MISSING_NAME = "Name is required" MISSING_PICTURE = "Picture is required" + TEAM_ID_REQUIRED_FOR_ASSIGNEE_FILTER = "teamId is required when filtering by assigneeId." SEARCH_QUERY_EMPTY = "Search query cannot be empty" TASK_ID_STRING_REQUIRED = "Task ID must be a string." INVALID_IS_ACTIVE_VALUE = "Invalid value for is_active" diff --git a/todo/serializers/get_tasks_serializer.py b/todo/serializers/get_tasks_serializer.py index 6f1a0e18..960bda55 100644 --- a/todo/serializers/get_tasks_serializer.py +++ b/todo/serializers/get_tasks_serializer.py @@ -77,12 +77,6 @@ def validate(self, attrs): assignee_ids = validated_data.pop("assigneeId", None) if assignee_ids is not None: - seen = set() - normalized_ids = [] - for assignee_id in assignee_ids: - if assignee_id not in seen: - normalized_ids.append(assignee_id) - seen.add(assignee_id) - validated_data["assignee_ids"] = normalized_ids + validated_data["assignee_ids"] = list(dict.fromkeys(assignee_ids)) return validated_data diff --git a/todo/views/task.py b/todo/views/task.py index 81d4ec48..f23d02f9 100644 --- a/todo/views/task.py +++ b/todo/views/task.py @@ -113,7 +113,9 @@ def get(self, request: Request): if assignee_ids: if not team_id: - raise ValidationError({"teamId": ["teamId is required when filtering by assigneeId."]}) + raise ValidationError( + {"teamId": [ValidationErrors.TEAM_ID_REQUIRED_FOR_ASSIGNEE_FILTER]} + ) team_members = set(UserTeamDetailsRepository.get_users_by_team_id(team_id)) invalid_assignees = [assignee_id for assignee_id in assignee_ids if assignee_id not in team_members] From 9297e8af2193b3db2c29233e9625bc23450b3935 Mon Sep 17 00:00:00 2001 From: Achintya-Chatterjee Date: Wed, 10 Dec 2025 22:27:32 +0530 Subject: [PATCH 4/6] fix: formatting --- todo/views/task.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/todo/views/task.py b/todo/views/task.py index f23d02f9..f923bf66 100644 --- a/todo/views/task.py +++ b/todo/views/task.py @@ -113,9 +113,7 @@ def get(self, request: Request): if assignee_ids: if not team_id: - raise ValidationError( - {"teamId": [ValidationErrors.TEAM_ID_REQUIRED_FOR_ASSIGNEE_FILTER]} - ) + raise ValidationError({"teamId": [ValidationErrors.TEAM_ID_REQUIRED_FOR_ASSIGNEE_FILTER]}) team_members = set(UserTeamDetailsRepository.get_users_by_team_id(team_id)) invalid_assignees = [assignee_id for assignee_id in assignee_ids if assignee_id not in team_members] From 4cf9f71ceb90c2f19457b4ed101c0bedbefe641e Mon Sep 17 00:00:00 2001 From: Achintya-Chatterjee Date: Sat, 13 Dec 2025 02:05:59 +0530 Subject: [PATCH 5/6] fix(tasks): validate assignee ids and clarify repo filter - ensure GetTaskQueryParamsSerializer surfaces invalid assigneeId values - rename repository helper set to assignee_lookup_values for readability --- todo/repositories/task_repository.py | 10 +++++----- todo/serializers/get_tasks_serializer.py | 13 +++++++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/todo/repositories/task_repository.py b/todo/repositories/task_repository.py index a56dd7fd..3d16a400 100644 --- a/todo/repositories/task_repository.py +++ b/todo/repositories/task_repository.py @@ -37,18 +37,18 @@ def _get_task_ids_for_assignees(cls, assignee_ids: List[str], team_id: str | Non if not assignee_ids: return [] - candidate_values = set() + assignee_lookup_values = set() for assignee_id in assignee_ids: - candidate_values.add(assignee_id) + assignee_lookup_values.add(assignee_id) if ObjectId.is_valid(assignee_id): - candidate_values.add(ObjectId(assignee_id)) + assignee_lookup_values.add(ObjectId(assignee_id)) - if not candidate_values: + if not assignee_lookup_values: return [] assignment_collection = TaskAssignmentRepository.get_collection() assignment_filter: dict = { - "assignee_id": {"$in": list(candidate_values)}, + "assignee_id": {"$in": list(assignee_lookup_values)}, "user_type": "user", "is_active": True, } diff --git a/todo/serializers/get_tasks_serializer.py b/todo/serializers/get_tasks_serializer.py index 960bda55..1d414d24 100644 --- a/todo/serializers/get_tasks_serializer.py +++ b/todo/serializers/get_tasks_serializer.py @@ -1,7 +1,9 @@ -from rest_framework import serializers from django.conf import settings +from rest_framework import serializers +from bson import ObjectId from todo.constants.task import SORT_FIELDS, SORT_ORDERS, SORT_FIELD_UPDATED_AT, SORT_FIELD_DEFAULT_ORDERS, TaskStatus +from todo.constants.messages import ValidationErrors class CaseInsensitiveChoiceField(serializers.ChoiceField): @@ -77,6 +79,13 @@ def validate(self, attrs): assignee_ids = validated_data.pop("assigneeId", None) if assignee_ids is not None: - validated_data["assignee_ids"] = list(dict.fromkeys(assignee_ids)) + normalized_ids = list(dict.fromkeys(assignee_ids)) + invalid_ids = [assignee_id for assignee_id in normalized_ids if not ObjectId.is_valid(assignee_id)] + if invalid_ids: + raise serializers.ValidationError( + {"assigneeId": [ValidationErrors.INVALID_OBJECT_ID.format(invalid_ids[0])]} + ) + + validated_data["assignee_ids"] = normalized_ids return validated_data From 9185f44bfaa18c6e49177f63be06b897328c5da8 Mon Sep 17 00:00:00 2001 From: Achintya Chatterjee <55826451+Achintya-Chatterjee@users.noreply.github.com> Date: Wed, 17 Dec 2025 01:02:29 +0530 Subject: [PATCH 6/6] test(tasks): cover team + assignee filtering scenarios (#294) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(tasks): cover team + assignee filtering scenarios - add serializer, view, service, and repository specs for multi-assignee queries - exercise team-scoped assignee resolution and list/count symmetry - protect regressions where team filters should bypass user-level fallbacks * test(tasks): cover team + assignee filtering scenarios - add serializer, view, service, and repository specs for multi-assignee queries - exercise team-scoped assignee resolution and list/count symmetry - protect regressions where team filters should bypass user-level fallbacks * test(tasks): cover team + assignee filtering scenarios - add serializer, view, service, and repository specs for multi-assignee queries - exercise team-scoped assignee resolution and list/count symmetry - protect regressions where team filters should bypass user-level fallbacks * fix: By stubbing is_user_team_member before asserting the repository calls, the test no longer bails out with a “FORBIDDEN” response; the mocked list/count methods are hit, and the assertions succeed. * fix:failing test * fix: formatting * test(tasks): update assignee filtering tests for objectid validation - use valid ObjectId strings in serializer and view tests - ensure team membership mock aligns with validated assignee ids --- .../test_task_sorting_integration.py | 54 ++++- .../integration/test_tasks_pagination.py | 2 + .../unit/repositories/test_task_repository.py | 193 ++++++++++++++++++ .../serializers/test_get_tasks_serializer.py | 23 ++- todo/tests/unit/services/test_task_service.py | 112 +++++++++- todo/tests/unit/views/test_task.py | 55 +++++ 6 files changed, 422 insertions(+), 17 deletions(-) diff --git a/todo/tests/integration/test_task_sorting_integration.py b/todo/tests/integration/test_task_sorting_integration.py index 2f431f72..175d60fb 100644 --- a/todo/tests/integration/test_task_sorting_integration.py +++ b/todo/tests/integration/test_task_sorting_integration.py @@ -26,7 +26,14 @@ def test_priority_sorting_integration(self, mock_list, mock_count): self.assertEqual(response.status_code, status.HTTP_200_OK) mock_list.assert_called_with( - 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, str(self.user_id), team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_PRIORITY, + SORT_ORDER_DESC, + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.repositories.task_repository.TaskRepository.count") @@ -40,7 +47,14 @@ def test_due_at_default_order_integration(self, mock_list, mock_count): self.assertEqual(response.status_code, status.HTTP_200_OK) mock_list.assert_called_with( - 1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, str(self.user_id), team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_DUE_AT, + SORT_ORDER_ASC, + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.repositories.task_repository.TaskRepository.count") @@ -55,7 +69,14 @@ def test_assignee_sorting_uses_aggregation(self, mock_list, mock_count): # Assignee sorting now falls back to createdAt sorting mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, str(self.user_id), team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_ASSIGNEE, + SORT_ORDER_ASC, + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.repositories.task_repository.TaskRepository.count") @@ -81,7 +102,14 @@ def test_field_specific_defaults_integration(self, mock_list, mock_count): self.assertEqual(response.status_code, status.HTTP_200_OK) mock_list.assert_called_with( - 1, 20, sort_field, expected_order, str(self.user_id), team_id=None, status_filter=None + 1, + 20, + sort_field, + expected_order, + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.repositories.task_repository.TaskRepository.count") @@ -95,7 +123,14 @@ def test_pagination_with_sorting_integration(self, mock_list, mock_count): self.assertEqual(response.status_code, status.HTTP_200_OK) mock_list.assert_called_with( - 3, 5, SORT_FIELD_CREATED_AT, SORT_ORDER_ASC, str(self.user_id), team_id=None, status_filter=None + 3, + 5, + SORT_FIELD_CREATED_AT, + SORT_ORDER_ASC, + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) def test_invalid_sort_parameters_integration(self): @@ -116,7 +151,14 @@ def test_default_behavior_integration(self, mock_list, mock_count): self.assertEqual(response.status_code, status.HTTP_200_OK) mock_list.assert_called_with( - 1, 20, SORT_FIELD_UPDATED_AT, SORT_ORDER_DESC, str(self.user_id), team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_UPDATED_AT, + SORT_ORDER_DESC, + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.repositories.user_repository.UserRepository.get_by_id") diff --git a/todo/tests/integration/test_tasks_pagination.py b/todo/tests/integration/test_tasks_pagination.py index dc7d22c9..64dde292 100644 --- a/todo/tests/integration/test_tasks_pagination.py +++ b/todo/tests/integration/test_tasks_pagination.py @@ -28,6 +28,7 @@ def test_pagination_settings_integration(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) mock_get_tasks.reset_mock() @@ -43,6 +44,7 @@ def test_pagination_settings_integration(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) # Verify API rejects values above max limit diff --git a/todo/tests/unit/repositories/test_task_repository.py b/todo/tests/unit/repositories/test_task_repository.py index f24029a1..0758efa5 100644 --- a/todo/tests/unit/repositories/test_task_repository.py +++ b/todo/tests/unit/repositories/test_task_repository.py @@ -91,6 +91,117 @@ def test_list_returns_empty_list_for_no_tasks(self): self.mock_collection.find.return_value.sort.return_value.skip.assert_called_once_with(10) self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.assert_called_once_with(10) + @patch.object(TaskRepository, "_get_task_ids_for_assignees", return_value=[]) + def test_list_returns_empty_when_assignee_filter_has_no_matches(self, mock_get_task_ids): + result = TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id=None, + assignee_ids=["user1"], + ) + + self.assertEqual(result, []) + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.find.assert_not_called() + + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_list_filters_by_assignee_ids(self, mock_get_task_ids): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id=None, + assignee_ids=["user1"], + ) + + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.find.assert_called_once() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) + ) + + @patch.object(TaskRepository, "_get_assigned_task_ids_for_user", return_value=[]) + def test_list_includes_created_tasks_when_user_has_no_assignments(self, mock_get_assigned): + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + user_id = "user_creator" + TaskRepository.list(1, 10, sort_by="createdAt", order="desc", user_id=user_id) + + self.mock_collection.find.assert_called_once() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any("$or" in condition and {"createdBy": user_id} in condition["$or"] for condition in query_filter["$and"]) + ) + mock_get_assigned.assert_called_once_with(user_id) + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[]) + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_list_with_team_and_assignee_ids_relies_on_assignee_tasks(self, mock_get_task_ids, mock_get_team_tasks): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id=None, + team_id="team123", + assignee_ids=["user1"], + ) + + mock_get_team_tasks.assert_not_called() # team filtering is handled by assignee helper when both provided + mock_get_task_ids.assert_called_once_with(["user1"], team_id="team123") + self.mock_collection.find.assert_called_once() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) + ) + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[ObjectId()]) + @patch.object(TaskRepository, "_get_assigned_task_ids_for_user") + def test_list_team_filter_skips_user_filter(self, mock_get_assigned, mock_get_team_tasks): + mock_cursor = MagicMock() + mock_cursor.__iter__ = MagicMock(return_value=iter(self.task_data)) + self.mock_collection.find.return_value.sort.return_value.skip.return_value.limit.return_value = mock_cursor + + TaskRepository.list( + 1, + 10, + sort_by=SORT_FIELD_CREATED_AT, + order=SORT_ORDER_DESC, + user_id="user123", + team_id="team123", + ) + + mock_get_team_tasks.assert_called_once() + mock_get_assigned.assert_not_called() + query_filter = self.mock_collection.find.call_args[0][0] + self.assertIn("$and", query_filter) + # Ensure no user OR clause present + for condition in query_filter["$and"]: + self.assertFalse("$or" in condition and {"createdBy": "user123"} in condition["$or"]) + def test_count_returns_total_task_count(self): self.mock_collection.count_documents.return_value = 42 @@ -122,6 +233,88 @@ def test_get_all_returns_empty_list_for_no_tasks(self): self.assertEqual(result, []) self.mock_collection.find.assert_called_once() + @patch("todo.repositories.task_repository.TaskAssignmentRepository.get_collection") + def test_get_task_ids_for_assignees_returns_object_ids(self, mock_get_collection): + mock_collection = MagicMock() + task_id_obj = ObjectId() + task_id_str = str(ObjectId()) + mock_collection.find.return_value = [{"task_id": task_id_obj}, {"task_id": task_id_str}] + mock_get_collection.return_value = mock_collection + + result = TaskRepository._get_task_ids_for_assignees([str(ObjectId()), "not-an-object-id"]) + + self.assertEqual( + set(result), + {task_id_obj, ObjectId(task_id_str)}, + ) + mock_collection.find.assert_called_once() + + @patch("todo.repositories.task_repository.TaskAssignmentRepository.get_collection") + def test_get_task_ids_for_assignees_filters_by_team(self, mock_get_collection): + mock_collection = MagicMock() + mock_collection.find.return_value = [{"task_id": ObjectId()}] + mock_get_collection.return_value = mock_collection + + team_id = str(ObjectId()) + TaskRepository._get_task_ids_for_assignees(["user1"], team_id=team_id) + + assignment_filter = mock_collection.find.call_args[0][0] + self.assertIn("team_id", assignment_filter) + values = assignment_filter["team_id"]["$in"] + self.assertIn(team_id, values) + self.assertTrue(any(ObjectId.is_valid(v) for v in values)) + + @patch.object(TaskRepository, "_get_task_ids_for_assignees", return_value=[]) + def test_count_returns_zero_when_assignee_filter_has_no_matches(self, mock_get_task_ids): + result = TaskRepository.count(assignee_ids=["user1"]) + + self.assertEqual(result, 0) + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.count_documents.assert_not_called() + + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_count_filters_by_assignee_ids(self, mock_get_task_ids): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + self.mock_collection.count_documents.return_value = 5 + + TaskRepository.count(assignee_ids=["user1"]) + + mock_get_task_ids.assert_called_once_with(["user1"], team_id=None) + self.mock_collection.count_documents.assert_called_once() + query_filter = self.mock_collection.count_documents.call_args[0][0] + self.assertIn("$and", query_filter) + self.assertTrue( + any(condition.get("_id", {}).get("$in") == [assignee_task_id] for condition in query_filter["$and"]) + ) + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[]) + @patch.object(TaskRepository, "_get_task_ids_for_assignees") + def test_count_with_team_and_assignee_ids_relies_on_assignee_tasks(self, mock_get_task_ids, mock_get_team_tasks): + assignee_task_id = ObjectId() + mock_get_task_ids.return_value = [assignee_task_id] + self.mock_collection.count_documents.return_value = 1 + + TaskRepository.count(team_id="team123", assignee_ids=["user1"]) + + mock_get_team_tasks.assert_not_called() + mock_get_task_ids.assert_called_once_with(["user1"], team_id="team123") + self.mock_collection.count_documents.assert_called_once() + + @patch.object(TaskRepository, "_get_team_task_ids", return_value=[ObjectId()]) + @patch.object(TaskRepository, "_get_assigned_task_ids_for_user") + def test_count_team_filter_skips_user_filter(self, mock_get_assigned, mock_get_team_tasks): + self.mock_collection.count_documents.return_value = 1 + + TaskRepository.count(team_id="team123", user_id="user123") + + mock_get_team_tasks.assert_called_once() + mock_get_assigned.assert_not_called() + query_filter = self.mock_collection.count_documents.call_args[0][0] + self.assertIn("$and", query_filter) + for condition in query_filter["$and"]: + self.assertFalse("$or" in condition and {"createdBy": "user123"} in condition["$or"]) + def test_get_by_id_returns_task_model_when_found(self): task_id_str = str(self.task_db_data_fixture["_id"]) self.mock_collection.find_one.return_value = self.task_db_data_fixture diff --git a/todo/tests/unit/serializers/test_get_tasks_serializer.py b/todo/tests/unit/serializers/test_get_tasks_serializer.py index c2423dbf..d1748e51 100644 --- a/todo/tests/unit/serializers/test_get_tasks_serializer.py +++ b/todo/tests/unit/serializers/test_get_tasks_serializer.py @@ -1,6 +1,8 @@ from unittest import TestCase -from rest_framework.exceptions import ValidationError from django.conf import settings +from django.http import QueryDict +from rest_framework.exceptions import ValidationError +from bson import ObjectId from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer from todo.constants.task import ( @@ -71,6 +73,25 @@ def test_serializer_ignores_undefined_extra_fields(self): self.assertEqual(serializer.validated_data["limit"], 5) self.assertNotIn("extra_field", serializer.validated_data) + def test_serializer_collects_assignee_ids_from_querydict(self): + query_params = QueryDict(mutable=True) + assignee_ids = [str(ObjectId()), str(ObjectId())] + query_params.setlist("assigneeId", assignee_ids) + + serializer = GetTaskQueryParamsSerializer(data=query_params) + self.assertTrue(serializer.is_valid()) + self.assertEqual(serializer.validated_data["assignee_ids"], assignee_ids) + + def test_serializer_deduplicates_assignee_ids(self): + query_params = QueryDict(mutable=True) + first_id = str(ObjectId()) + second_id = str(ObjectId()) + query_params.setlist("assigneeId", [first_id, first_id, second_id]) + + serializer = GetTaskQueryParamsSerializer(data=query_params) + self.assertTrue(serializer.is_valid()) + self.assertEqual(serializer.validated_data["assignee_ids"], [first_id, second_id]) + def test_serializer_uses_django_settings_values(self): """Test that the serializer correctly uses values from Django settings""" # Instead of mocking, we'll test against the actual settings values diff --git a/todo/tests/unit/services/test_task_service.py b/todo/tests/unit/services/test_task_service.py index e15c98be..6b97257f 100644 --- a/todo/tests/unit/services/test_task_service.py +++ b/todo/tests/unit/services/test_task_service.py @@ -72,9 +72,16 @@ def test_get_tasks_returns_paginated_response( ) mock_list.assert_called_once_with( - 2, 1, "createdAt", "desc", str(self.user_id), team_id=None, status_filter=None + 2, + 1, + "createdAt", + "desc", + str(self.user_id), + team_id=None, + status_filter=None, + assignee_ids=None, ) - mock_count.assert_called_once() + mock_count.assert_called_once_with(str(self.user_id), team_id=None, status_filter=None, assignee_ids=None) @patch("todo.services.task_service.UserRepository.get_by_id") @patch("todo.services.task_service.TaskRepository.count") @@ -113,8 +120,51 @@ def test_get_tasks_returns_empty_response_if_no_tasks_present(self, mock_list: M self.assertEqual(len(response.tasks), 0) self.assertIsNone(response.links) - mock_list.assert_called_once_with(1, 10, "createdAt", "desc", "test_user", team_id=None, status_filter=None) - mock_count.assert_called_once() + mock_list.assert_called_once_with( + 1, + 10, + "createdAt", + "desc", + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, + ) + mock_count.assert_called_once_with("test_user", team_id=None, status_filter=None, assignee_ids=None) + + @patch("todo.repositories.team_repository.TeamRepository.is_user_team_member", return_value=True) + @patch("todo.services.task_service.TaskRepository.count") + @patch("todo.services.task_service.TaskRepository.list") + def test_get_tasks_passes_assignee_ids_to_repo(self, mock_list: Mock, mock_count: Mock, mock_team_member: Mock): + mock_list.return_value = [] + mock_count.return_value = 0 + + TaskService.get_tasks( + page=1, + limit=10, + sort_by="createdAt", + order="desc", + user_id="request_user", + team_id="team123", + assignee_ids=["user1", "user2"], + ) + + mock_list.assert_called_once_with( + 1, + 10, + "createdAt", + "desc", + "request_user", + team_id="team123", + status_filter=None, + assignee_ids=["user1", "user2"], + ) + mock_count.assert_called_once_with( + "request_user", + team_id="team123", + status_filter=None, + assignee_ids=["user1", "user2"], + ) @patch("todo.services.task_service.TaskRepository.count") @patch("todo.services.task_service.TaskRepository.list") @@ -434,7 +484,14 @@ def test_get_tasks_default_sorting(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by="createdAt", order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_CREATED_AT, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -446,7 +503,14 @@ def test_get_tasks_explicit_sort_by_priority(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order=SORT_ORDER_DESC, user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_PRIORITY, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -458,7 +522,14 @@ def test_get_tasks_sort_by_due_at_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_DUE_AT, order="asc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_DUE_AT, + SORT_ORDER_ASC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -470,7 +541,14 @@ def test_get_tasks_sort_by_priority_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_PRIORITY, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -482,7 +560,14 @@ def test_get_tasks_sort_by_assignee_default_order(self, mock_list, mock_count): TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_ASSIGNEE, order="asc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_ASSIGNEE, + SORT_ORDER_ASC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskRepository.count") @@ -494,7 +579,14 @@ def test_get_tasks_sort_by_created_at_default_order(self, mock_list, mock_count) TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_CREATED_AT, order="desc", user_id="test_user") mock_list.assert_called_once_with( - 1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None + 1, + 20, + SORT_FIELD_CREATED_AT, + SORT_ORDER_DESC, + "test_user", + team_id=None, + status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.reverse_lazy", return_value="/v1/tasks") diff --git a/todo/tests/unit/views/test_task.py b/todo/tests/unit/views/test_task.py index db780734..94f41c01 100644 --- a/todo/tests/unit/views/test_task.py +++ b/todo/tests/unit/views/test_task.py @@ -53,6 +53,7 @@ def test_get_tasks_returns_200_for_valid_params(self, mock_get_tasks: Mock): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) self.assertEqual(response.status_code, status.HTTP_200_OK) expected_response = mock_get_tasks.return_value.model_dump(mode="json") @@ -72,6 +73,7 @@ def test_get_tasks_returns_200_without_params(self, mock_get_tasks: Mock): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -101,6 +103,50 @@ def test_get_tasks_returns_400_for_invalid_query_params(self): self.assertEqual(actual_error["source"]["parameter"], expected_error["source"]["parameter"]) self.assertEqual(actual_error["detail"], expected_error["detail"]) + def test_get_tasks_requires_team_for_assignee_filter(self): + response = self.client.get(self.url, {"assigneeId": str(ObjectId())}) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + error_sources = [error["source"].get("parameter") for error in response.data.get("errors", [])] + self.assertIn("teamId", error_sources) + + @patch("todo.views.task.UserTeamDetailsRepository.get_users_by_team_id", return_value=["user1"]) + def test_get_tasks_rejects_assignee_not_in_team(self, mock_get_team_members: Mock): + team_id = str(ObjectId()) + assignee_id = str(ObjectId()) + + response = self.client.get(self.url, {"teamId": team_id, "assigneeId": assignee_id}) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("errors", response.data) + self.assertTrue( + any(ValidationErrors.USER_NOT_TEAM_MEMBER in error.get("detail", "") for error in response.data["errors"]) + ) + mock_get_team_members.assert_called_once_with(team_id) + + @patch("todo.views.task.UserTeamDetailsRepository.get_users_by_team_id") + @patch("todo.services.task_service.TaskService.get_tasks") + def test_get_tasks_with_assignee_filter_passes_ids(self, mock_get_tasks: Mock, mock_get_team_members: Mock): + mock_get_tasks.return_value = GetTasksResponse(tasks=task_dtos) + team_id = str(ObjectId()) + assignee_ids = [str(ObjectId()), str(ObjectId())] + mock_get_team_members.return_value = assignee_ids + + response = self.client.get(self.url, {"teamId": team_id, "page": 1, "limit": 10, "assigneeId": assignee_ids}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_get_team_members.assert_called_once_with(team_id) + mock_get_tasks.assert_called_once_with( + page=1, + limit=10, + sort_by="updatedAt", + order="desc", + user_id=str(self.user_id), + team_id=team_id, + status_filter=None, + assignee_ids=assignee_ids, + ) + @patch("todo.services.task_service.TaskService.get_task_by_id") def test_get_single_task_success(self, mock_get_task_by_id: Mock): valid_task_id = str(ObjectId()) @@ -183,6 +229,7 @@ def test_get_tasks_with_default_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -201,6 +248,7 @@ def test_get_tasks_with_valid_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) def test_get_tasks_with_invalid_page(self): @@ -249,6 +297,7 @@ def test_get_tasks_with_sort_by_priority(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -266,6 +315,7 @@ def test_get_tasks_with_sort_by_and_order(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -295,6 +345,7 @@ def test_get_tasks_with_all_sort_fields(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -318,6 +369,7 @@ def test_get_tasks_with_all_order_values(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) def test_get_tasks_with_invalid_sort_by(self): @@ -353,6 +405,7 @@ def test_get_tasks_sorting_with_pagination(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) @patch("todo.services.task_service.TaskService.get_tasks") @@ -370,6 +423,7 @@ def test_get_tasks_default_behavior_unchanged(self, mock_get_tasks): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, ) def test_get_tasks_edge_case_combinations(self): @@ -387,6 +441,7 @@ def test_get_tasks_edge_case_combinations(self): user_id=str(self.user_id), team_id=None, status_filter=None, + assignee_ids=None, )