Skip to content

Conversation

@sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented Mar 25, 2025

Proposed changes (including videos or screenshots)

This endpoint is called every time a message is sent by an omnichannel agent. On every message we used update the inquiry's lastMessage, which caused an invalidation on the frontend so it mistakenly thought it would need to fetch the room data again (since something changed).

The proposal is that we update the inquiry's last message only if it has not been taken yet. Once it is taken we update only the room's last message property. If it happens to the room to be sent back to the queue, then we update the inquiry's last message so it can reflect the latest state. A test was added to cover this exact behavior.

This is chart of a production workspace that uses inquiries, showing how massive the amount of calls to the endpoint is:
image

Issue(s)

CTZ-28

Steps to test or reproduce

Further comments

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Mar 25, 2025

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

@changeset-bot
Copy link

changeset-bot bot commented Mar 25, 2025

🦋 Changeset detected

Latest commit: 9f12749

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

This PR includes changesets to release 36 packages
Name Type
@rocket.chat/models Patch
@rocket.chat/meteor Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@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/stream-hub-service Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/network-broker Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/ui-voip Patch
@rocket.chat/web-ui-registration Patch

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2025

PR Preview Action v1.6.0

🚀 View preview at
https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35618/

Built to branch gh-pages at 2025-04-10 14:46 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov
Copy link

codecov bot commented Mar 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.64%. Comparing base (2351a29) to head (9f12749).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #35618   +/-   ##
========================================
  Coverage    59.64%   59.64%           
========================================
  Files         2832     2832           
  Lines        68313    68313           
  Branches     15129    15129           
========================================
  Hits         40747    40747           
  Misses       24960    24960           
  Partials      2606     2606           
Flag Coverage Δ
unit 75.62% <ø> (ø)

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.

@sampaiodiego sampaiodiego changed the title Fix omni send message request room again fix: Omnichannel request rrom data after every message sent Mar 26, 2025
@sampaiodiego sampaiodiego added this to the 7.6.0 milestone Mar 26, 2025
@sampaiodiego sampaiodiego changed the title fix: Omnichannel request rrom data after every message sent fix: Omnichannel making a request for room data after every message sent Mar 26, 2025
@sampaiodiego sampaiodiego force-pushed the fix-omni-send-message-request-room-again branch from a04bc95 to 9192e27 Compare March 26, 2025 15:06
@sampaiodiego sampaiodiego marked this pull request as ready for review March 26, 2025 15:08
@sampaiodiego sampaiodiego requested review from a team as code owners March 26, 2025 15:08
@sampaiodiego sampaiodiego force-pushed the fix-omni-send-message-request-room-again branch from 9192e27 to 6d571d4 Compare March 26, 2025 16:46
Copy link
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.

There's one missing place:

return RoutingManager.unassignAgent(inquiry, departmentId, true);

Here, the chat is "transfered" but since there's no online agents, it ends up in the department's queue. We should do the same we do on the manual move to queue func.

@sampaiodiego sampaiodiego force-pushed the fix-omni-send-message-request-room-again branch 3 times, most recently from de6359b to ec4748f Compare April 9, 2025 19:48
KevLehman
KevLehman previously approved these changes Apr 10, 2025
Copy link
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.

There's a small change in behavior, where the last message of the inquiry after being sent to department's queue ends up being the system message instead of the actual "last message"

Think we can live with that, and fix in a separate PR (as the fix would involve some extra work on other files)

@KevLehman KevLehman added the stat: QA assured Means it has been tested and approved by a company insider label Apr 10, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Apr 10, 2025
@kodiakhq kodiakhq bot removed the stat: ready to merge PR tested and approved waiting for merge label Apr 10, 2025
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Apr 10, 2025

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

@sampaiodiego sampaiodiego merged commit a8896a7 into develop Apr 10, 2025
49 checks passed
@sampaiodiego sampaiodiego deleted the fix-omni-send-message-request-room-again branch April 10, 2025 17:22
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants