feat: Allow users to force close Omnichannel rooms #34940
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 33d54b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 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 |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #34940 +/- ##
===========================================
- Coverage 59.12% 59.12% -0.01%
===========================================
Files 2819 2819
Lines 67938 67935 -3
Branches 15134 15135 +1
===========================================
- Hits 40171 40169 -2
+ Misses 24934 24933 -1
Partials 2833 2833
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -240,10 +240,10 @@ class LivechatClass { | |||
| session: ClientSession, | |||
| ): Promise<{ room: IOmnichannelRoom; closedBy: ChatCloser; removedInquiry: ILivechatInquiryRecord | null }> { | |||
| const { comment } = params; | |||
| const { room } = params; | |||
| const { room, forceClose } = params; | |||
There was a problem hiding this comment.
I wonder if we should re-check the setting here 🤔
I know you're already checking in the endpoint level, but maybe we should have something at a lower level to prevent force closing if the setting is disabled. This is more of an opinion, so I'd like to hear what you think or if you thought about this already
There was a problem hiding this comment.
Hmmmm, not sure.
I mean, there's no other place that we expose that can use this param right now so thought checking at the caller makes sense.
Wdyt?
|
|
||
| await closeLivechatRoom(this.user, rid, { comment, tags, generateTranscriptPdf, transcriptEmail }); | ||
| const allowForceClose = rcSettings.get<boolean>('Omnichannel_allow_force_close_conversations'); |
There was a problem hiding this comment.
What if someone tries to force close a room in a workspace where this is not enabled? Shouldn't we throw an error? I wonder if just silently not force closing could be misleading
There was a problem hiding this comment.
It will throw an error. It will be the normal room closing error as of the setting was ignored.
Didn't want to add a new error just for this case, as it could be confusing
Co-authored-by: Matheus Barbosa Silva <[email protected]>
Proposed changes (including videos or screenshots)
The new param passed to
rooms.closeByUserendpoint allows users to forcefully close an omnichannel conversation.This may come handy when a room, for X or Y reason ends up in a bad state, given an error during the closing procedure, or an update after the room has been closed. Currently, calling the API again will throw an error, as the room state is not valid and as such we avoid updating it.
This new setting will allow admins to allow the
forceCloseparameter, which will skip most of the validations and run the closing process again.Issue(s)
https://rocketchat.atlassian.net/browse/CONN-475
Steps to test or reproduce
Further comments