Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Agents can't leave omnichannel rooms that have already been closed #32707

Merged
merged 24 commits into from
Jul 16, 2024

Conversation

matheusbsilva137
Copy link
Member

@matheusbsilva137 matheusbsilva137 commented Jul 1, 2024

Proposed changes (including videos or screenshots)

  • Allow users to leave omnichannel rooms after they have been closed (by the visitor). If a user tries to close a room that has already been closed AND they have a subscription in this room, we'll remove them from the room instead of throwing an error;
  • Display button to close livechat rooms when the user is still subscribed to the room and has any of the permissions (close-livechat-room or close-others-livechat-room) to close a room.

Due to some race condition, users could end up joining omnichannel rooms that have already been closed, which caused them to get stuck in there.

Issue(s)

Steps to test or reproduce

Since having a user stuck in an omnichannel room after it's been closed depends on a race condition, reproducing this issue may require manually inserting a subscription in the DB.
With a user that is stuck in an omnichannel/livechat room, try to close the room again through the UI or using the livechat.closeRoomByUser endpoint OR the livechat:closeRoom method. Both options currently throw an error in this case, but with this PR, the subscription is removed and a succesful result should be returned.

Further comments

I decided not to add this change to the livechat.closeRoom endpoint since this one is meant to be used by visitors -- which are not subscribed to the room directly, only agents are.

SUP-612

Copy link

changeset-bot bot commented Jul 1, 2024

🦋 Changeset detected

Latest commit: 10f01a1

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

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron 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/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/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/models Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status 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

Copy link
Contributor

dionisio-bot bot commented Jul 1, 2024

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

@matheusbsilva137 matheusbsilva137 changed the title improve: Allow removing dangling subscription when closing livechat rooms feat: Allow removing dangling subscription when closing livechat rooms Jul 1, 2024
@ggazzo
Copy link
Member

ggazzo commented Jul 2, 2024

Fix?

@KevLehman
Copy link
Contributor

Yep, should be fix.

Also, pls add a description of what this fixes (+ changeset). It should be clear which case of rooms getting stuck this fix clears 🤗

Copy link

codecov bot commented Jul 5, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 55.53%. Comparing base (8fc6ca8) to head (10f01a1).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #32707      +/-   ##
===========================================
+ Coverage    54.38%   55.53%   +1.14%     
===========================================
  Files         2552     2633      +81     
  Lines        55985    57197    +1212     
  Branches     11614    11848     +234     
===========================================
+ Hits         30450    31762    +1312     
+ Misses       22933    22745     -188     
- Partials      2602     2690      +88     
Flag Coverage Δ
e2e 54.28% <50.00%> (+2.28%) ⬆️
unit 72.11% <86.36%> (-0.03%) ⬇️

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

@matheusbsilva137 matheusbsilva137 changed the title feat: Allow removing dangling subscription when closing livechat rooms fix: Agents can't leave omnichannel rooms that have already been closed Jul 5, 2024
@matheusbsilva137 matheusbsilva137 added this to the 6.11 milestone Jul 5, 2024
@matheusbsilva137 matheusbsilva137 marked this pull request as ready for review July 6, 2024 00:07
@matheusbsilva137 matheusbsilva137 requested review from a team as code owners July 6, 2024 00:07
…nto fix/dangling-subscriptions-livechat-rooms
@matheusbsilva137 matheusbsilva137 requested a review from a team as a code owner July 15, 2024 17:05
@jessicaschelly jessicaschelly added the stat: QA assured Means it has been tested and approved by a company insider label Jul 16, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jul 16, 2024
@kodiakhq kodiakhq bot merged commit fa82159 into develop Jul 16, 2024
49 checks passed
@kodiakhq kodiakhq bot deleted the fix/dangling-subscriptions-livechat-rooms branch July 16, 2024 21:01
@scuciatto
Copy link
Member

/patch

@scuciatto
Copy link
Member

/backport 6.9.3

@ggazzo
Copy link
Member

ggazzo commented Jul 17, 2024

/patch

Copy link
Contributor

dionisio-bot bot commented Jul 17, 2024

    Sorry, I couldn't do that backport because of conflicts. Could you please solve them?
    
    you can do so by running the following commands:
git fetch
git checkout backport-6.10.1-32707
git cherry-pick fa82159492bb1589372ba4f7dcca716da7a8a74b
// solve the conflict
git push

after that just run /patch again

@RocketChat RocketChat deleted a comment from dionisio-bot bot Jul 17, 2024
@ggazzo
Copy link
Member

ggazzo commented Jul 17, 2024

/backport 6.9.4

Copy link
Contributor

dionisio-bot bot commented Jul 17, 2024

Sorry, I couldn't do that backport because of conflicts. Could you please solve them?

you can do so by running the following commands:

git fetch
git checkout backport-6.9.4-32707
git cherry-pick fa82159492bb1589372ba4f7dcca716da7a8a74b
// solve the conflict
git push

after that just run /backport 6.9.4 again

@ggazzo
Copy link
Member

ggazzo commented Jul 17, 2024

/patch

Copy link
Contributor

dionisio-bot bot commented Jul 17, 2024

Pull request #32819 added to Project: "Patch 6.10.1"

@ggazzo
Copy link
Member

ggazzo commented Jul 17, 2024

/backport 6.9.4

Copy link
Contributor

dionisio-bot bot commented Jul 17, 2024

Pull request #32822 added to Project: "Patch 6.9.4"

@RocketChat RocketChat deleted a comment from dionisio-bot bot Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad: core stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants