Chore: Update API response to replace task status "COMPLETED" with "DONE" #2410
Conversation
|
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 standardize the task status value returned by several endpoints in the Changes
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
controllers/tasks.js (1)
395-395: 💡 Verification agent🧩 Analysis chain
Consider updating COMPLETED references in status validation.
While these occurrences don't affect the API response transformation, for completeness and future-proofing, consider updating the status references in the validation logic as well.
- if (task.taskData.status === TASK_STATUS.DONE) { + if (task.taskData.status === TASK_STATUS.DONE) {- if (task.taskData.status === TASK_STATUS.COMPLETED && req.body.percentCompleted < 100) { + if (task.taskData.status === TASK_STATUS.DONE && req.body.percentCompleted < 100) {Let's verify if TASK_STATUS.COMPLETED and TASK_STATUS.DONE are defined in the constants:
Also applies to: 431-431
🏁 Script executed:
#!/bin/bash # Check if TASK_STATUS.COMPLETED or TASK_STATUS.DONE are defined in the constants rg -A 10 "TASK_STATUS\s*=" ../constants/tasks.jsLength of output: 112
🏁 Script executed:
#!/bin/bash # Locate any files named tasks.js to find where TASK_STATUS might be defined find . -type f -name "tasks.js" # Search across the entire repo for TASK_STATUS definitions or usages rg -n "TASK_STATUS" .Length of output: 9787
Update legacy COMPLETED status references to DONE
To ensure you’re using the current
TASK_STATUSvalues, replace all instances ofTASK_STATUS.COMPLETEDwithTASK_STATUS.DONEin the validation logic incontrollers/tasks.js. The check on line 395 is already usingDONEand does not require changes.• In controllers/tasks.js:
- Line 431
- if (task.taskData.status === TASK_STATUS.COMPLETED && req.body.percentCompleted < 100) { + if (task.taskData.status === TASK_STATUS.DONE && req.body.percentCompleted < 100) {- Line 432
- if (status === TASK_STATUS.COMPLETED || !status) { + if (status === TASK_STATUS.DONE || !status) {- Line 437
- (status === TASK_STATUS.COMPLETED || status === TASK_STATUS.VERIFIED) && + (status === TASK_STATUS.DONE || status === TASK_STATUS.VERIFIED) &&
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
controllers/tasks.js(5 hunks)
🔇 Additional comments (5)
controllers/tasks.js (5)
200-208: Good implementation of the COMPLETED to DONE transformation.This code correctly maps tasks with status "COMPLETED" to "DONE" in the fetchTasks response, aligning with the migration of task status terminology specified in the PR objectives.
212-212: LGTM - Response correctly uses transformed data.The spread operator is properly used to include the transformed task data in the response.
245-250: Consistent transformation applied in getUserTasks.The mapping logic correctly transforms tasks with status "COMPLETED" to "DONE" in the getUserTasks response, maintaining consistency with the fetchTasks implementation.
294-299: Consistent transformation applied in getSelfTasks.The mapping logic correctly transforms tasks with status "COMPLETED" to "DONE" in the getSelfTasks response, maintaining consistency with other endpoints despite this being a deprecated function.
304-304: Response correctly uses transformed data.The response now returns the statusChangedtasksData with the updated status values.
| if (taskData.status === "COMPLETED") { | ||
| taskData.status = "DONE"; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use consistent approach for task status transformation.
While this implementation works, it directly mutates the taskData object instead of creating a new object like in the other functions. This inconsistency in approach could make the code harder to maintain.
Consider using the same pattern as the other functions for consistency:
- if (taskData.status === "COMPLETED") {
- taskData.status = "DONE";
- }
+ if (taskData.status === "COMPLETED") {
+ taskData = { ...taskData, status: "DONE" };
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (taskData.status === "COMPLETED") { | |
| taskData.status = "DONE"; | |
| } | |
| if (taskData.status === "COMPLETED") { | |
| taskData = { ...taskData, status: "DONE" }; | |
| } |
vaishalimaheshwari2000
changed the title
vaishalimaheshwari2000
changed the title
vaishalimaheshwari2000
changed the title
vaishalimaheshwari2000
changed the title
controllers/tasks.js
Outdated
| const paginatedTasks = await fetchPaginatedTasks({ ...transformedQuery, prev, next, userFeatureFlag }); | ||
|
|
||
| const updatedData = { | ||
| ...paginatedTasks, // assuming your original JSON is stored in a variable called 'data' |
There was a problem hiding this comment.
Done , please check .
|
@vaishalimaheshwari2000 why make a controller level update on a separate pr and proposing to create a separate pr for db level ? Consider creating a transformer (for backword compatibility) , when returning any task , send it to the transformer firstly Note: From issue #2104 suggestion was said after wrapper "Once the DB has no more "COMPLETED" entries, remove the wrapper logic from step 1." if you do write the script then you can still leave the transformer though, can be useful (you can add a note as to why it was added ) |
Date: 30-04-2025
Developer Name: Vaishali
Issue Ticket Number
#2404
Description
This pull request updates the task status terminology from "COMPLETED" to "DONE" across the API responses and corresponding test cases. This change aligns with the updated terminology used across the system and improves clarity and consistency between backend logic, frontend usage, and developer understanding.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
ScreenShots
Screenshot 1
Screenshot 2
Screenshot 3
Screenshot 4
Test Coverage
Screenshot 1
Screenshot 2
Additional Notes