feat: update Livechat_enabled_when_agent_idle setting to not include offline agents in the query#39495
feat: update Livechat_enabled_when_agent_idle setting to not include offline agents in the query#39495nazabucciarelli wants to merge 39 commits intodevelopfrom
Livechat_enabled_when_agent_idle setting to not include offline agents in the query#39495Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: a4845d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
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 |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds 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
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 #39495 +/- ##
===========================================
- Coverage 70.56% 70.21% -0.36%
===========================================
Files 3271 3279 +8
Lines 116804 116816 +12
Branches 21060 20734 -326
===========================================
- Hits 82424 82019 -405
+ Misses 32331 31495 -836
- Partials 2049 3302 +1253
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
3480932 to
15311df
Compare
15311df to
d3fbcf9
Compare
f9512c8 to
04dc75a
Compare
8a4cb7b to
6ebec42
Compare
f579500 to
77bd762
Compare
apps/meteor/tests/e2e/omnichannel/omnichannel-custom-field-usage.spec.ts
Outdated
Show resolved
Hide resolved
| { | ||
| status: { | ||
| $exists: true, | ||
| $ne: UserStatus.OFFLINE, |
There was a problem hiding this comment.
As Kevin said, yes, the behavior changes, because before, in certain cases, the "virtual" state was evaluated, in others the connection state. now the state defined by the user is always taken into consideration. To be honest, I didn't find anything in Renato's explanation that distinguishes between the connection (the user REALLY being away or offline) or the user SAYING they are away/offline.
In other words, the user could 'opt out' and, even with the chat open, stop receiving messages. With the flag active, they couldn't do that (perhaps this is a mix-up or confusion of behaviors, but yes, the end result changes).
I'm not an omnichannel expert but, based on jira discussion, there is no need to test status, only statusConnection, I think the statusConnection query should set based on isLivechatEnabledWhenAgentIdle, but I think Renato also ignored or simplified that part (status/status connection).
I think everything should be around that part of the query.
statusConnection is only: online/away/offline so
statusConnection: isLivechatEnabledWhenAgentIdle ? {$ne: 'offline'} : 'online'
The primary conditions for auto-assignment of incoming chats are:
User has livechat-agent role and user’s status mustn’t be offlineand Livechat service status must be available; Unless global Accept with No Online Agents setting is ON;
Also, if Accept new omnichannel requests when the agent is idlesetting is OFF, then connection status mustn’t be away;
I thought function queryStatusAgentOnline was handling the use case properly given that business rules haven’t changed.
I'm not sure what the decision should or shouldn't be taken, but there are certainly differences. Furthermore, it would be great to have these behaviors written and described close to the code to avoid future reinterpretations. We could put them in the docs/features folder or comment them out near a routing function or in the function query itself, specifying whether the flag should or should not alter the behavior.
There was a problem hiding this comment.
Thanks for your review and bringing in these concerns!
I'm leaving the condition to filter out agents with status: offline, first because I thought that by default we should filter out agents whose status is offline because of product's answer:
User has livechat-agent role and user’s status mustn’t be offline
and second because in the following PR I'm adding that filter conditionally according to Livechat_accept_chats_with_no_agents value, setting that wasn't working correctly, since offline agents weren't being routed to visitors even though it's enabled. Take this PR as a second phase of the task, since it was something I discovered while working on this. I'll remove that filtering from this PR though, since it results really confusing, so I'll add it directly in the second PR.
I'll take this suggestion too:
statusConnection: isLivechatEnabledWhenAgentIdle ? {$ne: 'offline'} : 'online'
and I'll modify it according to my needs in the other PR solving the offline agents issue, since that setting will allow agents whose statusConnection is offline to be assigned, I'm not 100% sure if status should be considered in that case but I'll ask Renato to clarify.
I believe the confusion started on this PR, when we started considering agents whose status was offline as idle users, and as though they should be affected by the isLivechatEnabledWhenAgentIdle value. As you can see, previously we filtered out agents with status: offline so I assumed they needed to be filtered now too.
So, in conclusion, I'll ask to product if we need to filter out agents whose status is offline by default or not (just to make sure), and if Livechat_accept_chats_with_no_agents is intended to target status or statusConnection.
According to the answers, I'll elaborate a comment to add further up in the query function so that we have documented what Livechat_enabled_when_agent_idle and Livechat_accept_chats_with_no_agents should do.
Comment in Jira making the questions: https://rocketchat.atlassian.net/browse/CORE-1853?focusedCommentId=83052
…livechat-offline-agent-assignment
Livechat_enabled_when_agent_idle is set to trueLivechat_enabled_when_agent_idle is set to true
Livechat_enabled_when_agent_idle is set to trueLivechat_enabled_when_agent_idle setting to not include offline agents in the query
Livechat_enabled_when_agent_idle setting to not include offline agents in the queryLivechat_enabled_when_agent_idle setting to not include offline agents in the query
KevLehman
left a comment
There was a problem hiding this comment.
Hope gazzo is happy with the ws.
There was a problem hiding this comment.
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.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…cketChat/Rocket.Chat into fix/livechat-offline-agent-assignment
Proposed changes (including videos or screenshots)
The changes made in this PR were done following product team given directives.
statusis offline, regardless of theLivechat_enabled_when_agent_idlesetting. The setting now only affects agents whoseconnectionStatusis away.setUserAway, tests now authenticate a websocket connection viaddpLoginand callUserPresence:awayon the same authenticated socket (setUserAwayWS). This was necessary because the previous helper relied on a sessionId that did not exist during tests, causing the presence update to fail silently.process.env.DDP_LOGIN_PORT). This ensures compatibility with both CI (which runs on 3000) and local environments where tests may run against either the monolith or microservices setup.All created sockets are closed in an after hook to avoid websocket leaks between tests.
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 theWaiting queueoption and then setMax. number of simultaneous chatsto 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