refactor: Phase-out async state API #37944
Conversation
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces internal AsyncState/RecordList primitives with TanStack React Query across hooks, providers, and components; removes RecordList classes, AsyncState utilities, and many list-backed hooks/tests; migrates marketplace/apps orchestration to query-based hooks and updates consumers to query-result shapes (isPending/data/refetch). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Consumer (component/hook)
participant Stream as Stream Service
participant Cache as React Query Cache
participant API as Server API
Note over UI,Stream: useInfiniteMessageQueryUpdates live-update flow
UI->>Cache: read cached infinite pages (queryKey)
UI->>Stream: subscribe("room-messages")
UI->>Stream: subscribe("notify-room")
Stream-->>UI: message created/updated event
UI->>Cache: mutateQueryData (insert or update if filter passes)
Cache-->>UI: cache updated → rerender
Stream-->>UI: deleteMessage event
UI->>Cache: remove message by _id from cache
Stream-->>UI: deleteMessageBulk event
UI->>Cache: invalidateQueries(queryKey)
Cache->>API: refetch on next trigger
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (9)📚 Learning: 2025-12-02T22:23:49.593ZApplied to files:
📚 Learning: 2025-11-07T14:50:33.544ZApplied to files:
📚 Learning: 2025-10-06T20:32:23.658ZApplied to files:
📚 Learning: 2025-09-25T09:59:26.461ZApplied to files:
📚 Learning: 2025-11-19T12:32:29.696ZApplied to files:
📚 Learning: 2025-11-10T19:06:20.146ZApplied to files:
📚 Learning: 2025-11-19T18:20:07.720ZApplied to files:
📚 Learning: 2025-12-10T21:00:54.909ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
🧬 Code graph analysis (3)apps/meteor/client/views/marketplace/hooks/useApps.ts (5)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (2)
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (2)
🔇 Additional comments (8)
✏️ 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37944 +/- ##
===========================================
- Coverage 70.70% 70.67% -0.03%
===========================================
Files 3147 3133 -14
Lines 108849 108340 -509
Branches 19568 19513 -55
===========================================
- Hits 76964 76574 -390
+ Misses 29875 29771 -104
+ Partials 2010 1995 -15
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/15 15:07 (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: Thu, 15 Jan 2026 15:07:53 GMT |
tassoevan
force-pushed
the
chore/async-state
branch
5 times, most recently
from
361a56a to
aa4b3ef
Compare
|
Looks like this PR is ready to merge! 🎉 |
tassoevan
force-pushed
the
chore/async-state
branch
from
aa4b3ef to
a24d0ca
Compare
tassoevan
force-pushed
the
chore/async-state
branch
from
e1bcd88 to
032f070
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
15 issues found across 72 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.ts">
<violation number="1" location="apps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.ts:3">
P2: Hook returns `false` during loading state when data is undefined, causing UI flicker. The async hook's `data` is initially `undefined`, making the expression evaluate to `false` (disabled) before potentially becoming `true` once loaded. Consider returning the loading state alongside the value, or returning `undefined` during loading so consumers can handle it appropriately.</violation>
</file>
<file name="apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts">
<violation number="1" location="apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts:83">
P1: Query key does not include `type` and `text` parameters, causing stale data when filters change. React-query won't refetch when these parameters change since the key only contains `rid`.</violation>
<violation number="2" location="apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts:109">
P1: Pagination offset is calculated from filtered item count, but should track raw API response count. If client-side filtering removes any items, the offset will be incorrect - causing duplicate fetches and potentially missing items. Consider tracking the pre-filtered count separately for offset calculation, or remove client-side filtering if server-side filtering is comprehensive.</violation>
</file>
<file name="apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts">
<violation number="1" location="apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts:39">
P1: Spreading an array into an object creates numeric keys instead of preserving the `usersWaitingForE2EKeys` property. This corrupts the room object structure - the property won't exist, and numeric keys will be added instead.</violation>
</file>
<file name="apps/meteor/client/providers/AppsProvider.tsx">
<violation number="1" location="apps/meteor/client/providers/AppsProvider.tsx:11">
P2: Context value object is created inline, causing unnecessary re-renders of all consumers. Since `AppClientOrchestratorInstance` is a static import, define the value object outside the component to maintain referential stability.</violation>
</file>
<file name="apps/meteor/client/views/marketplace/hooks/useApps.ts">
<violation number="1" location="apps/meteor/client/views/marketplace/hooks/useApps.ts:127">
P2: Using `!isEnterprise` when `isEnterprise` can be `undefined` causes unintended license invalidation during loading. Since `isEnterprise` is explicitly set to `undefined` while loading (line 100), and `!undefined === true`, the license query will be invalidated before the enterprise status is determined. Use explicit equality check `isEnterprise === false` instead.</violation>
<violation number="2" location="apps/meteor/client/views/marketplace/hooks/useApps.ts:164">
P2: Missing error handling for `instance` query. Marketplace errors are propagated but instance errors are silently ignored, which could show no installed apps without any error indication if the instance query fails.</violation>
</file>
<file name="apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx">
<violation number="1" location="apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx:135">
P2: Missing `isSuccess` check for consistency with other conditions. All similar state variables (`noInstalledApps`, `noMarketplaceOrInstalledAppMatches`, `noInstalledAppMatches`) check `isSuccess` first before accessing `data`, but `noAppRequests` does not. Add the `isSuccess &&` check for consistency and to ensure `data` is only accessed after a successful query.
(Based on your team's feedback about avoiding treating undefined as falsy and adding explicit loading checks.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx">
<violation number="1" location="apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx:43">
P2: The mutation starts in `isIdle` state before the `useEffect` triggers it, causing the UI to briefly show nothing. Consider using `isPending || isIdle` or `!isError && !isSuccess` to show the loading state during the initial render and when params are missing.
(Based on your team's feedback about async hooks having undefined initial states that can cause UI flicker.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/meteor/client/components/message/content/location/MapView.tsx">
<violation number="1" location="apps/meteor/client/components/message/content/location/MapView.tsx:16">
P2: Consider destructuring `isLoading` to distinguish between "no image URL provided" and "image still loading" states. Currently, `!imageUrl` is true both when the API key is missing AND while the image is loading, which may cause UI flicker (showing fallback briefly before the image appears).
(Based on your team's feedback about adding explicit loading checks when destructuring from async hooks to prevent UI flicker.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx:34">
P2: Removed loading state causes UI flicker - during initial fetch, `data` is undefined so `itemCount` defaults to 0, causing the empty state ('No Canned Responses') to briefly display instead of a loading indicator before data arrives.</violation>
</file>
<file name="apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsx">
<violation number="1" location="apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsx:50">
P1: Caching breaks for undefined values after type change. The `!snapshot` check will always be true when `extractSnapshot` returns `undefined`, causing repeated API calls via `OmnichannelRoomIconManager.get()` until the icon loads. Use `snapshots.has()` instead to properly cache undefined values.</violation>
</file>
<file name="apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts">
<violation number="1" location="apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts:45">
P2: `itemCount` becomes stale after real-time updates because it preserves the original server count instead of reflecting the mutated array length. This affects Virtuoso's `totalCount` and pagination metadata in components using this hook.</violation>
</file>
<file name="apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts">
<violation number="1" location="apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts:63">
P1: The sorting methods mutate the array in-place with `apps.sort()`. In a react-query `select` function, this will corrupt the cached data since the same array reference is shared. Use `[...apps].sort()` to create a copy before sorting, or change `fallback` to return a copy: `(apps: App[]) => [...apps]`.</violation>
</file>
<file name="apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts">
<violation number="1" location="apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts:25">
P2: Consider adding `enabled: !!room` to prevent the query from running before `useUserRoom` returns room data. Without this, if `room` is temporarily undefined, the query may use the wrong API endpoint (defaults to `channels.files` instead of the correct endpoint for DMs or private groups).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
Show resolved
Hide resolved
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts
Outdated
Show resolved
Hide resolved
...teor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsx
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (1)
10-21: Handle case whenroomis undefined.If
useUserRoom(rid)returnsundefined(e.g., room not found or user lacks access),room?.tfalls back to'c'for the endpoint selection. This may lead to an API call to/v1/channels.filesfor a room the user cannot access, potentially resulting in a 403 or 404. Consider adding a guard or disabling the query when room is undefined.🔎 Suggested fix
export const useFilesList = ({ rid, type, text }: { rid: Required<IUpload>['rid']; type: string; text: string }) => { const room = useUserRoom(rid); const roomTypes = { c: '/v1/channels.files', l: '/v1/channels.files', v: '/v1/channels.files', d: '/v1/im.files', p: '/v1/groups.files', } as const; - const getFiles = useEndpoint('GET', roomTypes[room?.t ?? 'c']); + const roomType = room?.t ?? 'c'; + const getFiles = useEndpoint('GET', roomTypes[roomType]);Then add
enabled: !!roomto the query options:return useInfiniteQuery({ queryKey: roomsQueryKeys.files(rid, { type, text }), + enabled: !!room, queryFn: async ({ pageParam: offset }) => {apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (1)
86-91: Missing dependency in useEffect.The
useEffectdepends ont(from useTranslation) viacreateFilterStructureItems()which uses it, buttis not in the dependency array. This could cause stale translations if the locale changes.🔎 Proposed fix
useEffect(() => { setSortFilterStructure({ label: t('Sort_By'), items: createFilterStructureItems(), }); -}, [isRequested]); +}, [isRequested, t]);Note: You may also need to memoize
createFilterStructureItemsor inline it to avoid lint warnings.
🧹 Nitpick comments (11)
apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (1)
30-46: Consider adding error handling for E2E decryption.If
e2e.decryptFileContent(file)ore2e.getInstanceByRoomId()throws an error for any single file, the entire query will fail and no images will be displayed. Consider wrapping the decryption logic in a try-catch to gracefully handle corrupted or unreadable encrypted files while still displaying the rest.🔎 Proposed fix
for await (const file of items) { if (file.rid && file.content) { - const e2eRoom = await e2e.getInstanceByRoomId(file.rid); - if (e2eRoom?.shouldConvertReceivedMessages()) { - const decrypted = await e2e.decryptFileContent(file); - const key = Base64.encode( - JSON.stringify({ - ...decrypted.encryption, - name: String.fromCharCode(...new TextEncoder().encode(decrypted.name)), - type: decrypted.type, - }), - ); - decrypted.path = `/file-decrypt${decrypted.path}?key=${key}`; - Object.assign(file, decrypted); + try { + const e2eRoom = await e2e.getInstanceByRoomId(file.rid); + if (e2eRoom?.shouldConvertReceivedMessages()) { + const decrypted = await e2e.decryptFileContent(file); + const key = Base64.encode( + JSON.stringify({ + ...decrypted.encryption, + name: String.fromCharCode(...new TextEncoder().encode(decrypted.name)), + type: decrypted.type, + }), + ); + decrypted.path = `/file-decrypt${decrypted.path}?key=${key}`; + Object.assign(file, decrypted); + } + } catch { + // Skip files that fail to decrypt } } }apps/meteor/client/components/message/content/location/hooks/useAsyncImage.ts (1)
8-11: Consider removing redundant guard clause.The guard clause on lines 8-11 is redundant since
enabled: Boolean(src)on line 22 already prevents the query from running whensrcis falsy. React Query won't executequeryFnwhenenabledis false.🔎 Proposed simplification
queryFn: () => new Promise<string>((resolve, reject) => { - if (!src) { - reject(new Error('No src provided')); - return; - } - const image = new Image(); image.addEventListener('load', () => { resolve(image.src); }); image.addEventListener('error', (e) => { - reject(e.error); + reject(new Error('Failed to load image')); }); image.src = src; }),Note: If you keep the guard, add a TypeScript non-null assertion
image.src = src!;on line 20 since the type checker won't knowsrcis defined inside the closure.apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (2)
24-37: Consider memoizing the regex creation for repeated filter calls.The
filterfunction creates a newRegExpon every invocation. Since this filter is called for each message in bothqueryFnand real-time updates, the regex could be created once whentextchanges rather than on every filter call.🔎 Proposed optimization
+ const regex = text ? new RegExp(escapeRegExp(text), 'i') : undefined; + const filter = useEffectEvent((message: IMessage): message is IDiscussionMessage => { if (!isDiscussionMessageInRoom(message, rid)) { return false; } - if (text) { - const regex = new RegExp(escapeRegExp(text), 'i'); - if (!isDiscussionTextMatching(message, regex)) { - return false; - } + if (regex && !isDiscussionTextMatching(message, regex)) { + return false; } return true; }) as (message: IMessage) => message is IDiscussionMessage;
56-60: Redundant filtering after server-filtered API response.The server already filters discussions by
textat the database level infindDiscussionsByRoomAndText, applying a regex match on themsgfield. The client-side re-filtering via.filter(filter)duplicates this work. If this pattern is intentional for consistency with how real-time updates are handled throughuseInfiniteMessageQueryUpdates, consider documenting that intent; otherwise, the client-side filter on line 59 can be removed for API results and applied only to real-time updates.apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsx (1)
45-46: Consider usinghasNextPageguard before callingfetchNextPage.The
loadMoreItemscallback unconditionally callsfetchNextPage(). While React Query handles this gracefully (it won't refetch if there's no next page), adding a guard improves clarity and avoids unnecessary function calls.🔎 Proposed improvement
- const { isPending, error, data, fetchNextPage } = useDiscussionsList(options); + const { isPending, error, data, fetchNextPage, hasNextPage } = useDiscussionsList(options);- loadMoreItems={() => fetchNextPage()} + loadMoreItems={() => hasNextPage && fetchNextPage()}apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
28-66:itemCountmay become stale after real-time mutations.When messages are added or removed via subscriptions,
itemCount(line 45) is preserved from the last fetched page rather than recalculated. This could cause minor UI inconsistencies (e.g., a "load more" indicator appearing when all items are already loaded).Consider updating
itemCountto reflect the actualitems.lengthafter mutations, or document this as intentional (relying on the next fetch to reconcile).🔎 Optional fix to update itemCount
- const itemCount = lastPage.itemCount || items.length; + const itemCount = items.length;Or, if preserving server-side total is preferred, leave as-is and rely on query invalidation for reconciliation.
apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx (1)
31-31: Optional: Remove stable reference from dependency array.The
mutatefunction from React Query has a stable reference and doesn't need to be included in the dependency array. While not harmful, removing it makes the intent clearer.🔎 Proposed refactor
- }, [_id, createdAt, mutate]); + }, [_id, createdAt]);apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx (1)
27-32: Consider movingappRootinsidebeforeEachoritfor better test isolation.The
appRootis defined at describe-block scope but uses a mock that's configured inbeforeEach. While this works becauseappRoot.build()is called insideit(), placingappRootconstruction closer to usage would make the test's data flow clearer and prevent potential issues if tests are reordered or parallelized.apps/meteor/client/lib/queryKeys.ts (1)
164-164: Consider stronger typing forappsStoredarguments.Using
...args: unknown[]loses type information for cache key matching. If the arguments follow a known pattern (similar to howappsMarketplaceandappsInstanceusecanManageApps?: boolean), consider defining explicit parameter types.apps/meteor/client/providers/AppsProvider.tsx (1)
10-12: Use JSX children syntax instead of thechildrenprop.The
childrenprop pattern works but is non-idiomatic. Per React best practices (and flagged by Biome), pass children as JSX content.🔎 Proposed fix
-const AppsProvider = ({ children }: AppsProviderProps) => { - return <AppsContext.Provider children={children} value={{ orchestrator: AppClientOrchestratorInstance }} />; -}; +const AppsProvider = ({ children }: AppsProviderProps) => { + return <AppsContext.Provider value={{ orchestrator: AppClientOrchestratorInstance }}>{children}</AppsContext.Provider>; +};apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (1)
125-136: Consider guarding data access in noAppRequests check.At line 135,
data?.countis accessed without checkingisSuccessfirst. While the optional chaining prevents runtime errors, the condition could be true during loading state ifdatahappens to be undefined.🔎 Suggested improvement
-const noAppRequests = context === 'requested' && data?.count === 0; +const noAppRequests = isSuccess && context === 'requested' && data.count === 0;
📜 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 (72)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/components/message/content/location/MapView.tsxapps/meteor/client/components/message/content/location/hooks/useAsyncImage.tsapps/meteor/client/contexts/AppsContext.tsxapps/meteor/client/contexts/hooks/useAppsReload.tsapps/meteor/client/contexts/hooks/useAppsResult.tsapps/meteor/client/hooks/lists/useRecordList.tsapps/meteor/client/hooks/lists/useScrollableMessageList.tsapps/meteor/client/hooks/lists/useScrollableRecordList.tsapps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.tsapps/meteor/client/hooks/useAsyncState.tsapps/meteor/client/hooks/useComponentDidUpdate.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/lib/asyncState/AsyncState.tsapps/meteor/client/lib/asyncState/AsyncStatePhase.tsapps/meteor/client/lib/asyncState/functions.tsapps/meteor/client/lib/asyncState/index.tsapps/meteor/client/lib/lists/CannedResponseList.tsapps/meteor/client/lib/lists/DiscussionsList.tsapps/meteor/client/lib/lists/FilesList.tsapps/meteor/client/lib/lists/ImagesList.tsapps/meteor/client/lib/lists/MessageList.tsapps/meteor/client/lib/lists/RecordList.spec.tsapps/meteor/client/lib/lists/RecordList.tsapps/meteor/client/lib/lists/ThreadsList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/providers/AppsProvider.tsxapps/meteor/client/providers/AppsProvider/AppsProvider.tsxapps/meteor/client/providers/AppsProvider/index.tsapps/meteor/client/providers/AppsProvider/storeQueryFunction.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useAppInfo.tsapps/meteor/client/views/marketplace/hooks/useAppInstances.tsapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/hooks/useAppsReload.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/hooks/useInstallApp.tsxapps/meteor/client/views/marketplace/hooks/useLogs.tsapps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsxapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFiles.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfList.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfRecordList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsxapps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.tsapps/meteor/tests/mocks/client/marketplace.tsx
💤 Files with no reviewable changes (25)
- apps/meteor/client/lib/lists/MessageList.ts
- apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfRecordList.ts
- apps/meteor/client/lib/asyncState/AsyncState.ts
- apps/meteor/client/contexts/hooks/useAppsReload.ts
- apps/meteor/client/lib/lists/ImagesList.ts
- apps/meteor/client/hooks/useComponentDidUpdate.ts
- apps/meteor/client/contexts/hooks/useAppsResult.ts
- apps/meteor/client/providers/AppsProvider/AppsProvider.tsx
- apps/meteor/client/hooks/lists/useRecordList.ts
- apps/meteor/client/providers/AppsProvider/storeQueryFunction.ts
- apps/meteor/client/lib/asyncState/AsyncStatePhase.ts
- apps/meteor/client/contexts/AppsContext.tsx
- apps/meteor/client/lib/lists/RecordList.ts
- apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts
- apps/meteor/client/hooks/lists/useScrollableMessageList.ts
- apps/meteor/client/lib/lists/DiscussionsList.ts
- apps/meteor/client/lib/asyncState/functions.ts
- apps/meteor/client/hooks/lists/useScrollableRecordList.ts
- apps/meteor/client/hooks/useAsyncState.ts
- apps/meteor/client/lib/asyncState/index.ts
- apps/meteor/client/providers/AppsProvider/index.ts
- apps/meteor/client/lib/lists/ThreadsList.ts
- apps/meteor/client/lib/lists/CannedResponseList.ts
- apps/meteor/client/lib/lists/FilesList.ts
- apps/meteor/client/lib/lists/RecordList.spec.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:
apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfList.tsxapps/meteor/client/views/marketplace/hooks/useAppsReload.tsapps/meteor/client/views/marketplace/hooks/useAppInstances.tsapps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.tsapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsxapps/meteor/client/views/marketplace/hooks/useLogs.tsapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsxapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/marketplace/hooks/useAppInfo.tsapps/meteor/client/providers/AppsProvider.tsxapps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/components/message/content/location/hooks/useAsyncImage.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsxapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFiles.tsxapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/marketplace/hooks/useInstallApp.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.tsapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/components/message/content/location/MapView.tsxapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsxapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx
**/*.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:
apps/meteor/client/views/marketplace/hooks/useApps.spec.ts
🧠 Learnings (24)
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/marketplace/hooks/useAppsReload.tsapps/meteor/client/views/marketplace/hooks/useAppInstances.tsapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/hooks/useRoomsList.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.tsapps/meteor/client/views/marketplace/hooks/useAppInfo.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.tsapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useAppInfo.tsapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsx
📚 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/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/lib/queryKeys.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/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/client/views/marketplace/hooks/useApps.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.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/departments/EditDepartment.tsxapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/hooks/useRoomsList.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/lib/queryKeys.ts
📚 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/lib/queryKeys.ts
📚 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/lib/queryKeys.ts
📚 Learning: 2025-10-24T08:36:09.247Z
Learnt from: yiweigao0226
Repo: RocketChat/Rocket.Chat PR: 36746
File: apps/meteor/client/views/room/ShareLocation/LiveLocationModal.tsx:0-0
Timestamp: 2025-10-24T08:36:09.247Z
Learning: The location sharing feature in apps/meteor/client/views/room/ShareLocation/ uses MapLibre GL JS for map rendering instead of Google Maps or LocationIQ, eliminating the need for API keys in client code.
Applied to files:
apps/meteor/client/components/message/content/location/MapView.tsx
📚 Learning: 2025-10-24T08:37:21.101Z
Learnt from: yiweigao0226
Repo: RocketChat/Rocket.Chat PR: 36746
File: apps/meteor/client/views/room/ShareLocation/ShareLocationModal.tsx:0-0
Timestamp: 2025-10-24T08:37:21.101Z
Learning: The location sharing feature is migrating from Google Maps and LocationIQ to MapLibre GL JS to eliminate hardcoded API keys in client code, addressing security concerns about key exposure in the client bundle and message attachments.
Applied to files:
apps/meteor/client/components/message/content/location/MapView.tsx
🧬 Code graph analysis (29)
apps/meteor/client/views/marketplace/hooks/useAppsReload.ts (2)
packages/mock-providers/src/MockedAppRootBuilder.tsx (1)
queryClient(270-280)apps/meteor/client/lib/queryKeys.ts (1)
marketplaceQueryKeys(160-179)
apps/meteor/client/views/marketplace/hooks/useAppInstances.ts (2)
packages/rest-typings/src/index.ts (1)
OperationResult(191-193)apps/meteor/client/lib/queryKeys.ts (1)
marketplaceQueryKeys(160-179)
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx (2)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsx (1)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsx (1)
useOmnichannelRoomIcon(11-15)
apps/meteor/client/views/marketplace/hooks/useLogs.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
marketplaceQueryKeys(160-179)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
packages/core-typings/src/IMessage/IMessage.ts (1)
IDiscussionMessage(329-333)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-33)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
cannedResponsesQueryKeys(40-43)
apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsx (1)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
useDiscussionsList(19-74)
apps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsx (1)
apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (1)
useImagesList(9-62)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
packages/mock-providers/src/MockedAppRootBuilder.tsx (1)
queryClient(270-280)
apps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (1)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
useHistoryMessageList(21-57)
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx (1)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (1)
useFilesList(10-79)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (2)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(50-96)
apps/meteor/client/views/marketplace/hooks/useAppInfo.ts (1)
apps/meteor/client/views/marketplace/hooks/useApps.ts (1)
useApps(76-180)
apps/meteor/client/providers/AppsProvider.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)
apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
apps/meteor/client/views/marketplace/hooks/useApps.ts (1)
useApps(76-180)
apps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsx (1)
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
useTeamsChannelList(15-57)
apps/meteor/tests/mocks/client/marketplace.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)
apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx (2)
packages/ui-contexts/src/index.ts (1)
useTranslation(81-81)apps/meteor/client/hooks/useRoomsList.ts (1)
useRoomsList(6-41)
apps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsx (1)
packages/livechat/src/store/index.tsx (1)
Dispatch(209-209)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (6)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(316-320)apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-33)
apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
videoConferenceQueryKeys(181-184)
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
teamsQueryKeys(120-129)
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (3)
packages/ui-contexts/src/index.ts (1)
useTranslation(81-81)apps/meteor/client/views/marketplace/hooks/useAppsReload.ts (1)
useAppsReload(6-11)apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
useFilteredApps(15-130)
apps/meteor/client/components/message/content/location/MapView.tsx (1)
apps/meteor/client/components/message/content/location/hooks/useAsyncImage.ts (1)
useAsyncImage(3-25)
apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-33)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
useCannedResponseList(7-57)
apps/meteor/client/hooks/useRoomsList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-33)
🪛 Biome (2.1.2)
apps/meteor/client/providers/AppsProvider.tsx
[error] 11-11: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
⏰ 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). (10)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx
Outdated
Show resolved
Hide resolved
.../client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts
Outdated
Show resolved
Hide resolved
tassoevan
force-pushed
the
chore/async-state
branch
from
032f070 to
f8e2128
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (21)
apps/meteor/client/components/message/content/location/hooks/useAsyncImage.ts (1)
17-18: Fix error handling -e.erroris undefined.The
errorevent onImageelements provides a simpleEventobject, not anErrorEvent. TheEventobject doesn't have anerrorproperty, soe.errorwill be undefined.🔎 Proposed fix
image.addEventListener('error', (e) => { - reject(e.error); + reject(new Error('Failed to load image')); });apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsx (1)
50-59: Critical: Caching broken for undefined values.The
if (!snapshot)check at line 53 cannot distinguish between a missing cache entry and a cachedundefinedvalue. WhenextractSnapshotreturnsundefined(icon not yet loaded), every call togetSnapshotwill:
- Retrieve
undefinedfrom the map- Evaluate
!snapshotastrue- Call
extractSnapshotagain → triggersOmnichannelRoomIconManager.get()→ potentially fires another API request- Re-cache
undefinedThis defeats caching and causes repeated API calls until the icon loads.
Recommended fix using Map.has()
() => { - let snapshot = snapshots.get(`${app}-${iconName}`); - - if (!snapshot) { - snapshot = extractSnapshot(app, iconName); - snapshots.set(`${app}-${iconName}`, snapshot); + const key = `${app}-${iconName}`; + if (!snapshots.has(key)) { + snapshots.set(key, extractSnapshot(app, iconName)); } - - return snapshot; + return snapshots.get(key); },apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx (3)
25-31: Consider preventing duplicate mutation calls on remount.The mutation will re-trigger if the component remounts with the same route parameters. While unsubscription APIs are typically idempotent, you can prevent unnecessary calls by tracking whether the mutation has already been attempted using a ref.
🔎 Proposed refactor
+ const hasMutatedRef = useRef(false); + useEffect(() => { - if (!_id || !createdAt) { + if (!_id || !createdAt || hasMutatedRef.current) { return; } - + hasMutatedRef.current = true; mutate({ _id, createdAt }); }, [_id, createdAt, mutate]);
43-45: Consider handling the initial idle state to prevent blank UI.When the component first renders, the mutation is in
isIdlestate before theuseEffecttriggers it. During this brief moment, all state flags arefalse, causing the condition chain to evaluate tofalseand render nothing. While the flicker is minimal in React 18, you can eliminate it by checking for the idle state explicitly.🔎 Proposed refactor using isIdle
- const { isPending, isError, error, isSuccess } = useMailerUnsubscriptionMutation(); + const { isPending, isError, error, isSuccess, isIdle } = useMailerUnsubscriptionMutation(); return ( <HeroLayout> <Box color='default' marginInline='auto' marginBlock={16} maxWidth={800}> - {(isPending && <Throbber disabled />) || + {((isPending || isIdle) && <Throbber disabled />) || (isError && <Callout type='danger' title={error.message} />) || (isSuccess && <Callout type='success' title={t('You_have_successfully_unsubscribed')} />)} </Box>Alternatively, use a more concise pattern:
- {(isPending && <Throbber disabled />) || + {(!isError && !isSuccess && <Throbber disabled />) || (isError && <Callout type='danger' title={error.message} />) || (isSuccess && <Callout type='success' title={t('You_have_successfully_unsubscribed')} />)}
18-20: Fix error handling to pass error message string.The
dispatchToastMessageexpects a string for themessagefield, but the entire error object is being passed. Update the type annotation toError(matching the mutation's generic type) and passerror.message.🔎 Proposed fix
- onError: (error: unknown) => { - dispatchToastMessage({ type: 'error', message: error }); + onError: (error: Error) => { + dispatchToastMessage({ type: 'error', message: error.message }); },apps/meteor/client/providers/AppsProvider.tsx (1)
11-11: Define context value outside component to prevent unnecessary re-renders.The inline object creation
{ orchestrator: AppClientOrchestratorInstance }creates a new reference on every render, causing allAppsContextconsumers to re-render even though the value never changes. SinceAppClientOrchestratorInstanceis a static singleton, define the value object once outside the component.🔎 Proposed fix
+const contextValue = { orchestrator: AppClientOrchestratorInstance }; + const AppsProvider = ({ children }: AppsProviderProps) => { - return <AppsContext.Provider children={children} value={{ orchestrator: AppClientOrchestratorInstance }} />; + return <AppsContext.Provider value={contextValue}>{children}</AppsContext.Provider>; };apps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx (1)
97-97: Critical: undefined variablehistorywill cause ReferenceError.Line 97 references
history.lengthbut the variablehistoryis not defined in this component. Based on line 50, this should referencemessagesinstead.🔎 Proposed fix
- {!error && totalItemCount > 0 && history.length > 0 && ( + {!error && totalItemCount > 0 && messages.length > 0 && (apps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.ts (1)
3-3: Loading state causes prematurefalsereturn, potentially causing UI flicker.During the initial load,
dataisundefined, causing the expression to evaluate tofalse(since0 !== 0). If the actual license value is non-zero, the UI will briefly show the disabled state before switching to enabled once data loads.Consider returning the loading state alongside the boolean value, or return
undefinedduring loading so consumers can handle the pending state appropriately:export const usePrivateAppsEnabled = () => { const { data, isPending } = useLicense({ loadValues: true }); return { enabled: (data?.limits?.privateApps?.max ?? 0) !== 0, isPending, }; };Or return
undefinedduring loading:export const usePrivateAppsEnabled = () => { const { data, isPending } = useLicense({ loadValues: true }); if (isPending) return undefined; return (data?.limits?.privateApps?.max ?? 0) !== 0; };Based on the past review comment, this issue was previously identified.
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (2)
28-28: Departments fetched on every page is inefficient.The
getDepartmentscall executes for every page fetched within thequeryFn, causing unnecessary network requests. This should be extracted to a separate query with appropriate caching.Consider using a dedicated
useQueryfor departments:const { data: departments } = useQuery({ queryKey: ['departments'], queryFn: () => getDepartments({ text: '' }), staleTime: 5 * 60 * 1000, // Cache for 5 minutes });Then reference
departmentswithin the infinite query'sselectfunction or pass as an enabled dependency.Based on the past review comment, this issue was previously identified.
40-40: Unsafe non-null assertion when department may not exist.The non-null assertion
departmentName!is unsafe becausedepartmentNamecan beundefinedif:
- The department was deleted
- Permission restrictions prevent access
- The department ID is invalid
This could cause downstream issues in components expecting a defined value. Remove the assertion and update the return type:
-items: cannedResponses.map((cannedResponse): IOmnichannelCannedResponse & { departmentName: ILivechatDepartment['name'] } => { +items: cannedResponses.map((cannedResponse): IOmnichannelCannedResponse & { departmentName: ILivechatDepartment['name'] | undefined } => {- departmentName: departmentName!, + departmentName,Update consumers to handle the
undefinedcase with conditional rendering or fallback values.Based on the past review comment, this issue was previously identified.
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (1)
25-26: Consider addingenabled: !!roomto prevent incorrect endpoint usage.If
useUserRoom(rid)returnsundefinedinitially, the query runs with thechannels.filesendpoint (due to fallback on line 21). For DMs (d) or private groups (p), this would fetch from the wrong API endpoint until the room data loads.🔎 Proposed fix
return useInfiniteQuery({ + enabled: !!room, queryKey: roomsQueryKeys.files(rid, { type, text }),apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (1)
117-117:historyis undefined — will cause runtime ReferenceError.The variable
historyis not defined in this component. Based on line 65, it should bemessages.🔎 Proposed fix
- {!error && totalItemCount > 0 && history.length > 0 && ( + {!error && totalItemCount > 0 && messages.length > 0 && (apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
39-39: Bug: Spreading mapped array corrupts room object structure.Spreading
usersWaitingForE2EKeys?.map(...)directly into the object produces numeric keys ({0: {...}, 1: {...}}) instead of preservingusersWaitingForE2EKeysas an array property.🔎 Proposed fix
- ...usersWaitingForE2EKeys?.map(({ userId, ts }) => ({ userId, ts: new Date(ts) })), + ...(usersWaitingForE2EKeys && { + usersWaitingForE2EKeys: usersWaitingForE2EKeys.map(({ userId, ts }) => ({ userId, ts: new Date(ts) })), + }),apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
61-76: Sorting functions mutate React Query cached data.The sorting methods use
apps.sort()which mutates the array in-place. When combined withfallbackreturning the same array reference, this can corrupt the query cache. React Query expects cached data to remain immutable.🔎 Proposed fix: Create copies before sorting
const sortingMethods: Record<string, (apps: App[]) => App[]> = { urf: (apps: App[]) => - apps.sort( + [...apps].sort( (firstApp, secondApp) => (secondApp?.appRequestStats?.totalUnseen || 0) - (firstApp?.appRequestStats?.totalUnseen || 0), ), url: (apps: App[]) => - apps.sort( + [...apps].sort( (firstApp, secondApp) => (firstApp?.appRequestStats?.totalUnseen || 0) - (secondApp?.appRequestStats?.totalUnseen || 0), ), - az: (apps: App[]) => apps.sort((firstApp, secondApp) => sortAppsByAlphabeticalOrInverseOrder(firstApp.name, secondApp.name)), - za: (apps: App[]) => apps.sort((firstApp, secondApp) => sortAppsByAlphabeticalOrInverseOrder(secondApp.name, firstApp.name)), + az: (apps: App[]) => [...apps].sort((firstApp, secondApp) => sortAppsByAlphabeticalOrInverseOrder(firstApp.name, secondApp.name)), + za: (apps: App[]) => [...apps].sort((firstApp, secondApp) => sortAppsByAlphabeticalOrInverseOrder(secondApp.name, firstApp.name)), mru: (apps: App[]) => - apps.sort((firstApp, secondApp) => sortAppsByClosestOrFarthestModificationDate(firstApp.modifiedAt, secondApp.modifiedAt)), + [...apps].sort((firstApp, secondApp) => sortAppsByClosestOrFarthestModificationDate(firstApp.modifiedAt, secondApp.modifiedAt)), lru: (apps: App[]) => - apps.sort((firstApp, secondApp) => sortAppsByClosestOrFarthestModificationDate(secondApp.modifiedAt, firstApp.modifiedAt)), + [...apps].sort((firstApp, secondApp) => sortAppsByClosestOrFarthestModificationDate(secondApp.modifiedAt, firstApp.modifiedAt)), };apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
43-52:itemCountbecomes stale after real-time mutations.When messages are added or deleted via real-time updates,
itemCount(line 45) preserves the original server count instead of reflecting the actual mutated array length. This can cause pagination metadata inconsistencies and affect Virtuoso'stotalCountin consuming components.🔎 Suggested improvement
Consider updating
itemCountto reflect the actual items length after mutation:- const itemCount = lastPage.itemCount || items.length; + const itemCount = items.length;However, this may have trade-offs with server-side pagination expectations. An alternative is to keep
itemCountas the server's total but document this limitation.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
108-113: Pagination offset calculated from filtered items may cause inconsistencies.The
getNextPageParamcalculatesloadedItemsCountfrom the filteredpage.items.length. If client-side filtering (lines 101-104) removes items that the server returned, the offset sent to the next API call may not align with server expectations, potentially causing duplicates or gaps.The FIXME comment acknowledges this. Consider tracking the raw API response count separately for offset calculation, or ensure server-side filtering is comprehensive enough that client-side filtering is redundant.
apps/meteor/client/views/marketplace/hooks/useApps.ts (4)
13-13: sortByName mutates the input array.
Array.sort()mutates in-place, which can corrupt React Query's cached data. Create a copy before sorting.🔎 Proposed fix
-const sortByName = (apps: App[]): App[] => apps.sort((a, b) => (a.name.toLowerCase() > b.name.toLowerCase() ? 1 : -1)); +const sortByName = (apps: App[]): App[] => [...apps].sort((a, b) => (a.name.toLowerCase() > b.name.toLowerCase() ? 1 : -1));
127-129: Unintended license invalidation during loading state.When
isEnterpriseisundefined(during loading),!isEnterpriseevaluates totrue, triggering license invalidation before enterprise status is determined.🔎 Proposed fix
- if (['app/added', 'app/removed'].includes(key) && !isEnterprise) { + if (['app/added', 'app/removed'].includes(key) && isEnterprise === false) {
164-166: Missing error handling for instance query.Marketplace errors are propagated but instance query errors are silently ignored. If the instance query fails, users will see no installed apps without error indication.
🔎 Proposed fix
queryFn: async () => { if (marketplace.isError) { throw marketplace.error; } + if (instance.isError) { + throw instance.error; + }
33-33: Mutating cached marketplace data.
sortByName(marketplace.data || [])will mutate the cached array whenmarketplace.dataexists. This corrupts React Query's cache and can cause subtle bugs.🔎 Proposed fix
- sortByName(marketplace.data || []).forEach((app) => { + sortByName([...(marketplace.data || [])]).forEach((app) => {apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (1)
135-135: MissingisSuccesscheck for consistency.All similar state variables (
noInstalledApps,noMarketplaceOrInstalledAppMatches,noInstalledAppMatches) checkisSuccessbefore accessingdata, butnoAppRequestsdoes not. This is inconsistent and could access undefined data during loading.🔎 Proposed fix
- const noAppRequests = context === 'requested' && data?.count === 0; + const noAppRequests = isSuccess && context === 'requested' && data.count === 0;
🧹 Nitpick comments (5)
apps/meteor/client/components/message/content/location/hooks/useAsyncImage.ts (1)
8-11: Remove redundant guard clause.The
if (!src)check in thequeryFnis redundant becauseenabled: Boolean(src)on line 22 already prevents the query from running whensrcis falsy. React Query will not invokequeryFnwhenenabledisfalse.🔎 Proposed simplification
queryFn: () => new Promise<string>((resolve, reject) => { - if (!src) { - reject(new Error('No src provided')); - return; - } - const image = new Image(); image.addEventListener('load', () => { resolve(image.src);apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsx (1)
49-49: Remove outdated comment.This comment is now misleading after the type change—caching actually breaks for
undefinedvalues (see below). Per coding guidelines, avoid implementation comments.Proposed fix
- // No problem here, because it's return value is a cached in the snapshots map on subsequent calls () => {apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (1)
86-91: Missing dependency in useEffect.The
useEffectcallscreateFilterStructureItems()but only depends onisRequested. SincecreateFilterStructureItemsis defined inside the component and referencesisRequested, this works but could break if the function is refactored. Consider addingtto dependencies or extracting the logic.apps/meteor/client/lib/queryKeys.ts (1)
165-165: Consider stronger typing forappsStoredquery key.Using
...args: unknown[]accepts any arguments without type safety. Since this key is used inuseApps.tswith specific data shapes, consider narrowing the type or documenting the expected usage.🔎 Example with explicit optional parameters
- appsStored: (...args: unknown[]) => [...marketplaceQueryKeys.all, 'apps-stored', ...args] as const, + appsStored: (instanceData?: App[], marketplaceData?: App[]) => [...marketplaceQueryKeys.all, 'apps-stored', instanceData, marketplaceData] as const,Note: This would require importing the
Apptype or using a more general array type.apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (1)
30-46: E2E decryption runs on every fetch.The decryption logic runs inside
queryFn, meaning it executes on every page fetch. If decryption is expensive, consider caching decrypted results or moving decryption to theselectfunction to avoid re-decrypting on refetch. This is likely acceptable given the small page size (5 items).
📜 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 (72)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/components/message/content/location/MapView.tsxapps/meteor/client/components/message/content/location/hooks/useAsyncImage.tsapps/meteor/client/contexts/AppsContext.tsxapps/meteor/client/contexts/hooks/useAppsReload.tsapps/meteor/client/contexts/hooks/useAppsResult.tsapps/meteor/client/hooks/lists/useRecordList.tsapps/meteor/client/hooks/lists/useScrollableMessageList.tsapps/meteor/client/hooks/lists/useScrollableRecordList.tsapps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.tsapps/meteor/client/hooks/useAsyncState.tsapps/meteor/client/hooks/useComponentDidUpdate.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/lib/asyncState/AsyncState.tsapps/meteor/client/lib/asyncState/AsyncStatePhase.tsapps/meteor/client/lib/asyncState/functions.tsapps/meteor/client/lib/asyncState/index.tsapps/meteor/client/lib/lists/CannedResponseList.tsapps/meteor/client/lib/lists/DiscussionsList.tsapps/meteor/client/lib/lists/FilesList.tsapps/meteor/client/lib/lists/ImagesList.tsapps/meteor/client/lib/lists/MessageList.tsapps/meteor/client/lib/lists/RecordList.spec.tsapps/meteor/client/lib/lists/RecordList.tsapps/meteor/client/lib/lists/ThreadsList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/providers/AppsProvider.tsxapps/meteor/client/providers/AppsProvider/AppsProvider.tsxapps/meteor/client/providers/AppsProvider/index.tsapps/meteor/client/providers/AppsProvider/storeQueryFunction.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useAppInfo.tsapps/meteor/client/views/marketplace/hooks/useAppInstances.tsapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/hooks/useAppsReload.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/hooks/useInstallApp.tsxapps/meteor/client/views/marketplace/hooks/useLogs.tsapps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsxapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFiles.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfList.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfRecordList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsxapps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.tsapps/meteor/tests/mocks/client/marketplace.tsx
💤 Files with no reviewable changes (25)
- apps/meteor/client/contexts/hooks/useAppsReload.ts
- apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts
- apps/meteor/client/hooks/lists/useRecordList.ts
- apps/meteor/client/lib/asyncState/index.ts
- apps/meteor/client/providers/AppsProvider/index.ts
- apps/meteor/client/hooks/lists/useScrollableRecordList.ts
- apps/meteor/client/lib/lists/CannedResponseList.ts
- apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfRecordList.ts
- apps/meteor/client/lib/lists/DiscussionsList.ts
- apps/meteor/client/lib/asyncState/AsyncState.ts
- apps/meteor/client/lib/lists/FilesList.ts
- apps/meteor/client/hooks/useAsyncState.ts
- apps/meteor/client/lib/lists/ImagesList.ts
- apps/meteor/client/lib/lists/ThreadsList.ts
- apps/meteor/client/lib/lists/MessageList.ts
- apps/meteor/client/hooks/useComponentDidUpdate.ts
- apps/meteor/client/lib/asyncState/functions.ts
- apps/meteor/client/contexts/hooks/useAppsResult.ts
- apps/meteor/client/contexts/AppsContext.tsx
- apps/meteor/client/lib/lists/RecordList.spec.ts
- apps/meteor/client/hooks/lists/useScrollableMessageList.ts
- apps/meteor/client/lib/lists/RecordList.ts
- apps/meteor/client/lib/asyncState/AsyncStatePhase.ts
- apps/meteor/client/providers/AppsProvider/AppsProvider.tsx
- apps/meteor/client/providers/AppsProvider/storeQueryFunction.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/meteor/client/views/marketplace/hooks/useAppsReload.ts
- apps/meteor/client/views/marketplace/hooks/useAppInfo.ts
- apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFiles.tsx
- apps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.ts
- apps/meteor/client/views/marketplace/hooks/useLogs.ts
- apps/meteor/client/views/marketplace/hooks/useApps.spec.ts
- apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsx
- apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx
- apps/meteor/tests/mocks/client/marketplace.tsx
- apps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsx
- apps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
- apps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsx
- apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsx
- apps/meteor/client/views/marketplace/hooks/useInstallApp.tsx
- apps/meteor/client/components/message/content/location/MapView.tsx
🧰 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/room/contextualBar/Threads/ThreadList.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfList.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/components/message/content/location/hooks/useAsyncImage.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.tsapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsxapps/meteor/client/views/marketplace/hooks/useAppInstances.tsapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/providers/AppsProvider.tsx
🧠 Learnings (20)
📚 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/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/lib/queryKeys.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/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/hooks/useAppInstances.ts
📚 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/hooks/useRoomsList.tsapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/client/views/marketplace/hooks/useApps.ts
📚 Learning: 2025-11-07T14:50:33.544Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37423
File: packages/i18n/src/locales/en.i18n.json:18-18
Timestamp: 2025-11-07T14:50:33.544Z
Learning: Rocket.Chat settings: in apps/meteor/ee/server/settings/abac.ts, the Abac_Cache_Decision_Time_Seconds setting uses invalidValue: 0 as the fallback when ABAC is unlicensed. With a valid license, admins can still set the value to 0 to intentionally disable the ABAC decision cache.
Applied to files:
apps/meteor/client/views/marketplace/hooks/useApps.ts
📚 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/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/lib/queryKeys.ts
📚 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/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/lib/queryKeys.ts
📚 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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/lib/queryKeys.ts
🧬 Code graph analysis (19)
apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx (2)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
useThreadsList(48-120)packages/ui-client/src/components/Contextualbar/index.ts (1)
ContextualbarEmptyContent(34-34)
apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsx (1)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
useDiscussionsList(19-76)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (2)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
useHistoryMessageList(21-59)packages/ui-client/src/components/Contextualbar/index.ts (1)
ContextualbarEmptyContent(34-34)
apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx (2)
packages/ddp-client/src/legacy/RocketchatSDKLegacy.ts (1)
unsubscribe(246-248)packages/ui-contexts/src/index.ts (3)
useRouteParameter(65-65)useToastMessageDispatch(78-78)useTranslation(81-81)
apps/meteor/client/hooks/useRoomsList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (2)
apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)apps/meteor/client/lib/queryKeys.ts (1)
cannedResponsesQueryKeys(41-44)
apps/meteor/client/views/marketplace/hooks/useApps.ts (3)
apps/meteor/client/lib/queryKeys.ts (1)
marketplaceQueryKeys(161-180)apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)packages/ui-contexts/src/index.ts (2)
usePermission(57-57)useStream(77-77)
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx (1)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (1)
useFilesList(10-79)
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
teamsQueryKeys(121-130)
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (2)
apps/meteor/client/views/marketplace/hooks/useAppsReload.ts (1)
useAppsReload(6-11)apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
useFilteredApps(15-130)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (2)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(51-97)
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx (2)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)
apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
videoConferenceQueryKeys(182-185)
apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx (1)
apps/meteor/client/hooks/useRoomsList.ts (1)
useRoomsList(6-41)
apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
apps/meteor/client/views/marketplace/hooks/useApps.ts (1)
useApps(76-180)
apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (2)
apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
apps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsx (1)
apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (1)
useImagesList(9-62)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
packages/core-typings/src/IMessage/IMessage.ts (1)
IDiscussionMessage(329-333)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
apps/meteor/client/providers/AppsProvider.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(31-33)
🪛 Biome (2.1.2)
apps/meteor/client/providers/AppsProvider.tsx
[error] 11-11: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
tassoevan
force-pushed
the
chore/async-state
branch
from
4de5311 to
f1a19e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/client/views/omnichannel/hooks/useScopeDict.ts (1)
4-12: Potential empty label when scope is 'department' with undefined departmentName.The function now returns
string | undefined(implicit return type after removing the explicit: stringannotation). Whenscopeis'department'anddepartmentNameisundefined, the function returnsundefined, which renders as an empty label in the UI.Consider either:
- Providing a fallback label (e.g.,
t('Department')) whendepartmentNameis undefined, or- Explicitly documenting that an empty label is acceptable for this edge case.
🔎 Proposed fix with fallback label
-export const useScopeDict = (scope: IOmnichannelCannedResponse['scope'], departmentName: ILivechatDepartment['name'] | undefined) => { +export const useScopeDict = (scope: IOmnichannelCannedResponse['scope'], departmentName: ILivechatDepartment['name'] | undefined): string => { const { t } = useTranslation(); const dict: Record<string, string> = { global: t('Public'), user: t('Private'), }; - return dict[scope] || departmentName; + return dict[scope] || departmentName || t('Department'); };apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx (1)
34-34: Replaceanywith proper typing foronClickItem.The FIXME comment indicates this needs attention. The
dataparameter should be typed as the canned response item type rather thanany.🔎 Proposed fix
- onClickItem: (data: any) => void; // FIXME: fix typings + onClickItem: (data: IOmnichannelCannedResponse & { departmentName?: ILivechatDepartment['name'] }) => void;
🧹 Nitpick comments (5)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
28-29: Address TODO: Departments can be fetched once with a separate query.The TODO comments correctly identify that fetching departments on every page is inefficient. You can extract departments into a separate
useQuerywith appropriate caching to avoid refetching.🔎 Proposed refactor using a separate departments query
Extract department fetching into its own query and reference it in the infinite query:
export const useCannedResponseList = ({ filter, type }: { filter: string; type: string }) => { const getCannedResponses = useEndpoint('GET', '/v1/canned-responses'); const getDepartments = useEndpoint('GET', '/v1/livechat/department'); const count = 25; + + const { data: departmentsData } = useQuery({ + queryKey: ['departments'], + queryFn: () => getDepartments({ text: '' }), + staleTime: 5 * 60 * 1000, // Cache for 5 minutes + }); + + const departments = departmentsData?.departments ?? []; return useInfiniteQuery({ queryKey: cannedResponsesQueryKeys.list({ filter, type }), queryFn: async ({ pageParam: offset }) => { const { cannedResponses, total } = await getCannedResponses({ ...(filter && { text: filter }), ...(type && ['global', 'user'].find((option) => option === type) && { scope: type }), ...(type && !['global', 'user', 'all'].find((option) => option === type) && { scope: 'department', departmentId: type, }), offset, count, }); - // TODO: Optimize by creating an endpoint that returns canned responses with department names - // TODO: Use another query for departments to avoid refetching - const { departments } = await getDepartments({ text: '' }); - return { items: cannedResponses.map((cannedResponse): IOmnichannelCannedResponse & { departmentName?: ILivechatDepartment['name'] } => { const departmentName = cannedResponse.departmentId ? departments.find((department) => department._id === cannedResponse.departmentId)?.name : undefined; return { ...cannedResponse, _updatedAt: new Date(cannedResponse._updatedAt), _createdAt: new Date(cannedResponse._createdAt), departmentName, }; }), itemCount: total, }; }, + enabled: departmentsData !== undefined, initialPageParam: 0,Would you like me to generate a complete implementation or open a new issue to track this optimization?
apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (1)
63-66: React Query migration implemented correctly.The hook signature change and derived values are sound. The defaults (
[]for messages,0for totalItemCount) safely handle loading states.Optional: Consider destructuring
hasNextPageandisFetchingNextPagefrom the hook result to guard thefetchNextPage()call on line 127:const { isPending, error, isSuccess, data, fetchNextPage, hasNextPage, isFetchingNextPage } = useHistoryMessageList(query);Then on line 127:
endReached={() => hasNextPage && !isFetchingNextPage && fetchNextPage()}While
fetchNextPage()is safe to call without guards (React Query handles it), this is a common best practice for clarity and avoiding unnecessary calls.apps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx (3)
48-51: Verify pagination optimizations available from the hook.The query destructuring is correct, but the hook returns additional properties from
useInfiniteQuerythat could improve the UX. Consider destructuringhasNextPageandisFetchingNextPageto provide more explicit pagination control and loading states.const { isPending, error, isSuccess, data, fetchNextPage, hasNextPage, isFetchingNextPage } = useHistoryMessageList(query);Then use these in the
endReachedcallback (line 107) for more explicit control.
95-97: Adjust empty state condition to reflect client-filtered results.The empty state (line 95) checks
totalItemCount === 0, but should checkmessages.length === 0. Since the hook applies client-side filtering (per the relevant code snippet inuseHistoryMessageList.ts),totalItemCount(server count) can be greater than zero whilemessages(client-filtered) is empty. This leaves a gap where neither the empty state nor the list is displayed.🔎 Proposed fix
- {isSuccess && totalItemCount === 0 && <ContextualbarEmptyContent title={t('No_results_found')} />} + {isSuccess && messages.length === 0 && <ContextualbarEmptyContent title={t('No_results_found')} />}
107-107: Consider guardingfetchNextPage()withhasNextPagecheck.While
fetchNextPage()internally handles cases where there's no next page or a fetch is already in progress, explicitly checkinghasNextPage && !isFetchingNextPagewould make the pagination logic clearer and avoid unnecessary function calls.🔎 Proposed refinement
First, destructure the additional properties on line 48:
const { isPending, error, isSuccess, data, fetchNextPage, hasNextPage, isFetchingNextPage } = useHistoryMessageList(query);Then update the
endReachedcallback:- endReached={() => fetchNextPage()} + endReached={() => { + if (hasNextPage && !isFetchingNextPage) { + fetchNextPage(); + } + }}
📜 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 (9)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponse.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/Item.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponse.tsxapps/meteor/client/views/omnichannel/cannedResponses/modals/CreateCannedResponse/CreateCannedResponseModal.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/omnichannel/hooks/useScopeDict.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/cannedResponses/contextualBar/CannedResponse/Item.tsxapps/meteor/client/views/omnichannel/hooks/useScopeDict.tsapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponse.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/modals/CreateCannedResponse/CreateCannedResponseModal.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponse.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/hooks/useCannedResponseList.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/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx
📚 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/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (2)
apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)apps/meteor/client/lib/queryKeys.ts (1)
cannedResponsesQueryKeys(41-44)
apps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx (1)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
useHistoryMessageList(21-59)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (14)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponse.tsx (1)
22-22: LGTM: Type relaxation aligns with broader refactor.The optional
departmentNameis consistent with the updateduseScopeDictsignature and the broader PR objective to phase out AsyncState patterns.apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/Item.tsx (1)
11-11: LGTM: Consistent type update.The optional
departmentNamealigns with the related changes inCannedResponse.tsxandWrapCannedResponse.tsx.apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponse.tsx (1)
12-12: LGTM: Type consistency maintained.The optional
departmentNameincannedItemis properly propagated to theCannedResponsecomponent, which now accepts optionaldepartmentName.apps/meteor/client/views/omnichannel/cannedResponses/modals/CreateCannedResponse/CreateCannedResponseModal.tsx (1)
1-1: LGTM: Clean removal of departmentName dependency.The changes correctly replace
departmentNamewithdepartmentIdfrom theIOmnichannelCannedResponsetype, simplifying the data model. The conditional inclusion ofdepartmentIdin the save payload (line 47) ensures backward compatibility when the field is absent.Also applies to: 14-21, 24-24
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsx (1)
24-24: The parent component correctly implements the parameterlessloadMoreItemssignature withfetchNextPagefromuseInfiniteQuery. React Query manages pagination internally viapageParam, eliminating the need for explicit start/end parameters. The implementation is correct and all consumers are properly updated.apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
7-58: Well-structured migration to useInfiniteQuery.The refactor from AsyncState to react-query's
useInfiniteQueryis implemented correctly:
- Centralized query keys are used properly
- Offset-based pagination with
getNextPageParamis correct- The
selectfunction appropriately flattens pages into a consumable shape- Type transformations maintain compatibility with existing consumers
apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (6)
26-27: LGTM! Import changes align with React Query migration.The removal of
useUserIdis correct since the newuseHistoryMessageListhook no longer requires a userId parameter.
103-107: Loading state correctly usesisPending.The initial loading state is handled appropriately with the React Query
isPendingproperty.
115-115: Empty state condition is correct.The check ensures the empty state only appears when the query successfully returns zero results.
117-117: Content condition is correct.The past issue with the undefined
historyvariable has been addressed. The current condition correctly uses themessagesarray and safely guards the Virtuoso rendering.
127-127: Pagination trigger correctly usesfetchNextPage().The
endReachedcallback correctly triggers pagination with React Query'sfetchNextPage()method.
130-137:itemContentcallback correctly renders messages.The function logic is sound, and TypeScript inference handles the return type correctly. The implementation properly computes sequential and new-day flags for message grouping.
apps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx (2)
17-46: LGTM! Clean refactoring to react-query.The removal of
useUserIdand the query setup using react-query patterns are correct. The debounced query construction properly memoizes the parameters.
53-82: LGTM! Event handling and layout are correct.The search handler, message grouping logic, and footer rendering are implemented correctly.
Also applies to: 110-132
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/client/providers/AppsProvider.tsx (1)
10-12: Prefer idiomatic JSX children pattern.Using
children={children}as a prop works but is not the canonical React pattern. Wrap children in the JSX body for better readability and to align with React conventions.🔎 Proposed refactor
const AppsProvider = ({ children }: AppsProviderProps) => { - return <AppsContext.Provider children={children} value={AppClientOrchestratorInstance} />; + return <AppsContext.Provider value={AppClientOrchestratorInstance}>{children}</AppsContext.Provider>; };
📜 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 (10)
apps/meteor/client/contexts/AppsContext.tsxapps/meteor/client/providers/AppsProvider.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/hooks/useAppsOrchestration.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/tests/mocks/client/marketplace.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx
- apps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
- apps/meteor/client/views/marketplace/hooks/useApps.ts
- apps/meteor/tests/mocks/client/marketplace.tsx
🧰 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/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/hooks/useAppsOrchestration.tsapps/meteor/client/contexts/AppsContext.tsxapps/meteor/client/providers/AppsProvider.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/hooks/useAppsOrchestration.ts
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx
🧬 Code graph analysis (4)
apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
apps/meteor/client/views/marketplace/hooks/useApps.ts (1)
useApps(76-180)
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (4)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(29-29)packages/ui-contexts/src/index.ts (3)
useTranslation(81-81)useRouter(63-63)useRouteParameter(65-65)apps/meteor/client/views/marketplace/hooks/useAppsReload.ts (1)
useAppsReload(6-11)apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
useFilteredApps(15-130)
apps/meteor/client/views/marketplace/hooks/useAppsOrchestration.ts (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(29-29)
apps/meteor/client/providers/AppsProvider.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(29-29)
🪛 Biome (2.1.2)
apps/meteor/client/providers/AppsProvider.tsx
[error] 11-11: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
⏰ 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 (9)
apps/meteor/client/contexts/AppsContext.tsx (1)
27-29: LGTM! Clean API surface simplification.The context type has been appropriately simplified to expose only the orchestrator, removing the AsyncState-based properties. This aligns perfectly with the PR's objective to phase out the AsyncState pattern.
apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (2)
61-76: LGTM! Non-mutating sort correctly implemented.The use of
toSortedinstead ofsortensures the cached query data is not mutated, addressing the previous review concerns. This is the correct approach when transforming React Query data in aselectfunction.
34-130: Well-structured migration to React Query.The refactor successfully replaces the AsyncState pattern with a clean React Query
selecttransformation. The data flow is clear: context-based app source selection → composed filtering pipeline → pagination. The implementation properly handles all the filtering, sorting, and pagination logic in a single cohesive transformation.apps/meteor/client/views/marketplace/hooks/useAppsOrchestration.ts (1)
5-5: LGTM! Simplified hook implementation.The hook now returns the full context value directly instead of wrapping it in an object. This is a cleaner API that aligns with the AppsContext simplification. Consumers should note this is a breaking change requiring updates from
const { orchestrator } = useAppsOrchestration()toconst orchestrator = useAppsOrchestration().apps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsx (1)
47-50: LGTM! Cleaner presentational component.Removing the error guard simplifies this component to be purely presentational. Error handling is now appropriately managed at the parent level in
AppsPageContent, resulting in better separation of concerns.apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (4)
113-122: LGTM! Proper React Query integration.The migration to
useFilteredAppsreturning React Query's discriminated union type (isPending,isError,error,data) is implemented correctly. The return type is properly leveraged with early returns for pending and error states.
165-187: LGTM! Appropriate pending state handling.The pending state correctly renders the header, filters, and skeleton, providing good user feedback during data loading.
189-215: LGTM! Proper error state handling.The error state correctly distinguishes between unsupported version and connection errors, rendering appropriate UI for each case. The TODO comment on line 190 appropriately documents the technical debt around error message string comparison—this is existing technical debt, not introduced by this PR.
235-256: LGTM! Type-safe data access via early returns.The direct access to
dataproperties (without optional chaining) is safe because of the early returns forisPendingandisError. At this point in the code flow, TypeScript correctly infers thatdatais defined, ensuring type safety. The chain of empty state conditions with proper fallback toAppsPageContentBodyis well-structured.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts">
<violation number="1" location="apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts:107">
P2: The `itemCount` adjustment formula is inconsistent across pages. Each page calculates `itemCount` based only on that page's filtering ratio (`items.length - threads.length`), but `getNextPageParam` compares cumulative loaded items against the last page's `itemCount`. This causes the pagination threshold to change unpredictably depending on which items get filtered on the last page.
Consider tracking cumulative filtered items in the query context, or removing the adjustment entirely since the FIXME comment already acknowledges this is an estimation.</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
34-35: Address the TODO: Handle loading state to prevent UI flicker.During initial fetch,
datais undefined, causingitemCountto default to 0 andcannedItemsto[](lines 73-74). This will briefly show the empty state before data arrives, creating a jarring user experience.🔎 Suggested fix
Destructure
isPendingfrom the hook and handle it appropriately:- // TODO: handle pending and error states - const { data, fetchNextPage, refetch } = useCannedResponseList({ filter: debouncedText, type }); + const { data, fetchNextPage, refetch, isPending } = useCannedResponseList({ filter: debouncedText, type });Then either:
- Pass
isPendingtoCannedResponseListif it accepts a loading prop, or- Conditionally render a loading indicator before the list:
return ( + isPending ? </* loading indicator */> : ( <CannedResponseList loadMoreItems={fetchNextPage} cannedItems={data?.cannedItems ?? []} itemCount={data?.total ?? 0} ... /> + ) );Similarly, consider handling
isErroranderrorto show error states.apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx (1)
18-20: Fix error handling to pass error message string.The
dispatchToastMessageexpects a string for themessagefield, but the entire error object is being passed. This will cause incorrect toast display or runtime errors.🔎 Proposed fix
- onError: (error) => { - dispatchToastMessage({ type: 'error', message: error }); + onError: (error: Error) => { + dispatchToastMessage({ type: 'error', message: error.message }); },apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
110-116: Pagination offset estimation documented.The FIXME at lines 112-113 correctly documents that offset-based pagination using filtered item count is an estimation. Since the server filters with the same
typeandtextparameters, client-side filtering should rarely diverge. The acknowledged limitation is acceptable for this refactor, with cursor-based pagination as a future improvement path.
🧹 Nitpick comments (5)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (2)
24-37: Minor: Regex created on every filter invocation.The
RegExpconstructor is called each timefilteris invoked (once per message). For typical batch sizes, this is negligible, but for large datasets it could be optimized by memoizing the regex separately and passing it into the filter closure.Not blocking given the expected discussion list sizes and the complexity of refactoring around
useEffectEvent.
56-63: Consider whether client-side filtering is necessary.The API call at lines 49-54 already passes
roomIdandtextfor server-side filtering. The client-sidefilterat line 58 applies the same conditions (ridmatch and text regex).If this is intentional for:
- Type narrowing to
IDiscussionMessage— the filter serves as a type guard- Ensuring consistency with the real-time update filter used in
useInfiniteMessageQueryUpdates...then the redundancy is acceptable. However, if the server is guaranteed to return only matching items, consider whether the filter could be simplified to just the type guard (
isDiscussionMessageInRoom) for the initial fetch, reserving the full filter for real-time updates.The
itemCountadjustment at line 63 is a sensible defensive measure for edge cases where client and server filtering diverge.apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx (1)
28-28: Simplify mutation invocation.The
.call(null, ...)syntax is unnecessary. React Query'smutatefunction can be called directly.🔎 Proposed fix
- mutation.mutate.call(null, { _id, createdAt }); + mutation.mutate({ _id, createdAt });apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
75-86: Unnecessary type assertion before filter call.At line 76,
message as Tis cast before calling the filter, but the filter signature acceptsIMessageand narrows toT. The cast is redundant sinceIMessageis the expected input.🔎 Suggested simplification
const unsubscribeFromRoomMessages = subscribeToRoomMessages(roomId, (message) => { - if (!getFilter()(message as T)) return; + if (!getFilter()(message)) return; mutateQueryData((items) => { const index = items.findIndex((i) => i._id === message._id); if (index !== -1) { - items[index] = message as T; + items[index] = message as T; // Safe: filter confirmed message is T } else { items.push(message as T); items.sort(getCompare()); } }); });apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
43-51: Redundant double-sort of items.The items are sorted twice with the same
comparefunction: once at line 46 after filtering, then again at line 49 when returning. Remove one of the.toSorted(compare)calls.🔎 Proposed fix
const items = messages .map((message) => mapMessageFromApi(message)) .filter(filter) .toSorted(compare); return { - items: items.toSorted(compare), + items, itemCount: total + (items.length - messages.length), // adjust total to account for filtered out items };
📜 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 (6)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.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/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.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/omnichannel/contactHistory/MessageList/useHistoryMessageList.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/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
🧬 Code graph analysis (4)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (5)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(316-320)packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (2)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
useCannedResponseList(7-59)
⏰ 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 (14)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
1-10: LGTM!Imports are appropriate for the React Query-based approach and align with the broader refactoring pattern in this PR.
12-17: Well-structured helper functions.The type guards and comparison function are cleanly extracted, making the filtering and sorting logic testable and readable.
67-76: Pagination logic is sound with acknowledged limitations.The offset-based pagination via
getNextPageParamcorrectly computes the next offset from loaded items count. Theselecttransformer properly flattens pages and extracts the latestitemCount.The FIXME at lines 72-73 appropriately documents the inherent limitation with offset-based pagination when items can be created/removed mid-pagination. The suggested improvement (cursor-based pagination using
createdAt/updatedAt) would be a more robust long-term solution but is out of scope for this refactor.apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (2)
67-67: LGTM: Correctly using React Query'srefetch.The migration from
reloadtorefetchis appropriate for React Query.
72-85: LGTM: Props correctly mapped to React Query hook return values.The prop mappings align with the hook's return shape:
loadMoreItems={fetchNextPage}— correct for infinite queriescannedItems={data?.cannedItems ?? []}— appropriate fallback for undefined dataitemCount={data?.total ?? 0}— appropriate fallback for undefined datareload={refetch}— correct for manual re-fetchingapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx (1)
41-43: Rendering logic looks good.The inclusion of
isIdlein the loading state check addresses the initial render concern from previous reviews. The conditional rendering correctly handles all mutation states.Note: Line 42 assumes
errorhas a.messageproperty, which will be correct once the error handler fix (lines 18-20) is applied.apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (3)
8-18: LGTM!Well-structured generic hook signature with proper type constraints. The type guard pattern for
filterensures type safety when narrowingIMessagetoT.
46-47: Edge case whenpageSizefalls back toitems.length.If
lastPage.items.lengthis 0 (e.g., on first mutation before any data is loaded) but items exist after mutation, usingitems.lengthaspageSizecreates a single page containing all items. This works but could produce unexpectedly large pages. Since initial empty state is rare and the behavior is safe, this is acceptable.
102-107: LGTM!Clean subscription cleanup and appropriate dependency tracking. The
useEffectEventwrappers ensure stable references while accessing fresh values.apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (2)
15-31: LGTM!Clean setup with proper query key including
searchTermfor cache isolation. The filter and compare functions are appropriately simple.
53-64: Pagination estimation acknowledged.The FIXME comment correctly identifies that the offset-based pagination may drift if messages are created or deleted during pagination. Since the server handles
searchTermfiltering, client-side filtering should rarely remove items, minimizing pagination issues.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (3)
12-46: LGTM!Well-designed discriminated union for options and clean helper predicates. The compare function correctly prioritizes
tlm(thread last message time) with fallback tots.
66-70: Client-sidetunreadcheck may diverge from server.The
isThreadUnreadcheck at lines 66-70 uses the subscription'stunreadarray passed from the client, while the server'stype: 'unread'filter may have slightly different criteria or timing. If they diverge, some threads could be filtered out client-side that weren't filtered server-side, affecting pagination offset accuracy.This is likely rare in practice, and the FIXME at line 112-113 acknowledges the estimation limitation.
100-108: LGTM!The mapping, filtering, and sorting pipeline is correct. The
itemCountadjustment properly accounts for any items removed by client-side filtering.
tassoevan
force-pushed
the
chore/async-state
branch
from
f5d93bf to
493cfc4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
47-50: Acknowledged: Offset-based pagination has known limitations.The FIXME correctly identifies that offset-based pagination can become inconsistent when items are created or removed during pagination. This is a known limitation but acceptable for current UX requirements.
For future improvement, consider cursor-based pagination using
ts,_id, or server-returned continuation tokens to avoid offset drift.apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
58-61: Acknowledged: Offset-based pagination has known limitations.The FIXME correctly identifies that offset-based pagination can become inconsistent when discussions are created or removed during pagination. This is a known limitation but acceptable for current UX requirements.
For future improvement, consider cursor-based pagination using
ts,_id, or server-returned continuation tokens to avoid offset drift.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
73-78: Optional: Consider memoizing regex creation.The regex is recreated on every message filter (line 74). While
useEffectEventmemoizes the filter function, the regex itself could be pre-computed:const textRegex = text ? new RegExp(escapeRegExp(text), 'i') : null; // Later in filter: if (textRegex && !isThreadTextMatching(message, textRegex)) { return false; }This is a minor optimization and can be deferred.
📜 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 (4)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.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/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
🧠 Learnings (3)
📚 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/omnichannel/contactHistory/MessageList/useHistoryMessageList.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/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts
🧬 Code graph analysis (3)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (3)
apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(51-97)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (6)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(316-320)apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
packages/core-typings/src/IMessage/IMessage.ts (1)
IDiscussionMessage(329-333)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
🪛 ast-grep (0.40.3)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
[warning] 24-24: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(searchTerm, 'ig')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (2)
28-66: LGTM: mutateQueryData correctly maintains itemCount consistency.The mutation logic properly adjusts
itemCountbased on the delta between before and after mutation (line 52), ensuring the total count reflects real-time additions and removals. This addresses the previous concern about stale counts.
70-107: LGTM: Real-time subscription handling is robust.The effect correctly:
- Gates on
userIdandroomIdavailability- Handles message updates (update existing or add+sort)
- Handles individual and bulk deletions appropriately
- Cleans up all subscriptions on unmount
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
21-39: LGTM: Proper RegExp sanitization with escapeRegExp.The filter correctly uses
escapeRegExp(line 31) to sanitize thetextparameter before constructing the RegExp, preventing ReDoS vulnerabilities. This is the correct pattern that should be applied consistently across similar hooks.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
86-113: Pagination and query implementation looks correct.The refactoring successfully addresses the previous review concerns:
- Query key now includes
{ type, text }(lines 87, 53) ✓- Pagination offset is based on actual fetched items without client-side filtering in
queryFn(lines 98, 106) ✓itemCountuses server'stotaldirectly without adjustment (line 99) ✓The FIXME comment (lines 104-105) appropriately acknowledges the pagination estimation limitation.
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
19-19: Optional: Simplify the count parsing.The template literal wrapper is unnecessary. Consider simplifying to:
const count = Number(getConfig('historyMessageListSize', 10)) || 10;
📜 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 (2)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.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/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
🧠 Learnings (6)
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/room/contextualBar/Threads/hooks/useThreadsList.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/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.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/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts
🧬 Code graph analysis (1)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (2)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
omnichannelQueryKeys(51-97)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (3)
1-9: LGTM: Imports support the migration to react-query.The addition of
escapeRegExp(line 2) addresses the previous ReDoS security concern. The new react-query imports and supporting utilities are appropriate for the refactored implementation.
21-30: Excellent: ReDoS vulnerability resolved.The real-time updates hook is correctly configured. Line 27 now properly escapes
searchTermusingescapeRegExpbefore constructing the RegExp, which addresses the security concern from the previous review. The filter and sort logic correctly replicate server-side behavior.
32-58: Approve: Infinite query implementation follows best practices.The useInfiniteQuery setup is correct:
- Query key consistency ensures real-time updates work (lines 22 & 33)
- Sort order is consistent between client and server (ascending by timestamp)
- Data transformation and flattening logic is appropriate
The FIXME comment (lines 49-50) identifies a known limitation with offset-based pagination during real-time updates. While cursor-based pagination would be more robust, the current approach is functional and the developer is aware of the trade-off.
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (2)
58-62: Verify userId availability when type is 'following'.When
type === 'following'butuserIdis undefined (e.g., during session expiration), the follower check is skipped and the filter returnstruefor all messages (line 77). This could cause real-time updates to incorrectly include all threads instead of just followed ones.Confirm that your auth model guarantees
userIdis always defined when viewing threads, or add an explicit guard:if (type === 'following') { if (!userId) { return false; } if (!isThreadFollowedByUser(message, userId)) { return false; } }
42-110: Solid refactoring to React Query patterns.The migration from AsyncState to
useInfiniteQueryis well-structured:
- Server-side pagination with offset-based
getNextPageParam- Consistent query keys between initial fetch and real-time updates
- Clean separation: server filtering in
queryFn, client-side replication inuseInfiniteMessageQueryUpdatesfor real-time messages- Proper type safety with discriminated union for options
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (2)
14-14: Simplify the config parsing.The template literal wrapping
getConfigbefore parsing is verbose. Consider simplifying to make the intent clearer.🔎 Proposed simplification
- const count = parseInt(`${getConfig('discussionListSize', 10)}`, 10); + const count = Number(getConfig('discussionListSize', 10));
52-57: Consider tracking the pagination estimation issue.The FIXME correctly identifies that offset-based pagination can be unreliable when discussions are created or removed during pagination. This can lead to duplicate or missing items.
Consider one of the following approaches to resolve this:
- Server-side cursor support: Modify the API to return a cursor (e.g., the
_idortsof the last item) that can be used for consistent pagination.- Timestamp-based pagination: Use
createdAtorupdatedAtfields for pagination boundaries instead of offsets.- Track this technical debt: If immediate changes aren't feasible, create a follow-up issue to address this limitation.
Would you like me to open a new issue to track this pagination improvement, or help draft an API enhancement proposal?
📜 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 (2)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.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/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
🧠 Learnings (5)
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/room/contextualBar/Threads/hooks/useThreadsList.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/room/contextualBar/Threads/hooks/useThreadsList.ts
🧬 Code graph analysis (2)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (4)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(316-320)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)packages/core-typings/src/IMessage/IMessage.ts (1)
IDiscussionMessage(329-333)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (3)
11-29: LGTM on the type definition cleanup.The discriminated union is well-structured, and the previous issue with the unused
uidparameter has been properly addressed by removing it entirely from all variants.
38-70: Well-structured real-time update filtering.The previous issues have been properly addressed:
- Query key now includes
{ type, text }for proper cache invalidation- The
userIdcheck on line 48 prevents unsafe access when undefinedThe
tunreadarray is intentionally excluded from the query key since it changes frequently (as users read threads), anduseInfiniteMessageQueryUpdateshandles real-time updates appropriately.
72-99: Clean migration to React Query infinite pagination.The previous pagination issues have been resolved by:
- Removing client-side filtering from
queryFn- server now handles filtering viatypeandtextparams- Using raw
totalforitemCountwithout adjustment- Query key matching between
useInfiniteQueryanduseInfiniteMessageQueryUpdatesThe FIXME comment (lines 90-91) appropriately documents the known limitation with offset-based pagination estimation.
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
1-11: LGTM: Clean migration to React Query pattern.The hook signature is well-typed and the imports are appropriate for the new implementation using
useInfiniteQueryanduseInfiniteMessageQueryUpdates.
58-62: LGTM: Efficient data transformation with select.The
selectfunction efficiently flattens pages into a single array and extracts the total count from the last page. Using.at(-1)is a clean, modern approach for accessing the last element.
16-34: The client-side filter correctly replicates the server-side filtering logic. Both implementations check fordridexistence and apply the same case-insensitive regex matching usingescapeRegExp()for text filtering. The behavior is consistent when text is empty (both include all discussions) and when text is provided (both apply regex matching).
tassoevan
force-pushed
the
chore/async-state
branch
from
5310582 to
1c43aa8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
14-14: Simplify redundant template literal.The template literal wrapping is unnecessary before
parseInt. You can simplify this to:const count = parseInt(getConfig('discussionListSize', 10), 10);🔎 Proposed simplification
- const count = parseInt(`${getConfig('discussionListSize', 10)}`, 10); + const count = parseInt(getConfig('discussionListSize', 10), 10);apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (1)
72-99: LGTM! Infinite query implementation is correct.The pagination logic properly calculates offsets from loaded items (past issue resolved per 493cfc4), and the query key correctly includes filter parameters (past issue resolved per f8e2128). The FIXME comment appropriately documents the estimation limitation.
The type assertion on line 84 (
as IThreadMainMessage[]) assumes the API contract is correct. Consider adding runtime validation if API consistency is a concern, but this pattern is standard and acceptable for typed APIs.
📜 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 (2)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.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/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
🧠 Learnings (5)
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/room/contextualBar/Threads/hooks/useThreadsList.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/room/contextualBar/Threads/hooks/useThreadsList.ts
🧬 Code graph analysis (2)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (5)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)packages/ui-contexts/src/hooks/useUserId.ts (1)
useUserId(5-5)apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)packages/core-typings/src/IMessage/IMessage.ts (1)
IThreadMainMessage(316-320)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-34)packages/core-typings/src/IMessage/IMessage.ts (1)
IDiscussionMessage(329-333)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (7)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (3)
1-11: LGTM! Clean migration to React Query.The simplified hook signature and migration to
useInfiniteQueryalign well with the PR objectives to phase out the custom AsyncState pattern. The parameter structure is clear and type-safe.
16-34: Well-implemented real-time updates filter.The filter logic correctly:
- Type-guards for
IDiscussionMessageby checking thedridproperty- Escapes the text parameter to prevent regex injection
- Mirrors server-side filtering to maintain data consistency
52-56: Pagination limitation acknowledged.The FIXME comment correctly identifies that the offset-based pagination can be inaccurate when discussions are created or removed during pagination. The suggested approach of using timestamps (
createdAtorupdatedAt) would provide more stable pagination.This is a known trade-off with the current implementation. Consider prioritizing the server-side pagination improvement if stale data issues are observed in practice.
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts (4)
1-9: LGTM! Clean imports for React Query migration.The imports properly support the refactored infinite-query pattern with real-time updates.
11-29: LGTM! Type definition correctly models filter variants.The discriminated union properly enforces that
tunreadis required only for the 'unread' type, and the pastuidparameter issue has been resolved.
31-36: LGTM! Hook initialization is correct.The count parsing safely handles the config value, and
userIdis properly sourced from context.
38-70: LGTM! Real-time updates and filtering are well-implemented.The filter properly replicates server-side logic with correct handling of all edge cases (undefined
userId, text escaping, type guards). Past issues with query keys and undefineduserIdhave been resolved.
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts
Show resolved
Hide resolved
…geList`, `useThreadsList`, `useDiscussionsList`
tassoevan
force-pushed
the
chore/async-state
branch
from
760d127 to
ebff6b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsx (1)
50-58: Caching does not work forundefinedvalues.When
extractSnapshotreturnsundefined(icon not found), it's stored in the map. On subsequent calls,snapshots.get()returnsundefined, which fails the!snapshotcheck, causing repeated extractions. UseMap.has()to properly distinguish between "not cached" and "cached as undefined".🐛 Proposed fix
// No problem here, because it's return value is a cached in the snapshots map on subsequent calls () => { - let snapshot = snapshots.get(`${app}-${iconName}`); + const key = `${app}-${iconName}`; - if (!snapshot) { - snapshot = extractSnapshot(app, iconName); - snapshots.set(`${app}-${iconName}`, snapshot); + if (!snapshots.has(key)) { + snapshots.set(key, extractSnapshot(app, iconName)); } - return snapshot; + return snapshots.get(key); },apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
37-42: Type mismatch:departmentNameis required in callback but optional in data.The
onClickItemcallback expectsdepartmentName: ILivechatDepartment['name'](required), butcannedItemsfromuseCannedResponseListhavedepartmentName?: ILivechatDepartment['name'](optional). This could cause type errors when clicking items without a department.🔧 Suggested fix
const onClickItem = useEffectEvent( ( data: IOmnichannelCannedResponse & { - departmentName: ILivechatDepartment['name']; + departmentName?: ILivechatDepartment['name']; }, ) => {apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx (1)
78-104: Options object includesuidwhich is not inThreadsListOptions.The options object on lines 91-93 includes
uidfor the'following'type, but looking atThreadsListOptionsinuseThreadsList.ts, the'following'variant doesn't accept auidproperty. The hook usesuseUserId()internally instead.🔧 Suggested fix: Remove uid from options
case 'following': return { rid, text, type, - uid, };
🤖 Fix all issues with AI agents
In
@apps/meteor/client/components/message/content/location/hooks/useAsyncImage.ts:
- Around line 17-19: In useAsyncImage's image error handler replace the invalid
e.error usage (image.addEventListener('error', (e) => { reject(e.error); }))
with rejecting a real Error that includes useful context; e.g., get the failing
element via e.target (or image) and call reject(new Error(`Image failed to load:
${image.src || (e.target as HTMLImageElement)?.src || 'unknown'}`)) so the
promise receives a meaningful error message.
In @apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts:
- Around line 75-86: The code is casting message to T before calling the type
guard, which defeats the guard; update the subscribeToRoomMessages callback to
call doFilter(message) with the original message (no cast) so the guard can
narrow its type, and keep the existing casts inside mutateQueryData
(items[index] = message as T and items.push(message as T)) since those are used
after the guard and within useEffectEvent context; target the callback passed to
subscribeToRoomMessages and adjust the doFilter invocation accordingly, leaving
mutateQueryData and doCompare usage unchanged.
In @apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx:
- Line 168: The translation key construction t(`Apps_context_${context}`) in
AppsPageContent can produce a bogus key when context is undefined; update the
code to use the same fallback pattern as AppsFilters (use the existing
AppsFilters fallback value when context is falsy) so the t(...) call uses a
valid default (e.g., replace the direct template with one that falls back to the
AppsFilters default when context is undefined); apply the same change for the
other two occurrences of the template translation call in this file (the ones
also building `Apps_context_${context}`).
In @apps/meteor/client/views/marketplace/hooks/useApps.ts:
- Around line 161-162: The query key is unstable because it includes object
references (instance.data, marketplace.data); change the useQuery key to use
stable primitives instead (e.g. instance.data?.id or instanceId and
marketplace.data?.id or marketplaceUpdatedAt) or a single derived token like
marketplaceQueryKeys.appsStored(instanceId, marketplaceUpdatedAt) so the key
only changes when meaningful identifiers/timestamps change; update the call site
that constructs the key (marketplaceQueryKeys.appsStored and the useQuery
invocation) to accept and pass those stable values rather than the full data
objects.
In @apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts:
- Around line 61-76: The sortingMethods map uses Array.prototype.toSorted (in
entries like the urf/url/az/za/mru/lru handlers) which isn't
transpiled/polyfilled for our targets; replace uses of toSorted with a
compatible pattern such as cloning the array (e.g., using spread or slice) and
then calling .sort(...) with the same comparator to avoid mutating the original
array, and remove or actually import the array.prototype.tosorted polyfill if
you prefer polyfilling instead; update only the sortingFunctions inside
useFilteredApps.ts (the sortingMethods object) so each value clones apps then
sorts with the existing comparator logic.
In @apps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.ts:
- Line 3: The hook usePrivateAppsEnabled currently returns false when license
data is undefined, causing UI flicker during loading; change it to explicitly
check useLicense({ loadValues: true }).isPending and return null (or undefined)
while isPending is true so consumers can treat loading distinctly from a
definitive false, update the return type to boolean | null, and update any
consumers/tests (e.g., PrivateEmptyState, MarketplaceHeader.handleClickPrivate,
UninstallGrandfatheredAppModal) to handle the null loading state and adjust
tests to mock isPending: true where appropriate.
In @apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts:
- Line 84: The current cast "as IThreadMainMessage[]" hides potential API
mismatches because mapMessageFromApi returns IMessage; replace the unsafe
assertion by adding a runtime type guard (e.g., isThreadMainMessage) that checks
required IThreadMainMessage properties and use it to filter/transform the mapped
results (threads.map(mapMessageFromApi).filter(isThreadMainMessage)) before
assigning to items, or perform the validation/transformation immediately after
fetching /v1/chat.getThreadsList so the rest of the code can safely rely on a
type-safe IThreadMainMessage array; do not use the direct "as
IThreadMainMessage[]" assertion.
🧹 Nitpick comments (17)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (2)
28-30: Consider extracting department fetching to a separate cached query.The
getDepartmentscall insidequeryFnexecutes on every page fetch, causing redundant network requests. Since departments rarely change during pagination, consider using a separateuseQuerywith caching:♻️ Suggested approach
+import { useQuery, useInfiniteQuery } from '@tanstack/react-query'; + export const useCannedResponseList = ({ filter, type }: { filter: string; type: string }) => { const getCannedResponses = useEndpoint('GET', '/v1/canned-responses'); const getDepartments = useEndpoint('GET', '/v1/livechat/department'); + const { data: departmentsData } = useQuery({ + queryKey: ['livechat', 'departments'], + queryFn: () => getDepartments({ text: '' }), + staleTime: 5 * 60 * 1000, + }); + const count = 25; return useInfiniteQuery({ queryKey: cannedResponsesQueryKeys.list({ filter, type }), queryFn: async ({ pageParam: offset }) => { const { cannedResponses, total } = await getCannedResponses({ // ... existing params }); - // TODO: Optimize by creating an endpoint that returns canned responses with department names - // TODO: Use another query for departments to avoid refetching - const { departments } = await getDepartments({ text: '' }); + const departments = departmentsData?.departments ?? []; return { // ... rest unchanged }; }, + enabled: !!departmentsData, // ... rest unchanged }); };
17-23: Consider usingincludes()for clearer membership checks.The current approach using
.find()works butincludes()is more idiomatic for simple membership tests.♻️ Suggested refinement
- ...(type && ['global', 'user'].find((option) => option === type) && { scope: type }), - ...(type && - !['global', 'user', 'all'].find((option) => option === type) && { + ...(type && ['global', 'user'].includes(type) && { scope: type }), + ...(type && + !['global', 'user', 'all'].includes(type) && { scope: 'department', departmentId: type, }),apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (2)
10-11: Potential runtime error whenroomis undefined.If
useUserRoom(rid)returnsundefined(e.g., when the room is not yet loaded or the user doesn't have access),room?.tfalls back to'c', but the query will still execute and potentially fail or return unexpected results.Consider adding a check to disable the query when the room is not available:
♻️ Suggested improvement
export const useFilesList = ({ rid, type, text }: { rid: Required<IUpload>['rid']; type: string; text: string }) => { const room = useUserRoom(rid); // ... roomTypes definition ... const getFiles = useEndpoint('GET', roomTypes[room?.t ?? 'c']); const count = parseInt(`${getConfig('discussionListSize', 10)}`, 10); return useInfiniteQuery({ queryKey: roomsQueryKeys.files(rid, { type, text }), + enabled: !!room, queryFn: async ({ pageParam: offset }) => {
46-62: Consider using a regularfor...ofloop instead offor await.The
itemsarray is synchronous; usingfor awaiton it is valid but semantically misleading. Theawaitcalls inside the loop would work the same with a standardfor...ofloop.♻️ Suggested change
- for await (const file of items) { + for (const file of items) { if (file.rid && file.content) { const e2eRoom = await e2e.getInstanceByRoomId(file.rid);apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx (1)
30-41: Consider exposingisFetchingNextPagefor pagination loading state.Currently,
loading={isPending}only shows the loading indicator during the initial fetch. If you want to provide visual feedback while loading additional pages (e.g., a footer spinner in the list), you could also destructure and passisFetchingNextPageto the component.This is optional since the current UX may be intentional for seamless infinite scrolling.
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
21-30: Consider memoizing the regex for the filter callback.The filter callback creates a new
RegExpobject for each message evaluated. While the impact is likely minimal for update-based filtering, moving the regex creation outside the callback would be a slight performance improvement.♻️ Optional optimization
+ const searchRegex = searchTerm ? new RegExp(escapeRegExp(searchTerm), 'ig') : null; + useInfiniteMessageQueryUpdates({ queryKey: omnichannelQueryKeys.contactMessages(roomId, { searchTerm }), roomId, - // Replicates the filtering done server-side filter: (message): message is IMessage => (!('t' in message) || message.t === 'livechat-close') && - (!searchTerm || new RegExp(escapeRegExp(searchTerm), 'ig').test(message.msg)), - // Replicates the sorting forced on server-side + (!searchRegex || searchRegex.test(message.msg)), compare: (a, b) => a.ts.getTime() - b.ts.getTime(), });apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (1)
63-66: Consider destructuringhasNextPageandisFetchingNextPagefor better UX.The hook returns additional pagination state that could enhance the user experience—
hasNextPageto guardfetchNextPagecalls, andisFetchingNextPageto show a loading indicator at the list's end while fetching more items.♻️ Optional enhancement
- const { isPending, error, isSuccess, data, fetchNextPage } = useHistoryMessageList(query); + const { isPending, error, isSuccess, data, fetchNextPage, hasNextPage, isFetchingNextPage } = useHistoryMessageList(query);Then update the
endReachedcallback:- endReached={() => fetchNextPage()} + endReached={() => hasNextPage && !isFetchingNextPage && fetchNextPage()}apps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx (2)
48-51: Consider destructuringisFetchingNextPagefor better UX.When fetching additional pages, users have no visual feedback that more messages are loading. Destructuring
isFetchingNextPagefrom the query result would allow displaying a loading indicator at the bottom of the list during pagination.
110-117: Renamedataparameter to avoid variable shadowing.The
dataparameter shadows the outerdatafrom the query result (line 48), which can be confusing during maintenance. Consider renaming tomessagefor clarity and consistency with the type.Suggested change
- itemContent={(index, data) => { + itemContent={(index, message) => { const lastMessage = messages[index - 1]; - const isSequential = isMessageSequential(data, lastMessage, messageGroupingPeriod); - const isNewDay = isMessageNewDay(data, lastMessage); + const isSequential = isMessageSequential(message, lastMessage, messageGroupingPeriod); + const isNewDay = isMessageNewDay(message, lastMessage); return ( - <ContactHistoryMessage message={data} sequential={isSequential} isNewDay={isNewDay} showUserAvatar={showUserAvatar} /> + <ContactHistoryMessage message={message} sequential={isSequential} isNewDay={isNewDay} showUserAvatar={showUserAvatar} /> ); }}apps/meteor/client/providers/AppsProvider.tsx (1)
10-12: Avoid passing children using a prop.The static analysis tool correctly identifies that
childrenshould be passed as JSX children rather than as a prop. This is the canonical React pattern.♻️ Proposed fix
const AppsProvider = ({ children }: AppsProviderProps) => { - return <AppsContext.Provider children={children} value={AppClientOrchestratorInstance} />; + return <AppsContext.Provider value={AppClientOrchestratorInstance}>{children}</AppsContext.Provider>; };apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx (1)
26-29: Consider extracting duplicated appRoot setup.The
appRootconfiguration with translations is identical in both describe blocks. This could be extracted to a module-level constant.♻️ Suggested refactor
+const appRoot = mockAppRoot().withTranslations('en', 'core', { + Private_apps_upgrade_empty_state_title: 'Upgrade to unlock private apps', + No_private_apps_installed: 'No private apps installed', +}); + describe('with private apps enabled', () => { beforeEach(async () => { // ...mock setup }); - const appRoot = mockAppRoot().withTranslations('en', 'core', { - Private_apps_upgrade_empty_state_title: 'Upgrade to unlock private apps', - No_private_apps_installed: 'No private apps installed', - }); - it('should offer to upgrade to unlock private apps', () => { // ... }); }); describe('without private apps enabled', () => { beforeEach(async () => { // ...mock setup }); - const appRoot = mockAppRoot().withTranslations('en', 'core', { - Private_apps_upgrade_empty_state_title: 'Upgrade to unlock private apps', - No_private_apps_installed: 'No private apps installed', - }); - it('should offer to upgrade to unlock private apps', () => { // ... }); });Also applies to: 53-56
apps/meteor/client/lib/queryKeys.ts (1)
165-189: New marketplace and video conference query keys look good.The structure is consistent with other query key objects. Minor observation:
appsStoredon line 169 uses...args: unknown[]which is looser than other keys. Consider typing the args more specifically if the expected parameters are known.apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
39-41: Redundant optional chaining.The outer condition already ensures
usersWaitingForE2EKeysis truthy, so the inner?.on line 40 is unnecessary.Suggested fix
...(usersWaitingForE2EKeys && { - usersWaitingForE2EKeys: usersWaitingForE2EKeys?.map(({ userId, ts }) => ({ userId, ts: new Date(ts) })), + usersWaitingForE2EKeys: usersWaitingForE2EKeys.map(({ userId, ts }) => ({ userId, ts: new Date(ts) })), }),apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (1)
30-46: Consider error handling for E2E decryption failures.If
e2e.decryptFileContent(file)throws, the entire query will fail. Consider whether individual file decryption failures should be handled gracefully (e.g., marking the file as undecryptable) rather than failing the whole page fetch.💡 Optional: Graceful decryption error handling
for await (const file of items) { if (file.rid && file.content) { - const e2eRoom = await e2e.getInstanceByRoomId(file.rid); - if (e2eRoom?.shouldConvertReceivedMessages()) { - const decrypted = await e2e.decryptFileContent(file); - const key = Base64.encode( - JSON.stringify({ - ...decrypted.encryption, - name: String.fromCharCode(...new TextEncoder().encode(decrypted.name)), - type: decrypted.type, - }), - ); - decrypted.path = `/file-decrypt${decrypted.path}?key=${key}`; - Object.assign(file, decrypted); + try { + const e2eRoom = await e2e.getInstanceByRoomId(file.rid); + if (e2eRoom?.shouldConvertReceivedMessages()) { + const decrypted = await e2e.decryptFileContent(file); + const key = Base64.encode( + JSON.stringify({ + ...decrypted.encryption, + name: String.fromCharCode(...new TextEncoder().encode(decrypted.name)), + type: decrypted.type, + }), + ); + decrypted.path = `/file-decrypt${decrypted.path}?key=${key}`; + Object.assign(file, decrypted); + } + } catch { + // File remains encrypted; consumer should handle display } } }apps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsx (1)
162-162:endReachedshould check for more pages before fetching.Calling
fetchNextPage()unconditionally on everyendReachedevent may trigger unnecessary network requests when there are no more pages. React Query'shasNextPagecan guard this.♻️ Suggested improvement
-const { isPending, error, isSuccess, data, fetchNextPage } = useThreadsList(options); +const { isPending, error, isSuccess, data, fetchNextPage, hasNextPage } = useThreadsList(options); ... -endReached={() => fetchNextPage()} +endReached={() => hasNextPage && fetchNextPage()}apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (1)
14-14: Simplify the parseInt call.The template string wrapping is unnecessary.
getConfigreturnsstring | number, so you can simplify this:Suggested simplification
- const count = parseInt(`${getConfig('discussionListSize', 10)}`, 10); + const count = parseInt(String(getConfig('discussionListSize', 10)), 10);Or if the config always returns a string:
- const count = parseInt(`${getConfig('discussionListSize', 10)}`, 10); + const count = Number(getConfig('discussionListSize', 10)) || 10;apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (1)
85-90: Missing dependency in useEffect.The effect depends on
isRequestedbutcreateFilterStructureItems()internally uses thettranslation function. Whiletis typically stable, ESLint's exhaustive-deps rule would flag this.Suggested fix
useEffect(() => { setSortFilterStructure({ label: t('Sort_By'), items: createFilterStructureItems(), }); - }, [isRequested]); + }, [isRequested, t]);
📜 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 (79)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/components/message/content/location/MapView.tsxapps/meteor/client/components/message/content/location/hooks/useAsyncImage.tsapps/meteor/client/contexts/AppsContext.tsxapps/meteor/client/contexts/hooks/useAppsReload.tsapps/meteor/client/contexts/hooks/useAppsResult.tsapps/meteor/client/hooks/lists/useRecordList.tsapps/meteor/client/hooks/lists/useScrollableMessageList.tsapps/meteor/client/hooks/lists/useScrollableRecordList.tsapps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.tsapps/meteor/client/hooks/useAsyncState.tsapps/meteor/client/hooks/useComponentDidUpdate.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/lib/asyncState/AsyncState.tsapps/meteor/client/lib/asyncState/AsyncStatePhase.tsapps/meteor/client/lib/asyncState/functions.tsapps/meteor/client/lib/asyncState/index.tsapps/meteor/client/lib/lists/CannedResponseList.tsapps/meteor/client/lib/lists/DiscussionsList.tsapps/meteor/client/lib/lists/FilesList.tsapps/meteor/client/lib/lists/ImagesList.tsapps/meteor/client/lib/lists/MessageList.tsapps/meteor/client/lib/lists/RecordList.spec.tsapps/meteor/client/lib/lists/RecordList.tsapps/meteor/client/lib/lists/ThreadsList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/providers/AppsProvider.tsxapps/meteor/client/providers/AppsProvider/AppsProvider.tsxapps/meteor/client/providers/AppsProvider/index.tsapps/meteor/client/providers/AppsProvider/storeQueryFunction.tsapps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsxapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useAppInfo.tsapps/meteor/client/views/marketplace/hooks/useAppInstances.tsapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/hooks/useAppsOrchestration.tsapps/meteor/client/views/marketplace/hooks/useAppsReload.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/marketplace/hooks/useInstallApp.tsxapps/meteor/client/views/marketplace/hooks/useLogs.tsapps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponse.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/Item.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponse.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsxapps/meteor/client/views/omnichannel/cannedResponses/modals/CreateCannedResponse/CreateCannedResponseModal.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/omnichannel/hooks/useScopeDict.tsapps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsxapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFiles.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfList.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsxapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfRecordList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsxapps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.tsapps/meteor/tests/mocks/client/marketplace.tsx
💤 Files with no reviewable changes (24)
- apps/meteor/client/contexts/hooks/useAppsResult.ts
- apps/meteor/client/hooks/lists/useScrollableMessageList.ts
- apps/meteor/client/providers/AppsProvider/storeQueryFunction.ts
- apps/meteor/client/lib/lists/DiscussionsList.ts
- apps/meteor/client/providers/AppsProvider/index.ts
- apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts
- apps/meteor/client/lib/asyncState/AsyncState.ts
- apps/meteor/client/lib/lists/ImagesList.ts
- apps/meteor/client/lib/lists/RecordList.spec.ts
- apps/meteor/client/hooks/useComponentDidUpdate.ts
- apps/meteor/client/lib/asyncState/AsyncStatePhase.ts
- apps/meteor/client/lib/lists/RecordList.ts
- apps/meteor/client/lib/lists/CannedResponseList.ts
- apps/meteor/client/lib/asyncState/index.ts
- apps/meteor/client/contexts/hooks/useAppsReload.ts
- apps/meteor/client/hooks/lists/useScrollableRecordList.ts
- apps/meteor/client/providers/AppsProvider/AppsProvider.tsx
- apps/meteor/client/hooks/lists/useRecordList.ts
- apps/meteor/client/lib/asyncState/functions.ts
- apps/meteor/client/lib/lists/MessageList.ts
- apps/meteor/client/lib/lists/FilesList.ts
- apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfRecordList.ts
- apps/meteor/client/hooks/useAsyncState.ts
- apps/meteor/client/lib/lists/ThreadsList.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- apps/meteor/client/views/marketplace/hooks/useInstallApp.tsx
- apps/meteor/ee/server/apps/marketplace/fetchMarketplaceApps.ts
- apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponse.tsx
- apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponse.tsx
- apps/meteor/client/components/message/content/location/MapView.tsx
- apps/meteor/client/views/mailer/MailerUnsubscriptionPage.tsx
- apps/meteor/client/views/marketplace/hooks/useAppInfo.ts
- apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsx
- apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfListWithData.tsx
- apps/meteor/client/views/marketplace/hooks/useAppsReload.ts
- apps/meteor/client/views/marketplace/hooks/useAppsOrchestration.ts
- apps/meteor/client/views/omnichannel/cannedResponses/modals/CreateCannedResponse/CreateCannedResponseModal.tsx
- apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsx
- apps/meteor/client/views/room/ImageGallery/ImageGalleryData.tsx
- apps/meteor/client/views/marketplace/hooks/useAppInstances.ts
- apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/Item.tsx
- apps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppRequests/AppRequests.tsx
🧰 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:
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/hooks/useRoomsList.tsapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/omnichannel/hooks/useScopeDict.tsapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsxapps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsxapps/meteor/client/views/marketplace/hooks/usePrivateAppsEnabled.tsapps/meteor/client/views/room/contextualBar/RoomFiles/RoomFiles.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/providers/AppsProvider.tsxapps/meteor/client/views/room/contextualBar/Threads/ThreadList.tsxapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsxapps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/marketplace/hooks/useLogs.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/VideoConfList.tsxapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsxapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/CannedResponseList.tsxapps/meteor/client/contexts/AppsContext.tsxapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsxapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.tsapps/meteor/client/components/message/content/location/hooks/useAsyncImage.tsapps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx
**/*.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:
apps/meteor/client/views/marketplace/hooks/useApps.spec.ts
🧠 Learnings (25)
📚 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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/provider/OmnichannelRoomIconProvider.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.ts
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsxapps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
📚 Learning: 2025-12-02T22:23:49.593Z
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.593Z
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/hooks/useRoomsList.tsapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/views/omnichannel/hooks/useCannedResponseList.tsapps/meteor/client/hooks/useInfiniteMessageQueryUpdates.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/marketplace/hooks/useFilteredApps.tsapps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.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/hooks/useRoomsList.tsapps/meteor/client/views/omnichannel/departments/EditDepartment.tsxapps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/client/views/marketplace/hooks/useApps.spec.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.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/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 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/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.tsapps/meteor/client/views/room/ImageGallery/hooks/useImagesList.tsapps/meteor/client/lib/queryKeys.ts
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsx
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/AppsPageContentBody.tsxapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/marketplace/hooks/useApps.tsapps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx
📚 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/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx
📚 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/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsxapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx
📚 Learning: 2025-11-07T14:50:33.544Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37423
File: packages/i18n/src/locales/en.i18n.json:18-18
Timestamp: 2025-11-07T14:50:33.544Z
Learning: Rocket.Chat settings: in apps/meteor/ee/server/settings/abac.ts, the Abac_Cache_Decision_Time_Seconds setting uses invalidValue: 0 as the fallback when ABAC is unlicensed. With a valid license, admins can still set the value to 0 to intentionally disable the ABAC decision cache.
Applied to files:
apps/meteor/client/views/marketplace/hooks/useApps.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx
🧬 Code graph analysis (21)
apps/meteor/client/views/marketplace/AppsPage/PrivateEmptyState.spec.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/client/hooks/useRoomsList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-38)
apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx (2)
packages/ui-contexts/src/index.ts (1)
useTranslation(81-81)apps/meteor/client/hooks/useRoomsList.ts (1)
useRoomsList(6-41)
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
teamsQueryKeys(125-134)
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx (1)
apps/meteor/client/views/room/contextualBar/RoomFiles/hooks/useFilesList.ts (1)
useFilesList(10-80)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/OmnichannelAppSourceRoomIcon.tsx (1)
apps/meteor/client/components/RoomIcon/OmnichannelRoomIcon/context/OmnichannelRoomIconContext.tsx (1)
useOmnichannelRoomIcon(11-15)
apps/meteor/client/views/room/ImageGallery/hooks/useImagesList.ts (2)
apps/meteor/client/meteor/minimongo/Cursor.ts (1)
count(277-283)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-38)
apps/meteor/client/providers/AppsProvider.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(29-29)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
cannedResponsesQueryKeys(45-48)
apps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/WrapCannedResponseList.tsx (1)
apps/meteor/client/views/omnichannel/hooks/useCannedResponseList.ts (1)
useCannedResponseList(7-59)
apps/meteor/client/views/omnichannel/contactInfo/tabs/ContactInfoHistory/ContactInfoHistoryMessages.tsx (2)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
useHistoryMessageList(16-59)packages/ui-client/src/components/Contextualbar/index.ts (1)
ContextualbarEmptyContent(34-34)
apps/meteor/client/views/marketplace/hooks/useLogs.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
marketplaceQueryKeys(165-184)
apps/meteor/tests/mocks/client/marketplace.tsx (1)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(29-29)
apps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.ts (2)
apps/meteor/client/hooks/useInfiniteMessageQueryUpdates.ts (1)
useInfiniteMessageQueryUpdates(8-108)apps/meteor/client/lib/queryKeys.ts (1)
roomsQueryKeys(13-38)
apps/meteor/client/views/teams/contextualBar/channels/TeamsChannels.tsx (1)
packages/livechat/src/store/index.tsx (1)
Dispatch(209-209)
apps/meteor/client/views/marketplace/AppsPage/AppsPageContent.tsx (3)
apps/meteor/client/contexts/AppsContext.tsx (1)
AppsContext(29-29)apps/meteor/client/views/marketplace/hooks/useAppsReload.ts (1)
useAppsReload(6-11)apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
useFilteredApps(15-130)
apps/meteor/client/views/marketplace/hooks/useFilteredApps.ts (1)
apps/meteor/client/views/marketplace/hooks/useApps.ts (1)
useApps(76-180)
apps/meteor/client/views/teams/contextualBar/channels/TeamsChannelsWithData.tsx (1)
apps/meteor/client/views/teams/contextualBar/channels/hooks/useTeamsChannelList.ts (1)
useTeamsChannelList(15-59)
apps/meteor/client/views/marketplace/AppsPage/UnsupportedEmptyState.spec.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/client/views/room/contextualBar/VideoConference/VideoConfList/useVideoConfList.ts (1)
apps/meteor/client/lib/queryKeys.ts (1)
videoConferenceQueryKeys(186-189)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/ContactHistoryMessagesList.tsx (2)
apps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.ts (1)
useHistoryMessageList(16-59)packages/ui-client/src/components/Contextualbar/index.ts (1)
ContextualbarEmptyContent(34-34)
🪛 Biome (2.1.2)
apps/meteor/client/providers/AppsProvider.tsx
[error] 11-11: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
apps/meteor/client/components/message/content/location/hooks/useAsyncImage.ts
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts
Show resolved
Hide resolved
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
This pull request refactors the way asynchronous state and record list management is handled across the codebase, removing custom hooks and types related to
AsyncStatein favor of more standard or modern approaches (such as React Query). The changes simplify the context and hooks used for apps, omnichannel room icons, and message lists, making the code easier to maintain and reducing complexity.Omnichannel Room Icon Refactor:
useOmnichannelRoomIconhook and related context now return a simplestring | undefinedfor icon data, instead of anAsyncStateobject. This change removes the need to track loading/error phases for icons, simplifying both the context and consumer components. [1] [2] [3] [4] [5] [6]Apps Context Simplification:
AppsContextand related hooks (useAppsReload,useAppsResult) have been refactored to remove theAsyncStatepattern and instead directly use the orchestrator orundefined. This streamlines how app data is accessed and removes unnecessary complexity. [1] [2] [3] [4]Record List and Message List Hook Removal:
useRecordList,useScrollableMessageList,useScrollableRecordList,useStreamUpdatesForMessageList) have been removed, indicating a shift away from the previous event-driven and phase-tracking approach for list updates. [1] [2] [3] [4]Async State Hook Deletion:
useAsyncStatehook and related types have been deleted, further reducing reliance on the custom async state management system in favor of more standard solutions.MapView and useAsyncImage Modernization:
useAsyncImagehook is now implemented using React Query, providing a more robust and standard way to handle asynchronous image loading. TheMapViewcomponent is updated to use this new hook. [1] [2] [3]These changes collectively modernize the codebase, reduce boilerplate, and make asynchronous data handling more consistent and maintainable.
Issue(s)
ARCH-1914
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.