test: Add automated tests for custom fields functionality.#38743
test: Add automated tests for custom fields functionality.#38743Harmeet221 wants to merge 3 commits intodevelopfrom
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughAdds a new Playwright end-to-end test for omnichannel custom fields (room-scoped and visitor-scoped) and extends page object classes with accessors to locate room and visitor custom field UI elements and the contact info dialog. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38743 +/- ##
===========================================
- Coverage 70.51% 70.50% -0.02%
===========================================
Files 3176 3176
Lines 111139 111139
Branches 20050 20016 -34
===========================================
- Hits 78367 78355 -12
- Misses 30721 30729 +8
- Partials 2051 2055 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/12 22:57", "02/13 22:38", "02/16 14:04", "02/17 13:01 (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.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.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.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 17 Feb 2026 13:01:12 GMT |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts (1)
90-99: Reuse commonly used locators within the test.The same room custom field locator is queried multiple times; keep it in a constant and reuse to align with locator reuse guidance.
♻️ Suggested refactor
test('Should be allowed to set room custom field for a conversation', async () => { + const roomCustomFieldInput = poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel); + await test.step('Agent opens the conversation', async () => { await poHomeChannel.sidebar.getSidebarItemByName(visitor.name).click(); }); @@ await test.step('Agent fills room custom field and saves', async () => { - await poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel).fill(roomCustomFieldValue); + await roomCustomFieldInput.fill(roomCustomFieldValue); await poHomeChannel.editRoomInfo.btnSave.click(); }); @@ await test.step('Custom field should be updated successfully', async () => { await poHomeChannel.roomInfo.btnEdit.click(); await poHomeChannel.editRoomInfo.waitForDisplay(); - await expect(poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel)).toHaveValue(roomCustomFieldValue); + await expect(roomCustomFieldInput).toHaveValue(roomCustomFieldValue); }); });As per coding guidelines, "Store commonly used locators in variables/constants for reuse".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts` around lines 90 - 99, The test repeatedly calls poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel); extract that locator into a constant (e.g., const roomCustomField = poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel)) before the steps and then use roomCustomField.fill(roomCustomFieldValue) and the same roomCustomField in the assertion; keep other interactions (btnSave.click, roomInfo.btnEdit.click, editRoomInfo.waitForDisplay) unchanged and reference the new constant instead of repeated getRoomCustomField calls.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.tsapps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.ts
🧠 Learnings (13)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/edit-room-flextab.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-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 : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.ts
🪛 Biome (2.3.14)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
[error] 35-35: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
[error] 72-72: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (2)
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.ts (1)
88-90: Nice addition: room custom field accessor.Encapsulating the custom-field lookup behind the page object keeps test code clean and uses a semantic locator.
apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.ts (1)
42-48: Good accessors for contact info dialog and visitor fields.These additions keep custom-field access consistent and aligned with the page-object pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`:
- Around line 31-36: The test currently uses responses.forEach((res) =>
expect(res.status()).toBe(200)) which triggers the lint rule forbidding returns
from forEach callbacks; replace the forEach with an explicit loop (e.g., for
(const res of responses) { expect(res.status()).toBe(200); }) to make intent
clear and satisfy linting, and apply the same change to the other occurrence
around the responses/assertions at lines 70-72 (same responses variable created
from Promise.all of api.post calls).
- Around line 64-68: The test's beforeEach uses a non-semantic locator
page.locator('#main-content'); update the setup in test.beforeEach to use a
semantic Playwright locator instead (e.g., page.getByRole('main') or
page.getByTestId('main-content') or page.getByLabel(...) depending on the
markup) so the HomeOmnichannel initialization waits on an accessible element;
replace page.locator('#main-content').waitFor() with the appropriate semantic
call (keeping the await) to adhere to the locator guideline.
- Around line 14-77: The suite uses shared mutable state created in
test.beforeAll (roomCustomField, visitorCustomField, conversation) which breaks
parallel safety; move creation of createCustomField and createConversation into
test.beforeEach and corresponding deletion into test.afterEach (use
roomCustomField.delete(), visitorCustomField.delete(), conversation.delete()) so
each test gets isolated fixtures, or if sharing is unavoidable, convert the
suite to serial by using test.describe.serial instead of test.describe; update
references to test.beforeAll/test.afterAll accordingly.
---
Nitpick comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`:
- Around line 90-99: The test repeatedly calls
poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel); extract
that locator into a constant (e.g., const roomCustomField =
poHomeChannel.editRoomInfo.getRoomCustomField(roomCustomFieldLabel)) before the
steps and then use roomCustomField.fill(roomCustomFieldValue) and the same
roomCustomField in the assertion; keep other interactions (btnSave.click,
roomInfo.btnEdit.click, editRoomInfo.waitForDisplay) unchanged and reference the
new constant instead of repeated getRoomCustomField calls.
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
Outdated
Show resolved
Hide resolved
| const responses = await Promise.all([ | ||
| api.post('/livechat/users/agent', { username: 'user1' }), | ||
| api.post('/livechat/users/manager', { username: 'user1' }), | ||
| ]); | ||
| responses.forEach((res) => expect(res.status()).toBe(200)); | ||
|
|
There was a problem hiding this comment.
Avoid returning from forEach callbacks (lint error).
Use a loop to satisfy the lint rule and make intent explicit.
🧹 Proposed fix
- responses.forEach((res) => expect(res.status()).toBe(200));
+ for (const res of responses) {
+ expect(res.status()).toBe(200);
+ }
@@
- responses.forEach((res) => expect(res.status()).toBe(200));
+ for (const res of responses) {
+ expect(res.status()).toBe(200);
+ }Also applies to: 70-72
🧰 Tools
🪛 Biome (2.3.14)
[error] 35-35: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`
around lines 31 - 36, The test currently uses responses.forEach((res) =>
expect(res.status()).toBe(200)) which triggers the lint rule forbidding returns
from forEach callbacks; replace the forEach with an explicit loop (e.g., for
(const res of responses) { expect(res.status()).toBe(200); }) to make intent
clear and satisfy linting, and apply the same change to the other occurrence
around the responses/assertions at lines 70-72 (same responses variable created
from Promise.all of api.post calls).
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts (1)
124-137: Assertion usestoContainTexton the entire dialog — this is fragile.
toContainText(visitorCustomFieldValue)on the entire dialog root will match if the text appears anywhere in the dialog, which could produce false positives if the value happens to coincide with other dialog content. Consider asserting directly on the custom field element for a more precise check:Suggested improvement
await test.step('Assert custom field is set successfully', async () => { - await expect(poHomeChannel.contacts.contactInfo.dialogContactInfo).toContainText(visitorCustomFieldValue); + await expect(poHomeChannel.contacts.contactInfo.getVisitorCustomField(visitorCustomFieldLabel)).toHaveValue(visitorCustomFieldValue); });Note: If the field is rendered as read-only text rather than an input,
toContainTextscoped to the field element would still be more precise than asserting on the whole dialog. Same concern applies to line 158.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts` around lines 124 - 137, The test is asserting visitorCustomFieldValue against the whole dialog (poHomeChannel.contacts.contactInfo.dialogContactInfo), which is fragile; change the assertion to target the specific custom-field element instead. Use the contact-info helper to get the field element (e.g. poHomeChannel.contacts.contactInfo.getCustomFieldElement(customFieldName)) or scope a locator off dialogContactInfo such as dialogContactInfo.locator('selector-for-custom-field' or `:has-text("${customFieldLabel}")`). Then replace expect(...dialogContactInfo).toContainText(visitorCustomFieldValue) with expect(fieldElement).toHaveText(visitorCustomFieldValue) (and apply the same change for the other occurrence mentioned in the comment).apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.ts (1)
42-48:dialogContactInfois a trivial alias forthis.root.Consider whether exposing
this.rootdirectly under a new name adds clarity vs. just usingthis.rootin the base class. That said, it does give test call-sites a more descriptive name, so this is acceptable as-is.
getVisitorCustomFieldfollows the same pattern as the room counterpart — looks good.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.ts` around lines 42 - 48, The property dialogContactInfo is a trivial alias of this.root; either remove it and update all call sites to use this.root directly, or keep it but add a short JSDoc on dialogContactInfo explaining it’s an intentional, more descriptive alias for the root locator; leave getVisitorCustomField(label: string) as-is. Ensure you update any tests referencing dialogContactInfo if you choose removal, or add the JSDoc above the dialogContactInfo getter if you keep it.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.tsapps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Focus on comprehensive feature coverage and edge case validation in test design
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.tsapps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-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 : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts (3)
apps/meteor/tests/mocks/data.ts (1)
createFakeVisitor(346-351)apps/meteor/tests/e2e/page-objects/home-omnichannel.ts (1)
HomeOmnichannel(14-52)apps/meteor/tests/data/livechat/custom-fields.ts (1)
createCustomField(8-33)
🪛 Biome (2.3.14)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
[error] 35-35: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
[error] 72-72: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (4)
apps/meteor/tests/e2e/page-objects/fragments/edit-room-flextab.ts (1)
87-90: LGTM!Clean addition following the existing page object pattern and using a semantic locator (
getByLabel).apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts (3)
79-100: Good use oftest.step()for organizing the room custom field flow.The steps are clearly named and the assertion uses web-first
toHaveValue. Follows the page object model correctly.
139-160: Remove this comment —btnSaveis properly scoped and available.The
btnSavelocator is inherited fromFlexTaband scoped tothis.root, which is initialized inOmnichannelContactInfoas the persistent dialog{ name: 'Contact' }. BothbtnEditandbtnSaveremain scoped to the same root element throughout the operation, so the locator chain will resolve correctly without transitioning to a different dialog.Likely an incorrect or invalid review comment.
49-53: The concern aboutagentId: 'user1'is not valid. The API endpoint (GET /livechat/room) acceptsagentIdas a string and passes it tonormalizeAgent(), which callsUsers.getAgentInfo(agentId). This function accepts both user IDs and usernames, successfully resolving 'user1' to the corresponding user. If the agent is not found, the room is created without agent assignment by design—this is intentional behavior, not a silent failure. The widespread use of this pattern across 15+ e2e test files confirms it is the standard, tested approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`:
- Around line 64-68: Replace the non-semantic locator used in the test setup:
inside test.beforeEach where page.locator('#main-content').waitFor() is called,
switch to a semantic Playwright locator such as page.getByRole('main').waitFor()
(or another appropriate getByRole/getByTestId used by HomeOmnichannel) to adhere
to the guideline of preferring semantic locators; update the wait call in the
same test.beforeEach block so HomeOmnichannel initialization and subsequent
actions rely on the semantic locator.
- Line 14: The test suite declared with test.describe('OC - Custom fields usage,
scope : room and visitor') has sequential dependencies between tests (1→2 and
3→4); change the suite declaration to use test.describe.serial so the tests run
sequentially rather than in parallel; update the suite definition (the
test.describe call) to test.describe.serial(...) to ensure deterministic order
for the tests in this file.
- Line 35: Replace the Array.forEach callback that calls expect on each response
with a for...of loop to avoid implicit-return lint errors: iterate over the
responses array using "for (const res of responses)" and call
expect(res.status()).toBe(200) inside the loop; apply the same change to the
similar occurrence around line 72 that also iterates over responses and calls
expect. Ensure you update both spots that reference responses and res.status()
so the lint rule no longer flags the implicit return from the forEach callback.
---
Nitpick comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`:
- Around line 124-137: The test is asserting visitorCustomFieldValue against the
whole dialog (poHomeChannel.contacts.contactInfo.dialogContactInfo), which is
fragile; change the assertion to target the specific custom-field element
instead. Use the contact-info helper to get the field element (e.g.
poHomeChannel.contacts.contactInfo.getCustomFieldElement(customFieldName)) or
scope a locator off dialogContactInfo such as
dialogContactInfo.locator('selector-for-custom-field' or
`:has-text("${customFieldLabel}")`). Then replace
expect(...dialogContactInfo).toContainText(visitorCustomFieldValue) with
expect(fieldElement).toHaveText(visitorCustomFieldValue) (and apply the same
change for the other occurrence mentioned in the comment).
In `@apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-info.ts`:
- Around line 42-48: The property dialogContactInfo is a trivial alias of
this.root; either remove it and update all call sites to use this.root directly,
or keep it but add a short JSDoc on dialogContactInfo explaining it’s an
intentional, more descriptive alias for the root locator; leave
getVisitorCustomField(label: string) as-is. Ensure you update any tests
referencing dialogContactInfo if you choose removal, or add the JSDoc above the
dialogContactInfo getter if you keep it.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts (3)
134-136: Consider using the field-specific locator for a more precise assertion.
toContainTexton the entire dialog is a broad check. You already havegetVisitorCustomField(visitorCustomFieldLabel)available — using it here (e.g., checkingtoHaveTextortoContainTexton the specific field element) would make the assertion more targeted and less prone to false positives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts` around lines 134 - 136, The assertion is too broad: replace the generic check on poHomeChannel.contacts.contactInfo.dialogContactInfo with a targeted assertion against the specific custom-field locator returned by getVisitorCustomField(visitorCustomFieldLabel); call getVisitorCustomField(visitorCustomFieldLabel) and assert its text equals/contains visitorCustomFieldValue (e.g., toHaveText or toContainText) inside the same test.step 'Assert custom field is set successfully' to make the check precise and avoid false positives.
157-159: Same broad assertion pattern — prefer field-specific locator.Same as the previous test: asserting
toContainTexton the entire dialog. Consider usinggetVisitorCustomField(visitorCustomFieldLabel)for a targeted assertion on the updated value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts` around lines 157 - 159, The test currently asserts the whole dialog (`poHomeChannel.contacts.contactInfo.dialogContactInfo`) contains the updated value which is broad; replace that with a targeted assertion using the `getVisitorCustomField(visitorCustomFieldLabel)` locator: call `poHomeChannel.contacts.contactInfo.getVisitorCustomField(visitorCustomFieldLabel)` and assert it contains/equals `updatedVisitorCustomFieldValue` so the check is specific to the updated custom field.
17-23: Redundant variable aliases add confusion.
roomCustomFieldNameis identical toroomCustomFieldLabel, and likewise forvisitorCustomFieldName/visitorCustomFieldLabel. Using two names for the same value obscures the intent — readers have to verify they're equal. Consider using a single variable per custom field (e.g., justroomCustomFieldLabel) and passing it to bothfieldandlabelparameters directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts` around lines 17 - 23, The test defines redundant aliases roomCustomFieldName and visitorCustomFieldName that are identical to roomCustomFieldLabel and visitorCustomFieldLabel; remove the duplicate variables and use the single label variables (roomCustomFieldLabel and visitorCustomFieldLabel) when passing values to both the label and field parameters in the test flow (search for usages of roomCustomFieldName and visitorCustomFieldName and replace them with roomCustomFieldLabel/visitorCustomFieldLabel, keeping roomCustomFieldValue and visitorCustomFieldValue unchanged).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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
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/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
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 : Implement proper wait strategies for dynamic content 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 : Use `test.step()` for complex test scenarios to improve organization 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 : Maintain test isolation between test cases 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/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.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/omnichannel/omnichannel-custom-field-usage.spec.ts
🪛 Biome (2.3.14)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
[error] 35-35: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
[error] 72-72: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
⏰ 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 (2)
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts (2)
14-14: Good use oftest.describe.serialand proper setup/teardown pattern.The serial execution mode correctly addresses the shared state between tests. The
beforeEachproperly reinitializes the page object per test, andbeforeAll/afterAllhandle resource lifecycle cleanly.Also applies to: 64-68
84-88: The code at lines 84-88 is correct. Room info automatically displays when selecting an omnichannel conversation, andwaitForDisplay()is the appropriate way to wait for it. This pattern is consistent across multiple omnichannel test files (omnichannel-sla-policies-sidebar.spec.ts,omnichannel-assign-room-tags.spec.ts, etc.), all of which follow the same approach of clicking a sidebar item and then interacting with the now-visible room info panel without additional setup steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`:
- Line 35: Replace the responses.forEach callback (which causes the
useIterableCallbackReturn lint error) with an explicit for...of loop: iterate
using "for (const res of responses)" and call expect(res.status()).toBe(200)
inside the loop body; do the same replacement for the second occurrence around
the earlier mentioned Line 72 so both instances use for...of instead of
Array.prototype.forEach.
---
Nitpick comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts`:
- Around line 134-136: The assertion is too broad: replace the generic check on
poHomeChannel.contacts.contactInfo.dialogContactInfo with a targeted assertion
against the specific custom-field locator returned by
getVisitorCustomField(visitorCustomFieldLabel); call
getVisitorCustomField(visitorCustomFieldLabel) and assert its text
equals/contains visitorCustomFieldValue (e.g., toHaveText or toContainText)
inside the same test.step 'Assert custom field is set successfully' to make the
check precise and avoid false positives.
- Around line 157-159: The test currently asserts the whole dialog
(`poHomeChannel.contacts.contactInfo.dialogContactInfo`) contains the updated
value which is broad; replace that with a targeted assertion using the
`getVisitorCustomField(visitorCustomFieldLabel)` locator: call
`poHomeChannel.contacts.contactInfo.getVisitorCustomField(visitorCustomFieldLabel)`
and assert it contains/equals `updatedVisitorCustomFieldValue` so the check is
specific to the updated custom field.
- Around line 17-23: The test defines redundant aliases roomCustomFieldName and
visitorCustomFieldName that are identical to roomCustomFieldLabel and
visitorCustomFieldLabel; remove the duplicate variables and use the single label
variables (roomCustomFieldLabel and visitorCustomFieldLabel) when passing values
to both the label and field parameters in the test flow (search for usages of
roomCustomFieldName and visitorCustomFieldName and replace them with
roomCustomFieldLabel/visitorCustomFieldLabel, keeping roomCustomFieldValue and
visitorCustomFieldValue unchanged).
1 participant
JIRA TASK: https://rocketchat.atlassian.net/browse/QA-111
Description:
Added comprehensive end-to-end test coverage for omnichannel custom fields, including room and visitor-scoped field creation, updates, and validation.
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit