refactor: rendering of call history contact information#40773
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
Contributor
WalkthroughCentralizes call-history contact types into a shared definitions module, adds a CallHistoryUser router component, and updates renderers, pages, table rows, contextual bar, and a hook to use the unified contact API. ChangesCall History Contact Unification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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. Comment |
|
pierre-lehnen-rc
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40773 +/- ##
===========================================
- Coverage 69.75% 69.74% -0.01%
===========================================
Files 3327 3330 +3
Lines 123135 123157 +22
Branches 21991 21925 -66
===========================================
+ Hits 85887 85900 +13
+ Misses 33895 33891 -4
- Partials 3353 3366 +13
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
pierre-lehnen-rc
force-pushed
the
chore/call-history-contact
branch
from
297a5aa to
c753f42
Compare
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsx (1)
70-80:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPass through
contact.voiceCallExtensionto preserve the widget’scallerIddisplay
useMediaCallInternalHistoryActionsforwardscontact.voiceCallExtensionascallerId, butCallHistoryRowInternalUserbuilds a narrowedcontactwithoutvoiceCallExtension, so the widget’scallerIdUI will render empty (no compile error becausevoiceCallExtensionis optional).const actions = useMediaCallInternalHistoryActions({ contact: { _id: contact._id, username: contact.username ?? '', name: contact.name, displayName: contact.name || contact.username, }, messageId, messageRoomId: rid, openUserInfo: onClickUserInfo ? (userId) => onClickUserInfo(userId, rid) : undefined, });🤖 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/views/mediaCallHistory/CallHistoryRowInternalUser.tsx` around lines 70 - 80, CallHistoryRowInternalUser is creating a narrowed contact object missing voiceCallExtension so useMediaCallInternalHistoryActions can't pass a callerId; update the object passed to useMediaCallInternalHistoryActions to include contact.voiceCallExtension (preserve it on the contact you build) so the widget's callerId display is populated (refer to useMediaCallInternalHistoryActions, CallHistoryRowInternalUser, and the contact.voiceCallExtension property).
🧹 Nitpick comments (1)
packages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsx (1)
17-23: ⚡ Quick winThe conditional type for
contactis redundant and forces an unsafe cast downstream.Each branch resolves
Tto the exact type it just tested against (T extends CallHistoryInternalContact ? CallHistoryInternalContact : ...), so the whole conditional is equivalent tocontact: T. The added complexity also forces consumers to cast (seeMediaCallHistoryTable.stories.tsxLine 85,as CallHistoryInternalContact). Collapsing it tocontact: Twith a default keeps the same behavior and lets callers use the fullCallHistoryContactunion without a cast.♻️ Proposed simplification
-import type { - CallHistoryContact, - CallHistoryExternalContact, - CallHistoryInternalContact, - CallHistoryUnknownContact, -} from '../../definitions'; +import type { CallHistoryContact } from '../../definitions'; -export type CallHistoryTableRowProps<T extends CallHistoryContact> = { +export type CallHistoryTableRowProps<T extends CallHistoryContact = CallHistoryContact> = { _id: string; - contact: T extends CallHistoryInternalContact - ? CallHistoryInternalContact - : T extends CallHistoryExternalContact - ? CallHistoryExternalContact - : CallHistoryUnknownContact; + contact: T; type: 'outbound' | 'inbound';The story can then drop the cast and use
CallHistoryTableRowProps<CallHistoryContact>(or the bareCallHistoryTableRowProps).🤖 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/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsx` around lines 17 - 23, The generic conditional type on CallHistoryTableRowProps (contact: T extends CallHistoryInternalContact ? CallHistoryInternalContact : T extends CallHistoryExternalContact ? CallHistoryExternalContact : CallHistoryUnknownContact) is redundant and forces unsafe casts; simplify by making contact: T (or contact?: T = CallHistoryContact as the default generic) so the prop preserves the original union and callers can pass CallHistoryContact without casting—update the CallHistoryTableRowProps generic signature accordingly and remove downstream casts (e.g., in MediaCallHistoryTable.stories.tsx).
🤖 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 `@packages/ui-voip/src/definitions/callHistoryContacts.ts`:
- Around line 28-30: Replace the negative fallback logic in
isCallHistoryExternalContact with an explicit discriminant check: instead of
returning !isCallHistoryUnknownContact(contact) &&
!isCallHistoryInternalContact(contact), test for the presence and type of the
external contact property (e.g., 'number' in contact && typeof contact.number
=== "string") so malformed objects lacking number won't be misclassified; keep
references to the other guards (isCallHistoryUnknownContact,
isCallHistoryInternalContact) only if still needed for additional validation.
- Around line 24-26: The type guard isCallHistoryInternalContact mixes type
discrimination with value validation by checking Boolean(contact._id); remove
the value check and only discriminate by the presence of the property. Update
isCallHistoryInternalContact to return "'_id' in contact" (no Boolean coercion)
so it solely identifies a CallHistoryInternalContact by the _id property and
does not reject valid but falsy IDs; refer to the isCallHistoryInternalContact
function and the CallHistoryContact / CallHistoryInternalContact types when
making the change.
---
Outside diff comments:
In `@apps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsx`:
- Around line 70-80: CallHistoryRowInternalUser is creating a narrowed contact
object missing voiceCallExtension so useMediaCallInternalHistoryActions can't
pass a callerId; update the object passed to useMediaCallInternalHistoryActions
to include contact.voiceCallExtension (preserve it on the contact you build) so
the widget's callerId display is populated (refer to
useMediaCallInternalHistoryActions, CallHistoryRowInternalUser, and the
contact.voiceCallExtension property).
---
Nitpick comments:
In `@packages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsx`:
- Around line 17-23: The generic conditional type on CallHistoryTableRowProps
(contact: T extends CallHistoryInternalContact ? CallHistoryInternalContact : T
extends CallHistoryExternalContact ? CallHistoryExternalContact :
CallHistoryUnknownContact) is redundant and forces unsafe casts; simplify by
making contact: T (or contact?: T = CallHistoryContact as the default generic)
so the prop preserves the original union and callers can pass CallHistoryContact
without casting—update the CallHistoryTableRowProps generic signature
accordingly and remove downstream casts (e.g., in
MediaCallHistoryTable.stories.tsx).
🪄 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: 04a2c045-76e6-482c-b4f2-ff95d2cb1a9e
⛔ Files ignored due to path filters (1)
packages/ui-voip/src/views/CallHistoryContextualbar/__snapshots__/CallHistoryContextualbar.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
apps/meteor/client/views/mediaCallHistory/CallHistoryPage.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryRowExternalUser.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryExternal.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsxapps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.tspackages/ui-voip/src/components/CallHistoryExternalUser.tsxpackages/ui-voip/src/components/CallHistoryInternalUser.tsxpackages/ui-voip/src/components/CallHistoryUnknownUser.tsxpackages/ui-voip/src/components/CallHistoryUser.tsxpackages/ui-voip/src/definitions/callHistoryContacts.tspackages/ui-voip/src/definitions/index.tspackages/ui-voip/src/index.tspackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/index.tspackages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
🧰 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/ui-voip/src/components/CallHistoryUnknownUser.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/index.tspackages/ui-voip/src/index.tspackages/ui-voip/src/definitions/index.tspackages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsxpackages/ui-voip/src/components/CallHistoryExternalUser.tsxpackages/ui-voip/src/components/CallHistoryInternalUser.tsxpackages/ui-voip/src/definitions/callHistoryContacts.tsapps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsxpackages/ui-voip/src/components/CallHistoryUser.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryRowExternalUser.tsxapps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.tspackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryExternal.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPage.tsx
🧠 Learnings (9)
📚 Learning: 2026-02-26T19:22:29.385Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryActions.tsx:40-40
Timestamp: 2026-02-26T19:22:29.385Z
Learning: For TSX files in the UI VOIP package, ensure that when a media session state is 'unavailable', the voiceCall action is excluded from the actions object passed to CallHistoryActions so it does not render in the menu. This filtering should occur upstream (before getItems is called) to avoid tooltips or UI hints for unavailable actions. If there are multiple actions with availability states, implement a centralized helper to filter actions based on session state.
Applied to files:
packages/ui-voip/src/components/CallHistoryUnknownUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsxpackages/ui-voip/src/components/CallHistoryExternalUser.tsxpackages/ui-voip/src/components/CallHistoryInternalUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsxpackages/ui-voip/src/components/CallHistoryUser.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx
📚 Learning: 2026-05-05T12:34:29.042Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 40331
File: packages/ui-voip/src/views/MediaCallWidget/OngoingCallWithScreen.tsx:69-69
Timestamp: 2026-05-05T12:34:29.042Z
Learning: In Rocket.Chat’s `packages/ui-voip` UI (e.g., media/call widgets), voice/media calls are only supported in Direct Message (DM) rooms. Rocket.Chat models a DM as a “room” with exactly two participants, so handlers like `onClickDirectMessage` are the correct destination—even when the UI text/element says “Open in room” (e.g., on the shared screen card/`StreamCard`). During review, don’t flag a “DM vs room” mismatch for these cases; they intentionally map to the same destination.
Applied to files:
packages/ui-voip/src/components/CallHistoryUnknownUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsxpackages/ui-voip/src/components/CallHistoryExternalUser.tsxpackages/ui-voip/src/components/CallHistoryInternalUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsxpackages/ui-voip/src/components/CallHistoryUser.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx
📚 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:
packages/ui-voip/src/components/CallHistoryUnknownUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsxpackages/ui-voip/src/components/CallHistoryExternalUser.tsxpackages/ui-voip/src/components/CallHistoryInternalUser.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsxpackages/ui-voip/src/components/CallHistoryUser.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryRowExternalUser.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryExternal.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPage.tsx
📚 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/ui-voip/src/components/CallHistoryUnknownUser.tsxpackages/ui-voip/src/views/CallHistoryContextualbar/index.tspackages/ui-voip/src/index.tspackages/ui-voip/src/definitions/index.tspackages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsxpackages/ui-voip/src/components/CallHistoryExternalUser.tsxpackages/ui-voip/src/components/CallHistoryInternalUser.tsxpackages/ui-voip/src/definitions/callHistoryContacts.tsapps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsxpackages/ui-voip/src/views/MediaCallHistoryTable/CallHistoryTableRow.tsxpackages/ui-voip/src/components/CallHistoryUser.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryRowExternalUser.tsxapps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.tspackages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsxapps/meteor/client/views/mediaCallHistory/MediaCallHistoryExternal.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPage.tsx
📚 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/ui-voip/src/views/CallHistoryContextualbar/index.tspackages/ui-voip/src/index.tspackages/ui-voip/src/definitions/index.tspackages/ui-voip/src/definitions/callHistoryContacts.tsapps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.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/ui-voip/src/views/CallHistoryContextualbar/index.tspackages/ui-voip/src/index.tspackages/ui-voip/src/definitions/index.tspackages/ui-voip/src/definitions/callHistoryContacts.tsapps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts
📚 Learning: 2025-12-18T15:18:23.819Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:23.819Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, the claim is that internal call history items always have contactId equal to either caller.id or callee.id, so getContact will not produce undefined. Treat this as a file-specific guarantee and avoid adding guards that rely on other data sources in this path. If this invariant holds, you can rely on contact resolution not returning undefined, but consider adding a lightweight runtime assertion or a failing guard when the invariant is violated (e.g., throw or log) to surface unexpected data instead of silently handling undefined. This guideline applies only to this file path and should not be assumed project-wide without verification.
Applied to files:
apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx
📚 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/views/mediaCallHistory/useMediaCallInternalHistoryActions.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/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts
🔇 Additional comments (18)
packages/ui-voip/src/views/MediaCallHistoryTable/MediaCallHistoryTable.stories.tsx (1)
82-93: LGTM!apps/meteor/client/views/mediaCallHistory/CallHistoryPage.tsx (2)
48-58: LGTM!
145-225: LGTM!packages/ui-voip/src/definitions/callHistoryContacts.ts (2)
1-18: LGTM!
20-22: LGTM!packages/ui-voip/src/definitions/index.ts (1)
1-3: LGTM!packages/ui-voip/src/index.ts (1)
15-16: LGTM!packages/ui-voip/src/components/CallHistoryExternalUser.tsx (1)
1-27: LGTM!packages/ui-voip/src/components/CallHistoryInternalUser.tsx (1)
1-37: LGTM!packages/ui-voip/src/components/CallHistoryUnknownUser.tsx (1)
1-17: LGTM!packages/ui-voip/src/components/CallHistoryUser.tsx (1)
1-23: LGTM!apps/meteor/client/views/mediaCallHistory/MediaCallHistoryExternal.tsx (1)
1-62: LGTM!apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx (1)
1-64: LGTM!packages/ui-voip/src/views/CallHistoryContextualbar/CallHistoryContextualbar.tsx (1)
20-22: LGTM!Also applies to: 37-37, 65-65, 85-85, 95-95
packages/ui-voip/src/views/CallHistoryContextualbar/index.ts (1)
2-2: LGTM!apps/meteor/client/views/mediaCallHistory/CallHistoryRowExternalUser.tsx (1)
2-2: LGTM!Also applies to: 7-7
apps/meteor/client/views/mediaCallHistory/CallHistoryRowInternalUser.tsx (1)
4-4: LGTM!Also applies to: 11-11
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts (1)
4-4: LGTM!Also applies to: 8-8
![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 18 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx (1)
24-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast instead of manufacturing an invalid internal contact.
contact ?? {}turns an invariant break into a malformedCallHistoryInternalContact:_idcan beundefined, but the function still returns a value typed as a valid internal contact. That makes the failure much harder to diagnose and can break downstream actions that assume a real user id. Please keep the stronger return type and throw or log whenitem.contactIdmatches neithercallernorcallee.Suggested fix
const getContact = (item: InternalCallEndpointData['item'], call: InternalCallEndpointData['call']): CallHistoryInternalContact => { const { caller, callee } = call ?? {}; const contact = caller?.id === item.contactId ? caller : callee; - const { id, sipExtension, username, displayName, ...rest } = contact ?? {}; + if (!contact) { + throw new Error(`Unable to resolve internal call contact for contactId "${item.contactId}"`); + } + + const { id, sipExtension, username, displayName, ...rest } = contact; return { ...rest, _id: id, username: username ?? '', name: displayName, displayName, voiceCallExtension: sipExtension, }; };Based on learnings:
item.contactIdis guaranteed to match eithercaller.idorcallee.idin this file, so unexpected misses should be surfaced rather than silently handled.🤖 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/views/mediaCallHistory/MediaCallHistoryInternal.tsx` around lines 24 - 35, The getContact function currently masks a missing contact by using contact ?? {} which can produce a CallHistoryInternalContact with undefined _id; instead, detect when caller?.id and callee?.id do not match item.contactId and immediately surface the error (throw or processLogger.error + throw) so callers never receive an invalid contact. In getContact, locate the contact resolution logic (caller, callee, contact) and replace the fallback with an explicit check: if no contact found, log/throw a clear error referencing item.contactId and the call object, otherwise destructure contact and return the well-typed CallHistoryInternalContact as before. Ensure the thrown message includes item.contactId and call identifiers for debugging.
🤖 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.
Outside diff comments:
In `@apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx`:
- Around line 24-35: The getContact function currently masks a missing contact
by using contact ?? {} which can produce a CallHistoryInternalContact with
undefined _id; instead, detect when caller?.id and callee?.id do not match
item.contactId and immediately surface the error (throw or processLogger.error +
throw) so callers never receive an invalid contact. In getContact, locate the
contact resolution logic (caller, callee, contact) and replace the fallback with
an explicit check: if no contact found, log/throw a clear error referencing
item.contactId and the call object, otherwise destructure contact and return the
well-typed CallHistoryInternalContact as before. Ensure the thrown message
includes item.contactId and call identifiers for debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b7da18e-28e4-4e6d-96a1-3f50505d5ad2
📒 Files selected for processing (1)
apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
🧰 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/views/mediaCallHistory/MediaCallHistoryInternal.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-18T15:18:23.819Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:23.819Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, the claim is that internal call history items always have contactId equal to either caller.id or callee.id, so getContact will not produce undefined. Treat this as a file-specific guarantee and avoid adding guards that rely on other data sources in this path. If this invariant holds, you can rely on contact resolution not returning undefined, but consider adding a lightweight runtime assertion or a failing guard when the invariant is violated (e.g., throw or log) to surface unexpected data instead of silently handling undefined. This guideline applies only to this file path and should not be assumed project-wide without verification.
Applied to files:
apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx
📚 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/mediaCallHistory/MediaCallHistoryInternal.tsx
📚 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/views/mediaCallHistory/MediaCallHistoryInternal.tsx
ggazzo
approved these changes
Jun 3, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
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)
Reorganizes how the call history views handle rendering of contact information
Issue(s)
Derived from DMV-7
Steps to test or reproduce
Further comments
Summary by CodeRabbit