-
Notifications
You must be signed in to change notification settings - Fork 14
fix: add status filtering to tasks API with DONE exclusion by default #227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- 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
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe changes introduce an optional status filter for task retrieval and counting throughout the backend. The repository, service, serializer, and view layers are updated to support filtering tasks by their status, defaulting to excluding "DONE" tasks unless specified. Associated tests are modified to reflect the new parameter and filtering logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TaskListView
participant GetTaskQueryParamsSerializer
participant TaskService
participant TaskRepository
Client->>TaskListView: GET /tasks?status=IN_PROGRESS
TaskListView->>GetTaskQueryParamsSerializer: Validate query params
GetTaskQueryParamsSerializer-->>TaskListView: Return validated data (status)
TaskListView->>TaskService: get_tasks(..., status_filter="IN_PROGRESS")
TaskService->>TaskRepository: list(..., status_filter="IN_PROGRESS")
TaskRepository-->>TaskService: Return filtered tasks
TaskService-->>TaskListView: Return tasks
TaskListView-->>Client: Return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related issues
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
todo/repositories/task_repository.py
(3 hunks)todo/serializers/get_tasks_serializer.py
(1 hunks)todo/services/task_service.py
(3 hunks)todo/tests/integration/test_task_sorting_integration.py
(6 hunks)todo/tests/integration/test_tasks_pagination.py
(2 hunks)todo/tests/unit/repositories/test_task_repository.py
(1 hunks)todo/tests/unit/services/test_task_service.py
(8 hunks)todo/tests/unit/views/test_task.py
(11 hunks)todo/views/task.py
(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 in the Real-Dev-Squad/todo-backend repository comprehensively tracks user authentication implementation including registration, login, JWT tokens, and making task APIs require authentication. This covers replacing hardcoded user ID placeholders like "system_patch_user" with actual user ID extraction from authenticated requests.
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 tracks the implementation of user authentication in the todo-backend project, which includes extracting user ID from request context to replace hardcoded placeholders like "system_patch_user" in todo/views/task.py.
Learnt from: AnujChhikara
PR: Real-Dev-Squad/todo-backend#119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
Learnt from: AnujChhikara
PR: Real-Dev-Squad/todo-backend#75
File: todo/tests/integration/test_tasks_delete.py:20-23
Timestamp: 2025-06-08T15:58:01.548Z
Learning: In the Real-Dev-Squad/todo-backend repository, integration tests focus on API behavior (HTTP status codes, response structure) while unit tests handle verification of actual database operations and state changes. Database verification should not be added to integration tests.
Learnt from: VaibhavSingh8
PR: Real-Dev-Squad/todo-backend#81
File: .github/workflows/test.yml:10-10
Timestamp: 2025-06-16T11:09:47.782Z
Learning: In the todo-backend project, tests are mandatory but may be implemented in separate PRs. The "[skip tests]" condition in the GitHub workflow is used intentionally for specific PRs rather than as a general practice that could be abused.
Learnt from: VaibhavSingh8
PR: Real-Dev-Squad/todo-backend#81
File: todo_project/settings/base.py:70-77
Timestamp: 2025-06-16T18:05:07.813Z
Learning: The todo-backend project uses a custom pagination configuration system with `DEFAULT_PAGINATION_SETTINGS` nested under `REST_FRAMEWORK` in Django settings. This is not a DRF setting but a custom implementation used by their `PaginationConfig` class in `task_service.py`, serializers, and throughout their codebase. The configuration includes `DEFAULT_PAGE_LIMIT` and `MAX_PAGE_LIMIT` values.
todo/serializers/get_tasks_serializer.py (1)
Learnt from: shobhan-sundar-goutam
PR: #95
File: todo/views/label.py:21-21
Timestamp: 2025-07-02T18:28:01.803Z
Learning: In Django REST Framework serializers, when a CharField has default="" specified, the validated_data will always contain a string value (empty string if parameter not provided) rather than None, making None-checks unnecessary when accessing that field from validated_data.
todo/tests/unit/views/test_task.py (2)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
Learnt from: VaibhavSingh8
PR: #81
File: todo_project/settings/base.py:70-77
Timestamp: 2025-06-16T18:05:07.813Z
Learning: The todo-backend project uses a custom pagination configuration system with DEFAULT_PAGINATION_SETTINGS
nested under REST_FRAMEWORK
in Django settings. This is not a DRF setting but a custom implementation used by their PaginationConfig
class in task_service.py
, serializers, and throughout their codebase. The configuration includes DEFAULT_PAGE_LIMIT
and MAX_PAGE_LIMIT
values.
todo/tests/integration/test_task_sorting_integration.py (3)
Learnt from: AnujChhikara
PR: #75
File: todo/tests/integration/test_tasks_delete.py:20-23
Timestamp: 2025-06-08T15:58:01.548Z
Learning: In the Real-Dev-Squad/todo-backend repository, integration tests focus on API behavior (HTTP status codes, response structure) while unit tests handle verification of actual database operations and state changes. Database verification should not be added to integration tests.
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
Learnt from: VaibhavSingh8
PR: #83
File: todo/tests/unit/services/test_user_service.py:37-43
Timestamp: 2025-06-17T18:59:14.368Z
Learning: UserRepository.create_or_update is a static method in the todo application, so it should be mocked directly on the class rather than on an instance.
todo/tests/unit/services/test_task_service.py (3)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
Learnt from: VaibhavSingh8
PR: #83
File: todo/tests/unit/services/test_user_service.py:37-43
Timestamp: 2025-06-17T18:59:14.368Z
Learning: UserRepository.create_or_update is a static method in the todo application, so it should be mocked directly on the class rather than on an instance.
Learnt from: Achintya-Chatterjee
PR: #52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 tracks the implementation of user authentication in the todo-backend project, which includes extracting user ID from request context to replace hardcoded placeholders like "system_patch_user" in todo/views/task.py.
todo/tests/unit/repositories/test_task_repository.py (1)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
todo/views/task.py (2)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
Learnt from: Achintya-Chatterjee
PR: #52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 tracks the implementation of user authentication in the todo-backend project, which includes extracting user ID from request context to replace hardcoded placeholders like "system_patch_user" in todo/views/task.py.
todo/tests/integration/test_tasks_pagination.py (1)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
todo/services/task_service.py (1)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
todo/repositories/task_repository.py (1)
Learnt from: AnujChhikara
PR: #119
File: todo/repositories/task_repository.py:149-154
Timestamp: 2025-07-09T19:59:31.694Z
Learning: In the todo-backend project, per product requirements, tasks marked as deleted (isDeleted=True) should still be returned in user task queries. The get_tasks_for_user method in TaskRepository should not filter out deleted tasks, unlike typical soft deletion patterns.
🧬 Code Graph Analysis (1)
todo/services/task_service.py (2)
todo/repositories/task_repository.py (3)
TaskRepository
(15-255)count
(85-104)get_tasks_for_user
(231-242)todo/dto/responses/get_tasks_response.py (1)
GetTasksResponse
(7-8)
🔇 Additional comments (16)
todo/tests/integration/test_tasks_pagination.py (1)
24-31
: LGTM: Test assertions correctly updated for new method signature.The addition of
status_filter=None
appropriately represents the default behavior for pagination tests. The improved formatting with one parameter per line enhances readability.Also applies to: 39-46
todo/tests/integration/test_task_sorting_integration.py (1)
27-29
: LGTM: Consistent updates across all sorting integration tests.All test assertions have been properly updated to include
status_filter=None
, maintaining focus on sorting functionality while accommodating the new method signature.Also applies to: 41-43, 56-58, 81-83, 95-97, 116-118
todo/tests/unit/repositories/test_task_repository.py (1)
100-100
: LGTM: Test correctly validates the new default filtering behavior.The assertion properly reflects that the repository now excludes tasks with status "DONE" by default, using the MongoDB query
{"status": {"$ne": "DONE"}}
.todo/tests/unit/views/test_task.py (1)
48-55
: LGTM: Comprehensive test updates for new service method signature.All view test assertions have been consistently updated to include
status_filter=None
, properly reflecting the updatedTaskService.get_tasks
method signature while maintaining focus on their respective testing scenarios.Also applies to: 67-74, 178-185, 196-203, 244-251, 261-268, 289-296, 312-319, 347-354, 364-371, 387-387
todo/tests/unit/services/test_task_service.py (3)
74-76
: LGTM! Test assertion correctly updated for new method signature.The addition of
status_filter=None
to theTaskRepository.list
mock assertion aligns with the updated method signature in the repository layer.
116-116
: LGTM! Consistent test assertion update.The test correctly includes the new
status_filter=None
parameter in the mock assertion.
299-301
: LGTM! All sorting test assertions consistently updated.All the sorting test methods correctly include
status_filter=None
in theirTaskRepository.list
mock assertions, maintaining consistency across the test suite.Also applies to: 311-313, 323-325, 335-337, 347-349, 359-361
todo/views/task.py (3)
60-66
: LGTM! Well-documented OpenAPI parameter for status filtering.The OpenAPI parameter definition is comprehensive and clearly describes the available status values and functionality.
84-84
: LGTM! Consistent parameter extraction.The status parameter is consistently extracted from query parameters using the validated serializer data.
Also applies to: 106-106
89-89
: LGTM! Proper parameter forwarding to service layer.The status_filter parameter is correctly passed to both
get_tasks_for_user
andget_tasks
service methods, maintaining the filtering capability across different API endpoints.Also applies to: 114-114
todo/services/task_service.py (3)
74-74
: LGTM! Service method signature properly updated.The
status_filter
parameter addition to theget_tasks
method is well-positioned and maintains backward compatibility with a default value ofNone
.
93-96
: LGTM! Consistent parameter forwarding to repository layer.Both
TaskRepository.list
andTaskRepository.count
calls correctly include thestatus_filter
parameter, ensuring consistent filtering behavior across task retrieval and counting operations.
671-671
: LGTM! User-specific task retrieval properly updated.The
get_tasks_for_user
method correctly accepts and forwards thestatus_filter
parameter to the repository method, maintaining consistency with the mainget_tasks
method.Also applies to: 674-674
todo/repositories/task_repository.py (3)
12-12
: LGTM: Import addition is appropriate.The TaskStatus import is correctly added to support the new status filtering functionality.
19-48
: LGTM: Status filtering implementation is consistent and well-structured.The method correctly implements status filtering with proper defaults. The base filter logic excludes DONE tasks by default and is consistently applied across all query scenarios (team, user, or general filtering).
85-104
: LGTM: Consistent status filtering implementation.The count method correctly mirrors the filtering logic from the list method, ensuring consistency across repository operations. The user query properly includes both created and assigned tasks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
Category | Issue | Status |
---|---|---|
Duplicated Status Filter Logic ▹ view | ✅ Fix detected |
Files scanned
File Path | Reviewed |
---|---|
todo/serializers/get_tasks_serializer.py | ✅ |
todo/repositories/task_repository.py | ✅ |
todo/views/task.py | ✅ |
todo/services/task_service.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
Date:
July 22, 2025
Developer Name: @Achintya-Chatterjee
Issue Ticket Number
DONE
tasks from GET tasks API responses #224Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
/v1/tasksScreen.Recording.2025-07-22.at.14.21.12.mov
/v1/tasks?profile=true
Screen.Recording.2025-07-22.at.14.24.06.mov
/v1/tasks?teamId=687f5323a73f6eed0a0c4a1d
Screen.Recording.2025-07-22.at.14.38.42.mov
/v1/tasks?status=DONE
Screen.Recording.2025-07-23.at.01.28.14.mov
/v1/tasks?profile=true&status=DONE
Screen.Recording.2025-07-23.at.01.30.28.mp4
/v1/tasks?teamId=687f5323a73f6eed0a0c4a1d&status=DONE
Screen.Recording.2025-07-23.at.01.37.30.mov
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Add status filtering to the tasks API, excluding tasks with a status of "DONE" by default, and update related tests and documentation.
Why are these changes being made?
This change is being made to enhance the tasks API functionality by allowing clients to filter tasks based on status while excluding "DONE" tasks by default, which aligns with common task management behavior where completed tasks are typically hidden unless specified otherwise. This approach maintains backward compatibility and provides flexibility by allowing optional inclusion of "DONE" tasks through explicit status filtering.