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

[diegolmello]

Proposed changes

Removes the hidden Redux dependency from the VoIP navigateToCallRoom helper and backfills unit coverage around REST push-token registration, the connect → VoIP media-session lifecycle, and the iOS-specific connect branch.

• app/lib/services/voip/navigateToCallRoom.ts no longer reads auxStore; it now takes { isMasterDetail } as an argument. Callers — CallButtons and MediaCallHeader/Content — read state.app.isMasterDetail via useAppSelector and pass it in.
• app/lib/services/voip/navigateToCallRoom.test.ts drops the auxStore mock and adds a case asserting that isMasterDetail: false is forwarded into goRoom.
• app/lib/services/restApi.test.ts adds coverage for registerPushToken: early return when there is no device token, iOS registers the apn payload without voipToken when the VoIP token is missing (matches current feat.voip-lib-new behavior after the guard drop), Android registration without a VoIP token (no voipToken in payload), dedup on repeated calls, and the iOS APNs payload shape when both tokens are present.
• app/lib/services/connect.test.ts now asserts mediaSessionInstance.reset is called exactly once when disconnect() runs.
• app/lib/services/connect.ios.test.ts mocks ../methods/helpers/deviceInfo instead of ../methods/helpers so the isIOS override matches the module connect.ts actually imports.

Branch was rebased onto feat.voip-lib-new to pick up the iOS early-return removal; the iOS-no-voip test was flipped accordingly.

No behavior change for end users: the navigation UX is identical, and the VoIP lifecycle assertions pin existing behavior that was previously untested.

Issue(s)

Part of the voip-lib-new cleanup track (PR-3: service boundary + REST/connect test backfill).

How to test or reproduce

TZ=UTC yarn test --testPathPattern='restApi.test|navigateToCallRoom.test|connect.test.ts|connect.ios.test'

Manual smoke: place/receive a VoIP call, tap the message/header button — it should still navigate to the call's DM on both phone and tablet (master-detail) layouts.

Screenshots

N/A — refactor + tests only.

Types of changes

• Bugfix (non-breaking change which fixes an issue) — iOS connect test was mocking the wrong module path
• Improvement (non-breaking change which improves a current function) — removes hidden Redux coupling in VoIP nav helper; adds missing unit coverage
• 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

navigateToCallRoom was previously importable from anywhere but silently required the Redux auxStore to be populated. Injecting isMasterDetail makes the dependency explicit and matches how other navigation helpers already read layout state at the call site. The connect.ios.test mock fix is small but load-bearing — the previous mock targeted ../methods/helpers, which re-exports isIOS from ./deviceInfo, so the override never reached the copy imported by connect.ts.

Summary by CodeRabbit

• Refactor

  • Improved call navigation to better manage master detail layout state through explicit parameter passing.

• Tests

  • Expanded push token registration coverage with platform-specific iOS and Android behavior validation.
  • Enhanced VoIP service initialization and disconnect handling test scenarios.

[coderabbitai]

Walkthrough

navigateToCallRoom was changed to accept an explicit isMasterDetail parameter (caller-provided) instead of reading from the store. Call sites in UI components were updated to read state and pass that value. Tests and mocks were adjusted and expanded, including new registerPushToken tests and mediaSessionInstance reset assertions.

Changes

Cohort / File(s)	Summary
VoIP Navigation Refactor app/lib/services/voip/navigateToCallRoom.ts, app/lib/services/voip/navigateToCallRoom.test.ts	Function signature now requires { isMasterDetail }; removed internal store read. Tests updated to pass explicit { isMasterDetail: ... } and include a case asserting false is forwarded to goRoom.
UI Callers Updated app/containers/MediaCallHeader/components/Content.tsx, app/views/CallView/components/CallButtons.tsx	Both components now select state.app.isMasterDetail via useAppSelector and pass it to navigateToCallRoom({ isMasterDetail }) in their press handlers; other handler behavior unchanged.
VoIP/connect Tests & Mocks app/lib/services/connect.test.ts, app/lib/services/connect.ios.test.ts	Jest mocks for ./voip/MediaSessionInstance simplified to keep only reset. iOS test mocking narrowed to .../methods/helpers/deviceInfo with isIOS: true. Added test asserting disconnect() calls mediaSessionInstance.reset() once.
REST API / Push Token Tests app/lib/services/restApi.test.ts	Added sdk.post mock and mocks for getDeviceToken, getLastVoipToken, and react-native-device-info. Introduced helper to load registerPushToken under different Platform.OS values and tests covering early exit on missing token, platform-specific payloads, deduplication, and inclusion/omission of voipToken.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

type: chore

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% 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 change: decoupling navigateToCallRoom from Redux and adding test coverage for REST/connect services.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

app/lib/services/restApi.test.ts (1)

42-57: Consider making Platform.OS mutation explicit by requiring Platform after jest.resetModules()

While the current pattern works because jest-expo's Platform mock is a stable singleton that persists across module resets, re-requiring Platform after jest.resetModules() makes the intent clearer and removes reliance on implementation details:

♻️ More explicit approach

 function loadRegisterPushToken(platform: 'ios' | 'android' = 'android') {
 	jest.resetModules();
-	Object.defineProperty(Platform, 'OS', { configurable: true, writable: true, value: platform });
 	// eslint-disable-next-line `@typescript-eslint/no-require-imports`
+	const { Platform: FreshPlatform } = require('react-native');
+	Object.defineProperty(FreshPlatform, 'OS', { configurable: true, writable: true, value: platform });
+	// eslint-disable-next-line `@typescript-eslint/no-require-imports`
 	const notifications = require('../notifications');

This also aligns the afterEach pattern: update line 109 to restore from the fresh Platform instance if you adopt this change.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/lib/services/restApi.test.ts` around lines 42 - 57, The test mutates
Platform.OS using a Platform instance that may be stale after
jest.resetModules(); update loadRegisterPushToken to require/import Platform
immediately after jest.resetModules() and then set Platform.OS (i.e., re-require
Platform inside loadRegisterPushToken before Object.defineProperty), so the
mutation targets the fresh mock instance; also update the corresponding
afterEach restore to read/reset Platform from a freshly required Platform (the
same pattern referenced on line ~109) to restore OS on the new instance.

🤖 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/restApi.test.ts`:
- Around line 42-57: The test mutates Platform.OS using a Platform instance that
may be stale after jest.resetModules(); update loadRegisterPushToken to
require/import Platform immediately after jest.resetModules() and then set
Platform.OS (i.e., re-require Platform inside loadRegisterPushToken before
Object.defineProperty), so the mutation targets the fresh mock instance; also
update the corresponding afterEach restore to read/reset Platform from a freshly
required Platform (the same pattern referenced on line ~109) to restore OS on
the new instance.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 958cd412-5203-4185-a50c-82217885025a

📥 Commits

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

📒 Files selected for processing (7)

• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/connect.ios.test.ts
• app/lib/services/connect.test.ts
• app/lib/services/restApi.test.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/voip/navigateToCallRoom.ts
• app/views/CallView/components/CallButtons.tsx

📜 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 (7)

**/*.{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/connect.test.ts
• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/connect.ios.test.ts
• app/lib/services/voip/navigateToCallRoom.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/restApi.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/connect.test.ts
• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/connect.ios.test.ts
• app/lib/services/voip/navigateToCallRoom.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/restApi.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/connect.test.ts
• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/connect.ios.test.ts
• app/lib/services/voip/navigateToCallRoom.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/restApi.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/connect.test.ts
• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/connect.ios.test.ts
• app/lib/services/voip/navigateToCallRoom.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/restApi.test.ts

app/views/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place screen components in app/views/ directory

Files:

• app/views/CallView/components/CallButtons.tsx

app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in app/containers/ directory

Files:

• app/containers/MediaCallHeader/components/Content.tsx

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/navigateToCallRoom.ts
• app/lib/services/voip/navigateToCallRoom.test.ts

🧠 Learnings (12)

📓 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

📚 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/connect.ts : Manage server connection in app/lib/services/connect.ts

Applied to files:

• app/lib/services/connect.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/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/connect.test.ts
• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/connect.ios.test.ts
• app/lib/services/voip/navigateToCallRoom.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/restApi.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/connect.test.ts
• app/lib/services/connect.ios.test.ts
• app/lib/services/voip/navigateToCallRoom.test.ts
• app/lib/services/restApi.test.ts

📚 Learning: 2026-03-10T15:21:45.098Z

Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.

Applied to files:

• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.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/AppContainer.tsx : Implement root navigation container logic in app/AppContainer.tsx to switch between authentication states

Applied to files:

• app/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/voip/navigateToCallRoom.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/lib/hooks/useResponsiveLayout/**/*.{ts,tsx} : Use responsive layout with master-detail on tablets and single stack on phones via useResponsiveLayout hook

Applied to files:

• app/views/CallView/components/CallButtons.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/theme.tsx : Configure theming context in app/theme.tsx

Applied to files:

• app/views/CallView/components/CallButtons.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/views/CallView/components/CallButtons.tsx
• app/containers/MediaCallHeader/components/Content.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/stacks/**/*.{ts,tsx} : Use React Navigation 7 for navigation with stacks for InsideStack (authenticated), OutsideStack (login/register), MasterDetailStack (tablets), and ShareExtensionStack

Applied to files:

• app/containers/MediaCallHeader/components/Content.tsx
• app/lib/services/voip/navigateToCallRoom.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/lib/store/**/*.{ts,tsx} : Configure Redux store in app/lib/store/ with middleware for saga, app state, and internet state

Applied to files:

• app/lib/services/voip/navigateToCallRoom.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/lib/services/sdk.ts : Use Rocket.Chat JS SDK in app/lib/services/sdk.ts for WebSocket real-time subscriptions

Applied to files:

• app/lib/services/restApi.test.ts

🔇 Additional comments (7)

app/lib/services/connect.test.ts (1)

309-314: LGTM!

The new test correctly verifies that disconnect() invokes mediaSessionInstance.reset() exactly once, matching the simplified mock shape above.

app/lib/services/connect.ios.test.ts (1)

7-10: LGTM — mock target fix aligns test isIOS with connect.ts.

Narrowing the mock to ../methods/helpers/deviceInfo (instead of the barrel) ensures the isIOS consumed by connect.ts is the mocked value, which is the intended bugfix for this PR.

app/views/CallView/components/CallButtons.tsx (1)

31-60: LGTM!

Cleanly threads isMasterDetail from Redux via useAppSelector into navigateToCallRoom, removing the hidden store coupling inside the VoIP service.

app/containers/MediaCallHeader/components/Content.tsx (1)

3-34: LGTM!

Matches the new navigateToCallRoom API and the wiring mirrors CallButtons.tsx. Disabled state and error swallowing are preserved.

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

10-40: Nice boundary cleanup.

Making isMasterDetail an explicit parameter removes the direct auxStore dependency from the VoIP service, aligning with the PR-3 goal of keeping the VoIP layer framework-/store-agnostic. Signature is a non-breaking refactor confined to the two internal call sites updated in this PR.

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

210-226: Good coverage addition.

The new case asserting isMasterDetail: false is forwarded into goRoom complements the existing true assertions and guards against regressions where the argument might be dropped or hardcoded. Removal of the auxStore mock is consistent with the updated signature.

app/lib/services/restApi.test.ts (1)

105-186: Nice coverage on registerPushToken.

Good split across the four behaviors that matter (missing push token early-exit, iOS-only VoIP gating, Android voipToken omission with an explicit hasOwnProperty check on line 154, dedup across repeated calls with identical tokens, and the full iOS APN payload shape on lines 175-184). Using jest.resetModules() inside the helper so the dedup-cache state is per-test is the right call, and capturing mockSdkPost in the ./sdk mock factory closure keeps assertions consistent across module resets.