Skip to content

Commit

Permalink
fix: Old custom fields from migrated contacts are not ignored on upda…
Browse files Browse the repository at this point in the history 
…te (#34139)
  • Loading branch information
@matheusbsilva137
matheusbsilva137 authored Dec 18, 2024
1 parent 75a14b2 commit 037e692
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/wet-chicken-scream.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fixes contact update failing in case a custom field is removed from the workspace
28 changes: 24 additions & 4 deletions apps/meteor/app/livechat/server/lib/contacts/updateContact.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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: [] }),
});

Expand Down
8 changes: 4 additions & 4 deletions apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
Expand All @@ -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 }));
}
}

Expand Down
123 changes: 117 additions & 6 deletions apps/meteor/tests/end-to-end/api/livechat/contacts.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 037e692

Please sign in to comment.