fix: test expectations for idle user

[vinit717]

Date: 18-02-2025

Developer Name: @vinit717

---

Issue Ticket Number

• closes Refactor test for group idle #2569

Description

Fix the test expectations for the idle user and define mavens to an empty array

Documentation Updated?

• Yes
• No

Under Feature Flag

• Yes
• No

Database Changes

• Yes
• No

Breaking Changes

• Yes
• No

Development Tested?

• Yes
• No

Test Coverage

Screenshot 1

Additional Notes

[coderabbitai]

Summary by CodeRabbit

Release Notes

• Refactor
  • Internal code reorganization for improved variable scoping and consistency in Discord action handling.

---

No user-facing features or bug fixes in this release.

Walkthrough

This PR introduces a locally scoped allMavens array within the updateIdleUsersOnDiscord function, replacing the previous global reference. Test fixtures and expectations are updated to reflect new idle/role-addition counts.

Changes

Cohort / File(s)	Summary
Model Implementation models/discordactions.js	Introduces a locally scoped allMavens array inside updateIdleUsersOnDiscord, shadowing the previous global array to localize Maven ID accumulation.
Test Updates test/unit/models/discordactions.test.js	Discord member role fixture updated (index 3); test expectations adjusted for updateIdle7dUsersOnDiscord and updateGroupIdle to expect totalIdleUsers and totalUserRoleToBeAdded of 2 instead of 1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Refactor: improve idle user checks and error handling in Discord actions #2568: Modifies allMavens usage and Discord member ID tracking in models/discordactions.js.
• feat: enhance user idle status checks in Discord actions #2566: Changes user-accumulation logic and conditional gating for idle list population in models/discordactions.js.

Suggested reviewers

• AnujChhikara
• prakashchoudhary07
• iamitprakash

Poem

🐰 A variable finds its local home,
No longer in global's foam,
Shadows dance where allMavens play,
Tests align in their new way—
Scope refined, the tests agree! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: test expectations for idle user' is partially related to the changeset. It mentions test expectations but doesn't capture the main code change of introducing a locally scoped allMavens array, which is a significant part of the modification.
Description check	✅ Passed	The PR description is related to the changeset. It mentions fixing test expectations for idle users and defining mavens as an empty array, which aligns with the actual changes made.
Linked Issues check	✅ Passed	The PR addresses the linked issue #2569 by refactoring the test for the group idle model function. The code changes (updating test expectations and initializing allMavens locally) align with the refactoring objective mentioned in the issue.
Out of Scope Changes check	✅ Passed	All changes are in scope. The modifications to models/discordactions.js (introducing locally scoped allMavens) and test updates in test/unit/models/discordactions.test.js directly support the refactoring objective for the group idle test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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 2569-refactor-test-for-group-idle

---

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]

Caution

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

⚠️ Outside diff range comments (2)

models/discordactions.js (1)

40-40: ⚠️ Potential issue | 🟠 Major

updateIdle7dUsersOnDiscord still accumulates state into the module-level allMavens — same bug that was fixed for updateIdleUsersOnDiscord

The local const allMavens = []; at line 407 correctly isolates updateIdleUsersOnDiscord. However, updateIdle7dUsersOnDiscord still pushes to and reads from the module-level array (lines 674 and 697). Each invocation permanently appends Maven Discord IDs to this global, causing users who were once Mavens to be incorrectly excluded from the idle list in every subsequent call — even after they lose the Maven role. The array also grows without bound across cron/scheduler runs.

The fix should mirror line 407: add const allMavens = []; at the top of updateIdle7dUsersOnDiscord and remove the now-dead module-level declaration at line 40.

🔧 Proposed fix

-const allMavens = [];
-
 /**
  *
  * `@param` roleData { Object }: Data of the new role

 const updateIdle7dUsersOnDiscord = async (dev) => {
   let totalIdle7dUsers = 0;
   ...
   let groupIdle7dRoleId;
+  const allMavens = [];

   try {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@models/discordactions.js` at line 40, The module-level array allMavens is
being mutated by updateIdle7dUsersOnDiscord which causes state to leak across
invocations; mirror the fix used for updateIdleUsersOnDiscord by removing the
module-level declaration of allMavens and instead declare a local const
allMavens = [] at the start of the updateIdle7dUsersOnDiscord function, then
update the function to push/read from that local array only so the list is
cleared each run and no global growth occurs.

test/unit/models/discordactions.test.js (1)

991-991: ⚠️ Potential issue | 🟡 Minor

Stale test description — still says "as 3" but the assertions check for 2.

The description reads "should return totalIdleUsers as 3,totalRolesApplied as 3, totalRoleToBeAdded as 3 as no one is maven", but the actual expectations assert totalIdle7dUsers === 2 and totalUserRoleToBeAdded === 2. This should be updated to match.

✏️ Proposed fix

-    it("should return  totalIdleUsers as 3,totalRolesApplied as 3, totalRoleToBeAdded as 3 as no one is maven", async function () {
+    it("should return totalIdle7dUsers as 2, totalUserRoleToBeAdded as 2, totalUserRoleToBeRemoved as 1, totalArchivedUsers as 1", async function () {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/unit/models/discordactions.test.js` at line 991, Update the stale test
description in the it block whose title starts with "should return 
totalIdleUsers as 3,totalRolesApplied as 3, totalRoleToBeAdded as 3 as no one is
maven" to reflect the actual expected values (2) used in the assertions; change
the string to mention totalIdleUsers/totalIdle7dUsers and totalUserRoleToBeAdded
(and totalRolesApplied if relevant) as 2 so the description matches the
assertions referencing totalIdle7dUsers and totalUserRoleToBeAdded in the
discordactions.test.js test case.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@models/discordactions.js`:
- Line 40: The module-level array allMavens is being mutated by
updateIdle7dUsersOnDiscord which causes state to leak across invocations; mirror
the fix used for updateIdleUsersOnDiscord by removing the module-level
declaration of allMavens and instead declare a local const allMavens = [] at the
start of the updateIdle7dUsersOnDiscord function, then update the function to
push/read from that local array only so the list is cleared each run and no
global growth occurs.

In `@test/unit/models/discordactions.test.js`:
- Line 991: Update the stale test description in the it block whose title starts
with "should return  totalIdleUsers as 3,totalRolesApplied as 3,
totalRoleToBeAdded as 3 as no one is maven" to reflect the actual expected
values (2) used in the assertions; change the string to mention
totalIdleUsers/totalIdle7dUsers and totalUserRoleToBeAdded (and
totalRolesApplied if relevant) as 2 so the description matches the assertions
referencing totalIdle7dUsers and totalUserRoleToBeAdded in the
discordactions.test.js test case.