From 037e692bee67d89fd7ceb9a89638591012f83bd8 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Wed, 18 Dec 2024 08:41:01 -0300 Subject: [PATCH] fix: Old custom fields from migrated contacts are not ignored on update (#34139) --- .changeset/wet-chicken-scream.md | 5 + .../server/lib/contacts/updateContact.ts | 28 +++- .../lib/contacts/validateCustomFields.ts | 8 +- .../tests/end-to-end/api/livechat/contacts.ts | 123 +++++++++++++++++- 4 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 .changeset/wet-chicken-scream.md diff --git a/.changeset/wet-chicken-scream.md b/.changeset/wet-chicken-scream.md new file mode 100644 index 000000000000..610be5e8cd44 --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts index 8ec929ae4b34..752bb62b301f 100644 --- a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts +++ b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts @@ -24,8 +24,8 @@ export type UpdateContactParams = { export async function updateContact(params: UpdateContactParams): Promise { const { contactId, name, emails, phones, customFields: receivedCustomFields, contactManager, channels, wipeConflicts } = params; - const contact = await LivechatContacts.findOneById>(contactId, { - projection: { _id: 1, name: 1 }, + const contact = await LivechatContacts.findOneById>(contactId, { + projection: { _id: 1, name: 1, customFields: 1 }, }); if (!contact) { @@ -36,7 +36,27 @@ export async function updateContact(params: UpdateContactParams): Promise 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, @@ -44,7 +64,7 @@ export async function updateContact(params: UpdateContactParams): Promise ({ phoneNumber })), contactManager, channels, - customFields, + customFields: customFieldsToUpdate, ...(wipeConflicts && { conflictingFields: [] }), }); diff --git a/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts b/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts index 4efede65b266..3ab981cf2513 100644 --- a/apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts +++ b/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[], + allowedCustomFields: AtLeast[], customFields: Record, { ignoreAdditionalFields = false, @@ -16,7 +16,7 @@ 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; } @@ -24,7 +24,7 @@ export function validateCustomFields( 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 })); } } diff --git a/apps/meteor/tests/end-to-end/api/livechat/contacts.ts b/apps/meteor/tests/end-to-end/api/livechat/contacts.ts index daaffb1f9eb3..fc87d32fc5da 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/contacts.ts +++ b/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', () => {