chore: Improve PDF Transcript request lock mechanism (#35809)

apps/meteor/app/statistics/server/lib/getEEStatistics.ts

@@ -93,13 +93,8 @@ async function getEEStatistics(): Promise<EEOnlyStats | undefined> {
 		}),
 	);
 
-	// Number of PDF transcript requested
-	statsPms.push(
-		LivechatRooms.countRoomsWithPdfTranscriptRequested().then((count) => {
-			statistics.omnichannelPdfTranscriptRequested = count;
-		}),
-	);
-
+	// NOTE: keeping this for compatibility with current stats. Will be removed next major
+	statistics.omnichannelPdfTranscriptRequested = 0;
 	// Number of PDF transcript that succeeded
 	statsPms.push(
 		LivechatRooms.countRoomsWithTranscriptSent().then((count) => {

apps/meteor/ee/app/livechat-enterprise/server/api/transcript.ts

@@ -1,3 +1,4 @@
+import type { IOmnichannelRoom } from '@rocket.chat/core-typings';
 import { LivechatRooms } from '@rocket.chat/models';
 
 import { API } from '../../../../../app/api/server';
@@ -9,7 +10,12 @@ API.v1.addRoute(
 	{ authRequired: true, permissionsRequired: ['request-pdf-transcript'], license: ['livechat-enterprise'] },
 	{
 		async post() {
-			const room = await LivechatRooms.findOneById(this.urlParams.rid);
+			const room = await LivechatRooms.findOneById<Pick<IOmnichannelRoom, '_id' | 'open' | 'v' | 't' | 'pdfTranscriptFileId'>>(
+				this.urlParams.rid,
+				{
+					projection: { _id: 1, open: 1, v: 1, t: 1, pdfTranscriptFileId: 1 },
+				},
+			);
 			if (!room) {
 				throw new Error('error-invalid-room');
 			}

apps/meteor/ee/app/livechat-enterprise/server/lib/requestPdfTranscript.ts

@@ -1,12 +1,16 @@
 import { OmnichannelTranscript, QueueWorker } from '@rocket.chat/core-services';
 import type { AtLeast, IOmnichannelRoom } from '@rocket.chat/core-typings';
-import { LivechatRooms } from '@rocket.chat/models';
+import ExpiryMap from 'expiry-map';
 
 import { logger } from './logger';
 
+// Allow to request a transcript again after 15 seconds, assuming the first one didn't complete
+// This won't prevent multiple transcript generated for the same room in a multi-instance deployment since state is not shared, but we're ok with the drawbacks
+const LockMap = new ExpiryMap<string, boolean>(15000);
+
 const serviceName = 'omnichannel-transcript' as const;
 export const requestPdfTranscript = async (
-	room: AtLeast<IOmnichannelRoom, '_id' | 'open' | 'v' | 'pdfTranscriptRequested'>,
+	room: AtLeast<IOmnichannelRoom, '_id' | 'open' | 'v' | 'pdfTranscriptFileId'>,
 	requestedBy: string,
 ): Promise<void> => {
 	if (room.open) {
@@ -17,15 +21,14 @@ export const requestPdfTranscript = async (
 		throw new Error('improper-room-state');
 	}
 
-	// Don't request a transcript if there's already one requested :)
-	if (room.pdfTranscriptRequested) {
+	// Don't request a transcript if there's already one requested
+	if (LockMap.has(room._id) || room.pdfTranscriptFileId) {
 		// TODO: use logger
-		logger.info(`Transcript already requested for room ${room._id}`);
+		logger.info({ msg: `Transcript already requested`, roomId: room._id });
 		return;
 	}
 
-	// TODO: change this with a timestamp, allowing users to request a transcript again after a while if the first one fails
-	await LivechatRooms.setTranscriptRequestedPdfById(room._id);
+	LockMap.set(room._id, true);
 
 	const details = { details: { rid: room._id, userId: requestedBy, from: serviceName } };
 	// Make the whole process sync when running on test mode

apps/meteor/ee/server/models/raw/LivechatRooms.ts

@@ -43,7 +43,6 @@ declare module '@rocket.chat/model-typings' {
 		unsetPriorityByRoomId(roomId: string): Promise<UpdateResult>;
 		countPrioritizedRooms(): Promise<number>;
 		countRoomsWithSla(): Promise<number>;
-		countRoomsWithPdfTranscriptRequested(): Promise<number>;
 		countRoomsWithTranscriptSent(): Promise<number>;
 		getConversationsBySource(start: Date, end: Date, extraQuery: Filter<IOmnichannelRoom>): AggregationCursor<ReportResult>;
 		getConversationsByStatus(start: Date, end: Date, extraQuery: Filter<IOmnichannelRoom>): AggregationCursor<ReportResult>;
@@ -89,10 +88,6 @@ export class LivechatRoomsRawEE extends LivechatRoomsRaw implements ILivechatRoo
 		return this.countDocuments({ slaId: { $exists: true } });
 	}
 
-	countRoomsWithPdfTranscriptRequested(): Promise<number> {
-		return this.countDocuments({ pdfTranscriptRequested: true });
-	}
-
 	countRoomsWithTranscriptSent(): Promise<number> {
 		return this.countDocuments({ pdfTranscriptFileId: { $exists: true } });
 	}

apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts

@@ -2788,7 +2788,7 @@ describe('LIVECHAT - rooms', () => {
 
 			roomWithTranscriptGenerated = roomId;
 		});
-		it('should return immediately if transcript was already requested', async () => {
+		it('should return when transcript was already requested', async () => {
 			await request
 				.post(api(`omnichannel/${roomWithTranscriptGenerated}/request-transcript`))
 				.set(credentials)

apps/meteor/tests/unit/server/livechat/lib/requestPdfTranscript.spec.ts

@@ -3,18 +3,12 @@ import { describe, it, beforeEach, after } from 'mocha';
 import proxyquire from 'proxyquire';
 import sinon from 'sinon';
 
-const setStub = sinon.stub();
 const workOnPdfStub = sinon.stub();
 const queueWorkStub = sinon.stub();
 
 const { requestPdfTranscript } = proxyquire
 	.noCallThru()
 	.load('../../../../../ee/app/livechat-enterprise/server/lib/requestPdfTranscript.ts', {
-		'@rocket.chat/models': {
-			LivechatRooms: {
-				setTranscriptRequestedPdfById: setStub,
-			},
-		},
 		'@rocket.chat/core-services': {
 			OmnichannelTranscript: {
 				workOnPdf: workOnPdfStub,
@@ -29,7 +23,6 @@ describe('requestPdfTranscript', () => {
 	const currentTestModeValue = process.env.TEST_MODE;
 
 	beforeEach(() => {
-		setStub.reset();
 		workOnPdfStub.reset();
 		queueWorkStub.reset();
 	});
@@ -44,30 +37,37 @@ describe('requestPdfTranscript', () => {
 	it('should throw an error if room doesnt have a v property', async () => {
 		await expect(requestPdfTranscript({}, 'userId')).to.be.rejectedWith('improper-room-state');
 	});
-	it('should not request a transcript if it was already requested', async () => {
-		await requestPdfTranscript({ v: 1, pdfTranscriptRequested: true }, 'userId');
-		expect(setStub.callCount).to.equal(0);
-		expect(workOnPdfStub.callCount).to.equal(0);
-		expect(queueWorkStub.callCount).to.equal(0);
+	it('should not allow to request a transcript if it already exists', async () => {
+		const result = await requestPdfTranscript({ _id: 'roomIdxx', v: 1, pdfTranscriptFileId: 'afsdafadsfs' }, 'userId');
+		expect(result).to.be.undefined;
 	});
-	it('should set pdfTranscriptRequested to true on room', async () => {
-		await requestPdfTranscript({ _id: 'roomId', v: {}, pdfTranscriptRequested: false }, 'userId');
-		expect(setStub.calledWith('roomId')).to.be.true;
+	it('should not allow to request a transcript if it was already requested during the previous 15 seconds', async () => {
+		await requestPdfTranscript({ _id: 'roomId', v: 1 }, 'userId');
+		expect(workOnPdfStub.callCount).to.equal(0);
+		expect(queueWorkStub.callCount).to.equal(1);
+
+		const result = await requestPdfTranscript({ _id: 'roomId', v: 1 }, 'userId');
+		expect(workOnPdfStub.callCount).to.equal(0);
+		expect(queueWorkStub.callCount).to.equal(1);
+		expect(result).to.be.undefined;
 	});
 	it('should call workOnPdf if TEST_MODE is true', async () => {
 		process.env.TEST_MODE = 'true';
-		await requestPdfTranscript({ _id: 'roomId', v: {} }, 'userId');
-		expect(workOnPdfStub.getCall(0).calledWithExactly({ details: { rid: 'roomId', userId: 'userId', from: 'omnichannel-transcript' } })).to
-			.be.true;
+		await requestPdfTranscript({ _id: 'roomId-fasdafsdas', v: {} }, 'userId');
+		expect(
+			workOnPdfStub
+				.getCall(0)
+				.calledWithExactly({ details: { rid: 'roomId-fasdafsdas', userId: 'userId', from: 'omnichannel-transcript' } }),
+		).to.be.true;
 		expect(queueWorkStub.calledOnce).to.be.false;
 	});
 	it('should queue work if TEST_MODE is not set', async () => {
 		delete process.env.TEST_MODE;
-		await requestPdfTranscript({ _id: 'roomId', v: {} }, 'userId');
+		await requestPdfTranscript({ _id: 'roomId-afsdfaefzv', v: {} }, 'userId');
 		expect(workOnPdfStub.calledOnce).to.be.false;
 		expect(
 			queueWorkStub.getCall(0).calledWithExactly('work', 'omnichannel-transcript.workOnPdf', {
-				details: { rid: 'roomId', userId: 'userId', from: 'omnichannel-transcript' },
+				details: { rid: 'roomId-afsdfaefzv', userId: 'userId', from: 'omnichannel-transcript' },
 			}),
 		).to.be.true;
 	});

ee/packages/omnichannel-services/src/OmnichannelTranscript.spec.ts

@@ -37,8 +37,6 @@ jest.mock('@rocket.chat/core-services', () => ({
 jest.mock('@rocket.chat/models', () => ({
 	LivechatRooms: {
 		findOneById: jest.fn().mockResolvedValue({}),
-		unsetTranscriptRequestedPdfById: jest.fn(),
-		setPdfTranscriptFileIdById: jest.fn(),
 	},
 	Messages: {
 		findLivechatMessagesWithoutTypes: jest.fn().mockReturnValue({

ee/packages/omnichannel-services/src/OmnichannelTranscript.ts

@@ -18,6 +18,7 @@ import type {
 	ILivechatAgent,
 	IOmnichannelSystemMessage,
 	AtLeast,
+	IOmnichannelRoom,
 } from '@rocket.chat/core-typings';
 import { isQuoteAttachment, isFileAttachment, isFileImageAttachment } from '@rocket.chat/core-typings';
 import type { Logger } from '@rocket.chat/logger';
@@ -371,10 +372,19 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT
 		}
 		this.currentJobNumber++;
 		try {
-			const room = await LivechatRooms.findOneById(details.rid);
+			const room = await LivechatRooms.findOneById<Pick<IOmnichannelRoom, '_id' | 'v' | 'pdfTranscriptFileId' | 'closedAt' | 'servedBy'>>(
+				details.rid,
+				{
+					projection: { v: 1, servedBy: 1, pdfTranscriptFileId: 1, closedAt: 1 },
+				},
+			);
 			if (!room) {
 				throw new Error('room-not-found');
 			}
+			if (room.pdfTranscriptFileId) {
+				this.log.info(`Processing transcript for room ${details.rid} by user ${details.userId} - PDF already exists`);
+				return;
+			}
 			const messages = await this.getMessagesFromRoom({ rid: room._id });
 
 			const visitor =
@@ -436,18 +446,16 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT
 
 	private async pdfFailed({ details, e }: { details: WorkDetailsWithSource; e: Error }): Promise<void> {
 		this.log.error(`Transcript for room ${details.rid} by user ${details.userId} - Failed: ${e.message}`);
-		const room = await LivechatRooms.findOneById(details.rid);
+		const room = await LivechatRooms.findOneById<Pick<IOmnichannelRoom, '_id'>>(details.rid, { projection: { _id: 1 } });
 		if (!room) {
 			return;
 		}
-		const user = await Users.findOneById(details.userId);
+		// TODO: fix types of translate service (or deprecate, if possible)
+		const user = await Users.findOneById(details.userId, { projection: { _id: 1, language: 1 } });
 		if (!user) {
 			return;
 		}
 
-		// Remove `transcriptRequestedPdf` from room to allow another request
-		await LivechatRooms.unsetTranscriptRequestedPdfById(details.rid);
-
 		const { rid } = await roomService.createDirectMessage({ to: details.userId, from: 'rocket.cat' });
 		this.log.info(`Transcript for room ${details.rid} by user ${details.userId} - Sending error message to user`);
 		await messageService.sendMessage({

packages/core-typings/src/IRoom.ts

@@ -292,9 +292,6 @@ export interface IOmnichannelRoom extends IOmnichannelGenericRoom {
 	slaId?: string;
 	estimatedWaitingTimeQueue: IOmnichannelServiceLevelAgreements['dueTimeInMinutes']; // It should always have a default value for sorting mechanism to work
 
-	// Signals if the room already has a pdf transcript requested
-	// This prevents the user from requesting a transcript multiple times
-	pdfTranscriptRequested?: boolean;
 	// The ID of the pdf file generated for the transcript
 	// This will help if we want to have this file shown on other places of the UI
 	pdfTranscriptFileId?: string;

packages/model-typings/src/models/ILivechatRoomsModel.ts

@@ -128,9 +128,6 @@ export interface ILivechatRoomsModel extends IBaseModel<IOmnichannelRoom> {
 	setAutoTransferredAtById(roomId: string): Promise<UpdateResult>;
 
 	findAvailableSources(): AggregationCursor<Document>;
-
-	setTranscriptRequestedPdfById(rid: string): Promise<UpdateResult>;
-	unsetTranscriptRequestedPdfById(rid: string): Promise<UpdateResult>;
 	setPdfTranscriptFileIdById(rid: string, fileId: string): Promise<UpdateResult>;
 
 	setEmailTranscriptRequestedByRoomId(

packages/models/src/models/LivechatRooms.ts

@@ -68,7 +68,8 @@ export class LivechatRoomsRaw extends BaseRaw<IOmnichannelRoom> implements ILive
 				},
 			},
 			{ key: { 'livechatData.$**': 1 } },
-			{ key: { pdfTranscriptRequested: 1 }, sparse: true },
+			// TODO: Remove index on next major
+			// { key: { pdfTranscriptRequested: 1 }, sparse: true },
 			{ key: { pdfTranscriptFileId: 1 }, sparse: true }, // used on statistics
 			{ key: { callStatus: 1 }, sparse: true }, // used on statistics
 			{ key: { priorityId: 1 }, sparse: true },
@@ -1577,30 +1578,6 @@ export class LivechatRoomsRaw extends BaseRaw<IOmnichannelRoom> implements ILive
 		]);
 	}
 
-	// These 3 methods shouldn't be here :( but current EE model has a meteor dependency
-	// And refactoring it could take time
-	setTranscriptRequestedPdfById(rid: string) {
-		return this.updateOne(
-			{
-				_id: rid,
-			},
-			{
-				$set: { pdfTranscriptRequested: true },
-			},
-		);
-	}
-
-	unsetTranscriptRequestedPdfById(rid: string) {
-		return this.updateOne(
-			{
-				_id: rid,
-			},
-			{
-				$unset: { pdfTranscriptRequested: 1 },
-			},
-		);
-	}
-
 	setPdfTranscriptFileIdById(rid: string, fileId: string) {
 		return this.updateOne(
 			{
@@ -2708,10 +2685,6 @@ export class LivechatRoomsRaw extends BaseRaw<IOmnichannelRoom> implements ILive
 		throw new Error('Method not implemented.');
 	}
 
-	countRoomsWithPdfTranscriptRequested(): Promise<number> {
-		throw new Error('Method not implemented.');
-	}
-
 	countRoomsWithTranscriptSent(): Promise<number> {
 		throw new Error('Method not implemented.');
 	}