fix: Calendar status event status switch#39491
Merged
dionisio-bot[bot] merged 7 commits intodevelopfrom Mar 10, 2026
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 0040db3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughPatch fix for calendar status synchronization: replaces incorrect uses of Changes
Sequence Diagram(s)sequenceDiagram
participant CalendarSync as Calendar Sync
participant CalendarSvc as Calendar Service
participant ApplyChange as applyStatusChange
participant UserDB as User Record (DB)
participant Presence as Presence/Broadcast
CalendarSync->>CalendarSvc: Event start/end received
CalendarSvc->>ApplyChange: processEventStart/processEventEnd with event payload
ApplyChange->>UserDB: fetch user (statusDefault)
alt user missing or offline
ApplyChange-->>CalendarSvc: exit (no-op)
else user exists
ApplyChange->>UserDB: determine previousStatus (statusDefault)
ApplyChange->>UserDB: update status & statusDefault if needed
ApplyChange->>Presence: broadcast change (includes previousStatus, newStatus)
ApplyChange-->>CalendarSvc: confirm applied
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #39491 +/- ##
========================================
Coverage 70.91% 70.91%
========================================
Files 3196 3197 +1
Lines 113299 113337 +38
Branches 20568 20524 -44
========================================
+ Hits 80341 80369 +28
- Misses 30906 30918 +12
+ Partials 2052 2050 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/tests/end-to-end/api/calendar.ts">
<violation number="1" location="apps/meteor/tests/end-to-end/api/calendar.ts:692">
P2: Replace fixed `sleep(...)` waits with polling/retry-based assertions for status transitions to avoid timing-related test flakiness.
(Based on your team's feedback about concurrency and race-condition risks in async test sequencing.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts (1)
50-56: Consider simplifying duplicated branches.Both the
ifandelse ifbranches execute identical code. This can be combined into a single condition for clarity.♻️ Suggested simplification
- let statusChanged = false; - - if (newStatus === UserStatus.BUSY) { - await Users.updateStatusAndStatusDefault(uid, newStatus, newStatus); - statusChanged = true; - } else if (user.statusDefault === UserStatus.BUSY) { - await Users.updateStatusAndStatusDefault(uid, newStatus, newStatus); - statusChanged = true; - } + const shouldUpdateStatus = newStatus === UserStatus.BUSY || user.statusDefault === UserStatus.BUSY; + + if (shouldUpdateStatus) { + await Users.updateStatusAndStatusDefault(uid, newStatus, newStatus); + } - if (statusChanged) { + if (shouldUpdateStatus) { await api.broadcast('presence.status', {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts` around lines 50 - 56, The two identical branches inside the applyStatusChange function (the if and else if blocks) should be merged into a single conditional to remove duplication: replace the separate if and else if checks with one combined condition using a logical OR (e.g., conditionA || conditionB), keep the existing shared body (side-effects, logging, and return behavior) exactly as-is, and leave any remaining distinct else/else-if branches untouched so behavior is unchanged.apps/meteor/tests/end-to-end/api/calendar.ts (3)
689-702: Use consistent API helper pattern.The test uses raw API paths like
'/api/v1/calendar-events.create'while the rest of this file uses theapi()helper (e.g.,api('calendar-events.create')). Consider using the helper for consistency.♻️ Suggested fix for consistency
- const createResponse = await request.post('/api/v1/calendar-events.create').set(userCredentials).send(eventPayload).expect(200); + const createResponse = await request.post(api('calendar-events.create')).set(userCredentials).send(eventPayload).expect(200); const eventId = createResponse.body.id; await sleep(3000); - const statusResponseDuring = await request.get('/api/v1/users.getStatus').set(userCredentials).expect(200); + const statusResponseDuring = await request.get(api('users.getStatus')).set(userCredentials).expect(200); expect(statusResponseDuring.body.status).to.equal('busy'); await sleep(5000); - const statusResponseAfter = await request.get('/api/v1/users.getStatus').set(userCredentials).expect(200); + const statusResponseAfter = await request.get(api('users.getStatus')).set(userCredentials).expect(200); expect(statusResponseAfter.body.status).to.equal('away'); - await request.post('/api/v1/calendar-events.delete').set(userCredentials).send({ eventId }).expect(200); + await request.post(api('calendar-events.delete')).set(userCredentials).send({ eventId }).expect(200);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/end-to-end/api/calendar.ts` around lines 689 - 702, Replace the raw API path usage with the project's api() test helper for consistency: change calls like request.post('/api/v1/calendar-events.create') and request.get('/api/v1/users.getStatus') to use request.post(api('calendar-events.create')) and request.get(api('users.getStatus')) (and similarly for calendar-events.delete), keeping the existing .set(userCredentials).send(...)/.expect(...) chain intact so only the path construction changes.
669-703: Improve test cleanup robustness and consider enabling the required setting.Two concerns with this test:
Cleanup reliability: The event deletion at line 702 is inside the test body. If any assertion fails before this line, the event won't be cleaned up. Consider moving cleanup to an
afterhook or tracking the event for cleanup similar to other test suites in this file.Missing setting enablement: The test assumes
Calendar_BusyStatus_Enabledis already enabled. The service code at line 216 checks this setting and returns early if disabled. Consider explicitly enabling it in thebeforehook for test reliability.♻️ Suggested refactor for robust cleanup and explicit setup
(IS_EE ? describe : describe.skip)('[Calendar Events Status Sync]', () => { + let eventIdToCleanup: string | undefined; + before(async () => { + await request.post(api('settings/Calendar_BusyStatus_Enabled')).set(credentials).send({ value: true }).expect(200); await request.post('/api/v1/users.setStatus').set(userCredentials).send({ status: 'away' }).expect(200); }); + after(async () => { + if (eventIdToCleanup) { + await request.post(api('calendar-events.delete')).set(userCredentials).send({ eventId: eventIdToCleanup }); + } + }); + it('should set user status to busy during event and restore manual status after event ends', async () => { // ... test body ... const createResponse = await request.post(api('calendar-events.create')).set(userCredentials).send(eventPayload).expect(200); const eventId = createResponse.body.id; + eventIdToCleanup = eventId; // ... assertions ... - await request.post('/api/v1/calendar-events.delete').set(userCredentials).send({ eventId }).expect(200); + await request.post(api('calendar-events.delete')).set(userCredentials).send({ eventId }).expect(200); + eventIdToCleanup = undefined; }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/end-to-end/api/calendar.ts` around lines 669 - 703, The test in the '[Calendar Events Status Sync]' block creates an event with calendar-events.create and deletes it inside the test body (eventId) and assumes Calendar_BusyStatus_Enabled is enabled; to fix, move the event cleanup into an after hook that checks for and deletes the created event (use the same eventId variable or a tracked list) so deletion always runs even if assertions fail, and in the before hook explicitly enable the Calendar_BusyStatus_Enabled setting via the appropriate API or test helper before creating the event and setting user status (users.setStatus), ensuring the service code that checks the setting will run as expected.
670-672: Also use consistent API helper for status setup.before(async () => { - await request.post('/api/v1/users.setStatus').set(userCredentials).send({ status: 'away' }).expect(200); + await request.post(api('users.setStatus')).set(userCredentials).send({ status: 'away' }).expect(200); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/end-to-end/api/calendar.ts` around lines 670 - 672, The test currently calls the REST endpoint directly via request.post('/api/v1/users.setStatus') with userCredentials; replace that direct call with the project's shared test helper (e.g., setUserStatus) to keep API usage consistent—call setUserStatus(userCredentials, 'away') (or the equivalent helper used elsewhere in the file) and assert the expected response instead of the inline request.post invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts`:
- Line 23: The log prints user?.status but the user query only projects { roles,
username, name, statusDefault }, so user.status will always be undefined; in
applyStatusChange, either add status: 1 to the Mongo projection when fetching
the user (so the debug log can show the current status), or remove user?.status
from the logged object (and keep logging only roles/username/name/statusDefault)
to avoid misleading undefined values—update the code around the user fetch and
the log call that references user?.status accordingly.
- Around line 59-68: The presence.status broadcast is missing the required
statusText field: update the user projection in applyStatusChange (the
projection defined around the user projection at ~line 23) to include statusText
and add statusText to the broadcast payload emitted in applyStatusChange (the
payload built in the broadcast block around lines 59-68) so the user object
matches the presence.status shape in Events.ts; mirror how
setStatusText.ts/users.ts/federation-matrix/edu.ts include statusText when
constructing the user object for presence.status.
---
Nitpick comments:
In `@apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts`:
- Around line 50-56: The two identical branches inside the applyStatusChange
function (the if and else if blocks) should be merged into a single conditional
to remove duplication: replace the separate if and else if checks with one
combined condition using a logical OR (e.g., conditionA || conditionB), keep the
existing shared body (side-effects, logging, and return behavior) exactly as-is,
and leave any remaining distinct else/else-if branches untouched so behavior is
unchanged.
In `@apps/meteor/tests/end-to-end/api/calendar.ts`:
- Around line 689-702: Replace the raw API path usage with the project's api()
test helper for consistency: change calls like
request.post('/api/v1/calendar-events.create') and
request.get('/api/v1/users.getStatus') to use
request.post(api('calendar-events.create')) and
request.get(api('users.getStatus')) (and similarly for calendar-events.delete),
keeping the existing .set(userCredentials).send(...)/.expect(...) chain intact
so only the path construction changes.
- Around line 669-703: The test in the '[Calendar Events Status Sync]' block
creates an event with calendar-events.create and deletes it inside the test body
(eventId) and assumes Calendar_BusyStatus_Enabled is enabled; to fix, move the
event cleanup into an after hook that checks for and deletes the created event
(use the same eventId variable or a tracked list) so deletion always runs even
if assertions fail, and in the before hook explicitly enable the
Calendar_BusyStatus_Enabled setting via the appropriate API or test helper
before creating the event and setting user status (users.setStatus), ensuring
the service code that checks the setting will run as expected.
- Around line 670-672: The test currently calls the REST endpoint directly via
request.post('/api/v1/users.setStatus') with userCredentials; replace that
direct call with the project's shared test helper (e.g., setUserStatus) to keep
API usage consistent—call setUserStatus(userCredentials, 'away') (or the
equivalent helper used elsewhere in the file) and assert the expected response
instead of the inline request.post invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3b74597-5b89-4143-a581-bad445a7b6dd
📒 Files selected for processing (4)
.changeset/red-windows-breathe.mdapps/meteor/server/services/calendar/service.tsapps/meteor/server/services/calendar/statusEvents/applyStatusChange.tsapps/meteor/tests/end-to-end/api/calendar.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/services/calendar/service.tsapps/meteor/tests/end-to-end/api/calendar.tsapps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
🧠 Learnings (18)
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/services/calendar/service.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/services/calendar/service.tsapps/meteor/tests/end-to-end/api/calendar.tsapps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/services/calendar/service.tsapps/meteor/tests/end-to-end/api/calendar.tsapps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/red-windows-breathe.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2026-02-04T12:09:05.769Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38374
File: apps/meteor/tests/end-to-end/apps/app-logs-nested-requests.ts:26-37
Timestamp: 2026-02-04T12:09:05.769Z
Learning: In E2E tests at apps/meteor/tests/end-to-end/apps/, prefer sleeping for a fixed duration (e.g., 1 second) over implementing polling/retry logic when waiting for asynchronous operations to complete. Tests should fail deterministically if the expected result isn't available after the sleep.
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2026-03-02T16:31:41.304Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/end-to-end/api/calendar.ts
🔇 Additional comments (3)
.changeset/red-windows-breathe.md (1)
1-5: Changeset looks accurate.The patch bump and release note match the fix described in the PR and clearly communicate the user-facing impact.
apps/meteor/server/services/calendar/service.ts (2)
316-340: LGTM on the status restoration logic.The
processEventEndlogic correctly:
- Only restores status when
statusDefaultisBUSY(meaning the system set it)- Requires
previousStatusto exist- Ensures
previousStatusdiffers from currentstatusDefaultThis prevents accidental status changes when users manually changed their status during an event.
288-291: The implementation correctly handles undefinedstatusDefault—the guard should not be changed.The design is intentional:
statusDefaultbeingundefinedmeans the user has never manually set a default status, so the system is permitted to change their status for calendar events. Only whenstatusDefaultis explicitly set toOFFLINE(user's manual choice) should calendar status changes be skipped. The comment inAbstractBusinessHour.tsconfirms this: "statusDefault is the property set when a user manually changes their status. So if it's set to offline, we can be sure the user will be offline after login."The current guard is correct as-is.
KevLehman
commented
Mar 10, 2026
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
Outdated
Show resolved
Hide resolved
pierre-lehnen-rc
approved these changes
Mar 10, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts`:
- Around line 50-55: The code incorrectly treats setting BUSY as a change even
when the user's previousStatus (or statusDefault) is already UserStatus.BUSY;
update applyStatusChange to skip the Users.updateStatusAndStatusDefault call and
not set statusChanged when there is no actual state change: inside the BUSY
branch (where newStatus === UserStatus.BUSY) and the else-if branch checking
user.statusDefault === UserStatus.BUSY, add a guard comparing previousStatus
(and/or user.statusDefault) to newStatus and only call
Users.updateStatusAndStatusDefault(uid, newStatus, newStatus) and set
statusChanged = true when the stored status differs from newStatus so no-op
updates/broadcasts are avoided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5ee6d91-b05c-4b94-af41-e5b6b92400a3
📒 Files selected for processing (1)
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
🧠 Learnings (14)
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-23T17:53:18.785Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-03-10T08:13:44.506Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:44.506Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-03-06T18:10:23.330Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:23.330Z
Learning: In the RocketChat/Rocket.Chat `packages/gazzodown` package and more broadly, the HTML `<code>` element has an implicit ARIA role of `code` per WAI-ARIA 1.3, and `testing-library/dom` / jsdom supports it. Therefore, `screen.getByRole('code')` / `screen.findByRole('code')` correctly locates `<code>` elements without needing an explicit `role="code"` attribute. Do NOT flag `findByRole('code')` as invalid in future reviews.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts
KevLehman
added
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Open
KevLehman
added a commit
that referenced
this pull request
Mar 27, 2026
This reverts commit b29fb86.
KevLehman
added a commit
that referenced
this pull request
Mar 31, 2026
This reverts commit b29fb86.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-1890
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Tests