Fix group member count #2524

Kavyashri217 · 2025-12-23T00:16:55Z

Date: 23-12-2025

Developer Name: Kavyashri K R

Issue Ticket Number

Description

Changes made:

Updated enrichGroupDataWithMembershipInfo to compute memberCount from existing Discord role membership data.
No changes to Discord service integrations.
No frontend or schema changes.

What this changes:

Fixed inaccurate group member counts by updating how memberCount is computed in the backend.
memberCount is now derived from live Discord role membership.
Firestore group–user mappings continue to be used only for membership checks (isMember).

Scope:
Backend

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 · 2025-12-23T00:17:56Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The enrichGroupDataWithMembershipInfo function in models/discordactions.js has been modified to calculate member counts using live Discord member data via liveRoleIdToCountMap rather than Firestore-based group-to-user mappings. The rest of the group enrichment logic remains unchanged.

Changes

Cohort / File(s)	Summary
Member Count Data Source `models/discordactions.js`	Modified `enrichGroupDataWithMembershipInfo` to compute `memberCount` from live Discord member data (`liveRoleIdToCountMap` from `getDiscordMembers`) instead of Firestore-based role mappings (`roleIdToCountMap`). Added live mapping logic block and comments; includes commented-out alternative implementation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 From the vaults of Firestore deep we soared,
To Discord's live streams, freshly poured,
Where member counts now dance and flow,
Real-time magic sets the glow! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing group member count computation in the enrichGroupDataWithMembershipInfo function.
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 clearly outlines changes to enrichGroupDataWithMembershipInfo, explains the impact on group member counts, and specifies the scope and related considerations.

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

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

coderabbitai

Actionable comments posted: 5

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12f8cdf and 730d955.

📒 Files selected for processing (1)

models/discordactions.js

🧰 Additional context used

🪛 GitHub Actions: Tests

models/discordactions.js

[error] 1-1: Failed to parse URL from DISCORD_BASE_URL/discord-members during enrichGroupDataWithMembershipInfo.

[warning] 338-338: Generic Object Injection Sink security/detect-object-injection.

🪛 GitHub Check: build (22.10.0)

models/discordactions.js

[warning] 338-338:
Generic Object Injection Sink

coderabbitai · 2025-12-23T00:21:24Z

models/discordactions.js

+    // Discord live role membership (used for memberCount)
+    const discordMembers = await getDiscordMembers();
+    const liveRoleIdToCountMap = {};
+
+    discordMembers.forEach((member) => {
+      if (!member.roles) return;
+
+      member.roles.forEach((roleId) => {
+        liveRoleIdToCountMap[roleId] = (liveRoleIdToCountMap[roleId] || 0) + 1;
+      });
    });


⚠️ Potential issue | 🔴 Critical

Missing error handling for Discord API call causes pipeline failure.

The pipeline is failing with Failed to parse URL from DISCORD_BASE_URL/discord-members because getDiscordMembers() is called without error handling. If the Discord API is unavailable or misconfigured, the entire function fails with no fallback.

Consider wrapping this in a try/catch and falling back to the Firestore-based count (roleIdToCountMap) when Discord is unreachable:

🔎 Proposed fix with graceful degradation

// Discord live role membership (used for memberCount) - const discordMembers = await getDiscordMembers(); const liveRoleIdToCountMap = {}; + let useFirestoreFallback = false; - discordMembers.forEach((member) => { - if (!member.roles) return; + try { + const discordMembers = await getDiscordMembers(); + discordMembers.forEach((member) => { + if (!member.roles) return; - member.roles.forEach((roleId) => { - liveRoleIdToCountMap[roleId] = (liveRoleIdToCountMap[roleId] || 0) + 1; + member.roles.forEach((roleId) => { + liveRoleIdToCountMap[roleId] = (liveRoleIdToCountMap[roleId] || 0) + 1; + }); }); - }); + } catch (error) { + logger.warn("Failed to fetch Discord members, falling back to Firestore counts", error); + useFirestoreFallback = true; + }

Then update line 352:

- memberCount: liveRoleIdToCountMap[group.roleid] || 0, + memberCount: useFirestoreFallback + ? (roleIdToCountMap[group.roleid] || 0) + : (liveRoleIdToCountMap[group.roleid] || 0),

🧰 Tools

🪛 GitHub Actions: Tests

[warning] 338-338: Generic Object Injection Sink security/detect-object-injection.

🪛 GitHub Check: build (22.10.0)

[warning] 338-338:
Generic Object Injection Sink

[warning] 338-338:
Generic Object Injection Sink

🤖 Prompt for AI Agents

In models/discordactions.js around lines 330 to 340, the call to getDiscordMembers() has no error handling so any Discord API failure crashes the flow; wrap the await getDiscordMembers() in a try/catch, on success populate liveRoleIdToCountMap as shown, and on error log the error and set liveRoleIdToCountMap = {} (or leave undefined) so the later code falls back to the Firestore-derived roleIdToCountMap; ensure the catch does not rethrow and that subsequent merges/use of liveRoleIdToCountMap check for its presence before overriding Firestore counts.

coderabbitai · 2025-12-23T00:21:24Z

models/discordactions.js

-      // This says how many users we have for a given roleId
-      roleIdToCountMap[groupToUserMapping.roleid] = (roleIdToCountMap[groupToUserMapping.roleid] ?? 0) + 1;
+    // Discord live role membership (used for memberCount)
+    const discordMembers = await getDiscordMembers();


🧹 Nitpick | 🔵 Trivial

Consider caching Discord member data to reduce API calls.

getDiscordMembers() is now called on every invocation of enrichGroupDataWithMembershipInfo. If this function is called frequently (e.g., on group listing pages), this could:

Increase latency due to network round-trips

Hit Discord API rate limits

Cause failures if Discord is temporarily unavailable

Consider implementing a short-lived cache (e.g., 1-5 minutes) for the Discord members data, or fetching it at a higher level and passing it as a parameter.

coderabbitai · 2025-12-23T00:21:24Z

models/discordactions.js

+      member.roles.forEach((roleId) => {
+        liveRoleIdToCountMap[roleId] = (liveRoleIdToCountMap[roleId] || 0) + 1;
+      });


🧹 Nitpick | 🔵 Trivial

Consider using Map instead of plain object to avoid prototype pollution risk.

Static analysis flagged this as a "Generic Object Injection Sink." While Discord role IDs are safe numeric strings, using a Map would eliminate the warning and provide better type safety:

🔎 Alternative using Map

- const liveRoleIdToCountMap = {}; + const liveRoleIdToCountMap = new Map(); discordMembers.forEach((member) => { if (!member.roles) return; member.roles.forEach((roleId) => { - liveRoleIdToCountMap[roleId] = (liveRoleIdToCountMap[roleId] || 0) + 1; + liveRoleIdToCountMap.set(roleId, (liveRoleIdToCountMap.get(roleId) || 0) + 1); }); });

Then update line 352:

- memberCount: liveRoleIdToCountMap[group.roleid] || 0, + memberCount: liveRoleIdToCountMap.get(group.roleid) || 0,

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 GitHub Actions: Tests

[warning] 338-338: Generic Object Injection Sink security/detect-object-injection.

🪛 GitHub Check: build (22.10.0)

[warning] 338-338:
Generic Object Injection Sink

[warning] 338-338:
Generic Object Injection Sink

🤖 Prompt for AI Agents

In models/discordactions.js around lines 337-339, replace the plain object usage for liveRoleIdToCountMap with a Map to avoid prototype pollution warnings: initialize liveRoleIdToCountMap as new Map(), then inside the loop read the current count with liveRoleIdToCountMap.get(roleId) || 0 and update it with liveRoleIdToCountMap.set(roleId, count + 1). Also update any later code that expects a plain object to either accept a Map or convert the Map to an object via Object.fromEntries(liveRoleIdToCountMap) where needed.

models/discordactions.js

…ount

…MembershipInfo to Tells NYC: don’t enforce coverage here

package.json

MayankBansal12 · 2025-12-24T14:40:42Z

services/discordMembersService.js

+
+const rawBaseUrl = config.get("services.discordBot.baseUrl");
+const DISCORD_BASE_URL = rawBaseUrl && rawBaseUrl !== "DISCORD_BASE_URL" ? rawBaseUrl : "http://localhost:8080";


why is this change required? isn't the config file already handling fallback value?

undone the changes as the config file was already handling the fallback value

MayankBansal12 · 2025-12-24T14:41:36Z

test/unit/models/discordactions.test.js

the use of adding fetchStub here is unclear, test changes in this file don't seem to be using it, can you point me where is it being used?

The fetch stub isn’t used directly in this test. The request hits /discord-actions/groups, which internally calls getDiscordMembers().

The stub ensures that call doesn’t make a real Discord API request and keeps the integration test deterministic.

MayankBansal12 · 2025-12-24T14:43:54Z

models/discordactions.js

 * @returns {Promise<Array<object>>} - An array of group objects with enriched information.
 */
+
+/* istanbul ignore next */


please remove unnecessary comments from this file

/* istanbul ignore next */ was added to exclude a defensive error path that depends on external Discord failures and isn’t reliably testable. Without this, coverage reports false negatives even though the core logic is already covered.

@Kavyashri217 please remove this

Removed the comment

test/unit/services/discordService.test.js

MayankBansal12 · 2025-12-24T14:48:18Z

@Kavyashri217

Please add screen recording for your changes not the code screenshot
Please attach test coverage as well
One of the Tests is failing, please fix that

.github/workflows/codeql.yml

Kavyashri217 added 2 commits December 23, 2025 02:58

Initialize branch

4c81b38

Fix the memberCount error by making changes discordactions.js file

730d955

coderabbitai bot reviewed Dec 23, 2025

View reviewed changes

Kavyashri217 added 9 commits December 23, 2025 06:27

Use Map for liveRoleIdToCountMap to avoid object injection warning

36bfdcd

Merge branch 'RealDevSquad:develop' into kavyashri/fix-group-member-c…

d8a01e3

…ount

Change a comment, remove commented original code

c049c33

Add /* istanbul ignore next */ above the function enrichGroupDataWith…

17185a2

…MembershipInfo to Tells NYC: don’t enforce coverage here

Guard DISCORD_BASE_URL to avoid invalid URL in tests

8d6a58b

Fix group member count using live Discord roles

1d38a1c

Make enrichGroupDataWithMembershipInfo use the stubbed function

30f7254

Stub global fetch to avoid discord base url dependency

a87ac33

Commit changes

4282ffa

MayankBansal12 reviewed Dec 24, 2025

View reviewed changes

Kavyashri217 added 3 commits December 24, 2025 21:10

Make changes as per developer review 1

ceb35a6

Change as per the PR review 1

a49c0aa

Upgrade CodeQL GitHub Action to v3

8156560

iamitprakash reviewed Dec 25, 2025

View reviewed changes

.github/workflows/codeql.yml Outdated Show resolved Hide resolved

Kavyashri217 added 3 commits January 1, 2026 21:28

Resolved the comments

ea1da1a

Undo changes made in codeql.yml

bee6f8b

Delete .github/workflows/codeql.yml

2e55732

Kavyashri217 closed this Jan 1, 2026

Kavyashri217 deleted the kavyashri/fix-group-member-count branch January 1, 2026 16:56


		const rawBaseUrl = config.get("services.discordBot.baseUrl");
		const DISCORD_BASE_URL = rawBaseUrl && rawBaseUrl !== "DISCORD_BASE_URL" ? rawBaseUrl : "http://localhost:8080";

Fix group member count #2524

Fix group member count #2524

Conversation

Kavyashri217 commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Ticket Number

Description

Documentation Updated?

Under Feature Flag

Database Changes

Breaking Changes

Development Tested?

Screenshots

Test Coverage

Additional Notes

Uh oh!

coderabbitai bot commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MayankBansal12 commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kavyashri217 commented Dec 23, 2025 •

edited

Loading

coderabbitai bot commented Dec 23, 2025 •

edited

Loading

MayankBansal12 commented Dec 24, 2025 •

edited

Loading