test: Add e2e test for change password in account security page #37765
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughRefactors E2E account security tests and page objects: introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📦 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 12:45", "12/11 15:40 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 18 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Thu, 11 Dec 2025 15:40:20 GMT |
dougfabris
force-pushed
the
test/change-password
branch
from
00aa07f to
ffd1b32
Compare
dougfabris
force-pushed
the
test/change-password
branch
from
ffd1b32 to
0130b50
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37765 +/- ##
===========================================
- Coverage 67.69% 67.69% -0.01%
===========================================
Files 3452 3452
Lines 113983 113983
Branches 20943 20945 +2
===========================================
- Hits 77163 77162 -1
+ Misses 34693 34692 -1
- Partials 2127 2129 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
apps/meteor/tests/e2e/page-objects/fragments/sidebar.ts (1)
111-125: AccountSidebar follows existing sidebar patterns; consider de-duplicating close()
AccountSidebarcorrectly scopes to the Account navigation region and exposes a semanticlinkSecuritylocator, and itsclose()implementation mirrorsAdminSidebar.close(). This fits the existing fragment hierarchy and POM approach.Since both
AdminSidebarandAccountSidebarnow share the sameclose()logic (click close button, wait for dismissal), you could optionally liftclose()into the baseSidebarclass and drop the duplicated method bodies, unless you expect sidebar-specific variants later.apps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.ts (1)
1-27: EnterPasswordModal is a solid modal page-object; align type imports with the rest of the suiteThe modal abstraction looks good: it scopes to the 'Please enter your password' dialog, uses
getByRoleselectors for the password field and Verify button, andenterPasswordwaits for dismissal, which matches the “web‑first” wait strategy guidelines. This integrates cleanly withAccountSecurity.changePassword.Two small consistency nits you may consider:
- Other page-objects in this tree typically import
Pagefrom@playwright/testrather thanplaywright-core. For consistency and to avoid mixing type sources, consider switching the import here to@playwright/test.- If
toastMessagesis not used by any consumer yet, you can either remove it for now or start asserting modal-level toasts through it; keeping only the pieces that are exercised tends to keep page-objects easier to maintain.Example for the type import tweak:
-import type { Page } from 'playwright-core'; +import type { Page } from '@playwright/test';apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
80-89: Consider consistent naming withpoAccountSecurityprefix.The variable is named
accountSecurityPagehere, but other files in this PR (and the second describe block in this file at line 185) usepoAccountSecurity. Consider aligning for consistency across the test suite.- const accountSecurityPage = new AccountSecurity(page); + const poAccountSecurity = new AccountSecurity(page); const loginPage = new LoginPage(page); // Reset the E2EE key to start the flow from the beginning - await accountSecurityPage.goto(); - await accountSecurityPage.resetE2EEPassword(); + await poAccountSecurity.goto(); + await poAccountSecurity.resetE2EEPassword();
115-137: Same naming inconsistency as noted above.Apply the same
poAccountSecuritynaming convention here for consistency.- const accountSecurityPage = new AccountSecurity(page); + const poAccountSecurity = new AccountSecurity(page);And update the references on lines 135-137 accordingly.
apps/meteor/tests/e2e/page-objects/account-security.ts (4)
49-51: Prefer semantic locators over CSS selectors.Per coding guidelines, avoid
page.locator()and prefer semantic locators likepage.getByRole().get securityHeader(): Locator { - return this.page.locator('h1[data-qa-type="PageHeader-title"]:has-text("Security")'); + return this.page.getByRole('heading', { name: 'Security', level: 1 }); }
53-63: UsegetByRolefor button locators.These section buttons can use semantic locators for better readability and resilience.
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' }); }
65-71: Convert role selector strings togetByRole.The role selector syntax inside
page.locator()can be replaced with the more idiomaticgetByRole().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' }); }
81-83: Use more specific semantic locator for modal button.The current locator
'dialog >> button'is fragile and may match multiple buttons. Consider using a more specific selector.get required2faModalSetUpButton(): Locator { - return this.page.locator('dialog >> button'); + return this.page.getByRole('dialog').getByRole('button', { name: /set up/i }); }If the button text varies, verify the actual button name and adjust accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
apps/meteor/client/views/account/AccountSidebar.tsx(1 hunks)apps/meteor/tests/e2e/access-security-page.spec.ts(0 hunks)apps/meteor/tests/e2e/account-profile.spec.ts(0 hunks)apps/meteor/tests/e2e/account-security.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts(2 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(6 hunks)apps/meteor/tests/e2e/enforce-2FA.spec.ts(5 hunks)apps/meteor/tests/e2e/page-objects/account-profile.ts(0 hunks)apps/meteor/tests/e2e/page-objects/account-security.ts(2 hunks)apps/meteor/tests/e2e/page-objects/account.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/account-sidenav.ts(0 hunks)apps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/sidebar.ts(1 hunks)apps/meteor/tests/e2e/page-objects/index.ts(1 hunks)apps/meteor/tests/e2e/utils/index.ts(1 hunks)apps/meteor/tests/e2e/utils/updateOwnUserInfo.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/meteor/tests/e2e/account-profile.spec.ts
- apps/meteor/tests/e2e/page-objects/fragments/account-sidenav.ts
- apps/meteor/tests/e2e/page-objects/account-profile.ts
- apps/meteor/tests/e2e/access-security-page.spec.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/page-objects/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/client/views/account/AccountSidebar.tsxapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/utils/updateOwnUserInfo.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.spec.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/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/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/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/utils/updateOwnUserInfo.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.spec.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/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/enforce-2FA.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/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/enforce-2FA.spec.ts
🧠 Learnings (19)
📓 Common learnings
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
📚 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/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/page-objects/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/page-objects/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/index.tsapps/meteor/tests/e2e/page-objects/fragments/sidebar.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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 tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/page-objects/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/utils/updateOwnUserInfo.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/page-objects/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/page-objects/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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 : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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/page-objects/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/page-objects/account-security.tsapps/meteor/tests/e2e/enforce-2FA.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 : 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/sidebar.tsapps/meteor/tests/e2e/page-objects/account.tsapps/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 `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.tsapps/meteor/tests/e2e/enforce-2FA.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/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/account-security.spec.tsapps/meteor/tests/e2e/enforce-2FA.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/e2e-encryption/e2ee-passphrase-management.spec.tsapps/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 `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/tests/e2e/account-security.spec.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/tests/e2e/utils/updateOwnUserInfo.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, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.
Applied to files:
apps/meteor/tests/e2e/utils/updateOwnUserInfo.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/enter-password-modal.ts
🧬 Code graph analysis (6)
apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)
apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurity(6-103)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (1)
apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurity(6-103)
apps/meteor/tests/e2e/account-security.spec.ts (2)
apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurity(6-103)apps/meteor/tests/e2e/utils/updateOwnUserInfo.ts (1)
updateOwnUserPassword(8-16)
apps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/toast-messages.ts (1)
ToastMessages(5-29)
apps/meteor/tests/e2e/page-objects/account.ts (3)
packages/core-services/src/index.ts (1)
Account(161-161)apps/meteor/tests/e2e/page-objects/fragments/toast-messages.ts (1)
ToastMessages(5-29)apps/meteor/tests/e2e/page-objects/fragments/sidebar.ts (1)
AccountSidebar(112-125)
apps/meteor/tests/e2e/enforce-2FA.spec.ts (2)
apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurity(6-103)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(7-124)
🪛 GitHub Check: CodeQL
apps/meteor/tests/e2e/utils/updateOwnUserInfo.ts
[failure] 14-14: Use of password hash with insufficient computational effort
Password from an access to currentPassword is hashed insecurely.
Password from an access to currentPassword is hashed insecurely.
🔇 Additional comments (12)
apps/meteor/client/views/account/AccountSidebar.tsx (1)
20-25: Accessible aria-label on Sidebar looks correctUsing
aria-label={t('Account')}aligns with the visible header title and improves screen-reader semantics for this navigation region. No issues from a typing or behavioral perspective as long asSidebar’s newaria-labelprop is typed as optionalstring.apps/meteor/tests/e2e/page-objects/index.ts (1)
2-2: AccountSecurity page-object barrel export looks correctAdding
export * from './account-security';keeps the page-objects index in sync with the new AccountSecurity class and allows tests to import it from the single barrel entry point. No issues.apps/meteor/tests/e2e/utils/updateOwnUserInfo.ts (1)
1-19: Password update helper matches API contract and is safely scoped to testsThe
updateOwnUserPassword+updateOwnUserInfopairing is clean: it hashescurrentPasswordonce with SHA‑256, passes it ascurrentPasswordalongsidenewPassword, and posts{ data }with a correctly typed payload (UsersUpdateOwnBasicInfoParamsPOST['data']). Given this is test-side client hashing for a specific 2FA flow (not storage), the implementation is appropriate, and the explanatory comment documents the security context well.apps/meteor/tests/e2e/utils/index.ts (1)
5-5: Re-export ofupdateOwnUserInfoutilities is consistentExposing
./updateOwnUserInfothrough the utils barrel keeps consumers from reaching into file paths directly and aligns with existing exports.apps/meteor/tests/e2e/page-objects/account.ts (1)
3-17: Account base now cleanly exposes shared sidebar and save button locatorsAdding
readonly sidebar: AccountSidebarand the protectedsaveChangesButtongetter centralizes common account-page locators in the base class and uses semanticgetByRoleselectors, which is exactly what the page-object guidelines recommend. This also keepsAccountSecuritylean by reusing these shared interactions.apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)
5-6: Migration from AccountProfile to AccountSecurity is consistentSwitching to
AccountSecurityhere and usingsecurityE2EEncryptionSection/securityE2EEncryptionResetKeyButtonmatches the new page-object API and keeps the flow identical (navigate to security, open E2EE section, trigger reset, expect logout). The refactor is minimal and preserves the original behavior.Also applies to: 41-46
apps/meteor/tests/e2e/enforce-2FA.spec.ts (2)
1-19: LGTM! Clean migration to AccountSecurity page object.The import and initialization changes correctly follow the Page Object Model pattern. The naming convention
poAccountSecurityis consistent with other page objects in the file (poHomeChannel).
51-65: Well-structured test with proper web-first assertions.The test correctly uses
toBeVisible,toHaveAttribute, andnot.toBeVisibleassertions as per Playwright best practices. The flow testing 2FA setup is clear and follows the expected user journey.apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
184-194: Proper Page Object Model pattern with shared instance.Good use of
let poAccountSecurityat the describe block level with initialization inbeforeEach. This ensures clean state for each test while following the established naming convention.
205-209: LGTM! Correct usage of AccountSecurity page object properties.The interactions with
securityE2EEncryptionSectionandsecurityE2EEncryptionResetKeyButtoncorrectly use the new AccountSecurity API.apps/meteor/tests/e2e/page-objects/account-security.ts (2)
26-31: Well-designed password change flow encapsulation.The
changePasswordmethod cleanly orchestrates the password change workflow: fill fields → save → enter current password via modal. Good use of composition withEnterPasswordModal.
6-12: Clean class structure following Page Object Model.The class properly extends
Account, composesEnterPasswordModal, and initializes via constructor. This follows the established POM pattern in the codebase. Based on learnings, this aligns with the consistent Page Object Model pattern.
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/meteor/tests/e2e/page-objects/account-security.ts (2)
58-64: Replacepage.locator()with semantic locators and follow naming convention.These button getters have two issues:
- They use
page.locator()instead of semanticgetByRole()method- Per past review feedback, button locators should start with
btnprefixApply this diff:
-get securityE2EEncryptionResetKeyButton(): Locator { - return this.page.locator("role=button[name='Reset E2EE password']"); +get btnSecurityE2EEncryptionResetKey(): Locator { + return this.page.getByRole('button', { name: 'Reset E2EE password' }); } -get securityE2EEncryptionSavePasswordButton(): Locator { - return this.page.locator("role=button[name='Save changes']"); +get btnSecurityE2EEncryptionSavePassword(): Locator { + return this.page.getByRole('button', { name: 'Save changes' }); }Based on learnings, semantic locators should be preferred, and past review comments indicate button locators should follow the
btnprefix convention.
74-76: Replacepage.locator()with semantic locator and follow naming convention.This getter has two issues:
- It uses
page.locator()with a CSS selector instead of a semantic locator- Per past review feedback, button locators should start with
btnprefixApply this diff:
-get required2faModalSetUpButton(): Locator { - return this.page.locator('dialog >> button'); +get btnRequired2faModalSetUp(): Locator { + return this.page.getByRole('dialog').getByRole('button'); }Based on learnings, semantic locators should be preferred, and past review comments indicate button locators should follow the
btnprefix convention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/tests/e2e/page-objects/account-security.ts(1 hunks)
🧰 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 (12)
📓 Common learnings
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/**/*.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
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
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()`
📚 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-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/**/*.{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 : 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/**/*.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 : 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 : Group related tests in the same file
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 : Ensure tests run reliably in parallel without shared state conflicts
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-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/account-security.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/page-objects/account-security.ts (2)
packages/core-services/src/index.ts (1)
Account(161-161)apps/meteor/tests/e2e/page-objects/fragments/enter-password-modal.ts (1)
EnterPasswordModal(6-27)
⏰ 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/tests/e2e/page-objects/account-security.ts (3)
1-12: LGTM!The imports and constructor properly follow the Page Object Model pattern, correctly extending the
Accountbase class and initializing theEnterPasswordModalfragment.
18-40: LGTM!All input and button getters correctly use semantic locators (
getByRole) as per coding guidelines, and button locators follow thebtnprefix naming convention.
78-102: LGTM!All methods properly implement their respective workflows using the page object pattern, correctly delegating to internal getters, fragments, and the sidebar for navigation.
Proposed changes (including videos or screenshots)
Relates to #37745
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.