refactor: Get custom fields from useCustomFieldsQuery
#37674
Conversation
|
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 |
|
WalkthroughCentralizes omnichannel livechat custom-fields fetching and query keys: introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
📦 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/15 22:28", "11/16 01:28", "11/17 23:50", "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 19:10", "12/03 20:15 (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]
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]
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]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
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]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-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]
Statistics (last 12 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 03 Dec 2025 20:15:12 GMT |
dougfabris
force-pushed
the
refactor/unify-customFields-query
branch
from
0a47bbc to
0f22d21
Compare
dougfabris
force-pushed
the
refactor/unify-customFields-query
branch
from
4d99f18 to
2ac5ec5
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37674 +/- ##
===========================================
- Coverage 68.78% 68.78% -0.01%
===========================================
Files 3363 3362 -1
Lines 114202 114188 -14
Branches 20617 20615 -2
===========================================
- Hits 78556 78546 -10
+ Misses 33551 33545 -6
- Partials 2095 2097 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx (1)
6-6: Centralizing custom-field visibility via useValidCustomFields looks good; confirm data shape and consider string coercionWiring ChatInfo to
useValidCustomFields(livechatData)and renderingcustomFieldEntriesthrough<CustomField>aligns this panel with the existing contact-info behavior and removes duplicated fetch/visibility logic, which is a solid refactor.Two points to double‑check:
useValidCustomFieldscurrently expects the “user custom fields” shape and applies visitor‑scope +view-livechat-room-customfieldspermission checks. Please confirm thatlivechatDatacarries the same semantics you intend to display here (e.g., that you’re not dropping room‑scope-only fields that were previously shown).- At render time you do
value as string; iflivechatDatacan contain non‑string values, consider normalizing withString(value)to avoid passing non‑string values intoCustomFieldat runtime while keeping types honest.Also applies to: 17-17, 66-66, 164-165
📜 Review details
Configuration used: CodeRabbit 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 (13)
apps/meteor/client/lib/queryKeys.ts(1 hunks)apps/meteor/client/views/omnichannel/contactInfo/hooks/useCustomFieldsQuery.ts(0 hunks)apps/meteor/client/views/omnichannel/contactInfo/hooks/useValidCustomFields.ts(1 hunks)apps/meteor/client/views/omnichannel/currentChats/CurrentChatsPage.tsx(2 hunks)apps/meteor/client/views/omnichannel/currentChats/hooks/useAllCustomFields.tsx(0 hunks)apps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsx(3 hunks)apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx(2 hunks)apps/meteor/client/views/omnichannel/customFields/useRemoveCustomField.tsx(2 hunks)apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx(4 hunks)apps/meteor/client/views/omnichannel/directory/chats/ChatsFiltersContextualBar.tsx(3 hunks)apps/meteor/client/views/omnichannel/directory/hooks/useCustomFieldsMetadata.tsx(2 hunks)apps/meteor/client/views/omnichannel/directory/utils/formatCustomFieldsMetadata.tsx(1 hunks)apps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/omnichannel/currentChats/hooks/useAllCustomFields.tsx
- apps/meteor/client/views/omnichannel/contactInfo/hooks/useCustomFieldsQuery.ts
🧰 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/omnichannel/directory/hooks/useCustomFieldsMetadata.tsxapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/omnichannel/customFields/useRemoveCustomField.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.tsapps/meteor/client/views/omnichannel/directory/utils/formatCustomFieldsMetadata.tsxapps/meteor/client/views/omnichannel/contactInfo/hooks/useValidCustomFields.tsapps/meteor/client/views/omnichannel/directory/chats/ChatsFiltersContextualBar.tsxapps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsxapps/meteor/client/views/omnichannel/currentChats/CurrentChatsPage.tsxapps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx
🧠 Learnings (5)
📚 Learning: 2025-12-02T22:23:49.554Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.554Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsx
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx
🧬 Code graph analysis (7)
apps/meteor/client/views/omnichannel/customFields/useRemoveCustomField.tsx (1)
apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(30-85)
apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx (1)
apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(30-85)
apps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.ts (2)
packages/rest-typings/src/index.ts (1)
OperationResult(195-197)apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(30-85)
apps/meteor/client/views/omnichannel/directory/chats/ChatsFiltersContextualBar.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.ts (1)
useCustomFieldsQuery(8-11)
apps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.ts (1)
useCustomFieldsQuery(8-11)
apps/meteor/client/views/omnichannel/currentChats/CurrentChatsPage.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.ts (1)
useCustomFieldsQuery(8-11)
apps/meteor/client/views/omnichannel/directory/chats/ChatInfo/ChatInfo.tsx (2)
apps/meteor/client/views/omnichannel/contactInfo/hooks/useValidCustomFields.ts (1)
useValidCustomFields(12-25)packages/livechat/src/components/Header/index.tsx (1)
CustomField(96-100)
🔇 Additional comments (9)
apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx (1)
31-31: LGTM! Query key centralization applied correctly.The hard-coded query key has been successfully replaced with the centralized
omnichannelQueryKeys.livechat.customFields(), improving maintainability and consistency across the codebase.Also applies to: 90-90
apps/meteor/client/lib/queryKeys.ts (1)
53-53: LGTM! New query key follows established patterns.The
customFieldsMetadatafunction properly extends the query key hierarchy with a type-safe scope parameter, maintaining consistency with the existing architecture.apps/meteor/client/views/omnichannel/customFields/useRemoveCustomField.tsx (1)
7-7: LGTM! Cache invalidation updated correctly.The query invalidation now uses the centralized query key, ensuring consistency with the new data-fetching architecture.
Also applies to: 22-22
apps/meteor/client/views/omnichannel/contactInfo/hooks/useValidCustomFields.ts (1)
1-1: LGTM! Hook relocation and type improvements applied.The import path correctly points to the new centralized hook location, and the type signature has been improved from generic
Record<string, string | unknown>[]to the more specificILivechatCustomField[], enhancing type safety.Also applies to: 5-5, 7-7
apps/meteor/client/views/omnichannel/directory/chats/ChatsFiltersContextualBar.tsx (1)
22-22: LGTM! Data fetching successfully migrated to centralized hook.The component now uses
useCustomFieldsQuery()instead of direct endpoint calls, simplifying the implementation while maintaining the same data access pattern.Also applies to: 36-37
apps/meteor/client/views/omnichannel/hooks/useCustomFieldsQuery.ts (1)
1-11: LGTM! Clean centralized hook implementation.This hook properly encapsulates the custom fields data fetching logic with correct typing and centralized query key management, forming a solid foundation for the refactor.
apps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsx (1)
3-4: LGTM! Migration to centralized hook completed successfully.The component has been properly updated to use
useCustomFieldsQuery()instead of direct endpoint calls. The addition ofuseRoutercorrectly supports the navigation functionality on lines 32-33.Also applies to: 22-22, 26-26, 50-50
apps/meteor/client/views/omnichannel/currentChats/CurrentChatsPage.tsx (1)
31-31: useCustomFieldsQuery swap preserves behavior and typesUsing
useCustomFieldsQueryand destructuring{ data: allCustomFields }aligns with the new shared hook’s return type, and downstream usages (hasCustomFields,CustomFieldsList) still consumeallCustomFields?.customFieldsin the same way. No further changes needed here.Also applies to: 141-141
apps/meteor/client/views/omnichannel/directory/utils/formatCustomFieldsMetadata.tsx (1)
1-1: Updated input type toILivechatCustomField[] | undefinedis safeAccepting
customFields: ILivechatCustomField[] | undefinedcombined with the early!customFieldsguard cleanly handles the undefined case while leaving the filtering/mapping behavior unchanged for valid arrays. This aligns the function with the new query result shape without altering output semantics.Also applies to: 4-4
apps/meteor/client/views/omnichannel/directory/hooks/useCustomFieldsMetadata.tsx
Show resolved
Hide resolved
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Centralize the usage of the getCustomFields endpoint in a single hook
Issue(s)
Steps to test or reproduce
Further comments
CORE-1442
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.