refactor: return created record from registerGuest (#32620)

Co-authored-by: Guilherme Gazzo <[email protected]> Co-authored-by: Diego Sampaio <[email protected]>
RocketChat · Jun 27, 2024 · 7a57f34 · 7a57f34
1 parent b5866a1
commit 7a57f34
Show file tree

Hide file tree

Showing 11 changed files with 192 additions and 113 deletions.
diff --git a/apps/meteor/app/apps/server/bridges/livechat.ts b/apps/meteor/app/apps/server/bridges/livechat.ts
@@ -201,7 +201,33 @@ export class AppLivechatBridge extends LivechatBridge {
 			...(visitor.visitorEmails?.length && { email: visitor.visitorEmails[0].address }),
 		};
 
-		return LivechatTyped.registerGuest(registerData);
+		const livechatVisitor = await LivechatTyped.registerGuest(registerData);
+
+		if (!livechatVisitor) {
+			throw new Error('Invalid visitor, cannot create');
+		}
+
+		return livechatVisitor._id;
+	}
+
+	protected async createAndReturnVisitor(visitor: IVisitor, appId: string): Promise<IVisitor | undefined> {
+		this.orch.debugLog(`The App ${appId} is creating a livechat visitor.`);
+
+		const registerData = {
+			department: visitor.department,
+			username: visitor.username,
+			name: visitor.name,
+			token: visitor.token,
+			email: '',
+			connectionData: undefined,
+			id: visitor.id,
+			...(visitor.phone?.length && { phone: { number: visitor.phone[0].phoneNumber } }),
+			...(visitor.visitorEmails?.length && { email: visitor.visitorEmails[0].address }),
+		};
+
+		const livechatVisitor = await LivechatTyped.registerGuest(registerData);
+
+		return this.orch.getConverters()?.get('visitors').convertVisitor(livechatVisitor);
 	}
 
 	protected async transferVisitor(visitor: IVisitor, transferData: ILivechatTransferData, appId: string): Promise<boolean> {

diff --git a/apps/meteor/app/livechat/imports/server/rest/sms.ts b/apps/meteor/app/livechat/imports/server/rest/sms.ts
@@ -67,8 +67,13 @@ const defineVisitor = async (smsNumber: string, targetDepartment?: string) => {
 		data.department = targetDepartment;
 	}
 
-	const id = await LivechatTyped.registerGuest(data);
-	return LivechatVisitors.findOneEnabledById(id);
+	const livechatVisitor = await LivechatTyped.registerGuest(data);
+
+	if (!livechatVisitor) {
+		throw new Meteor.Error('error-invalid-visitor', 'Invalid visitor');
+	}
+
+	return livechatVisitor;
 };
 
 const normalizeLocationSharing = (payload: ServiceData) => {

diff --git a/apps/meteor/app/livechat/server/api/v1/message.ts b/apps/meteor/app/livechat/server/api/v1/message.ts
@@ -251,7 +251,7 @@ API.v1.addRoute(
 		async post() {
 			const visitorToken = this.bodyParams.visitor.token;
 
-			let visitor = await LivechatVisitors.getVisitorByToken(visitorToken, {});
+			const visitor = await LivechatVisitors.getVisitorByToken(visitorToken, {});
 			let rid: string;
 			if (visitor) {
 				const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {});
@@ -267,8 +267,10 @@ API.v1.addRoute(
 				const guest: typeof this.bodyParams.visitor & { connectionData?: unknown } = this.bodyParams.visitor;
 				guest.connectionData = normalizeHttpHeaderData(this.request.headers);
 
-				const visitorId = await LivechatTyped.registerGuest(guest);
-				visitor = await LivechatVisitors.findOneEnabledById(visitorId);
+				const visitor = await LivechatTyped.registerGuest(guest);
+				if (!visitor) {
+					throw new Error('error-livechat-visitor-registration');
+				}
 			}
 
 			const guest = visitor;

diff --git a/apps/meteor/app/livechat/server/api/v1/visitor.ts b/apps/meteor/app/livechat/server/api/v1/visitor.ts
@@ -1,4 +1,4 @@
-import type { ILivechatCustomField, ILivechatVisitor, IRoom } from '@rocket.chat/core-typings';
+import type { ILivechatCustomField, IRoom } from '@rocket.chat/core-typings';
 import { LivechatVisitors as VisitorsRaw, LivechatCustomField, LivechatRooms } from '@rocket.chat/models';
 import { Match, check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
@@ -47,27 +47,29 @@ API.v1.addRoute('livechat/visitor', {
 			connectionData: normalizeHttpHeaderData(this.request.headers),
 		};
 
-		const visitorId = await LivechatTyped.registerGuest(guest);
-
-		let visitor: ILivechatVisitor | null = await VisitorsRaw.findOneEnabledById(visitorId, {});
-		if (visitor) {
-			const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {});
-			// If it's updating an existing visitor, it must also update the roomInfo
-			const rooms = await LivechatRooms.findOpenByVisitorToken(visitor?.token, {}, extraQuery).toArray();
-			await Promise.all(
-				rooms.map(
-					(room: IRoom) =>
-						visitor &&
-						LivechatTyped.saveRoomInfo(room, {
-							_id: visitor._id,
-							name: visitor.name,
-							phone: visitor.phone?.[0]?.phoneNumber,
-							livechatData: visitor.livechatData as { [k: string]: string },
-						}),
-				),
-			);
+		const visitor = await LivechatTyped.registerGuest(guest);
+		if (!visitor) {
+			throw new Meteor.Error('error-livechat-visitor-registration', 'Error registering visitor', {
+				method: 'livechat/visitor',
+			});
 		}
 
+		const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {});
+		// If it's updating an existing visitor, it must also update the roomInfo
+		const rooms = await LivechatRooms.findOpenByVisitorToken(visitor?.token, {}, extraQuery).toArray();
+		await Promise.all(
+			rooms.map(
+				(room: IRoom) =>
+					visitor &&
+					LivechatTyped.saveRoomInfo(room, {
+						_id: visitor._id,
+						name: visitor.name,
+						phone: visitor.phone?.[0]?.phoneNumber,
+						livechatData: visitor.livechatData as { [k: string]: string },
+					}),
+			),
+		);
+
 		if (customFields && Array.isArray(customFields) && customFields.length > 0) {
 			const keys = customFields.map((field) => field.key);
 			const errors: string[] = [];
@@ -96,21 +98,21 @@ API.v1.addRoute('livechat/visitor', {
 			if (processedKeys.length !== keys.length) {
 				LivechatTyped.logger.warn({
 					msg: 'Some custom fields were not processed',
-					visitorId,
+					visitorId: visitor._id,
 					missingKeys: keys.filter((key) => !processedKeys.includes(key)),
 				});
 			}
 
 			if (errors.length > 0) {
 				LivechatTyped.logger.error({
 					msg: 'Error updating custom fields',
-					visitorId,
+					visitorId: visitor._id,
 					errors,
 				});
 				throw new Error('error-updating-custom-fields');
 			}
 
-			visitor = await VisitorsRaw.findOneEnabledById(visitorId, {});
+			return API.v1.success({ visitor: await VisitorsRaw.findOneEnabledById(visitor._id) });
 		}
 
 		if (!visitor) {

diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts
@@ -43,7 +43,7 @@ import { serverFetch as fetch } from '@rocket.chat/server-fetch';
 import { Match, check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
 import moment from 'moment-timezone';
-import type { Filter, FindCursor, UpdateFilter } from 'mongodb';
+import type { Filter, FindCursor } from 'mongodb';
 import UAParser from 'ua-parser-js';
 
 import { callbacks } from '../../../../lib/callbacks';
@@ -76,6 +76,13 @@ import { QueueManager } from './QueueManager';
 import { RoutingManager } from './RoutingManager';
 import { isDepartmentCreationAvailable } from './isDepartmentCreationAvailable';
 
+type RegisterGuestType = Partial<Pick<ILivechatVisitor, 'token' | 'name' | 'department' | 'status' | 'username'>> & {
+	id?: string;
+	connectionData?: any;
+	email?: string;
+	phone?: { number: string };
+};
+
 type GenericCloseRoomParams = {
 	room: IOmnichannelRoom;
 	comment?: string;
@@ -426,6 +433,7 @@ class LivechatClass {
 
 		if (room == null) {
 			const defaultAgent = await callbacks.run('livechat.checkDefaultAgentOnNewRoom', agent, guest);
+
 			// if no department selected verify if there is at least one active and pick the first
 			if (!defaultAgent && !guest.department) {
 				const department = await this.getRequiredDepartment();
@@ -439,6 +447,7 @@ class LivechatClass {
 
 			// delegate room creation to QueueManager
 			Livechat.logger.debug(`Calling QueueManager to request a room for visitor ${guest._id}`);
+
 			room = await QueueManager.requestRoom({
 				guest,
 				message,
@@ -666,105 +675,89 @@ class LivechatClass {
 		id,
 		token,
 		name,
+		phone,
 		email,
 		department,
-		phone,
 		username,
 		connectionData,
 		status = UserStatus.ONLINE,
-	}: {
-		id?: string;
-		token: string;
-		name?: string;
-		email?: string;
-		department?: string;
-		phone?: { number: string };
-		username?: string;
-		connectionData?: any;
-		status?: ILivechatVisitor['status'];
-	}) {
+	}: RegisterGuestType): Promise<ILivechatVisitor | null> {
 		check(token, String);
 		check(id, Match.Maybe(String));
 
 		Livechat.logger.debug(`New incoming conversation: id: ${id} | token: ${token}`);
 
-		let userId;
-		type Mutable<Type> = {
-			-readonly [Key in keyof Type]: Type[Key];
-		};
-
-		type UpdateUserType = Required<Pick<UpdateFilter<ILivechatVisitor>, '$set'>>;
-		const updateUser: Required<Pick<UpdateFilter<ILivechatVisitor>, '$set'>> = {
-			$set: {
-				token,
-				status,
-				...(phone?.number ? { phone: [{ phoneNumber: phone.number }] } : {}),
-				...(name ? { name } : {}),
-			},
+		const visitorDataToUpdate: Partial<ILivechatVisitor> & { userAgent?: string; ip?: string; host?: string } = {
+			token,
+			status,
+			...(phone?.number ? { phone: [{ phoneNumber: phone.number }] } : {}),
+			...(name ? { name } : {}),
 		};
 
 		if (email) {
-			email = email.trim().toLowerCase();
-			validateEmail(email);
-			(updateUser.$set as Mutable<UpdateUserType['$set']>).visitorEmails = [{ address: email }];
+			const visitorEmail = email.trim().toLowerCase();
+			validateEmail(visitorEmail);
+			visitorDataToUpdate.visitorEmails = [{ address: visitorEmail }];
 		}
 
 		if (department) {
 			Livechat.logger.debug(`Attempt to find a department with id/name ${department}`);
 			const dep = await LivechatDepartment.findOneByIdOrName(department, { projection: { _id: 1 } });
 			if (!dep) {
-				Livechat.logger.debug('Invalid department provided');
+				Livechat.logger.debug(`Invalid department provided: ${department}`);
 				throw new Meteor.Error('error-invalid-department', 'The provided department is invalid');
 			}
 			Livechat.logger.debug(`Assigning visitor ${token} to department ${dep._id}`);
-			(updateUser.$set as Mutable<UpdateUserType['$set']>).department = dep._id;
+			visitorDataToUpdate.department = dep._id;
 		}
 
-		const user = await LivechatVisitors.getVisitorByToken(token, { projection: { _id: 1 } });
+		const livechatVisitor = await LivechatVisitors.getVisitorByToken(token, { projection: { _id: 1 } });
+
+		visitorDataToUpdate.token = livechatVisitor?.token || token;
+
 		let existingUser = null;
 
-		if (user) {
+		if (livechatVisitor) {
 			Livechat.logger.debug('Found matching user by token');
-			userId = user._id;
+			visitorDataToUpdate._id = livechatVisitor._id;
 		} else if (phone?.number && (existingUser = await LivechatVisitors.findOneVisitorByPhone(phone.number))) {
 			Livechat.logger.debug('Found matching user by phone number');
-			userId = existingUser._id;
+			visitorDataToUpdate._id = existingUser._id;
 			// Don't change token when matching by phone number, use current visitor token
-			(updateUser.$set as Mutable<UpdateUserType['$set']>).token = existingUser.token;
+			visitorDataToUpdate.token = existingUser.token;
 		} else if (email && (existingUser = await LivechatVisitors.findOneGuestByEmailAddress(email))) {
 			Livechat.logger.debug('Found matching user by email');
-			userId = existingUser._id;
-		} else {
+			visitorDataToUpdate._id = existingUser._id;
+		} else if (!livechatVisitor) {
 			Livechat.logger.debug(`No matches found. Attempting to create new user with token ${token}`);
-			if (!username) {
-				username = await LivechatVisitors.getNextVisitorUsername();
-			}
 
-			const userData = {
-				username,
-				status,
-				ts: new Date(),
-				token,
-				...(id && { _id: id }),
-			};
+			visitorDataToUpdate._id = id || undefined;
+			visitorDataToUpdate.username = username || (await LivechatVisitors.getNextVisitorUsername());
+			visitorDataToUpdate.status = status;
+			visitorDataToUpdate.ts = new Date();
 
 			if (settings.get('Livechat_Allow_collect_and_store_HTTP_header_informations')) {
 				Livechat.logger.debug(`Saving connection data for visitor ${token}`);
-				const connection = connectionData;
-				if (connection?.httpHeaders) {
-					(updateUser.$set as Mutable<UpdateUserType['$set']>).userAgent = connection.httpHeaders['user-agent'];
-					(updateUser.$set as Mutable<UpdateUserType['$set']>).ip =
-						connection.httpHeaders['x-real-ip'] || connection.httpHeaders['x-forwarded-for'] || connection.clientAddress;
-					(updateUser.$set as Mutable<UpdateUserType['$set']>).host = connection.httpHeaders.host;
+				const { httpHeaders, clientAddress } = connectionData;
+				if (httpHeaders) {
+					visitorDataToUpdate.userAgent = httpHeaders['user-agent'];
+					visitorDataToUpdate.ip = httpHeaders['x-real-ip'] || httpHeaders['x-forwarded-for'] || clientAddress;
+					visitorDataToUpdate.host = httpHeaders?.host;
 				}
 			}
-
-			userId = (await LivechatVisitors.insertOne(userData)).insertedId;
 		}
 
-		await LivechatVisitors.updateById(userId, updateUser);
+		const upsertedLivechatVisitor = await LivechatVisitors.updateOneByIdOrToken(visitorDataToUpdate, {
+			upsert: true,
+			returnDocument: 'after',
+		});
+
+		if (!upsertedLivechatVisitor.value) {
+			Livechat.logger.debug(`No visitor found after upsert`);
+			return null;
+		}
 
-		return userId;
+		return upsertedLivechatVisitor.value;
 	}
 
 	private async getBotAgents(department?: string) {