test: Missing department removal setting in Omnichannel tests#39371
Merged
dionisio-bot[bot] merged 3 commits intodevelopfrom Mar 17, 2026
Merged
test: Missing department removal setting in Omnichannel tests#39371dionisio-bot[bot] merged 3 commits intodevelopfrom
dionisio-bot[bot] merged 3 commits intodevelopfrom
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
WalkthroughMultiple Omnichannel E2E tests now use the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 #39371 +/- ##
===========================================
- Coverage 70.89% 70.88% -0.01%
===========================================
Files 3207 3209 +2
Lines 113334 113890 +556
Branches 20538 20660 +122
===========================================
+ Hits 80349 80736 +387
- Misses 30940 31102 +162
- Partials 2045 2052 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dougfabris
force-pushed
the
test/departments-tears-down
branch
from
1ed5327 to
01bbe0c
Compare
dougfabris
force-pushed
the
test/departments-tears-down
branch
from
fa8e2f2 to
b382985
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts:37">
P2: Reset the global setting in a `finally` block so cleanup failures do not leak state into later tests.</violation>
</file>
<file name="apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts:137">
P1: This reset is not guaranteed to run. If any earlier cleanup in the preceding `Promise.all` fails, `afterAll` exits before restoring `Omnichannel_enable_department_removal`, leaking test state into later suites.</violation>
</file>
<file name="apps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.ts:38">
P2: Restore this global setting in a `finally` path; if any preceding cleanup fails, the suite currently leaves department removal enabled for later tests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts (1)
371-378:⚠️ Potential issue | 🟡 MinorEnable the department removal setting in Manager role tests.
The Manager role
test.afterAll()deletes departments but never togglesOmnichannel_enable_department_removal. The Monitor role tests in this file correctly enable the setting before cleanup and disable it afterwards. This pattern is also followed consistently across all other omnichannel test files (omnichannel-manager-role.spec.ts,omnichannel-units.spec.ts,omnichannel-tags.spec.ts,omnichannel-departaments.spec.ts, etc.). Add the setting toggle to the Manager roletest.beforeAll()andtest.afterAll()to match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts` around lines 371 - 378, The Manager role tests delete departments in test.afterAll() but never enable the Omnichannel_enable_department_removal setting; add a test.beforeAll() that sets Omnichannel_enable_department_removal to true and a test.afterAll() that resets it to false (mirroring the Monitor role pattern). Locate the Manager role suite setup/teardown surrounding the managers, agents, departments arrays (functions test.beforeAll and test.afterAll in omnichannel-chat-transfers.spec.ts) and call the same settings helper or API used in the Monitor tests to toggle "Omnichannel_enable_department_removal" before creating/deleting departments and revert it after cleanup. Ensure the enable call runs before any department creation and the disable call runs after all delete() calls complete.
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts (1)
27-33: Prefer the shared setting helper here for consistency.Line 28 and Line 32 use raw
/settings/...posts, while sibling Omnichannel specs in this PR usesetSettingValueById. Unifying this avoids duplicated setting-write patterns.Proposed refactor
import { OmnichannelAgents } from '../page-objects/omnichannel'; +import { setSettingValueById } from '../utils'; import { createDepartment } from '../utils/omnichannel/departments'; import { test, expect } from '../utils/test'; @@ test.beforeAll(async ({ api }) => { - expect((await api.post('/settings/Omnichannel_enable_department_removal', { value: true })).status()).toBe(200); + await setSettingValueById(api, 'Omnichannel_enable_department_removal', true); }); @@ test.afterAll(async ({ api }) => { - expect((await api.post('/settings/Omnichannel_enable_department_removal', { value: false })).status()).toBe(200); + await setSettingValueById(api, 'Omnichannel_enable_department_removal', false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts` around lines 27 - 33, Replace the raw api.post calls in test.beforeAll and test.afterAll with the shared helper setSettingValueById for consistency; specifically, in the test.beforeAll that currently calls api.post('/settings/Omnichannel_enable_department_removal', { value: true }) and the test.afterAll that calls api.post(..., { value: false }), import/use the setSettingValueById helper and call it with the api fixture and the setting id "Omnichannel_enable_department_removal" and true/false respectively, then remove the direct api.post calls so the tests use setSettingValueById throughout.
🤖 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-monitor-department.spec.ts`:
- Around line 71-79: The afterAll teardown must always reset the setting even if
deletes fail: wrap the deletion logic inside a try block and call
setSettingValueById(api, 'Omnichannel_enable_department_removal', false) in a
finally block so it always runs; update the test.afterAll that currently awaits
Promise.all([...agents.map(agent => agent.delete()),
...departments.map(department => department.delete()), ...units.map(unit =>
unit.delete()), monitor.delete()]) to instead perform the Promise.all within try
and move the setSettingValueById call into finally, referencing the existing
symbols test.afterAll, agents, departments, units, monitor.delete, and
setSettingValueById(api, 'Omnichannel_enable_department_removal', false).
---
Outside diff comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts`:
- Around line 371-378: The Manager role tests delete departments in
test.afterAll() but never enable the Omnichannel_enable_department_removal
setting; add a test.beforeAll() that sets Omnichannel_enable_department_removal
to true and a test.afterAll() that resets it to false (mirroring the Monitor
role pattern). Locate the Manager role suite setup/teardown surrounding the
managers, agents, departments arrays (functions test.beforeAll and test.afterAll
in omnichannel-chat-transfers.spec.ts) and call the same settings helper or API
used in the Monitor tests to toggle "Omnichannel_enable_department_removal"
before creating/deleting departments and revert it after cleanup. Ensure the
enable call runs before any department creation and the disable call runs after
all delete() calls complete.
---
Nitpick comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts`:
- Around line 27-33: Replace the raw api.post calls in test.beforeAll and
test.afterAll with the shared helper setSettingValueById for consistency;
specifically, in the test.beforeAll that currently calls
api.post('/settings/Omnichannel_enable_department_removal', { value: true }) and
the test.afterAll that calls api.post(..., { value: false }), import/use the
setSettingValueById helper and call it with the api fixture and the setting id
"Omnichannel_enable_department_removal" and true/false respectively, then remove
the direct api.post calls so the tests use setSettingValueById throughout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbe62777-69dc-47a1-a9f0-b446e633549e
📒 Files selected for processing (11)
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts
📜 Review details
⏰ 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). (5)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 📦 Meteor Build (coverage)
🧰 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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
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
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/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:18.066Z
Learning: In `apps/meteor/tests/end-to-end/apps/` (and Livechat E2E tests broadly), calling `createAgent()` and `makeAgentAvailable()` immediately after `updateSetting('Livechat_enabled', true)` — without any intermediate sleep — is an established, non-flaky pattern used across 10+ tests in the codebase. Do not flag this sequence as a potential race condition or suggest adding a delay between them.
📚 Learning: 2026-03-02T16:31:41.304Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts
📚 Learning: 2026-03-16T11:50:18.066Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:18.066Z
Learning: In `apps/meteor/tests/end-to-end/apps/` (and Livechat E2E tests broadly), calling `createAgent()` and `makeAgentAvailable()` immediately after `updateSetting('Livechat_enabled', true)` — without any intermediate sleep — is an established, non-flaky pattern used across 10+ tests in the codebase. Do not flag this sequence as a potential race condition or suggest adding a delay between them.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.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-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-units.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-agents.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts
📚 Learning: 2026-03-16T22:56:50.405Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:50.405Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.ts
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.tsapps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.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-chat-transfers.spec.ts
🔇 Additional comments (10)
apps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts (1)
7-7: LGTM! Proper setup/teardown pattern for the department removal setting.The implementation correctly enables the setting in
beforeAlland resets it inafterAll, ensuring test isolation. The placement after resource creation and before cleanup deletion is appropriate.Also applies to: 44-46, 48-55
apps/meteor/tests/e2e/omnichannel/omnichannel-business-hours.spec.ts (1)
29-29: LGTM! Setting toggle with status assertions.The implementation correctly enables and disables the department removal setting with status code verification, ensuring the API calls succeed.
Also applies to: 38-38
apps/meteor/tests/e2e/omnichannel/omnichannel-manager-role.spec.ts (1)
8-8: LGTM! Consistent setup/teardown pattern.The setting is correctly enabled after creating test resources and disabled after cleanup, maintaining proper test isolation.
Also applies to: 83-84, 97-98
apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts (1)
8-8: LGTM! Setting toggle for Auto Selection tests.The setting is correctly toggled in the first describe block. Note that the second describe block (
OC - Contact Center [Manual Selection]) does not include this toggle, which appears intentional as those tests don't involve department deletion.Also applies to: 112-113, 137-138
apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts (2)
5-5: LGTM! Setting toggle with proper assertions.The department removal setting is correctly enabled at the start and disabled in cleanup, with HTTP 200 status assertions for reliability.
Also applies to: 31-31, 68-75
79-79: Good change: Usingnavbar.openChat()for more reliable navigation.Switching from sidebar-based navigation to
navbar.openChat()addresses the flakiness caused by random sidebar ordering. The navbar method uses search functionality and waits for the channel to be fully loaded, making tests more deterministic. This aligns with the PR objective.Also applies to: 123-123
apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts (1)
7-7: LGTM for Monitor role tests! Setting toggle correctly implemented.The setting is enabled after units creation and properly disabled in cleanup for the Monitor role tests.
Also applies to: 98-99, 121-130
apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts (1)
6-6: LGTM! Consistent setup/teardown pattern.The setting toggle follows the established pattern, enabling after room setup and disabling after cleanup.
Also applies to: 132-133, 151-152
apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts (1)
7-7: LGTM! Clean separation of setting toggle in dedicated hooks.The setting is enabled in a dedicated
beforeAlland properly disabled after cleanup, following the pattern used in other test files.Also applies to: 33-36, 37-42
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-role.spec.ts (1)
44-53: Nice consolidation of department-removal toggling.Using
setSettingValueByIdin both setup and teardown makes this suite’s environment handling more consistent and easier to maintain.Also applies to: 118-130
apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts
Show resolved
Hide resolved
juliajforesti
requested changes
Mar 17, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts (1)
28-34: Consider movingbeforeAll/afterAllbeforebeforeEachblocks for conventional ordering.While Playwright executes hooks in the correct order regardless of declaration position, placing
beforeAllbeforebeforeEachimproves readability and follows conventional test structure.♻️ Suggested reordering
test.use({ storageState: Users.admin.state }); test.describe.serial('OC - Manage Agents', () => { let poOmnichannelAgents: OmnichannelAgents; let department: Awaited<ReturnType<typeof createDepartment>>; + test.beforeAll(async ({ api }) => { + expect((await setSettingValueById(api, 'Omnichannel_enable_department_removal', true)).status()).toBe(200); + }); + + test.afterAll(async ({ api }) => { + expect((await setSettingValueById(api, 'Omnichannel_enable_department_removal', false)).status()).toBe(200); + }); + // Create agent and department test.beforeEach(async ({ api }) => { department = await createDepartment(api); }); // Create page object and redirect to home test.beforeEach(async ({ page }) => { poOmnichannelAgents = new OmnichannelAgents(page); await page.goto('/omnichannel'); await poOmnichannelAgents.sidebar.linkAgents.click(); }); - test.beforeAll(async ({ api }) => { - expect((await setSettingValueById(api, 'Omnichannel_enable_department_removal', true)).status()).toBe(200); - }); - - test.afterAll(async ({ api }) => { - expect((await setSettingValueById(api, 'Omnichannel_enable_department_removal', false)).status()).toBe(200); - }); - // Ensure that there is no leftover data even if test fails test.afterEach(async ({ api }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts` around lines 28 - 34, Move the test.beforeAll and test.afterAll blocks so they appear before the test.beforeEach blocks to follow conventional hook ordering and improve readability; locate the hooks named test.beforeAll (which calls setSettingValueById with 'Omnichannel_enable_department_removal') and test.afterAll and reposition them above the test.beforeEach declaration(s) in omnichannel-agents.spec.ts, preserving their async behavior and assertions so execution order is unchanged but declaration order is conventional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts`:
- Around line 28-34: Move the test.beforeAll and test.afterAll blocks so they
appear before the test.beforeEach blocks to follow conventional hook ordering
and improve readability; locate the hooks named test.beforeAll (which calls
setSettingValueById with 'Omnichannel_enable_department_removal') and
test.afterAll and reposition them above the test.beforeEach declaration(s) in
omnichannel-agents.spec.ts, preserving their async behavior and assertions so
execution order is unchanged but declaration order is conventional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1453fb30-62fd-49aa-a2af-c925511e2122
📒 Files selected for processing (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts
📜 Review details
⏰ 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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 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-agents.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-agents.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-agents.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-agents.spec.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:18.066Z
Learning: In `apps/meteor/tests/end-to-end/apps/` (and Livechat E2E tests broadly), calling `createAgent()` and `makeAgentAvailable()` immediately after `updateSetting('Livechat_enabled', true)` — without any intermediate sleep — is an established, non-flaky pattern used across 10+ tests in the codebase. Do not flag this sequence as a potential race condition or suggest adding a delay between them.
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
📚 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-agents.spec.ts
📚 Learning: 2026-03-02T16:31:41.304Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.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-agents.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/omnichannel/omnichannel-agents.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-agents.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-agents.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-agents.spec.ts
📚 Learning: 2026-03-16T11:50:18.066Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:18.066Z
Learning: In `apps/meteor/tests/end-to-end/apps/` (and Livechat E2E tests broadly), calling `createAgent()` and `makeAgentAvailable()` immediately after `updateSetting('Livechat_enabled', true)` — without any intermediate sleep — is an established, non-flaky pattern used across 10+ tests in the codebase. Do not flag this sequence as a potential race condition or suggest adding a delay between them.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.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-agents.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-agents.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-agents.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts
🔇 Additional comments (1)
apps/meteor/tests/e2e/omnichannel/omnichannel-agents.spec.ts (1)
5-5: LGTM!Import correctly added for the
setSettingValueByIdutility used in the new hooks.
juliajforesti
approved these changes
Mar 17, 2026
juliajforesti
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
FLAKY-1568
Summary by CodeRabbit