fix: Old custom fields from migrated contacts are not ignored on update

.changeset/wet-chicken-scream.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixes contact update failing in case a custom field is removed from the workspace

apps/meteor/app/livechat/server/lib/contacts/updateContact.ts

@@ -24,8 +24,8 @@ export type UpdateContactParams = {
 export async function updateContact(params: UpdateContactParams): Promise<ILivechatContact> {
 	const { contactId, name, emails, phones, customFields: receivedCustomFields, contactManager, channels, wipeConflicts } = params;
 
-	const contact = await LivechatContacts.findOneById<Pick<ILivechatContact, '_id' | 'name'>>(contactId, {
-		projection: { _id: 1, name: 1 },
+	const contact = await LivechatContacts.findOneById<Pick<ILivechatContact, '_id' | 'name' | 'customFields'>>(contactId, {
+		projection: { _id: 1, name: 1, customFields: 1 },
 	});
 
 	if (!contact) {
@@ -36,15 +36,35 @@ export async function updateContact(params: UpdateContactParams): Promise<ILivec
 		await validateContactManager(contactManager);
 	}
 
-	const customFields = receivedCustomFields && validateCustomFields(await getAllowedCustomFields(), receivedCustomFields);
+	const workspaceAllowedCustomFields = await getAllowedCustomFields();
+	const workspaceAllowedCustomFieldsIds = workspaceAllowedCustomFields.map((customField) => customField._id);
+	const currentCustomFieldsIds = Object.keys(contact.customFields || {});
+	const notRegisteredCustomFields = currentCustomFieldsIds
+		.filter((customFieldId) => !workspaceAllowedCustomFieldsIds.includes(customFieldId))
+		.map((customFieldId) => ({ _id: customFieldId }));
+
+	const customFieldsToUpdate =
+		receivedCustomFields &&
+		validateCustomFields(workspaceAllowedCustomFields, receivedCustomFields, {
+			ignoreAdditionalFields: !!notRegisteredCustomFields.length,
+		});
+
+	if (receivedCustomFields && customFieldsToUpdate && notRegisteredCustomFields.length) {
+		const allowedCustomFields = [...workspaceAllowedCustomFields, ...notRegisteredCustomFields];
+		validateCustomFields(allowedCustomFields, receivedCustomFields);
+
+		notRegisteredCustomFields.forEach((notRegisteredCustomField) => {
+			customFieldsToUpdate[notRegisteredCustomField._id] = contact.customFields?.[notRegisteredCustomField._id] as string;
+		});
+	}
 
 	const updatedContact = await LivechatContacts.updateContact(contactId, {
 		name,
 		emails: emails?.map((address) => ({ address })),
 		phones: phones?.map((phoneNumber) => ({ phoneNumber })),
 		contactManager,
 		channels,
-		customFields,
+		customFields: customFieldsToUpdate,
 		...(wipeConflicts && { conflictingFields: [] }),
 	});
 

apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts

@@ -4,7 +4,7 @@ import { trim } from '../../../../../lib/utils/stringUtils';
 import { i18n } from '../../../../utils/lib/i18n';
 
 export function validateCustomFields(
-	allowedCustomFields: AtLeast<ILivechatCustomField, '_id' | 'label' | 'regexp' | 'required'>[],
+	allowedCustomFields: AtLeast<ILivechatCustomField, '_id'>[],
 	customFields: Record<string, string | unknown>,
 	{
 		ignoreAdditionalFields = false,
@@ -16,15 +16,15 @@ export function validateCustomFields(
 	for (const cf of allowedCustomFields) {
 		if (!customFields.hasOwnProperty(cf._id)) {
 			if (cf.required && !ignoreValidationErrors) {
-				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label }));
+				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label || cf._id }));
 			}
 			continue;
 		}
 		const cfValue: string = trim(customFields[cf._id]);
 
 		if (!cfValue || typeof cfValue !== 'string') {
 			if (cf.required && !ignoreValidationErrors) {
-				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label }));
+				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label || cf._id }));
 			}
 			continue;
 		}
@@ -36,7 +36,7 @@ export function validateCustomFields(
 					continue;
 				}
 
-				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label }));
+				throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label || cf._id }));
 			}
 		}
 

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

@@ -142,18 +142,37 @@ describe('LIVECHAT - contacts', () => {
 		});
 
 		describe('Custom Fields', () => {
+			let contactId: string;
 			before(async () => {
-				await createCustomField({
-					field: 'cf1',
-					label: 'Custom Field 1',
-					scope: 'visitor',
+				const defaultProps = {
+					scope: 'visitor' as const,
 					visibility: 'public',
 					type: 'input',
-					required: true,
 					regexp: '^[0-9]+$',
 					searchable: true,
 					public: true,
-				});
+				};
+
+				await Promise.all([
+					createCustomField({
+						...defaultProps,
+						field: 'cf1',
+						label: 'Custom Field 1',
+						required: true,
+					}),
+					createCustomField({
+						...defaultProps,
+						field: 'cf2',
+						label: 'Custom Field 2',
+						required: false,
+					}),
+					createCustomField({
+						...defaultProps,
+						field: 'cfOptional',
+						label: 'Optional Custom Field',
+						required: false,
+					}),
+				]);
 			});
 
 			after(async () => {
@@ -211,6 +230,98 @@ describe('LIVECHAT - contacts', () => {
 				expect(res.body).to.have.property('error');
 				expect(res.body.error).to.be.equal('Invalid value for Custom Field 1 field');
 			});
+
+			it('should keep a legacy custom field, but not update it, nor throw an error if it is specified on update', async () => {
+				const createRes = await request
+					.post(api('omnichannel/contacts'))
+					.set(credentials)
+					.send({
+						name: faker.person.fullName(),
+						emails: [faker.internet.email().toLowerCase()],
+						phones: [faker.phone.number()],
+						customFields: {
+							cf1: '123',
+							cf2: '456',
+						},
+					});
+				expect(createRes.body).to.have.property('success', true);
+				expect(createRes.body).to.have.property('contactId').that.is.a('string');
+				contactId = createRes.body.contactId;
+
+				await deleteCustomField('cf2');
+
+				const updateRes = await request
+					.post(api('omnichannel/contacts.update'))
+					.set(credentials)
+					.send({
+						contactId,
+						customFields: {
+							cf1: '456',
+							cf2: '789',
+						},
+					});
+				expect(updateRes.body).to.have.property('success', true);
+				expect(updateRes.body).to.have.property('contact').that.is.an('object');
+				expect(updateRes.body.contact).to.have.property('_id', contactId);
+				expect(updateRes.body.contact).to.have.property('customFields').that.is.an('object');
+				expect(updateRes.body.contact.customFields).to.have.property('cf1', '456');
+				expect(updateRes.body.contact.customFields).to.have.property('cf2', '456');
+			});
+
+			it('should keep a legacy custom field and not throw an error if it is not specified on update', async () => {
+				const updateRes = await request
+					.post(api('omnichannel/contacts.update'))
+					.set(credentials)
+					.send({
+						contactId,
+						customFields: {
+							cf1: '789',
+							cfOptional: '567',
+						},
+					});
+				expect(updateRes.body).to.have.property('success', true);
+				expect(updateRes.body).to.have.property('contact').that.is.an('object');
+				expect(updateRes.body.contact).to.have.property('_id', contactId);
+				expect(updateRes.body.contact).to.have.property('customFields').that.is.an('object');
+				expect(updateRes.body.contact.customFields).to.have.property('cf1', '789');
+				expect(updateRes.body.contact.customFields).to.have.property('cfOptional', '567');
+				expect(updateRes.body.contact.customFields).to.have.property('cf2', '456');
+			});
+
+			it('should keep a legacy custom field, but remove an optional registered custom field if it is not specified on update', async () => {
+				const updateRes = await request
+					.post(api('omnichannel/contacts.update'))
+					.set(credentials)
+					.send({
+						contactId,
+						customFields: {
+							cf1: '789',
+						},
+					});
+				expect(updateRes.body).to.have.property('success', true);
+				expect(updateRes.body).to.have.property('contact').that.is.an('object');
+				expect(updateRes.body.contact).to.have.property('_id', contactId);
+				expect(updateRes.body.contact).to.have.property('customFields').that.is.an('object');
+				expect(updateRes.body.contact.customFields).to.have.property('cf1', '789');
+				expect(updateRes.body.contact.customFields).to.have.property('cf2', '456');
+				expect(updateRes.body.contact.customFields).to.not.have.property('cfOptional');
+			});
+
+			it('should throw an error if trying to update a custom field that is not registered in the workspace and does not exist in the contact', async () => {
+				const updateRes = await request
+					.post(api('omnichannel/contacts.update'))
+					.set(credentials)
+					.send({
+						contactId,
+						customFields: {
+							cf1: '123',
+							cf3: 'invalid',
+						},
+					});
+				expect(updateRes.body).to.have.property('success', false);
+				expect(updateRes.body).to.have.property('error');
+				expect(updateRes.body.error).to.be.equal('Custom field cf3 is not allowed');
+			});
 		});
 
 		describe('Fields Validation', () => {