[NEW] Add GUI for customFields in Omnichannel conversations by antkaz · Pull Request #15840 · RocketChat/Rocket.Chat

antkaz · 2019-11-21T16:35:18Z

Closes RocketChat/feature-requests#151

renatobecker-zz · 2019-11-21T17:28:21Z

Hi @antkaz!
Thanks for your effort in implementing it.
It's, for sure, a very nice feature, I'll review this PR ASAP!

Cheers!

antkaz · 2019-11-21T17:34:33Z

@renatobecker Thank you! 😊

app/livechat/client/views/app/tabbar/visitorEdit.js

renatobecker-zz · 2019-12-06T19:22:57Z

app/livechat/client/views/app/tabbar/visitorEdit.html

+						<label class="rc-input__label">
+							<div class="rc-input__title">{{label}}</div>
+							<div class="rc-input__wrapper">
+								<input class="rc-input__element" type="text" name="{{name}}" autocomplete="off" data-visitorLivechatData="true" value="{{value}}">


Let's create a new permission to allow/deny editing custom field values, what do you think?
By default, The new permission would be assigned to the livechat-agent, livechat-managers and admin.

Do I need to add these permissions for the REST API?

Do I need to add these permissions for the REST API?

No, It's not necessary.
Since we only allow fetching data through the REST API, we don't need to change the REST API permissions.

I meant the endpoint for updating values POST livechat/custom.field (and livechat/custom.fields). If we restrict access to editing values of custom fields for GUI, the user can still change the values through these API methods.
I think that if we restrict the permissions to edit values of custom fields in the interface, then these restrictions should be for the API. What do you think?

I meant the endpoint for updating values POST livechat/custom.field (and livechat/custom.fields). If we restrict access to editing values of custom fields for GUI, the user can still change the values through these API methods.
I think that if we restrict the permissions to edit values of custom fields in the interface, then these restrictions should be for the API. What do you think?

I see.. The fact is that the Livechat Widget calls those endpoits too, so we can't add permission restrictions there because there is no authenticated user on the Widget-side, you know?
So, let's just skip the idea of implementing new permissions to deal with custom fields, okay?

I did not know... Thank you. Ok, then I won't add new permissions

Perfect!
If you're gonna add the validation option, I'll wait for your new commit to get back to the review process, okay?

Yes, I will add and let you know

app/livechat/client/views/app/tabbar/visitorInfo.html

renatobecker-zz · 2019-12-18T17:39:16Z

app/livechat/client/views/app/tabbar/visitorInfo.js

+		const fields = [];
+		let livechatData = {};
+		const user = Template.instance().user.get();
+		if (user) {


Suggested change

if (user) {

const { livechatData = {} } = user || {};

renatobecker-zz · 2019-12-18T18:02:46Z

app/livechat/client/views/app/tabbar/visitorInfo.js

+			livechatData = _.extend(livechatData, user.livechatData);
+		}
+
+		if (!_.isEmpty(livechatData)) {


Looking at the complexity of your algorithm, I'd say you can write it better.
For instance:

... customRoomFields() { const customFields = Template.instance().customFields.get(); if (!customFields || customFields.length === 0) { return []; } const fields = []; const room = Template.instance().room.get(); const { livechatData = {} } = room || {}; ...

of course! Thank you)

renatobecker-zz · 2019-12-18T18:03:07Z

app/livechat/client/views/app/tabbar/visitorInfo.js

+		}
+
+		if (!_.isEmpty(livechatData)) {
+			for (const _id in livechatData) {


And Here, why not use Object.keys instead of these if's ?
You can do something like this:

Object.keys(livechatData).forEach((key) => { const field = _.findWhere(customFields, { key }); ... });

renatobecker-zz · 2019-12-18T18:15:31Z

app/livechat/server/lib/Livechat.js

 	},

-	saveGuest({ _id, name, email, phone }) {
+	saveGuest({ _id, name, email, phone, livechatData }) {


Suggested change

saveGuest({ _id, name, email, phone, livechatData }) {

saveGuest({ _id, name, email, phone, livechatData = {} }) {

renatobecker-zz · 2019-12-18T18:15:50Z

app/livechat/server/lib/Livechat.js

 		if (phone) {
 			updateData.phone = phone;
 		}
+		if (livechatData) {


You don't need to check this here..

renatobecker-zz · 2019-12-18T18:16:49Z

app/livechat/server/lib/Livechat.js

 			updateData.phone = phone;
 		}
+		if (livechatData) {
+			updateData.livechatData = {};


Suggested change

updateData.livechatData = {};

const customFields = {};

fields.forEach((field) => {

if (!livechatData.hasOwnProperty(field._id)) {

return;

}

const value = s.trim(livechatData[field._id]);

if (value === '') {

return;

}

if (field.regexp !== undefined && field.regexp !== '') {

const regexp = new RegExp(field.regexp);

if (!regexp.test(value)) {

throw new Meteor.Error(TAPi18n.__('error-invalid-custom-field-value', { field: field.label }));

}

}

customFields[field._id] = value;

});

Object.assign(updateData, customFields);

app/models/server/models/LivechatRooms.js

lgtm-com · 2019-12-20T10:03:07Z

This pull request introduces 2 alerts when merging 6590db5 into 18ba691 - view on LGTM.com

new alerts:

2 for Unused variable, import, function or class

renatobecker · 2020-01-07T19:42:19Z

Hi @antkaz!

Did you push the last requested changes here?
I mean, is the PR ready for a final review?

Thanks.

antkaz · 2020-01-09T05:21:37Z

Hi @antkaz!

Did you push the last requested changes here?
I mean, is the PR ready for a final review?

Thanks.

Hi @renatobecker! I rebased branch to last develop and pushed changes. You can review PR. Thanks!

renatobecker

Hi @antkaz,

Your PR looks good, I just found a few things to be fixed.
After that, I'll merge it, looking forward to your next commit. =D

Thanks.

renatobecker · 2020-01-10T14:39:13Z

app/livechat/client/views/app/tabbar/visitorEdit.js

 import { hasRole } from '../../../../../authorization';
 import './visitorEdit.html';
 import { APIClient } from '../../../../../utils/client';
+import { LivechatCustomField } from '../../../collections/LivechatCustomField';


We have been deprecating all of the Meteor pub/sub, we're replacing it by the REST API approach, so It would be better to apply the same approach here, okay?

Here you can see an example of the REST API approach working:
https://github.com/RocketChat/Rocket.Chat/blob/develop/app/livechat/client/views/app/livechatCustomFields.js#L75

renatobecker · 2020-01-10T14:40:57Z

app/livechat/client/views/app/tabbar/visitorEdit.js

+	this.subscribe('livechat:customFields');
 	this.autorun(async () => {
 		const { room } = await APIClient.v1.get(`rooms.info?roomId=${ rid }`);
+		const customFields = LivechatCustomField.find().fetch();


As I mentioned above, we need to replace the implementation based on collection by the REST API approach.

renatobecker · 2020-01-10T14:57:29Z

app/livechat/server/lib/Livechat.js

+				return;
+			}
+			const value = s.trim(livechatData[field._id]);
+			if (value === '') {


You can't apply this validation, otherwise, the Agent won't be able to clear/remove a field value, right?

renatobecker · 2020-01-10T14:57:44Z

app/livechat/server/lib/Livechat.js

+				return;
+			}
+			const value = s.trim(livechatData[field._id]);
+			if (value === '') {


You can't apply this validation, otherwise, the Agent won't be able to clear/remove a field value, right?

Hi @antkaz,

Your PR looks good, I just found a few things to be fixed.
After that, I'll merge it, looking forward to your next commit. =D

Thanks.

Hello. I fixed your remarks =)

User can't finish their review.

renatobecker · 2020-01-10T16:56:59Z

Thanks, @antkaz for your effort on this improvement.

renatobecker-zz added feat: livechat type: improvement labels Nov 21, 2019

renatobecker-zz self-requested a review November 21, 2019 17:22

renatobecker-zz added this to the 3.0.0 milestone Dec 2, 2019

renatobecker-zz suggested changes Dec 6, 2019

View reviewed changes

renatobecker-zz previously requested changes Dec 18, 2019

View reviewed changes

renatobecker-zz self-assigned this Dec 19, 2019

antkaz requested a review from renatobecker-zz December 20, 2019 09:53

renatobecker-zz modified the milestones: 2.4.0, 3.0.0 Dec 23, 2019

renatobecker requested review from renatobecker and removed request for renatobecker-zz January 7, 2020 19:25

renatobecker assigned renatobecker and unassigned renatobecker-zz Jan 7, 2020

renatobecker requested changes Jan 10, 2020

View reviewed changes

antkaz added 7 commits January 10, 2020 20:27

Add GUI for customFields in Livechat

5f051d2

strong for Department in VisitorInfo

cfb3c89

feat(livechat): add validation for custom fields

8709da5

chore: fix review

b92916b

fix: set new custom fields values for livechat

e2d11de

perf: replace getCustomFields by REST

293f7d3

fix: remove empty custom fields

9e7705a

renatobecker changed the title ~~[NEW] Add GUI for customFields in Livechat~~ [NEW] Add GUI for customFields in Omnichannel conversations Jan 10, 2020

renatobecker approved these changes Jan 10, 2020

View reviewed changes

renatobecker requested a review from renatobecker-zz January 10, 2020 16:41

renatobecker removed the request for review from renatobecker-zz January 10, 2020 16:42

renatobecker merged commit bfdb8b9 into RocketChat:develop Jan 10, 2020

sampaiodiego mentioned this pull request Feb 15, 2020

Release 3.0.0 #16608

Merged

tassoevan added subj: livechat squad: omnichannel and removed feat: livechat labels Oct 26, 2022

	saveGuest({ _id, name, email, phone, livechatData }) {
	saveGuest({ _id, name, email, phone, livechatData = {} }) {

-			updateData.livechatData = {};
+			const customFields = {};
+			fields.forEach((field) => {
+				if (!livechatData.hasOwnProperty(field._id)) {
+					return;
+				}
+				const value = s.trim(livechatData[field._id]);
+				if (value === '') {
+					return;
+				}
+				if (field.regexp !== undefined && field.regexp !== '') {
+					const regexp = new RegExp(field.regexp);
+					if (!regexp.test(value)) {
+						throw new Meteor.Error(TAPi18n.__('error-invalid-custom-field-value', { field: field.label }));
+					}
+				}
+				customFields[field._id] = value;
+			});
+			Object.assign(updateData, customFields);

Conversation

antkaz commented Nov 21, 2019

Uh oh!

renatobecker-zz commented Nov 21, 2019

Uh oh!

antkaz commented Nov 21, 2019

Uh oh!

Uh oh!

renatobecker-zz Dec 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lgtm-com bot commented Dec 20, 2019

Uh oh!

renatobecker commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antkaz commented Jan 9, 2020

Uh oh!

renatobecker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renatobecker commented Jan 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

renatobecker-zz Dec 6, 2019 •

edited

Loading

renatobecker commented Jan 7, 2020 •

edited

Loading