Optimize updateAllUserStatus Query by Soumava-221B · Pull Request #2598 · RealDevSquad/website-backend

Soumava-221B · 2026-06-16T15:15:29Z

Date: 16/06/2026

Developer Name: Soumava Das

Issue Ticket Number

Description

Optimize the updateAllUserStatus cron job by delegating scheduled status filtering to Firestore to reduce document reads.

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

Optimizes the database query in updateAllUserStatus by performing range filtering on the database level instead of in-memory application logic. Key Changes: - Added filter on futureStatus.from directly to the query. - Replaced the internal snapshot property _size with public size property. - Instantiated today once at the top of the updateAllUserStatus model.

coderabbitai · 2026-06-16T15:15:53Z

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.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4c691d5c-c7ec-43f6-93ca-3c6b4a20e82e

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

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

updateAllUserStatus in models/userStatus.js gains a Firestore where clause restricting results to documents whose futureStatus.from is <= Date.now(). today is now computed with Date.now(), and summary.usersCount is read from userStatusDocs.size instead of _size.

Changes

Firestore Query Optimization in updateAllUserStatus

Layer / File(s)	Summary
Query filter and usersCount fix `models/userStatus.js`	Adds `futureStatus.from <= today` filter (with `today = Date.now()`) to the Firestore query and replaces `userStatusDocs._size` with `userStatusDocs.size` for `summary.usersCount`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related issues

Refactor: Optimize updateAllUserStatus Cron Job Query to Prevent Full Collection Scan #2597 — This PR directly implements the timestamp-based filter (futureStatus.from <= Date.now()) proposed in that issue to prevent full collection scans in updateAllUserStatus.

Poem

🐇 A query once scanned every doc in the store,
Now timestamp and state keep it lean to the core.
Date.now() decides who deserves to be found,
And _size was swapped for the .size that's sound.
Fewer reads, cleaner hops — what a burrow to bound! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the main change: optimizing the updateAllUserStatus Firestore query.
Description check	✅ Passed	The description matches the changeset by describing Firestore-side status filtering to reduce document reads.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@models/userStatus.js`:
- Around line 301-304: The query in userStatusModel combining an `in` filter on
`futureStatus.state` with a range filter on `futureStatus.from` requires a
composite Firestore index that is not currently defined in the repository.
Create a composite index for the `futureStatus.state` field (in ascending order)
and `futureStatus.from` field (in ascending order) either through the Firebase
Console or by adding it to the firestore.indexes.json configuration file and
deploying it. This index must be deployed to the production Firestore instance
before merging to avoid FAILED_PRECONDITION errors at runtime when the cron
executes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 862df3df-1b4e-4a15-8e7f-af534058bd47

📥 Commits

Reviewing files that changed from the base of the PR and between 0552516 and 6a88a00.

📒 Files selected for processing (1)

models/userStatus.js

MayankBansal12 · 2026-06-23T18:05:12Z

@Soumava-221B please add proof for your changes, screen recording for the api request
and improve PR title and simplify the description (refer to other merged PRs)

Removes obsolete date-comparison filters and dead code branches in models/userStatus.js. Since the query already filters by 'futureStatus.from <= today' (via commit 6a88a00), the corresponding nested checks are always true and have been simplified.

This reverts commit e6151c4.

…s updates Introduce comprehensive tests to verify the automatic updating of user status from Out-of-Office (OOO) to Active based on the scheduled start date. Key Changes: - Unit Tests: Added cases for updateAllUserStatus (past, today boundary, and future scenarios). - Integration Tests: Verified PATCH /users/status/update endpoint under superuser auth and checked DB states. - Clock Mocking: Utilized Sinon fake timers to ensure deterministic date-based testing.

MayankBansal12 · 2026-07-14T19:14:15Z

      expect(response).to.have.status(401);
    });
+
+    it("Should update user status and return 200 for a superuser request", async function () {


where's the test for checking that future status documents aren't being read? And wasn't the update behaviour being tested in existing tests?

Test for checking the future status documents is done by the combination of integration and unit test

Integration Test

In test/integration/userStatus.test.js:
It seeds three user status documents:

userToUpdateId (scheduled for yesterday, so it should update).

userBoundaryUpdateId (scheduled for today, so it should update).

userNotToUpdateId (scheduled for tomorrow, so it should NOT update).
It verifies that the document with the future date is ignored during the update run using these assertions:

API Response Assertions:

expect(response.body.data.usersCount).to.equal(2); // Only matches the 2 eligible documents expect(response.body.data.oooUsersAltered).to.equal(2);

Database State Assertion:

const nonUpdatedDoc = await userNotToUpdateStatusRef.get(); expect(nonUpdatedDoc.data().currentStatus.state).to.equal(userState.OOO); expect(nonUpdatedDoc.data().futureStatus.state).to.equal(userState.ACTIVE); // remains untouched

Unit Test

In test/unit/models/userStatus.js:
The specific test is: Should not update user status when futureStatus.from > today (e.g. from is in the future).
It seeds a single user status document with a future date (today + 24 hours) and verifies:

const summary = await updateAllUserStatus(); expect(summary.usersCount).to.equal(0); // Verifies the query returned 0 documents to be read/updated expect(summary.oooUsersAltered).to.equal(0);

Previously the bulk update behaviour was not tested in the existing integration tests.

MayankBansal12 · 2026-07-14T19:18:07Z

+      const userBoundaryUpdateId = await addUser(userData[3]);
+
+      const userToUpdateStatusRef = firestore.collection("usersStatus").doc();
+      await userToUpdateStatusRef.set({


the mock value for userStatus can be defined in constants once and reused...there might be existing value that can be reused here

Currently there is no exsisting constant in the codebase that can be reused directly for these tests.

please define the constant then since you are using this in multiple places or you can reuse the same func you defined for other tests

MayankBansal12 · 2026-07-14T19:20:42Z

    expect(response.data.futureStatus.state).to.equal("UPCOMING");
  });
+
+  describe("updateAllUserStatus", function () {


you can refactor all tests in this describe block, all of them users same userStatus mock data

also please remove comments if aren't necessary

MayankBansal12 · 2026-07-14T19:25:46Z

@Soumava-221B can you please add proof for your changes? screenshot or any other relevant recording...

Soumava-221B · 2026-07-15T03:55:48Z

@Soumava-221B can you please add proof for your changes? screenshot or any other relevant recording...

Sure

Before the cron job run and calls the PATCH API (Query operation benchmark)

Manually hitting PATCH API endpoint

After the userStatus is updated (Query operation benchmark)

Soumava-221B · 2026-07-15T04:17:52Z

Test output of userStatus.test.js (Integration)

Test output of userStatus.js (Model)

MayankBansal12 · 2026-07-16T01:18:54Z

+
+      const userToUpdateId = await addUser(userData[1]);
+      const userNotToUpdateId = await addUser(userData[2]);
+      const userBoundaryUpdateId = await addUser(userData[3]);


what's the boundaryUpdateId?

The boundaryUpdateId is when the futureStatus.from is set to today

MayankBansal12 · 2026-07-16T01:22:25Z

+      await cleanDb();
+    });
+
+    function buildUserStatusMock(


please use better function naming

place this func ideally in a utils or helper file so it can be reused in multiple places

MayankBansal12 · 2026-07-16T01:25:34Z

+      const userNotToUpdateId = await addUser(userData[2]);
+      const userBoundaryUpdateId = await addUser(userData[3]);
+
+      const userToUpdateStatusRef = firestore.collection("usersStatus").doc();


in lines 170, 193, 216, why are you querying collections three times? what behaviour are you testing here? does this match the pattern for logic used in actual function?

shouldn't the func query one time and then filter accordingly? let me know if i'm missing something

MayankBansal12 · 2026-07-16T01:26:13Z

+      const userBoundaryUpdateId = await addUser(userData[3]);
+
+      const userToUpdateStatusRef = firestore.collection("usersStatus").doc();
+      await userToUpdateStatusRef.set({


please define the constant then since you are using this in multiple places or you can reuse the same func you defined for other tests

MayankBansal12 · 2026-07-16T01:37:05Z

-          toUpdate = !toUpdate;
-          summary.oooUsersAltered++;
-        } else {
-          summary.oooUsersUnaltered++;


previously there was oooUsersUnaltered for keeping track of status that aren't updated, are we still using it anywhere for OOO users who's status aren't changed?

Yes, we are still using it check line 296

coderabbitai Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread models/userStatus.js

Merge branch 'RealDevSquad:develop' into develop

09fe69e

MayankBansal12 reviewed Jun 23, 2026

View reviewed changes

Comment thread models/userStatus.js

Soumava-221B changed the title ~~Optimize updateAllUserStatus Query & Filter Scheduled Statuses on Database Level~~ Optimize updateAllUserStatus Query Jun 26, 2026

Soumava-221B added 5 commits July 2, 2026 21:37

fixed missing test cases

e6151c4

Revert "fixed missing test cases"

973eca1

This reverts commit e6151c4.

fixed the unnecessary test file code changes

4e013fa

MayankBansal12 reviewed Jul 14, 2026

View reviewed changes

updated the test files

961731b

MayankBansal12 reviewed Jul 16, 2026

View reviewed changes

coderabbitai Bot mentioned this pull request Jul 16, 2026

Create Firestore composite index for usersStatus (futureStatus.state + futureStatus.from) on staging #2605

Closed

3 tasks

Uh oh!

Conversation

Soumava-221B commented Jun 16, 2026 • 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 Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MayankBansal12 commented Jun 23, 2026

Uh oh!

MayankBansal12 Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Integration Test

API Response Assertions:

Database State Assertion:

Unit Test

Previously the bulk update behaviour was not tested in the existing integration tests.

Uh oh!

MayankBansal12 Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

MayankBansal12 commented Jul 14, 2026

Uh oh!

Soumava-221B commented Jul 15, 2026

Before the cron job run and calls the PATCH API (Query operation benchmark)

Manually hitting PATCH API endpoint

After the userStatus is updated (Query operation benchmark)

Uh oh!

Soumava-221B commented Jul 15, 2026

Test output of userStatus.test.js (Integration)

Test output of userStatus.js (Model)

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!

Reviewers

Assignees

Labels

Soumava-221B commented Jun 16, 2026 •

edited

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

MayankBansal12 Jul 14, 2026 •

edited

Loading

MayankBansal12 Jul 14, 2026 •

edited

Loading