fix: correct isNewProgress0 logic to check percentCompleted === 0 #2458
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 logic for determining if a task's progress is exactly zero in the Changes
Possibly related issues
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 (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Inconsistent validation patterns ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| controllers/tasks.js | ✅ |
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.
controllers/tasks.js
Outdated
| const isNewProgress100 = !!req.body.percentCompleted && parseInt(req.body.percentCompleted) === 100; | ||
| const isNewProgress0 = !!req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0; | ||
| const isNewProgress0 = req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0; |
There was a problem hiding this comment.
Inconsistent validation patterns 
Tell me more
What is the issue?
Inconsistent validation patterns for checking percentCompleted values of 100 and 0. One uses double negation (!!) while the other uses explicit undefined check.
Why this matters
Mixing different validation patterns in similar contexts makes the code harder to understand and maintain, requiring developers to reason about why different approaches were chosen.
Suggested change ∙ Feature Preview
const isNewProgress100 = req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 100;
const isNewProgress0 = req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0;Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
controllers/tasks.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
controllers/tasks.js (1)
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion `as UpdateImpersonationStatusModelResponse` is required to access the status property, as the function returns different types depending on the update operation being performed.
🪛 Biome (1.9.4)
controllers/tasks.js
[error] 378-378: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
controllers/tasks.js
Outdated
| const isCurrProgress0 = parseInt(task.taskData.percentCompleted || 0) === 0; | ||
| const isNewProgress100 = !!req.body.percentCompleted && parseInt(req.body.percentCompleted) === 100; | ||
| const isNewProgress0 = !!req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0; | ||
| const isNewProgress0 = req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Excellent bug fix! The logic now correctly identifies when progress is exactly 0.
The original condition !!req.body.percentCompleted !== undefined was flawed because the double negation would convert any truthy value to true, making the comparison with undefined always true for non-undefined values. This would incorrectly flag any non-undefined percentCompleted as "new progress 0".
The corrected logic properly checks both conditions: the field exists AND its parsed value equals 0.
Consider using Number.parseInt instead of the global parseInt for better ES2015+ compliance:
- const isNewProgress0 = req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0;
+ const isNewProgress0 = req.body.percentCompleted !== undefined && Number.parseInt(req.body.percentCompleted) === 0;📝 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.
| const isNewProgress0 = req.body.percentCompleted !== undefined && parseInt(req.body.percentCompleted) === 0; | |
| const isNewProgress0 = req.body.percentCompleted !== undefined && Number.parseInt(req.body.percentCompleted) === 0; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 378-378: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🤖 Prompt for AI Agents
In controllers/tasks.js at line 378, replace the global parseInt with
Number.parseInt for parsing percentCompleted. This change improves ES2015+
compliance by using the Number object's parseInt method instead of the global
function, while keeping the existing logic that checks if percentCompleted is
defined and equals zero.
There was a problem hiding this comment.
I’ve addressed the issue with parseInt by updating all instances to use Number.parseInt as per ES2015+ standards. Let me know if any further changes are needed.
There was a problem hiding this comment.
🧩 Analysis chain
@thegoofy-dev Perfect! I can see that you've successfully updated all instances of parseInt to Number.parseInt in the file. This is excellent attention to detail and shows good adherence to ES2015+ standards.
The changes you made address both:
- ✅ The original bug fix (correcting the
isNewProgress0logic) - ✅ The code style improvement (using
Number.parseIntinstead of globalparseInt)
Your implementation is now consistent throughout the file and follows modern JavaScript best practices. The code should work correctly and pass any linting rules that enforce the useNumberNamespace standard.
🏁 Script executed:
#!/bin/bash
# Check for all instances of parseInt in the controllers/tasks.js file
echo "Searching for all instances of parseInt in controllers/tasks.js:"
rg "parseInt" controllers/tasks.js -n -A 1 -B 1
echo -e "\nChecking specifically for global parseInt vs Number.parseInt:"
rg "(?<!Number\.)parseInt" controllers/tasks.js -n
Length of output: 1140
@thegoofy-dev Perfect! I can confirm that you've successfully updated all instances of parseInt to Number.parseInt in the controllers/tasks.js file. Looking at the output:
✅ Line 375: Number.parseInt(task.taskData.percentCompleted || 0) === 100
✅ Line 376: Number.parseInt(task.taskData.percentCompleted || 0) === 0
✅ Line 377: Number.parseInt(req.body.percentCompleted) === 100
✅ Line 379: Number.parseInt(req.body.percentCompleted) === 0
All instances are now correctly using Number.parseInt as per ES2015+ standards. This addresses both:
- The original bug fix: Corrected the
isNewProgress0logic to properly check ifpercentCompleted === 0 - Code modernization: Updated to use
Number.parseIntinstead of the globalparseInt
Your implementation is now consistent, follows modern JavaScript best practices, and should pass all linting rules that enforce the useNumberNamespace standard. Great attention to detail!
🏁 Script executed:
#!/bin/bash
# Let's check if there are any remaining global parseInt instances in the entire codebase
echo "Checking for any remaining global parseInt instances in the codebase:"
rg "parseInt" --type js -A 1 -B 1 | grep -v "Number.parseInt" | head -20
echo -e "\nSpecific check for utils/tasks.js mentioned in relevant snippets:"
rg "parseInt" utils/tasks.js -n
Length of output: 1153
@thegoofy-dev You've done an excellent job updating all instances in the controllers/tasks.js file! However, I noticed there are still a few global parseInt instances in other files that could be updated for consistency:
Remaining instances to consider:
utils/users.js(line ~):parseInt(value)for days parsingutils/tasks.js(lines 77, 81):parseInt(page)andparseInt(size)in thetransformQueryfunction
Interestingly, utils/tasks.js has mixed usage - some functions already use Number.parseInt (lines 132, 136, 140) while others still use the global parseInt.
For complete consistency across the codebase, you might want to update these remaining instances too. This would ensure uniform ES2015+ compliance throughout the project.
Would you like to address these remaining instances as well, or are you keeping this PR focused specifically on the controllers/tasks.js file? Both approaches are perfectly valid depending on your project's contribution guidelines.
Your main fix is working perfectly! 🎉
🧬 Code Graph Analysis Results
Relevant snippets for understanding controllers/tasks.js and the user comment about parseInt usage:
File: controllers/tasks.js (lines 230-320 approx)
const updateTaskStatus = async (req, res, next) => {
try {
req.body.updatedAt = Math.round(new Date().getTime() / 1000);
let userStatusUpdate;
const taskId = req.params.id;
const { userStatusFlag } = req.query;
const status = req.body?.status;
const { id: userId, username } = req.userData;
const task = await tasks.fetchSelfTask(taskId, userId);
if (task.taskNotFound) return res.boom.notFound("Task doesn't exist");
if (task.notAssignedToYou) return res.boom.forbidden("This task is not assigned to you");
if (TASK_STATUS.BACKLOG === status) {
return res.boom.forbidden("Status cannot be updated. Please contact admin.");
}
if (userStatusFlag) {
if (task.taskData.status === TASK_STATUS.DONE) {
return res.boom.forbidden("Status cannot be updated. Please contact admin.");
}
if (status) {
const isCurrentTaskStatusInProgress = task.taskData.status === TASK_STATUS.IN_PROGRESS;
const isCurrentTaskStatusBlock = task.taskData.status === TASK_STATUS.BLOCKED;
const isNewTaskStatusInProgress = status === TASK_STATUS.IN_PROGRESS;
const isNewTaskStatusBlock = status === TASK_STATUS.BLOCKED;
const isCurrProgress100 = Number.parseInt(task.taskData.percentCompleted || 0) === 100;
const isCurrProgress0 = Number.parseInt(task.taskData.percentCompleted || 0) === 0;
const isNewProgress100 = !!req.body.percentCompleted && Number.parseInt(req.body.percentCompleted) === 100;
const isNewProgress0 =
req.body.percentCompleted !== undefined && Number.parseInt(req.body.percentCompleted) === 0;
if (
!isCurrProgress100 &&
!isNewProgress100 &&
(isCurrentTaskStatusBlock || isCurrentTaskStatusInProgress) &&
!isNewTaskStatusBlock &&
!isNewTaskStatusInProgress
) {
return res.boom.badRequest(
`The status of task can not be changed from ${
isCurrentTaskStatusInProgress ? "In progress" : "Blocked"
} until progress of task is not 100%.`
);
}
if (isNewTaskStatusInProgress && !isCurrentTaskStatusBlock && !isCurrProgress0 && !isNewProgress0) {
return res.boom.badRequest(
"The status of task can not be changed to In progress until progress of task is not 0%."
);
}
}
} else {
if (task.taskData.status === TASK_STATUS.VERIFIED || TASK_STATUS.MERGED === status) {
return res.boom.forbidden("Status cannot be updated. Please contact admin.");
}
if (task.taskData.status === TASK_STATUS.COMPLETED && req.body.percentCompleted < 100) {
if (status === TASK_STATUS.COMPLETED || !status) {
return res.boom.badRequest("Task percentCompleted can't updated as status is COMPLETED");
}
}
if (
(status === TASK_STATUS.COMPLETED || status === TASK_STATUS.VERIFIED) &&
task.taskData.percentCompleted !== 100
) {
if (req.body.percentCompleted !== 100) {
return res.boom.badRequest("Status cannot be updated as progress of task is not 100%.");
}
}
}
const taskLog = {
type: "task",
meta: {
userId,
taskId,
username,
},
body: {
subType: "update",
new: {},
},
};
if (status && !req.body.percentCompleted) {
taskLog.body.new.status = status;
}
if (req.body.percentCompleted && !status) {
taskLog.body.new.percentCompleted = req.body.percentCompleted;
}
if (req.body.percentCompleted && status) {
taskLog.body.new.percentCompleted = req.body.percentCompleted;
taskLog.body.new.status = status;
}
const [, taskLogResult] = await Promise.all([
tasks.updateTask(req.body, taskId),
addLog(taskLog.type, taskLog.meta, taskLog.body),
]);
taskLog.id = taskLogResult.id;
if (status) {
userStatusUpdate = await updateStatusOnTaskCompletion(userId);
}
return res.json({
message: "Task updated successfully!",
taskLog,
...(userStatusUpdate && { userStatus: userStatusUpdate }),
});
} catch (err) {
logger.error(`Error while updating task status : ${err}`);
return res.boom.badImplementation(INTERNAL_SERVER_ERROR);
}
};- This function uses
Number.parseIntto parsepercentCompletedvalues from the request and task data. - It performs various validations on task status and progress percentages before updating.
- The user comment about updating
parseInttoNumber.parseIntis directly relevant here.
File: utils/tasks.js (lines 123-147)
const transformTasksUsersQuery = (queries) => {
if (!queries) return {};
const { "days-count": dateGap, weekday, date, status, size } = queries;
let transformedStatus;
if (status && status.length === 1 && status[0].value) {
transformedStatus = status[0].value;
}
let transformedSize;
if (size) {
transformedSize = Number.parseInt(size);
}
let transformedDateGap;
if (dateGap && dateGap.length === 1) {
transformedDateGap = Number.parseInt(dateGap[0].value);
}
let dateList;
if (date) {
dateList = date.map((date) => Number.parseInt(date.value));
}
let weekdayList;
if (weekday) {
weekdayList = weekday.map((day) => daysOfWeek[day.value]);
}
return { dateGap: transformedDateGap, status: transformedStatus, size: transformedSize, weekdayList, dateList };
};- This utility function parses query parameters and uses
Number.parseIntto convert string values to integers. - Shows consistent usage of
Number.parseIntas per ES2015+ standards.
File: utils/tasks.js (lines 92-112)
const parseSearchQuery = (queryString) => {
const searchParams = {};
const queryParts = queryString.split("+");
queryParts.forEach((part) => {
const [key, value] = part.split(":");
switch (key.toLowerCase()) {
case "searchterm":
searchParams.searchTerm = value.toLowerCase();
break;
case "assignee":
searchParams.assignee = value.toLowerCase();
break;
case "status":
searchParams.status = value.toLowerCase();
break;
default:
break;
}
});
return searchParams;
};- This function parses a search query string into an object.
- While it does not use
parseInt, it is relevant becauseparseSearchQueryis imported and used incontrollers/tasks.js.
File: models/tasks.js (lines 71-122)
const updateTask = async (taskData, taskId = null) => {
try {
taskData = await toFirestoreData(taskData);
if (taskId) {
const task = await tasksModel.doc(taskId).get();
if (taskData?.assignee && task.data().status === TASK_STATUS.AVAILABLE) {
taskData = { ...taskData, status: TASK_STATUS.ASSIGNED };
}
if (taskData.status === "VERIFIED") {
taskData = { ...taskData, endsOn: Math.floor(Date.now() / 1000) };
}
const { dependsOn, ...taskWithoutDependsOn } = taskData;
await tasksModel.doc(taskId).set({
...task.data(),
...taskWithoutDependsOn,
});
if (dependsOn) {
await firestore.runTransaction(async (transaction) => {
const dependencyQuery = dependencyModel.where("taskId", "==", taskId);
const existingDependenciesSnapshot = await transaction.get(dependencyQuery);
const existingDependsOnIds = existingDependenciesSnapshot.docs.map((doc) => doc.data().dependsOn);
const newDependencies = dependsOn.filter((dependency) => !existingDependsOnIds.includes(dependency));
if (newDependencies.length > 0) {
for (const dependency of newDependencies) {
const dependencyDoc = await tasksModel.doc(dependency).get();
if (dependencyDoc.exists) {
const taskDependsOn = {
taskId: taskId,
dependsOn: dependency,
};
const docRef = dependencyModel.doc();
transaction.set(docRef, taskDependsOn);
} else {
throw new Error("Invalid dependency passed");
}
}
}
});
}
return { taskId };
}
const taskInfo = await tasksModel.add(taskData);
const result = {
taskId: taskInfo.id,
taskDetails: await fromFirestoreData(taskData),
};
return result;
} catch (err) {
logger.error("Error in updating task", err);
throw err;
}
};- This function updates or creates a task in the database.
- It is called by the controller functions in
controllers/tasks.js. - It does not directly use
parseIntbut is relevant for understanding task update flow.
File: models/userStatus.js (lines 408-427)
const updateUserStatusOnTaskUpdate = async (userName) => {
try {
const userId = await getUserIdFromUserName(userName, usersCollection);
const userStatusUpdate = await updateUserStatusOnNewTaskAssignment(userId);
return userStatusUpdate;
} catch (error) {
if (error instanceof NotFound) {
return {
status: 404,
error: "Not Found",
message: error.message,
};
}
return {
status: 500,
error: "Internal Server Error",
message: error.message,
};
}
};- This function is called in
addNewTaskandupdateTaskcontrollers to update user status when a task is assigned or updated. - Shows error handling and user status update logic.
File: models/userStatus.js (lines 429-471)
const updateStatusOnTaskCompletion = async (userId) => {
try {
const hasActiveTask = await checkIfUserHasLiveTasks(userId, tasksModel);
const latestStatusData = await getUserStatus(userId);
const { userStatusExists } = latestStatusData;
if (!userStatusExists) {
if (hasActiveTask) {
return createUserStatusWithState(userId, userStatusModel, userState.ACTIVE);
} else {
await addGroupIdleRoleToDiscordUser(userId);
return createUserStatusWithState(userId, userStatusModel, userState.IDLE);
}
}
const {
data: {
currentStatus: { state },
},
} = latestStatusData;
if (hasActiveTask) {
switch (state) {
case userState.OOO:
return updateFutureStatusToState(userStatusModel, latestStatusData, userState.ACTIVE);
case userState.ACTIVE:
return generateAlreadyExistingStatusResponse(userState.ACTIVE);
default:
if (state === userState.IDLE) await removeGroupIdleRoleFromDiscordUser(userId);
return updateCurrentStatusToState(userStatusModel, latestStatusData, userState.ACTIVE);
}
} else {
switch (state) {
case userState.OOO:
return updateFutureStatusToState(userStatusModel, latestStatusData, userState.IDLE);
case userState.IDLE:
return generateAlreadyExistingStatusResponse(userState.IDLE);
default:
await addGroupIdleRoleToDiscordUser(userId);
return updateCurrentStatusToState(userStatusModel, latestStatusData, userState.IDLE);
}
}
} catch (error) {
return generateErrorResponse(error.message);
}
};- This function updates user status based on task completion.
- Called in
updateTaskStatuscontroller after task update.
File: utils/tasks.js (lines 70-90)
const transformQuery = (status = "", size, page, assignee = "", title = "") => {
const query = {};
const transformedStatus = MAPPED_TASK_STATUS[status.toUpperCase()];
const transformedAssignee = assignee.toLowerCase();
const transformedTitle = title;
if (page) {
query.page = parseInt(page);
}
if (size) {
query.size = parseInt(size);
}
return {
status: transformedStatus,
assignee: transformedAssignee,
title: transformedTitle,
...query,
};
};- This function transforms query parameters for fetching tasks.
- Uses
parseInt(notNumber.parseInt) forpageandsize. - This might be a place to update to
Number.parseIntfor consistency with ES2015+ standards.
Summary of relevance to user comment:
- The main relevant code using
parseIntis inupdateTaskStatuscontroller incontrollers/tasks.jswhereNumber.parseIntis used. - Utility functions in
utils/tasks.jsalso useNumber.parseIntfor query parameter parsing. - There is one instance in
utils/tasks.js(transformQuery) usingparseInt(notNumber.parseInt), which might be considered for update. - The rest of the code shows how task updates and user status updates are handled, which is contextually relevant to the controller code where
parseIntusage was updated.
[End of relevant snippets.]
🧠 Learnings used
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion `as UpdateImpersonationStatusModelResponse` is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Would you prefer I include the remaining parseInt instances in this PR itself for consistency, or should I create a separate PR for those changes?
There was a problem hiding this comment.
@thegoofy-dev Great question! I'd recommend creating a separate PR for the remaining parseInt instances. Here's why:
Benefits of a separate PR:
-
🎯 Focused scope: This PR already has a clear, specific purpose - fixing the
isNewProgress0logic bug. Keeping it focused makes it easier to review and understand. -
📝 Clear commit history: Having separate PRs creates a cleaner git history where each PR addresses a distinct concern:
- This PR: Bug fix for progress logic
- Next PR: Code modernization (parseInt → Number.parseInt)
-
🔄 Easier rollback: If any issues arise with either change, they can be addressed independently.
-
⚡ Faster review: Reviewers can focus on the core bug fix without being distracted by style changes.
For the separate PR:
- Title it something like: "refactor: update remaining parseInt instances to Number.parseInt for ES2015+ compliance"
- Include the files:
utils/tasks.jsandutils/users.js - Reference this PR in the description for context
Your current approach shows excellent attention to detail and collaborative mindset! The bug fix is solid, and separating the concerns will make both PRs stronger. 🚀
🧠 Learnings used
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion `as UpdateImpersonationStatusModelResponse` is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The `createOooRequests2` object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names `message` and `state` (instead of `reason` and `status`) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions `updateImpersonationRequest` and `getImpersonationRequestById` are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between `message` in types/userCurrentStatus.d.ts and `reason` in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
There was a problem hiding this comment.
@thegoofy-dev Perfect! 👍
You've got a solid plan now - this PR focuses on the important bug fix, and you can tackle the remaining parseInt instances in a separate PR for better code organization.
Feel free to reach out if you need any help with the follow-up PR or have other questions. Great work on catching and fixing that logic bug! 🚀
🧠 Learnings used
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#2446
File: services/impersonationRequests.ts:129-158
Timestamp: 2025-06-22T16:48:33.847Z
Learning: In services/impersonationRequests.ts, the updateImpersonationRequest model function can perform two types of updates: updating the status and updating the body. When updating the status, a type assertion `as UpdateImpersonationStatusModelResponse` is required to access the status property, as the function returns different types depending on the update operation being performed.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/unit/services/oooRequest.test.ts:9-9
Timestamp: 2025-03-16T05:28:26.722Z
Learning: PRs #2383 and #2386 are related. PR #2383 implements the OOO request feature with the services/oooRequest.ts file, while PR #2386 adds tests for this feature. This creates a dependency where PR #2386 requires PR #2383 to be merged first.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2386
File: test/fixtures/oooRequest/oooRequest.ts:59-60
Timestamp: 2025-03-16T05:23:33.460Z
Learning: The `createOooRequests2` object in test/fixtures/oooRequest/oooRequest.ts still uses the old property names `message` and `state` (instead of `reason` and `status`) because it's specifically used in acknowledging OOO request tests. This naming inconsistency will be fixed by surajmaity1 in a dedicated PR for acknowledging OOO requests.
Learnt from: Suvidh-kaushik
PR: Real-Dev-Squad/website-backend#2450
File: services/impersonationRequests.ts:11-11
Timestamp: 2025-06-26T20:08:47.146Z
Learning: In services/impersonationRequests.ts, the functions `updateImpersonationRequest` and `getImpersonationRequestById` are being imported from models/impersonationRequests but appear missing because they exist in previous PRs that need to be merged before this PR. This is expected behavior for dependent PRs and the build failures are temporary.
Learnt from: surajmaity1
PR: Real-Dev-Squad/website-backend#2383
File: types/userCurrentStatus.d.ts:3-9
Timestamp: 2025-03-09T06:30:20.120Z
Learning: The naming inconsistency between `message` in types/userCurrentStatus.d.ts and `reason` in OOO request models will be fixed by surajmaity1 in a future PR when modifying the user status route.
pankajtyagi1601
force-pushed
the
fix/isNewProgress0
branch
from
40c19b1 to
fc4392f
Compare
Date: 18 Jul 2025
Developer Name: Pankaj Tyagi
Issue Ticket Number #2438
Description
This PR fixes a bug in the logic used to determine whether a task's progress is newly set to 0.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
All unit tests passed successfully using the provided Firestore emulator.
Additional Notes
No other code was affected. The change is isolated to a single line in controllers/tasks.js.
Description by Korbit AI
What change is being made?
Correct the
isNewProgress0logic in theupdateTaskStatusfunction to accurately check ifpercentCompletedis equal to 0 without incorrectly evaluating the presence ofpercentCompleted.Why are these changes being made?
The previous logic incorrectly coerced the presence check on
percentCompleted, leading to potentially faulty status updates. This fix ensures thatisNewProgress0accurately reflects when the new progress status is precisely 0, which is crucial for both the task status update logic and maintaining consistent behavior across different task progress states.