Skip to content

chore(voip): consolidate PR #6918 review-cleanup#7295

Merged
diegolmello merged 18 commits into
feat.voip-lib-newfrom
chore.review-voip
May 4, 2026
Merged

chore(voip): consolidate PR #6918 review-cleanup#7295
diegolmello merged 18 commits into
feat.voip-lib-newfrom
chore.review-voip

Conversation

@diegolmello

@diegolmello diegolmello commented May 4, 2026

Copy link
Copy Markdown
Member

Proposed changes

Consolidates the review-comment cleanup for PR #6918 into a single mergeable PR. Eight scoped fixes, each landed individually on this branch and reviewed in isolation, addressing three problem classes:

Observability (rollout-blocking)

  • Route 12 VoIP console.error/console.warn sites in useCallStore, MediaSessionInstance, and playCallEndedSound through the project log() helper so failures reach Bugsnag during rollout.
  • Drop the raw FCM/APNs token from the push.ts registration success log line (PII surfaces in screenshots and paste-bins).

Native cleanup

  • Delete dead method-call queue (queueMethodCall, flushQueuedMethodCalls, etc.) from DDPClient.kt and the now-empty flushPendingQueuedSignalsIfNeeded callsite in VoipNotification.kt. Verified zero producers ever called the queue.
  • Extract a single deviceId(context) helper that collapses four Settings.Secure.getString(... ANDROID_ID) duplications.
  • Move 8 hardcoded English literals in VoipNotification.kt and VoipCallService.kt to a new strings_voip.xml (channel descriptions, incoming-call title/text, ongoing-call title/text). Reuse existing incoming_call_accept/incoming_call_reject for action labels. Parameterize Call-from text with %1\$s for translator-friendly reordering. English values only; values-<locale>/ work is a follow-up.

Regression repair

  • Restore redux-logger middleware in the __DEV__ enhancer chain of app/lib/store/index.ts (was unintentionally commented out).
  • Restore height: '100%' on the shared fullContainer ActionSheet style (its removal was collapsing VideoConf, RoomView, and NewMessageView sheets to content height on Android). Introduce a new hugContent opt-in plumbed through Provider/ActionSheet/BottomSheetContent. Only useNewMediaCall opts in (hugContent: isAndroid), so its Android sheet keeps hugging content while every other consumer returns to full-screen behavior. iOS unchanged across all consumers.
  • Remove the dangling .cursor/skills/agent-skills gitlink (no .gitmodules entry, breaks fresh clones). .cursor/ was already in .gitignore.

Hygiene

  • Strip stale // TESTING: labels and an empty no-op .on('stateChange', ...) listener in MediaSessionInstance.ts.
  • Annotate resetMediaCallEventsStateForTesting as @internal.

Issue(s)

Cleanup for #6918.

How to test or reproduce

  • iOS: existing tests cover unchanged behavior (no native iOS changes in this PR).
  • Android: smoke-test that
    • Incoming-call notification still displays correctly with English text (Incoming call, Call from <caller>, Decline, Accept).
    • Ongoing-call foreground notification still displays (VoIP Call, Call in progress).
    • The NewMediaCall action sheet still hugs content on Android.
    • VideoConf actions, RoomView action sheets, and NewMessageView/Item sheets render full-screen again on Android (the regression introduced by 3fefee400 is fixed).
  • JS unit suite: TZ=UTC yarn test should pass for the modified files. Test additions cover the new log() mocks and the new hugContent forwarding assertion.

Screenshots

n/a — no visual additions.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

  • The PR consolidates 8 independent slices that were each reviewed and merged onto this branch separately. Reviewer can step through individual slice commits via git log --first-parent for a per-concern walk.
  • getDisplayMedia typo in MediaSessionStore.ts, the RNCallKeep.setup move to MainApplication, polish refactors ('use memo', useCaller, useAppSelector, MediaCallHeader selector tightening, CallView hook extraction, NewMessageView/Item subcomponent split), and @VisibleForTesting annotation pass on DDPClient.kt are intentionally deferred.
  • Native values-<locale>/ directories for the new VoIP strings are a separate translation pass.

Summary by CodeRabbit

  • New Features

    • Android VoIP notification UI is now localized.
    • Action sheet layout improved for Android "hug" behavior.
  • Bug Fixes

    • Push registration no longer logs the raw device token.
  • Chores

    • Centralized device identifier handling for VoIP flows.
    • Unified error reporting across VoIP services (replaced console logs with shared logger).
    • Enabled improved runtime logging in development builds.
    • Simplified internal call-queue handling.

diegolmello and others added 17 commits May 4, 2026 11:34
Replaces 12 console.error/warn sites in useCallStore, MediaSessionInstance,
and playCallEndedSound with the project log helper so VoIP failures reach
Bugsnag during rollout. Tests extended to mock log and assert Error-typed
calls on each path.
Removes the FCM/APNs token argument from the success-path console.log in
push.ts. Tokens are device PII and surface in screenshots and paste-bins;
the keep-tokenless success message preserves the diagnostic without leaking.
Removes two // TESTING: leftovers in MediaSessionInstance.ts and the empty
.on('stateChange', ...) registration that had no effect. Annotates
resetMediaCallEventsStateForTesting as @internal so callers know it is a
test seam.
Reverts the unintended commenting-out from PR #6918 so dispatched actions
log in dev builds again, matching develop.
Removes the leftover Cursor IDE submodule pointer that has no .gitmodules
entry and breaks fresh clones. .cursor/ is already in .gitignore.
Reverts the unintended removal of height: '100%' from the shared
fullContainer style (which was collapsing VideoConf, RoomView, and
NewMessageView sheets to content height on Android) and introduces
a new hugContent flag plumbed through Provider/ActionSheet/
BottomSheetContent. Only useNewMediaCall opts in (with isAndroid),
so its Android sheet keeps hugging content while every other consumer
returns to full-screen behavior. iOS unchanged.
…ing.*

Moves 8 hardcoded English literals in VoipNotification and VoipCallService
to a new strings_voip.xml so translators can supply locale-specific values.
Reuses existing incoming_call_accept and incoming_call_reject for action
labels. Call-from text uses %1\$s for translator-friendly reordering.
English values only; values-<locale>/ work is a follow-up.
@coderabbitai

coderabbitai Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8ae26d7-e892-4264-a030-eced6da96246

📥 Commits

Reviewing files that changed from the base of the PR and between 6780afc and f977c92.

📒 Files selected for processing (1)
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
📜 Recent 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). (2)
  • GitHub Check: ESLint and Test / run-eslint-and-test
  • GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/NewMediaCall/VoipCallLifecycle.integration.test.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{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

**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms

Files:

  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with @rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Create reusable UI components in app/containers/ directory

Files:

  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.

Applied to files:

  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
🔇 Additional comments (2)
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx (2)

355-357: Good alignment with the new logging path.

Moving this allowlist entry to the error side matches the updated VoIP logging behavior and keeps the noise filter narrow.


594-596: Good assertion for the regression path.

This verifies the missing-call branch on the error channel and checks the exact message emitted for the failed accept flow.


Walkthrough

Removes a DDP queued-method infrastructure, centralizes Android device-id access, internationalizes VoIP notification strings, replaces console.* calls with a shared log helper across VoIP codepaths, adds an ActionSheet hugContent layout option (used for MediaCall UI), enables the Redux logger in dev, and removes a cursor submodule reference.

Changes

VoIP Refactor (Android native + RN logic + tests)

Layer / File(s) Summary
Data / Resources
android/app/src/main/res/values/strings_voip.xml
Adds VoIP notification string resources used by native code.
Android Native — Notification & Device ID
android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt, android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt
Notification titles/text/channel descriptions now use R.string.*. Introduces deviceId(context) helper to centralize Settings.Secure.ANDROID_ID usage across accept/reject and reconciliation paths.
Android Native — DDP client
android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt
Removes internal queued-method-call data class and storage; deletes public queue APIs (queueMethodCall, hasQueuedMethodCalls, flushQueuedMethodCalls, clearQueuedMethodCalls) and stops clearing queued calls on disconnect().
JS — Shared logging adoption
app/lib/services/voip/MediaSessionInstance.ts, app/lib/services/voip/playCallEndedSound.ts, app/lib/services/voip/useCallStore.ts
Replaces console.error/console.warn with the shared log(error) helper in multiple VoIP flows (answering, REST signal replay, room-id resolution, InCallManager and audio-route sync failures, speaker toggle, playback errors).
Tests — logging assertions & coverage
app/lib/services/voip/MediaSessionInstance.test.ts, app/lib/services/voip/playCallEndedSound.test.ts, app/lib/services/voip/useCallStore.test.ts, app/lib/services/voip/useCallStore.ios.test.ts
Mocks the shared log helper and updates/extends tests to assert log is called on various failure paths; adds tests asserting optimistic room-id clearing and other error-reporting behaviors.
Integration test noise handling
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
Moves “[VoIP] Call not found after accept:” message from warn allowlist to error allowlist and updates assertions to expect an error log.

ActionSheet Layout Control for Media Calls

Layer / File(s) Summary
Type / Options
app/containers/ActionSheet/Provider.tsx
Adds optional hugContent?: boolean to TActionSheetOptions.
Component API
app/containers/ActionSheet/ActionSheet.tsx, app/containers/ActionSheet/BottomSheetContent.tsx
ActionSheet passes hugContent into BottomSheetContent. IBottomSheetContentProps adds hugContent?: boolean. Fallback container styling applies fullContainer only when not (hugContent && isAndroid).
Styling
app/containers/ActionSheet/styles.ts
fullContainer now includes height: '100%'.
Hook integration & tests
app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx, app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
useNewMediaCall sends hugContent: isAndroid (replacing prior fullContainer). Tests updated to expect hugContent: true on Android and false on iOS.

Miscellaneous

Layer / File(s) Summary
Submodule removal
.cursor/skills/agent-skills
Removed the subproject commit reference (line removed).
Redux dev tooling
app/lib/store/index.ts
Enables logger middleware in dev by including it in the enhancer compose call alongside saga middleware.
Push registration log
app/lib/notifications/push.ts
Logs a generic “registered successfully” message when a push token is present instead of printing the token.
Docs / Comments
app/lib/services/voip/MediaCallEvents.ts
Marks clearVoipAcceptDedupeSentinels() as @internal in comment (still exported for tests).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: consolidating review-cleanup fixes from PR #6918 into a single branch, which aligns with the multi-faceted improvements across observability, native cleanup, UI behavior, and tests.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (1)
app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx (1)

275-298: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid jest.resetModules() in this hook test.

Resetting the module registry here and then require-ing the hook can load a second React instance, which is a common cause of invalid hook call failures in Jest/RN tests. Use an isolated import block instead.

Suggested safer pattern
-	it('should pass hugContent: true to the action sheet when isAndroid is true', () => {
-		jest.resetModules();
-		jest.doMock('../../methods/helpers/deviceInfo', () => ({
-			isAndroid: true
-		}));
-		// Must load hook after doMock so `hugContent: isAndroid` uses Android.
-		// eslint-disable-next-line `@typescript-eslint/no-var-requires`
-		const { useNewMediaCall: useNewMediaCallAndroid } = require('./useNewMediaCall');
-
-		mockUseSubscription.mockReturnValue(undefined);
-		mockUseMediaCallPermission.mockReturnValue(false);
-
-		const { result } = renderHook(() => useNewMediaCallAndroid('room-id'));
-
-		act(() => {
-			result.current.openNewMediaCall();
-		});
-
-		expect(mockShowActionSheetRef).toHaveBeenCalledTimes(1);
-		const [actionSheetArgs] = mockShowActionSheetRef.mock.calls[0];
-		expect(React.isValidElement(actionSheetArgs.children)).toBe(true);
-		expect(actionSheetArgs.hugContent).toBe(true);
-	});
+	it('should pass hugContent: true to the action sheet when isAndroid is true', () => {
+		jest.isolateModules(() => {
+			jest.doMock('../../methods/helpers/deviceInfo', () => ({
+				isAndroid: true
+			}));
+			const { useNewMediaCall: useNewMediaCallAndroid } = require('./useNewMediaCall');
+
+			mockUseSubscription.mockReturnValue(undefined);
+			mockUseMediaCallPermission.mockReturnValue(false);
+
+			const { result } = renderHook(() => useNewMediaCallAndroid('room-id'));
+
+			act(() => {
+				result.current.openNewMediaCall();
+			});
+		});
+
+		expect(mockShowActionSheetRef).toHaveBeenCalledTimes(1);
+		const [actionSheetArgs] = mockShowActionSheetRef.mock.calls[0];
+		expect(React.isValidElement(actionSheetArgs.children)).toBe(true);
+		expect(actionSheetArgs.hugContent).toBe(true);
+	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx` around lines 275 -
298, The test currently calls jest.resetModules() and then
require('./useNewMediaCall'), which can load a second React instance; replace
that pattern with jest.isolateModules to avoid touching the global module
registry. Remove jest.resetModules(), move the
jest.doMock('../../methods/helpers/deviceInfo', () => ({ isAndroid: true })) and
the require('./useNewMediaCall') call into a jest.isolateModules(() => { ... })
block (i.e. call jest.isolateModules(() => { jest.doMock(...); const {
useNewMediaCall: useNewMediaCallAndroid } = require('./useNewMediaCall'); /*
renderHook and interact here or assign to an outer variable before assertions */
});) and keep mockUseSubscription.mockReturnValue and
mockUseMediaCallPermission.mockReturnValue set as before so the test asserts
mockShowActionSheetRef and actionSheetArgs.hugContent without creating multiple
React instances.
🤖 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 `@app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx`:
- Around line 275-298: The test currently calls jest.resetModules() and then
require('./useNewMediaCall'), which can load a second React instance; replace
that pattern with jest.isolateModules to avoid touching the global module
registry. Remove jest.resetModules(), move the
jest.doMock('../../methods/helpers/deviceInfo', () => ({ isAndroid: true })) and
the require('./useNewMediaCall') call into a jest.isolateModules(() => { ... })
block (i.e. call jest.isolateModules(() => { jest.doMock(...); const {
useNewMediaCall: useNewMediaCallAndroid } = require('./useNewMediaCall'); /*
renderHook and interact here or assign to an outer variable before assertions */
});) and keep mockUseSubscription.mockReturnValue and
mockUseMediaCallPermission.mockReturnValue set as before so the test asserts
mockShowActionSheetRef and actionSheetArgs.hugContent without creating multiple
React instances.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b0f44ba2-98b9-4755-9447-a03cd2443f8f

📥 Commits

Reviewing files that changed from the base of the PR and between 4707489 and 6780afc.

📒 Files selected for processing (21)
  • .cursor/skills/agent-skills
  • android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt
  • android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt
  • android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
  • android/app/src/main/res/values/strings_voip.xml
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ActionSheet/Provider.tsx
  • app/containers/ActionSheet/styles.ts
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/lib/notifications/push.ts
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/playCallEndedSound.test.ts
  • app/lib/services/voip/playCallEndedSound.ts
  • app/lib/services/voip/useCallStore.ios.test.ts
  • app/lib/services/voip/useCallStore.test.ts
  • app/lib/services/voip/useCallStore.ts
  • app/lib/store/index.ts
💤 Files with no reviewable changes (2)
  • .cursor/skills/agent-skills
  • android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt
📜 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 (6)
**/*.{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/services/voip/useCallStore.ios.test.ts
  • app/lib/store/index.ts
  • app/containers/ActionSheet/Provider.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/playCallEndedSound.test.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/lib/services/voip/playCallEndedSound.ts
  • app/lib/services/voip/useCallStore.test.ts
  • app/lib/notifications/push.ts
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/useCallStore.ts
  • app/containers/ActionSheet/styles.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{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

**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms

Files:

  • app/lib/services/voip/useCallStore.ios.test.ts
  • app/lib/store/index.ts
  • app/containers/ActionSheet/Provider.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/playCallEndedSound.test.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/lib/services/voip/playCallEndedSound.ts
  • app/lib/services/voip/useCallStore.test.ts
  • app/lib/notifications/push.ts
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/useCallStore.ts
  • app/containers/ActionSheet/styles.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with @rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/services/voip/useCallStore.ios.test.ts
  • app/lib/store/index.ts
  • app/containers/ActionSheet/Provider.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/playCallEndedSound.test.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/lib/services/voip/playCallEndedSound.ts
  • app/lib/services/voip/useCallStore.test.ts
  • app/lib/notifications/push.ts
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/useCallStore.ts
  • app/containers/ActionSheet/styles.ts
app/lib/services/voip/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration

Files:

  • app/lib/services/voip/useCallStore.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/playCallEndedSound.test.ts
  • app/lib/services/voip/playCallEndedSound.ts
  • app/lib/services/voip/useCallStore.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/useCallStore.ts
app/lib/store/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Configure Redux middleware (saga, app state, internet state) in app/lib/store/ directory

Files:

  • app/lib/store/index.ts
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Create reusable UI components in app/containers/ directory

Files:

  • app/containers/ActionSheet/Provider.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/styles.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.

Applied to files:

  • app/lib/services/voip/useCallStore.ios.test.ts
  • app/lib/store/index.ts
  • app/containers/ActionSheet/Provider.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/playCallEndedSound.test.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/lib/services/voip/playCallEndedSound.ts
  • app/lib/services/voip/useCallStore.test.ts
  • app/lib/notifications/push.ts
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/useCallStore.ts
  • app/containers/ActionSheet/styles.ts
🔇 Additional comments (20)
app/lib/services/voip/MediaCallEvents.ts (1)

56-56: Good internal-doc clarification for test-only export.

Line 56 makes the intent explicit and avoids ambiguity around why this function remains exported.

app/lib/notifications/push.ts (1)

188-188: Good change: success logging no longer leaks token data.

This keeps registration observability while removing sensitive token exposure from logs.

app/lib/store/index.ts (1)

8-23: Dev logger wiring looks correct.

The logger is only added in __DEV__, so production behavior stays unchanged while development gets the expected middleware visibility.

app/containers/ActionSheet/styles.ts (1)

69-73: Restoring the full-container height looks correct.

height: '100%' is a sensible addition here and keeps the existing full-container behavior intact for callers that still opt in.

app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx (1)

29-32: The new hugContent opt-in is wired through cleanly.

Passing hugContent: isAndroid here keeps the VoIP action sheet behavior platform-specific without affecting other ActionSheet callers.

app/containers/ActionSheet/ActionSheet.tsx (1)

125-133: Prop forwarding for hugContent looks right.

This keeps the ActionSheet/BottomSheetContent contract aligned with the new option and preserves existing behavior for callers that do not set it.

app/containers/ActionSheet/Provider.tsx (1)

22-39: The option type update is consistent with the new ActionSheet flow.

Adding hugContent?: boolean here is the right place for the new prop and keeps the public options shape aligned with the downstream components.

app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx (2)

84-90: The helper now checks the right prop.

Switching the expectation from fullContainer to hugContent keeps the test aligned with the updated ActionSheet contract.


300-314: The iOS assertion matches the new default behavior.

Checking hugContent: false here is the right expectation for the non-Android path.

app/containers/ActionSheet/BottomSheetContent.tsx (1)

14-24: The new hugContent gating is consistent with the intended layout split.

This keeps the existing full-container path for callers that still opt in, while letting Android consumers opt out cleanly when they want the sheet to hug its content.

Also applies to: 26-45, 81-88

app/lib/services/voip/useCallStore.ts (1)

7-7: Looks good: the shared logger is wired through the async and sync error paths.

This keeps the existing behavior intact while routing the failures through the project logging helper.

Also applies to: 171-181, 262-270, 323-333

app/lib/services/voip/useCallStore.ios.test.ts (1)

8-12: Good coverage for the iOS no-op path and logger mock.

The test setup stays aligned with the production import change and continues to verify that audio route sync is not invoked on iOS.

Also applies to: 90-107

app/lib/services/voip/MediaSessionInstance.ts (1)

31-31: Good: failure paths now report through the shared logger without changing session flow.

This keeps the recovery behavior intact while making the VoIP errors observable in the project logging pipeline.

Also applies to: 57-59, 74-76, 129-131, 153-177, 186-190

app/lib/services/voip/MediaSessionInstance.test.ts (1)

12-16: Nice: the added assertions cover the new logging and rollback behavior.

This gives solid coverage for the translated failure paths and the optimistic roomId cleanup on rejected start attempts.

Also applies to: 892-907, 911-994

app/lib/services/voip/useCallStore.test.ts (1)

9-13: Solid test coverage for the new logger-based error handling.

The additions cover the changed VoIP failure paths without widening the behavioral surface area of the store.

Also applies to: 390-399, 500-543

app/lib/services/voip/playCallEndedSound.test.ts (1)

3-7: Good: the load failure path is now asserted against the shared logger.

That keeps the test aligned with the production swap from console logging to the app logger.

Also applies to: 37-37, 154-161

app/lib/services/voip/playCallEndedSound.ts (1)

3-3: Looks good: playback failures are still contained, and the lock is released.

The change only swaps the reporting path to the shared logger, which keeps the playback flow intact.

Also applies to: 60-64

android/app/src/main/res/values/strings_voip.xml (1)

1-9: Nice cleanup.

The VoIP notification copy is centralized here cleanly, and the %1$s placeholder is the right way to inject the caller name.

android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (1)

86-88: Good consolidation here.

Reusing deviceId(context) across the REST and DDP reconciliation paths keeps the contractId behavior consistent, and switching the notification text/actions to string resources is a solid cleanup.

Also applies to: 174-177, 321-324, 336-339, 507-510, 560-563, 701-701, 941-942, 950-951

android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt (1)

6-6: Looks good.

The service notification copy is now resource-backed without changing the foreground-service behavior.

Also applies to: 133-134, 152-152

… path

Tracks PR #7295 — `console.warn('[VoIP] Call not found after accept:', id)`
became `log(new Error('[VoIP] Call not found after accept: <id>'))`. In tests
log() falls through to `console.error(Error)`, so the allowlist entry moves
from the warn list to the error list and the assertion targets consoleErrorSpy.
@diegolmello diegolmello had a problem deploying to approve_e2e_testing May 4, 2026 15:29 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to official_android_build May 4, 2026 15:34 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to official_ios_build May 4, 2026 15:34 — with GitHub Actions Failure
@diegolmello diegolmello temporarily deployed to experimental_ios_build May 4, 2026 15:34 — with GitHub Actions Inactive
@diegolmello diegolmello temporarily deployed to experimental_android_build May 4, 2026 15:34 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat Experimental 4.72.0.108749

@diegolmello diegolmello had a problem deploying to upload_experimental_android May 4, 2026 16:26 — with GitHub Actions Failure
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

Android Build Available

Rocket.Chat Experimental 4.72.0.108748

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNRPIc430HePGGaAjFFicZ9eypREC_E6JSPROQcfJbWFrAwV5qNawYQNMpV8t_S9AGfWJe5JldSXVOEBkGjZ

@diegolmello diegolmello merged commit 57447d0 into feat.voip-lib-new May 4, 2026
12 of 16 checks passed
@diegolmello diegolmello deleted the chore.review-voip branch May 4, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant