-
Notifications
You must be signed in to change notification settings - Fork 10.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Agents can't leave omnichannel rooms that have already been clos…
…ed (#32707)
- Loading branch information
1 parent
8fc6ca8
commit fa82159
Showing
6 changed files
with
256 additions
and
52 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@rocket.chat/meteor": patch | ||
--- | ||
|
||
Fixed issue with livechat agents not being able to leave omnichannel rooms if joining after a room has been closed by the visitor (due to race conditions) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
import type { IUser, IRoom, IOmnichannelRoom } from '@rocket.chat/core-typings'; | ||
import { isOmnichannelRoom } from '@rocket.chat/core-typings'; | ||
import { LivechatRooms, Subscriptions } from '@rocket.chat/models'; | ||
|
||
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; | ||
import type { CloseRoomParams } from '../../../livechat/server/lib/LivechatTyped'; | ||
import { Livechat } from '../../../livechat/server/lib/LivechatTyped'; | ||
|
||
export const closeLivechatRoom = async ( | ||
user: IUser, | ||
roomId: IRoom['_id'], | ||
{ | ||
comment, | ||
tags, | ||
generateTranscriptPdf, | ||
transcriptEmail, | ||
}: { | ||
comment?: string; | ||
tags?: string[]; | ||
generateTranscriptPdf?: boolean; | ||
transcriptEmail?: | ||
| { | ||
sendToVisitor: false; | ||
} | ||
| { | ||
sendToVisitor: true; | ||
requestData: Pick<NonNullable<IOmnichannelRoom['transcriptRequest']>, 'email' | 'subject'>; | ||
}; | ||
}, | ||
): Promise<void> => { | ||
const room = await LivechatRooms.findOneById(roomId); | ||
if (!room || !isOmnichannelRoom(room)) { | ||
throw new Error('error-invalid-room'); | ||
} | ||
|
||
if (!room.open) { | ||
const subscriptionsLeft = await Subscriptions.countByRoomId(roomId); | ||
if (subscriptionsLeft) { | ||
await Subscriptions.removeByRoomId(roomId); | ||
return; | ||
} | ||
throw new Error('error-room-already-closed'); | ||
} | ||
|
||
const subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, user._id, { projection: { _id: 1 } }); | ||
if (!subscription && !(await hasPermissionAsync(user._id, 'close-others-livechat-room'))) { | ||
throw new Error('error-not-authorized'); | ||
} | ||
|
||
const options: CloseRoomParams['options'] = { | ||
clientAction: true, | ||
tags, | ||
...(generateTranscriptPdf && { pdfTranscript: { requestedBy: user._id } }), | ||
...(transcriptEmail && { | ||
...(transcriptEmail.sendToVisitor | ||
? { | ||
emailTranscript: { | ||
sendToVisitor: true, | ||
requestData: { | ||
email: transcriptEmail.requestData.email, | ||
subject: transcriptEmail.requestData.subject, | ||
requestedAt: new Date(), | ||
requestedBy: user, | ||
}, | ||
}, | ||
} | ||
: { | ||
emailTranscript: { | ||
sendToVisitor: false, | ||
}, | ||
}), | ||
}), | ||
}; | ||
|
||
await Livechat.closeRoom({ | ||
room, | ||
user, | ||
options, | ||
comment, | ||
}); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
154 changes: 154 additions & 0 deletions
154
apps/meteor/tests/unit/app/lib/server/functions/closeLivechatRoom.tests.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
import { expect } from 'chai'; | ||
import { describe, it } from 'mocha'; | ||
import proxyquire from 'proxyquire'; | ||
import sinon from 'sinon'; | ||
|
||
import { createFakeRoom, createFakeSubscription, createFakeUser } from '../../../../../mocks/data'; | ||
|
||
const subscriptionsStub = { | ||
findOneByRoomIdAndUserId: sinon.stub(), | ||
removeByRoomId: sinon.stub(), | ||
countByRoomId: sinon.stub(), | ||
}; | ||
|
||
const livechatRoomsStub = { | ||
findOneById: sinon.stub(), | ||
}; | ||
|
||
const livechatStub = { | ||
closeRoom: sinon.stub(), | ||
}; | ||
|
||
const hasPermissionStub = sinon.stub(); | ||
|
||
const { closeLivechatRoom } = proxyquire.noCallThru().load('../../../../../../app/lib/server/functions/closeLivechatRoom.ts', { | ||
'../../../livechat/server/lib/LivechatTyped': { | ||
Livechat: livechatStub, | ||
}, | ||
'../../../authorization/server/functions/hasPermission': { | ||
hasPermissionAsync: hasPermissionStub, | ||
}, | ||
'@rocket.chat/models': { | ||
Subscriptions: subscriptionsStub, | ||
LivechatRooms: livechatRoomsStub, | ||
}, | ||
}); | ||
|
||
describe('closeLivechatRoom', () => { | ||
const user = createFakeUser(); | ||
const room = createFakeRoom({ t: 'l', open: true }); | ||
const subscription = createFakeSubscription({ rid: room._id, u: { username: user.username, _id: user._id } }); | ||
|
||
beforeEach(() => { | ||
subscriptionsStub.findOneByRoomIdAndUserId.reset(); | ||
subscriptionsStub.removeByRoomId.reset(); | ||
subscriptionsStub.countByRoomId.reset(); | ||
livechatRoomsStub.findOneById.reset(); | ||
livechatStub.closeRoom.reset(); | ||
hasPermissionStub.reset(); | ||
}); | ||
|
||
it('should not perform any operation when an invalid room id is provided', async () => { | ||
livechatRoomsStub.findOneById.resolves(null); | ||
hasPermissionStub.resolves(true); | ||
|
||
await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-invalid-room'); | ||
expect(livechatStub.closeRoom.notCalled).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; | ||
}); | ||
|
||
it('should not perform any operation when a non-livechat room is provided', async () => { | ||
livechatRoomsStub.findOneById.resolves({ ...room, t: 'c' }); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(subscription); | ||
hasPermissionStub.resolves(true); | ||
|
||
await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-invalid-room'); | ||
expect(livechatStub.closeRoom.notCalled).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; | ||
}); | ||
|
||
it('should not perform any operation when a closed room with no subscriptions is provided and the caller is not subscribed to it', async () => { | ||
livechatRoomsStub.findOneById.resolves({ ...room, open: false }); | ||
subscriptionsStub.countByRoomId.resolves(0); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); | ||
hasPermissionStub.resolves(true); | ||
|
||
await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-room-already-closed'); | ||
expect(livechatStub.closeRoom.notCalled).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; | ||
expect(subscriptionsStub.countByRoomId.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; | ||
}); | ||
|
||
it('should remove dangling subscription when a closed room with subscriptions is provided and the caller is not subscribed to it', async () => { | ||
livechatRoomsStub.findOneById.resolves({ ...room, open: false }); | ||
subscriptionsStub.countByRoomId.resolves(1); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); | ||
hasPermissionStub.resolves(true); | ||
|
||
await closeLivechatRoom(user, room._id, {}); | ||
expect(livechatStub.closeRoom.notCalled).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; | ||
expect(subscriptionsStub.countByRoomId.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.calledOnceWith(room._id)).to.be.true; | ||
}); | ||
|
||
it('should remove dangling subscription when a closed room is provided but the user is still subscribed to it', async () => { | ||
livechatRoomsStub.findOneById.resolves({ ...room, open: false }); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(subscription); | ||
subscriptionsStub.countByRoomId.resolves(1); | ||
hasPermissionStub.resolves(true); | ||
|
||
await closeLivechatRoom(user, room._id, {}); | ||
expect(livechatStub.closeRoom.notCalled).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.notCalled).to.be.true; | ||
expect(subscriptionsStub.countByRoomId.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.calledOnceWith(room._id)).to.be.true; | ||
}); | ||
|
||
it('should not perform any operation when the caller is not subscribed to an open room and does not have the permission to close others rooms', async () => { | ||
livechatRoomsStub.findOneById.resolves(room); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); | ||
subscriptionsStub.countByRoomId.resolves(1); | ||
hasPermissionStub.resolves(false); | ||
|
||
await expect(closeLivechatRoom(user, room._id, {})).to.be.rejectedWith('error-not-authorized'); | ||
expect(livechatStub.closeRoom.notCalled).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.calledOnceWith(room._id, user._id)).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; | ||
}); | ||
|
||
it('should close the room when the caller is not subscribed to it but has the permission to close others rooms', async () => { | ||
livechatRoomsStub.findOneById.resolves(room); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(null); | ||
subscriptionsStub.countByRoomId.resolves(1); | ||
hasPermissionStub.resolves(true); | ||
|
||
await closeLivechatRoom(user, room._id, {}); | ||
expect(livechatStub.closeRoom.calledOnceWith(sinon.match({ room, user }))).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.calledOnceWith(room._id, user._id)).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; | ||
}); | ||
|
||
it('should close the room when the caller is subscribed to it and does not have the permission to close others rooms', async () => { | ||
livechatRoomsStub.findOneById.resolves(room); | ||
subscriptionsStub.findOneByRoomIdAndUserId.resolves(subscription); | ||
subscriptionsStub.countByRoomId.resolves(1); | ||
hasPermissionStub.resolves(false); | ||
|
||
await closeLivechatRoom(user, room._id, {}); | ||
expect(livechatStub.closeRoom.calledOnceWith(sinon.match({ room, user }))).to.be.true; | ||
expect(livechatRoomsStub.findOneById.calledOnceWith(room._id)).to.be.true; | ||
expect(subscriptionsStub.findOneByRoomIdAndUserId.calledOnceWith(room._id, user._id)).to.be.true; | ||
expect(subscriptionsStub.removeByRoomId.notCalled).to.be.true; | ||
}); | ||
}); |