[HOLD for payment 2021-09-08] [PAY 9/7] Final review modal is showing @expensify.sms for Users with Phone Numbers as their login

[aman-atg]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Login to newDot
2. Request money from an account who has phone number as their login
3. Enter amount and Continue
4. You will see @expensify.sms after the phone number

Expected Result:

Should show only the phone number

Actual Result:

Workaround:

Visual Issue.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[aman-atg]

Proposal

• To remove the smsDomain we need to pass alternateText: Str.removeSMSDomain(participant.login) here
  

  App/src/libs/OptionsListUtils.js

  Line 580 in ec38ff9

  ...participant, descriptiveText: amountText,

[MelvinBot]

@sakluger Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[sakluger]

Thanks for creating this issue @aman-atg! I'm reviewing the issue now.

[MelvinBot]

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

[sakluger]

This issue is unique and the suggested behavior is in line with what we did in this related issue.

This should be able to be handled by an external contributor.

The su

[MelvinBot]

Triggered auto assignment to @arielgreen (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[mallenexpensify]

Putting on hold for a week-ish

[mallenexpensify]

Job posted https://www.upwork.com/jobs/~01b9648852c0022ec5
@chiragsalian will you review @aman-atg 's proposal above? Thx

[chiragsalian]

I just tried this on dev and i don't see @expensify.sms next to the phone number

Also tried it on production and i don't see the issue,

Might be some old issue. @aman-atg can you confirm, if the issue does not exist let's close this GH issue.

[aman-atg]

@chiragsalian It's still happening for me.
It's strange this should happen for any number.

Dev

PROD

[chiragsalian]

Weird, i don't see it for the same number you provided.

Maybe its because you have first and last name loaded. Trying that next,
I created a phone account with test aug17th as the name but i don't see expensify.sms domain.

Trying your account, i also don't see expensify.sms. lol, weird.

I just cleared my cache and retested,

So i'm a little unsure how you are getting the issue. Did you also try on google chrome and did you clear your cache?

[chiragsalian]

@mallenexpensify are you also able to reproduce the issue?

[mallenexpensify]

No, unable to reproduce on desktop, Version 1.0.85-9 (production version released today

[aman-atg]

Did you also try on google chrome and did you clear your cache?

Well, this is a bizarre situation.. I tried with chrome as well and cleared my cache.
Even tried on a different machine.

I think it's only occurring when we request money directly from the chat.

Video for reference.

test.mp4

If it's still not reproducible then we can maybe close this for now.

[mallenexpensify]

I was able to reproduce! Just took a handful of tries.

• open staging.new.expensify.com in chrome.
• go to chat with a user where their account login is a phone number
• click the plus icon next to the compose box (not the big green one)
• enter amount
• observe expensify.sms at end of username.

I cleared my local storage and signed out/back in and the issue persists.

@chiragsalian can you test the above steps, try to reproduce, and if so... review @aman-atg 's proposal to fix?

[parasharrajat]

Proposal

I propose that we fix the original change that caused this. In newDot we trim the SMS domain from phone numbers while preparing data options for the viewing purpose. Every component follows that. But I see that this #3651 PR added this to fix the issue with IOU Creation but at that time we didn't think of this regression.

• To Keep the app consistent, we remove the logic to add the SMS domain to options from here

  App/src/libs/OptionsListUtils.js

  Line 109 in ec38ff9

  login: addSMSDomainIfPhoneNumber(login),

  .
• Then we fix the Request money - Endless loading spinner when requesting money from phone number account #3614 we add the SMS domain while passing the logins to createIOUTransaction. This is the same pattern we are following in WorkspaceInvitePage.js

  App/src/libs/actions/Policy.js

  Line 152 in 2b57f0f

  const newEmployeeList = _.map(logins, login => addSMSDomainIfPhoneNumber(login));

Why do this?

As we create the first drop SMS domain while creating options for the listing and then we are re-adding the SMS domain here and again if remove the SMS Domain from here

App/src/libs/OptionsListUtils.js

Line 580 in ec38ff9

...participant, descriptiveText: amountText,

. It is so confusing to manage. The main source of the data should only be one location OptionsListUtils file. This is the reason we create options for each component from this module only.

App/src/libs/OptionsListUtils.js

Line 191 in ec38ff9

function createOption(personalDetailList, report, draftComments, {

[MelvinBot]

@chiragsalian, @mallenexpensify it looks like no one is assigned to work on this job.
Please double the price or add a comment stating why the job isn't being doubled.

[chiragsalian]

Yup, i can reproduce the issue when i request for money within the chat. Previously i had only tried it via the big green one. Cool, looks like a real issue.

I like @parasharrajat, @mallenexpensify can you hire him on upwork.
@parasharrajat, feel free to create the PR.

[mallenexpensify]

@chiragsalian , since @aman-atg created the issue, we often them allow them "first dibs" to have their proposal reviewed. Can you confirm that you've reviewed @aman-atg proposal above and, if you choose to not go with it, provide a synopsis of why not? Thx

[chiragsalian]

Oh hahaha, I just realized I didn't complete my thought earlier and just wrote,

I like @parasharrajat

😂 😂

Lol, i meant to say,

I like @parasharrajat solution because I'd like the data to exist cleanly in login without sms domain, than pass an additional correct value to alternate text.

[mallenexpensify]

@parasharrajat , can you submit a proposal in Upwork and I'll hire? https://www.upwork.com/jobs/~01b9648852c0022ec5. Please confirm in a message here once you have

[parasharrajat]

@mallenexpensify Done.

[mallenexpensify]

Hired in Upwork, assigned to @parasharrajat , removed Help Wanted label...

[botify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[botify]

🚀 Deployed to staging by @Julesssss in version: 1.0.90-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[botify]

🚀 Deployed to production by @roryabraham in version: 1.0.91-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

[mallenexpensify]

Paid @parasharrajat in Upwork