Skip to content

refactor(voip): MediaCallEvents Redux adapters and resetVoipState - #7178

Merged
diegolmello merged 3 commits into
feat.voip-lib-newfrom
refactor/voip-pr5-media-call-events
Apr 20, 2026
Merged

refactor(voip): MediaCallEvents Redux adapters and resetVoipState#7178
diegolmello merged 3 commits into
feat.voip-lib-newfrom
refactor/voip-pr5-media-call-events

Conversation

@diegolmello

@diegolmello diegolmello commented Apr 17, 2026

Copy link
Copy Markdown
Member

Proposed changes

This PR decouples VoIP native event wiring from Redux: MediaCallEvents no longer imports the global store or deepLinkingOpen. Instead, Root passes MediaCallEventsAdapters (getActiveServerUrl, onOpenDeepLink) into setupMediaCallEvents and getInitialMediaCallEvents, so reads and dispatches happen at the composition root with current store state.

VoIP teardown after a failed native accept is centralized in new resetVoipState() (now also clears module-level VoIP accept dedupe sentinels, then resets native call id, then full call-store reset). The deep-linking saga calls that helper instead of duplicating useCallStore calls.

resetMediaCallEventsStateForTesting() is exported for tests, and iOS endCall clears module-level VoIP accept dedupe keys so a later call with the same UUID is not incorrectly suppressed. Android coverage for the same dedupe reset is provided through resetVoipState() (no iOS-only endCall path on Android).

Logging that used console.log/console.error in this module now goes through MediaCallLogger. MediaCallLogger.error/warn correctly route to console.error/console.warn (previously silently downgraded to console.log). Unit tests mock MediaCallLogger to avoid noisy output.

Review fixes (latest commit)

  • MediaCallLogger.error / warn now use console.error / console.warn (severity downgrade bug)
  • VoipAcceptSucceeded sentinel write moved past the type !== 'incoming_call' guard so non-incoming events cannot poison dedupe for a real incoming call
  • clearVoipAcceptDedupeSentinels exported and called from resetVoipState() for cross-platform sentinel reset
  • Remaining console.error calls migrated to mediaCallLogger.error
  • Removed unused path?: string field from VoipDeepLinkParams
  • Added regression tests: sentinel poisoning, distinct callIds not suppressed, getActiveServerUrl undefined → deep-link path, resetVoipState clear-then-reset ordering

Tests: Android suite updated for adapter injection; iOS suite adds coverage for VoipPushTokenRegistered (registers push token), cold-start getInitialMediaCallEvents (answered + same vs foreign workspace), endCall dedupe reset, and a small resetVoipState ordering test. 35 tests across 3 suites pass.

Issue(s)

How to test or reproduce

  • TZ=UTC yarn test --testPathPattern='MediaCallEvents|resetVoipState'
  • Smoke: cold start / incoming VoIP on iOS and Android; accept failure toast path; workspace switch during accept.
  • Verify on Android: rapid-accept the same call twice → sentinel reset prevents the second accept from being suppressed.

Screenshots

N/A (behavioral / architectural change).

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

Cold-start NativeVoipModule.clearInitialEvents() coverage on answered paths matches the previous implementation (unchanged in this PR). endCall listener remains iOS-scoped as before. Reviewers flagged possible follow-ups for native stash lifecycle and Android parity; those can be separate if product agrees.

Summary by CodeRabbit

  • Improvements

    • More robust VoIP call handling and startup behavior, with reliable cold-start and server-matching for incoming calls.
    • Reduced direct state coupling for cleaner call flow and clearer diagnostics.
  • Bug Fixes

    • End-call now reliably clears accept deduplication so repeated call flows can be retriggered after reset.
  • Tests

    • Expanded VoIP test coverage, including idempotency, cold-start, and cross-platform event scenarios.

- Add resetVoipState helper and use it from deepLinking saga on accept failure
- Remove Redux store imports from MediaCallEvents; wire adapters from Root
- Export resetMediaCallEventsStateForTesting; clear accept dedupe on iOS endCall
- Route VoIP diagnostics through MediaCallLogger; mock logger in unit tests
- Add iOS cold start, VoipPushTokenRegistered, endCall dedupe, resetVoipState tests

Made-with: Cursor
@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Replaces direct Redux/store coupling in VoIP handling with dependency-injected adapters, adds deduplication controls and reset helper, updates initialization to pass adapters, and expands tests and mocks for adapter-based VoIP event flows and sentinel lifecycle.

Changes

Cohort / File(s) Summary
Media Call Events Core
app/lib/services/voip/MediaCallEvents.ts
Introduce exported types VoipDeepLinkParams and MediaCallEventsAdapters; change setupMediaCallEvents and getInitialMediaCallEvents to accept adapters; replace direct store reads/dispatches with adapters.getActiveServerUrl() / adapters.onOpenDeepLink(); add resetMediaCallEventsStateForTesting() and clearVoipAcceptDedupeSentinels(); use mediaCallLogger.
App entry
app/index.tsx
Wire initialization to call setupMediaCallEvents(this.getMediaCallEventsAdapters()) and getInitialMediaCallEvents(this.getMediaCallEventsAdapters()); add Root.getMediaCallEventsAdapters() producing getActiveServerUrl and onOpenDeepLink.
Tests — Media Call Events
app/lib/services/voip/MediaCallEvents.test.ts, app/lib/services/voip/MediaCallEvents.ios.test.ts
Refactor tests to inject makeTestAdapters(); replace store-dispatch deep-link assertions with mockOnOpenDeepLink checks; add shared native emitter mock and utilities; expand coverage for push-token registration, cold-start answered-call handling, accept-sentinel idempotency, and dedupe clearing on endCall; mock MediaCallLogger.
VoIP Reset Utility & Tests
app/lib/services/voip/resetVoipState.ts, app/lib/services/voip/resetVoipState.test.ts
Add resetVoipState() that calls clearVoipAcceptDedupeSentinels() then resetNativeCallId() and reset() from call store in sequence; tests validate ordering and interaction with dedupe state.
Saga update
app/sagas/deepLinking.js
Replace direct useCallStore resets with resetVoipState() inside handleVoipAcceptFailed.
Logger tweak
app/lib/services/voip/MediaCallLogger.ts
Switch warn/error to use console.warn / console.error (no signature changes).

Sequence Diagram(s)

sequenceDiagram
participant Native as Native VoIP
participant Media as MediaCallEvents
participant Adapters as Adapters (getActiveServerUrl,onOpenDeepLink)
participant App as App / Root
participant Store as Call Store (resetVoipState)
participant CallKeep as RNCallKeep

Native->>Media: emit VoipAcceptSucceeded / VoipAcceptFailed / VoipPushTokenRegistered
Media->>Adapters: getActiveServerUrl()
alt host matches or differs (decision inside Media)
Media->>Adapters: onOpenDeepLink(host, callId, flags)
end
Media->>CallKeep: listen for endCall
CallKeep->>Media: endCall -> Media clears dedupe sentinels
App->>Media: init -> setupMediaCallEvents(adapters)
App->>Media: init -> getInitialMediaCallEvents(adapters)
App->>Store: resetVoipState()
Store->>Media: clearVoipAcceptDedupeSentinels()
Store->>CallKeep: resetNativeCallId()
Store->>Store: reset()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 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 changes: refactoring VoIP MediaCallEvents to use Redux adapters and introducing a new resetVoipState utility function.

✏️ 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.

❤️ Share

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.

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)
app/lib/services/voip/MediaCallEvents.ts (1)

81-92: ⚠️ Potential issue | 🟡 Minor

Dedupe sentinel is set before the type guard — a non-incoming event can suppress a later incoming one.

At Lines 83-88, lastHandledVoipAcceptSucceededCallId is written as soon as a callId is present, but the data.type !== 'incoming_call' short-circuit at Line 89 happens after. If a VoipAcceptSucceeded is delivered with type !== 'incoming_call' for callId=X, the sentinel is poisoned with X; a subsequent legitimate incoming_call event with the same callId would then be silently dropped (no setNativeAcceptedCallId, no onOpenDeepLink).

Move the sentinel write past the type guard so we only mark "handled" once we actually handle it.

🔧 Proposed fix
 function handleVoipAcceptSucceededFromNative(data: VoipPayload, adapters: MediaCallEventsAdapters) {
 	const { callId } = data;
 	if (callId && lastHandledVoipAcceptSucceededCallId === callId) {
 		return;
 	}
-	if (callId) {
-		lastHandledVoipAcceptSucceededCallId = callId;
-	}
 	if (data.type !== 'incoming_call') {
 		mediaCallLogger.log(`${TAG} VoipAcceptSucceeded: not an incoming call`);
 		return;
 	}
+	if (callId) {
+		lastHandledVoipAcceptSucceededCallId = callId;
+	}
 	mediaCallLogger.log(`${TAG} VoipAcceptSucceeded:`, data);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/services/voip/MediaCallEvents.ts` around lines 81 - 92, The dedupe
sentinel lastHandledVoipAcceptSucceededCallId is set too early in
handleVoipAcceptSucceededFromNative; move the write (the assignment to
lastHandledVoipAcceptSucceededCallId) to after the type guard (after verifying
data.type === 'incoming_call') so that non-incoming events do not poison the
sentinel and prevent later legitimate incoming_call handling; ensure that the
existing logic that calls setNativeAcceptedCallId and adapters.onOpenDeepLink
still runs before or alongside the sentinel write inside
handleVoipAcceptSucceededFromNative.
🧹 Nitpick comments (1)
app/lib/services/voip/MediaCallEvents.ts (1)

97-99: Inconsistent logging: console.error remains after migration to MediaCallLogger.

The PR replaces console.log with mediaCallLogger.log/.warn, but four console.error call sites remain (REST signals failures, VoipAcceptSucceeded catch, initial REST signals failure, and the outer getInitialMediaCallEvents catch). The mocked MediaCallLogger in tests already exposes .error, so this is straightforward to align.

🔧 Proposed fix
-		console.error(`${TAG} applyRestStateSignals failed:`, error);
+		mediaCallLogger.error(`${TAG} applyRestStateSignals failed:`, error);

and similarly for the three other sites.

Also applies to: 194-196, 269-271, 284-285

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/services/voip/MediaCallEvents.ts` around lines 97 - 99, Replace
remaining console.error calls with the MediaCallLogger.error method to be
consistent with the rest of the file: change the error handler on
mediaSessionInstance.applyRestStateSignals() to call mediaCallLogger.error(...),
update the catch in the VoipAcceptSucceeded handling to use
mediaCallLogger.error, replace the initial REST signals failure log and the
outer getInitialMediaCallEvents catch to call mediaCallLogger.error as well;
locate these call sites by searching for
mediaSessionInstance.applyRestStateSignals, VoipAcceptSucceeded catch blocks,
and getInitialMediaCallEvents and swap console.error(...) for
mediaCallLogger.error(...) while preserving the original message and error
object.
🤖 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/lib/services/voip/MediaCallEvents.ios.test.ts`:
- Around line 42-62: The NativeEventEmitter class currently has an empty
constructor which triggers the no-empty-function lint rule; either remove the
explicit constructor entirely or give it a trivial body (e.g., a comment or noop
statement) or extend the eslint disable to include no-empty-function; update the
constructor in the NativeEventEmitter class (or delete it) so the CI lint error
is resolved while keeping the addListener behavior intact.
- Around line 6-16: Imports in MediaCallEvents.ios.test.ts are misordered and
causing ESLint/CI failures; reorder them so external modules (e.g.,
react-native-callkeep -> RNCallKeep) come first, then type imports (e.g.,
VoipPayload), then relative project imports (NativeVoipModule,
getInitialMediaCallEvents, resetMediaCallEventsStateForTesting,
setupMediaCallEvents, MediaCallEventsAdapters, useCallStore, registerPushToken),
and ensure there are single blank lines separating these groups; move
registerPushToken into the relative-import group and add blank lines between
external, type, and relative blocks to satisfy import-order rules.

---

Outside diff comments:
In `@app/lib/services/voip/MediaCallEvents.ts`:
- Around line 81-92: The dedupe sentinel lastHandledVoipAcceptSucceededCallId is
set too early in handleVoipAcceptSucceededFromNative; move the write (the
assignment to lastHandledVoipAcceptSucceededCallId) to after the type guard
(after verifying data.type === 'incoming_call') so that non-incoming events do
not poison the sentinel and prevent later legitimate incoming_call handling;
ensure that the existing logic that calls setNativeAcceptedCallId and
adapters.onOpenDeepLink still runs before or alongside the sentinel write inside
handleVoipAcceptSucceededFromNative.

---

Nitpick comments:
In `@app/lib/services/voip/MediaCallEvents.ts`:
- Around line 97-99: Replace remaining console.error calls with the
MediaCallLogger.error method to be consistent with the rest of the file: change
the error handler on mediaSessionInstance.applyRestStateSignals() to call
mediaCallLogger.error(...), update the catch in the VoipAcceptSucceeded handling
to use mediaCallLogger.error, replace the initial REST signals failure log and
the outer getInitialMediaCallEvents catch to call mediaCallLogger.error as well;
locate these call sites by searching for
mediaSessionInstance.applyRestStateSignals, VoipAcceptSucceeded catch blocks,
and getInitialMediaCallEvents and swap console.error(...) for
mediaCallLogger.error(...) while preserving the original message and error
object.
🪄 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: 07c75b89-7dff-4299-b2f7-f7e8404cdcc1

📥 Commits

Reviewing files that changed from the base of the PR and between 9c84d4c and deb289c.

📒 Files selected for processing (7)
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/sagas/deepLinking.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/sagas/deepLinking.js
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/sagas/deepLinking.js
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.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/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.ts
app/lib/services/voip/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Files:

  • app/lib/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.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/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/sagas/deepLinking.js
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.ts
app/index.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Configure Redux provider, theme, navigation, and notifications in app/index.tsx

Files:

  • app/index.tsx
🧠 Learnings (7)
📓 Common 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/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
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
📚 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/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Applied to files:

  • app/lib/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/sagas/deepLinking.js
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.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/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/lib/services/voip/resetVoipState.ts
  • app/sagas/deepLinking.js
  • app/index.tsx
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.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/deepLinking.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/deepLinking.js
  • app/index.tsx
📚 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/index.tsx : Configure Redux provider, theme, navigation, and notifications in app/index.tsx

Applied to files:

  • app/index.tsx
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
🪛 ESLint
app/lib/services/voip/MediaCallEvents.ios.test.ts

[error] 6-6: There should be at least one empty line between import groups

(import/order)


[error] 7-7: There should be at least one empty line between import groups

(import/order)


[error] 7-7: react-native-callkeep import should occur before type import of ../../../definitions/Voip

(import/order)


[error] 44-44: Unexpected empty constructor.

(no-empty-function)

🪛 GitHub Check: ESLint and Test / run-eslint-and-test
app/lib/services/voip/MediaCallEvents.ios.test.ts

[failure] 44-44:
Unexpected empty constructor


[failure] 7-7:
react-native-callkeep import should occur before type import of ../../../definitions/Voip


[failure] 7-7:
There should be at least one empty line between import groups


[failure] 6-6:
There should be at least one empty line between import groups

🔇 Additional comments (6)
app/lib/services/voip/resetVoipState.ts (1)

1-8: LGTM.

The ordering (resetNativeCallId before reset) is correct: per useCallStore.ts:297-310, reset() reads nativeAcceptedCallId and re-sets it in the new state, so clearing it first via resetNativeCallId() is required to actually drop the native-accepted id. Good consolidation from the saga.

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

10-25: LGTM.

Ordering is the critical invariant here and the test asserts it directly.

app/sagas/deepLinking.js (1)

30-30: LGTM.

Clean replacement; resetVoipState() preserves the prior call order and keeps the saga free of Zustand-store coupling.

Also applies to: 99-105

app/index.tsx (1)

36-40: LGTM — clean composition-root wiring of adapters.

getActiveServerUrl and onOpenDeepLink are lazy closures over store, so they correctly see live state at event time. Only two call sites (componentDidMount and init), so no perf concern from constructing a fresh adapters object each time.

Also applies to: 116-121, 137-137, 167-167

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

27-32: LGTM — good coverage of adapter injection and dedupe reset.

The new makeTestAdapters() helper keeps the test boilerplate minimal, and the added allows a second failure delivery … after reset test explicitly pins the behavior of resetMediaCallEventsStateForTesting(), which is what the iOS endCall path also relies on.

Also applies to: 69-76, 114-114, 214-221

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

51-59: LGTM — centralized dedupe clear + iOS endCall hook.

Exposing resetMediaCallEventsStateForTesting() (backed by the private clearVoipAcceptDedupeSentinels) and invoking the same helper from the CallKit endCall listener correctly closes the hole where a reused UUID in a later call was being suppressed. Tests exercise both paths.

Also applies to: 127-132

Comment thread app/lib/services/voip/MediaCallEvents.ios.test.ts Outdated
Comment thread app/lib/services/voip/MediaCallEvents.ios.test.ts

@diegolmello diegolmello left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Critical Review — PR #7178

Multi-angle review: anti-slop (code-simplifier), architecture, and test specialist agents, consolidated with the CodeRabbit findings already on this PR. Two confirmed defects, one of which is high-severity and silently introduced by this PR.


🔴 High — MediaCallLogger.error is a no-op for severity (silently downgrades errors)

app/lib/services/voip/MediaCallLogger.ts:14-20

error(...args: unknown[]): void {
    console.log(`[Media Call Error] ${JSON.stringify(args)}`);
}

warn(...args: unknown[]): void {
    console.log(`[Media Call Warning] ${JSON.stringify(args)}`);
}

error and warn both call console.log. The PR's stated intent is to migrate noisy logging through MediaCallLogger. Any future migration of the four remaining console.error sites (MediaCallEvents.ts:98, 195, 270, 285) to mediaCallLogger.error would silently demote severity, breaking Sentry/log-pipeline filtering. Fix in this PR before the planned migration runs.

-    error(...args: unknown[]): void {
-        console.log(`[Media Call Error] ${JSON.stringify(args)}`);
-    }
-    warn(...args: unknown[]): void {
-        console.log(`[Media Call Warning] ${JSON.stringify(args)}`);
-    }
+    error(...args: unknown[]): void {
+        console.error(`[Media Call Error] ${JSON.stringify(args)}`);
+    }
+    warn(...args: unknown[]): void {
+        console.warn(`[Media Call Warning] ${JSON.stringify(args)}`);
+    }

🔴 High — Dedupe sentinel set BEFORE type guard (also flagged by CodeRabbit, outside-diff)

app/lib/services/voip/MediaCallEvents.ts:81-92

function handleVoipAcceptSucceededFromNative(data: VoipPayload, adapters: MediaCallEventsAdapters) {
    const { callId } = data;
    if (callId && lastHandledVoipAcceptSucceededCallId === callId) {
        return;
    }
    if (callId) {
        lastHandledVoipAcceptSucceededCallId = callId;   // ← poisons sentinel
    }
    if (data.type !== 'incoming_call') {                 // ← guard fires AFTER
        mediaCallLogger.log(`${TAG} VoipAcceptSucceeded: not an incoming call`);
        return;
    }
    ...
}

A non-incoming VoipAcceptSucceeded with callId="X" writes the sentinel, then a legitimate incoming_call with the same callId="X" is silently dropped — no setNativeAcceptedCallId, no onOpenDeepLink. Move the sentinel write below the type guard.

No regression test catches this. Add: emit outgoing_call/non-incoming_call with callId X, then incoming_call with callId X, assert setNativeAcceptedCallId was called.


🟠 Medium — Android dedupe sentinels never reset

app/lib/services/voip/MediaCallEvents.ts:115-132

clearVoipAcceptDedupeSentinels() is called only from the iOS endCall listener, gated by if (isIOS). On Android there is no equivalent reset hook. If the server ever reuses a callId across calls (or across a failure → retry), the second event is silently suppressed.

Suggested: also call clearVoipAcceptDedupeSentinels() from resetVoipState() so both platforms reset through a single teardown entry point. This further justifies the helper extraction.


🟠 Medium — Cold-start workspace check runs against a possibly-unhydrated store

app/index.tsx:116-121 & MediaCallEvents.ts:268

getInitialMediaCallEvents is invoked from Root.init() which runs from the constructor — before appInitLocalSettings has finished hydrating state.server.server. getActiveServerUrl() can therefore return a falsy value, isVoipIncomingHostCurrentWorkspace returns false, and the same-workspace shortcut (applyRestStateSignals) is skipped in favor of the deep-link path. Functionally safe, but diverges from the documented intent and is testable: add a test where getActiveServerUrl returns undefined and assert the deep-link path runs.


🟠 Medium — Test coverage gaps

(from test specialist)

  1. Sentinel poisoning via type guard — no test (see High #2).
  2. Different callId NOT suppressed — same-id suppression is tested; the inverse is not.
  3. Sentinel reset on endCall — iOS-only test; no Android symmetric test or doc explaining absence.
  4. getActiveServerUrl returning undefined/null/"" — branch at MediaCallEvents.ts:40-41 untested.
  5. Catch-block coverage — all four console.error sites (98, 195, 270, 285) are unexercised; the try/catch at lines 192-197 is the most relevant since it was added defensively.
  6. registerPushToken assertion is too weakexpect(registerPushToken).toHaveBeenCalled() without verifying args (MediaCallEvents.ios.test.ts:225-230).
  7. iOS cold-start non-answered branch — only the answered branch is tested.
  8. resetVoipState failure path — only ordering tested.

🟡 Minor — Anti-slop / simplification

  • Mock duplication in MediaCallEvents.ios.test.ts:24-61DeviceEventEmitter.addListener and NativeEventEmitter.addListener bodies are byte-identical. Extract a makeAddListener(bucket) factory.
  • Pattern regression — the previous getMuteHandler() helper was removed and replaced by 4 inline mockAddEventListener.mock.calls.find(...) with non-null assertions. Re-extract.
  • resetMediaCallEventsStateForTesting is a one-line passthrough whose JSDoc claims it "is invoked internally on CallKit endCall" — but the internal caller invokes clearVoipAcceptDedupeSentinels directly, not this export. Either inline, or rename + drop the misleading JSDoc.
  • Speculative path field on VoipDeepLinkParams (MediaCallEvents.ts:20-27) — exported but never set; remove or document.
  • getMediaCallEventsAdapters() private method (app/index.tsx:113-127) — called twice in the same component; an inline const adapters = ... would read more directly.

CodeRabbit findings already on this PR (echoed)

  1. 🟡 MediaCallEvents.ios.test.ts:6-16 — import ordering ESLint failure (CI failing).
  2. 🟡 MediaCallEvents.ios.test.ts:42-62 — empty constructor no-empty-function (CI failing).
  3. 🔴 Outside-diff [MediaCallEvents.ts:81-92] — same sentinel ordering bug as High #2 above.
  4. 🟢 Nitpick [MediaCallEvents.ts:97-99, 194-196, 269-271, 284-285] — four remaining console.error to migrate to mediaCallLogger.error (blocked by High #1 above — fix the logger first).

Verdict

Request changes. The architectural goals (Redux decoupling, teardown centralization) are achieved cleanly, but the PR introduces a silent log-severity downgrade via MediaCallLogger.error and ships an existing dedupe-ordering bug uncovered by tests. Fix the logger, fix the sentinel ordering, add the missing regression tests, then this is mergeable. The iOS/Android sentinel asymmetry and cold-start hydration window can be follow-ups if scope is tight.

Fixes from PR review:
- MediaCallLogger.error/warn now route to console.error/console.warn
  (were silently downgraded to console.log)
- Move VoipAcceptSucceeded sentinel write past the type guard so
  non-incoming events cannot poison dedupe for a real incoming call
- Export clearVoipAcceptDedupeSentinels and call it from resetVoipState
  so Android (no iOS endCall path) also clears module-level sentinels
- Replace remaining console.error calls with mediaCallLogger.error
- Drop unused path field from VoipDeepLinkParams

Tests: regression coverage for sentinel poisoning, distinct callIds,
undefined active server, and resetVoipState ordering.

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/lib/services/voip/MediaCallEvents.ios.test.ts (1)

321-329: Prove the dedupe guard is active before testing the reset.

As written, this test also passes if VoipAcceptSucceeded is never deduped. Emit the duplicate once before endCall and assert it is suppressed, then verify endCall re-enables the same call ID.

🧪 Proposed fix
 it('allows a second VoipAcceptSucceeded with the same callId after endCall', () => {
 	setupMediaCallEvents(makeTestAdapters());
 	const payload = buildIncomingPayload({ callId: 'reuse-id', host: 'https://foreign.example.com' });
 	emitNativeVoipEvent('VoipAcceptSucceeded', payload);
+	emitNativeVoipEvent('VoipAcceptSucceeded', payload);
 	expect(mockOnOpenDeepLink).toHaveBeenCalledTimes(1);
 	getEndCallHandler()({ callUUID: 'any' });
 	emitNativeVoipEvent('VoipAcceptSucceeded', payload);
 	expect(mockOnOpenDeepLink).toHaveBeenCalledTimes(2);
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/services/voip/MediaCallEvents.ios.test.ts` around lines 321 - 329,
The test currently only verifies that a second VoipAcceptSucceeded after endCall
is handled, but doesn't prove dedupe was active; change the sequence so you emit
VoipAcceptSucceeded twice before calling getEndCallHandler() and assert
mockOnOpenDeepLink was called only once (verifying the duplicate was
suppressed), then call getEndCallHandler()({ callUUID: 'any' }) and emit
VoipAcceptSucceeded again with the same payload and assert mockOnOpenDeepLink
was called a second time (verifying the dedupe reset). Use the existing helpers
setupMediaCallEvents, emitNativeVoipEvent, getEndCallHandler, and
mockOnOpenDeepLink and the same payload (buildIncomingPayload with callId
'reuse-id') so the test proves both suppression and reset.
🤖 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/lib/services/voip/MediaCallEvents.ios.test.ts`:
- Around line 269-290: The test is missing an assertion that ensures REST state
for a foreign workspace is not applied locally; after calling
getInitialMediaCallEvents and the existing expects, add a negative assertion
that the mocked applyRestStateSignals handler was not invoked (e.g.,
expect(mockApplyRestStateSignals).not.toHaveBeenCalled() or
expect(applyRestStateSignals).not.toHaveBeenCalled()) to lock in the branch
where foreign-workspace accepts only open the deep link and do not apply REST
state locally; place this assertion alongside the existing expect calls in the
'returns true and opens deep link when answered on cold start but host differs
from workspace' test.

---

Nitpick comments:
In `@app/lib/services/voip/MediaCallEvents.ios.test.ts`:
- Around line 321-329: The test currently only verifies that a second
VoipAcceptSucceeded after endCall is handled, but doesn't prove dedupe was
active; change the sequence so you emit VoipAcceptSucceeded twice before calling
getEndCallHandler() and assert mockOnOpenDeepLink was called only once
(verifying the duplicate was suppressed), then call getEndCallHandler()({
callUUID: 'any' }) and emit VoipAcceptSucceeded again with the same payload and
assert mockOnOpenDeepLink was called a second time (verifying the dedupe reset).
Use the existing helpers setupMediaCallEvents, emitNativeVoipEvent,
getEndCallHandler, and mockOnOpenDeepLink and the same payload
(buildIncomingPayload with callId 'reuse-id') so the test proves both
suppression and reset.
🪄 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: 0830aa5d-a435-414d-905f-3030aee60137

📥 Commits

Reviewing files that changed from the base of the PR and between deb289c and 21c4bb5.

📒 Files selected for processing (6)
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/MediaCallEvents.ts
  • app/lib/services/voip/MediaCallLogger.ts
  • app/lib/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/resetVoipState.ts
✅ Files skipped from review due to trivial changes (1)
  • app/lib/services/voip/MediaCallLogger.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • app/lib/services/voip/resetVoipState.test.ts
  • app/lib/services/voip/MediaCallEvents.test.ts
  • app/lib/services/voip/resetVoipState.ts
  • app/lib/services/voip/MediaCallEvents.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). (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/lib/services/voip/MediaCallEvents.ios.test.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/services/voip/MediaCallEvents.ios.test.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/services/voip/MediaCallEvents.ios.test.ts
app/lib/services/voip/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Files:

  • app/lib/services/voip/MediaCallEvents.ios.test.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/services/voip/MediaCallEvents.ios.test.ts
🧠 Learnings (5)
📓 Common 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/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
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
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
📚 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/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.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/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/lib/services/voip/MediaCallEvents.ios.test.ts
📚 Learning: 2026-04-07T17:49:25.836Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-07T17:49:25.836Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Avoid deeply nested code; refactor complex logic into helper functions

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.ts
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.ts

Comment thread app/lib/services/voip/MediaCallEvents.ios.test.ts
Add negative expectation for applyRestStateSignals on the
cross-workspace iOS cold-start path (CodeRabbit review).

Made-with: Cursor

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

🧹 Nitpick comments (1)
app/lib/services/voip/MediaCallEvents.ios.test.ts (1)

1-332: LGTM — iOS test suite cleanly reworked for adapter-based wiring.

The shared mockNativeVoipListeners bucket + mockMakeAddListener factory is a clean way to simulate both NativeEventEmitter and DeviceEventEmitter, and the three prior review items (import ordering, empty NativeEventEmitter constructor, foreign-workspace negative applyRestStateSignals assertion at Line 291) are properly addressed. The new coverage for VoipPushTokenRegistered, cold-start answered/same-vs-foreign host branches, and endCall clearing the accept-dedupe sentinel all map directly to the behaviors introduced by MediaCallEventsAdapters and resetVoipState.

One small nit (optional): the workspace URL 'https://workspace-ios.example.com' is repeated as a literal on Lines 113, 272, and 297, and the shared-listener cleanup (Object.keys(mockNativeVoipListeners).forEach(k => delete mockNativeVoipListeners[k])) is duplicated across every beforeEach. Extracting a WORKSPACE_URL constant and a resetListeners() helper would DRY up the suite without changing behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/services/voip/MediaCallEvents.ios.test.ts` around lines 1 - 332,
Tests duplicate the workspace URL literal and repeat listener cleanup in every
beforeEach; extract a WORKSPACE_URL constant (used by mockServerSelector and the
test payloads in buildIncomingPayload calls) and add a resetListeners() helper
that does Object.keys(mockNativeVoipListeners).forEach(k => delete
mockNativeVoipListeners[k]) and call it inside each beforeEach alongside
resetMediaCallEventsStateForTesting(), then replace the three hard-coded
'https://workspace-ios.example.com' occurrences with WORKSPACE_URL and call
resetListeners() where the manual cleanup was used (refer to
mockNativeVoipListeners, mockServerSelector, buildIncomingPayload, and the
various beforeEach blocks to locate spots to update).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/lib/services/voip/MediaCallEvents.ios.test.ts`:
- Around line 1-332: Tests duplicate the workspace URL literal and repeat
listener cleanup in every beforeEach; extract a WORKSPACE_URL constant (used by
mockServerSelector and the test payloads in buildIncomingPayload calls) and add
a resetListeners() helper that does
Object.keys(mockNativeVoipListeners).forEach(k => delete
mockNativeVoipListeners[k]) and call it inside each beforeEach alongside
resetMediaCallEventsStateForTesting(), then replace the three hard-coded
'https://workspace-ios.example.com' occurrences with WORKSPACE_URL and call
resetListeners() where the manual cleanup was used (refer to
mockNativeVoipListeners, mockServerSelector, buildIncomingPayload, and the
various beforeEach blocks to locate spots to update).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 983b6884-d12c-45d5-a08d-9317cd83963e

📥 Commits

Reviewing files that changed from the base of the PR and between 21c4bb5 and 4e2251c.

📒 Files selected for processing (1)
  • app/lib/services/voip/MediaCallEvents.ios.test.ts
📜 Review details
🧰 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/lib/services/voip/MediaCallEvents.ios.test.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/services/voip/MediaCallEvents.ios.test.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/services/voip/MediaCallEvents.ios.test.ts
app/lib/services/voip/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Files:

  • app/lib/services/voip/MediaCallEvents.ios.test.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/services/voip/MediaCallEvents.ios.test.ts
🧠 Learnings (5)
📓 Common 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/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
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
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
📚 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/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.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/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/lib/services/voip/MediaCallEvents.ios.test.ts
📚 Learning: 2026-04-07T17:49:25.836Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-07T17:49:25.836Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Avoid deeply nested code; refactor complex logic into helper functions

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.ts
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.

Applied to files:

  • app/lib/services/voip/MediaCallEvents.ios.test.ts

@diegolmello
diegolmello had a problem deploying to experimental_ios_build April 20, 2026 20:18 — with GitHub Actions Failure
@diegolmello
diegolmello had a problem deploying to official_android_build April 20, 2026 20:18 — with GitHub Actions Failure
@diegolmello
diegolmello had a problem deploying to experimental_android_build April 20, 2026 20:18 — with GitHub Actions Failure
@diegolmello
diegolmello merged commit 41394a9 into feat.voip-lib-new Apr 20, 2026
5 of 10 checks passed
@diegolmello
diegolmello deleted the refactor/voip-pr5-media-call-events branch April 20, 2026 20:34
diegolmello added a commit that referenced this pull request Apr 22, 2026
…/Decline (#7215)

* merge feat.voip-lib

* feat(voip): enhance call handling with UUID mapping and event listeners

* Base call UI

* feat(voip): integrate Zustand for call state management and enhance CallView UI

* feat(voip): add simulateCall function for mock call handling in UI development

* refactor(CallView): update button handlers and improve UI responsiveness

* Add pause-shape-unfilled icon

* Base CallHeader

* toggleFocus

* collapse buttons

* Header components

* Hide header when no call

* Timer

* Add use memo

* Add voice call item on sidebar

* cleanup

* Temp use @rocket.chat/media-signaling from .tgz

* cleanup

* Check module and permissions to enable voip

* Refactor stop method to use optional chaining for media signal listeners

* voip push first test

* Add VoIP call handling with pending call management

- Implemented VoIP push notification handling in index.js, including storing call info for later processing.
- Added CallKeep event handlers for answering and ending calls from a cold start.
- Introduced a new CallIdUUID module to convert call IDs to deterministic UUIDs for compatibility with CallKit.
- Created a pending call store to manage incoming calls when the app is not fully initialized.
- Updated deep linking actions to include VoIP call handling.
- Enhanced MediaSessionInstance to process pending calls and manage call states effectively.

* Remove pending store and create getInitialEvents on app/index

* Attempt to make iOS calls work from cold state

* lint and format

* Patch callkeep ios

* Temp send iOS voip push token on gcm

* Temp fix require cycle

* chore: format code and fix lint issues [skip ci]

* CallIDUUID module on android and voip push

* Add setCallUUID on useCallStore to persist calls accepted on native Android

* remove callkeep from notification

* Android Incoming Call UI POC

* Refactor VoIP handling: Migrate VoIP-related classes to a new package structure, removing deprecated modules and consolidating functionality. Update imports in MainApplication and NotificationIntentHandler to reflect changes. This cleanup enhances code organization and prepares for future VoIP feature enhancements.

* Remove VoipForegroundService

* cleanup and use caller instead of callerName

* Cleanup and make iOS build again

* Refactor VoIP handling: Remove unused event emissions for call answered and declined, switch from SharedPreferences to in-memory storage for pending VoIP call data, and update method signatures for better clarity. This cleanup enhances performance and prepares for future VoIP feature improvements.

* Refactor VoIP handling: Introduce a new VoipPayload class to encapsulate call data, streamline notification processing, and enhance method signatures across the VoIP module. This update improves code clarity and prepares for future feature enhancements.

* Migrate react-native-voip-push-notifications to VoipModule

* Refactor VoIP module: Update package structure by moving VoipTurboPackage to the main package and removing the obsolete NativeVoipSpec class. Adjust imports in MainApplication and VoipModule to reflect these changes, enhancing code organization and maintainability.

* Unify emitters

* Move CallKeep listeners from MediaSessionInstance to getInitialEvents

* Clear callkeep on endcall

* Unify getInitialEvents logic

* getInitialEvents -> MediaCallEvents

* chore: format code and fix lint issues [skip ci]

* feat(Android): Add full screen incoming call (#6977)

* feat: Update call UI (#6990)

* feat: Handle audio routing, e.g., Bluetooth headset vs. internal speaker switching (#6992)

* fix: empty space when not on call (#6993)

* feat: Dialpad (#7000)

* action: organized translations

* feat: start call (#7024)

* chore: format code and fix lint issues

* feat: Pre flight (#7038)

* action: organized translations

* feat: Receive voip push notifications from backend (#7045)

* feat: Refactor media session handling and improve disconnect logic (#7065)

* feat: Control incoming call from native (#7066)

* feat: Voice message blocks (#7057)

* feat: native accept success event (#7068)

* feat(voip): call waiting, busy detection, and videoconf blocking (#7077)

* action: organized translations

* feat(voip): tap-to-hide call controls with animations (#7078)

* feat(voip): navigate to call DM from message button and header (#7082)

* feat(voip): tablet and landscape layout (#7110)

* chore: develop into feat.voip-lib-new (RN 81 + Expo 54 + reanimated 4 + true-sheet + iOS 26) (#7114)

* chore: format code and fix lint issues

* feat(voip): android landscape layout for IncomingCallActivity (#7116)

* Update agents files

* feat(voip): Support a11y (#7106)

* Fix content cutting on iOS on some edge cases

* pods

* Ignore .worktrees on jest

* chore: Merge develop into feat.voip-lib-new (#7129)

* fix(voip): show CallKit UI when call is active in background (#7128)

* chore: Update media-signaling to 0.2.0 (#7153)

* feat(voip): migrate iOS accept/reject from DDP to REST (#7124)

* Fix icons

* feat(voip): migrate Android accept/reject from DDP to REST (#7127)

* test(voip): integration tests for CallView pipeline (#7161)

* feat(voip): display video conf provider as subtitle (#7160)

* fix(voip): CallView button grid and correct landscape/dialpad layouts (#7164)

* fix(voip): prevent stale MMKV cache on Android first-install accept

MMKVKeyManager.initialize ran in MainApplication.onCreate before the JS
engine started and opened the default MMKV file via the Tencent 1.2 JAR
when it was still empty. Tencent caches instances per-ID in a singleton
registry, so that empty-state view was held for the rest of the process.
JS later wrote credentials through react-native-mmkv (MMKV Core 2.0),
which has its own separate registry. When a VoIP push arrived,
Ejson.getMMKV() got the cached empty Tencent instance and reported
"No userId found in MMKV for server". Closing and reopening the app
cleared the cache, which is why only the very first call after install
failed.

Drop the open/verify block — the encryption key is already cached from
SecureKeystore, so no MMKV handle is needed here. The first Tencent
instance is now created inside Ejson.getMMKV() after JS has written,
so it scans the file fresh.

* fix(voip): prevent duplicate ringtone on Android incoming call (#7158)

* fix(voip): set explicit snaps for NewMediaCall bottom sheet (#7165)

* Update app/lib/services/voip/MediaSessionStore.ts

Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>

* fix: make startVoipFork reactive to permissions-changed (#7151)

* fix(android): remove MediaProjectionService from merged manifest (#7190)

* fix(voip): Phone account creation (#7170)

* feat: add Enable Mobile Ringing toggle in user preferences (#7155)

* fix(voip): ship blockers for PushKit, licensing, outbound calls, push tokens (#7167)

* fix(android): Play Store mic discoverability, safer FCM logs, avatar auth via headers (#7171)

* fix(ios): serialize VoipService bridge statics (#7169)

* fix(voip): Android DDP thread safety and VoipPayload bundle parity (#7168)

* chore(voip): dead-code and hygiene sweep (#7174)

* refactor(voip): decouple navigateToCallRoom from Redux and backfill REST/connect tests (#7176)

* test(voip): tighten ringing endCall assertion and add VideoConf VoIP-lock saga coverage (#7177)

* fix(ios): harden VoIP DDP WebSocket client on receive failures and TLS (#7173)

* refactor(voip): MediaCallEvents Redux adapters and resetVoipState (#7178)

* refactor(voip): decouple peer autocomplete from Redux; simplify NewMediaCall (#7175)

* fix(ios): add NS_SWIFT_NAME to Challenge.runChallenge for Swift 6.2 compatibility

Swift 6.2 (Xcode 26.x / macos-26 runner) auto-renames the Objective-C
method runChallenge:didReceiveChallenge:completionHandler: to
run(_:didReceive:completionHandler:) when imported into Swift.

Add NS_SWIFT_NAME to explicitly pin the Swift import name, preventing
the compiler from applying its heuristics. This keeps the existing
Swift call site in DDPClient.swift working without changes.

* fix(ios): cancel old URLSession/webSocketTask before reconnecting in DDPClient.connect (#7197)

* fix(ios): add NSLock to nativeAcceptHandledCallIds and 10s REST timeout to handleNativeAccept (#7198)

* feat(android): create VoipCallService with FOREGROUND_SERVICE_MICROPHONE (#7199)

* fix(android): start VoipCallService on accept, stop on hangup/timeout, install end-call listener (#7200)

* fix(voip): enable DM nav for users with SIP extension (#7203)

* fix(android): handle null VoiceConnection in answerIncomingCall, notify JS (#7201)

* fix(voip): resolve closure capture ordering in handleNativeAccept (#7209)

* fix(android): integrate VoIP modules with SSL-pinned OkHttpClient (#7208)

* fix(push): gate id and voipToken behind server version checks, fix VideoConf caller extra (#7210)

* fix(voip): remove sensitive data from production logs (#7207)

* fix(android): remove isRunning guard + add double-tap guard on Accept/Decline

- VoipCallService: remove if (!isRunning) guard, call startForeground unconditionally
  (idempotent on Android, fixes Android 14+ foreground service requirement)
- IncomingCallActivity: add AtomicBoolean guard on handleAccept/handleDecline
  to prevent double-tap from triggering multiple service starts

---------

Co-authored-by: diegolmello <diegolmello@users.noreply.github.com>
Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>
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