Fix idle 7 days + calculation logic#2584
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR introduces a utility function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@models/discordactions.js`:
- Around line 696-697: The current conditional in the idle-days check (if
(idleDays <= 7) return;) incorrectly excludes users with exactly 7 idle days;
update the boundary to be inclusive of 7 days by changing the condition to check
for strictly less than 7 (i.e., use idleDays < 7) in the function handling the
"group-idle-7d+" logic so users with exactly 7 days are included; locate the
conditional that references idleDays in models/discordactions.js and replace the
comparison accordingly.
- Around line 687-695: The idle-7d scan is doing an extra Firestore read per
user by calling userStatusModel.doc(userStatus.id).get() inside the Promise.all;
remove that read and instead ensure the userStatus objects already include the
fields needed by computeIdleDaysExcludingOOO (idleWindowStartedAt, lastOooFrom,
lastOooUntil, and currentStatus.from). Update the producer that builds
userStatus (the getAllUserStatus function in models/userStatus.js) to populate
those fields on each userStatus so the code in models/discordactions.js can call
computeIdleDaysExcludingOOO using the existing userStatus object without any
further doc.get().
In `@models/userStatus.js`:
- Around line 256-258: When transitioning back to ACTIVE, clear any stale idle
anchor by resetting newStatusData.idleWindowStartedAt to null/undefined;
specifically, in the state-transition logic that checks requestedNextState and
previousState (the branch where requestedNextState === userState.ACTIVE and
previousState === userState.IDLE or similar), set
newStatusData.idleWindowStartedAt = null (or delete it) so the idle-day
calculation doesn't use a stale value; update the same control paths that set
idleWindowStartedAt (the branch using currentStatus?.from ??
currentStatus?.updatedAt) to ensure symmetry.
In `@test/unit/utils/userStatus.test.js`:
- Around line 114-139: The time-based tests calling computeIdleDaysExcludingOOO
use multiple Date.now() calls which can drift; fix each affected "it" block (the
tests "should return total idle days when no OOO period", "should exclude last
OOO period from idle days", and "should fall back to currentStatusFrom when
idleWindowStartedAt is missing") by capturing a single const now = Date.now() at
the top of the test and deriving windowStart, oooFrom, oooUntil, and
currentStatusFrom from that single now before calling
computeIdleDaysExcludingOOO so all timestamps are deterministic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
models/discordactions.jsmodels/userStatus.jstest/unit/utils/userStatus.test.jsutils/userStatus.js
…in computeIdleDaysExcludingOOO
…ns in user status
… enhance user status management with new OOO period handling
| } | ||
| } | ||
| } | ||
| const lastOooUntilUpdate = resolveLastOooUntil({ |
There was a problem hiding this comment.
previously we were storing lastOooUntilUpdate in userStatus, since we are removing that here can you please confirm that isn't being used anywhere?
There was a problem hiding this comment.
lastOooUntilUpdate isn't being used anywhere anymore
the fields lastOooFrom, lastOooUntil, and oooPeriods are effectively dead in terms of being updated in the usersStatus collection. The new getApprovedOooPeriods() handles everything dynamically via the requests collection now
There was a problem hiding this comment.
website-backend/models/discordactions.js
Line 1156 in a07167f
can you please check getMissedProgressUpdatesUsers in discordactions
There was a problem hiding this comment.
lastOooUntil is still stored in the userStatus collection and is used by getMissedProgressUpdatesUsers (around line 1180) to apply a grace period check.
Even though we stopped updating lastOooFrom/lastOooUntil during the refactor, the field was intentionally kept for backward compatibility. Existing users may still have valid lastOooUntil values, which the cron job relies on to grant grace periods.
In the future, this can be refactored to use the new requests collection (getApprovedOooPeriods()), but for now, everything remains safe and functional, and no behavior is broken
Since now we are just targeting the idle user, not the progress calculation, so I didn't update it here
…on for accurate group idle 7d+ tracking, alongside new documentation.
…ns and update user status handling accordingly
…hance OOO state management
…l` for improved OOO state transition handling
…in user status management and update related tests
…rove time handling in idle user calculations
…ons for user idle state
| const snapshot = await requestsModel | ||
| .where("requestedBy", "==", userId) | ||
| .where("type", "==", "OOO") | ||
| .where("until", ">=", windowStart) | ||
| .get(); | ||
|
|
||
| const periods = []; | ||
| snapshot.forEach((doc) => { | ||
| const data = doc.data(); | ||
| const isApproved = data.state === "APPROVED"; | ||
| if (!isApproved) return; | ||
|
|
||
| const from = normalizeTimestamp(data.from); | ||
| const until = normalizeTimestamp(data.until); |
There was a problem hiding this comment.
@vinit717 This can be part of the query itself, get the approved requests only
prakashchoudhary07
left a comment
prakashchoudhary07
left a comment
There was a problem hiding this comment.
Approving it for now.
A follow-up PR will be raised by @vinit717 for improvements, idle time edge case fix, and also for the issue where users come back early from OOO, then the status for them
4 participants
Date: 27-02-2026
Developer Name: @vinit717
Issue Ticket Number
Description
Fix the 7-day+ idle users calculation logic
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Test Coverage
Additional Notes