Refactor: improve idle user checks and error handling in Discord actions

[vinit717]

Date: 10-2-2026

Developer Name: @vinit717

Description

Streamline check for idle user tagging

Documentation Updated?

• Yes
• No

Under Feature Flag

• Yes
• No

Database Changes

• Yes
• No

Breaking Changes

• Yes
• No

Development Tested?

• Yes
• No

Screenshots

Screenshot 1

Test Coverage

Screenshot 1

Additional Notes

[coderabbitai]

Walkthrough

Adds defensive guards and identity validation to Discord idle-user processing: introduces a discordMemberIds set, ensures idle group and discordId presence, switches downstream references to verified discordId, tightens archived/role checks, and replaces error rethrows with contextual logging.

Changes

Cohort / File(s)	Summary
Discord actions logic models/discordactions.js	Adds guard ensuring idle group exists; introduces discordMemberIds Set to track verified Discord member IDs; validates and uses discordId for downstream lookups; uses optional chaining for role access; refines archived/idle determination; logs errors with user context instead of rethrowing.
Unit tests (expectations updated) test/unit/models/discordactions.test.js	Adjusts test expectations to reflect fewer idle/archived users and updated role-add/remove counts; minor formatting cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: enhance user idle status checks in Discord actions #2566: Modifies idle-user logic in the same area (adds active-task/OOO gating before marking users idle).

Suggested reviewers

• AnujChhikara
• iamitprakash
• MayankBansal12

Poem

🐇 I hopped through code to find a trace,
Verified IDs in tidy place.
Guards set up, no more surprise,
Roles read safe through careful eyes.
Logs now whisper who went wrong—hooray!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: refactoring to improve idle user checks and error handling in Discord actions, which directly aligns with the substantial code modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The pull request description is related to the changeset, describing improvements to idle user checks and error handling in Discord actions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/idle-user-tag

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

models/discordactions.js (1)

667-708: ⚠️ Potential issue | 🟠 Major

updateIdle7dUsersOnDiscord has the same vulnerabilities that were just fixed in updateIdleUsersOnDiscord.

The refactored guards (optional chaining on roles, discordId extraction, discordMemberIds set, null-safe userData access, graceful error handling) were not applied here. Specific issues:

• Line 668-670: discordUser.roles.includes(...) — no optional chaining; crashes if roles is undefined.
• Line 692: userData.data().roles.archived — accessed before the .exists check on line 693; throws if data() returns null.
• Line 707: throw new Error(...) inside Promise.all map — one user failure aborts processing of all remaining users.

Apply the same defensive pattern used in updateIdleUsersOnDiscord.

🤖 Fix all issues with AI agents

In `@models/discordactions.js`:
- Around line 417-436: The module-level array allMavens is leaking state across
invocations; move its declaration into the function(s) that use it (e.g., inside
updateIdleUsersOnDiscord and updateIdle7dUsersOnDiscord) or explicitly reset it
at the start of those functions before any .push() calls so each run starts with
an empty list; update any usage sites (including the
!allMavens.includes(discordId) check and where allMavens is passed/returned) to
use the locally-scoped variable or newly-passed parameter to avoid stale
entries.

In `@test/unit/models/discordactions.test.js`:
- Around line 1082-1089: The test title for the case exercising
updateIdleUsersOnDiscord is out of sync with its expectations; either change the
string describing the test or update the asserts. Update the it(...) description
to reflect the asserted values (expect res.totalIdleUsers === 1,
res.totalUserRoleToBeAdded === 1, res.totalUserRoleToBeRemoved === 1,
res.totalArchivedUsers === 0) so the test name matches the assertions,
referencing the test block that calls updateIdleUsersOnDiscord and inspects
res.totalIdleUsers, res.totalUserRoleToBeAdded, res.totalUserRoleToBeRemoved,
and res.totalArchivedUsers.

[vinit717]

test will be handle here
#2569

[prakashchoudhary07]

Handle this also, please, in the test PR.
Please make sure that we do not change the tests, until there a actual functionality change