fix: centralize DM subscription stub via lib/methods/createDirectMessage wrapper#7261
Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds a wrapper Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller (UI / Saga)
participant REST as REST API
participant Stub as createDirectMessageSubscriptionStub
participant Redux as Redux Store
participant DB as Database
participant Logger as Logger
Caller->>REST: createDirectMessageRest(username)
REST-->>Caller: { success, room }
alt success && room._id
Caller->>Stub: createDirectMessageSubscriptionStub({rid, username, fname}) [async]
Stub->>Stub: validate inputs
Stub->>DB: getSubscriptionByRoomId(rid)
DB-->>Stub: subscription or null
alt subscription exists
Stub-->>Caller: resolve (skip write)
else subscription missing
Stub->>Redux: store.getState() -> user
Redux-->>Stub: user data or null
alt user valid
Stub->>DB: db.write(() => subscriptions.create(stubDoc))
DB-->>Stub: success or error
alt write error
Stub->>Logger: log(error)
end
else missing user
Stub-->>Caller: resolve (skip write)
end
end
else not successful
REST-->>Caller: return result (no stub)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Review rate limit: 4/8 reviews remaining, refill in 25 minutes and 8 seconds.Comment |
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Claude review (cadence-babysit)
Reviewed against slice spec 2026-04-27-voip-dm-username-prefill / 01-stub-subscription-on-create-direct-message.
Verdict
LGTM. Implementation matches the spec one-to-one: idempotent stub, never-throws contract, narrow wire-in to the createDirectMessage success branch only.
Findings (informational, no severity)
app/lib/methods/createDirectMessageSubscriptionStub.ts: writes boths._raw.idands._raw._id = rid— consistent with how other WatermelonDB consumers in this repo seed Subscription rows; not a correctness concern.rid.replace(currentUserId, '').trim()to derive the peer userId is a heuristic; for the DM-rid format (concatenation of two ids) it is sound, and the function gracefully degrades to a single-elementuidsarray if extraction yields empty. No action.- The function's
try/catchswallows errors vialog(), satisfying the "navigation must proceed" contract from the spec. - Test file covers the spec's required cases (insert, idempotent skip, error swallow on
db.writeand ongetSubscriptionByRoomId, missing redux user, empty-input guards). - Goroom wire-in correctly
awaits the stub beforenavigate(...)souseSubscriptionreads the row on RoomView mount.
Out-of-scope items (per slice spec, intentionally not addressed)
- Generalising stub creation to other room-creation paths (channel join, federated, room create).
- Making
useSubscriptionobserve WatermelonDB changes. - The pre-existing
lr/lmschema-vs-interface mismatch.
CI snapshot at review time
- ESLint and Test: PASS
- Layne Security Scan: PASS
- Snyk: PASS (no manifest changes)
- CLA: signed
- CodeRabbit: review still in progress
- Platform builds (iOS / Android Experimental & Official) and E2E: gated behind
Holdjobs awaiting manual approval — these will not auto-run.
No outstanding review threads. Recommend a maintainer approve the platform-build Hold jobs once CodeRabbit settles.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/lib/methods/createDirectMessageSubscriptionStub.test.ts (1)
1-6: Replace magic string assertion with enum constant.Line 88 hardcodes
'd'. This makes the test fragile against enum-value changes and obscures intent.Proposed fix
import { createDirectMessageSubscriptionStub } from './createDirectMessageSubscriptionStub'; import { getSubscriptionByRoomId } from '../database/services/Subscription'; import database from '../database'; import { store as reduxStore } from '../store/auxStore'; import log from './helpers/log'; +import { SubscriptionType } from '../../definitions'; @@ - expect(created.t).toBe('d'); + expect(created.t).toBe(SubscriptionType.DIRECT);As per coding guidelines, "Use enums for sets of related constants rather than magic strings or numbers".
Also applies to: 88-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/createDirectMessageSubscriptionStub.test.ts` around lines 1 - 6, The test in createDirectMessageSubscriptionStub.test uses the magic string 'd' in an assertion; replace that literal with the appropriate enum constant (e.g., SubscriptionType.DIRECT or RoomType.DIRECT) by importing the enum at the top of the test file and updating the assertion that references 'd' to use the enum value (locate the assertion inside the createDirectMessageSubscriptionStub test). Ensure the enum import matches the actual exported enum name from your codebase and run tests to confirm no other hardcoded 'd' usages remain.
🤖 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/methods/createDirectMessageSubscriptionStub.ts`:
- Line 49: The current extraction using rid.replace(currentUserId, '') can
remove mid-string occurrences and corrupt otherUserId; in
createDirectMessageSubscriptionStub locate the peer id by checking whether rid
startsWith or endsWith currentUserId and then slice only that prefix/suffix (or
split on the known separator if present) and finally trim any separator
characters, assigning the result to otherUserId; update the logic around
variables rid, currentUserId, and otherUserId to ensure only the leading or
trailing occurrence is removed rather than any mid-string match.
---
Nitpick comments:
In `@app/lib/methods/createDirectMessageSubscriptionStub.test.ts`:
- Around line 1-6: The test in createDirectMessageSubscriptionStub.test uses the
magic string 'd' in an assertion; replace that literal with the appropriate enum
constant (e.g., SubscriptionType.DIRECT or RoomType.DIRECT) by importing the
enum at the top of the test file and updating the assertion that references 'd'
to use the enum value (locate the assertion inside the
createDirectMessageSubscriptionStub test). Ensure the enum import matches the
actual exported enum name from your codebase and run tests to confirm no other
hardcoded 'd' usages remain.
🪄 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: 4e4f30f6-c3c2-4418-9ae6-36aacc2ed70f
📒 Files selected for processing (3)
app/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/lib/methods/helpers/goRoom.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 (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/helpers/goRoom.tsapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.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/methods/helpers/goRoom.tsapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.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-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/helpers/goRoom.tsapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : 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
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/sdk.ts : Use the Rocket.Chat JS SDK in app/lib/services/sdk.ts for WebSocket real-time subscriptions
Applied to files:
app/lib/methods/createDirectMessageSubscriptionStub.ts
🔇 Additional comments (2)
app/lib/methods/helpers/goRoom.ts (1)
102-106: Good placement of stub seeding before navigation.Awaiting
createDirectMessageSubscriptionStubin the successful DM-create path cleanly addresses the fresh-DM prefill race without changing the outer control flow.app/lib/methods/createDirectMessageSubscriptionStub.test.ts (1)
72-216: Great test matrix for the stub helper.The suite covers happy path, idempotency, guard clauses, and swallowed-error behavior comprehensively for this helper.
…age wrapper - Add app/lib/methods/createDirectMessage.ts wrapping restApi.createDirectMessage, stamps WatermelonDB Subscription stub on success via createDirectMessageSubscriptionStub. - Recreate createDirectMessageSubscriptionStub.ts and its test (from PR #7261). - Migrate all 6 callers: MessageActions, DirectoryView, RoomInfoView, RoomMembersView/helpers, sagas/messages, canOpenRoom, goRoom. - Fixes peer-prefill regression on all DM-creation flows (spotlight, RoomInfo Message, message action menu, DirectoryView, RoomMembersView, deep-link).
diegolmello
force-pushed
the
claude/2026-04-27-voip-dm-username-prefill/01-stub-subscription-on-create-direct-message
branch
from
a354952 to
a784431
Compare
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
changed the title
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
temporarily deployed
to
experimental_ios_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/methods/createDirectMessage.test.ts`:
- Around line 64-73: The test currently expects createDirectMessage('foo') to
reject when mockedStub (the local stub write) rejects, but the intended behavior
is that createDirectMessage should succeed despite stub write failures; update
the test in createDirectMessage.test.ts so that
mockedRest.mockResolvedValue(restResult) and
mockedStub.mockRejectedValue(stubError) still result in await
createDirectMessage('foo') resolving (not throwing), assert the returned value
equals restResult.room, and keep assertions that mockedRest was called with
'foo' and mockedStub was called once; optionally assert error logging was
invoked if a logger mock exists.
In `@app/lib/methods/createDirectMessage.ts`:
- Around line 6-11: The call to createDirectMessageSubscriptionStub inside
createDirectMessage is currently awaited without error handling so stub failures
can abort DM creation; wrap that side-effect so errors are swallowed (or logged)
and do not propagate: when result?.success && result.room?._id is true, call
createDirectMessageSubscriptionStub({ rid: result.room._id, username, fname:
(result.room as any)?.fname }) inside a try/catch (or attach .catch(...)) and
ensure any thrown/rejected error is caught and handled locally (e.g., log via
the module logger) without rethrowing so createDirectMessage succeeds even if
the local DB stub write fails.
🪄 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: 0d2d0355-dffa-480b-8a37-a2500fd69d3a
📒 Files selected for processing (11)
app/containers/MessageActions/index.tsxapp/lib/methods/canOpenRoom.tsapp/lib/methods/createDirectMessage.test.tsapp/lib/methods/createDirectMessage.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/lib/methods/helpers/goRoom.tsapp/sagas/messages.jsapp/views/DirectoryView/index.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomMembersView/helpers.ts
✅ Files skipped from review due to trivial changes (7)
- app/lib/methods/canOpenRoom.ts
- app/containers/MessageActions/index.tsx
- app/views/RoomInfoView/index.tsx
- app/views/DirectoryView/index.tsx
- app/views/RoomMembersView/helpers.ts
- app/sagas/messages.js
- app/lib/methods/helpers/goRoom.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/lib/methods/createDirectMessageSubscriptionStub.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 (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/createDirectMessage.tsapp/lib/methods/createDirectMessage.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.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/methods/createDirectMessage.tsapp/lib/methods/createDirectMessage.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.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-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/createDirectMessage.tsapp/lib/methods/createDirectMessage.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : 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
🔇 Additional comments (1)
app/lib/methods/createDirectMessageSubscriptionStub.test.ts (1)
31-216: Comprehensive coverage for the stub’s edge cases and failure modes.This suite nicely validates idempotency, fallback behavior, and “never-throw” error handling paths without overcoupling to implementation details.
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Review — #7261 — 2026-04-28T15:00:00Z
Reviewed against slice spec 2026-04-28-voip-dm-stub-wrapper / 01-create-direct-message-wrapper, commit a7844312a.
Findings: 2 (critical: 0, major: 1, minor: 1, nit: 0, spec-mismatch: 0)
Verdict: COMMENT
Skipped (duplicate of existing thread): 2
minor: rid.replace(currentUserId, '') can corrupt otherUserId if id appears mid-string
Path: app/lib/methods/createDirectMessageSubscriptionStub.ts:49
Category: correctness
Suggested fix: Use startsWith/endsWith guards or split on the known separator:
const otherUserId = rid.startsWith(currentUserId)
? rid.slice(currentUserId.length).trim()
: rid.endsWith(currentUserId)
? rid.slice(0, -currentUserId.length).trim()
: rid.trim();String.replace removes the first occurrence anywhere in the string. Rocket.Chat DM room IDs are userId1userId2 (concatenated, no separator), so the current user's ID could theoretically appear in the middle of the peer's ID and produce a corrupted otherUserId. The stub's uids field is overwritten by the server sync shortly after, so the blast radius is low — but this is a correctness issue worth fixing to keep the stub accurate during the brief pre-sync window. CodeRabbit thread already open on this line; marking as duplicate to avoid double-posting.
(Duplicate of existing CodeRabbit thread — not re-posting inline, just noting here.)
major: Test 5 asserts stub rejection propagates, but the stub never throws by design
Path: app/lib/methods/createDirectMessage.test.ts:64–73
Category: test-coverage / design
The fifth test case ('propagates stub rejection (stub owns its own error handling)') mocks the stub as rejecting and asserts that createDirectMessage rejects. However:
createDirectMessageSubscriptionStubis documented and implemented to never throw — it wraps its entire body intry/catch { log(e) }(see stub line 32–80).- The wrapper (
createDirectMessage.ts) does not have its own try/catch around the stub call, meaning if the stub mock is made to throw (bypassing the real implementation's guard), the rejection propagates.
This creates a correctness gap: the test validates behavior that conflicts with the stated design contract ("stub owns its own error handling" says the stub never throws, yet the test proves the wrapper will propagate if the stub throws). If the real stub's try/catch is ever accidentally removed, this test will catch the regression correctly — but the test name is misleading.
Recommended fix: Either (a) add a try/catch in the wrapper so it genuinely cannot propagate stub failures regardless of stub implementation details, or (b) change the test to assert resolves (matching the design intent that stub failures are swallowed). Option (a) is more defensive. CodeRabbit's thread on createDirectMessage.ts:11 proposes (a); the thread on the test file proposes (b). They are linked — resolving one entails the other.
(Note: CodeRabbit thread on createDirectMessage.ts:11 misidentifies the missing guard as absent — the stub has the guard; the wrapper does not. The analysis is correct that the wrapper should guard too, but the guard location matters.)
Observations (no action needed)
createDirectMessageSubscriptionStub.tshas comprehensive internal error handling — all DB failures are caught and logged. The only propagation risk is if the stub's own try/catch is removed, which the wrapper guard would also protect against.- All 6 callers correctly migrated to the new wrapper import path.
goRoom.tssimplified correctly — inline stub call removed.- Slice acceptance criteria met for code structure, caller migration, and test count (5 wrapper + 10 stub = 15 tests).
- Wrap createDirectMessageSubscriptionStub in try/catch + log in createDirectMessage - Replace rid.replace(peer) with prefix/suffix-safe extraction (no nested ternary) - Unit test: stub rejection no longer fails wrapper when REST succeeded Addresses CodeRabbit review threads on PR #7261. Made-with: Cursor
|
Cadence pass |
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/methods/createDirectMessage.ts`:
- Line 5: The exported async function createDirectMessage lacks an explicit
return type; update its signature to declare the correct Promise return type
(matching whatever createDirectMessageRest returns) so the function is not using
an inferred type. Locate createDirectMessage and add a return type like
Promise<...> that mirrors createDirectMessageRest's return type (import or
reference the appropriate type/interface if needed), ensuring the exported
signature is explicitly typed.
🪄 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: 52425122-5410-4663-b653-3a237d59e1b7
📒 Files selected for processing (3)
app/lib/methods/createDirectMessage.test.tsapp/lib/methods/createDirectMessage.tsapp/lib/methods/createDirectMessageSubscriptionStub.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/lib/methods/createDirectMessage.test.ts
- app/lib/methods/createDirectMessageSubscriptionStub.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 (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/createDirectMessage.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/methods/createDirectMessage.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-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/createDirectMessage.ts
🔇 Additional comments (2)
app/lib/methods/createDirectMessage.ts (2)
1-3: Clean dependency boundary and naming.Imports are clear and keep responsibilities separated (REST call, stub method, logger).
7-16: Good isolation of non-critical stub failures.Line 8-Line 16 correctly prevents local stub-write errors from failing DM creation when the REST call succeeds.
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
temporarily deployed
to
experimental_ios_build
GitHub Actions
Inactive
diegolmello
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
|
Review Overall: clean migration, good test coverage. Three real issues.
|
diegolmello
had a problem deploying
to
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.72.0.108705 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTQAy25zwBz1vpamkfOhY9-p9C--l4sHhUf5RPHQMrKr_XRLT91UyezVWkkl-u76Onwls_3QEVZ45QGbIlK |
|
iOS Build Available Rocket.Chat Experimental 4.72.0.108710 |
…age wrapper - Add app/lib/methods/createDirectMessage.ts wrapping restApi.createDirectMessage, stamps WatermelonDB Subscription stub on success via createDirectMessageSubscriptionStub. - Recreate createDirectMessageSubscriptionStub.ts and its test (from PR #7261). - Migrate all 6 callers: MessageActions, DirectoryView, RoomInfoView, RoomMembersView/helpers, sagas/messages, canOpenRoom, goRoom. - Fixes peer-prefill regression on all DM-creation flows (spotlight, RoomInfo Message, message action menu, DirectoryView, RoomMembersView, deep-link).
- Wrap createDirectMessageSubscriptionStub in try/catch + log in createDirectMessage - Replace rid.replace(peer) with prefix/suffix-safe extraction (no nested ternary) - Unit test: stub rejection no longer fails wrapper when REST succeeded Addresses CodeRabbit review threads on PR #7261. Made-with: Cursor
diegolmello
force-pushed
the
claude/2026-04-27-voip-dm-username-prefill/01-stub-subscription-on-create-direct-message
branch
from
bc0d787 to
95f7014
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Failure
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Failure
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Failure
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Failure
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Failure
diegolmello
deleted the
claude/2026-04-27-voip-dm-username-prefill/01-stub-subscription-on-create-direct-message
branch
2 participants
Summary
app/lib/methods/createDirectMessage.tswrapsservices/restApi.createDirectMessageand stamps the WatermelonDB Subscription stub on success (helper from prior shape of this PR:createDirectMessageSubscriptionStub).containers/MessageActions,views/DirectoryView,views/RoomInfoView,views/RoomMembersView,sagas/messages.js,lib/methods/canOpenRoom,lib/methods/helpers/goRoom.https://rocketchat.atlassian.net/browse/VMUX-83
Why this shape
REST stays REST (
services/restApi.tsunchanged). DB writes happen at the methods layer — same convention aslib/methods/subscriptions/rooms.ts,lib/methods/getMessages.ts. Single source of truth: every "create a DM" caller imports fromlib/methods/createDirectMessage. Future callers automatically heal.Why develop (was feat.voip-lib-new)
The fix is not VoIP-specific — none of the touched files are VoIP-only. E2E noise on feat.voip-lib-new masked CI signal; develop gives a clean read.
Test plan
Summary by CodeRabbit