refactor: move system message calls from sendMessage to saveSystemMes…

…sage (#32842) Co-authored-by: Guilherme Gazzo <[email protected]>
RocketChat · Aug 19, 2024 · 8162986 · 8162986
1 parent bbdff10
commit 8162986
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 70 deletions.
diff --git a/apps/meteor/app/discussion/server/methods/createDiscussion.ts b/apps/meteor/app/discussion/server/methods/createDiscussion.ts
@@ -27,13 +27,11 @@ async function createDiscussionMessage(
 	drid: IRoom['_id'],
 	msg: IMessage['msg'],
 	messageEmbedded?: MessageAttachmentDefault,
-): Promise<IMessage | null> {
-	const msgId = await Message.saveSystemMessage('discussion-created', rid, msg, user, {
+): Promise<IMessage> {
+	return Message.saveSystemMessage('discussion-created', rid, msg, user, {
 		drid,
 		...(messageEmbedded && { attachments: [messageEmbedded] }),
 	});
-
-	return Messages.findOneById(msgId);
 }
 
 async function mentionMessage(

diff --git a/apps/meteor/app/lib/server/lib/notifyUsersOnMessage.ts b/apps/meteor/app/lib/server/lib/notifyUsersOnMessage.ts
@@ -183,6 +183,20 @@ export async function notifyUsersOnMessage(message: IMessage, room: IRoom, roomU
 	return message;
 }
 
+export async function notifyUsersOnSystemMessage(message: IMessage, room: IRoom): Promise<IMessage> {
+	const roomUpdater = Rooms.getUpdater();
+	Rooms.setIncMsgCountAndSetLastMessageUpdateQuery(1, message, !!settings.get('Store_Last_Message'), roomUpdater);
+
+	if (roomUpdater.hasChanges()) {
+		await Rooms.updateFromUpdater({ _id: room._id }, roomUpdater);
+	}
+
+	// TODO: Rewrite to use just needed calls from the function
+	await updateUsersSubscriptions(message, room);
+
+	return message;
+}
+
 callbacks.add(
 	'afterSaveMessage',
 	async (message, { room, roomUpdater }) => {

diff --git a/apps/meteor/app/livechat/server/api/v1/pageVisited.ts b/apps/meteor/app/livechat/server/api/v1/pageVisited.ts
@@ -1,5 +1,4 @@
 import type { IOmnichannelSystemMessage } from '@rocket.chat/core-typings';
-import { Messages } from '@rocket.chat/models';
 import { isPOSTLivechatPageVisitedParams } from '@rocket.chat/rest-typings';
 
 import { API } from '../../../../api/server';
@@ -11,17 +10,13 @@ API.v1.addRoute(
 	{
 		async post() {
 			const { token, rid, pageInfo } = this.bodyParams;
-			const msgId = await Livechat.savePageHistory(token, rid, pageInfo);
-			if (!msgId) {
-				return API.v1.success();
-			}
 
-			const message = await Messages.findOneById<IOmnichannelSystemMessage>(msgId);
+			const message = await Livechat.savePageHistory(token, rid, pageInfo);
 			if (!message) {
 				return API.v1.success();
 			}
 
-			const { msg, navigation } = message;
+			const { msg, navigation } = message as IOmnichannelSystemMessage;
 			return API.v1.success({ page: { msg, navigation } });
 		},
 	},

diff --git a/apps/meteor/app/livechat/server/lib/Helper.ts b/apps/meteor/app/livechat/server/lib/Helper.ts
@@ -36,7 +36,6 @@ import { validateEmail as validatorFunc } from '../../../../lib/emailValidator';
 import { i18n } from '../../../../server/lib/i18n';
 import { hasRoleAsync } from '../../../authorization/server/functions/hasRole';
 import { sendNotification } from '../../../lib/server';
-import { sendMessage } from '../../../lib/server/functions/sendMessage';
 import {
 	notifyOnLivechatDepartmentAgentChanged,
 	notifyOnLivechatDepartmentAgentChangedByAgentsAndDepartmentId,
@@ -141,10 +140,7 @@ export const createLivechatRoom = async <
 	}
 
 	await callbacks.run('livechat.newRoom', room);
-
-	// TODO: replace with `Message.saveSystemMessage`
-
-	await sendMessage(guest, { t: 'livechat-started', msg: '', groupable: false, token: guest.token }, room);
+	await Message.saveSystemMessageAndNotifyUser('livechat-started', rid, '', { _id, username }, { groupable: false, token: guest.token });
 
 	return result.value as IOmnichannelRoom;
 };

diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts
@@ -252,7 +252,6 @@ class LivechatClass {
 		const isRoomClosedByVisitorParams = (params: CloseRoomParams): params is CloseRoomParamsByVisitor =>
 			(params as CloseRoomParamsByVisitor).visitor !== undefined;
 
-		let chatCloser: any;
 		if (isRoomClosedByUserParams(params)) {
 			const { user } = params;
 			this.logger.debug(`Closing by user ${user?._id}`);
@@ -261,7 +260,6 @@ class LivechatClass {
 				_id: user?._id || '',
 				username: user?.username,
 			};
-			chatCloser = user;
 		} else if (isRoomClosedByVisitorParams(params)) {
 			const { visitor } = params;
 			this.logger.debug(`Closing by visitor ${params.visitor._id}`);
@@ -270,7 +268,6 @@ class LivechatClass {
 				_id: visitor._id,
 				username: visitor.username,
 			};
-			chatCloser = visitor;
 		} else {
 			throw new Error('Error: Please provide details of the user or visitor who closed the room');
 		}
@@ -296,35 +293,27 @@ class LivechatClass {
 
 		this.logger.debug(`DB updated for room ${room._id}`);
 
-		const transcriptRequested =
-			!!transcriptRequest || (!settings.get('Livechat_enable_transcript') && settings.get('Livechat_transcript_send_always'));
-
-		// Retrieve the closed room
 		const newRoom = await LivechatRooms.findOneById(rid);
 
 		if (!newRoom) {
 			throw new Error('Error: Room not found');
 		}
 
 		this.logger.debug(`Sending closing message to room ${room._id}`);
-		await sendMessage(
-			chatCloser,
-			{
-				t: 'livechat-close',
-				msg: comment,
-				groupable: false,
-				transcriptRequested,
-				...(isRoomClosedByVisitorParams(params) && { token: chatCloser.token }),
-			},
-			newRoom,
-		);
+
+		const transcriptRequested =
+			!!transcriptRequest || (!settings.get('Livechat_enable_transcript') && settings.get('Livechat_transcript_send_always'));
+
+		await Message.saveSystemMessageAndNotifyUser('livechat-close', rid, comment ?? '', closeData.closedBy, {
+			groupable: false,
+			transcriptRequested,
+			...(isRoomClosedByVisitorParams(params) && { token: params.visitor.token }),
+		});
 
 		if (settings.get('Livechat_enable_transcript') && !settings.get('Livechat_transcript_send_always')) {
 			await Message.saveSystemMessage('command', rid, 'promptTranscript', closeData.closedBy);
 		}
 
-		this.logger.debug(`Running callbacks for room ${newRoom._id}`);
-
 		process.nextTick(() => {
 			/**
 			 * @deprecated the `AppEvents.ILivechatRoomClosedHandler` event will be removed
@@ -1254,31 +1243,20 @@ class LivechatClass {
 		const scopeData = scope || (nextDepartment ? 'department' : 'agent');
 		this.logger.info(`Storing new chat transfer of ${room._id} [Transfered by: ${_id} to ${scopeData}]`);
 
-		await sendMessage(
-			transferredBy,
-			{
-				t: 'livechat_transfer_history',
-				rid: room._id,
+		const transferMessage = {
+			...(transferData.transferredBy.userType === 'visitor' && { token: room.v.token }),
+			transferData: {
+				transferredBy,
 				ts: new Date(),
-				msg: '',
-				u: {
-					_id,
-					username,
-				},
-				groupable: false,
-				...(transferData.transferredBy.userType === 'visitor' && { token: room.v.token }),
-				transferData: {
-					transferredBy,
-					ts: new Date(),
-					scope: scopeData,
-					comment,
-					...(previousDepartment && { previousDepartment }),
-					...(nextDepartment && { nextDepartment }),
-					...(transferredTo && { transferredTo }),
-				},
+				scope: scopeData,
+				comment,
+				...(previousDepartment && { previousDepartment }),
+				...(nextDepartment && { nextDepartment }),
+				...(transferredTo && { transferredTo }),
 			},
-			room,
-		);
+		};
+
+		await Message.saveSystemMessageAndNotifyUser('livechat_transfer_history', room._id, '', { _id, username }, transferMessage);
 	}
 
 	async saveGuest(guestData: Pick<ILivechatVisitor, '_id' | 'name' | 'livechatData'> & { email?: string; phone?: string }, userId: string) {

diff --git a/apps/meteor/app/message-pin/server/pinMessage.ts b/apps/meteor/app/message-pin/server/pinMessage.ts
@@ -134,7 +134,7 @@ Meteor.methods<ServerMethods>({
 
 		const pinMessageType = originalMessage.t === 'e2e' ? 'message_pinned_e2e' : 'message_pinned';
 
-		const msgId = await Message.saveSystemMessage(pinMessageType, originalMessage.rid, '', me, {
+		return Message.saveSystemMessage(pinMessageType, originalMessage.rid, '', me, {
 			attachments: [
 				{
 					text: originalMessage.msg,
@@ -145,8 +145,6 @@ Meteor.methods<ServerMethods>({
 				},
 			],
 		});
-
-		return Messages.findOneById(msgId);
 	},
 	async unpinMessage(message) {
 		check(message._id, String);

diff --git a/apps/meteor/server/services/messages/service.ts b/apps/meteor/server/services/messages/service.ts
@@ -6,7 +6,8 @@ import { Messages, Rooms } from '@rocket.chat/models';
 import { deleteMessage } from '../../../app/lib/server/functions/deleteMessage';
 import { sendMessage } from '../../../app/lib/server/functions/sendMessage';
 import { updateMessage } from '../../../app/lib/server/functions/updateMessage';
-import { notifyOnMessageChange } from '../../../app/lib/server/lib/notifyListener';
+import { notifyOnRoomChangedById, notifyOnMessageChange } from '../../../app/lib/server/lib/notifyListener';
+import { notifyUsersOnSystemMessage } from '../../../app/lib/server/lib/notifyUsersOnMessage';
 import { executeSendMessage } from '../../../app/lib/server/methods/sendMessage';
 import { executeSetReaction } from '../../../app/reactions/server/setReaction';
 import { settings } from '../../../app/settings/server';
@@ -97,19 +98,38 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
 		return executeSetReaction(userId, reaction, messageId, shouldReact);
 	}
 
+	async saveSystemMessageAndNotifyUser<T = IMessage>(
+		type: MessageTypesValues,
+		rid: string,
+		messageText: string,
+		owner: Pick<IUser, '_id' | 'username' | 'name'>,
+		extraData?: Partial<T>,
+	): Promise<IMessage> {
+		const createdMessage = await this.saveSystemMessage(type, rid, messageText, owner, extraData);
+
+		const room = await Rooms.findOneById(rid);
+		if (!room) {
+			throw new Error('Failed to find the room.');
+		}
+
+		await notifyUsersOnSystemMessage(createdMessage, room);
+
+		return createdMessage;
+	}
+
 	async saveSystemMessage<T = IMessage>(
 		type: MessageTypesValues,
 		rid: string,
 		message: string,
 		owner: Pick<IUser, '_id' | 'username' | 'name'>,
 		extraData?: Partial<T>,
-	): Promise<IMessage['_id']> {
+	): Promise<IMessage> {
 		const { _id: userId, username, name } = owner;
 		if (!username) {
 			throw new Error('The username cannot be empty.');
 		}
 
-		const [result] = await Promise.all([
+		const [{ insertedId }] = await Promise.all([
 			Messages.createWithTypeRoomIdMessageUserAndUnread(
 				type,
 				rid,
@@ -121,11 +141,19 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
 			Rooms.incMsgCountById(rid, 1),
 		]);
 
-		void notifyOnMessageChange({
-			id: result.insertedId,
-		});
+		if (!insertedId) {
+			throw new Error('Failed to save system message.');
+		}
+
+		const createdMessage = await Messages.findOneById(insertedId);
+		if (!createdMessage) {
+			throw new Error('Failed to find the created message.');
+		}
+
+		void notifyOnMessageChange({ id: createdMessage._id, data: createdMessage });
+		void notifyOnRoomChangedById(rid);
 
-		return result.insertedId;
+		return createdMessage;
 	}
 
 	async beforeSave({

diff --git a/packages/core-services/src/types/IMessageService.ts b/packages/core-services/src/types/IMessageService.ts
@@ -8,7 +8,14 @@ export interface IMessageService {
 		message: string,
 		user: Pick<IUser, '_id' | 'username' | 'name'>,
 		extraData?: Partial<T>,
-	): Promise<IMessage['_id']>;
+	): Promise<IMessage>;
+	saveSystemMessageAndNotifyUser<T = IMessage>(
+		type: MessageTypesValues,
+		rid: string,
+		message: string,
+		user: Pick<IUser, '_id' | 'username' | 'name'>,
+		extraData?: Partial<T>,
+	): Promise<IMessage>;
 	beforeSave(param: { message: IMessage; room: IRoom; user: IUser }): Promise<IMessage>;
 	sendMessageWithValidation(user: IUser, message: Partial<IMessage>, room: Partial<IRoom>, upsert?: boolean): Promise<IMessage>;
 	deleteMessage(user: IUser, message: IMessage): Promise<void>;