feat: User phone identity lookup for external voice calls - #40680
Open
aleksandernsilva wants to merge 1 commit into
Open
feat: User phone identity lookup for external voice calls#40680aleksandernsilva wants to merge 1 commit into
aleksandernsilva wants to merge 1 commit into
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
WalkthroughCastDirector now resolves SIP extensions through an asynchronous phone lookup. The users model provides an indexed exact-match query. Unique matches add username and display name fields to SIP contacts; missing or ambiguous matches do not resolve to a user. ChangesPhone-based media-call contact resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change adds phone-number-based user lookup and enriches media-call contacts; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CastDirector
participant Users
participant SIPContactList
CastDirector->>Users: findByPhone(sipExtension, restricted projection)
Users-->>CastDirector: zero, one, or multiple users
CastDirector->>SIPContactList: build contact with extension, username, and display name
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/server/methods/saveUserProfile.ts (1)
211-221:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep DDP
saveUserProfiletyping in sync with runtimephonessupport.
saveUserProfilenow acceptssettings.phonesat runtime, but the@rocket.chat/ddp-clientServerMethodsdeclaration still omits it, forcing typed callers to cast.Suggested fix
interface ServerMethods { saveUserProfile( settings: { email?: string; username?: string; realname?: string; newPassword?: string; statusText?: string; statusType?: string; bio?: string; nickname?: string; + phones?: IUserPhoneNumber[]; }, customFields: Record<string, any>, ...args: unknown[] ): boolean; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/server/methods/saveUserProfile.ts` around lines 211 - 221, Update the saveUserProfile typing to include the runtime phones field: add a phones property to the settings parameter type used by saveUserProfile and mirror that same change in the `@rocket.chat/ddp-client` ServerMethods declaration for saveUserProfile so typed callers can pass phones without casting; ensure the phones shape matches the runtime structure (an array of phone objects used by the method) and update any related type aliases or imports referenced by saveUserProfile to keep signatures consistent.
🧹 Nitpick comments (2)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx (1)
46-47: ⚡ Quick winOptimize validation: skip format check for empty values.
The
validvalidator runs even when the field is empty, producing a redundant "invalid format" error alongside the "required" error. While only the first error displays, both validators execute unnecessarily.♻️ Proposed refinement
valid: (value: string) => - E164_PHONE_REGEX.test(value) ? true : t('__field__is_invalid', { field: `${t('Phone_number')} ${index + 1}` }), + !value || E164_PHONE_REGEX.test(value) ? true : t('__field__is_invalid', { field: `${t('Phone_number')} ${index + 1}` }),Apply the same pattern to the label validator if needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx` around lines 46 - 47, The current validator for the phone field runs E164_PHONE_REGEX.test even when the value is empty, causing redundant format checks alongside required validation; update the valid function in PhoneNumberFieldList (the arrow validator using E164_PHONE_REGEX and t('__field__is_invalid')) to first return true when value is falsy/empty (e.g., if (!value) return true) so format validation is skipped for empty inputs, and apply the same empty-short-circuit pattern to the label validator if it also performs format checks.packages/model-typings/src/models/IUsersModel.ts (1)
408-408: ⚡ Quick winAlign
setPhonestyping with clear/unset behavior.Line 408 currently requires
IUserPhoneNumber[], but the model implementation acceptsIUser['phones']and unsets when absent/empty. Keep the interface contract consistent so callers can clear phone data without casts.Proposed typing fix
- setPhones(userId: string, phones: IUserPhoneNumber[]): Promise<UpdateResult>; + setPhones(userId: string, phones: IUser['phones']): Promise<UpdateResult>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/model-typings/src/models/IUsersModel.ts` at line 408, The setPhones declaration currently requires IUserPhoneNumber[] but the implementation accepts and unsets using IUser['phones']; update the interface so setPhones(userId: string, phones: IUser['phones']): Promise<UpdateResult> (i.e., allow the same optional/null/empty shape as IUser['phones']) so callers can clear phone data without casts and the model can unset when phones is absent/empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/hooks/useAgentSection.tsx`:
- Around line 32-36: The code in useAgentSection.tsx currently picks
primaryPhone using user.phones?.[0], which can be wrong when a different phone
has primary: true; update the logic that defines primaryPhone (used by phoneItem
and onSelect) to first search user.phones for the entry with phone.primary ===
true and use its number, falling back to user.phone or the first phone number if
no primary flag is found; keep using formatPhoneNumber(primaryPhone) and the
existing phoneItem.id and onClick behavior unchanged.
In `@packages/models/src/models/Users.ts`:
- Around line 71-72: The phones.number index is non-unique, which makes
findOneByPhone ambiguous; either make phone numbers globally unique by changing
the index on phones.number to a unique (or unique-sparse) index, or modify the
findOneByPhone implementation to deterministically select a single owner by
adding filters such as 'phones.verified': true and 'phones.primary': true (and
create a corresponding compound index like { 'phones.number':1,
'phones.verified':1, 'phones.primary':1 } to support the query). Update the
Users model index definition (currently the phones.number index) and the
findOneByPhone query to use the chosen approach and document conflict behavior.
In `@packages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.ts`:
- Around line 71-85: The request schema currently allows client-controlled
phones[].verified; update the UsersUpdateOwnBasicInfoParamsPOST schema to remove
the verified property from phones (i.e., delete the
phones.items.properties.verified entry) and, in the server flow, ensure
saveUserProfile/Users.setPhones does not persist client-provided verified flags
by stripping or overriding verified before doing the $set (e.g., sanitize the
phones array in saveUserProfile so each phone’s verified is omitted or set
server-side). Ensure additionalProperties remains false and that required/other
phone fields are unchanged.
---
Outside diff comments:
In `@apps/meteor/server/methods/saveUserProfile.ts`:
- Around line 211-221: Update the saveUserProfile typing to include the runtime
phones field: add a phones property to the settings parameter type used by
saveUserProfile and mirror that same change in the `@rocket.chat/ddp-client`
ServerMethods declaration for saveUserProfile so typed callers can pass phones
without casting; ensure the phones shape matches the runtime structure (an array
of phone objects used by the method) and update any related type aliases or
imports referenced by saveUserProfile to keep signatures consistent.
---
Nitpick comments:
In `@apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx`:
- Around line 46-47: The current validator for the phone field runs
E164_PHONE_REGEX.test even when the value is empty, causing redundant format
checks alongside required validation; update the valid function in
PhoneNumberFieldList (the arrow validator using E164_PHONE_REGEX and
t('__field__is_invalid')) to first return true when value is falsy/empty (e.g.,
if (!value) return true) so format validation is skipped for empty inputs, and
apply the same empty-short-circuit pattern to the label validator if it also
performs format checks.
In `@packages/model-typings/src/models/IUsersModel.ts`:
- Line 408: The setPhones declaration currently requires IUserPhoneNumber[] but
the implementation accepts and unsets using IUser['phones']; update the
interface so setPhones(userId: string, phones: IUser['phones']):
Promise<UpdateResult> (i.e., allow the same optional/null/empty shape as
IUser['phones']) so callers can clear phone data without casts and the model can
unset when phones is absent/empty.
🪄 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: 0f65dbe3-78e6-411a-a619-b64eba170eef
📒 Files selected for processing (31)
apps/meteor/app/api/server/v1/users.tsapps/meteor/app/lib/server/functions/getFullUserData.tsapps/meteor/app/lib/server/functions/saveUser/saveNewUser.tsapps/meteor/app/lib/server/functions/saveUser/saveUser.tsapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsxapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsxapps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/client/components/UserInfo/UserInfo.tsxapps/meteor/client/views/account/profile/AccountProfileForm.tsxapps/meteor/client/views/account/profile/getProfileInitialValues.tsapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/views/admin/users/AdminUserInfoWithData.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/hooks/useAgentSection.tsxapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsxapps/meteor/ee/server/settings/voip.tsapps/meteor/server/methods/saveUserProfile.tsapps/meteor/server/services/media-call/service.tsee/packages/media-calls/src/definition/IMediaCallCastDirector.tsee/packages/media-calls/src/definition/IMediaCallServer.tsee/packages/media-calls/src/server/CastDirector.tsee/packages/media-calls/src/server/MediaCallServer.tsee/packages/media-calls/src/server/getDefaultSettings.tspackages/core-typings/src/IUser.tspackages/i18n/src/locales/en.i18n.jsonpackages/i18n/src/locales/pt-BR.i18n.jsonpackages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.tspackages/rest-typings/src/v1/Ajv.tspackages/rest-typings/src/v1/users/UserCreateParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateParamsPOST.ts
💤 Files with no reviewable changes (6)
- apps/meteor/server/services/media-call/service.ts
- ee/packages/media-calls/src/definition/IMediaCallCastDirector.ts
- ee/packages/media-calls/src/server/MediaCallServer.ts
- ee/packages/media-calls/src/definition/IMediaCallServer.ts
- ee/packages/media-calls/src/server/getDefaultSettings.ts
- apps/meteor/ee/server/settings/voip.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). (5)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build (coverage)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsxapps/meteor/app/lib/server/functions/saveUser/saveNewUser.tsapps/meteor/app/lib/server/functions/getFullUserData.tsapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/hooks/useAgentSection.tsxapps/meteor/client/views/account/profile/AccountProfileForm.tsxapps/meteor/app/lib/server/functions/saveUser/saveUser.tspackages/rest-typings/src/v1/users/UserCreateParamsPOST.tspackages/rest-typings/src/v1/Ajv.tsapps/meteor/app/api/server/v1/users.tsapps/meteor/client/views/admin/users/AdminUserInfoWithData.tsxpackages/model-typings/src/models/IUsersModel.tspackages/rest-typings/src/v1/users/UsersUpdateParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.tspackages/models/src/models/Users.tsapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsxapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsxpackages/core-typings/src/IUser.tsapps/meteor/server/methods/saveUserProfile.tsapps/meteor/client/components/UserInfo/UserInfo.tsxee/packages/media-calls/src/server/CastDirector.tsapps/meteor/client/views/account/profile/getProfileInitialValues.ts
🧠 Learnings (13)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/client/views/account/profile/getProfileInitialValues.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/client/views/account/profile/getProfileInitialValues.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/app/lib/server/functions/saveUser/saveNewUser.tsapps/meteor/app/lib/server/functions/getFullUserData.tsapps/meteor/app/lib/server/functions/saveUser/saveUser.tspackages/rest-typings/src/v1/users/UserCreateParamsPOST.tspackages/rest-typings/src/v1/Ajv.tsapps/meteor/app/api/server/v1/users.tspackages/model-typings/src/models/IUsersModel.tspackages/rest-typings/src/v1/users/UsersUpdateParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.tspackages/models/src/models/Users.tspackages/core-typings/src/IUser.tsapps/meteor/server/methods/saveUserProfile.tsee/packages/media-calls/src/server/CastDirector.tsapps/meteor/client/views/account/profile/getProfileInitialValues.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/app/lib/server/functions/saveUser/saveNewUser.tsapps/meteor/app/lib/server/functions/getFullUserData.tsapps/meteor/app/lib/server/functions/saveUser/saveUser.tspackages/rest-typings/src/v1/users/UserCreateParamsPOST.tspackages/rest-typings/src/v1/Ajv.tsapps/meteor/app/api/server/v1/users.tspackages/model-typings/src/models/IUsersModel.tspackages/rest-typings/src/v1/users/UsersUpdateParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.tspackages/models/src/models/Users.tspackages/core-typings/src/IUser.tsapps/meteor/server/methods/saveUserProfile.tsee/packages/media-calls/src/server/CastDirector.tsapps/meteor/client/views/account/profile/getProfileInitialValues.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/components/PhoneNumberFieldList/index.tsapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsxapps/meteor/app/lib/server/functions/saveUser/saveNewUser.tsapps/meteor/app/lib/server/functions/getFullUserData.tsapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/hooks/useAgentSection.tsxapps/meteor/client/views/account/profile/AccountProfileForm.tsxapps/meteor/app/lib/server/functions/saveUser/saveUser.tspackages/rest-typings/src/v1/users/UserCreateParamsPOST.tspackages/rest-typings/src/v1/Ajv.tsapps/meteor/app/api/server/v1/users.tsapps/meteor/client/views/admin/users/AdminUserInfoWithData.tsxpackages/model-typings/src/models/IUsersModel.tspackages/rest-typings/src/v1/users/UsersUpdateParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.tspackages/models/src/models/Users.tsapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsxapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsxpackages/core-typings/src/IUser.tsapps/meteor/server/methods/saveUserProfile.tsapps/meteor/client/components/UserInfo/UserInfo.tsxee/packages/media-calls/src/server/CastDirector.tsapps/meteor/client/views/account/profile/getProfileInitialValues.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/hooks/useAgentSection.tsxapps/meteor/client/views/account/profile/AccountProfileForm.tsxapps/meteor/client/views/admin/users/AdminUserInfoWithData.tsxapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsxapps/meteor/client/views/admin/users/AdminUserForm.tsxapps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsxapps/meteor/client/components/UserInfo/UserInfo.tsx
📚 Learning: 2026-05-11T23:14:59.316Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40469
File: packages/rest-typings/src/v1/users.ts:337-337
Timestamp: 2026-05-11T23:14:59.316Z
Learning: In Rocket.Chat REST endpoint typings (e.g., packages/rest-typings/src/v1/users.ts and other rest-typings files), keep the established convention of deriving field types from the domain model (e.g., use IUser indexed access like IUser['statusExpiresAt']) rather than swapping individual fields to serialized primitives (like string) in an ad-hoc way. If a truly different “serialized” representation is needed, perform the refactor consistently across the codebase (not just a single endpoint/field) and ensure all related REST typings stay aligned with the shared serialization types.
Applied to files:
packages/rest-typings/src/v1/users/UserCreateParamsPOST.tspackages/rest-typings/src/v1/Ajv.tspackages/rest-typings/src/v1/users/UsersUpdateParamsPOST.tspackages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.ts
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/users.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/users.ts
📚 Learning: 2026-03-15T14:31:25.380Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:25.380Z
Learning: Do not flag this type/schema misalignment in the OpenAPI/migration review for apps/meteor/app/api/server/v1/users.ts. The UserCreateParamsPOST type intentionally uses non-optional fields: fields: string and settings?: IUserSettings without an AJV schema entry, carried over from the original rest-typings (PR `#39647`). Treat this as a known pre-existing divergence and document it as a separate follow-up fix; do not block or mark it as a review issue during the migration.
Applied to files:
apps/meteor/app/api/server/v1/users.ts
📚 Learning: 2026-03-16T23:33:11.443Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: apps/meteor/app/api/server/v1/users.ts:862-869
Timestamp: 2026-03-16T23:33:11.443Z
Learning: In rockets OpenAPI/AJV migration reviews for RocketChat/Rocket.Chat, when reviewing migrations that involve apps/meteor/app/api/server/v1/users.ts, do not require or flag a missing query AJV schema for the fields consumed by parseJsonQuery() (i.e., fields, sort, query) as part of this endpoint's migration PR. The addition of global query-param schemas for parseJsonQuery() usage is a cross-cutting concern and out of scope for individual endpoint migrations. Only flag violations related to the specific scope of the migration, not the absence of a query schema for parseJsonQuery() in this file.
Applied to files:
apps/meteor/app/api/server/v1/users.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsx
📚 Learning: 2026-03-09T18:39:14.020Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:14.020Z
Learning: When implementing batch processing in server methods, favor a single data pass to collect both the items and any derived fields needed for validation. Use the same dataset for both validation and processing to avoid races between validation and execution, and document the approach in code comments. Apply this pattern to similar Meteor Rocket.Chat server methods under apps/meteor/server/methods to prevent race conditions and ensure consistent batch behavior.
Applied to files:
apps/meteor/server/methods/saveUserProfile.ts
🧬 Code graph analysis (11)
apps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/hooks/useAgentSection.tsx (1)
apps/meteor/client/lib/formatPhoneNumber.ts (1)
formatPhoneNumber(24-36)
apps/meteor/client/views/account/profile/AccountProfileForm.tsx (2)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx (2)
PhoneNumberFieldList(24-122)PhoneNumberFieldListProps(16-22)apps/meteor/client/views/account/profile/getProfileInitialValues.ts (1)
AccountProfileFormValues(5-17)
packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts (1)
packages/core-typings/src/IUser.ts (1)
IUserPhoneNumber(155-160)
packages/model-typings/src/models/IUsersModel.ts (1)
packages/core-typings/src/IUser.ts (1)
IUserPhoneNumber(155-160)
packages/rest-typings/src/v1/users/UsersUpdateParamsPOST.ts (1)
packages/core-typings/src/IUser.ts (1)
IUserPhoneNumber(155-160)
packages/rest-typings/src/v1/users/UsersUpdateOwnBasicInfoParamsPOST.ts (1)
packages/core-typings/src/IUser.ts (1)
IUserPhoneNumber(155-160)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx (1)
apps/meteor/client/components/PhoneNumberFieldList/index.ts (1)
PhoneField(2-2)
apps/meteor/client/views/admin/users/AdminUserForm.tsx (1)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx (3)
PhoneNumberFieldList(24-122)PhoneField(9-14)PhoneNumberFieldListProps(16-22)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsx (1)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx (3)
PhoneNumberFieldList(24-122)PhoneField(9-14)PhoneNumberFieldListProps(16-22)
apps/meteor/server/methods/saveUserProfile.ts (1)
packages/core-typings/src/IUser.ts (1)
IUserPhoneNumber(155-160)
apps/meteor/client/components/UserInfo/UserInfo.tsx (1)
apps/meteor/client/lib/formatPhoneNumber.ts (1)
formatPhoneNumber(24-36)
🔇 Additional comments (28)
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsx (2)
34-122: LGTM!
7-7: Phone validation intentionally allows an optional “+”
apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.tsxvalidates phones with/^\+?[1-9]\d{1,14}$/, and the REST API uses the same rule via AJV formatbasic_phone_number(packages/rest-typings/src/v1/Ajv.ts). This means the backend accepts phone numbers without a leading+, so the optional+matches current application requirements (it’s “basic_phone_number”, not strict E.164).apps/meteor/client/components/PhoneNumberFieldList/index.ts (1)
1-2: LGTM!apps/meteor/client/components/PhoneNumberFieldList/PhoneNumberFieldList.spec.tsx (1)
1-209: LGTM!apps/meteor/client/views/account/profile/getProfileInitialValues.ts (1)
1-31: LGTM!apps/meteor/client/views/account/profile/AccountProfileForm.tsx (3)
19-19: LGTM!Also applies to: 24-24, 102-106
108-141: LGTM!
313-326: LGTM!apps/meteor/client/views/admin/users/AdminUserForm.tsx (2)
1-1: LGTM!Also applies to: 34-34, 43-43, 59-59, 89-89, 129-129
534-547: LGTM!ee/packages/media-calls/src/server/CastDirector.ts (1)
7-7: LGTM!Also applies to: 97-100, 154-154
packages/core-typings/src/IUser.ts (1)
155-160: LGTM!Also applies to: 212-216
packages/model-typings/src/models/IUsersModel.ts (1)
4-4: LGTM!Also applies to: 448-448
packages/models/src/models/Users.ts (1)
3067-3070: LGTM!packages/rest-typings/src/v1/Ajv.ts (1)
28-28: LGTM!Also applies to: 34-34
packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts (1)
1-1: LGTM!Also applies to: 23-23, 48-62
packages/rest-typings/src/v1/users/UsersUpdateParamsPOST.ts (1)
1-1: LGTM!Also applies to: 26-26, 111-125
apps/meteor/app/api/server/v1/users.ts (1)
204-204: LGTM!apps/meteor/app/lib/server/functions/getFullUserData.ts (1)
31-31: LGTM!apps/meteor/server/methods/saveUserProfile.ts (1)
2-2: LGTM!Also applies to: 37-38, 122-124, 240-241
apps/meteor/app/lib/server/functions/saveUser/saveUser.ts (1)
57-57: LGTM!Also applies to: 186-188
apps/meteor/app/lib/server/functions/saveUser/saveNewUser.ts (1)
58-60: LGTM!apps/meteor/client/components/UserInfo/UserInfo.tsx (2)
1-1: LGTM!Also applies to: 27-27, 39-39, 68-68, 169-172, 174-184
172-173: ⚡ Quick winUse a collision-safe key for phone rows.
key={p.number}can collide when two phone entries share the samenumber, which may cause incorrect row reconciliation. This only becomes safe if upstream guarantees uniqueness ofp.number(after any normalization/deduping); otherwise use a stable composite key (or preferably a phone-specific id).💡 Proposed fix
- {phones.map((p) => ( - <InfoPanelText key={p.number} display='flex' flexDirection='row' alignItems='center'> + {phones.map((p, index) => ( + <InfoPanelText key={`${p.number}-${p.label ?? ''}-${index}`} display='flex' flexDirection='row' alignItems='center'>apps/meteor/client/views/admin/users/AdminUserInfoWithData.tsx (1)
60-61: LGTM!Also applies to: 75-77, 86-86
apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx (1)
66-67: LGTM!Also applies to: 73-75, 87-87
packages/i18n/src/locales/en.i18n.json (1)
2775-2775: LGTM!Also applies to: 3035-3035, 4165-4166, 4487-4487
packages/i18n/src/locales/pt-BR.i18n.json (1)
2760-2760: LGTM!Also applies to: 3012-3012, 4142-4143, 4465-4465
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/voice-identity-lookup #40680 +/- ##
==============================================================
+ Coverage 69.31% 69.34% +0.03%
==============================================================
Files 4257 4257
Lines 168675 168675
Branches 30072 30077 +5
==============================================================
+ Hits 116912 116967 +55
+ Misses 46595 46525 -70
- Partials 5168 5183 +15
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
May 26, 2026 15:27
31ed886 to
c8ecfc8
Compare
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup
branch
from
May 26, 2026 16:28
d8facc2 to
54a6296
Compare
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
May 26, 2026 16:50
c8ecfc8 to
2a63581
Compare
aleksandernsilva
changed the title
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
May 26, 2026 16:53
2a63581 to
d69914e
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/models/src/models/Users.ts">
<violation number="1" location="packages/models/src/models/Users.ts:2772">
P1: Phone lookup query uses non-standard field path. Can miss real users stored with `phone`. Query both shapes or use the existing field.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| findByPhone<T extends Document = IUser>(phoneNumber: string, options: FindOptions<IUser> = {}): FindCursor<T> { | ||
| return this.find<T>( | ||
| { | ||
| 'phones.number': phoneNumber, |
Contributor
There was a problem hiding this comment.
P1: Phone lookup query uses non-standard field path. Can miss real users stored with phone. Query both shapes or use the existing field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/models/src/models/Users.ts, line 2772:
<comment>Phone lookup query uses non-standard field path. Can miss real users stored with `phone`. Query both shapes or use the existing field.</comment>
<file context>
@@ -2765,6 +2766,15 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
+ findByPhone<T extends Document = IUser>(phoneNumber: string, options: FindOptions<IUser> = {}): FindCursor<T> {
+ return this.find<T>(
+ {
+ 'phones.number': phoneNumber,
+ } as Filter<IUser>,
+ options,
</file context>
ggazzo
pushed a commit
that referenced
this pull request
Jun 2, 2026
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup
branch
from
June 3, 2026 13:42
54a6296 to
b53598d
Compare
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup
branch
from
June 3, 2026 13:58
b53598d to
449757e
Compare
aleksandernsilva
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
June 3, 2026 15:09
d69914e to
d729aa6
Compare
ggazzo
pushed a commit
that referenced
this pull request
Jun 3, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jun 3, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jun 3, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jun 5, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 2, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 3, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 3, 2026
ggazzo
force-pushed
the
feat/voice-identity-lookup
branch
from
July 3, 2026 13:24
d7d1477 to
738c2ab
Compare
ggazzo
pushed a commit
that referenced
this pull request
Jul 3, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 4, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 7, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 7, 2026
ggazzo
pushed a commit
that referenced
this pull request
Jul 7, 2026
ggazzo
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
July 7, 2026 20:19
505df1e to
8c9bbcd
Compare
ggazzo
pushed a commit
that referenced
this pull request
Jul 8, 2026
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
July 10, 2026 03:34
8c9bbcd to
0667150
Compare
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup
branch
from
August 6, 2026 13:57
e331698 to
beb4694
Compare
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
August 6, 2026 17:44
0667150 to
3f475ad
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ee/packages/media-calls/src/server/CastDirector.ts`:
- Around line 106-108: Update the warning payload in findUserByPhone so it no
longer logs the raw phoneNumber; remove that field or replace it with the
repository-approved redacted or hashed representation while preserving the
existing warning and null return behavior.
- Around line 98-100: Update the Users.findByPhone call in CastDirector to apply
a limit of 2 before toArray(), preserving the existing projection and result
handling.
- Around line 165-171: Update the ContactList construction in CastDirector’s
getContactForExtensionNumber flow to set its user field from the matched user
returned by findUserByPhone, while preserving the existing SIP contact. Ensure
requiredType: 'user' resolves the matched contact instead of returning null.
In `@packages/models/src/models/Users.ts`:
- Line 72: Update the user phone handling around findByPhone and its index to
use the persisted scalar phone field instead of phones.number. Change the index,
query path, and shared IUser type consistently to phone, preserving optionality
and existing lookup behavior.
🪄 Autofix
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 Plus
Run ID: 624bc204-1ec6-4cd8-ae69-1435563a6193
📒 Files selected for processing (3)
ee/packages/media-calls/src/server/CastDirector.tspackages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.tsee/packages/media-calls/src/server/CastDirector.ts
🧠 Learnings (4)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
packages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.tsee/packages/media-calls/src/server/CastDirector.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
packages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.tsee/packages/media-calls/src/server/CastDirector.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.tsee/packages/media-calls/src/server/CastDirector.ts
📚 Learning: 2026-07-15T01:31:50.632Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 41382
File: packages/model-typings/src/models/IPushTokenModel.ts:10-10
Timestamp: 2026-07-15T01:31:50.632Z
Learning: In Rocket.Chat’s push-token model, APNs/GCM standard tokens and PushKit VoIP tokens use distinct, globally-unique token string values. As a result, when deduplicating/looking up tokens, key identity solely on `{ tokenValue, appName }` (e.g., for `findOneByTokenAndAppName`, `removeDuplicateTokens`) and do **not** include `tokenType` in the uniqueness criteria to avoid collisions between standard and VoIP tokens.
Applied to files:
packages/model-typings/src/models/IUsersModel.tspackages/models/src/models/Users.ts
🔇 Additional comments (3)
packages/model-typings/src/models/IUsersModel.ts (1)
434-434: LGTM!ee/packages/media-calls/src/server/CastDirector.ts (2)
5-10: LGTM!Also applies to: 92-95
102-104: 📐 Maintainability & Code QualityConfirm the missing-match logging requirement.
These lines return
nullwithout logging. The PR objective says to log missing and multiple matches. If missing matches must be observable, add a privacy-safe log. Otherwise, confirm that only multiple matches should be logged.
Comment on lines
+98
to
+100
| const users = await Users.findByPhone<Pick<IUser, '_id' | 'name' | 'username' | 'freeSwitchExtension'>>(phoneNumber, { | ||
| projection: { name: 1, username: 1, freeSwitchExtension: 1 }, | ||
| }).toArray(); |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate CastDirector.ts =="
fd -a 'CastDirector\.ts$' . || true
echo "== file outline =="
ast-grep outline ee/packages/media-calls/src/server/CastDirector.ts || true
echo "== relevant lines =="
sed -n '1,180p' ee/packages/media-calls/src/server/CastDirector.ts
echo "== Users.findByPhone definitions/usages =="
rg -n "findByPhone|findUserByPhone|phones\.number|limit\(|\.toArray\(\)" ee/packages/media-calls/src -S || trueRepository: RocketChat/Rocket.Chat
Length of output: 7434
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate Users model files =="
fd -a '.*User.*|users' ee packages -t f 2>/dev/null | sed -n '1,120p' || true
echo "== findByPhone definitions/usages across repo (focused) =="
rg -n "findByPhone|phones\.number|createdAt_1|_id|limit\(|toArray\(\)" ee packages -S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' | sed -n '1,240p'
echo "== inspect MediaCallCastDirector methods that use findUserByPhone =="
sed -n '150,175p' ee/packages/media-calls/src/server/CastDirector.ts
echo "== behavioral probe: query chain shape in source text =="
python3 - <<'PY'
from pathlib import Path
p=Path('ee/packages/media-calls/src/server/CastDirector.ts')
s=p.read_text()
start=s.index("private async findUserByPhone")
end=s.index("public async getAgentForActorAndRole")
block=s[start:end]
print("contains findByPhone:", "Users.findByPhone" in block)
print("chain includes limit before toArray:", ".limit(2)" in block or ".limit(1)" in block)
print("returns null if more than one:", "users.length > 1" in block and "return null" in block)
PYRepository: RocketChat/Rocket.Chat
Length of output: 40603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect Users model relevant methods =="
wc -l packages/models/src/models/Users.ts
sed -n '1,260p' packages/models/src/models/Users.ts
echo "== inspect Users model typing relevant fields =="
wc -l packages/model-typings/src/models/IUsersModel.ts
sed -n '1,220p' packages/model-typings/src/models/IUsersModel.ts
echo "== search for phone index definitions/usages =="
rg -n "phones:|phones\.number|phones|phonesIndex|index" ee packages -S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' | sed -n '1,260p'Repository: RocketChat/Rocket.Chat
Length of output: 42065
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact findByPhone occurrences with context =="
rg -n -C 8 "findByPhone|findByPhoneNumber|find.*Phone|Phones|phones" ee packages models --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' -S || true
echo "== Users.ts methods around phone-related names =="
rg -n "^\s*(find|findOne).*[Pp]hone|^\s*.*find.*Phone" packages/models/src/models/Users.ts packages/model-typings/src/models/IUsersModel.ts -S || trueRepository: RocketChat/Rocket.Chat
Length of output: 50378
Limit the phone lookup to two results.
Users.findByPhone() is non-unique because the phones.number index is not unique. This lookup only needs zero, one, or multiple results, but .toArray() fetches every matching user before deciding. Add .limit(2) before .toArray().
Proposed change
- }).toArray();
+ }).limit(2).toArray();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const users = await Users.findByPhone<Pick<IUser, '_id' | 'name' | 'username' | 'freeSwitchExtension'>>(phoneNumber, { | |
| projection: { name: 1, username: 1, freeSwitchExtension: 1 }, | |
| }).toArray(); | |
| const users = await Users.findByPhone<Pick<IUser, '_id' | 'name' | 'username' | 'freeSwitchExtension'>>(phoneNumber, { | |
| projection: { name: 1, username: 1, freeSwitchExtension: 1 }, | |
| }).limit(2).toArray(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ee/packages/media-calls/src/server/CastDirector.ts` around lines 98 - 100,
Update the Users.findByPhone call in CastDirector to apply a limit of 2 before
toArray(), preserving the existing projection and result handling.
Comment on lines
+106
to
+108
| if (users.length > 1) { | ||
| logger.warn({ msg: 'Multiple users found for phone number, identity cannot be resolved', phoneNumber }); | ||
| return null; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate CastDirector.ts and logger/redaction helpers"
fd -a 'CastDirector\.ts$' . || true
echo "--- logger/redaction search ---"
rg -n "redact|hash\\(|phoneNumber|personal |PII|logger\\.(warn|error|info)" --glob '!node_modules' --glob '!dist' --glob '!build' | head -200
echo "--- target file outline ---"
target="$(fd 'CastDirector\.ts$' . | head -1 || true)"
if [ -n "${target:-}" ]; then
wc -l "$target"
ast-grep outline "$target" --view expanded || true
echo "--- target relevant lines ---"
sed -n '1,180p' "$target" | cat -n
fiRepository: RocketChat/Rocket.Chat
Length of output: 398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral check: parse JS/TS logger calls and print whether a logger method call includes a spread/string/variable,
# and whether any nearby identifier could represent redaction/hashing.
python3 - <<'PY'
from pathlib import Path
import re
target = next(Path('.').rglob('ee/packages/media-calls/src/server/CastDirector.ts'), None)
if not target:
print("TARGET_NOT_FOUND")
raise SystemExit
text = target.read_text()
for i, line in enumerate(text.splitlines(), 1):
if re.search(r'\blogger\.\w+\s*\(', line):
print(f"LINE{i}: {line.strip()}")
PYRepository: RocketChat/Rocket.Chat
Length of output: 334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target="$(fd 'CastDirector\.ts$' . | grep '/ee/packages/media-calls/src/server/CastDirector.ts$' | head -1 || true)"
if [ -z "${target:-}" ]; then
echo "CastDirector.ts not found"
exit 1
fi
echo "--- target file outline ---"
ast-grep outline "$target" --view expanded || true
echo "--- relevant lines 1-180 ---"
sed -n '1,180p' "$target" | cat -n
echo "--- repository-wide logger/redaction calls/phrases (compact) ---"
rg -n "(redact|hash\\(|phoneNumber|personal data|PII|logger\\.(warn|error|info|debug))" --glob '!node_modules' --glob '!dist' --glob '!build' | head -300 || trueRepository: RocketChat/Rocket.Chat
Length of output: 8776
Do not log the raw phone number.
findUserByPhone passes phoneNumber into logger.warn, so the server log captures personal data. Remove the field from the warning payload or replace it with the repository’s approved redacted/hashed value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ee/packages/media-calls/src/server/CastDirector.ts` around lines 106 - 108,
Update the warning payload in findUserByPhone so it no longer logs the raw
phoneNumber; remove that field or replace it with the repository-approved
redacted or hashed representation while preserving the existing warning and null
return behavior.
Comment on lines
+165
to
+171
| const user = await this.findUserByPhone(sipExtension); | ||
|
|
||
| const data: Partial<MediaCallContact> = { | ||
| ...defaultContactInfo, | ||
| ...(sipExtension && { sipExtension }), | ||
| ...(user?.username && { username: user.username }), | ||
| ...(user?.name && { displayName: user.name }), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return the matched user contact for requiredType: 'user'.
Line 165 obtains a unique user, but the returned ContactList still sets user: null. ee/packages/media-calls/src/sip/providers/IncomingSipCall.ts calls getContactForExtensionNumber with { requiredType: 'user' }. getContactFromList therefore returns null, and the invite fails with SipErrorCodes.TEMPORARILY_UNAVAILABLE even for a unique phone match.
Populate the user contact from the matched user while retaining the original SIP contact.
Proposed change
const data: Partial<MediaCallContact> = {
...defaultContactInfo,
...(sipExtension && { sipExtension }),
...(user?.username && { username: user.username }),
...(user?.name && { displayName: user.name }),
};
+ const userContact = user ? this.buildContactListForUser(user, defaultContactInfo).user : null;
return {
- user: null,
+ user: userContact,
sip: {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const user = await this.findUserByPhone(sipExtension); | |
| const data: Partial<MediaCallContact> = { | |
| ...defaultContactInfo, | |
| ...(sipExtension && { sipExtension }), | |
| ...(user?.username && { username: user.username }), | |
| ...(user?.name && { displayName: user.name }), | |
| const user = await this.findUserByPhone(sipExtension); | |
| const data: Partial<MediaCallContact> = { | |
| ...defaultContactInfo, | |
| ...(sipExtension && { sipExtension }), | |
| ...(user?.username && { username: user.username }), | |
| ...(user?.name && { displayName: user.name }), | |
| }; | |
| const userContact = user ? this.buildContactListForUser(user, defaultContactInfo).user : null; | |
| return { | |
| user: userContact, | |
| sip: { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ee/packages/media-calls/src/server/CastDirector.ts` around lines 165 - 171,
Update the ContactList construction in CastDirector’s
getContactForExtensionNumber flow to set its user field from the matched user
returned by findUserByPhone, while preserving the existing SIP contact. Ensure
requiredType: 'user' resolves the matched contact instead of returning null.
| { key: { openBusinessHours: 1 }, sparse: true }, | ||
| { key: { statusLivechat: 1 }, sparse: true }, | ||
| { key: { freeSwitchExtension: 1 }, sparse: true, unique: true }, | ||
| { key: { 'phones.number': 1 }, sparse: true }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 --glob '*.ts' --glob '*.tsx' --glob '*.js' '\bphones\b|\bphone\b' .Repository: RocketChat/Rocket.Chat
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files of interest:\n'
git ls-files 'packages/models/src/models/Users.ts' 'packages/core-typings/src/IUser.ts' | sed -n '1,80p'
printf '\nUsers.ts index and findByPhone context:\n'
sed -n '60,85p' packages/models/src/models/Users.ts
sed -n '2520,2548p' packages/models/src/models/Users.ts
printf '\nIUser.ts:\n'
sed -n '1,140p' packages/core-typings/src/IUser.ts
printf '\nExact Users.ts/phone schemas and writes:\n'
rg -n -C 3 --glob '*.ts' --glob '*.tsx' '\bphones\.number\b|schema:\s*\{[^{}]*phone\b|phone\b.*\?(\{|\[[^]*\]?|}\)|phone:?\b|schemas/Users|Users' packages/server packages/applications packages/core services packages/models | sed -n '1,220p'Repository: RocketChat/Rocket.Chat
Length of output: 5375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Git diff summary:\n'
git diff --stat || true
printf '\nRelevant tracked files for Users/phone schema:\n'
git ls-files | rg '(^|/)Users\.ts$|IUser\.ts$|Phone|phone|Users\.schema|users' | sed -n '1,240p'
printf '\nFocused schema/phone searches:\n'
rg -n --glob '*.ts' --glob '*.tsx' 'phones\.number|phones\s*:|phone\s*(\?|[:=])|Schema.*Users|users.*Schema|phone.*number|number.*phone' packages/server packages/models packages/settings packages/applications packages/core-typings packages/rest-typings 2>/dev/null | sed -n '1,260p'
printf '\nFindAll/phone writer candidates:\n'
rg -n --glob '*.ts' --glob '*.tsx' 'set\s*:\s*\{[^{}]*phone|\$set[^}]*phone|phone:|phoneNumber|createUser|register|set password|password' packages/services packages/applications packages/server | sed -n '1,260p'Repository: RocketChat/Rocket.Chat
Length of output: 16281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Read relevant Users schema excerpt:\n'
sed -n '50,95p' packages/models/src/models/Users.ts
printf '\nRead core typings phone section:\n'
sed -n '200,225p' packages/core-typings/src/IUser.ts
printf '\nRead Users phone projection excerpt:\n'
sed -n '1958,1972p' packages/models/src/models/Users.ts
printf '\nRead REST user phone types:\n'
rg -n --glob '*.ts' 'phone.*string|phones.*string\[\]|phone?:|phone:' packages/rest-typings/src/v1/users packages/apps-engine/src/definition/users packages/core-typings/src/IUser.ts | sed -n '1,160p'
printf '\nCheck current index/query exact strings:\n'
python3 - <<'PY'
from pathlib import Path
users = Path('packages/models/src/models/Users.ts')
text = users.read_text()
for needle in ["'phones.number'", "\"phones.number\"", "findByPhone"]:
print(needle, text.find(needle))
PYRepository: RocketChat/Rocket.Chat
Length of output: 3341
Align findByPhone with the persisted user phone field.
findByPhone and the new index use phones.number, but user documents and IUser store a scalar phone?: string. This sparse index will not support the query path. Use the actual persisted field for the index, query, and shared type.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/models/src/models/Users.ts` at line 72, Update the user phone
handling around findByPhone and its index to use the persisted scalar phone
field instead of phones.number. Change the index, query path, and shared IUser
type consistently to phone, preserving optionality and existing lookup behavior.
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup
branch
2 times, most recently
from
August 18, 2026 17:32
1557faa to
c018d07
Compare
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
August 18, 2026 20:54
3f475ad to
3c6b054
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
pierre-lehnen-rc
added a commit
that referenced
this pull request
Aug 19, 2026
pierre-lehnen-rc
added a commit
that referenced
this pull request
Aug 19, 2026
pierre-lehnen-rc
added a commit
that referenced
this pull request
Aug 19, 2026
pierre-lehnen-rc
added a commit
that referenced
this pull request
Aug 19, 2026
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup
branch
from
August 20, 2026 19:44
c018d07 to
9b5baed
Compare
pierre-lehnen-rc
force-pushed
the
feat/voice-identity-lookup-user-phones
branch
from
August 20, 2026 19:45
3c6b054 to
66c99cc
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
pierre-lehnen-rc
added a commit
that referenced
this pull request
Aug 20, 2026
pierre-lehnen-rc
added a commit
that referenced
this pull request
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
This pull request introduces enhancements to user lookup functionality for media calls, specifically enabling the system to find users by their phone numbers. This allows for more accurate identification of users when only a SIP extension (phone number) is available, and improves the handling of ambiguous or missing user data. The changes also add a new database index to optimize these queries.
User lookup improvements:
findByPhoneto theUsersmodel and its interface, allowing efficient searching for users by phone number (phones.number).phones.numberfield in theUserscollection to improve query performance for phone-based lookups.Media call contact resolution enhancements:
MediaCallCastDirectorclass to asynchronously build contact lists for SIP extensions by looking up users via phone number, and to enrich contact information with username and display name when available.findUserByPhoneinMediaCallCastDirectorto handle user lookup by phone number, including logging and handling cases where multiple users are found for the same number.Code organization:
CastDirector.tsfor consistency.Issue(s)
DMV-2
Steps to test or reproduce
Further comments
Summary by CodeRabbit