regression: Do not throw an error on custom fields validation when mi…

…grating visitors to contacts (#34030)
RocketChat · Dec 5, 2024 · 9e9b2c8 · 9e9b2c8
1 parent 745f5f5
commit 9e9b2c8
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 11 deletions.
diff --git a/apps/meteor/app/livechat/server/lib/contacts/ContactMerger.ts b/apps/meteor/app/livechat/server/lib/contacts/ContactMerger.ts
@@ -262,8 +262,6 @@ export class ContactMerger {
 			customFieldsPerName.get(customField.type)?.push(customField);
 		}
 
-		const customFieldConflicts: CustomFieldAndValue[] = [];
-
 		for (const [key, customFields] of customFieldsPerName) {
 			const fieldName = key.replace('customFields.', '');
 
@@ -274,8 +272,6 @@ export class ContactMerger {
 					dataToSet[key] = first.value;
 				}
 			}
-
-			customFieldConflicts.push(...customFields);
 		}
 
 		const allConflicts: ILivechatContactConflictingField[] =
@@ -284,7 +280,6 @@ export class ContactMerger {
 				: [
 						...newNames.map((name): ILivechatContactConflictingField => ({ field: 'name', value: name })),
 						...newManagers.map((manager): ILivechatContactConflictingField => ({ field: 'manager', value: manager as string })),
-						...customFieldConflicts.map(({ type, value }): ILivechatContactConflictingField => ({ field: type, value })),
 					];
 
 		// Phones, Emails and Channels are simply added to the contact's existing list

diff --git a/apps/meteor/app/livechat/server/lib/contacts/mapVisitorToContact.spec.ts b/apps/meteor/app/livechat/server/lib/contacts/mapVisitorToContact.spec.ts
@@ -6,11 +6,13 @@ import sinon from 'sinon';
 import type { CreateContactParams } from './createContact';
 
 const getContactManagerIdByUsername = sinon.stub();
+const getAllowedCustomFields = sinon.stub();
 
 const { mapVisitorToContact } = proxyquire.noCallThru().load('./mapVisitorToContact', {
 	'./getContactManagerIdByUsername': {
 		getContactManagerIdByUsername,
 	},
+	'./getAllowedCustomFields': { getAllowedCustomFields },
 });
 
 const testDate = new Date();
@@ -140,6 +142,10 @@ const dataMap: [Partial<ILivechatVisitor>, IOmnichannelSource, CreateContactPara
 		{
 			_id: 'visitor1',
 			username: 'Username',
+			livechatData: {
+				customFieldId: 'customFieldValue',
+				invalidCustomFieldId: 'invalidCustomFieldValue',
+			},
 			activity: [],
 		},
 		{
@@ -166,7 +172,9 @@ const dataMap: [Partial<ILivechatVisitor>, IOmnichannelSource, CreateContactPara
 					},
 				},
 			],
-			customFields: undefined,
+			customFields: {
+				customFieldId: 'customFieldValue',
+			},
 			contactManager: undefined,
 		},
 	],
@@ -182,6 +190,7 @@ describe('mapVisitorToContact', () => {
 
 			return undefined;
 		});
+		getAllowedCustomFields.resolves([{ _id: 'customFieldId', label: 'custom-field-label' }]);
 	});
 
 	const index = 0;

diff --git a/apps/meteor/app/livechat/server/lib/contacts/mapVisitorToContact.ts b/apps/meteor/app/livechat/server/lib/contacts/mapVisitorToContact.ts
@@ -1,7 +1,9 @@
 import type { ILivechatVisitor, IOmnichannelSource } from '@rocket.chat/core-typings';
 
 import type { CreateContactParams } from './createContact';
+import { getAllowedCustomFields } from './getAllowedCustomFields';
 import { getContactManagerIdByUsername } from './getContactManagerIdByUsername';
+import { validateCustomFields } from './validateCustomFields';
 
 export async function mapVisitorToContact(visitor: ILivechatVisitor, source: IOmnichannelSource): Promise<CreateContactParams> {
 	return {
@@ -25,7 +27,12 @@ export async function mapVisitorToContact(visitor: ILivechatVisitor, source: IOm
 				lastChat: visitor.lastChat,
 			},
 		],
-		customFields: visitor.livechatData,
+		customFields:
+			visitor.livechatData &&
+			validateCustomFields(await getAllowedCustomFields(), visitor.livechatData, {
+				ignoreAdditionalFields: true,
+				ignoreValidationErrors: true,
+			}),
 		lastChat: visitor.lastChat,
 		contactManager: visitor.contactManager?.username && (await getContactManagerIdByUsername(visitor.contactManager.username)),
 	};

diff --git a/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.spec.ts b/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.spec.ts
@@ -14,6 +14,12 @@ describe('validateCustomFields', () => {
 		expect(() => validateCustomFields(mockCustomFields, {})).to.throw();
 	});
 
+	it('should not throw an error if a required custom field is missing, but the ignoreValidationErrors option is provided', () => {
+		expect(() => validateCustomFields(mockCustomFields, {}, { ignoreValidationErrors: true }))
+			.not.to.throw()
+			.and.to.equal({});
+	});
+
 	it('should NOT throw an error when a non-required custom field is missing', () => {
 		const allowedCustomFields = [{ _id: 'field1', label: 'Field 1', required: false }];
 		const customFields = {};
@@ -25,6 +31,12 @@ describe('validateCustomFields', () => {
 		expect(() => validateCustomFields(mockCustomFields, { cf1: 'invalid' })).to.throw();
 	});
 
+	it('should not throw an error if a custom field value does not match the regexp, but the ignoreValidationErrors option is provided', () => {
+		expect(() => validateCustomFields(mockCustomFields, { cf1: 'invalid' }, { ignoreValidationErrors: true }))
+			.not.to.throw()
+			.and.to.equal({});
+	});
+
 	it('should handle an empty customFields input without throwing an error', () => {
 		const allowedCustomFields = [{ _id: 'field1', label: 'Field 1', required: false }];
 		const customFields = {};
@@ -38,4 +50,13 @@ describe('validateCustomFields', () => {
 
 		expect(() => validateCustomFields(allowedCustomFields, customFields)).to.throw();
 	});
+
+	it('should not throw an error if a extra custom field is passed, but the ignoreValidationErrors option is provided', () => {
+		const allowedCustomFields = [{ _id: 'field1', label: 'Field 1', required: false }];
+		const customFields = { field2: 'value' };
+
+		expect(() => validateCustomFields(allowedCustomFields, customFields, { ignoreValidationErrors: true }))
+			.not.to.throw()
+			.and.to.equal({});
+	});
 });
diff --git a/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts b/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts
@@ -6,21 +6,24 @@ import { i18n } from '../../../../utils/lib/i18n';
 export function validateCustomFields(
 	allowedCustomFields: AtLeast<ILivechatCustomField, '_id' | 'label' | 'regexp' | 'required'>[],
 	customFields: Record<string, string | unknown>,
-	options?: { ignoreAdditionalFields?: boolean },
+	{
+		ignoreAdditionalFields = false,
+		ignoreValidationErrors = false,
+	}: { ignoreAdditionalFields?: boolean; ignoreValidationErrors?: boolean } = {},
 ): Record<string, string> {
 	const validValues: Record<string, string> = {};
 
 	for (const cf of allowedCustomFields) {
 		if (!customFields.hasOwnProperty(cf._id)) {
-			if (cf.required) {
+			if (cf.required && !ignoreValidationErrors) {
 				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label }));
 			}
 			continue;
 		}
 		const cfValue: string = trim(customFields[cf._id]);
 
 		if (!cfValue || typeof cfValue !== 'string') {
-			if (cf.required) {
+			if (cf.required && !ignoreValidationErrors) {
 				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label }));
 			}
 			continue;
@@ -29,14 +32,18 @@ export function validateCustomFields(
 		if (cf.regexp) {
 			const regex = new RegExp(cf.regexp);
 			if (!regex.test(cfValue)) {
+				if (ignoreValidationErrors) {
+					continue;
+				}
+
 				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label }));
 			}
 		}
 
 		validValues[cf._id] = cfValue;
 	}
 
-	if (!options?.ignoreAdditionalFields) {
+	if (!ignoreAdditionalFields) {
 		const allowedCustomFieldIds = new Set(allowedCustomFields.map((cf) => cf._id));
 		for (const key in customFields) {
 			if (!allowedCustomFieldIds.has(key)) {