fix: fetch user presence when app come to foreground#7120
fix: fetch user presence when app come to foreground#7120
Conversation
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
WalkthroughThe pull request adds automatic refresh of direct-message user presence at key lifecycle points: after login, when the app returns to foreground, and when the Meteor connection succeeds. For server versions >= 3.0.0, presence requests now include a timestamp parameter. Connection readiness checks are implemented before presence operations. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Lifecycle
participant Login as Login Saga
participant DM as DM Presence<br/>Refresh
participant DB as WatermelonDB<br/>Subscriptions
participant API as Presence API
App->>Login: User logs in
Login->>DM: call(refreshDmUsersPresence)
DM->>DB: getDirectMessageUserIds()
DB-->>DM: [dmUserIds...]
alt Non-empty DM list
DM->>API: getUsersPresence(dmUserIds)
API-->>DM: presence data
else Empty list
DM-->>DM: abort (no DMs)
end
DM-->>Login: success/error
App->>App: Foreground event
App->>App: waitForConnection()
alt Connection ready
App->>API: setUserPresenceOnline()
App->>DM: call(refreshDmUsersPresence)
DM->>DB: getDirectMessageUserIds()
DB-->>DM: [dmUserIds...]
DM->>API: getUsersPresence(dmUserIds)
else Not ready
App-->>App: abort foreground tasks
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Rohit3523
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/containers/Status/index.tsx`:
- Around line 23-28: The effect currently calls getUserPresence(id) on every
mount when connected which causes duplicate enqueues during virtualized
remounts; restore a guard that only fetches on actual reconnects (e.g., re-add
the statusState === 'loading' check used before) or add deduping inside
getUserPresence/batching logic so repeated id enqueues are ignored; update the
useEffect (referencing useEffect, connected, id, status/statusState) to only
call getUserPresence when connection state transitions to connected or
statusState indicates an initial load, or modify getUserPresence to check an
in-flight set and skip enqueueing duplicate ids.
In `@app/lib/methods/getUsersPresence.ts`:
- Around line 159-167: The helper refreshDmUsersPresence currently swallows all
errors, preventing callers in app/sagas/login.js and app/sagas/state.js from
seeing failures; change it to either rethrow the caught error or log it before
returning — specifically update refreshDmUsersPresence (which calls
getDirectMessageUserIds() and getUsersPresence(dmUserIds)) so that in the catch
block you either throw the error (allowing the saga callers to handle/log it) or
call the shared logger (e.g., processLogger or the project's logging utility)
with the error and contextual message, rather than silently ignoring it.
- Around line 145-153: getDirectMessageUserIds currently picks sub.uids?.[0]
which may be the logged-in user; change getDirectMessageUserIds so for each
subscription (subscriptions / sub.uids) you find the peer id by excluding the
current user's id (e.g. obtain currentUserId via Meteor.userId() or the existing
auth helper) and selecting the remaining uid (for DMs, the other participant),
then collect and dedupe those peer ids instead of using uids[0]; ensure to
handle cases where uids may have multiple entries or missing values by filtering
falsy values before deduping.
🪄 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: CHILL
Plan: Pro
Run ID: d76acab2-b30d-45c0-8569-84d7dad3fa1a
📒 Files selected for processing (4)
app/containers/Status/index.tsxapp/lib/methods/getUsersPresence.tsapp/sagas/login.jsapp/sagas/state.js
📜 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). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line
Files:
app/containers/Status/index.tsxapp/sagas/login.jsapp/sagas/state.jsapp/lib/methods/getUsersPresence.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase configuration including React, React Native, TypeScript, and Jest plugins
Files:
app/containers/Status/index.tsxapp/sagas/login.jsapp/sagas/state.jsapp/lib/methods/getUsersPresence.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/containers/Status/index.tsxapp/lib/methods/getUsersPresence.ts
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in app/containers/ directory
Files:
app/containers/Status/index.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/Status/index.tsxapp/sagas/login.jsapp/sagas/state.jsapp/lib/methods/getUsersPresence.ts
🧠 Learnings (5)
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/AppContainer.tsx : Implement root navigation container logic in app/AppContainer.tsx to switch between authentication states
Applied to files:
app/containers/Status/index.tsx
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/containers/Status/index.tsxapp/lib/methods/getUsersPresence.ts
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/sagas/**/*.{ts,tsx} : Place Redux sagas in app/sagas/ directory with separate files for init, login, rooms, messages, encryption, deepLinking, and videoConf side effects
Applied to files:
app/sagas/login.jsapp/sagas/state.js
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/store/**/*.{ts,tsx} : Configure Redux store in app/lib/store/ with middleware for saga, app state, and internet state
Applied to files:
app/sagas/state.js
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to @(app/sagas/videoConf.ts|app/lib/methods/videoConf.ts) : Manage video conferencing via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using server-managed Jitsi integration; do not conflate with VoIP
Applied to files:
app/sagas/state.js
🔇 Additional comments (1)
app/sagas/state.js (1)
75-84:⚠️ Potential issue | 🟠 MajorGuard the reconnect resubscribe path against duplicate subscriptions.
handleMeteorConnect()runs on everyMETEOR.SUCCESS(app/sagas/state.js:92) and unconditionally callssubscribeUsersPresence(), which issues multiplesdk.subscribe()calls without any guard or unsubscribe logic. Reconnect flaps will accumulate duplicate presence subscriptions. Add a flag to track subscription state or explicitly unsubscribe before resubscribing to prevent duplicate stream events.⛔ Skipped due to learnings
Learnt from: CR Repo: RocketChat/Rocket.Chat.ReactNative PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-07T17:49:17.538Z Learning: Applies to app/lib/services/sdk.ts : Use Rocket.Chat JS SDK in app/lib/services/sdk.ts for WebSocket real-time subscriptions
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/lib/methods/getUsersPresence.ts (1)
158-160:⚠️ Potential issue | 🟠 MajorDon’t silently swallow DB/query failures in DM ID lookup.
Line 159 returns
[]on errors, which hides failures and bypasses the logging path inrefreshDmUsersPresence(Line 170). Let the error bubble or log it here with context.Suggested fix (bubble to caller logger)
} catch (e) { - return []; + throw e; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/getUsersPresence.ts` around lines 158 - 160, In getUsersPresence.ts the catch block that returns [] silently swallows DB/query failures; change the catch in the async function (the block around "} catch (e) { return []; }") to either log the error with context (including the query/DM id) and then rethrow, or simply rethrow the original error so refreshDmUsersPresence can handle/log it; do not return an empty array on exceptions—use throw e or throw new Error(`getUsersPresence failed for ${dmId}: ${e.message}`) so refreshDmUsersPresence's logger sees the failure.
🧹 Nitpick comments (1)
app/lib/methods/getUsersPresence.ts (1)
149-156: Guard early whenloggedUserIdis unavailable.If
loggedUserIdis missing on Line 149, Line 155 can pick an arbitrary UID and request the wrong presence target. Consider short-circuiting before mapping subscriptions.Suggested tweak
export const getDirectMessageUserIds = async (): Promise<string[]> => { try { const db = database.active; const loggedUserId = reduxStore.getState().login.user?.id; + if (!loggedUserId) { + return []; + } const subscriptionsCollection = db.get('subscriptions'); const subscriptions = await subscriptionsCollection .query(Q.where('t', 'd'), Q.where('open', true), Q.where('archived', false)) .fetch();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/getUsersPresence.ts` around lines 149 - 156, Guard against a missing loggedUserId by short-circuiting before processing subscriptions: check reduxStore.getState().login.user?.id (loggedUserId) and if undefined/null return an empty userIds result or skip the mapping logic that builds userIds from subscriptionsCollection/subscriptions; this prevents the map in subscriptions.map(...).find(...) from selecting arbitrary UIDs when loggedUserId is unavailable and ensures functions that consume userIds receive a safe empty array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/lib/methods/getUsersPresence.ts`:
- Around line 158-160: In getUsersPresence.ts the catch block that returns []
silently swallows DB/query failures; change the catch in the async function (the
block around "} catch (e) { return []; }") to either log the error with context
(including the query/DM id) and then rethrow, or simply rethrow the original
error so refreshDmUsersPresence can handle/log it; do not return an empty array
on exceptions—use throw e or throw new Error(`getUsersPresence failed for
${dmId}: ${e.message}`) so refreshDmUsersPresence's logger sees the failure.
---
Nitpick comments:
In `@app/lib/methods/getUsersPresence.ts`:
- Around line 149-156: Guard against a missing loggedUserId by short-circuiting
before processing subscriptions: check reduxStore.getState().login.user?.id
(loggedUserId) and if undefined/null return an empty userIds result or skip the
mapping logic that builds userIds from subscriptionsCollection/subscriptions;
this prevents the map in subscriptions.map(...).find(...) from selecting
arbitrary UIDs when loggedUserId is unavailable and ensures functions that
consume userIds receive a safe empty array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09aa515c-81ae-4e56-a0ec-afde1f01e8b1
📒 Files selected for processing (1)
app/lib/methods/getUsersPresence.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line
Files:
app/lib/methods/getUsersPresence.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase configuration including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/getUsersPresence.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/methods/getUsersPresence.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/getUsersPresence.ts
🧠 Learnings (1)
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/lib/methods/getUsersPresence.ts
🔇 Additional comments (2)
app/lib/methods/getUsersPresence.ts (2)
60-61: Good cache-busting addition for on-demand presence refresh.Adding
_t: Date.now()on Line 61 is a solid way to avoid staleusers.presenceresponses for supported server versions.
163-171: Nice improvement adding error logging in foreground/login refresh flow.The
log(e)on Line 170 makes refresh failures diagnosable instead of fully silent.
Rohit3523
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
Rohit3523
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
Rohit3523
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
official_android_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
upload_experimental_android
GitHub Actions
Waiting
|
Android Build Available Rocket.Chat Experimental 4.72.0.108522 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNSoNf01ZWHmg0FgRbh7Abupghc0Noz_-XxEDgz8CA9PZQktD7d17JnOA6UBNdXu-RxJyUHI_3Rz2dM7RLM0 |
1 participant
Proposed changes
Issue(s)
https://rocketchat.atlassian.net/browse/PRES-13
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Improvements