From 9e9b2c86e2142d8e7c871d453e6b578107b4374f Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:07:21 -0300 Subject: [PATCH] regression: Do not throw an error on custom fields validation when migrating visitors to contacts (#34030) --- .../server/lib/contacts/ContactMerger.ts | 5 ----- .../lib/contacts/mapVisitorToContact.spec.ts | 11 +++++++++- .../lib/contacts/mapVisitorToContact.ts | 9 +++++++- .../lib/contacts/validateCustomFields.spec.ts | 21 +++++++++++++++++++ .../lib/contacts/validateCustomFields.ts | 15 +++++++++---- 5 files changed, 50 insertions(+), 11 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/contacts/ContactMerger.ts b/apps/meteor/app/livechat/server/lib/contacts/ContactMerger.ts index a4bd703f1473..48b6dcc41f9a 100644 --- 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 index bd7b52c4d428..3f4d74430280 100644 --- 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, IOmnichannelSource, CreateContactPara { _id: 'visitor1', username: 'Username', + livechatData: { + customFieldId: 'customFieldValue', + invalidCustomFieldId: 'invalidCustomFieldValue', + }, activity: [], }, { @@ -166,7 +172,9 @@ const dataMap: [Partial, 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 index e1cbaf9d40a6..a35d41e9beca 100644 --- 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 { 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 index 0dcd478176db..39684a62fd91 100644 --- 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 index e389d1b34ac9..4efede65b266 100644 --- a/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts +++ b/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts @@ -6,13 +6,16 @@ import { i18n } from '../../../../utils/lib/i18n'; export function validateCustomFields( allowedCustomFields: AtLeast[], customFields: Record, - options?: { ignoreAdditionalFields?: boolean }, + { + ignoreAdditionalFields = false, + ignoreValidationErrors = false, + }: { ignoreAdditionalFields?: boolean; ignoreValidationErrors?: boolean } = {}, ): Record { const validValues: Record = {}; 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; @@ -20,7 +23,7 @@ export function validateCustomFields( 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,6 +32,10 @@ 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 })); } } @@ -36,7 +43,7 @@ export function validateCustomFields( 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)) {