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

Jump to bottom

Profile - Phone number duplicated when adding as a secondary contact #17722

Closed
2 of 6 tasks
kbecciv opened this issue Apr 20, 2023 · 35 comments
Closed
2 of 6 tasks

Profile - Phone number duplicated when adding as a secondary contact #17722

kbecciv opened this issue Apr 20, 2023 · 35 comments
Assignees
@Beamanator @dylanexpensify
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Internal Requires API changes or must be handled by Expensify staff Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Apr 20, 2023 •
edited by melvin-bot bot
Loading

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

Issue found when executing PR #16804

Action Performed:

  1. Launch the app and login
  2. Open Settings -> Profile -> Contact method
  3. Add your number and it should appear on the list

Expected Result:

Only one number must be displayed in the list

Actual Result:

Phone number gets duplicated and same number listed two times

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.3.2.3

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6025487_Recording__1932.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause- Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019cd0980077a37963
  • Upwork Job ID: 1650516917226029056
  • Last Price Increase: 2023-04-24
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 20, 2023
@MelvinBot
Copy link

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

@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

@mountiny
Copy link
Contributor

@koko57 seems to be linked to your PR #16804 could you have a look into this

@koko57
Copy link
Contributor

koko57 commented Apr 21, 2023

@koko57 seems to be linked to your PR #16804 could you have a look into this

Yeah, of course!

@koko57
Copy link
Contributor

koko57 commented Apr 21, 2023 •
edited
Loading

@mountiny looks like another change has introduced this issue - having this in v1.3.1-3
IMG_6514
IMG_6513

@mountiny
Copy link
Contributor

Ok maybe @cristipaval or @Beamanator might know they are working on contact methods, sorry for the ping if not :D

@melvin-bot melvin-bot bot added the Overdue label Apr 24, 2023
@dylanexpensify
Copy link
Contributor

Nice, should I proceed with checklist @mountiny !?

@melvin-bot melvin-bot bot removed the Overdue label Apr 24, 2023
@mountiny
Copy link
Contributor

I would wait for @cristipaval and @Beamanator to confirm before exporting this

@dylanexpensify
Copy link
Contributor

dylanexpensify commented Apr 24, 2023 via email

@Beamanator
Copy link
Contributor

Eyooo we just arrived in London so will check this out now :D

@Beamanator Beamanator self-assigned this Apr 24, 2023
@Beamanator
Copy link
Contributor

Ya @cristipaval and I checked this out - this happened b/c in the backend we started normalizing & formatting the contact method that we sent to the front-end in Onyx, so they end up not matching & hence creating the duplicate. I am looking into this to see if we want to just not send a formatted contact method in the Onyx key, or if we want to format the phone number in Onyx in the front-end

@Beamanator Beamanator mentioned this issue Apr 24, 2023
Closed
22 tasks
@Beamanator
Copy link
Contributor

Beamanator commented Apr 24, 2023 •
edited
Loading

Ok my plan is to check out what kind of formatting our back-end does, then make sure we do the same kind of formatting on the front-end 👍

Why? I believe we have to format on the back-end (will find the reason shortly), and this stores phone numbers in a certain format in the server. When we refresh NewDot, we don't want the format to change when OpenApp pulls the latest contact method format. So we want to make sure we store the final version in Onyx from the beginning so nothing change in the future

Related server-side PR: https://github.com/Expensify/Web-Expensify/pull/37060

@Beamanator
Copy link
Contributor

Beamanator commented Apr 24, 2023 •
edited
Loading

Actually now I'm 99% sure the problem is that we add @expensify.sms as well as the country code (via twilio) in the backend. Will work on a backend PR to make sure everything works out well

@Beamanator Beamanator added Engineering Internal Requires API changes or must be handled by Expensify staff labels Apr 24, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~019cd0980077a37963

@MelvinBot
Copy link

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

@Beamanator Beamanator added the Reviewing Has a PR in review label Apr 25, 2023
@dylanexpensify
Copy link
Contributor

Not overdue - any updates @Beamanator ?

@Beamanator
Copy link
Contributor

Yes the fix should be in staging! But apparently Applause had an issue (#18031) but I couldn't reproduce so I closed it 😬

@dylanexpensify
Copy link
Contributor

Nice nice! Do we need to keep this open then!?

@Beamanator
Copy link
Contributor

Oh true I guess not 😅

@kbecciv
Copy link
Author

kbecciv commented Sep 5, 2023

One of our Contributor is faced the issue again, reopening
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693458155721979

Screenshot_2023-08-31-07-44-14-105

@kbecciv kbecciv reopened this Sep 5, 2023
@Beamanator
Copy link
Contributor

seems like a recent regression if this is reproducible again!

@dylanexpensify
Copy link
Contributor

Ah interesting! @Beamanator you good taking this on? Or shall I run it through BZ process?

@dylanexpensify
Copy link
Contributor

Bump @Beamanator 🙇‍♂️

@Beamanator
Copy link
Contributor

I can def take this on, but will make it weekly - it seems like it relates to front & backend so should keep it internal

@Beamanator Beamanator added Weekly KSv2 and removed Daily KSv2 Reviewing Has a PR in review labels Oct 5, 2023
@dylanexpensify
Copy link
Contributor

Nice! Agree, let me know roughly when you'll be able to action it, friend! 🙇‍♂️

@Beamanator
Copy link
Contributor

Yessss probably end of next week or early the following 😳

@dylanexpensify
Copy link
Contributor

Love it! ty!

@dylanexpensify
Copy link
Contributor

Maybe this week @Beamanator? Would this be a good new hire first pick potentially?

@dylanexpensify
Copy link
Contributor

Still pending

@dylanexpensify
Copy link
Contributor

@alex-mechler is out of office this week, but next week might be it!

@cristipaval
Copy link
Contributor

I think you meant @Beamanator, hahah

@dylanexpensify
Copy link
Contributor

hahahah yes I did 🙇‍♂️

@dylanexpensify
Copy link
Contributor

bump @Beamanator should we find someone else here?

@Beamanator
Copy link
Contributor

arg i haven't prioritized this, sorry - do you know if this is still happening? If you don't mind testing that would be great - I can try to look into this next week

@dylanexpensify
Copy link
Contributor

Not happening, just tried to repro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Beamanator Beamanator

@dylanexpensify dylanexpensify

Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Internal Requires API changes or must be handled by Expensify staff Weekly KSv2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@Beamanator @MelvinBot @cristipaval @koko57 @mountiny @dylanexpensify @thesahindia @kbecciv