fix: `contact.conflictingFields` being updated with nullish values (#36097)

Co-authored-by: Kevin Aleman <kaleman960@gmail.com>

.changeset/four-parents-hide.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixes `contact.conflictingFields` being updated with nullish values.

apps/meteor/app/livechat/server/lib/custom-fields.ts

@@ -20,7 +20,9 @@ export const validateRequiredCustomFields = (customFields: string[], livechatCus
 };
 
 export async function updateContactsCustomFields(contact: ILivechatContact, key: string, value: string, overwrite: boolean): Promise<void> {
-	if (overwrite || !contact.customFields || !contact.customFields[key]) {
+	const shouldUpdateCustomFields = overwrite || !contact.customFields || !contact.customFields[key];
+
+	if (shouldUpdateCustomFields) {
 		contact.customFields ??= {};
 		contact.customFields[key] = value;
 	} else {
@@ -28,7 +30,10 @@ export async function updateContactsCustomFields(contact: ILivechatContact, key:
 		contact.conflictingFields.push({ field: `customFields.${key}`, value });
 	}
 
-	await LivechatContacts.updateContact(contact._id, { customFields: contact.customFields, conflictingFields: contact.conflictingFields });
+	await LivechatContacts.updateContact(contact._id, {
+		...(shouldUpdateCustomFields && { customFields: contact.customFields }),
+		...(contact.conflictingFields && { conflictingFields: contact.conflictingFields }),
+	});
 
 	livechatLogger.debug({ msg: `Contact ${contact._id} updated with custom fields` });
 }

apps/meteor/tests/end-to-end/api/livechat/03-custom-fields.ts

@@ -1,11 +1,11 @@
 import type { ILivechatVisitor } from '@rocket.chat/core-typings';
 import { expect } from 'chai';
-import { before, describe, it } from 'mocha';
+import { after, before, describe, it } from 'mocha';
 import type { Response } from 'supertest';
 
 import { getCredentials, api, request, credentials } from '../../../data/api-data';
-import { createCustomField } from '../../../data/livechat/custom-fields';
-import { createVisitor } from '../../../data/livechat/rooms';
+import { createCustomField, deleteCustomField } from '../../../data/livechat/custom-fields';
+import { createVisitor, deleteVisitor } from '../../../data/livechat/rooms';
 import { updatePermission, updateSetting } from '../../../data/permissions.helper';
 
 describe('LIVECHAT - custom fields', () => {
@@ -245,6 +245,16 @@ describe('LIVECHAT - custom fields', () => {
 			});
 		});
 
+		after(async () => {
+			// TODO: add clean up for contacts, visitors, etc
+			await Promise.all([
+				deleteCustomField(customFieldName),
+				deleteCustomField(`${customFieldName}_2`),
+				deleteCustomField(`${customFieldName}_3`),
+				deleteVisitor(visitor.token),
+			]);
+		});
+
 		it('should save the custom field on Contact when available', async () => {
 			// Save the custom field on Visitor/Contact
 			await request
@@ -415,5 +425,111 @@ describe('LIVECHAT - custom fields', () => {
 					expect(res.body.contact.conflictingFields[0]).to.not.have.property('field', `customFields.${customFieldName}_3`);
 				});
 		});
+
+		it('should not save contact conflictingFields as nullish if not modified', async () => {
+			// Create a Visitor
+			const visitor2 = await createVisitor();
+			let contactId2: string | undefined;
+
+			// Create a Contact and store id on var
+			await request
+				.post(api('omnichannel/contacts'))
+				.set(credentials)
+				.send({
+					name: visitor2.name,
+					emails: [visitor2.visitorEmails?.[0].address],
+					phones: [visitor2.phone?.[0].phoneNumber],
+				})
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('contactId');
+
+					contactId2 = res.body.contactId;
+				});
+
+			await request.get(api('livechat/room')).query({ token: visitor2.token });
+
+			// Save the custom field on Contact
+			await request
+				.post(api('livechat/custom.field'))
+				.send({ token: visitor2.token, key: `${customFieldName}`, value: customFieldValue, overwrite: true })
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+				});
+
+			// Fetch the visitor's contact to validate custom fields are properly set.
+			await request
+				.get(api(`omnichannel/contacts.get`))
+				.set(credentials)
+				.query({ contactId: contactId2 })
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('contact');
+					expect(res.body.contact).to.have.property('customFields');
+					expect(res.body.contact.customFields).to.have.property(`${customFieldName}`, customFieldValue);
+
+					// Validate conflictingFields was not saved as null
+					expect(res.body.contact).to.not.have.property('conflictingFields');
+				});
+
+			await deleteVisitor(visitor2.token);
+		});
+
+		it('should not save contact conflictingFields as nullish if not modified (through visitor update)', async () => {
+			// Create a Visitor
+			const visitor2 = await createVisitor();
+			let contactId2: string | undefined;
+
+			// Create a Contact and store id on var
+			await request
+				.post(api('omnichannel/contacts'))
+				.set(credentials)
+				.send({
+					name: visitor2.name,
+					emails: [visitor2.visitorEmails?.[0].address],
+					phones: [visitor2.phone?.[0].phoneNumber],
+				})
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('contactId');
+
+					contactId2 = res.body.contactId;
+				});
+
+			await request.get(api('livechat/room')).query({ token: visitor2.token });
+
+			// Save the custom field on Contact
+			await request
+				.post(api('livechat/visitor'))
+				.set(credentials)
+				.send({
+					visitor: {
+						token: visitor2.token,
+						customFields: [{ key: `${customFieldName}`, value: customFieldValue, overwrite: true }],
+					},
+				});
+
+			// Fetch the visitor's contact to validate custom fields are properly set.
+			await request
+				.get(api(`omnichannel/contacts.get`))
+				.set(credentials)
+				.query({ contactId: contactId2 })
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('contact');
+					expect(res.body.contact).to.have.property('customFields');
+					expect(res.body.contact.customFields).to.have.property(`${customFieldName}`, customFieldValue);
+
+					// Validate conflictingFields was not saved as null
+					expect(res.body.contact).to.not.have.property('conflictingFields');
+				});
+
+			await deleteVisitor(visitor2.token);
+		});
 	});
 });

apps/meteor/tests/unit/app/livechat/server/lib/custom-fields.spec.ts

@@ -0,0 +1,61 @@
+import type { ILivechatContact } from '@rocket.chat/core-typings';
+import { expect } from 'chai';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+
+const modelsMock = {
+	LivechatContacts: {
+		updateContact: sinon.stub(),
+	},
+};
+
+const { updateContactsCustomFields } = proxyquire.noCallThru().load('../../../../../../app/livechat/server/lib/custom-fields.ts', {
+	'@rocket.chat/models': modelsMock,
+});
+
+describe('[Custom Fields] updateContactsCustomFields', () => {
+	beforeEach(() => {
+		modelsMock.LivechatContacts.updateContact.reset();
+	});
+
+	it('should not add conflictingFields to the update data when its nullish', async () => {
+		const contact: Partial<ILivechatContact> = {
+			_id: 'contactId',
+			customFields: {
+				customField: 'value',
+			},
+		};
+
+		modelsMock.LivechatContacts.updateContact.resolves({ ...contact, customFields: { customField: 'newValue' } });
+
+		await updateContactsCustomFields(contact, 'customField', 'newValue', true);
+
+		expect(modelsMock.LivechatContacts.updateContact.calledOnce).to.be.true;
+		expect(modelsMock.LivechatContacts.updateContact.getCall(0).args[0]).to.be.equal('contactId');
+		expect(modelsMock.LivechatContacts.updateContact.getCall(0).args[1]).to.deep.equal({
+			customFields: { customField: 'newValue' },
+		});
+	});
+
+	it('should add conflictingFields to the update data only when it is modified', async () => {
+		const contact: Partial<ILivechatContact> = {
+			_id: 'contactId',
+			customFields: {
+				customField: 'value',
+			},
+		};
+
+		modelsMock.LivechatContacts.updateContact.resolves({
+			...contact,
+			conflictingFields: [{ field: 'customFields.customField', value: 'newValue' }],
+		});
+
+		await updateContactsCustomFields(contact, 'customField', 'newValue', false);
+
+		expect(modelsMock.LivechatContacts.updateContact.calledOnce).to.be.true;
+		expect(modelsMock.LivechatContacts.updateContact.getCall(0).args[0]).to.be.equal('contactId');
+		expect(modelsMock.LivechatContacts.updateContact.getCall(0).args[1]).to.deep.equal({
+			conflictingFields: [{ field: 'customFields.customField', value: 'newValue' }],
+		});
+	});
+});