From a0ee702dc6d12765a5d66c4b0a8aac6aea31b81f Mon Sep 17 00:00:00 2001 From: cauefcr Date: Fri, 10 Mar 2023 15:34:09 -0300 Subject: [PATCH 1/8] [FIX] fixes broken error messages on room.saveInfo --- .../meteor/app/livechat/server/api/v1/room.ts | 15 +++- .../tests/end-to-end/api/livechat/00-rooms.ts | 71 ++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/livechat/server/api/v1/room.ts b/apps/meteor/app/livechat/server/api/v1/room.ts index ee6524493b94..78d932e16dd1 100644 --- a/apps/meteor/app/livechat/server/api/v1/room.ts +++ b/apps/meteor/app/livechat/server/api/v1/room.ts @@ -432,7 +432,20 @@ API.v1.addRoute( delete guestData.phone; } - await Promise.allSettled([Livechat.saveGuest(guestData, this.userId), Livechat.saveRoomInfo(roomData)]); + const pms = (await Promise.allSettled([Livechat.saveGuest(guestData, this.userId), Livechat.saveRoomInfo(roomData)])) as { + status: string; + reason: { + isClientSafe: boolean; + error: string; + message: string; + errorType: string; + }; + }[]; + const filtered = pms.filter((pm) => pm.status === 'rejected'); + + if (filtered.length > 0) { + return API.v1.failure(filtered[0].reason.error); + } callbacks.run('livechat.saveInfo', LivechatRooms.findOneById(roomData._id), { user: this.user, diff --git a/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts b/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts index 7a1b6c8c6363..9d6d8e49f959 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts @@ -9,7 +9,7 @@ import { LivechatPriorityWeight } from '@rocket.chat/core-typings'; import type { Response } from 'supertest'; import faker from '@faker-js/faker'; -import { getCredentials, api, request, credentials } from '../../../data/api-data'; +import { getCredentials, api, request, credentials, methodCall } from '../../../data/api-data'; import { createVisitor, createLivechatRoom, @@ -1486,6 +1486,75 @@ describe('LIVECHAT - rooms', function () { .expect('Content-Type', 'application/json') .expect(400); }); + (IS_EE ? it : it.skip)('should throw an error if a valid custom field fails the check', async () => { + await request + .post(methodCall('livechat:saveCustomField')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'livechat:saveCustomField', + params: [ + null, + { + field: 'intfield', + label: 'intfield', + scope: 'room', + visibility: 'visible', + regexp: '\\d+', + searchable: true, + type: 'input', + required: false, + defaultValue: '0', + options: '', + public: false, + }, + ], + id: 'id', + msg: 'method', + }), + }) + .expect(200); + const newVisitor = await createVisitor(); + const newRoom = await createLivechatRoom(newVisitor.token); + + const response = await request + .post(api('livechat/room.saveInfo')) + .set(credentials) + .send({ + roomData: { + _id: newRoom._id, + livechatData: { intfield: 'asdasd' }, + }, + guestData: { + _id: newVisitor._id, + }, + }) + .expect('Content-Type', 'application/json') + .expect(400); + expect(response.body).to.have.property('success', false); + expect(response.body).to.have.property('error', 'Invalid value for intfield field'); + }); + (IS_EE ? it : it.skip)('should not throw an error if a valid custom field passes the check', async () => { + const newVisitor = await createVisitor(); + const newRoom = await createLivechatRoom(newVisitor.token); + + const response2 = await request + .post(api('livechat/room.saveInfo')) + .set(credentials) + .send({ + roomData: { + _id: newRoom._id, + livechatData: { intfield: '1' }, + }, + guestData: { + _id: newVisitor._id, + }, + }) + .expect('Content-Type', 'application/json') + .expect(200); + expect(response2.body).to.have.property('success', true); + }); + (IS_EE ? it : it.skip)('should update room priority', async () => { await addPermissions({ 'save-others-livechat-room-info': ['admin', 'livechat-manager'], From 00fc07c3b0f572fa3ac972286da654590c5e943f Mon Sep 17 00:00:00 2001 From: murtaza98 Date: Mon, 3 Apr 2023 19:21:11 +0530 Subject: [PATCH 2/8] missing validations on save contact API --- .../app/livechat/server/lib/Contacts.ts | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/Contacts.ts b/apps/meteor/app/livechat/server/lib/Contacts.ts index 9ca0f2472610..1c04d05fd5d1 100644 --- a/apps/meteor/app/livechat/server/lib/Contacts.ts +++ b/apps/meteor/app/livechat/server/lib/Contacts.ts @@ -3,6 +3,9 @@ import { Meteor } from 'meteor/meteor'; import type { MatchKeysAndValues, OnlyFieldsOfType } from 'mongodb'; import { LivechatVisitors, Users, LivechatRooms, LivechatCustomField, LivechatInquiry, Rooms, Subscriptions } from '@rocket.chat/models'; import type { ILivechatCustomField, ILivechatVisitor, IOmnichannelRoom } from '@rocket.chat/core-typings'; +import { TAPi18n } from 'meteor/rocketchat:tap-i18n'; + +import { trim } from '../../../../lib/utils/stringUtils'; type RegisterContactProps = { _id?: string; @@ -68,16 +71,37 @@ export const Contacts = { } } - const allowedCF = await LivechatCustomField.findByScope>('visitor', { projection: { _id: 1 } }) - .map(({ _id }) => _id) - .toArray(); + const allowedCF = LivechatCustomField.findByScope>('visitor', { + projection: { _id: 1, label: 1, regexp: 1, required: 1 }, + }); + + const livechatData: Record = {}; - const livechatData = Object.keys(customFields) - .filter((key) => allowedCF.includes(key) && customFields[key] !== '' && customFields[key] !== undefined) - .reduce((obj: Record, key) => { - obj[key] = customFields[key]; - return obj; - }, {}); + for await (const cf of allowedCF) { + if (!customFields.hasOwnProperty(cf._id)) { + if (cf.required) { + throw new Error(TAPi18n.__('error-required-custom-field-value', { field: cf.label })); + } + continue; + } + const cfValue: string | undefined = trim(customFields[cf._id] || ''); + + if (!cfValue || typeof cfValue !== 'string') { + if (cf.required) { + throw new Error(TAPi18n.__('error-req1-custom-field-value', { field: cf.label })); + } + continue; + } + + if (cf.regexp) { + const regex = new RegExp(cf.regexp); + if (!regex.test(cfValue)) { + throw new Error(TAPi18n.__('error-invalid-custom-field-value', { field: cf.label })); + } + } + + livechatData[cf._id] = cfValue; + } const updateUser: { $set: MatchKeysAndValues; $unset?: OnlyFieldsOfType } = { $set: { From bef9d07549b816cbba5f9e23d659344660525030 Mon Sep 17 00:00:00 2001 From: murtaza98 Date: Mon, 3 Apr 2023 19:21:55 +0530 Subject: [PATCH 3/8] unify error messages --- apps/meteor/app/livechat/server/lib/Contacts.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/Contacts.ts b/apps/meteor/app/livechat/server/lib/Contacts.ts index 1c04d05fd5d1..1978f3d310d6 100644 --- a/apps/meteor/app/livechat/server/lib/Contacts.ts +++ b/apps/meteor/app/livechat/server/lib/Contacts.ts @@ -80,7 +80,7 @@ export const Contacts = { for await (const cf of allowedCF) { if (!customFields.hasOwnProperty(cf._id)) { if (cf.required) { - throw new Error(TAPi18n.__('error-required-custom-field-value', { field: cf.label })); + throw new Error(TAPi18n.__('error-invalid-custom-field-value', { field: cf.label })); } continue; } @@ -88,7 +88,7 @@ export const Contacts = { if (!cfValue || typeof cfValue !== 'string') { if (cf.required) { - throw new Error(TAPi18n.__('error-req1-custom-field-value', { field: cf.label })); + throw new Error(TAPi18n.__('error-invalid-custom-field-value', { field: cf.label })); } continue; } From 35b4b59c621e64ee8a56889ffeaa9b3ed532544e Mon Sep 17 00:00:00 2001 From: murtaza98 Date: Fri, 19 May 2023 18:16:32 +0530 Subject: [PATCH 4/8] remove old i18n usage --- apps/meteor/app/livechat/server/lib/Contacts.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/Contacts.ts b/apps/meteor/app/livechat/server/lib/Contacts.ts index 1978f3d310d6..6921ee7deaea 100644 --- a/apps/meteor/app/livechat/server/lib/Contacts.ts +++ b/apps/meteor/app/livechat/server/lib/Contacts.ts @@ -3,9 +3,9 @@ import { Meteor } from 'meteor/meteor'; import type { MatchKeysAndValues, OnlyFieldsOfType } from 'mongodb'; import { LivechatVisitors, Users, LivechatRooms, LivechatCustomField, LivechatInquiry, Rooms, Subscriptions } from '@rocket.chat/models'; import type { ILivechatCustomField, ILivechatVisitor, IOmnichannelRoom } from '@rocket.chat/core-typings'; -import { TAPi18n } from 'meteor/rocketchat:tap-i18n'; import { trim } from '../../../../lib/utils/stringUtils'; +import { i18n } from '../../../utils/lib/i18n'; type RegisterContactProps = { _id?: string; @@ -80,7 +80,7 @@ export const Contacts = { for await (const cf of allowedCF) { if (!customFields.hasOwnProperty(cf._id)) { if (cf.required) { - throw new Error(TAPi18n.__('error-invalid-custom-field-value', { field: cf.label })); + throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label })); } continue; } @@ -88,7 +88,7 @@ export const Contacts = { if (!cfValue || typeof cfValue !== 'string') { if (cf.required) { - throw new Error(TAPi18n.__('error-invalid-custom-field-value', { field: cf.label })); + throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label })); } continue; } @@ -96,7 +96,7 @@ export const Contacts = { if (cf.regexp) { const regex = new RegExp(cf.regexp); if (!regex.test(cfValue)) { - throw new Error(TAPi18n.__('error-invalid-custom-field-value', { field: cf.label })); + throw new Error(i18n.t('error-invalid-custom-field-value', { field: cf.label })); } } From f619cba65de1b3d53469973269cfce08cbb5d615 Mon Sep 17 00:00:00 2001 From: Murtaza Patrawala <34130764+murtaza98@users.noreply.github.com> Date: Fri, 19 May 2023 18:18:11 +0530 Subject: [PATCH 5/8] Create blue-cougars-wave.md --- .changeset/blue-cougars-wave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/blue-cougars-wave.md diff --git a/.changeset/blue-cougars-wave.md b/.changeset/blue-cougars-wave.md new file mode 100644 index 000000000000..6fa1408f9155 --- /dev/null +++ b/.changeset/blue-cougars-wave.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +fix: broken error messages on room.saveInfo & missing CF validations on omni/contact api From 4d09e88a4d8741826e0535ff31fe6e7fd817b7bc Mon Sep 17 00:00:00 2001 From: Murtaza Patrawala <34130764+murtaza98@users.noreply.github.com> Date: Thu, 25 May 2023 17:53:32 +0530 Subject: [PATCH 6/8] Create chilled-owls-pretend.md --- .changeset/chilled-owls-pretend.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/chilled-owls-pretend.md diff --git a/.changeset/chilled-owls-pretend.md b/.changeset/chilled-owls-pretend.md new file mode 100644 index 000000000000..6fa1408f9155 --- /dev/null +++ b/.changeset/chilled-owls-pretend.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +fix: broken error messages on room.saveInfo & missing CF validations on omni/contact api From 7d62b5f825f67db49b7d4f28b43dc8eae4307444 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 25 May 2023 13:45:23 -0600 Subject: [PATCH 7/8] CR suggestions --- .changeset/chilled-owls-pretend.md | 5 ----- apps/meteor/app/livechat/server/api/v1/room.ts | 15 +-------------- apps/meteor/app/livechat/server/lib/Contacts.ts | 2 +- 3 files changed, 2 insertions(+), 20 deletions(-) delete mode 100644 .changeset/chilled-owls-pretend.md diff --git a/.changeset/chilled-owls-pretend.md b/.changeset/chilled-owls-pretend.md deleted file mode 100644 index 6fa1408f9155..000000000000 --- a/.changeset/chilled-owls-pretend.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@rocket.chat/meteor": patch ---- - -fix: broken error messages on room.saveInfo & missing CF validations on omni/contact api diff --git a/apps/meteor/app/livechat/server/api/v1/room.ts b/apps/meteor/app/livechat/server/api/v1/room.ts index 21b898172aa3..af0307c5871c 100644 --- a/apps/meteor/app/livechat/server/api/v1/room.ts +++ b/apps/meteor/app/livechat/server/api/v1/room.ts @@ -448,20 +448,7 @@ API.v1.addRoute( delete guestData.phone; } - const pms = (await Promise.allSettled([Livechat.saveGuest(guestData, this.userId), Livechat.saveRoomInfo(roomData)])) as { - status: string; - reason: { - isClientSafe: boolean; - error: string; - message: string; - errorType: string; - }; - }[]; - const filtered = pms.filter((pm) => pm.status === 'rejected'); - - if (filtered.length > 0) { - return API.v1.failure(filtered[0].reason.error); - } + await Promise.all([Livechat.saveGuest(guestData, this.userId), Livechat.saveRoomInfo(roomData)]); await callbacks.run('livechat.saveInfo', await LivechatRooms.findOneById(roomData._id), { user: this.user, diff --git a/apps/meteor/app/livechat/server/lib/Contacts.ts b/apps/meteor/app/livechat/server/lib/Contacts.ts index 6921ee7deaea..448e486761e6 100644 --- a/apps/meteor/app/livechat/server/lib/Contacts.ts +++ b/apps/meteor/app/livechat/server/lib/Contacts.ts @@ -84,7 +84,7 @@ export const Contacts = { } continue; } - const cfValue: string | undefined = trim(customFields[cf._id] || ''); + const cfValue: string = trim(customFields[cf._id]); if (!cfValue || typeof cfValue !== 'string') { if (cf.required) { From ecc985834c324eb3459db0d6f3abc4b9bece0d4f Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 25 May 2023 14:49:18 -0600 Subject: [PATCH 8/8] use right property --- apps/meteor/app/livechat/server/api/v1/room.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/meteor/app/livechat/server/api/v1/room.ts b/apps/meteor/app/livechat/server/api/v1/room.ts index af0307c5871c..aa3c125f6b43 100644 --- a/apps/meteor/app/livechat/server/api/v1/room.ts +++ b/apps/meteor/app/livechat/server/api/v1/room.ts @@ -448,7 +448,13 @@ API.v1.addRoute( delete guestData.phone; } - await Promise.all([Livechat.saveGuest(guestData, this.userId), Livechat.saveRoomInfo(roomData)]); + // We want this both operations to be concurrent, so we have to go with Promise.allSettled + const result = await Promise.allSettled([Livechat.saveGuest(guestData, this.userId), Livechat.saveRoomInfo(roomData)]); + + const firstError = result.find((item) => item.status === 'rejected'); + if (firstError) { + throw new Error((firstError as PromiseRejectedResult).reason.error); + } await callbacks.run('livechat.saveInfo', await LivechatRooms.findOneById(roomData._id), { user: this.user,