From fa52d8e1e01ed4b85a61253806708607c64c8f50 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 6 Dec 2024 14:39:52 -0300 Subject: [PATCH 1/7] fix: Delete invalid custom fields on contact update --- .../server/lib/contacts/updateContact.ts | 24 +++++++++++++++---- .../lib/contacts/validateCustomFields.ts | 8 +++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts index 8ec929ae4b34..d7bac0627c28 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,23 @@ export async function updateContact(params: UpdateContactParams): Promise customField._id)]); + const currentCustomFieldsIds = Object.keys(contact.customFields || {}); + const notRegisteredCustomFields = currentCustomFieldsIds + .filter((customFieldId) => !workspaceAllowedCustomFieldsIds.has(customFieldId)) + .map((customFieldId) => ({ _id: customFieldId })); + + const customFieldsToUpdate = + receivedCustomFields && + validateCustomFields(workspaceAllowedCustomFields, receivedCustomFields, { + ignoreAdditionalFields: !!notRegisteredCustomFields.length, + }); + + if (receivedCustomFields && notRegisteredCustomFields.length) { + const allowedCustomFields = [...workspaceAllowedCustomFields, ...notRegisteredCustomFields]; + validateCustomFields(allowedCustomFields, receivedCustomFields); + } const updatedContact = await LivechatContacts.updateContact(contactId, { name, @@ -44,7 +60,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 })); } } From 1fbeef1fdaafaf079e962b74dd8b37a65fa2d3d7 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 6 Dec 2024 14:40:05 -0300 Subject: [PATCH 2/7] tests: Add end-to-end tests --- .../tests/end-to-end/api/livechat/contacts.ts | 67 ++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) 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..617b50ed4f37 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/contacts.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/contacts.ts @@ -29,7 +29,7 @@ import { createUser, deleteUser } from '../../../data/users.helper'; import { expectInvalidParams } from '../../../data/validation.helper'; import { IS_EE } from '../../../e2e/config/constants'; -describe('LIVECHAT - contacts', () => { +describe.only('LIVECHAT - contacts', () => { let agentUser: IUser; let livechatAgent: ILivechatAgent; before((done) => getCredentials(done)); @@ -142,6 +142,7 @@ describe('LIVECHAT - contacts', () => { }); describe('Custom Fields', () => { + let contactId: string; before(async () => { await createCustomField({ field: 'cf1', @@ -154,6 +155,17 @@ describe('LIVECHAT - contacts', () => { searchable: true, public: true, }); + await createCustomField({ + field: 'cf2', + label: 'Custom Field 2', + scope: 'visitor', + visibility: 'public', + type: 'input', + required: false, + regexp: '^[0-9]+$', + searchable: true, + public: true, + }); }); after(async () => { @@ -211,6 +223,59 @@ 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 remove an old custom field from a contact on update if it is not registered in the workspace anymore', 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.not.have.property('cf2'); + }); + + 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', () => { From ba223209bf3d6937f4068b7ebff5c5e893321db9 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 6 Dec 2024 14:55:02 -0300 Subject: [PATCH 3/7] Remove describe.only --- apps/meteor/tests/end-to-end/api/livechat/contacts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 617b50ed4f37..97474ecba85f 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/contacts.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/contacts.ts @@ -29,7 +29,7 @@ import { createUser, deleteUser } from '../../../data/users.helper'; import { expectInvalidParams } from '../../../data/validation.helper'; import { IS_EE } from '../../../e2e/config/constants'; -describe.only('LIVECHAT - contacts', () => { +describe('LIVECHAT - contacts', () => { let agentUser: IUser; let livechatAgent: ILivechatAgent; before((done) => getCredentials(done)); From f0b6330ff985bcf69ecd1966b96ef5427b893a5e Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:59:00 -0300 Subject: [PATCH 4/7] Create changeset --- .changeset/wet-chicken-scream.md | 5 +++++ 1 file changed, 5 insertions(+) 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 From 9044b7cf05ce89bf998d19d25b51ebc977fce286 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 6 Dec 2024 18:51:51 -0300 Subject: [PATCH 5/7] Keep legacy custom fields on contact update --- .../server/lib/contacts/updateContact.ts | 6 ++++- .../tests/end-to-end/api/livechat/contacts.ts | 24 ++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts index d7bac0627c28..188ee654467e 100644 --- a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts +++ b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts @@ -49,9 +49,13 @@ export async function updateContact(params: UpdateContactParams): Promise { + customFieldsToUpdate[notRegisteredCustomField._id] = contact.customFields?.[notRegisteredCustomField._id] as string; + }); } const updatedContact = await LivechatContacts.updateContact(contactId, { 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 97474ecba85f..eaaae072da48 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/contacts.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/contacts.ts @@ -29,7 +29,7 @@ import { createUser, deleteUser } from '../../../data/users.helper'; import { expectInvalidParams } from '../../../data/validation.helper'; import { IS_EE } from '../../../e2e/config/constants'; -describe('LIVECHAT - contacts', () => { +describe.only('LIVECHAT - contacts', () => { let agentUser: IUser; let livechatAgent: ILivechatAgent; before((done) => getCredentials(done)); @@ -224,7 +224,7 @@ describe('LIVECHAT - contacts', () => { expect(res.body.error).to.be.equal('Invalid value for Custom Field 1 field'); }); - it('should remove an old custom field from a contact on update if it is not registered in the workspace anymore', async () => { + 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) @@ -258,7 +258,25 @@ describe('LIVECHAT - contacts', () => { 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.not.have.property('cf2'); + 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', + }, + }); + 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'); }); 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 () => { From fac974928e59fde7bc69f6eddff234fdc12d09a4 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 6 Dec 2024 19:06:03 -0300 Subject: [PATCH 6/7] Add tests for removing an optional custom field --- .../tests/end-to-end/api/livechat/contacts.ts | 64 +++++++++++++------ 1 file changed, 46 insertions(+), 18 deletions(-) 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 eaaae072da48..fc87d32fc5da 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/contacts.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/contacts.ts @@ -29,7 +29,7 @@ import { createUser, deleteUser } from '../../../data/users.helper'; import { expectInvalidParams } from '../../../data/validation.helper'; import { IS_EE } from '../../../e2e/config/constants'; -describe.only('LIVECHAT - contacts', () => { +describe('LIVECHAT - contacts', () => { let agentUser: IUser; let livechatAgent: ILivechatAgent; before((done) => getCredentials(done)); @@ -144,28 +144,35 @@ describe.only('LIVECHAT - contacts', () => { describe('Custom Fields', () => { let contactId: string; before(async () => { - await createCustomField({ - field: 'cf1', - label: 'Custom Field 1', - scope: 'visitor', - visibility: 'public', - type: 'input', - required: true, - regexp: '^[0-9]+$', - searchable: true, - public: true, - }); - await createCustomField({ - field: 'cf2', - label: 'Custom Field 2', - scope: 'visitor', + const defaultProps = { + scope: 'visitor' as const, visibility: 'public', type: 'input', - required: false, 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 () => { @@ -262,6 +269,26 @@ describe.only('LIVECHAT - contacts', () => { }); 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) @@ -277,6 +304,7 @@ describe.only('LIVECHAT - contacts', () => { 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 () => { From 0930af578aaa657e098a4ff193f9fea6c4e165e9 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 11 Dec 2024 10:50:55 -0300 Subject: [PATCH 7/7] Replace set by array --- apps/meteor/app/livechat/server/lib/contacts/updateContact.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts index 188ee654467e..752bb62b301f 100644 --- a/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts +++ b/apps/meteor/app/livechat/server/lib/contacts/updateContact.ts @@ -37,10 +37,10 @@ export async function updateContact(params: UpdateContactParams): Promise customField._id)]); + const workspaceAllowedCustomFieldsIds = workspaceAllowedCustomFields.map((customField) => customField._id); const currentCustomFieldsIds = Object.keys(contact.customFields || {}); const notRegisteredCustomFields = currentCustomFieldsIds - .filter((customFieldId) => !workspaceAllowedCustomFieldsIds.has(customFieldId)) + .filter((customFieldId) => !workspaceAllowedCustomFieldsIds.includes(customFieldId)) .map((customFieldId) => ({ _id: customFieldId })); const customFieldsToUpdate =