refactor: Decouple useDirectMessageAction from RoomContext and expose base hook from ui-client#38165
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughReplaces local direct-message navigation logic with a new exported hook Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Component as UI Component
participant Hook as useGoToDirectMessage
participant Sub as Subscription Store
participant Perm as Permission Service
participant Router
User->>Component: clicks "Direct Message"
Component->>Hook: request goToDirectMessage(username, openRoomId)
Hook->>Sub: check subscription for username
Hook->>Perm: check create-direct permission
alt cannot navigate (no access or room already open)
Hook-->>Component: returns undefined (no action)
else can navigate
Component->>Hook: invoke returned function
Hook->>Router: navigate('direct', { rid: username })
Router-->>Component: route updated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-18T15:18:31.688ZApplied to files:
📚 Learning: 2025-11-19T18:20:37.116ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts (2)
⏰ 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). (3)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/ui-client/src/hooks/useGoToDirectMessage.spec.ts (1)
7-41: Test coverage looks good for core scenarios.The tests cover the main conditional branches well: no permission/subscription, already open room, permission-only, and subscription-only paths.
Consider adding edge case coverage:
- When
targetUser.usernameisundefinedor empty string- Verifying the returned function actually calls
router.navigatewith correct params (mocking the router)packages/ui-client/src/hooks/useGoToDirectMessage.ts (1)
20-27: Potential unnecessary hook call when username is undefined or empty.When
targetUser.usernameisundefinedor empty,useUserSubscriptionByName('')still executes, potentially triggering an unnecessary subscription lookup.Consider early-returning or memoizing based on username presence:
♻️ Optional optimization
export const useGoToDirectMessage = (targetUser: { username?: string }, openRoomId?: string): (() => void) | undefined => { + const username = targetUser.username; - const usernameSubscription = useUserSubscriptionByName(targetUser.username ?? ''); + const usernameSubscription = useUserSubscriptionByName(username ?? ''); const router = useRouter(); const canOpenDirectMessage = usePermission('create-d'); - const hasPermissionOrSubscription = usernameSubscription || canOpenDirectMessage; + const hasPermissionOrSubscription = username && (usernameSubscription || canOpenDirectMessage); const alreadyOpen = openRoomId && usernameSubscription?.rid === openRoomId; const shouldOpen = hasPermissionOrSubscription && !alreadyOpen;apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts (1)
7-7: Remove commented-out import.Dead code should be removed rather than left commented.
🧹 Remove dead code
-// import { useDirectMessageAction } from '../room/hooks/useUserInfoActions/actions/useDirectMessageAction';
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.tsapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useDirectMessageAction.tspackages/ui-client/src/hooks/index.tspackages/ui-client/src/hooks/useGoToDirectMessage.spec.tspackages/ui-client/src/hooks/useGoToDirectMessage.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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-client/src/hooks/useGoToDirectMessage.tspackages/ui-client/src/hooks/useGoToDirectMessage.spec.tspackages/ui-client/src/hooks/index.tsapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useDirectMessageAction.tsapps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
packages/ui-client/src/hooks/useGoToDirectMessage.spec.ts
🧠 Learnings (4)
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
packages/ui-client/src/hooks/useGoToDirectMessage.spec.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts
📚 Learning: 2025-12-18T15:18:31.688Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:31.688Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, for internal call history items, the item.contactId is guaranteed to always match either the caller.id or callee.id in the call data, so the contact resolution in getContact will never result in undefined.
Applied to files:
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts
🧬 Code graph analysis (2)
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useDirectMessageAction.ts (2)
apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts (1)
UserInfoAction(45-45)packages/ui-client/src/hooks/useGoToDirectMessage.ts (1)
useGoToDirectMessage(20-39)
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts (2)
packages/ui-client/src/hooks/useGoToDirectMessage.ts (1)
useGoToDirectMessage(20-39)packages/media-signaling/src/lib/Call.ts (1)
contact(81-83)
⏰ 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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (7)
packages/ui-client/src/hooks/useGoToDirectMessage.ts (2)
29-38: Clean implementation with proper guards.The use of
useEffectEventprovides a stable callback reference, and the guardtargetUser.username &&before navigation prevents errors when username is missing.
4-12: Module augmentation is documented and necessary.The TODO comment appropriately explains why this inline type declaration is needed. The pathname template and pattern are consistent.
packages/ui-client/src/hooks/index.ts (1)
12-12: Export added correctly.The new hook is properly re-exported from the barrel file, extending the public API surface as intended.
apps/meteor/client/views/mediaCallHistory/useMediaCallInternalHistoryActions.ts (2)
52-52: Hook usage is correct.The hook is called with the appropriate parameters matching the expected signature.
76-84: Return structure is well-organized.The memoized return object properly includes
goToDirectMessageand conditionally includes other actions based on availability.apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useDirectMessageAction.ts (2)
8-26: Clean refactor that properly delegates to the new hook.The refactored implementation correctly:
- Delegates DM navigation logic to
useGoToDirectMessage- Preserves the return type
UserInfoAction | undefined- Uses proper memoization with correct dependencies
13-24: Memoization is appropriate.The
useMemoprevents creating new object references on each render, which is important since this action object is likely used in dependency arrays of consuming components.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38165 +/- ##
===========================================
- Coverage 70.66% 70.64% -0.03%
===========================================
Files 3135 3136 +1
Lines 108592 108592
Branches 19555 19528 -27
===========================================
- Hits 76740 76718 -22
- Misses 29845 29871 +26
+ Partials 2007 2003 -4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "01/17 15:49 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Sat, 17 Jan 2026 15:49:44 GMT |
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
2 participants
Proposed changes (including videos or screenshots)
Refactored the
useDirectMessageActionby extracting a base (generic) hook an exposing it from theui-clientpackage so that it can be used in other places.Issue(s)
VGA-137
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.