Skip to content

Commit 3e51495

Browse files
Achintya-ChatterjeeAchintya-Chatterjee
authored
fix: add status filtering to tasks API with DONE exclusion by default (#227)
* fix: add status filtering to tasks API with DONE exclusion by default - Exclude DONE tasks from GET /v1/tasks by default for cleaner active task view - Add ?status=DONE query parameter to specifically retrieve DONE tasks - Implement status filtering across all task endpoints: * GET /v1/tasks - general tasks with status filtering * GET /v1/tasks?profile=true - user's tasks with status filtering * GET /v1/tasks?teamId={id} - team tasks with status filtering - Fix get_tasks_for_user to include both created AND assigned tasks - Update repository layer (list, count, get_tasks_for_user) with status_filter parameter - Update service and view layers to pass status filter from query params - Update serializer to accept status query parameter - Fix all test assertions to match new method signatures * chore: extract status filtering logic into a class method * chore: enhance the status validation by using todo status constants * chore: added uppercase handling
1 parent 34c3f4f commit 3e51495

File tree

9 files changed

+190
-42
lines changed

9 files changed

+190
-42
lines changed

‎todo/repositories/task_repository.py‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,31 +9,51 @@
99
from todo.repositories.common.mongo_repository import MongoRepository
1010
from todo.repositories.task_assignment_repository import TaskAssignmentRepository
1111
from todo.constants.messages import ApiErrors, RepositoryErrors
12-
from todo.constants.task import SORT_FIELD_PRIORITY, SORT_FIELD_ASSIGNEE, SORT_ORDER_DESC
12+
from todo.constants.task import SORT_FIELD_PRIORITY, SORT_FIELD_ASSIGNEE, SORT_ORDER_DESC, TaskStatus
1313

1414

1515
class TaskRepository(MongoRepository):
1616
collection_name = TaskModel.collection_name
1717

18+
@classmethod
19+
def _build_status_filter(cls, status_filter: str = None) -> dict:
20+
"""
21+
Build status filter for task queries.
22+
23+
"""
24+
if status_filter:
25+
return {"status": status_filter}
26+
else:
27+
return {"status": {"$ne": TaskStatus.DONE.value}}
28+
1829
@classmethod
1930
def list(
20-
cls, page: int, limit: int, sort_by: str, order: str, user_id: str = None, team_id: str = None
31+
cls,
32+
page: int,
33+
limit: int,
34+
sort_by: str,
35+
order: str,
36+
user_id: str = None,
37+
team_id: str = None,
38+
status_filter: str = None,
2139
) -> List[TaskModel]:
2240
tasks_collection = cls.get_collection()
2341
logger = logging.getLogger(__name__)
2442

43+
base_filter = cls._build_status_filter(status_filter)
44+
2545
if team_id:
2646
logger.debug(f"TaskRepository.list: team_id={team_id}")
2747
team_assignments = TaskAssignmentRepository.get_by_assignee_id(team_id, "team")
2848
team_task_ids = [assignment.task_id for assignment in team_assignments]
2949
logger.debug(f"TaskRepository.list: team_task_ids={team_task_ids}")
30-
query_filter = {"_id": {"$in": team_task_ids}}
50+
query_filter = {"$and": [base_filter, {"_id": {"$in": team_task_ids}}]}
3151
logger.debug(f"TaskRepository.list: query_filter={query_filter}")
3252
elif user_id:
3353
assigned_task_ids = cls._get_assigned_task_ids_for_user(user_id)
34-
query_filter = {"_id": {"$in": assigned_task_ids}}
54+
query_filter = {"$and": [base_filter, {"_id": {"$in": assigned_task_ids}}]}
3555
else:
36-
query_filter = {}
56+
query_filter = base_filter
3757

3858
if sort_by == SORT_FIELD_PRIORITY:
3959
sort_direction = 1 if order == SORT_ORDER_DESC else -1
@@ -70,17 +90,22 @@ def _get_assigned_task_ids_for_user(cls, user_id: str) -> List[ObjectId]:
7090
return direct_task_ids + team_task_ids
7191

7292
@classmethod
73-
def count(cls, user_id: str = None, team_id: str = None) -> int:
93+
def count(cls, user_id: str = None, team_id: str = None, status_filter: str = None) -> int:
7494
tasks_collection = cls.get_collection()
95+
96+
base_filter = cls._build_status_filter(status_filter)
97+
7598
if team_id:
7699
team_assignments = TaskAssignmentRepository.get_by_assignee_id(team_id, "team")
77100
team_task_ids = [assignment.task_id for assignment in team_assignments]
78-
query_filter = {"_id": {"$in": team_task_ids}}
101+
query_filter = {"$and": [base_filter, {"_id": {"$in": team_task_ids}}]}
79102
elif user_id:
80103
assigned_task_ids = cls._get_assigned_task_ids_for_user(user_id)
81-
query_filter = {"$or": [{"createdBy": user_id}, {"_id": {"$in": assigned_task_ids}}]}
104+
query_filter = {
105+
"$and": [base_filter, {"$or": [{"createdBy": user_id}, {"_id": {"$in": assigned_task_ids}}]}]
106+
}
82107
else:
83-
query_filter = {}
108+
query_filter = base_filter
84109
return tasks_collection.count_documents(query_filter)
85110

86111
@classmethod
@@ -208,10 +233,13 @@ def update(cls, task_id: str, update_data: dict) -> TaskModel | None:
208233
return None
209234

210235
@classmethod
211-
def get_tasks_for_user(cls, user_id: str, page: int, limit: int) -> List[TaskModel]:
236+
def get_tasks_for_user(cls, user_id: str, page: int, limit: int, status_filter: str = None) -> List[TaskModel]:
212237
tasks_collection = cls.get_collection()
213238
assigned_task_ids = cls._get_assigned_task_ids_for_user(user_id)
214-
query = {"_id": {"$in": assigned_task_ids}}
239+
240+
base_filter = cls._build_status_filter(status_filter)
241+
242+
query = {"$and": [base_filter, {"_id": {"$in": assigned_task_ids}}]}
215243
tasks_cursor = tasks_collection.find(query).skip((page - 1) * limit).limit(limit)
216244
return [TaskModel(**task) for task in tasks_cursor]
217245

‎todo/serializers/get_tasks_serializer.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from rest_framework import serializers
22
from django.conf import settings
33

4-
from todo.constants.task import SORT_FIELDS, SORT_ORDERS, SORT_FIELD_CREATED_AT, SORT_FIELD_DEFAULT_ORDERS
4+
from todo.constants.task import SORT_FIELDS, SORT_ORDERS, SORT_FIELD_CREATED_AT, SORT_FIELD_DEFAULT_ORDERS, TaskStatus
55

66

77
class GetTaskQueryParamsSerializer(serializers.Serializer):
@@ -37,6 +37,12 @@ class GetTaskQueryParamsSerializer(serializers.Serializer):
3737

3838
teamId = serializers.CharField(required=False, allow_blank=False, allow_null=True)
3939

40+
status = serializers.ChoiceField(
41+
choices=[status.value for status in TaskStatus],
42+
required=False,
43+
allow_null=True,
44+
)
45+
4046
def validate(self, attrs):
4147
validated_data = super().validate(attrs)
4248

‎todo/services/task_service.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ def get_tasks(
7171
order: str,
7272
user_id: str,
7373
team_id: str = None,
74+
status_filter: str = None,
7475
) -> GetTasksResponse:
7576
try:
7677
cls._validate_pagination_params(page, limit)
@@ -89,8 +90,10 @@ def get_tasks(
8990
},
9091
)
9192

92-
tasks = TaskRepository.list(page, limit, sort_by, order, user_id, team_id=team_id)
93-
total_count = TaskRepository.count(user_id, team_id=team_id)
93+
tasks = TaskRepository.list(
94+
page, limit, sort_by, order, user_id, team_id=team_id, status_filter=status_filter
95+
)
96+
total_count = TaskRepository.count(user_id, team_id=team_id, status_filter=status_filter)
9497

9598
if not tasks:
9699
return GetTasksResponse(tasks=[], links=None)
@@ -665,9 +668,10 @@ def get_tasks_for_user(
665668
user_id: str,
666669
page: int = PaginationConfig.DEFAULT_PAGE,
667670
limit: int = PaginationConfig.DEFAULT_LIMIT,
671+
status_filter: str = None,
668672
) -> GetTasksResponse:
669673
cls._validate_pagination_params(page, limit)
670-
tasks = TaskRepository.get_tasks_for_user(user_id, page, limit)
674+
tasks = TaskRepository.get_tasks_for_user(user_id, page, limit, status_filter=status_filter)
671675
if not tasks:
672676
return GetTasksResponse(tasks=[], links=None)
673677

‎todo/tests/integration/test_task_sorting_integration.py‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ def test_priority_sorting_integration(self, mock_list, mock_count):
2424
response = self.client.get("/v1/tasks", {"sort_by": "priority", "order": "desc"})
2525

2626
self.assertEqual(response.status_code, status.HTTP_200_OK)
27-
mock_list.assert_called_with(1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, str(self.user_id), team_id=None)
27+
mock_list.assert_called_with(
28+
1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, str(self.user_id), team_id=None, status_filter=None
29+
)
2830

2931
@patch("todo.repositories.task_repository.TaskRepository.count")
3032
@patch("todo.repositories.task_repository.TaskRepository.list")
@@ -36,7 +38,9 @@ def test_due_at_default_order_integration(self, mock_list, mock_count):
3638

3739
self.assertEqual(response.status_code, status.HTTP_200_OK)
3840

39-
mock_list.assert_called_with(1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, str(self.user_id), team_id=None)
41+
mock_list.assert_called_with(
42+
1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, str(self.user_id), team_id=None, status_filter=None
43+
)
4044

4145
@patch("todo.repositories.task_repository.TaskRepository.count")
4246
@patch("todo.repositories.task_repository.TaskRepository.list")
@@ -49,7 +53,9 @@ def test_assignee_sorting_uses_aggregation(self, mock_list, mock_count):
4953
self.assertEqual(response.status_code, status.HTTP_200_OK)
5054

5155
# Assignee sorting now falls back to createdAt sorting
52-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, str(self.user_id), team_id=None)
56+
mock_list.assert_called_once_with(
57+
1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, str(self.user_id), team_id=None, status_filter=None
58+
)
5359

5460
@patch("todo.repositories.task_repository.TaskRepository.count")
5561
@patch("todo.repositories.task_repository.TaskRepository.list")
@@ -72,7 +78,9 @@ def test_field_specific_defaults_integration(self, mock_list, mock_count):
7278
response = self.client.get("/v1/tasks", {"sort_by": sort_field})
7379

7480
self.assertEqual(response.status_code, status.HTTP_200_OK)
75-
mock_list.assert_called_with(1, 20, sort_field, expected_order, str(self.user_id), team_id=None)
81+
mock_list.assert_called_with(
82+
1, 20, sort_field, expected_order, str(self.user_id), team_id=None, status_filter=None
83+
)
7684

7785
@patch("todo.repositories.task_repository.TaskRepository.count")
7886
@patch("todo.repositories.task_repository.TaskRepository.list")
@@ -84,7 +92,9 @@ def test_pagination_with_sorting_integration(self, mock_list, mock_count):
8492

8593
self.assertEqual(response.status_code, status.HTTP_200_OK)
8694

87-
mock_list.assert_called_with(3, 5, SORT_FIELD_CREATED_AT, SORT_ORDER_ASC, str(self.user_id), team_id=None)
95+
mock_list.assert_called_with(
96+
3, 5, SORT_FIELD_CREATED_AT, SORT_ORDER_ASC, str(self.user_id), team_id=None, status_filter=None
97+
)
8898

8999
def test_invalid_sort_parameters_integration(self):
90100
response = self.client.get("/v1/tasks", {"sort_by": "invalid_field"})
@@ -103,7 +113,9 @@ def test_default_behavior_integration(self, mock_list, mock_count):
103113

104114
self.assertEqual(response.status_code, status.HTTP_200_OK)
105115

106-
mock_list.assert_called_with(1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, str(self.user_id), team_id=None)
116+
mock_list.assert_called_with(
117+
1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, str(self.user_id), team_id=None, status_filter=None
118+
)
107119

108120
@patch("todo.services.task_service.reverse_lazy", return_value="/v1/tasks")
109121
@patch("todo.repositories.task_repository.TaskRepository.count")

‎todo/tests/integration/test_tasks_pagination.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ def test_pagination_settings_integration(self, mock_get_tasks):
2121

2222
self.assertEqual(response.status_code, 200)
2323
mock_get_tasks.assert_called_with(
24-
page=1, limit=default_limit, sort_by="createdAt", order="desc", user_id=str(self.user_id), team_id=None
24+
page=1,
25+
limit=default_limit,
26+
sort_by="createdAt",
27+
order="desc",
28+
user_id=str(self.user_id),
29+
team_id=None,
30+
status_filter=None,
2531
)
2632

2733
mock_get_tasks.reset_mock()
@@ -30,7 +36,13 @@ def test_pagination_settings_integration(self, mock_get_tasks):
3036

3137
self.assertEqual(response.status_code, 200)
3238
mock_get_tasks.assert_called_with(
33-
page=1, limit=10, sort_by="createdAt", order="desc", user_id=str(self.user_id), team_id=None
39+
page=1,
40+
limit=10,
41+
sort_by="createdAt",
42+
order="desc",
43+
user_id=str(self.user_id),
44+
team_id=None,
45+
status_filter=None,
3446
)
3547

3648
# Verify API rejects values above max limit

‎todo/tests/unit/repositories/test_task_repository.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def test_count_returns_total_task_count(self):
9797
result = TaskRepository.count()
9898

9999
self.assertEqual(result, 42)
100-
self.mock_collection.count_documents.assert_called_once_with({})
100+
self.mock_collection.count_documents.assert_called_once_with({"status": {"$ne": "DONE"}})
101101

102102
def test_get_all_returns_all_tasks(self):
103103
self.mock_collection.find.return_value = self.task_data

‎todo/tests/unit/services/test_task_service.py‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ def test_get_tasks_returns_paginated_response(
7171
response.links.prev, f"{self.mock_reverse_lazy('tasks')}?page=1&limit=1&sort_by=createdAt&order=desc"
7272
)
7373

74-
mock_list.assert_called_once_with(2, 1, "createdAt", "desc", str(self.user_id), team_id=None)
74+
mock_list.assert_called_once_with(
75+
2, 1, "createdAt", "desc", str(self.user_id), team_id=None, status_filter=None
76+
)
7577
mock_count.assert_called_once()
7678

7779
@patch("todo.services.task_service.UserRepository.get_by_id")
@@ -111,7 +113,7 @@ def test_get_tasks_returns_empty_response_if_no_tasks_present(self, mock_list: M
111113
self.assertEqual(len(response.tasks), 0)
112114
self.assertIsNone(response.links)
113115

114-
mock_list.assert_called_once_with(1, 10, "createdAt", "desc", "test_user", team_id=None)
116+
mock_list.assert_called_once_with(1, 10, "createdAt", "desc", "test_user", team_id=None, status_filter=None)
115117
mock_count.assert_called_once()
116118

117119
@patch("todo.services.task_service.TaskRepository.count")
@@ -294,7 +296,9 @@ def test_get_tasks_default_sorting(self, mock_list, mock_count):
294296

295297
TaskService.get_tasks(page=1, limit=20, sort_by="createdAt", order="desc", user_id="test_user")
296298

297-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None)
299+
mock_list.assert_called_once_with(
300+
1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None
301+
)
298302

299303
@patch("todo.services.task_service.TaskRepository.count")
300304
@patch("todo.services.task_service.TaskRepository.list")
@@ -304,7 +308,9 @@ def test_get_tasks_explicit_sort_by_priority(self, mock_list, mock_count):
304308

305309
TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order=SORT_ORDER_DESC, user_id="test_user")
306310

307-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None)
311+
mock_list.assert_called_once_with(
312+
1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None
313+
)
308314

309315
@patch("todo.services.task_service.TaskRepository.count")
310316
@patch("todo.services.task_service.TaskRepository.list")
@@ -314,7 +320,9 @@ def test_get_tasks_sort_by_due_at_default_order(self, mock_list, mock_count):
314320

315321
TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_DUE_AT, order="asc", user_id="test_user")
316322

317-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, "test_user", team_id=None)
323+
mock_list.assert_called_once_with(
324+
1, 20, SORT_FIELD_DUE_AT, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None
325+
)
318326

319327
@patch("todo.services.task_service.TaskRepository.count")
320328
@patch("todo.services.task_service.TaskRepository.list")
@@ -324,7 +332,9 @@ def test_get_tasks_sort_by_priority_default_order(self, mock_list, mock_count):
324332

325333
TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_PRIORITY, order="desc", user_id="test_user")
326334

327-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None)
335+
mock_list.assert_called_once_with(
336+
1, 20, SORT_FIELD_PRIORITY, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None
337+
)
328338

329339
@patch("todo.services.task_service.TaskRepository.count")
330340
@patch("todo.services.task_service.TaskRepository.list")
@@ -334,7 +344,9 @@ def test_get_tasks_sort_by_assignee_default_order(self, mock_list, mock_count):
334344

335345
TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_ASSIGNEE, order="asc", user_id="test_user")
336346

337-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, "test_user", team_id=None)
347+
mock_list.assert_called_once_with(
348+
1, 20, SORT_FIELD_ASSIGNEE, SORT_ORDER_ASC, "test_user", team_id=None, status_filter=None
349+
)
338350

339351
@patch("todo.services.task_service.TaskRepository.count")
340352
@patch("todo.services.task_service.TaskRepository.list")
@@ -344,7 +356,9 @@ def test_get_tasks_sort_by_created_at_default_order(self, mock_list, mock_count)
344356

345357
TaskService.get_tasks(page=1, limit=20, sort_by=SORT_FIELD_CREATED_AT, order="desc", user_id="test_user")
346358

347-
mock_list.assert_called_once_with(1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None)
359+
mock_list.assert_called_once_with(
360+
1, 20, SORT_FIELD_CREATED_AT, SORT_ORDER_DESC, "test_user", team_id=None, status_filter=None
361+
)
348362

349363
@patch("todo.services.task_service.reverse_lazy", return_value="/v1/tasks")
350364
def test_build_page_url_includes_sort_parameters(self, mock_reverse):

0 commit comments

Comments
 (0)