From c8b8cf8556b138f4ec735fb731ec7093312bd7ce Mon Sep 17 00:00:00 2001 From: Lucas Pelegrino Date: Fri, 30 May 2025 13:37:27 -0300 Subject: [PATCH] fix: `contact.conflictingFields` being updated with nullish values (#36097) Co-authored-by: Kevin Aleman --- .changeset/four-parents-hide.md | 5 + .../app/livechat/server/lib/custom-fields.ts | 9 +- .../api/livechat/03-custom-fields.ts | 122 +++++++++++++++++- .../livechat/server/lib/custom-fields.spec.ts | 61 +++++++++ 4 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 .changeset/four-parents-hide.md create mode 100644 apps/meteor/tests/unit/app/livechat/server/lib/custom-fields.spec.ts diff --git a/.changeset/four-parents-hide.md b/.changeset/four-parents-hide.md new file mode 100644 index 00000000000..1c6119eee9f --- /dev/null +++ b/.changeset/four-parents-hide.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixes `contact.conflictingFields` being updated with nullish values. diff --git a/apps/meteor/app/livechat/server/lib/custom-fields.ts b/apps/meteor/app/livechat/server/lib/custom-fields.ts index e63fb2e43c6..ad82ea4dd7d 100644 --- a/apps/meteor/app/livechat/server/lib/custom-fields.ts +++ b/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 { - 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` }); } diff --git a/apps/meteor/tests/end-to-end/api/livechat/03-custom-fields.ts b/apps/meteor/tests/end-to-end/api/livechat/03-custom-fields.ts index d18e1aa4974..ead1054c50f 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/03-custom-fields.ts +++ b/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); + }); }); }); diff --git a/apps/meteor/tests/unit/app/livechat/server/lib/custom-fields.spec.ts b/apps/meteor/tests/unit/app/livechat/server/lib/custom-fields.spec.ts new file mode 100644 index 00000000000..7b061f0e606 --- /dev/null +++ b/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 = { + _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 = { + _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' }], + }); + }); +});