Skip to content

feat: update Livechat_enabled_when_agent_idle setting to not include offline agents in the query#39495

Open
nazabucciarelli wants to merge 52 commits intodevelopfrom
fix/livechat-offline-agent-assignment
Open

feat: update Livechat_enabled_when_agent_idle setting to not include offline agents in the query#39495
nazabucciarelli wants to merge 52 commits intodevelopfrom
fix/livechat-offline-agent-assignment

Conversation

@nazabucciarelli
Copy link
Copy Markdown
Contributor

@nazabucciarelli nazabucciarelli commented Mar 10, 2026

Proposed changes (including videos or screenshots)

The changes made in this PR were done following product team given directives.

  • Updated the routing query to always exclude agents whose status is offline, regardless of the Livechat_enabled_when_agent_idle setting. The setting now only affects agents whose connectionStatus is away.
  • Migrated API Tests to E2E: Removed the manual WebSocket helper logic (previously used to force presence updates). The routing tests that required complex presence manipulation were converted to End-to-End (E2E) Playwright tests. This leverages the browser's native WebSocket connection, simulating real agent behavior naturally and eliminating the need for custom socket management.
  • E2E Refactoring: Updated existing Playwright tests to properly handle agent states (online/away), ensuring accurate routing scenarios.
  • Page Objects: Updated Omnichannel Page Objects to support the newly converted E2E tests, including better UI-based waiters and status validations.

Issue(s)

CORE-1853 [Investigation] The Omnichannel routing system (Load Balancer) assigns new chats to offline agents if their Livechat toggle was left as "Available".

Steps to test or reproduce

Before testing these cases, you'd need to have a EE license and to create at least two agents and a department with them. Also, you'll need to go to Settings -> Omnichannel -> Queue Management, enable the Waiting queue option and then set Max. number of simultaneous chats to only 1. Please check the task's comment, I've left some recordings there testing this.

Further comments

IMPORTANT NOTE

This PR will introduce a new regression: CORE-1972 - Fix 'Livechat_accept_chats_with_no_agents' omnichannel setting not being respected. This happens because we weren't filtering out offline agents at all, making like the Livechat_accept_chats_with_no_agents (setting intended to assign offline agents to visitors) was always true. So after merging this PR to develop, a new PR will be raised for this 'introduced' regression.

Summary by CodeRabbit

  • Tests
    • Test suites now use real-time WebSocket sessions for agent away/idle signaling, include DDP login and away-signaling helpers, and ensure sockets are closed during teardown.
  • Bug Fixes
    • Improved agent availability filtering to avoid routing visitors to offline agents.
  • Chores
    • Added a changeset for patch releases and set a CI environment variable to support WebSocket-based test flows.

@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot bot commented Mar 10, 2026

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 10, 2026

🦋 Changeset detected

Latest commit: f332eaf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/models Minor
@rocket.chat/meteor Minor
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/server-fetch Patch
@rocket.chat/ui-client Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/media-calls Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/network-broker Patch
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Major
@rocket.chat/ui-contexts Major
@rocket.chat/ui-voip Major
@rocket.chat/web-ui-registration Major
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds DDP/WebSocket test utilities and integrates WebSocket-based agent away signaling into multiple livechat tests and helpers; tests now track/close WebSocket handles. Adjusts omnichannel agent-status filter logic and adds CI env var and a changeset entry.

Changes

Cohort / File(s) Summary
WebSocket DDP utilities
apps/meteor/tests/data/users.helper.ts
Adds connectWS, ddpLogin(resume): Promise<WebSocket>, and setUserAwayWS(ws): Promise<void) to perform DDP WebSocket connect, login by resume token, and call UserPresence:away with result/error handling and ping/pong support.
Livechat test helpers
apps/meteor/tests/data/livechat/users.ts, apps/meteor/tests/data/livechat/department.ts
createAnAwayAgent and createDepartmentWithAnAwayAgent now obtain a WebSocket via ddpLogin and return the ws alongside credentials and user; callers updated to destructure and propagate ws.
E2E / Integration tests
apps/meteor/tests/e2e/omnichannel/..., apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts, apps/meteor/tests/end-to-end/api/livechat/24-routing.ts
Tests now call ddpLogin and setUserAwayWS(ws) to mark agents away over DDP, collect sockets in a sockets: WebSocket[], and close them in global teardown. 24-routing.ts also removes one deprecated test.
Agent status filtering
packages/models/src/helpers/omnichannel/agentStatus.ts
Modifies the $or condition used when composing agent availability filters, changing inclusion logic for bots vs. non-offline agents in routing queries.
CI / Meta
.github/workflows/ci-test-e2e.yml, .changeset/purple-boxes-shout.md
Adds DDP_LOGIN_PORT=3000 to CI e2e workflow env and adds a changeset entry marking patch releases for affected packages.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Test
participant WS_Client as "WebSocket (client)\nrgba(0,128,255,0.5)"
participant Meteor as "Meteor/DDP Server\nrgba(0,200,0,0.5)"
Test->>WS_Client: open WebSocket (connect)
WS_Client->>Meteor: { msg: "connect", version, support }
Meteor-->>WS_Client: { msg: "connected" }
Test->>WS_Client: send DDP method "login" with resume and id
WS_Client->>Meteor: DDP method call (login)
Meteor-->>WS_Client: { msg: "result", id: loginId, result }
Test->>WS_Client: send DDP method "UserPresence:away" with id
WS_Client->>Meteor: DDP method call (UserPresence:away)
Meteor-->>WS_Client: { msg: "result", id: awayId }
Note over Test,WS_Client: Tests keep ws handle and close in teardown

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses CORE-1853 by filtering offline agents at the query root, preventing assignment to disconnected agents. However, it introduces a regression in CORE-1972 by breaking Livechat_accept_chats_with_no_agents, which the PR acknowledges is tracked separately but not fixed in this PR. The PR should either fully implement the fix for CORE-1972 by passing Livechat_accept_chats_with_no_agents parameter to routing methods, or clearly document that CORE-1972 will be addressed in a separate follow-up PR with specific tracking.
✅ Passed checks (4 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed All changes are within scope: WebSocket-based presence updates for tests (ddpLogin, setUserAwayWS helpers), query filtering logic for offline agents, and environment configuration (DDP_LOGIN_PORT) directly support the core objective of fixing offline agent assignment.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately describes the main change: updating the Livechat_enabled_when_agent_idle setting to exclude offline agents from query results.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nazabucciarelli nazabucciarelli changed the title move offline agents filtering to query root fix: move offline agents filtering to query root Mar 10, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.16%. Comparing base (2353766) to head (f332eaf).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #39495      +/-   ##
===========================================
- Coverage    70.19%   70.16%   -0.04%     
===========================================
  Files         3281     3281              
  Lines       116915   116915              
  Branches     20671    20815     +144     
===========================================
- Hits         82073    82037      -36     
- Misses       31555    31594      +39     
+ Partials      3287     3284       -3     
Flag Coverage Δ
e2e 59.68% <ø> (-0.07%) ⬇️
e2e-api 46.40% <ø> (-0.05%) ⬇️
unit 71.02% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nazabucciarelli nazabucciarelli force-pushed the fix/livechat-offline-agent-assignment branch from 3480932 to 15311df Compare March 10, 2026 18:04
@nazabucciarelli nazabucciarelli force-pushed the fix/livechat-offline-agent-assignment branch from 15311df to d3fbcf9 Compare March 10, 2026 19:33
@nazabucciarelli nazabucciarelli force-pushed the fix/livechat-offline-agent-assignment branch from f9512c8 to 04dc75a Compare March 11, 2026 01:43
@nazabucciarelli nazabucciarelli added this to the 8.3.0 milestone Mar 12, 2026
@ggazzo ggazzo removed the community label Mar 12, 2026
@nazabucciarelli nazabucciarelli force-pushed the fix/livechat-offline-agent-assignment branch from 8a4cb7b to 6ebec42 Compare March 13, 2026 18:24
@nazabucciarelli nazabucciarelli force-pushed the fix/livechat-offline-agent-assignment branch from f579500 to 77bd762 Compare March 17, 2026 01:21
@alfredodelfabro alfredodelfabro removed this from the 8.3.0 milestone Mar 17, 2026
KevLehman
KevLehman previously approved these changes Apr 13, 2026
Copy link
Copy Markdown
Member

@KevLehman KevLehman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope gazzo is happy with the ws.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

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=".changeset/purple-boxes-shout.md">

<violation number="1" location=".changeset/purple-boxes-shout.md:6">
P2: The changeset note is inaccurate: it says offline agents are excluded only when the setting is enabled, but this PR’s behavior is to always exclude offline agents.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread .changeset/purple-boxes-shout.md Outdated
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
KevLehman
KevLehman previously approved these changes Apr 13, 2026
Copy link
Copy Markdown
Member

@ggazzo ggazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering these ws additions, we either should use the SDK to make things easier, or we should move this to UI testing, in my opinion.

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai bot commented Apr 15, 2026

You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment @cubic-dev-ai review.

@nazabucciarelli
Copy link
Copy Markdown
Contributor Author

@cubic-dev-ai review

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai bot commented Apr 15, 2026

@cubic-dev-ai review

@nazabucciarelli I have started the AI code review. It will take a few minutes to complete.

await page.goto('/');
await poOmnichannel.waitForHome();

if (conversations.length === 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave this one for a FE to take a look 👀

curious if there's a better approach to fix it.

});

test.afterEach(async ({ api }) => {
if (livechatPage) await livechatPage.context().close();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we prefer long ifs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in e7898f2

should we add a new linter rule?

Comment on lines +66 to +67
await setSettingValueById(api, 'Livechat_waiting_queue', false);
await setSettingValueById(api, 'Accounts_Default_User_Preferences_idleTimeLimit', 300);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do u need this to happen after each test step? why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of closing contexts, I need that after each test in order to not get multiple windows opened. In case of Accounts_Default_User_Preferences_idleTimeLimit set to 300, it's needed since we have 2 agents on this test, and if the value of the setting remains as 1 in the beginning of each test, it would make onlineAgent to be away too. Regarding the Livechat_waiting_queue setting you were right, it wasn't strictly needed in the beforeEach, but I added it on a new step at the end of the test cases that use it. Addresed in e7898f2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants