regression: old omnichannel rooms only being migrated after their dat…

…a is loaded by the client (#34090)

apps/meteor/app/api/server/lib/maybeMigrateLivechatRoom.ts

@@ -1,5 +1,6 @@
-import { isOmnichannelRoom, type IRoom } from '@rocket.chat/core-typings';
-import { Rooms } from '@rocket.chat/models';
+import { isOmnichannelRoom } from '@rocket.chat/core-typings';
+import type { IOmnichannelRoom, IRoom } from '@rocket.chat/core-typings';
+import { LivechatRooms } from '@rocket.chat/models';
 import type { FindOptions } from 'mongodb';
 
 import { projectionAllowsAttribute } from './projectionAllowsAttribute';
@@ -9,7 +10,10 @@ import { migrateVisitorIfMissingContact } from '../../../livechat/server/lib/con
  * If the room is a livechat room and it doesn't yet have a contact, trigger the migration for its visitor and source
  * The migration will create/use a contact and assign it to every room that matches this visitorId and source.
  **/
-export async function maybeMigrateLivechatRoom(room: IRoom | null, options: FindOptions<IRoom> = {}): Promise<IRoom | null> {
+export async function maybeMigrateLivechatRoom(
+	room: IOmnichannelRoom | null,
+	options: FindOptions<IRoom> = {},
+): Promise<IOmnichannelRoom | null> {
 	if (!room || !isOmnichannelRoom(room)) {
 		return room;
 	}
@@ -32,5 +36,5 @@ export async function maybeMigrateLivechatRoom(room: IRoom | null, options: Find
 	}
 
 	// Load the room again with the same options so it can be reloaded with the contactId in place
-	return Rooms.findOneById(room._id, options);
+	return LivechatRooms.findOneById(room._id, options);
 }

apps/meteor/app/api/server/v1/rooms.ts

@@ -32,7 +32,6 @@ import { composeRoomWithLastMessage } from '../helpers/composeRoomWithLastMessag
 import { getPaginationItems } from '../helpers/getPaginationItems';
 import { getUserFromParams } from '../helpers/getUserFromParams';
 import { getUploadFormData } from '../lib/getUploadFormData';
-import { maybeMigrateLivechatRoom } from '../lib/maybeMigrateLivechatRoom';
 import {
 	findAdminRoom,
 	findAdminRooms,
@@ -442,10 +441,8 @@ API.v1.addRoute(
 			const { team, parentRoom } = await Team.getRoomInfo(room);
 			const parent = discussionParent || parentRoom;
 
-			const options = { projection: fields };
-
 			return API.v1.success({
-				room: (await maybeMigrateLivechatRoom(await Rooms.findOneByIdOrName(room._id, options), options)) ?? undefined,
+				room: await Rooms.findOneByIdOrName(room._id, { projection: fields }),
 				...(team && { team }),
 				...(parent && { parent }),
 			});

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

@@ -14,7 +14,6 @@ import { Meteor } from 'meteor/meteor';
 import { API } from '../../../../api/server';
 import { getPaginationItems } from '../../../../api/server/helpers/getPaginationItems';
 import { createContact } from '../../lib/contacts/createContact';
-import { getContactByChannel } from '../../lib/contacts/getContactByChannel';
 import { getContactChannelsGrouped } from '../../lib/contacts/getContactChannelsGrouped';
 import { getContactHistory } from '../../lib/contacts/getContactHistory';
 import { getContacts } from '../../lib/contacts/getContacts';
@@ -133,13 +132,13 @@ API.v1.addRoute(
 	{ authRequired: true, permissionsRequired: ['view-livechat-contact'], validateParams: isGETOmnichannelContactsProps },
 	{
 		async get() {
-			const { contactId, visitor } = this.queryParams;
+			const { contactId } = this.queryParams;
 
-			if (!contactId && !visitor) {
+			if (!contactId) {
 				return API.v1.notFound();
 			}
 
-			const contact = await (contactId ? LivechatContacts.findOneById(contactId) : getContactByChannel(visitor));
+			const contact = await LivechatContacts.findOneById(contactId);
 
 			if (!contact) {
 				return API.v1.notFound();

apps/meteor/app/livechat/server/startup.ts

@@ -13,6 +13,7 @@ import { callbacks } from '../../../lib/callbacks';
 import { beforeLeaveRoomCallback } from '../../../lib/callbacks/beforeLeaveRoomCallback';
 import { i18n } from '../../../server/lib/i18n';
 import { roomCoordinator } from '../../../server/lib/rooms/roomCoordinator';
+import { maybeMigrateLivechatRoom } from '../../api/server/lib/maybeMigrateLivechatRoom';
 import { hasPermissionAsync } from '../../authorization/server/functions/hasPermission';
 import { notifyOnUserChange } from '../../lib/server/lib/notifyListener';
 import { settings } from '../../settings/server';
@@ -21,7 +22,7 @@ import './roomAccessValidator.internalService';
 const logger = new Logger('LivechatStartup');
 
 Meteor.startup(async () => {
-	roomCoordinator.setRoomFind('l', (_id) => LivechatRooms.findOneById(_id));
+	roomCoordinator.setRoomFind('l', async (id) => maybeMigrateLivechatRoom(await LivechatRooms.findOneById(id)));
 
 	beforeLeaveRoomCallback.add(
 		(user, room) => {

apps/meteor/tests/end-to-end/api/livechat/contacts.ts

@@ -710,7 +710,6 @@ describe('LIVECHAT - contacts', () => {
 	describe('[GET] omnichannel/contacts.get', () => {
 		let contactId: string;
 		let contactId2: string;
-		let association: ILivechatContactVisitorAssociation;
 
 		const email = faker.internet.email().toLowerCase();
 		const phone = faker.phone.number();
@@ -743,14 +742,7 @@ describe('LIVECHAT - contacts', () => {
 
 			const visitor = await createVisitor(undefined, contact.name, email, phone);
 
-			const room = await createLivechatRoom(visitor.token);
-			association = {
-				visitorId: visitor._id,
-				source: {
-					type: room.source.type,
-					id: room.source.id,
-				},
-			};
+			await createLivechatRoom(visitor.token);
 		});
 
 		after(async () => {
@@ -774,23 +766,6 @@ describe('LIVECHAT - contacts', () => {
 			expect(res.body.contact.contactManager).to.be.equal(contact.contactManager);
 		});
 
-		it('should be able get a contact by visitor association', async () => {
-			const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials).query({ visitor: association });
-
-			expect(res.status).to.be.equal(200);
-			expect(res.body).to.have.property('success', true);
-			expect(res.body.contact).to.have.property('createdAt');
-			expect(res.body.contact._id).to.be.equal(contactId);
-			expect(res.body.contact.name).to.be.equal(contact.name);
-			expect(res.body.contact.emails).to.be.deep.equal([
-				{
-					address: contact.emails[0],
-				},
-			]);
-			expect(res.body.contact.phones).to.be.deep.equal([{ phoneNumber: contact.phones[0] }]);
-			expect(res.body.contact.contactManager).to.be.equal(contact.contactManager);
-		});
-
 		it('should return 404 if contact does not exist using contactId', async () => {
 			const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials).query({ contactId: 'invalid' });
 
@@ -799,28 +774,6 @@ describe('LIVECHAT - contacts', () => {
 			expect(res.body).to.have.property('error', 'Resource not found');
 		});
 
-		it('should return 404 if contact does not exist using visitor association', async () => {
-			const res = await request
-				.get(api(`omnichannel/contacts.get`))
-				.set(credentials)
-				.query({ visitor: { ...association, visitorId: 'invalidId' } });
-
-			expect(res.status).to.be.equal(404);
-			expect(res.body).to.have.property('success', false);
-			expect(res.body).to.have.property('error', 'Resource not found');
-		});
-
-		it('should return 404 if contact does not exist using visitor source', async () => {
-			const res = await request
-				.get(api(`omnichannel/contacts.get`))
-				.set(credentials)
-				.query({ visitor: { ...association, source: { type: 'email' } } });
-
-			expect(res.status).to.be.equal(404);
-			expect(res.body).to.have.property('success', false);
-			expect(res.body).to.have.property('error', 'Resource not found');
-		});
-
 		it("should return an error if user doesn't have 'view-livechat-contact' permission", async () => {
 			await removePermissionFromAllRoles('view-livechat-contact');
 
@@ -835,21 +788,7 @@ describe('LIVECHAT - contacts', () => {
 		it('should return an error if contactId and visitor association is missing', async () => {
 			const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials);
 
-			expectInvalidParams(res, [
-				"must have required property 'contactId'",
-				"must have required property 'visitor'",
-				'must match exactly one schema in oneOf [invalid-params]',
-			]);
-		});
-
-		it('should return an error if more than one field is provided', async () => {
-			const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials).query({ contactId, visitor: association });
-
-			expectInvalidParams(res, [
-				'must NOT have additional properties',
-				'must NOT have additional properties',
-				'must match exactly one schema in oneOf [invalid-params]',
-			]);
+			expectInvalidParams(res, ["must have required property 'contactId' [invalid-params]"]);
 		});
 
 		describe('Contact Channels', () => {
@@ -1258,7 +1197,7 @@ describe('LIVECHAT - contacts', () => {
 			expect(res.status).to.be.equal(200);
 			expect(res.body).to.have.property('success', true);
 
-			const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ visitor: association });
+			const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ contactId: room.contactId });
 
 			expect(body.contact.channels).to.be.an('array');
 			expect(body.contact.channels.length).to.be.equal(1);
@@ -1388,7 +1327,7 @@ describe('LIVECHAT - contacts', () => {
 		it('should be able to unblock a contact channel', async () => {
 			await request.post(api('omnichannel/contacts.block')).set(credentials).send({ visitor: association });
 
-			const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ visitor: association });
+			const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ contactId: room.contactId });
 
 			expect(body.contact.channels).to.be.an('array');
 			expect(body.contact.channels.length).to.be.equal(1);
@@ -1399,7 +1338,7 @@ describe('LIVECHAT - contacts', () => {
 			expect(res.status).to.be.equal(200);
 			expect(res.body).to.have.property('success', true);
 
-			const { body: body2 } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ visitor: association });
+			const { body: body2 } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ contactId: room.contactId });
 
 			expect(body2.contact.channels).to.be.an('array');
 			expect(body2.contact.channels.length).to.be.equal(1);

packages/rest-typings/src/v1/omnichannel.ts

@@ -1362,28 +1362,16 @@ export const ContactVisitorAssociationSchema = {
 };
 
 const GETOmnichannelContactsSchema = {
-	oneOf: [
-		{
-			type: 'object',
-			properties: {
-				contactId: {
-					type: 'string',
-					nullable: false,
-					isNotEmpty: true,
-				},
-			},
-			required: ['contactId'],
-			additionalProperties: false,
-		},
-		{
-			type: 'object',
-			properties: {
-				visitor: ContactVisitorAssociationSchema,
-			},
-			required: ['visitor'],
-			additionalProperties: false,
+	type: 'object',
+	properties: {
+		contactId: {
+			type: 'string',
+			nullable: false,
+			isNotEmpty: true,
 		},
-	],
+	},
+	required: ['contactId'],
+	additionalProperties: false,
 };
 
 export const isGETOmnichannelContactsProps = ajv.compile<GETOmnichannelContactsProps>(GETOmnichannelContactsSchema);