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.

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

Conversation

AlexanderKanakis
Copy link
Collaborator

Proposed changes (including videos or screenshots)

For Issue: #1139

Updates the visitor entry by unsetting the phone field id the Test Widget phone field is empty.

Issue(s)

Steps to test or reproduce

Further comments

@Shailesh351
Copy link

@AlexanderKanakis I was testing it locally. Currently, it is clearing phoneNumber from the DB every time. You can check the collection rocketchat_livechat_visitor

@ear-dev
Copy link

ear-dev commented Apr 21, 2022

ISSUE: This might clear the phoneNumber from a whatsApp user as well. This will be a problem. This feature needs somehow to be specific to only testWidget users, from a browser, using our test widget.

This implementation is currently running for every user, and it needs to only run for the specific widget user in play.

@ear-dev
Copy link

ear-dev commented Apr 21, 2022

New Design!!! Back to the original design...... create an unsetPhone function and call it from the widget in the case that no phoneNumber has been provided.

@@ -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

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?

Comment on lines +315 to +318
if (phone === undefined) {
updateUser.$unset = { phone: [] };
}

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

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?

@ear-dev ear-dev closed this May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Livechat] Refactor setGuestPhone to remove entry upon empty string
3 participants