fix: Agents can't leave omnichannel rooms that have already been clos…

…ed (#32822)

Co-authored-by: Matheus Barbosa Silva <[email protected]>

.changeset/happy-peaches-nail.md

@@ -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)

apps/meteor/app/lib/server/functions/closeLivechatRoom.ts

@@ -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,
+	});
+};

apps/meteor/app/livechat/server/api/v1/room.ts

@@ -1,7 +1,7 @@
 import { Omnichannel } from '@rocket.chat/core-services';
 import type { ILivechatAgent, IOmnichannelRoom, IUser, SelectedAgent, TransferByData } from '@rocket.chat/core-typings';
 import { isOmnichannelRoom, OmnichannelSourceType } from '@rocket.chat/core-typings';
-import { LivechatVisitors, Users, LivechatRooms, Subscriptions, Messages } from '@rocket.chat/models';
+import { LivechatVisitors, Users, LivechatRooms, Messages } from '@rocket.chat/models';
 import { Random } from '@rocket.chat/random';
 import {
 	isLiveChatRoomForwardProps,
@@ -22,6 +22,7 @@ import { isWidget } from '../../../../api/server/helpers/isWidget';
 import { canAccessRoomAsync, roomAccessAttributes } from '../../../../authorization/server';
 import { hasPermissionAsync } from '../../../../authorization/server/functions/hasPermission';
 import { addUserToRoom } from '../../../../lib/server/functions/addUserToRoom';
+import { closeLivechatRoom } from '../../../../lib/server/functions/closeLivechatRoom';
 import { settings as rcSettings } from '../../../../settings/server';
 import { normalizeTransferredByData } from '../../lib/Helper';
 import type { CloseRoomParams } from '../../lib/LivechatTyped';
@@ -177,51 +178,7 @@ API.v1.addRoute(
 		async post() {
 			const { rid, comment, tags, generateTranscriptPdf, transcriptEmail } = this.bodyParams;
 
-			const room = await LivechatRooms.findOneById(rid);
-			if (!room || !isOmnichannelRoom(room)) {
-				throw new Error('error-invalid-room');
-			}
-
-			if (!room.open) {
-				throw new Error('error-room-already-closed');
-			}
-
-			const subscription = await Subscriptions.findOneByRoomIdAndUserId(rid, this.userId, { projection: { _id: 1 } });
-			if (!subscription && !(await hasPermissionAsync(this.userId, 'close-others-livechat-room'))) {
-				throw new Error('error-not-authorized');
-			}
-
-			const options: CloseRoomParams['options'] = {
-				clientAction: true,
-				tags,
-				...(generateTranscriptPdf && { pdfTranscript: { requestedBy: this.userId } }),
-				...(transcriptEmail && {
-					...(transcriptEmail.sendToVisitor
-						? {
-								emailTranscript: {
-									sendToVisitor: true,
-									requestData: {
-										email: transcriptEmail.requestData.email,
-										subject: transcriptEmail.requestData.subject,
-										requestedAt: new Date(),
-										requestedBy: this.user,
-									},
-								},
-						  }
-						: {
-								emailTranscript: {
-									sendToVisitor: false,
-								},
-						  }),
-				}),
-			};
-
-			await LivechatTyped.closeRoom({
-				room,
-				user: this.user,
-				options,
-				comment,
-			});
+			await closeLivechatRoom(this.user, rid, { comment, tags, generateTranscriptPdf, transcriptEmail });
 
 			return API.v1.success();
 		},

apps/meteor/app/livechat/server/methods/closeRoom.ts

@@ -60,6 +60,16 @@ Meteor.methods<ServerMethods>({
 			});
 		}
 
+		const subscription = await SubscriptionRaw.findOneByRoomIdAndUserId(roomId, userId, {
+			projection: {
+				_id: 1,
+			},
+		});
+		if (!room.open && subscription) {
+			await SubscriptionRaw.removeByRoomId(roomId);
+			return;
+		}
+
 		if (!room.open) {
 			throw new Meteor.Error('room-closed', 'Room closed', { method: 'livechat:closeRoom' });
 		}
@@ -71,11 +81,6 @@ Meteor.methods<ServerMethods>({
 			});
 		}
 
-		const subscription = await SubscriptionRaw.findOneByRoomIdAndUserId(roomId, user._id, {
-			projection: {
-				_id: 1,
-			},
-		});
 		if (!subscription && !(await hasPermissionAsync(userId, 'close-others-livechat-room'))) {
 			throw new Meteor.Error('error-not-authorized', 'Not authorized', {
 				method: 'livechat:closeRoom',

apps/meteor/client/views/room/Header/Omnichannel/QuickActions/hooks/useQuickActions.tsx

@@ -10,6 +10,7 @@ import {
 	useMethod,
 	useTranslation,
 	useRouter,
+	useUserSubscription,
 } from '@rocket.chat/ui-contexts';
 import React, { useCallback, useState, useEffect } from 'react';
 
@@ -47,6 +48,7 @@ export const useQuickActions = (): {
 	const visitorRoomId = room.v._id;
 	const rid = room._id;
 	const uid = useUserId();
+	const subscription = useUserSubscription(rid);
 	const roomLastMessage = room.lastMessage;
 
 	const getVisitorInfo = useEndpoint('GET', '/v1/livechat/visitors.info');
@@ -330,7 +332,7 @@ export const useQuickActions = (): {
 			case QuickActionsEnum.TranscriptPDF:
 				return hasLicense && !isRoomOverMacLimit && canSendTranscriptPDF;
 			case QuickActionsEnum.CloseChat:
-				return !!roomOpen && (canCloseRoom || canCloseOthersRoom);
+				return (subscription && (canCloseRoom || canCloseOthersRoom)) || (!!roomOpen && canCloseOthersRoom);
 			case QuickActionsEnum.OnHoldChat:
 				return !!roomOpen && canPlaceChatOnHold;
 			default:

apps/meteor/tests/unit/app/lib/server/functions/closeLivechatRoom.tests.ts

@@ -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;
+	});
+});