Skip to content

Commit 459f635

Browse files
KevLehmanKevLehman
authored
fix: Contact custom fields not being updated properly (#36345)
1 parent 9826bc2 commit 459f635

File tree

9 files changed

+563
-122
lines changed

9 files changed

+563
-122
lines changed

‎.changeset/mean-roses-shout.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
"@rocket.chat/model-typings": patch
4+
"@rocket.chat/models": patch
5+
---
6+
7+
Fixes an issue that prevented all custom fields from being saved when multiple updates were issued on a single call

‎apps/meteor/app/livechat/server/api/v1/customField.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ API.v1.addRoute(
3535
throw new Error('invalid-token');
3636
}
3737

38+
// TODO: do on one shot instead of multiple calls
3839
const fields = await Promise.all(
3940
this.bodyParams.customFields.map(
4041
async (customField: {

‎apps/meteor/app/livechat/server/api/v1/visitor.ts‎

Lines changed: 49 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -82,66 +82,59 @@ API.v1.addRoute(
8282
),
8383
);
8484

85-
if (customFields && Array.isArray(customFields) && customFields.length > 0) {
86-
const errors: string[] = [];
87-
const keys = customFields.map((field) => field.key);
88-
89-
const livechatCustomFields = await LivechatCustomField.findByScope(
90-
'visitor',
91-
{ projection: { _id: 1, required: 1 } },
92-
false,
93-
).toArray();
94-
validateRequiredCustomFields(keys, livechatCustomFields);
95-
96-
const matchingCustomFields = livechatCustomFields.filter((field: ILivechatCustomField) => keys.includes(field._id));
97-
const processedKeys = await Promise.all(
98-
matchingCustomFields.map(async (field: ILivechatCustomField) => {
99-
const customField = customFields.find((f) => f.key === field._id);
100-
if (!customField) {
101-
return;
102-
}
103-
104-
const { key, value, overwrite } = customField;
105-
// TODO: Change this to Bulk update
106-
if (!(await VisitorsRaw.updateLivechatDataByToken(token, key, value, overwrite))) {
107-
errors.push(key);
108-
}
109-
110-
// TODO deduplicate this code and the one at the function setCustomFields (apps/meteor/app/livechat/server/lib/custom-fields.ts)
111-
const contacts = await LivechatContacts.findAllByVisitorId(visitor._id).toArray();
112-
if (contacts.length > 0) {
113-
await Promise.all(contacts.map((contact) => updateContactsCustomFields(contact, key, value, overwrite)));
114-
}
115-
116-
return key;
117-
}),
118-
);
119-
120-
if (processedKeys.length !== keys.length) {
121-
livechatLogger.warn({
122-
msg: 'Some custom fields were not processed',
123-
visitorId: visitor._id,
124-
missingKeys: keys.filter((key) => !processedKeys.includes(key)),
125-
});
126-
}
127-
128-
if (errors.length > 0) {
129-
livechatLogger.error({
130-
msg: 'Error updating custom fields',
131-
visitorId: visitor._id,
132-
errors,
133-
});
134-
throw new Error('error-updating-custom-fields');
135-
}
136-
137-
return API.v1.success({ visitor: await VisitorsRaw.findOneEnabledById(visitor._id) });
85+
if (!Array.isArray(customFields) || !customFields.length) {
86+
return API.v1.success({ visitor });
13887
}
13988

140-
if (!visitor) {
141-
throw new Meteor.Error('error-saving-visitor', 'An error ocurred while saving visitor');
89+
const keys = customFields.map((field) => field.key);
90+
91+
const livechatCustomFields = await LivechatCustomField.findByScope(
92+
'visitor',
93+
{ projection: { _id: 1, required: 1 } },
94+
false,
95+
).toArray();
96+
validateRequiredCustomFields(keys, livechatCustomFields);
97+
98+
const matchingCustomFields = livechatCustomFields.filter((field: ILivechatCustomField) => keys.includes(field._id));
99+
const validCustomFields = customFields.filter((cf) => matchingCustomFields.find((mcf) => cf.key === mcf._id));
100+
if (!validCustomFields.length) {
101+
return API.v1.success({ visitor });
102+
}
103+
104+
const visitorCustomFieldsToUpdate = validCustomFields.reduce(
105+
(prev, curr) => {
106+
if (curr.overwrite) {
107+
prev[`livechatData.${curr.key}`] = curr.value;
108+
return prev;
109+
}
110+
111+
if (!visitor?.livechatData?.[curr.key]) {
112+
prev[`livechatData.${curr.key}`] = curr.value;
113+
}
114+
115+
return prev;
116+
},
117+
{} as Record<string, string>,
118+
);
119+
120+
if (Object.keys(visitorCustomFieldsToUpdate).length) {
121+
await VisitorsRaw.updateAllLivechatDataByToken(visitor.token, visitorCustomFieldsToUpdate);
122+
}
123+
124+
const contacts = await LivechatContacts.findAllByVisitorId(visitor._id).toArray();
125+
if (contacts.length) {
126+
await Promise.all(contacts.map((contact) => updateContactsCustomFields(contact, validCustomFields)));
127+
}
128+
129+
if (validCustomFields.length !== keys.length) {
130+
livechatLogger.warn({
131+
msg: 'Some custom fields were not processed',
132+
visitorId: visitor._id,
133+
missingKeys: keys.filter((key) => !validCustomFields.map((v) => v.key).includes(key)),
134+
});
142135
}
143136

144-
return API.v1.success({ visitor });
137+
return API.v1.success({ visitor: await VisitorsRaw.findOneEnabledById(visitor._id) });
145138
},
146139
},
147140
);

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

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,33 @@ export const validateRequiredCustomFields = (customFields: string[], livechatCus
1919
}
2020
};
2121

22-
export async function updateContactsCustomFields(contact: ILivechatContact, key: string, value: string, overwrite: boolean): Promise<void> {
23-
const shouldUpdateCustomFields = overwrite || !contact.customFields || !contact.customFields[key];
22+
export async function updateContactsCustomFields(
23+
contact: ILivechatContact,
24+
validCustomFields: {
25+
key: string;
26+
value: string;
27+
overwrite: boolean;
28+
}[],
29+
): Promise<void> {
30+
const contactCustomFieldsToUpdate = validCustomFields.reduce(
31+
(prev, curr) => {
32+
if (curr.overwrite || !contact?.customFields?.[curr.key]) {
33+
prev[`customFields.${curr.key}`] = curr.value;
34+
return prev;
35+
}
36+
prev.conflictingFields ??= contact.conflictingFields || [];
37+
prev.conflictingFields.push({ field: `customFields.${curr.key}`, value: curr.value });
38+
return prev;
39+
},
40+
{} as Record<string, any>,
41+
);
2442

25-
if (shouldUpdateCustomFields) {
26-
contact.customFields ??= {};
27-
contact.customFields[key] = value;
28-
} else {
29-
contact.conflictingFields ??= [];
30-
contact.conflictingFields.push({ field: `customFields.${key}`, value });
43+
if (!Object.keys(contactCustomFieldsToUpdate).length) {
44+
return;
3145
}
3246

33-
await LivechatContacts.updateContactCustomFields(contact._id, {
34-
...(shouldUpdateCustomFields && { customFields: contact.customFields }),
35-
...(contact.conflictingFields && { conflictingFields: contact.conflictingFields }),
36-
});
37-
38-
livechatLogger.debug({ msg: `Contact ${contact._id} updated with custom fields` });
47+
livechatLogger.debug({ msg: 'Updating custom fields for contact', contactId: contact._id, contactCustomFieldsToUpdate });
48+
await LivechatContacts.updateById(contact._id, { $set: contactCustomFieldsToUpdate });
3949
}
4050

4151
export async function setCustomFields({
@@ -73,7 +83,7 @@ export async function setCustomFields({
7383
if (visitor) {
7484
const contacts = await LivechatContacts.findAllByVisitorId(visitor._id).toArray();
7585
if (contacts.length > 0) {
76-
await Promise.all(contacts.map((contact) => updateContactsCustomFields(contact, key, value, overwrite)));
86+
await Promise.all(contacts.map((contact) => updateContactsCustomFields(contact, [{ key, value, overwrite }])));
7787
}
7888
}
7989
}

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

Lines changed: 164 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import type { ILivechatVisitor } from '@rocket.chat/core-typings';
1+
import type { ILivechatVisitor, IOmnichannelRoom } from '@rocket.chat/core-typings';
22
import { expect } from 'chai';
33
import { after, before, describe, it } from 'mocha';
44
import type { Response } from 'supertest';
55

66
import { getCredentials, api, request, credentials } from '../../../data/api-data';
77
import { createCustomField, deleteCustomField } from '../../../data/livechat/custom-fields';
8-
import { createVisitor, deleteVisitor } from '../../../data/livechat/rooms';
8+
import { closeOmnichannelRoom, createLivechatRoom, createVisitor, deleteVisitor } from '../../../data/livechat/rooms';
99
import { updatePermission, updateSetting } from '../../../data/permissions.helper';
1010

1111
describe('LIVECHAT - custom fields', () => {
@@ -118,6 +118,52 @@ describe('LIVECHAT - custom fields', () => {
118118
});
119119

120120
describe('livechat/custom.fields', () => {
121+
const customFieldName = `new_custom_field_${Date.now()}_1`;
122+
const customFieldName2 = `new_custom_field_${Date.now()}_2`;
123+
const customFieldName3 = `new_custom_field_${Date.now()}_3`;
124+
let visitor: ILivechatVisitor;
125+
let visitorRoom: IOmnichannelRoom;
126+
127+
before(async () => {
128+
await createCustomField({
129+
searchable: true,
130+
field: customFieldName,
131+
label: customFieldName,
132+
defaultValue: 'test_default_address',
133+
scope: 'visitor',
134+
visibility: 'public',
135+
regexp: '',
136+
});
137+
await createCustomField({
138+
searchable: true,
139+
field: customFieldName2,
140+
label: customFieldName2,
141+
defaultValue: 'test_default_address',
142+
scope: 'visitor',
143+
visibility: 'public',
144+
regexp: '',
145+
});
146+
await createCustomField({
147+
searchable: true,
148+
field: customFieldName3,
149+
label: customFieldName3,
150+
defaultValue: 'test_default_address',
151+
scope: 'visitor',
152+
visibility: 'public',
153+
regexp: '',
154+
});
155+
visitor = await createVisitor();
156+
// start a room for visitor2
157+
visitorRoom = await createLivechatRoom(visitor.token);
158+
});
159+
after(async () => {
160+
await Promise.all([
161+
deleteCustomField(customFieldName),
162+
deleteCustomField(customFieldName2),
163+
deleteCustomField(customFieldName3),
164+
closeOmnichannelRoom(visitorRoom._id),
165+
]);
166+
});
121167
it('should fail when token is not on body params', async () => {
122168
await request.post(api('livechat/custom.fields')).expect(400);
123169
});
@@ -163,16 +209,6 @@ describe('LIVECHAT - custom fields', () => {
163209
});
164210
it('should save a custom field on visitor', async () => {
165211
const visitor = await createVisitor();
166-
const customFieldName = `new_custom_field_${Date.now()}`;
167-
await createCustomField({
168-
searchable: true,
169-
field: customFieldName,
170-
label: customFieldName,
171-
defaultValue: 'test_default_address',
172-
scope: 'visitor',
173-
visibility: 'public',
174-
regexp: '',
175-
});
176212

177213
const { body } = await request
178214
.post(api('livechat/custom.fields'))
@@ -188,6 +224,122 @@ describe('LIVECHAT - custom fields', () => {
188224
expect(body.fields).to.have.lengthOf(1);
189225
expect(body.fields[0]).to.have.property('value', 'test_address');
190226
});
227+
it('should save multiple custom fields on a visitor', async () => {
228+
const visitor = await createVisitor();
229+
230+
const { body } = await request
231+
.post(api('livechat/custom.fields'))
232+
.send({
233+
token: visitor.token,
234+
customFields: [
235+
{ key: customFieldName, value: 'test_address', overwrite: true },
236+
{ key: customFieldName2, value: 'test_address2', overwrite: true },
237+
{ key: customFieldName3, value: 'test_address3', overwrite: true },
238+
],
239+
})
240+
.expect(200);
241+
242+
expect(body).to.have.property('success', true);
243+
expect(body).to.have.property('fields');
244+
expect(body.fields).to.be.an('array');
245+
expect(body.fields).to.have.lengthOf(3);
246+
expect(body.fields[0]).to.have.property('value', 'test_address');
247+
expect(body.fields[1]).to.have.property('value', 'test_address2');
248+
expect(body.fields[2]).to.have.property('value', 'test_address3');
249+
});
250+
it('should save multiple custom fields on contact when visitor already has custom fields and an update with multiple fields is issued', async () => {
251+
const { body } = await request
252+
.post(api('livechat/custom.fields'))
253+
.send({
254+
token: visitor.token,
255+
customFields: [{ key: customFieldName, value: 'test_address', overwrite: true }],
256+
})
257+
.expect(200);
258+
259+
expect(body).to.have.property('success', true);
260+
expect(body).to.have.property('fields');
261+
expect(body.fields).to.be.an('array');
262+
expect(body.fields).to.have.lengthOf(1);
263+
expect(body.fields[0]).to.have.property('value', 'test_address');
264+
265+
await request
266+
.post(api('livechat/custom.fields'))
267+
.send({
268+
token: visitor.token,
269+
customFields: [
270+
{ key: customFieldName2, value: 'test_address2', overwrite: true },
271+
{ key: customFieldName3, value: 'test_address3', overwrite: true },
272+
],
273+
})
274+
.expect(200);
275+
276+
await request
277+
.get(api(`omnichannel/contacts.get`))
278+
.set(credentials)
279+
.query({ contactId: visitorRoom.contactId })
280+
.expect(200)
281+
.expect((res) => {
282+
expect(res.body).to.have.property('contact');
283+
expect(res.body.contact).to.have.property('customFields');
284+
expect(res.body.contact.customFields).to.have.property(customFieldName, 'test_address');
285+
expect(res.body.contact.customFields).to.have.property(customFieldName2, 'test_address2');
286+
expect(res.body.contact.customFields).to.have.property(customFieldName3, 'test_address3');
287+
});
288+
});
289+
it('should mark a conflict on a contact custom fields when overwrite is true and visitor already has the custom field set', async () => {
290+
await request
291+
.post(api('livechat/custom.fields'))
292+
.send({
293+
token: visitor.token,
294+
customFields: [{ key: customFieldName, value: 'test_address_conflict', overwrite: false }],
295+
})
296+
.expect(200);
297+
298+
await request
299+
.get(api(`omnichannel/contacts.get`))
300+
.set(credentials)
301+
.query({ contactId: visitorRoom.contactId })
302+
.expect(200)
303+
.expect((res) => {
304+
expect(res.body).to.have.property('contact');
305+
expect(res.body.contact).to.have.property('customFields');
306+
expect(res.body.contact.customFields).to.have.property(customFieldName, 'test_address');
307+
expect(res.body.contact.customFields).to.have.property(customFieldName2, 'test_address2');
308+
expect(res.body.contact.customFields).to.have.property(customFieldName3, 'test_address3');
309+
expect(res.body.contact).to.have.property('conflictingFields').that.is.an('array');
310+
expect(res.body.contact.conflictingFields[0]).to.deep.equal({
311+
field: `customFields.${customFieldName}`,
312+
value: 'test_address_conflict',
313+
});
314+
});
315+
});
316+
it('should overwrite the contact custom field when overwrite is true', async () => {
317+
await request
318+
.post(api('livechat/custom.fields'))
319+
.send({
320+
token: visitor.token,
321+
customFields: [{ key: customFieldName2, value: 'test_new_add', overwrite: true }],
322+
})
323+
.expect(200);
324+
325+
await request
326+
.get(api(`omnichannel/contacts.get`))
327+
.set(credentials)
328+
.query({ contactId: visitorRoom.contactId })
329+
.expect(200)
330+
.expect((res) => {
331+
expect(res.body).to.have.property('contact');
332+
expect(res.body.contact).to.have.property('customFields');
333+
expect(res.body.contact.customFields).to.have.property(customFieldName, 'test_address');
334+
expect(res.body.contact.customFields).to.have.property(customFieldName2, 'test_new_add');
335+
expect(res.body.contact.customFields).to.have.property(customFieldName3, 'test_address3');
336+
expect(res.body.contact).to.have.property('conflictingFields').that.is.an('array');
337+
expect(res.body.contact.conflictingFields[0]).to.deep.equal({
338+
field: `customFields.${customFieldName}`,
339+
value: 'test_address_conflict',
340+
});
341+
});
342+
});
191343
});
192344

193345
describe('livechat/custom.field [with Contacts]', () => {

0 commit comments

Comments
 (0)
⚡