Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughConsolidates unread and omnichannel badge rendering into new reusable components and a hook: adds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Potential focus areas:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 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 12:47 (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]
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]
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]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 18 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Fri, 12 Dec 2025 12:47:13 GMT |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
apps/meteor/client/sidebar/badges/UnreadBadge.tsx (1)
17-17: Avoidas anytype assertion for style prop.The
as anytype assertion bypasses TypeScript's type safety. The Badge component from Fuselage should accept valid style props without this cast.Consider one of these approaches:
- If Badge doesn't accept
styledirectly, use a wrapper with proper styling:- <Badge - role='status' - {...({ style: { display: 'inline-flex', flexShrink: 0 } } as any)} - variant={variant} + <Box display='inline-flex' flexShrink={0}> + <Badge + role='status' + variant={variant}
- Or verify that Badge accepts these style properties and remove the cast if it does.
apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx (1)
108-108: Remove unnecessary async from test functions.The test functions are marked as
asyncbut do not useawaitanywhere. This adds unnecessary complexity.Apply this pattern to all test functions:
-it('should return correct unread data for [Direct message unread]', async () => { +it('should return correct unread data for [Direct message unread]', () => {apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
98-100: Consider using highlightUnread from useUnreadDisplay.The
highlightedvariable is computed manually fromhideUnreadStatus,alert, andunread(line 100), butuseUnreadDisplaylikely returns ahighlightUnreadproperty that encapsulates this same logic (as seen in the test file at line 139, 203, etc.).Apply this diff to use the hook's output:
- const { unreadCount, unreadTitle, showUnread } = useUnreadDisplay(room); + const { unreadCount, unreadTitle, showUnread, highlightUnread } = useUnreadDisplay(room); - const highlighted = Boolean(!hideUnreadStatus && (alert || unread)); + const highlighted = highlightUnread;This eliminates duplicate logic and ensures consistency with other components using the same hook.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
apps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsx(3 hunks)apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx(4 hunks)apps/meteor/client/sidebar/badges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebar/badges/UnreadBadge.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx(1 hunks)apps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx(4 hunks)apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/UnreadBadge.tsx(1 hunks)apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts(1 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsx(4 hunks)apps/meteor/client/views/navigation/sidebar/badges/OmnichannelBadges.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts(1 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsx(3 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx(1 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx(1 hunks)apps/meteor/lib/getSubscriptionUnreadData.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/client/views/navigation/sidebar/badges/OmnichannelBadges.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/sidebar/badges/SidebarItemBadges.tsxapps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsxapps/meteor/client/sidebarv2/badges/UnreadBadge.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.tsxapps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/badges/UnreadBadge.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/sidebarv2/hooks/useUnreadDisplay.tsapps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsxapps/meteor/lib/getSubscriptionUnreadData.tsapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.tsapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsxapps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsxapps/meteor/client/sidebar/badges/OmnichannelBadges.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsxapps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsxapps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx
🧠 Learnings (15)
📓 Common learnings
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
📚 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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.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/sidebarv2/hooks/useUnreadDisplay.tsapps/meteor/lib/getSubscriptionUnreadData.tsapps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.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/navigation/sidebar/RoomList/SidebarItemWithData.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsx
📚 Learning: 2025-11-27T17:56:26.027Z
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.027Z
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/navigation/sidebar/RoomList/SidebarItemWithData.tsxapps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsx
🧬 Code graph analysis (12)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (2)
IRoom(21-95)isOmnichannelRoom(361-361)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
isOmnichannelRoom(361-361)
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
isOmnichannelRoom(361-361)
apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx (1)
apps/meteor/lib/getSubscriptionUnreadData.ts (2)
UnreadData(36-39)getSubscriptionUnreadData(41-64)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx (2)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)
apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx (3)
apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx (1)
useUnreadDisplay(6-10)
apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts (2)
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (1)
useUnreadDisplay(6-10)apps/meteor/lib/getSubscriptionUnreadData.ts (1)
UnreadData(36-39)
apps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsx (1)
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (1)
useUnreadDisplay(6-10)
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (2)
apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts (1)
useUnreadDisplay(6-10)apps/meteor/lib/getSubscriptionUnreadData.ts (1)
UnreadData(36-39)
apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx (3)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/client/lib/appLayout.tsx (1)
render(26-28)apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)
apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx (1)
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (1)
useUnreadDisplay(6-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (15)
apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx (1)
6-10: LGTM!The hook provides a clean abstraction for obtaining unread display data with i18n support.
apps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx (1)
16-16: LGTM! Clean refactoring to centralize badge rendering.The inline badge logic has been successfully replaced with the dedicated SidebarItemBadges component, improving code organization and reusability.
Also applies to: 96-96, 142-142
apps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsx (1)
13-13: LGTM! Consistent refactoring pattern.The changes successfully centralize badge rendering and optimize the destructuring to only extract needed fields from useUnreadDisplay.
Also applies to: 35-35, 76-76
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsx (1)
9-9: LGTM! Sidepanel badge rendering properly refactored.The changes align with the broader refactoring effort, replacing inline badge logic with the dedicated RoomSidePanelItemBadges component.
Also applies to: 69-69
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx (1)
1-24: LGTM!Test setup properly mocks dependencies and configures translations for badge rendering verification.
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx (1)
13-23: Verify intentional layout difference from non-v2 SidebarItemBadges.This implementation differs from
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx, which wraps the badges in<Margins inlineStart={8}>. Ensure this difference is intentional and that both sidebar variants achieve the desired spacing.apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
1-30: LGTM!Test setup and helper function are appropriate for unit testing badge rendering behavior.
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
9-12: Verify type consistency with sidebarv2 implementation.The sidebarv2 version (
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx) usesSubscriptionWithRoomwhile this version usesISubscription & IRoom. Ensure both type definitions are compatible and that the difference is intentional.apps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsx (1)
9-9: LGTM!The refactor successfully consolidates badge rendering logic into the shared
SidebarItemBadgescomponent, improving maintainability.Also applies to: 24-24, 36-36
apps/meteor/client/sidebar/badges/OmnichannelBadges.tsx (1)
8-8: LGTM!Converting to a default export aligns with the consumption pattern in
SidebarItemBadgesand maintains the same component behavior.Also applies to: 23-23
apps/meteor/lib/getSubscriptionUnreadData.ts (1)
36-39: LGTM!Exporting the
UnreadDatatype promotes consistency and reusability across hooks and components that handle unread display logic.apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts (1)
3-3: LGTM!Using the exported
UnreadDatatype improves type consistency and eliminates duplication across the codebase.Also applies to: 6-6
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (1)
1-10: LGTM: Clean type refactoring.The change from an inline
Picktype to the dedicatedUnreadDatatype improves maintainability and consistency across the codebase. The implementation correctly delegates togetSubscriptionUnreadData, maintaining the existing behavior while improving the type safety.apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx (1)
8-23: I encountered a technical issue accessing the repository directly. Since I cannot clone and search the codebase through shell scripts, I'm unable to definitively verify whether all imports ofOmnichannelBadgeshave been updated from named imports to default imports.
Unable to verify: all imports updated to default export.
The original review comment correctly identifies a breaking change from named export to default export. However, without access to the codebase, I cannot confirm whether all 47 consumers (or however many exist) have been updated. This verification requires searching the entire codebase for remaining named import patterns like
import { OmnichannelBadges }and confirming all are now usingimport OmnichannelBadges from.This requires manual verification or retry with codebase access.
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx (1)
1-25: No organizational consolidation needed—sidebarv2 is an intentional architectural separation supporting incremental migration.The review comment's concern about duplication is based on a misunderstanding of Rocket.Chat's architecture. The web search reveals that
sidebarv2exists as a deliberate architectural pattern for incremental migration from the legacy sidebar (v1) to a new implementation. Both versions coexist temporarily through portal/compatibility layers, allowing teams to migrate without breaking backward compatibility. This is a known and intentional design decision, not a consolidation issue.The file correctly imports from
sidebarv2as part of the forward migration strategy. No action is required.Likely an incorrect or invalid review comment.
apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.tsx
Show resolved
Hide resolved
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37638 +/- ##
========================================
Coverage 67.71% 67.71%
========================================
Files 3452 3458 +6
Lines 113979 113955 -24
Branches 20943 20927 -16
========================================
- Hits 77182 77167 -15
- Misses 34663 34664 +1
+ Partials 2134 2124 -10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx (2)
26-26: Fix typo in test description."shound" should be "should".
Apply this diff:
- it('should render UnreadBadge when there are unread messages', () => { + it('should render UnreadBadge when there are unread messages', () => {
34-34: Fix typo in test description."shound" should be "should".
Apply this diff:
- it('should not render UnreadBadge when there are no unread messages', () => { + it('should not render UnreadBadge when there are no unread messages', () => {
🧹 Nitpick comments (1)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx (1)
39-39: Imprecise assertion could produce false positive.The assertion checks for a specific accessible name
'Test Room'that would never match the actual badge format ('{{count}} mention(s) from {{roomTitle}}'). If the badge incorrectly renders, this test would still pass.Apply this diff to check that no badge with role 'status' exists at all:
- expect(screen.queryByRole('status', { name: 'Test Room' })).not.toBeInTheDocument(); + expect(screen.queryByRole('status')).not.toBeInTheDocument();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx(4 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx
- apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx
- apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx
- apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx
- apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.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/sidebarv2/badges/SidebarItemBadges.spec.tsx
🧠 Learnings (11)
📚 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/sidebarv2/badges/SidebarItemBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/badges/SidebarItemBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.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/sidebarv2/badges/SidebarItemBadges.spec.tsx
🧬 Code graph analysis (1)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx (3)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/client/lib/appLayout.tsx (1)
render(26-28)apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.tsx
Show resolved
Hide resolved
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx
Outdated
Show resolved
Hide resolved
aleksandernsilva
force-pushed
the
refactor/sidebar-badges
branch
from
20fd9dd to
a1e635d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
14-24: LGTM! Clean centralization of badge rendering logic.The component properly:
- Uses
useUnreadDisplayhook for unread state- Conditionally renders
UnreadBadgebased onshowUnread- Conditionally renders
OmnichannelBadgesfor omnichannel rooms- Applies consistent margin spacing via
MarginsNote: The empty line at Line 20 was already flagged in a previous review.
apps/meteor/client/sidebarv2/badges/UnreadBadge.tsx (1)
11-24: Handle optionalroomTitlein translation.The
roomTitleprop is optional but passed directly to the translation function. When undefined, this could result in "undefined" being interpolated into the aria-label string.Apply this diff to provide a fallback:
- aria-label={t('__unreadTitle__from__roomTitle__', { unreadTitle: title, roomTitle })} + aria-label={t('__unreadTitle__from__roomTitle__', { unreadTitle: title, roomTitle: roomTitle ?? '' })}apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx (1)
44-54: Prefer semantic text-based queries overdata-testidfor OmnichannelBadgesSince the mock renders visible text (
OmnichannelBadges), you can avoidgetByTestId/queryByTestIdand assert on that text instead, aligning with Testing Library’s guidance to use user-facing selectors.- expect(screen.getByTestId('omnichannel-badges')).toBeInTheDocument(); + expect(screen.getByText('OmnichannelBadges')).toBeInTheDocument(); @@ - expect(screen.queryByTestId('omnichannel-badges')).not.toBeInTheDocument(); + expect(screen.queryByText('OmnichannelBadges')).not.toBeInTheDocument();
🧹 Nitpick comments (2)
apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts (1)
6-10: Consider consolidating duplicate hook implementations.This hook is now identical to
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts. While the duplication may be intentional for different sidebar variants, consolidating into a shared location would improve maintainability and ensure consistency.Example: Move to a shared location like
apps/meteor/client/hooks/useUnreadDisplay.tsand import from both sidebars.apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx (1)
35-42: Tighten the negative assertion to check for any unread status elementInstead of matching a specific accessible name, you can simplify the negative case to assert that no
statuselement is present at all, which more directly encodes the intent “no unread badge is rendered.”- expect(screen.queryByRole('status', { name: 'Test Room' })).not.toBeInTheDocument(); + expect(screen.queryByRole('status')).not.toBeInTheDocument();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (22)
apps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsx(3 hunks)apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx(4 hunks)apps/meteor/client/sidebar/badges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebar/badges/UnreadBadge.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx(1 hunks)apps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx(4 hunks)apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/UnreadBadge.tsx(1 hunks)apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts(1 hunks)apps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsx(4 hunks)apps/meteor/client/views/navigation/sidebar/badges/OmnichannelBadges.tsx(0 hunks)apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts(1 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsx(3 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx(1 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/client/views/navigation/sidebar/badges/OmnichannelBadges.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.tsx
- apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx
- apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx
- apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx
- apps/meteor/client/sidebar/badges/UnreadBadge.tsx
- apps/meteor/client/sidebar/hooks/useUnreadDisplay.spec.tsx
- apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts
- apps/meteor/client/views/navigation/sidebar/badges/SidebarItemBadges.spec.tsx
- apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx
- apps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsx
- apps/meteor/client/sidebarv2/badges/SidebarItemBadges.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/sidebarv2/RoomList/SidebarItemTemplateWithData.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.tsxapps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsxapps/meteor/client/sidebarv2/badges/UnreadBadge.tsxapps/meteor/client/sidebar/badges/OmnichannelBadges.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsxapps/meteor/client/sidebarv2/hooks/useUnreadDisplay.tsapps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx
🧠 Learnings (14)
📚 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/sidebar/RoomList/SideBarItemTemplateWithData.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.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: 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/sidebarv2/badges/UnreadBadge.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/sidebarv2/badges/UnreadBadge.tsxapps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.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/sidebarv2/hooks/useUnreadDisplay.tsapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx
🧬 Code graph analysis (6)
apps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx (1)
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (1)
useUnreadDisplay(6-10)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (2)
IRoom(21-95)isOmnichannelRoom(361-361)
apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx (1)
useUnreadDisplay(6-10)
apps/meteor/client/sidebarv2/badges/UnreadBadge.tsx (1)
packages/ui-contexts/src/index.ts (1)
useTranslation(80-80)
apps/meteor/client/sidebar/badges/OmnichannelBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts (2)
apps/meteor/client/views/navigation/sidebar/hooks/useUnreadDisplay.ts (1)
useUnreadDisplay(6-10)apps/meteor/lib/getSubscriptionUnreadData.ts (1)
UnreadData(36-39)
⏰ 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 (9)
apps/meteor/client/sidebar/badges/OmnichannelBadges.tsx (1)
8-23: LGTM! Clean conversion to default export.The change from named to default export aligns with the broader refactor pattern across the PR. Component logic remains unchanged.
apps/meteor/client/sidebarv2/RoomList/SidebarItemTemplateWithData.tsx (1)
96-96: LGTM! Clean integration of SidebarItemBadges.The refactor properly delegates badge rendering to the centralized
SidebarItemBadgescomponent while maintaining the unread display logic viauseUnreadDisplay. The hook outputs are correctly used for aria-label and menu props.Also applies to: 142-142
apps/meteor/client/NavBarV2/NavBarSearch/NavBarSearchItemWithData.tsx (1)
24-24: LGTM! Consistent badge refactor pattern.The changes align with the broader refactoring effort, properly using
useUnreadDisplayand delegating badge rendering toSidebarItemBadges.Also applies to: 36-36
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
18-60: LGTM! Well-structured test coverage.The test suite properly covers the key badge rendering scenarios:
- UnreadBadge visibility based on unread state
- OmnichannelBadges visibility based on room type
Good use of mock providers and test isolation via
jest.resetAllMocks().apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
96-98: LGTM! Consistent refactor for sidebar v1.The changes properly integrate the new badge architecture:
- Uses
useUnreadDisplayfor centralized unread state- Delegates badge rendering to
SidebarItemBadges- Correctly derives
threadUnreadfromunreadCount.threads- Maintains proper accessibility with dynamic aria-label
Also applies to: 138-138, 144-144, 152-152
apps/meteor/client/sidebarv2/hooks/useUnreadDisplay.ts (1)
3-4: LGTM! Clean type import.Using the exported
UnreadDatatype improves code clarity and maintainability compared to inline type definitions.apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItem.tsx (1)
2-2: Badge refactor wiring and unread integration look consistentThe switch to
useUnreadDisplay(highlightUnread/unreadCount), passingthreadUnread={unreadCount.threads > 0}, usingSidebarItemIconwithhighlighted, and delegatingbadgestoRoomSidePanelItemBadgesare all internally consistent and preserve the existing RoomSidePanelItem structure.Also applies to: 9-9, 29-29, 40-56, 64-69
apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.spec.tsx (2)
1-10: Test setup and OmnichannelBadges mock are appropriateUsing
mockAppRoot().withTranslations(...).build()plus a module mock forSidePanelOmnichannelBadgescleanly isolates translations and omnichannel rendering, andafterEach(jest.resetAllMocks)keeps the suite independent.Also applies to: 12-24
26-33: Accessible-name assertion for unread badge is solidThe positive unread case correctly asserts on the
statusrole and full accessible name ('1 mention from Test Room'), which tightly couples the test to the intended i18n output and ARIA semantics.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsx (1)
17-17: Consider tightening the type assertion.The double type assertion
as unknown as ISubscription & IRoomcompletely bypasses TypeScript's type checking. While this may be necessary ifcreateFakeSubscriptiondoesn't return an exact match, consider:
- Updating
createFakeSubscriptionto return the correct type, or- Using a more specific type assertion that validates compatibility
This would catch type mismatches earlier and improve type safety in tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/__snapshots__/OmnichannelBadges.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx(0 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.tsx(1 hunks)apps/meteor/client/views/navigation/sidepanel/omnichannel/SidePanelOmnichannelBadges/SidePanelOmnichannelBadges.tsx(0 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/client/views/navigation/sidepanel/omnichannel/SidePanelOmnichannelBadges/SidePanelOmnichannelBadges.tsx
- apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx
- apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx
- apps/meteor/client/views/navigation/sidepanel/SidepanelItem/RoomSidePanelItemBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.tsapps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsxapps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx
🧠 Learnings (11)
📚 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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.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/components/OmnichannelBadges/OmnichannelBadges.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsx (3)
apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
⏰ 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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx (2)
4-6: LGTM: Import paths updated appropriately.The import paths have been updated to use relative references, which is appropriate for internal module organization.
8-23: LGTM: Export conversion aligns with PR objectives.Converting from named export to default export standardizes the component's export pattern, making it consistent with the new SidebarItemBadges architecture referenced in the PR objectives.
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.ts (1)
1-1: LGTM: Standard barrel export pattern.This enables consumers to import from the directory path rather than the specific file, which is a standard pattern for module organization.
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx (2)
1-12: LGTM: Story-based testing approach is effective.Using
composeStoriesto generate tests from Storybook stories ensures consistency between the component's documentation and test coverage. Snapshot tests provide regression protection for rendering behavior.
14-19: LGTM: Accessibility testing ensures inclusive UI.Including automated accessibility checks with jest-axe is excellent practice, ensuring that badge components meet accessibility standards across different scenarios.
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsx (2)
19-41: LGTM: Decorator properly provides required context.The decorator correctly wraps stories with both
OmnichannelContextandAppRoot, providing the necessary context for rendering OmnichannelBadges. The mock data structure appropriately simulates both priority and MAC limit states.
46-68: LGTM: Story variants cover key scenarios.The two stories effectively demonstrate different badge rendering states:
WithPriority: Tests priority badge displayOverMacLimit: Tests MAC limit indicatorThis provides good visual documentation and test coverage for the component's different states.
aleksandernsilva
force-pushed
the
refactor/sidebar-badges
branch
from
95840d6 to
1e690f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/__snapshots__/OmnichannelBadges.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx(0 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.ts
- apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.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/sidebar/badges/SidebarItemBadges.tsxapps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsxapps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsxapps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx
🧠 Learnings (11)
📚 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/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx
🧬 Code graph analysis (2)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (2)
IRoom(21-95)isOmnichannelRoom(361-361)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
isOmnichannelRoom(361-361)
⏰ 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/sidebarv2/badges/SidebarItemBadges.tsx (1)
1-24: Well-factored badge wrapper; hook and type usage look solidThe component cleanly encapsulates unread and omnichannel badge logic:
useUnreadDisplay(room)is used once, unread rendering is correctly guarded byshowUnread, andisOmnichannelRoom(room)is applied before renderingOmnichannelBadges. TheSubscriptionWithRoomprop type is appropriate for both the unread hook and the omnichannel type guard, and the fragment-based return keeps this reusable across different sidebar layouts. No issues spotted.apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx (3)
1-5: LGTM!The imports are appropriate for Storybook-driven component tests with accessibility checking.
7-7: LGTM!The test case generation from Storybook stories follows the standard pattern and includes a sensible fallback for story names.
14-19: LGTM!The accessibility test correctly uses
async/awaitfor theaxeanalysis and follows the standard pattern for jest-axe validation.apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
1-25: LGTM! Clean refactor that consolidates badge rendering.The component effectively extracts badge rendering logic into a reusable, testable unit. The use of
useUnreadDisplaycentralizes unread calculations, and the conditional rendering for both UnreadBadge and OmnichannelBadges is straightforward and correct.apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx (2)
4-6: The relative import paths all resolve correctly.useOmnichannelPrioritiesexists atapps/meteor/client/views/omnichannel/hooks/useOmnichannelPriorities.ts,PriorityIconatapps/meteor/client/views/omnichannel/priorities/PriorityIcon.tsx, andRoomActivityIconatapps/meteor/client/views/omnichannel/components/RoomActivityIcon.tsx.
8-23: All imports of OmnichannelBadges have been correctly updated to use the default export.Verification confirms no outdated named imports (
import { OmnichannelBadges }) exist in the codebase. All three consumer files now use the correct default import pattern:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsxapps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.tsxThe
index.tsfile properly re-exports the default, maintaining a clean export structure.
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx
Outdated
Show resolved
Hide resolved
aleksandernsilva
force-pushed
the
refactor/sidebar-badges
branch
from
1e690f4 to
15f6577
Compare
aleksandernsilva
force-pushed
the
refactor/sidebar-badges
branch
from
15f6577 to
e6658db
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
32-32: Typos already corrected.The test names correctly use "should" rather than "shound" as flagged in the previous review.
Also applies to: 41-41
🧹 Nitpick comments (1)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx (1)
4-23: Export refactor and omnichannel guard look correctThe component keeps the previous behavior (priority + activity badges guarded by
isOmnichannelRoom) while switching cleanly to a default export and updated import paths; hook usage is safe and typingroomasISubscription & IRoomis consistent with the existing pattern. If this prop shape is reused elsewhere in the future, you could optionally extract atype OmnichannelBadgesProps = { room: ISubscription & IRoom };for reuse, but it’s not necessary for this change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/__snapshots__/OmnichannelBadges.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx(0 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.tsx(1 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx(2 hunks)apps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/client/sidebarv2/badges/OmnichannelBadges.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/sidebarv2/badges/SidebarItemBadges.spec.tsx
- apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.stories.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/omnichannel/components/OmnichannelBadges/index.tsapps/meteor/client/sidebar/badges/SidebarItemBadges.tsxapps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsxapps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsxapps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx
🧠 Learnings (15)
📚 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/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.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 : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.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/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.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/sidebar/badges/SidebarItemBadges.spec.tsx
🧬 Code graph analysis (3)
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (2)
IRoom(21-95)isOmnichannelRoom(361-361)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx (1)
packages/core-typings/src/IRoom.ts (1)
isOmnichannelRoom(361-361)
⏰ 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 (8)
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/OmnichannelBadges.spec.tsx (1)
1-19: Story-based snapshot + a11y coverage looks solidData-driven tests over all composed stories with a snapshot plus an axe check is a good balance of coverage and maintenance, and the test names clearly describe the expectations. No changes needed here.
apps/meteor/client/views/omnichannel/components/OmnichannelBadges/index.ts (1)
1-1: Barrel re-export matches new default exportThe index file correctly re-exports the default
OmnichannelBadgescomponent, aligning with the refactor and keeping import paths stable.apps/meteor/client/sidebarv2/badges/SidebarItemBadges.tsx (4)
8-11: LGTM!The type definition is clear, concise, and properly typed.
24-24: LGTM!Default export is appropriate for this component.
13-22: Verify the useUnreadDisplay hook signature matches the room parameter type.The component passes
room(typeSubscriptionWithRoom) to theuseUnreadDisplayhook. Confirm that the hook's parameter type is compatible withSubscriptionWithRoom, especially if the hook was recently updated to acceptUnreadDatainstead. Check the hook's actual signature inuseUnreadDisplay.tsto ensure there is no type mismatch.
5-5: No issues found. The OmnichannelBadges import path../../views/omnichannel/components/OmnichannelBadgesis correct and resolves to the component via the index.ts barrel file, which properly re-exports the default export from OmnichannelBadges.tsx. The import statement uses the correct default import syntax.apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
1-61: LGTM! Comprehensive test coverage.The test suite is well-structured with proper mocking, clear test cases covering both positive and negative scenarios for UnreadBadge and OmnichannelBadges rendering, and appropriate use of React Testing Library patterns.
apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx (1)
14-23: LGTM! Clean component structure.The component logic is well-organized with appropriate conditional rendering for UnreadBadge and OmnichannelBadges, good use of the useUnreadDisplay hook for state management, and proper encapsulation of badge display logic.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/sidebar/RoomList/RoomListRow.tsx (1)
2-59: Add string union type foritempropThe
useRoomListhook inserts section header strings (like'Channels','Direct_Messages') alongside room objects into the list (line 141 in useRoomList.ts:return [key, ...group]), butRoomListRow'sitemprop is typed only asSubscriptionWithRoom. The component correctly handles both cases withtypeof item === 'string', but the type annotation must match:-const RoomListRow = ({ data, item }: { data: RoomListRowProps; item: SubscriptionWithRoom }): ReactElement => { +const RoomListRow = ({ data, item }: { data: RoomListRowProps; item: SubscriptionWithRoom | string }): ReactElement => {This restores proper type narrowing so TypeScript correctly infers the type in each branch.
🧹 Nitpick comments (3)
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
34-40: Consider a more explicit assertion pattern.The test correctly verifies that UnreadBadge doesn't render when there are no unread messages. However, the accessible name
'Test Room'doesn't match any expected pattern and is somewhat arbitrary. For clarity, consider querying without a specific name or checking that no unread-related status elements exist.Example alternative:
- expect(screen.queryByRole('status', { name: 'Test Room' })).not.toBeInTheDocument(); + expect(screen.queryByRole('status', { name: /mention.*Test Room/ })).not.toBeInTheDocument();Or verify no UnreadBadge-related status exists by checking all status elements don't match the unread pattern.
apps/meteor/client/sidebar/RoomList/RoomList.tsx (1)
5-22: AligncomputeItemKeytyping with actualroomsListitem shape
useRoomListreturns a heterogeneous list (section label strings plus room-like entries), butcomputeItemKeyis typed as if it only ever receivesSubscriptionWithRoom. At runtime this works becauseroom._idjust evaluates toundefinedfor non-room entries and falls back toindex, but the current signature misrepresents the data shape and can mislead TypeScript consumers.Consider widening the parameter type (e.g., union that matches
useRoomList’s item type, or evenunknownwith a small runtime type guard for_id) so the key function’s typing stays honest with the actual data flowing through Virtuoso.apps/meteor/client/sidebar/hooks/useRoomList.ts (1)
1-59: MakeuseRoomListreturn type reflect the mixed item list
useRoomListis declared to returnSubscriptionWithRoom[], and the debounced state is typed the same way:export const useRoomList = (): SubscriptionWithRoom[] => { const [roomList, setRoomList] = useDebouncedState<SubscriptionWithRoom[]>([], 150);But the constructed array actually contains:
- Section label strings (
key),- Room subscription entries (
room),- Livechat inquiries from
queue: ILivechatInquiryRecord[].So the true item shape is a union, not a pure
SubscriptionWithRoom. The current typing relies onanyescaping fromgroupsand weakens static guarantees for all consumers (includingRoomListRow, which still relies on string headers).Consider introducing a dedicated union type for the room list items and using it consistently:
-import type { SubscriptionWithRoom } from '@rocket.chat/ui-contexts'; +import type { SubscriptionWithRoom } from '@rocket.chat/ui-contexts'; +type RoomListItem = SubscriptionWithRoom | ILivechatInquiryRecord | string; -export const useRoomList = (): SubscriptionWithRoom[] => { - const [roomList, setRoomList] = useDebouncedState<SubscriptionWithRoom[]>([], 150); +export const useRoomList = (): RoomListItem[] => { + const [roomList, setRoomList] = useDebouncedState<RoomListItem[]>([], 150);This keeps the hook’s contract aligned with what it actually returns and makes downstream components’ typings (like
RoomListRow) easier to keep correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/meteor/client/sidebar/RoomList/RoomList.tsx(2 hunks)apps/meteor/client/sidebar/RoomList/RoomListRow.tsx(2 hunks)apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx(5 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useAvatarTemplate.tsx(2 hunks)apps/meteor/client/sidebar/hooks/useRoomList.ts(2 hunks)apps/meteor/client/sidebar/search/Row.tsx(1 hunks)apps/meteor/client/sidebar/search/SearchList.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/sidebar/badges/SidebarItemBadges.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/sidebar/search/Row.tsxapps/meteor/client/sidebar/RoomList/RoomListRow.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsxapps/meteor/client/sidebar/hooks/useAvatarTemplate.tsx
🧠 Learnings (16)
📚 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/sidebar/search/Row.tsxapps/meteor/client/sidebar/RoomList/RoomListRow.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/hooks/useAvatarTemplate.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/sidebar/search/Row.tsxapps/meteor/client/sidebar/RoomList/RoomListRow.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/hooks/useAvatarTemplate.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/sidebar/RoomList/RoomListRow.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsxapps/meteor/client/sidebar/hooks/useAvatarTemplate.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/sidebar/hooks/useRoomList.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 : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.spec.tsx
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: 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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.spec.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/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/search/SearchList.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/sidebar/badges/SidebarItemBadges.spec.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx
🧬 Code graph analysis (2)
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (2)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)
apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx (1)
useUnreadDisplay(6-10)
⏰ 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 (11)
apps/meteor/client/sidebar/search/Row.tsx (1)
1-11: LGTM! Clean type migration.The type update from
ISubscription & IRoomtoSubscriptionWithRoomis consistent with the PR's broader refactoring objectives. The component logic remains unchanged, ensuring no behavioral impact.apps/meteor/client/sidebar/search/SearchList.tsx (1)
6-71: LGTM! Type update aligns with refactoring.The return type change from
(ISubscription & IRoom)[]toSubscriptionWithRoom[]is consistent with the broader type migration across search components. The query logic and data processing remain unchanged, ensuring functional parity.apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (4)
1-10: LGTM! Clean test setup.The imports and mock configuration are well-structured. The OmnichannelBadges mock correctly uses
role='status'andaria-labelfor testability.
12-24: LGTM! Proper test suite configuration.The test setup provides all necessary translations and ensures proper test isolation with the
afterEachcleanup hook.
26-32: LGTM! Well-structured positive test case.The test correctly verifies UnreadBadge rendering with appropriate accessible name assertion matching the translation pattern.
42-52: LGTM! Comprehensive omnichannel badge coverage.Both test cases correctly validate the conditional rendering of OmnichannelBadges based on room type, with appropriate positive and negative assertions.
apps/meteor/client/sidebar/hooks/useAvatarTemplate.tsx (1)
34-34: Consider documenting therid || _idfallback pattern.The defensive fallback
room.rid || room._idsuggests thatridmight not always be present on the room object, which could indicate either optional typing or a type mismatch. IfSubscriptionWithRoommakesridoptional, this is correct defensive coding. However, ifridis typed as required but can be undefined at runtime, this represents a type safety gap.apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (4)
1-4: SubscriptionWithRoom migration and badge imports look consistentSwitching
roomtoSubscriptionWithRoomingetMessageandRoomListRowProps, plus routing badge concerns throughSidebarItemBadges, is coherent with the new unread/badge architecture and stays compatible with the existing room helpers used here. No issues spotted in these type/import changes.Also applies to: 17-21, 67-68
97-100: Unread display refactor via useUnreadDisplay is coherentDestructuring
unread = 0and delegatingunreadCount,unreadTitle,showUnread, andhighlightUnreadtouseUnreadDisplay(room)cleanly removes local unread math and keepshighlightedas the single source for unread styling. The default forunreadpreserves prior semantics while avoidingundefinedcases; this segment looks solid.
151-156: threadUnread now derives from unreadCount.threads, which is mathematically equivalent to prior logicThe change to
threadUnread={unreadCount.threads > 0}is a safe refactor rather than a behavioral change.unreadCount.threadsis computed astunread?.length || 0ingetSubscriptionUnreadData, making the condition semantically identical to checking whether the thread unread array has entries. The pattern is applied consistently across sidebar, sidebarv2, and sidepanel implementations, andunreadCountis always guaranteed to return with a numericthreadsfield.
139-145: ARIA label and SidebarItemBadges integration are correctThe
__unreadTitle__from__roomTitle__i18n key is defined in all supported locales (en, sv, pt-BR, nb), andSidebarItemBadgesproperly encapsulates both unread and omnichannel badge rendering. Test coverage confirms full badge parity: unread badges render whenshowUnreadis true with the correct aria-label format, and omnichannel badges render for omnichannel rooms (type 'l').
aleksandernsilva
force-pushed
the
refactor/sidebar-badges
branch
from
6e48946 to
5aa2110
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
34-40: Optional: align negative unread assertion with positive case nameThe negative test currently checks for absence of a
statuselement named'Test Room'. To mirror the positive case more tightly and make intent clearer, you could assert against the same pattern used in the positive test, e.g.'1 mention from Test Room'(or a regex that matches the unread label format). This would better document that the unread badge (with its specific accessible name) disappears when there are no unread messages.- expect(screen.queryByRole('status', { name: 'Test Room' })).not.toBeInTheDocument(); + expect(screen.queryByRole('status', { name: '1 mention from Test Room' })).not.toBeInTheDocument();apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
97-100: Consider unifying unread semantics between row and RoomMenu
useUnreadDisplay(room)now driveshighlighted,showUnread,unreadTitle, andunreadCount.threads, whileRoomMenustill receivesunread={!!unread}from the raw room object:
- Row highlight +
data-unreadrely onhighlightUnread.aria-labelusesshowUnread/unreadTitleand the__unreadTitle__from__roomTitle__key, matching the new tests and improving a11y.- Thread indicator uses
unreadCount.threads > 0, which is consistent with the unread aggregation logic.If
useUnreadDisplaybecomes the canonical source of truth for unread state, you might eventually wantRoomMenu’sunreadprop to also be derived from that (e.g., fromunreadCount.totalorshowUnread) rather than the rawunreadfield, to avoid potential divergence if unread semantics evolve.Also applies to: 139-146, 153-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/meteor/client/sidebar/RoomList/RoomList.tsx(2 hunks)apps/meteor/client/sidebar/RoomList/RoomListRow.tsx(2 hunks)apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx(5 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx(1 hunks)apps/meteor/client/sidebar/badges/SidebarItemBadges.tsx(1 hunks)apps/meteor/client/sidebar/hooks/useAvatarTemplate.tsx(2 hunks)apps/meteor/client/sidebar/hooks/useRoomList.ts(2 hunks)apps/meteor/client/sidebar/search/Row.tsx(1 hunks)apps/meteor/client/sidebar/search/SearchList.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/meteor/client/sidebar/hooks/useRoomList.ts
- apps/meteor/client/sidebar/RoomList/RoomListRow.tsx
- apps/meteor/client/sidebar/hooks/useAvatarTemplate.tsx
- apps/meteor/client/sidebar/badges/SidebarItemBadges.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/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsxapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsxapps/meteor/client/sidebar/search/Row.tsx
🧠 Learnings (15)
📚 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/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/search/Row.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/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/search/Row.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/sidebar/badges/SidebarItemBadges.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: 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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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/sidebar/badges/SidebarItemBadges.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.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/sidebar/badges/SidebarItemBadges.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/sidebar/search/SearchList.tsxapps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx
🧬 Code graph analysis (2)
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (2)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)apps/meteor/tests/mocks/data.ts (1)
createFakeSubscription(63-90)
apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
apps/meteor/client/sidebar/hooks/useUnreadDisplay.tsx (1)
useUnreadDisplay(6-10)
⏰ 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/sidebar/search/Row.tsx (1)
1-11: LGTM! Type update aligns with refactor.The change from
ISubscription & IRoomtoSubscriptionWithRoomis a clean type consolidation with no behavioral impact.apps/meteor/client/sidebar/RoomList/RoomList.tsx (1)
5-5: LGTM! Function signature updated correctly.The
computeItemKeytype update fromIRoomtoSubscriptionWithRoomis consistent with the broader refactor. The implementation remains unchanged.Also applies to: 22-22
apps/meteor/client/sidebar/search/SearchList.tsx (1)
6-6: LGTM! Hook return type updated correctly.The
useSearchItemsreturn type change toSubscriptionWithRoom[]aligns with the type consolidation across sidebar components. No implementation changes.Also applies to: 71-71
apps/meteor/client/sidebar/badges/SidebarItemBadges.spec.tsx (1)
7-53: Well‑structured RTL tests with good a11y‑oriented assertionsThe suite cleanly sets up translations via
mockAppRoot, usescreateFakeSubscriptionoverrides to pin unread/mention state, and tests behavior throughrole="status"queries and accessible names. The OmnichannelBadges mock isolates SidebarItemBadges logic without coupling tests to its internal markup. Overall, this is solid and focused coverage for unread vs. omnichannel badges.apps/meteor/client/sidebar/RoomList/SideBarItemTemplateWithData.tsx (1)
1-4: Typed move toSubscriptionWithRoomand hook integration looks consistentUpdating
getMessageandRoomListRowProps.roomtoSubscriptionWithRoom, importing the newuseUnreadDisplayhook, and wiring inSidebarItemBadgesare all consistent with the shift toward a unified subscription+room shape. The types line up with existing usage ofisDirectMessageRoom/isMultipleDirectMessageRoom, and the new badges component centralizes unread/omnichannel UI nicely without changing call‑sites’ responsibilities.Also applies to: 17-21, 67-68
aleksandernsilva
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
aleksandernsilva
removed
the
stat: QA assured
dionisio-bot
bot
removed
the
stat: ready to merge
aleksandernsilva
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
2 participants
Proposed changes (including videos or screenshots)
This pull request refactors the sidebar badges extracting the logic that currently is inline and hard to test and moving them to a new
SidebarItemBadgescomponent .This is an effort to allow better testing for this specific part of the UI and prepare the code for the new invitation badge to be introduced by #37635.
SidebarItemBadgescomponent to encapsulate logic for displaying unread and omnichannel badges.SidebarItemBadgesfor badge rendering.useUnreadDisplayhook to centralize unread message and mention calculation (same thing we already do for the other sidebars) and updated sidebar V1 to use it.UnreadBadgecomponent for consistency.OmnichannelBadgesto be a default export and updated imports following guidelinesSidebarItemBadges.Issue(s)
FB-64
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.