-
Notifications
You must be signed in to change notification settings - Fork 14
Refactor: get task query to handle deferred task logic #237
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
- Updated the TaskRepository to improve status filtering logic, allowing for more precise queries based on task status and deferred details. - Refactored the TaskService to adjust validation for deferring tasks, ensuring that deferred dates are correctly compared to due dates. - Modified integration and unit tests to reflect changes in task deferral logic and removed unused constants for cleaner code. These enhancements improve the accuracy of task management operations and ensure better validation during task deferral.
- Adjusted task status assignment in TaskService to account for deferred tasks, ensuring that tasks with deferred details are correctly marked as DEFERRED. - Updated the return statement to reflect the new task status logic, improving the accuracy of task status representation. - Initialized task status to TODO in the update payload for task modifications, enhancing consistency in task state management.
- Added logic to clear deferred details when a task's status is updated, ensuring that tasks with deferred information are correctly managed during status changes. - This change improves the accuracy of task updates and maintains consistency in task state management.
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 overhaul task status filtering logic in the repository, update service methods to refine deferral and status handling, and adjust related integration and unit tests to align with the new logic. Deferral and status transitions are now more tightly controlled, and tests use fixed date offsets instead of constants. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TaskService
participant TaskRepository
participant MongoDB
Client->>TaskService: Request to filter tasks (with status_filter)
TaskService->>TaskRepository: Call with status_filter
TaskRepository->>TaskRepository: Build complex filter (status, deferredDetails, dates)
TaskRepository->>MongoDB: Query with constructed filter
MongoDB-->>TaskRepository: Return matching tasks
TaskRepository-->>TaskService: Return tasks
TaskService-->>Client: Return tasks
sequenceDiagram
participant Client
participant TaskService
participant TaskRepository
participant MongoDB
Client->>TaskService: Request to defer a task (with deferred_till)
TaskService->>TaskService: Validate deferred_till < due_at
alt deferred_till >= due_at
TaskService-->>Client: Raise UnprocessableEntityException
else deferred_till < due_at
TaskService->>TaskRepository: Update task (set deferredDetails, status TODO)
TaskRepository->>MongoDB: Update operation
MongoDB-->>TaskRepository: Update result
TaskRepository-->>TaskService: Confirmation
TaskService-->>Client: Success response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
todo/repositories/task_repository.py
(1 hunks)todo/services/task_service.py
(6 hunks)todo/tests/integration/test_task_defer_api.py
(4 hunks)todo/tests/unit/repositories/test_task_repository.py
(1 hunks)todo/tests/unit/services/test_task_service.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 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#231
File: todo/repositories/task_repository.py:93-109
Timestamp: 2025-07-25T20:12:36.483Z
Learning: Issue #215 in the Real-Dev-Squad/todo-backend repository addresses the problem where tasks assigned to team members disappear from the team's todo list. The expected behavior is that tasks assigned to individual team members should still be visible in the team's todo list, which is implemented by aggregating both direct team assignments and member assignments in the _get_assigned_task_ids_for_team method.
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#227
File: todo/repositories/task_repository.py:0-0
Timestamp: 2025-07-23T19:26:43.747Z
Learning: In the todo-backend project, the get_tasks_for_user method in TaskRepository is intentionally designed to return only tasks assigned to the user (not tasks created by them), while the count method includes both tasks created by and assigned to the user. This behavioral difference is by design to serve different use cases.
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: 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: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#231
File: todo/repositories/task_repository.py:93-109
Timestamp: 2025-07-25T20:12:36.483Z
Learning: In the todo-backend project's TaskAssignmentRepository, the update_assignment method ensures exclusive task assignments by deactivating all current active assignments for a task before creating a new assignment. This prevents any task from being simultaneously assigned to both a team and individual team members.
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.
📚 Learning: in the todo-backend project, the get_tasks_for_user method in taskrepository is intentionally design...
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#227
File: todo/repositories/task_repository.py:0-0
Timestamp: 2025-07-23T19:26:43.747Z
Learning: In the todo-backend project, the get_tasks_for_user method in TaskRepository is intentionally designed to return only tasks assigned to the user (not tasks created by them), while the count method includes both tasks created by and assigned to the user. This behavioral difference is by design to serve different use cases.
Applied to files:
todo/tests/unit/repositories/test_task_repository.py
todo/repositories/task_repository.py
📚 Learning: in the todo-backend project, per product requirements, tasks marked as deleted (isdeleted=true) shou...
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.
Applied to files:
todo/tests/unit/repositories/test_task_repository.py
todo/repositories/task_repository.py
📚 Learning: in the real-dev-squad/todo-backend repository, integration tests focus on api behavior (http status ...
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.
Applied to files:
todo/tests/unit/repositories/test_task_repository.py
📚 Learning: the todo-backend project uses a global exception handler that automatically handles tasknotfoundexce...
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:98-112
Timestamp: 2025-06-02T17:02:22.424Z
Learning: The todo-backend project uses a global exception handler that automatically handles TaskNotFoundException, BsonInvalidId, ValueError with ApiErrorResponse, and DRFValidationError. Views should avoid try-catch blocks and let exceptions bubble up to the global handler for consistent error formatting and status codes.
Applied to files:
todo/tests/integration/test_task_defer_api.py
todo/services/task_service.py
📚 Learning: in the real-dev-squad/todo-backend project, the get v1/labels endpoint is designed to return only th...
Learnt from: shobhan-sundar-goutam
PR: Real-Dev-Squad/todo-backend#95
File: todo/services/label_service.py:86-91
Timestamp: 2025-07-02T18:44:05.550Z
Learning: In the Real-Dev-Squad/todo-backend project, the GET v1/labels endpoint is designed to return only three fields in the response: id, name, and color. The prepare_label_dto method in todo/services/label_service.py intentionally excludes other LabelDTO fields like createdAt, updatedAt, createdBy, and updatedBy from the API response.
Applied to files:
todo/services/task_service.py
📚 Learning: the todo-backend project uses a custom pagination configuration system with `default_pagination_sett...
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.
Applied to files:
todo/services/task_service.py
📚 Learning: the constant repositoryerrors.user_operation_failed in todo/constants/messages.py is defined as "use...
Learnt from: VaibhavSingh8
PR: Real-Dev-Squad/todo-backend#81
File: todo/repositories/user_repository.py:47-55
Timestamp: 2025-06-16T19:35:44.948Z
Learning: The constant RepositoryErrors.USER_OPERATION_FAILED in todo/constants/messages.py is defined as "User operation failed" without any placeholder formatting like {0}.
Applied to files:
todo/services/task_service.py
🧬 Code Graph Analysis (2)
todo/repositories/task_repository.py (1)
todo/constants/task.py (1)
TaskStatus
(4-9)
todo/tests/integration/test_task_defer_api.py (1)
todo/constants/task.py (2)
TaskPriority
(12-15)TaskStatus
(4-9)
🔇 Additional comments (3)
todo/tests/unit/repositories/test_task_repository.py (1)
100-105
: LGTM!The test correctly validates the new complex filter structure with
$and
and$or
operators, aligning with the refactored_build_status_filter
method.todo/tests/unit/services/test_task_service.py (1)
1027-1027
: LGTM!The test correctly validates the new deferral constraint where tasks cannot be deferred to or past their due date.
todo/tests/integration/test_task_defer_api.py (1)
6-6
: LGTM!The removal of
MINIMUM_DEFERRAL_NOTICE_DAYS
and the use of fixed day offsets simplifies the test cases while maintaining test coverage for all deferral scenarios.Also applies to: 55-55, 79-79, 82-82, 131-131
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 |
---|---|---|
Incorrect DONE Status Filter Logic ▹ view |
Files scanned
File Path | Reviewed |
---|---|
todo/repositories/task_repository.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.
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.
Test coverage screenshot is missing
""" | ||
if status_filter: | ||
if status_filter == TaskStatus.DONE.value: | ||
return {} # No status filtering, include all 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.
May I know why you deleted this ??
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.
It's not deleted ,we have modified it to not to include deferred task
added |
Date: 1 Aug, 2025
Developer Name: @AnujChhikara
Issue Ticket Number
Description
deferredDetails
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
defer-f.mp4
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Refactor the task query logic to accurately handle task status, including deferred tasks, and adjust related service handling, validation, and tests.
Why are these changes being made?
The refactoring aims to enhance the task filtering system to properly accommodate tasks with 'deferred' status by using more precise date-based evaluations. The previous logic did not adequately reflect tasks that are deferred until a future date. Changes in service logic ensure tasks are updated correctly based on their deferred status, while testing updates verify that deferred logic behaves as expected in different scenarios. This approach addresses deferred task logic more rigorously and aligns deferred status handling with business rules.