fix: UserAutoCompleteMultiple not removing selected items
#37823
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 5a9fae7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughConsolidates federated and non-federated user-autocomplete into a single Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 20:41", "12/18 21:51 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
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, 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, 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, 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.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, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 24 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Thu, 18 Dec 2025 21:51:23 GMT |
juliajforesti
force-pushed
the
fix/userAutoCompleteMultiple
branch
from
a6731ba to
d762778
Compare
juliajforesti
force-pushed
the
fix/userAutoCompleteMultiple
branch
from
85ff404 to
864c52b
Compare
juliajforesti
force-pushed
the
fix/userAutoCompleteMultiple
branch
from
864c52b to
a70b8aa
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37823 +/- ##
===========================================
- Coverage 67.72% 67.72% -0.01%
===========================================
Files 3464 3464
Lines 113728 113713 -15
Branches 20908 20908
===========================================
- Hits 77024 77008 -16
- Misses 34527 34530 +3
+ Partials 2177 2175 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.ts (1)
33-33: Consider explicit delay for consistency.The
pressSequentiallycall here doesn't specify a delay parameter, while the same pattern inhome-team.ts(line 33) uses{ delay: 100 }. Consider adding an explicit delay for consistent behavior across test interactions with autocomplete components.- await this.page.getByRole('textbox', { name: 'Choose users' }).pressSequentially(username); + await this.page.getByRole('textbox', { name: 'Choose users' }).pressSequentially(username, { delay: 100 });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/meteor/client/sidebar/header/CreateTeam/__snapshots__/CreateTeamModal.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
.changeset/cuddly-eels-perform.md(1 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsx(2 hunks)apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateDirectMessage.tsx(2 hunks)apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx(2 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx(1 hunks)apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleFederated.tsx(0 hunks)apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsx(2 hunks)apps/meteor/client/sidebar/header/CreateDirectMessage.tsx(2 hunks)apps/meteor/client/views/audit/components/tabs/DirectTab.tsx(1 hunks)apps/meteor/client/views/audit/components/tabs/UsersTab.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/AddUsers/AddUsers.tsx(2 hunks)apps/meteor/tests/e2e/administration.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.ts(1 hunks)apps/meteor/tests/e2e/page-objects/home-team.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleFederated.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/client/views/audit/components/tabs/DirectTab.tsxapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.tsapps/meteor/client/sidebar/header/CreateDirectMessage.tsxapps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/AddUsers/AddUsers.tsxapps/meteor/client/views/audit/components/tabs/UsersTab.tsxapps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsxapps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/administration.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/administration.spec.ts
🧠 Learnings (19)
📚 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 : Avoid using `page.locator()` in Playwright tests - always prefer semantic locators such as `page.getByRole()`, `page.getByLabel()`, `page.getByText()`, or `page.getByTitle()`
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.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/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.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/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.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/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.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 : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.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 : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.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 : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/administration.spec.tsapps/meteor/tests/e2e/page-objects/home-team.ts
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-team.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/administration.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/e2e/administration.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/e2e/administration.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/administration.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/e2e/administration.spec.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/client/sidebar/header/CreateDirectMessage.tsxapps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/AddUsers/AddUsers.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/header/CreateDirectMessage.tsxapps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/AddUsers/AddUsers.tsxapps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateDirectMessage.tsx
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsx.changeset/cuddly-eels-perform.mdapps/meteor/client/views/room/contextualBar/RoomMembers/AddUsers/AddUsers.tsx
🧬 Code graph analysis (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleOptions.tsx (1)
OptionsContext(22-24)
🔇 Additional comments (18)
apps/meteor/tests/e2e/page-objects/fragments/home-flextab-members.ts (1)
33-34: Excellent migration to semantic role-based selectors.The shift from locator-based selectors to
getByRole('textbox')andgetByRole('option')aligns with Playwright best practices and improves test stability. UsingpressSequentiallyinstead offillis appropriate for autocomplete components that filter results as users type.Based on coding guidelines: "Avoid using
page.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole()".apps/meteor/tests/e2e/administration.spec.ts (1)
314-314: LGTM: Appropriate change for autocomplete interaction.Switching from
filltopressSequentiallycorrectly simulates character-by-character typing, which triggers the autocomplete filtering logic in the newUserAutoCompleteMultiplecomponent. This aligns with the updated page object patterns.Note: For consistency with
home-team.tsline 33, consider adding{ delay: 100 }if test flakiness occurs due to rapid typing.apps/meteor/tests/e2e/page-objects/home-team.ts (1)
33-34: Strong improvement: semantic selectors replace brittle CSS selectors.The migration from CSS class selectors (
.rcx-option__content) and string-based role locators to propergetByRolecalls significantly improves test maintainability. The explicit{ delay: 100 }parameter inpressSequentiallyprovides consistent typing behavior for the autocomplete component.Note: Consider standardizing the delay parameter across similar interactions in
home-flextab-members.tsline 33 andadministration.spec.tsline 314, which currently don't specify an explicit delay.Based on coding guidelines: "Avoid using
page.locator()- always prefer semantic locators" and "Store commonly used locators in variables/constants for reuse.".changeset/cuddly-eels-perform.md (1)
1-5: LGTM! Changeset properly documents the bugfix.The changeset correctly specifies a patch version for the bugfix and clearly describes the issue being resolved.
apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx (1)
9-9: LGTM! Migration to unified component is correct.The import path and component usage are properly updated to use the generalized
UserAutoCompleteMultiplewith thefederatedprop enabled.Also applies to: 60-60
apps/meteor/client/views/audit/components/tabs/DirectTab.tsx (1)
36-36: LGTM! Error prop correctly updated to string type.The change from a boolean error indicator to passing the actual error message (
usersFieldState.error?.message) aligns with the updatedUserAutoCompleteMultipleAPI and provides better error context.apps/meteor/client/sidebar/header/CreateDirectMessage.tsx (1)
23-23: LGTM! Migration preserves all functionality and accessibility.The import and component usage are properly updated. The
federatedprop enables federated user selection, and all accessibility attributes (aria-describedby,aria-required,aria-invalid) are correctly maintained.Also applies to: 79-90
apps/meteor/client/views/room/contextualBar/RoomMembers/AddUsers/AddUsers.tsx (1)
40-40: LGTM! Excellent refactoring with improved validation.The migration to the unified component simplifies the code by removing conditional rendering. The added validation rule correctly prevents external users from being added to non-federated rooms, improving data integrity.
Also applies to: 76-90
apps/meteor/client/views/audit/components/tabs/UsersTab.tsx (1)
35-35: LGTM! Error handling updated consistently.The error prop change from boolean to string message is consistent with the updated component API and matches the pattern used in DirectTab.tsx.
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateChannelModal.tsx (1)
37-37: LGTM! Migration with proper validation.The component migration is correct, with the
federatedprop properly connected to form state. The validation rule correctly prevents external members from being added to non-federated channels, maintaining data integrity.Also applies to: 255-270
apps/meteor/client/NavBarV2/NavBarPagesGroup/actions/CreateDirectMessage.tsx (1)
23-23: LGTM! Consistent migration preserving accessibility.The import and component usage are properly updated, mirroring the sidebar version. The
federatedprop enables federated user selection, and all accessibility attributes are correctly maintained.Also applies to: 81-92
apps/meteor/client/sidebar/header/CreateChannel/CreateChannelModal.tsx (2)
37-37: LGTM!The import path correctly references the unified
UserAutoCompleteMultiplecomponent, aligning with the PR objective to consolidate federated and non-federated functionality.
260-266: LGTM!The component integration is correct. The
federatedprop properly controls the component's behavior based on the form state, and the Controller integration with react-hook-form is appropriate.apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (5)
1-9: LGTM!The imports correctly reflect the component's transition from
AutoCompletetoMultiSelectFilteredand include all necessary dependencies for the new implementation.
11-27: LGTM!The type definitions are well-structured and accurately model the component's data requirements, including support for federated users.
29-35: LGTM!The regex pattern correctly matches Matrix federation format, and the debounce interval of 500ms provides a good balance between responsiveness and API call frequency.
36-55: LGTM!The data fetching implementation is solid. The
queryKeycorrectly includes thefederatedflag for cache separation, and the conditional injection of federated users based on the Matrix pattern is appropriate. The use ofkeepPreviousDataprevents UI flickering during refetches.
70-78: LGTM!The
onRemoveUsercallback correctly maintains immutability when removing users from the cache.
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Show resolved
Hide resolved
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Show resolved
Hide resolved
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (3)
60-69: Throwing on cache miss can crash the component.As noted in the previous review, throwing an error when a user is not found could crash the component during race conditions or when dealing with stale data. Defensive error handling with a warning would be more resilient.
81-91: Only processing the first added/removed username.As previously noted, the current logic only handles the first added and first removed username via
[0]. If multiple usernames are added or removed (e.g., programmatically or via paste), the remaining items won't be properly cached or uncached, leading to display issues.
120-120: Options list can contain duplicates.As noted in the previous review, concatenating fetched options with
selectedCacheentries can create duplicate username entries in the dropdown when a selected user also appears in search results.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx(1 hunks)apps/meteor/client/lib/queryKeys.ts(1 hunks)
🧰 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/lib/queryKeys.tsapps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
🧬 Code graph analysis (1)
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (2)
apps/meteor/client/lib/queryKeys.ts (1)
usersQueryKeys(122-127)apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultipleOptions.tsx (1)
OptionsContext(22-24)
⏰ 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 (3)
apps/meteor/client/lib/queryKeys.ts (1)
126-126: LGTM! Query key factory follows established patterns.The new
userAutoCompletekey factory is consistent with the existing query key structure and properly typed withas const. The inclusion of bothfilterandfederatedparameters ensures proper cache segmentation for different autocomplete contexts.apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (2)
12-28: Type definitions are clear and well-structured.The prop types correctly reflect the multi-select behavior with
Array<string>for value and onChange, and the internal cache types (UserAutoCompleteOptionTypeandUserAutoCompleteOptions) properly support the selected-item caching strategy.
36-58: Data fetching logic is well-implemented.The 500ms debounce provides good balance between responsiveness and API call reduction. The use of
keepPreviousDataprevents UI flicker during re-queries, and the federated user injection logic correctly adds synthetic options for Matrix-pattern usernames.
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Show resolved
Hide resolved
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Show resolved
Hide resolved
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx
Outdated
Show resolved
Hide resolved
MartinSchoeler
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
CORE-1559
Proposed changes (including videos or screenshots)
UserAutoCompleteMultipleFederatedandUserAutoCompleteMultiplefederatedflag to change behaviorhasExternalMembersvalidation onCreateChannelModalsince it's not necessary anymore: the field will only show external users whenfederatedflag is trueIssue(s)
closes #37809
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
New Features / UX
Validation
Tests
Chore
✏️ Tip: You can customize this high-level summary in your review settings.