Inviting users with phone number displays weird message on phone

kavimuru commented 1 year ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Go to staging dot on web chrome and login with User A
Create a new workspace and go to invite members
Invite a user that has phone number (it's compulsory), and on the message field , remove the current message and add your own message within quotation marks " " and also add signs like & , backtik ` and <
Check the message that may have arrived on your phone number that you invited before
Notice that quotation marks is displayed as &quot and looks weird in the message. Also the backtick, & and < are also shown weirdly
Expected Result:

Message while Inviting users with phone number doesn't display properly on the message that has arrived on phone

Actual Result:

Message while Inviting users with phone numbers should be displayed properly

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.25-3 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/43996225/f94c7644-c038-4e87-bb2a-f90ade3f34c7

Expensify/Expensify Issue URL: Issue reported by: @priya-zha Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1685941205156389

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0108b7537c7a8d0d92
Upwork Job ID: 1669098003016310784
Last Price Increase: 2023-06-14

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

[ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
[ ] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
[ ] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

dukenv0307 commented 1 year ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Inviting users with phone number displays weird message on phone

What is the root cause of that problem?

When call AddMembersToWorkspace API, we pass welcomeNote parameter in payload as the escape of the correct message to enable them to appear in the invite email. But it can't convert to character in SMS https://github.com/Expensify/App/blob/79f18eca86c35fca8055e7036ca2d449ad34f85a/src/libs/actions/Policy.js#L384

What changes do you think we should make in order to solve the problem?

We could pass welcomeNote parameter in payload as the correct invite message. And then in BE, we check if we send an invite to email we send the convert of invite message or send the correct invite message if we send an invite to phone number.

What alternative solutions did you explore? (Optional)

benjaminbennet commented 1 year ago

This is backend issue and should be internal.

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @alex-mechler (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

stephanieelliott commented 1 year ago

Hey @alex-mechler, adding you here for your take on whether this should be internal or external?

alex-mechler commented 1 year ago

Yes, this will have to be internal

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~0108b7537c7a8d0d92

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to Contributor Plus for review of internal employee PR - @fedirjh (Internal)

stephanieelliott commented 1 year ago

Thanks @alex-mechler, guess you're the lucky one on assignment here then!

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @NicMendonca (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

[ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
[ ] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
[ ] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

stephanieelliott commented 1 year ago

Reapplying the Bug label to get another BZ member on this while I am OOO til July 3. Thanks @NicMendonca, I'll grab this back from you when I return!

alex-mechler commented 1 year ago

Have not had a chance to dive into this, and am OOO friday-monday as a heads up here. I don't think this is urgent enough to warrent reassignment, but feel free to spin the wheel for another engineer if needed

NicMendonca commented 1 year ago

@alex-mechler should this still be a daily?

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @cead22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

michaelhaxhiu commented 1 year ago

🧇 I'm removing AlexM's assignment and triggering assignment of a new engineer

melvin-bot[bot] commented 1 year ago

@cead22 @NicMendonca @stephanieelliott @fedirjh this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

NicMendonca commented 1 year ago

@cead22 let me know if I need to find someone else internally 👋

cead22 commented 1 year ago

Quick investigation so far

I reproduced inviting my phone number to an expensifail.com workspace
Logs
AccountID #2211126 called 'InviteUsers' for policy '8F24C5EAEC6CADA3' message 'Test & " >' emailList '+14153166423@expensify.sms'
It looks like we're HTML-encoding the message before make the api call, so we're sending the encoded message to the api. It's possible we need a front end fix as well

Processing 'AddMembersToWorkspace' for 'expensify.com' from '75.80.102.185' ~~ 
command: 'AddMembersToWorkspace' 
employees: '[{"email":"+14153166423@expensify.sms"}]' 

welcomeNote: 'Test &amp; &quot; &gt;'   <=====================

policyID: '8F24C5EAEC6CADA3'
reportCreationData: '{"+14153166423@expensify.sms":{"reportID":"3871915553990080","reportActionID":"668730370059106033"}}' 
appversion: '1.3.31-3' 
pusherSocketID: '479922.296723' 
authToken: '<REDACTED>' 
referer: 'ecash' 
platform: 'web' 
api_setCookie: 'false' 
email: 'carlos@expensifail.com' 
partnerName: 'expensify.com'
browserGUID: '64963360b5db0' 
HTTP_CF_BOT_SCORE: '93' 
HTTP_CF_JA3_HASH: '21374ef547fa5844c59fd5d2f4ffdbaf'

cead22 commented 1 year ago

Looks like we intentionally html-escape here https://github.com/Expensify/App/blob/820fa93ad1e799c12d8b639b98018b5350ea17af/src/libs/actions/Policy.js#L408-L409

melvin-bot[bot] commented 1 year ago

@cead22 @NicMendonca @stephanieelliott @fedirjh this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

melvin-bot[bot] commented 1 year ago

@cead22, @NicMendonca, @stephanieelliott, @fedirjh Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

NicMendonca commented 1 year ago

@cead22 any update on this one?

melvin-bot[bot] commented 1 year ago

@cead22, @NicMendonca, @stephanieelliott, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick!

cead22 commented 1 year ago

Sorry, I don't have any updates for this yet, I've been kinda slammed. Ionatan posted a message today that makes me think that the solution I was thinking about might not be the right one, so I need to check with him on that before trying to code the solution

MariaHCD commented 1 year ago

I think this is related to: https://github.com/Expensify/App/issues/20081

@cristipaval has a solution here: https://github.com/Expensify/Web-Expensify/pull/38012 but let's confirm this is actually the holistic solution we want.

cristipaval commented 1 year ago

I think this is related to: #20081

@cristipaval has a solution here: Expensify/Web-Expensify#38012 but let's confirm this is actually the holistic solution we want.

Yes, it seems to be a dupe of https://github.com/Expensify/App/issues/20081

stephanieelliott commented 1 year ago

Ah yep, I agree this is a dupe and is solved with https://github.com/Expensify/App/issues/20081. Gonna close this out but feel free to reopen if you disagree

Expensify / App