test: Replace data-qa by accessible locators
#36757
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #36757 +/- ##
===========================================
- Coverage 70.65% 70.60% -0.06%
===========================================
Files 3143 3143
Lines 108693 108684 -9
Branches 19557 19514 -43
===========================================
- Hits 76797 76735 -62
- Misses 29886 29949 +63
+ Partials 2010 2000 -10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRemoved many Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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/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 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 15:42 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.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, 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, 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.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [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, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [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.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 29 Dec 2025 15:42:19 GMT |
juliajforesti
force-pushed
the
test/replace-data-qa
branch
from
ab8eb32 to
251ad19
Compare
juliajforesti
force-pushed
the
test/replace-data-qa
branch
from
251ad19 to
e5a1c63
Compare
…role-based selector
…or for accessibility
juliajforesti
force-pushed
the
test/replace-data-qa
branch
from
9a677ae to
1fcce8a
Compare
juliajforesti
force-pushed
the
test/replace-data-qa
branch
from
994a94a to
33c8a6e
Compare
juliajforesti
force-pushed
the
test/replace-data-qa
branch
from
3a1b4e8 to
14fd569
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/meteor/tests/e2e/page-objects/omnichannel-room-info.ts (1)
35-41: Replacepage.locator()with semantic locators.These methods use
page.locator()with string selectors, which violates the Playwright testing guideline. Semantic locators likegetByText()are more maintainable and resilient.🔎 Proposed refactor
getInfo(value: string): Locator { - return this.page.locator(`span >> text="${value}"`); + return this.page.getByText(value, { exact: true }); } getLabel(label: string): Locator { - return this.page.locator(`div >> text="${label}"`); + return this.page.getByText(label, { exact: true }); }Based on coding guidelines: "Avoid using
page.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()."apps/meteor/tests/e2e/homepage.spec.ts (1)
37-37: Replacelocator()with semantic selectors.Multiple instances use
page.locator()with deprecatedrole=syntax or text selectors. The coding guidelines require using semantic locators likegetByRole(),getByText(),getByLabel()instead oflocator().🔎 Proposed refactoring
-await expect(adminPage.locator('role=button[name="Customize"]')).toBeVisible(); +await expect(adminPage.getByRole('button', { name: 'Customize' })).toBeVisible(); -await expect(adminPage.locator('div >> text="Admins may insert content html to be rendered in this white space."')).toBeVisible(); +await expect(adminPage.getByText('Admins may insert content html to be rendered in this white space.')).toBeVisible(); -await expect(adminPage.locator('role=button[name="Show to workspace"]')).toBeDisabled(); +await expect(adminPage.getByRole('button', { name: 'Show to workspace' })).toBeDisabled(); -await expect(adminPage.locator('role=button[name="Show only this content"]')).toBeDisabled(); +await expect(adminPage.getByRole('button', { name: 'Show only this content' })).toBeDisabled(); -await expect(adminPage.locator('span >> text="Not visible to workspace"')).toBeVisible(); +await expect(adminPage.getByText('Not visible to workspace')).toBeVisible(); -await expect(adminPage.locator('role=heading[name="Welcome to Rocket.Chat"]')).not.toBeVisible(); +await expect(adminPage.getByRole('heading', { name: 'Welcome to Rocket.Chat' })).not.toBeVisible();Apply similar changes to all other
locator()usages in the file.As per coding guidelines, avoid using
page.locator()- always prefer semantic locators for better readability and maintainability.Also applies to: 52-52, 56-57, 61-61, 73-73, 77-78, 93-93, 97-98, 102-102, 125-125, 141-141, 165-165, 189-189, 211-211
🧹 Nitpick comments (6)
apps/meteor/tests/e2e/page-objects/home-discussion.ts (1)
21-35: Consider modernizing locator syntax to use Playwright's semantic selectors.The current implementation uses the older string-based
page.locator('role=...')syntax. Playwright's modern API provides dedicated semantic methods that are more readable and maintainable.🔎 Proposed refactor using modern Playwright API
get inputChannelName(): Locator { - return this.page.locator('role=textbox[name="Parent channel or team"]'); + return this.page.getByRole('textbox', { name: 'Parent channel or team' }); } get inputName(): Locator { - return this.page.locator('role=textbox[name="Name"]'); + return this.page.getByRole('textbox', { name: 'Name' }); } get inputMessage(): Locator { - return this.page.locator('role=textbox[name="Message"]'); + return this.page.getByRole('textbox', { name: 'Message' }); } get btnCreate(): Locator { - return this.page.locator('role=dialog >> role=group >> role=button[name="Create"]'); + return this.page.getByRole('dialog').getByRole('group').getByRole('button', { name: 'Create' }); }Based on coding guidelines and learnings.
apps/meteor/tests/e2e/federation/page-objects/account-profile.ts (1)
11-11: Existing selector also uses localized text.The
inputNamelocator uses an XPath selector with the localized string "Name", which has the same i18n fragility as the newly changed selectors. While this predates the current PR, consider refactoring it to use a stable selector for consistency:-return this.page.locator('//label[contains(text(), "Name")]/..//input'); +return this.page.getByLabel('Name'); // or use data-testid for stabilityNote:
getByLabelwith text still has i18n concerns; adata-testidwould be more stable.Based on learnings, prefer stable selectors over localized text for i18n compatibility.
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
123-123: Consider using semantic locator for main.The selector chain uses
this.page.locator('main')which could be replaced withthis.page.getByRole('main')for consistency with Playwright's semantic locator patterns.🔎 Proposed refactor
- return this.page.locator('main').getByRole('heading', { name: 'Home' }); + return this.page.getByRole('main').getByRole('heading', { name: 'Home' });As per coding guidelines.
apps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.ts (2)
44-44: Replace XPath with semantic selector.XPath selectors are harder to maintain and less readable than semantic selectors. Replace with
getByLabel():🔎 Proposed refactor
- await this.page.locator('//label[contains(text(), "Choose users")]/..//input').type(username); + await this.page.getByLabel(/Choose users/i).type(username);Note: This also uses localized text, so the same language-stability concern from line 23 applies here as well.
38-38: Consider semantic selectors instead of class-based selectors.These selectors rely on internal CSS class names which may change during refactoring. Consider:
- Adding
data-testidattributes to these elements for stable selection- Using semantic selectors where applicable (e.g.,
getByRole()for the danger button)Note: Lines 52-53 also use localized text ("Online", "All"), raising the same language-stability concerns as line 23.
Also applies to: 52-53
apps/meteor/tests/e2e/admin-room.spec.ts (1)
26-27: Consider extracting selectors to the AdminRooms page object.For consistency with the Page Object Model pattern used elsewhere in the codebase, these selectors could be extracted to getter methods in the
AdminRoomsclass:// In AdminRooms page object get pageHeading() { return this.page.getByRole('main').getByRole('heading', { level: 1, name: 'Rooms', exact: true }); } get roomsTable() { return this.page.getByRole('main').getByRole('table'); }Then use in the test:
await expect(adminRooms.pageHeading).toBeVisible(); await expect(adminRooms.roomsTable).toBeVisible();This improves reusability and maintains consistency with the pattern used in other tests (e.g., line 31 uses
adminRooms.inputSearchRooms).As per coding guidelines for Page Object Model pattern.
📜 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/views/admin/permissions/PermissionsTable/__snapshots__/PermissionsTable.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (45)
apps/meteor/client/components/UserCard/UserCard.tsxapps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsxapps/meteor/client/sidebar/RoomList/SidebarItemTemplateWithData.tsxapps/meteor/client/sidebar/RoomList/SidebarItemWithData.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/account/security/ChangePassphrase.tsxapps/meteor/client/views/account/security/ResetPassphrase.tsxapps/meteor/client/views/admin/engagementDashboard/EngagementDashboardCardErrorBoundary.tsxapps/meteor/client/views/admin/permissions/PermissionsPage.tsxapps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTableFilter.tsxapps/meteor/client/views/home/CustomHomePage.tsxapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/client/views/home/HomePageHeader.tsxapps/meteor/client/views/marketplace/components/AppInstallModal/AppInstallModal.tsxapps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsxapps/meteor/client/views/omnichannel/additionalForms/AutoCompleteUnits.tsxapps/meteor/client/views/omnichannel/agents/AgentEdit.tsxapps/meteor/client/views/omnichannel/agents/AgentInfo.tsxapps/meteor/client/views/omnichannel/components/AutoCompleteMultipleAgent.tsxapps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/omnichannel/departments/DepartmentAgentsTable/DepartmentAgentsTable.tsxapps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTable.tsxapps/meteor/client/views/omnichannel/reports/components/ReportCardErrorState.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsxapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsxapps/meteor/tests/e2e/admin-room.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/fragments/home-sidenav.tsapps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/settings-int.spec.tspackages/i18n/src/locales/en.i18n.jsonpackages/ui-client/src/components/Page/PageHeaderNoShadow.tsx
💤 Files with no reviewable changes (10)
- apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx
- apps/meteor/tests/e2e/page-objects/fragments/index.ts
- apps/meteor/client/sidebar/RoomList/SidebarItemWithData.tsx
- apps/meteor/client/views/navigation/sidebar/RoomList/SidebarItemWithData.tsx
- apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx
- apps/meteor/client/sidebar/RoomList/SidebarItemTemplateWithData.tsx
- apps/meteor/client/views/omnichannel/components/AutoCompleteMultipleAgent.tsx
- apps/meteor/client/views/omnichannel/additionalForms/AutoCompleteUnits.tsx
- apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts
- apps/meteor/tests/e2e/federation/page-objects/fragments/home-content.ts
🧰 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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/client/views/marketplace/components/AppInstallModal/AppInstallModal.tsxapps/meteor/client/views/home/CustomHomePage.tsxapps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTableFilter.tsxapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/client/views/account/security/ChangePassphrase.tsxapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/client/views/omnichannel/agents/AgentEdit.tsxapps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsxapps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsxapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/client/views/home/HomePageHeader.tsxapps/meteor/client/views/omnichannel/reports/components/ReportCardErrorState.tsxapps/meteor/client/views/account/security/ResetPassphrase.tsxapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTable.tsxapps/meteor/client/views/omnichannel/agents/AgentInfo.tsxapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/client/views/admin/engagementDashboard/EngagementDashboardCardErrorBoundary.tsxapps/meteor/client/components/UserCard/UserCard.tsxpackages/ui-client/src/components/Page/PageHeaderNoShadow.tsxapps/meteor/client/views/omnichannel/departments/DepartmentAgentsTable/DepartmentAgentsTable.tsxapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/client/views/admin/permissions/PermissionsPage.tsxapps/meteor/tests/e2e/page-objects/omnichannel-room-info.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/homepage.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/homepage.spec.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.ts
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/account-security.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.ts
🧠 Learnings (26)
📓 Common learnings
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.
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()`
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
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/`
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
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
📚 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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.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 : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/homepage.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 : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/client/views/home/CustomHomePage.tsxapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.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/admin-room.spec.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/page-objects/home-channel.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/client/views/home/CustomHomePage.tsxapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.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 : 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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/client/views/home/DefaultHomePage.tsxapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.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/admin-room.spec.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.spec.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.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/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/federation/page-objects/account-profile.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/homepage.spec.ts
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/tests/e2e/admin-room.spec.tsapps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/homepage.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 : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/settings-assets.spec.tsapps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.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 : Use `page.waitFor()` with specific conditions instead of hardcoded timeouts in Playwright tests
Applied to files:
apps/meteor/tests/e2e/settings-int.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/homepage.spec.ts
📚 Learning: 2025-10-30T19:30:46.541Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsxapps/meteor/client/views/account/security/ResetPassphrase.tsxapps/meteor/client/views/account/profile/AccountProfilePage.tsxapps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsxapps/meteor/client/views/admin/permissions/PermissionsPage.tsx
📚 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/account-security.tsapps/meteor/tests/e2e/page-objects/account-profile.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/page-objects/home-discussion.tsapps/meteor/tests/e2e/page-objects/omnichannel-room-info.ts
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/ResetPassphrase.tsx
📚 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/tests/e2e/federation/page-objects/fragments/home-flextab-members.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/page-objects/home-channel.tsapps/meteor/tests/e2e/page-objects/home-discussion.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-11-17T22:38:48.631Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 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:
packages/i18n/src/locales/en.i18n.json
📚 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/page-objects/home-discussion.tsapps/meteor/tests/e2e/homepage.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 : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/tests/e2e/homepage.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 **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/tests/e2e/homepage.spec.ts
🧬 Code graph analysis (2)
apps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTable.tsx (1)
packages/ui-client/src/components/GenericTable/GenericTableHeaderCell.tsx (1)
GenericTableHeaderCell(14-31)
apps/meteor/tests/e2e/page-objects/omnichannel-room-info.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/sidebar.ts (1)
RoomSidebar(21-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (36)
apps/meteor/tests/e2e/settings-assets.spec.ts (1)
16-16: Use semanticgetByRole()assertions with English text, following the established pattern in the test suite.This change aligns with the codebase convention—similar role-based assertions with hardcoded English text appear consistently throughout e2e tests (homepage.spec.ts, settings-int.spec.ts, admin-room.spec.ts, etc.). Tests are not configured for multi-language execution, and the migration from
data-qaselectors to semantic accessibility-driven locators is the intended approach across all settings and admin page tests.apps/meteor/tests/e2e/page-objects/home-discussion.ts (2)
3-3: LGTM! Import cleanup aligns with removedHomeSidenavclass.The removal of
HomeSidenavfrom the imports is consistent with the broader refactoring that deleted thehome-sidenav.tsfragment.
14-19: Sidenav removal is safe and verified.The comprehensive search across all test files found zero references to
sidenav, confirming that the removal of thesidenavmember from the constructor will not break any existing tests.apps/meteor/tests/e2e/page-objects/account-security.ts (1)
43-43: This change aligns with the project's established pattern and direction.The current implementation using semantic locators with localized text is consistent throughout the file (lines 19, 23, 27, 31, 35, 39) and across the page-objects directory in similar structures. The project is actively migrating away from
data-qaselectors toward semantic locators, as evidenced by recent commits in this area. While localized text can be fragile for i18n, the project's approach prioritizes accessible, semantic selectors per the coding guidelines.No changes needed.
Likely an incorrect or invalid review comment.
apps/meteor/client/views/omnichannel/agents/AgentInfo.tsx (3)
16-17: LGTM!Import reordering has no functional impact.
72-72: No E2E tests currently target this component.The AgentInfo component is not covered by E2E tests, so the removal of
data-qaattributes does not break any existing test infrastructure. If E2E tests are added in the future for agent information display, follow the project guidelines to use semantic locators (e.g.,getByRole(),getByLabel()) rather than custom attributes.
57-57: No action needed; UserInfoAvatar is not tested via data-qa attributes.UserInfoAvatar is a simple wrapper component around UserAvatar with no data-qa attributes. Since the component is not directly referenced in E2E tests and no data-qa selectors are associated with it, there are no test stability concerns from this change. The broader PR effort to replace data-qa selectors in E2E tests is separate from this component's usage.
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
3-3: Import cleanup looks correct.The removal of
HomeSidenavfrom imports aligns with the elimination of thesidenavmember and the deletion of the HomeSidenav page-object fragment.apps/meteor/tests/e2e/page-objects/omnichannel-room-info.ts (3)
3-3: LGTM!The import change from
HomeSidenavtoRoomSidebaris correct and aligns with the refactoring to use the updated fragment.
8-13: LGTM!The property rename from
homeSidenavtosidebarand the constructor update are internally consistent with the import change.
72-74: Localization stability concern with selector approach.The refactoring from
this.homeSidenavtothis.sidebaris correct. However,getBadgeIndicatorchainsgetSidebarItemByName(), which usesgetByRole('link', { name })andgetByText(name)with thenameparameter. Bothnameand thetitleparameter passed togetByTitle()are localized text, which may cause test failures when the UI language changes. While this appears to be an intentional shift toward semantic locators per the PR objectives, verify that the team accepts the localization stability tradeoff and confirm whether tests for localized content should be handled separately.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx (1)
37-37: This component was newly created without data-qa-id; no existing selector was removed.The review assumes
data-qa-id='UserUserInfo-menu'was removed, but the file was created fresh in commit d328ea1 without ever having this attribute. No E2E tests reference this selector anywhere in the codebase, so there are no test migrations or breaking changes to verify. The architectural decision to create this new component without data-qa-id is intentional and part of the broader refactoring effort.Likely an incorrect or invalid review comment.
apps/meteor/client/views/omnichannel/agents/AgentEdit.tsx (1)
125-125: No issues found. The removal ofdata-qa-idfrom the name field is intentional and follows a consistent pattern: read-only fields (name, username, email) use onlyidattributes for accessibility, while interactive fields (status, save button) retaindata-qa-idfor test selectors. Tests do not interact with read-only fields, so alternative selectors are not needed. The page object confirms this pattern with no getter defined for the name field.apps/meteor/client/views/omnichannel/reports/components/ReportCardErrorState.tsx (1)
16-16: LGTM - Verify replacement selectors in tests.The removal of the
data-qaattribute is consistent with the PR objective. The error state component maintains clear structure with semantic Fuselage components.Ensure that any E2E tests targeting this error state use stable, language-agnostic selectors rather than localized button text.
apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx (1)
224-224: LGTM - Verify save button selector stability in tests.The removal of
data-qa-idfrom the Save button aligns with the PR objective. The button retains all functional attributes and proper disabled state logic.Based on learnings, verify that replacement test selectors for this save button are stable and not reliant on translated button text, as this can cause test failures when UI language changes.
apps/meteor/client/views/omnichannel/departments/DepartmentAgentsTable/DepartmentAgentsTable.tsx (1)
28-28: LGTM - Good accessibility retained, verify test selector stability.The removal of
data-qafrom the AddAgent component aligns with PR objectives. The component appropriately retains thearia-labelledbyattribute for accessibility.Based on learnings, ensure that replacement test selectors are stable and not dependent on localized text to prevent test failures when UI language changes.
apps/meteor/client/views/admin/permissions/PermissionsTable/PermissionsTableFilter.tsx (1)
20-20: LGTM - Cosmetic formatting improvement.The change from multi-line to single-line JSX for the TextInput is purely stylistic. All props are preserved, and there are no functional changes.
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
199-199: LGTM - Verify E2EE password save button test selectors are stable.The removal of
data-qa-typefrom the Save button aligns with the PR objective. The button maintains all functional attributes and proper disabled state logic based on form validation.Based on learnings, ensure that replacement test selectors for this security-related save action are stable and not dependent on translated button text, as language-dependent selectors can cause test failures.
apps/meteor/client/views/omnichannel/customFields/CustomFieldsTable.tsx (1)
107-107: Verify that replacement test selectors for the custom fields table are stable.The removal of the
data-qaattribute aligns with the PR objective. The table retains good accessibility attributes (aria-busy,aria-live). Ensure that any E2E tests targeting this table use stable selectors (e.g.,data-qa-idattributes or semantic role-based locators) rather than localized UI text, as language-dependent selectors can cause test failures when the UI language changes.apps/meteor/client/views/home/HomePageHeader.tsx (1)
15-15: The removal ofdata-qa-id='home-header'has no impact on tests—this selector was never referenced in the test suite.Tests already rely on a stable semantic selector:
getByRole('heading', { level: 1, name: 'Home', exact: true }), which binds to the component's accessibility role and title prop. This selector is language-agnostic because the title value is dynamic (sourced from theLayout_Home_Titlesetting). The test suite validates that the header text updates correctly when the setting changes, confirming the selector strategy is robust across different UI languages.apps/meteor/client/views/account/security/ResetPassphrase.tsx (1)
17-17: LGTM!Clean removal of the test selector attribute. Functionality remains unchanged.
apps/meteor/client/views/marketplace/components/AppInstallModal/AppInstallModal.tsx (1)
71-71: LGTM!Clean removal of the test selector attribute. Functionality remains unchanged.
apps/meteor/client/views/admin/permissions/PermissionsPage.tsx (1)
68-70: LGTM!Clean removal of the test selector attribute with improved formatting. Functionality remains unchanged.
apps/meteor/client/views/home/DefaultHomePage.tsx (1)
27-31: LGTM!Clean removal of test selector attributes. Functionality remains unchanged.
apps/meteor/client/views/omnichannel/directory/chats/ChatsTable/ChatsTable.tsx (1)
66-66: LGTM!Clean removal of the test selector attribute. Functionality remains unchanged.
apps/meteor/client/views/account/profile/AccountProfilePage.tsx (1)
142-144: LGTM!Clean removal of the test selector attribute with improved formatting. Functionality remains unchanged.
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (1)
314-316: Verify localization stability of text-based selector.The new selector uses the localized text "User card" which may cause test failures if the UI language changes or if translations are updated.
Based on learnings, stable selectors like data-qa-id are preferred over localized text to prevent test fragility across different language settings.
Consider either:
- Ensuring tests always run in a fixed language (e.g., English)
- Using a non-localized attribute selector if stability across languages is required
- Adding language-specific test validation separately
Based on learnings, text-based selectors can break when UI language changes.
apps/meteor/tests/e2e/page-objects/account-profile.ts (1)
14-36: Verify localization stability of text-based selectors.Multiple selectors now use localized text ('Use URL for avatar', 'Add URL', 'Save changes', 'profile picture'), which may cause test failures if the UI language changes or translations are updated.
Based on learnings, stable selectors like data-qa-id are preferred over localized text to prevent test fragility across different language settings.
Consider either:
- Ensuring tests always run in a fixed language (e.g., English)
- Using non-localized attribute selectors if stability across languages is required
- Adding language-specific test validation separately
Based on learnings, text-based selectors can break when UI language changes.
packages/ui-client/src/components/Page/PageHeaderNoShadow.tsx (1)
40-42: LGTM!The removal of the
data-qa-typeattribute is appropriate and aligns with the PR objective to replace data-qa selectors with accessibility-driven approaches. The h1 semantic element and title content remain available for role-based selectors.apps/meteor/client/components/UserCard/UserCard.tsx (1)
52-52: LGTM! Accessibility improvement.The change from
data-qa='UserCard'totitle={t('User_card')}improves accessibility by providing a translated title for the dialog. This aligns well with the PR objective to use accessibility-driven selectors.apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx (1)
249-249: LGTM!The removal of
data-qa-typeattributes from the topic input and create button is appropriate. These elements remain accessible via semantic selectors:
- The topic input has a proper label association (
htmlFor={topicId})- The submit button can be selected by role and type attributes
Also applies to: 408-410
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx (1)
83-83: LGTM! Excellent accessibility improvement.Adding the
altattribute with a translated, descriptive label significantly improves accessibility compliance for screen readers. This change replaces the removeddata-qa-idattributes with a more semantic and user-friendly approach.apps/meteor/client/views/admin/engagementDashboard/EngagementDashboardCardErrorBoundary.tsx (1)
35-37: LGTM!The removal of the
data-qaattribute is appropriate. The error boundary's UI elements remain accessible through semantic selectors (the "Retry" button can be selected by role and text content).packages/i18n/src/locales/en.i18n.json (2)
5763-5763: VoIP beta alert copy looks goodText is clear, matches existing “beta” warnings in tone and casing, and is scoped specifically to internal SIP routing. No changes needed.
7084-7085: New profile‑picture and user‑card labels are consistent
__username__profile_picturecorrectly uses{{username}}interpolation and reads naturally as alt/aria text.User_cardlabel is simple and aligned with existing keys likeUser_Info.Just make sure the caller passes the actual username string into
{{username}}(not a full name) so the alt text matches what tests and users expect. Based on learnings, keeping these as English‑only additions here is the right workflow.apps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.ts (1)
10-12: No action needed:data-qaselector is correct and functional.The
data-qa="MemberItem-${username}"attribute exists in the RoomMembersItem component (apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx, line 69) and is properly used here. The locator is correctly encapsulated in the Page Object Model method following test conventions.
apps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab-members.ts
Show resolved
Hide resolved
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 46 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/tests/e2e/homepage.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/homepage.spec.ts:206">
P1: Case sensitivity bug: 'Rocket.chat' should be 'Rocket.Chat'. With `exact: true`, this selector will never match the actual heading text, making the test ineffective (it will always pass regardless of whether the element is visible).</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
dougfabris
changed the title
data-qadata-qa by accessible locators
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
dionisio-bot
bot
removed
the
stat: ready to merge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/page-objects/account-security.ts (1)
46-76: Complete the migration to semantic locators for consistency.While
securityHeader(line 43) was successfully migrated to usegetByRole(), the remaining locators in this file still usepage.locator()with CSS selectors or role selector strings. This creates inconsistency within the page object.As per coding guidelines, prefer semantic locators (
getByRole(),getByLabel(),getByText()) overpage.locator(). Consider completing the migration:
- Lines 47-56: Replace
page.locator('[role="button"]:has-text("...")')withpage.getByRole('button', { name: '...' })- Lines 59, 63: Replace
page.locator("role=button[name='...']")withpage.getByRole('button', { name: '...' })- Lines 67, 71: Can be simplified using
getByLabel()or nestedgetByRole()- Line 75: Replace
page.locator('dialog >> button')withpage.getByRole('dialog').getByRole('button')🔎 Example refactor for consistent semantic locators
get securityPasswordSection(): Locator { - return this.page.locator('[role="button"]:has-text("Password")'); + return this.page.getByRole('button', { name: 'Password' }); } get security2FASection(): Locator { - return this.page.locator('[role="button"]:has-text("Two Factor Authentication")'); + return this.page.getByRole('button', { name: 'Two Factor Authentication' }); } get securityE2EEncryptionSection(): Locator { - return this.page.locator('[role="button"]:has-text("End-to-end encryption")'); + return this.page.getByRole('button', { name: 'End-to-end encryption' }); } get securityE2EEncryptionResetKeyButton(): Locator { - return this.page.locator("role=button[name='Reset E2EE password']"); + return this.page.getByRole('button', { name: 'Reset E2EE password' }); } get securityE2EEncryptionSavePasswordButton(): Locator { - return this.page.locator("role=button[name='Save changes']"); + return this.page.getByRole('button', { name: 'Save changes' }); } get required2faModalSetUpButton(): Locator { - return this.page.locator('dialog >> button'); + return this.page.getByRole('dialog').getByRole('button'); }
📜 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/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/page-objects/fragments/index.ts
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/page-objects/fragments/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/account-security.ts
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/account-security.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/account-security.ts
🧠 Learnings (11)
📓 Common learnings
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.
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()`
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/`
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
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
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
📚 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/account-security.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/account-security.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/account-security.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/account-security.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/account-security.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/account-security.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/account-security.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/account-security.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 `page.waitFor()` with specific conditions instead of hardcoded timeouts in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/account-security.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/account-security.ts
⏰ 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 (1)
apps/meteor/tests/e2e/page-objects/account-security.ts (1)
42-44: The role-based locator usinggetByRole('heading', { level: 1, name: 'Security' })is correct and aligns with the PR objectives and coding guidelines. The hardcoded text 'Security' is acceptable here because the account-security tests do not currently include language-switching scenarios. If multilingual test support is added for this page in the future, the locators should be updated accordingly, but this is not a blocker for the current change.
juliajforesti
added
the
stat: ready to merge
dougfabris
added
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
CORE-1571
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Accessibility Improvements
Localization
Tests
✏️ Tip: You can customize this high-level summary in your review settings.