Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Unset Visitor Phone on Empty String for Test Widget #1142

Closed
wants to merge 5 commits into from
Closed

[FIX] Unset Visitor Phone on Empty String for Test Widget #1142

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4c9ff0b
Unset Visitor Phone on Empty String for Test Widget
AlexanderKanakis Apr 19, 2022
c59fd77
created unsetGuest phone function and changed condition to unset phone
AlexanderKanakis May 5, 2022
081b124
fixed linting
AlexanderKanakis May 5, 2022
c54835d
minor code change
AlexanderKanakis May 9, 2022
1cbc15f
update condition for setting visitor phone number
AlexanderKanakis May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/livechat/server/lib/Livechat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,15 @@ export const Livechat = {
const updateUser = {
$set: {
token,
...(phone?.number ? { phone: [{ phoneNumber: phone.number }] } : {}),
...(phone?.number && phone.number !== 'delete' ? { phone: [{ phoneNumber: phone.number }] } : {}),
Copy link

@ear-dev ear-dev May 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're still looking for 'delete' here? didn't you change this to 'null'? so should this be phone.number !== undefined? Or, do you even need this condition here?

...(name ? { name } : {}),
},
};

if (phone === undefined) {
updateUser.$unset = { phone: [] };
}

Comment on lines +315 to +318
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexanderKanakis @ear-dev I'm still seeing the same behavior. In the DB, the phone number is not stored.
Screenshot 2022-05-12 at 1 36 26 PM

When I uncommented this code. It is being stored in the DB.

Screenshot 2022-05-12 at 1 38 22 PM

And it needs to be stored in DB. Because... For Twilio integration, we use this phone number to send messages.

https://github.com/WideChat/Rocket.Chat/blob/develop_pwa/app/livechat/server/sendMessageBySMS.js#L63

@AlexanderKanakis I think the reason for this issue is because registerGuest function is used for registering users as well as updating users. So it can be called multiple times. And when registering/updating for some other property, phoneNumber becomes undefined and that unsets it in db

Copy link

@ear-dev ear-dev May 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps then using 'delete', the way @AlexanderKanakis originally designed it is the best solution? It is surely only in our test widget that 'delete' would be set for phone number. @Shailesh351 is that our best design do you think?

if (email) {
email = email.trim();
validateEmail(email);
Expand Down
5 changes: 5 additions & 0 deletions packages/rocketchat-livechat/assets/rocket-livechat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,10 @@
callHook('setGuestPhone', phone);
};

var unsetGuestPhone = function() {
callHook('unsetGuestPhone');
};

var setGuestEmail = function(email) {
callHook('setGuestEmail', email);
};
Expand Down Expand Up @@ -771,6 +775,7 @@
clearDepartment: clearDepartment,
setGuestToken: setGuestToken,
setGuestPhone: setGuestPhone,
unsetGuestPhone: unsetGuestPhone,
setGuestName: setGuestName,
setGuestEmail: setGuestEmail,
registerGuest: registerGuest,
Expand Down