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

[Hold #16099] [$1000] Phone number on new phone number page is displayed with @expensify.sms #15732

Closed
6 tasks done
kavimuru opened this issue Mar 7, 2023 · 21 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Mar 7, 2023

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:

  1. Open the app and login with user which has pending authentication phone number
  2. Open settings
  3. Open Profile
  4. Open Contact Method
  5. Open Phone number
  6. Observe phone number displayed in text

Expected Result:

Phone number shouldn't have @expensify.sms in it

Actual Result:

Phone number is displayed with @expensify.sms in the end

Workaround:

unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.80-0
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:
Untitled

expensify.sms.phone.number.page.issue.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1678212736675549

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0109f2fc4c3a9ac778
  • Upwork Job ID: 1633544502312452096
  • Last Price Increase: 2023-03-08
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 7, 2023
@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot locked and limited conversation to collaborators Mar 7, 2023
@MelvinBot
Copy link

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

@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Mar 8, 2023
@melvin-bot melvin-bot bot unlocked this conversation Mar 8, 2023
@melvin-bot melvin-bot bot changed the title Phone number on new phone number page is displayed with @expensify.sms [$1000] Phone number on new phone number page is displayed with @expensify.sms Mar 8, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~0109f2fc4c3a9ac778

@MelvinBot
Copy link

Current assignee @puneetlath is eligible for the External assigner, not assigning anyone new.

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 8, 2023
@MelvinBot
Copy link

Current assignee @puneetlath is eligible for the External assigner, not assigning anyone new.

@PrashantMangukiya
Copy link
Contributor

Proposal

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

Phone number on new phone number page is displayed with @expensify.sms

What is the root cause of that problem?

We are removing sms domain while displaying phone number as shown below:

<Text>
{this.props.translate('contacts.enterMagicCode', {contactMethod})}
</Text>

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

We have to remove SMS domain while displaying as shown below:

src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.js

<Text>
    {this.props.translate('contacts.enterMagicCode', {contactMethod})} // *** OLD CODE ***
    {this.props.translate('contacts.enterMagicCode', {contactMethod: Str.removeSMSDomain(contactMethod)})} // *** NEW CODE ***
</Text>

What alternative solutions did you explore? (Optional)

None

@wildan-m
Copy link
Contributor

Proposal

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

Phone number on new phone number page is displayed with @expensify.sms

What is the root cause of that problem?

The phone number information is retrieved from route params which contain the sms domain by default.

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

Not sure if my proposal is considered the same as the prev proposal or not, but if we can call the method to remove the domain once then I think it would be better. I would prefer to use PersonalDetails.getDisplayName like what we did here.

Add

const contactMethodDisplayName = PersonalDetails.getDisplayName(contactMethod);

and use it in HeaderWithCloseButton title and here:

{this.props.translate('contacts.enterMagicCode', {contactMethod: contactMethodDisplayName})}

What alternative solutions did you explore? (Optional)

N/A

@melvin-bot melvin-bot bot added the Overdue label Mar 10, 2023
@MelvinBot
Copy link

📣 @wildan-m! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Screen Shot 2022-11-16 at 4 42 54 PM

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@MelvinBot
Copy link

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

@puneetlath
Copy link
Contributor

@mollfpr what do you think of the proposals so far?

@melvin-bot melvin-bot bot removed the Overdue label Mar 10, 2023
@mollfpr
Copy link
Contributor

mollfpr commented Mar 11, 2023

@puneetlath It's not reproducible now because we revert the PR here #15761

@puneetlath
Copy link
Contributor

Ah got it. Ok, let's close this out then. Thanks everyone!

@mollfpr
Copy link
Contributor

mollfpr commented Mar 30, 2023

@puneetlath This issue is resurfacing again. We should open, and I'll review the proposals above.

Screenshot 2023-03-30 at 20 25 01

Simulator Screen Shot - iPhone 13 - 2023-03-30 at 20 25 05

@puneetlath puneetlath reopened this Mar 30, 2023
@puneetlath
Copy link
Contributor

Argh! Reopened.

@PrashantMangukiya
Copy link
Contributor

@puneetlath I think we have to go with submitted proposal, because some proposal already posted once Help wanted label applied.

@puneetlath
Copy link
Contributor

Ah, that's a great point that @koko57 is redoing how phone numbers will be displayed all over the app in #16099, so it should get handled there. I'll put this on hold for that and we can look back into it after #16099 is done.

@puneetlath puneetlath changed the title [$1000] Phone number on new phone number page is displayed with @expensify.sms [Hold #16099] [$1000] Phone number on new phone number page is displayed with @expensify.sms Mar 30, 2023
@melvin-bot melvin-bot bot added the Overdue label Apr 3, 2023
@MelvinBot
Copy link

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

@puneetlath
Copy link
Contributor

Still on hold.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 3, 2023
@puneetlath puneetlath added Weekly KSv2 and removed Daily KSv2 labels Apr 6, 2023
@melvin-bot melvin-bot bot removed the Overdue label Apr 6, 2023
@puneetlath
Copy link
Contributor

Moving to weekly while it's on hold.

@melvin-bot melvin-bot bot added the Overdue label Apr 17, 2023
@puneetlath
Copy link
Contributor

Going to go ahead and close since it is being handled by the linked issue.

@melvin-bot melvin-bot bot removed the Overdue label Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants