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

[$250] Card - 10 digit phone number isn't accepted when the member adds shipping details #50282

Closed
6 tasks done
IuliiaHerets opened this issue Oct 5, 2024 · 14 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Oct 5, 2024

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


Version Number: 9.0.44-8
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #49452
Issue reported by: Applause Internal Team

Action Performed:

  1. Admin: Navigate to https://staging.new.expensify.com/
  2. Admin: Log in with a new Expensifail account
  3. Admin: Create a workspace
  4. Admin: Navigate to Workspace settings - More features
  5. Admin: Enable "Expensify Card"
  6. Admin: Navigate to Workspace settings - Members
  7. Admin: Invite a Gmail address that doesn't have an account yet
  8. Admin: Navigate to Workspace settings - Expensify Card
  9. Admin: Click on the "Issue new card" button
  10. Admin: Add bank account details and finish the BA flow with Plaid Regions bank
  11. Admin: Click on the "Issue new card" button
  12. Admin: Select the new member
  13. Admin: Select "Physical card
  14. Admin: Add the card with any limit type, limit and name
  15. Admin: Navigate to the workspace chat with the member
  16. Member: Log in as a member added in step 7
  17. Member: Navigate to the workspace chat
  18. Member: Click on the "Add shipping details" button
  19. Member: Set any name and click on the "Next" button
  20. Member: Set any DOB and click on the "Next" button
  21. Member: Set any address and click on the "Next" button
  22. Member: Input "8005555555" as the phone number and click on the "Next" buttons

Expected Result:

10 digit phone number should be accepted.

Actual Result:

10 digit phone number isn't accepted when the member adds shipping details. Only 8 digits are accepted.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6624081_1728026724870.b.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021843278716830129573
  • Upwork Job ID: 1843278716830129573
  • Last Price Increase: 2024-10-14
Issue OwnerCurrent Issue Owner: @rayane-djouah
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 5, 2024
Copy link

melvin-bot bot commented Oct 5, 2024

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #wave-control

@IuliiaHerets
Copy link
Author

@lschurr FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@twilight2294
Copy link
Contributor

twilight2294 commented Oct 6, 2024

Edited by proposal-police: This proposal was edited at 2024-10-06 19:06:35 UTC.

Proposal

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

10 digit phone number isn't accepted when the member adds shipping details.

What is the root cause of that problem?

The validation functions we use to check phone number is wrong, this gives us the error on the frontend

const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumber);
if (!parsedPhoneNumber.possible || !Str.isValidE164Phone(phoneNumber.slice(0))) {
errors[INPUT_IDS.PHONE_NUMBER] = translate('bankAccount.error.phoneNumber');

The above validation will give errors for 10 digit numbers.

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

We should use the validation logic we have on the loginIn page:

const phoneLogin = LoginUtils.appendCountryCode(LoginUtils.getPhoneNumberWithoutSpecialChars(loginTrim));
const parsedPhoneNumber = parsePhoneNumber(phoneLogin);
if (!Str.isValidEmail(loginTrim) && !parsedPhoneNumber.possible) {
if (ValidationUtils.isNumericWithSpecialChars(loginTrim)) {
setFormError('common.error.phoneNumber');

We should use the same logic for the add shipping details RHP too, this way we make sure that we are validating against the correct value and using the current functions to validate. If the same bug exists at other places we should fix them too

 
const phoneLogin = LoginUtils.appendCountryCode(LoginUtils.getPhoneNumberWithoutSpecialChars(values[INPUT_IDS.PHONE_NUMBER]));
const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumber);
if (!parsedPhoneNumber.possible) {

                   errors[INPUT_IDS.PHONE_NUMBER] = translate('bankAccount.error.phoneNumber');

NOTE:
@rayane-djouah the flow for shipping details will only be used when we have added a bank account so, the check is valid and correct for this use case

What alternative solutions did you explore? (Optional)

@lschurr lschurr added the External Added to denote the issue can be worked on by a contributor label Oct 7, 2024
Copy link

melvin-bot bot commented Oct 7, 2024

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

@melvin-bot melvin-bot bot changed the title Card - 10 digit phone number isn't accepted when the member adds shipping details [$250] Card - 10 digit phone number isn't accepted when the member adds shipping details Oct 7, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 7, 2024
Copy link

melvin-bot bot commented Oct 7, 2024

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

@twilight2294
Copy link
Contributor

@rayane-djouah my proposal is here above

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 9, 2024

Proposal

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

10 digit phone number isn't accepted when the member adds shipping details. Only 8 digits are accepted.

What is the root cause of that problem?

On the sign-in page, countryCodeByIP is the US country code by default. Therefore, this phone number on the login page is valid.

Onyx.connect({
key: ONYXKEYS.COUNTRY_CODE,
callback: (val) => (countryCodeByIP = val ?? 1),
});

After the user is logged in, if the user is not in USA, the countryCodeByIP isn't 1 which leads parsedPhoneNumber.possible is false because the phoneNumber appends the wrong country code

function appendCountryCode(phone: string): string {

const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumber);
if (!parsedPhoneNumber.possible || !Str.isValidE164Phone(phoneNumber.slice(0))) {

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

In the other place of phone number input in bank account flow, we use isValidUSPhone to check whether the phone number is valid or not like here, here. So I think we also can do this here

if (!values[INPUT_IDS.PHONE_NUMBER] || !ValidationUtils.isValidUSPhone(values[INPUT_IDS.PHONE_NUMBER], true)) {
    errors.phoneNumber = translate('bankAccount.error.phoneNumber');
}

const parsedPhoneNumber = PhoneNumberUtils.parsePhoneNumber(phoneNumber);
if (!parsedPhoneNumber.possible || !Str.isValidE164Phone(phoneNumber.slice(0))) {

OPTIONAL: We also can limit the US address in the address step by adding isLimitedToUSA prop here

What alternative solutions did you explore? (Optional)

@lschurr
Copy link
Contributor

lschurr commented Oct 9, 2024

@rayane-djouah could you review the proposals here? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Oct 9, 2024
@rayane-djouah
Copy link
Contributor

Reviewing 👀

@melvin-bot melvin-bot bot removed the Overdue label Oct 9, 2024
@rayane-djouah
Copy link
Contributor

rayane-djouah commented Oct 11, 2024

@lschurr @IuliiaHerets I don't believe this is a bug. The app automatically appends the country code to the phone number based on the user's IP address during validation. If the user is not in the USA, the corresponding country code for their location will be added to the number 8005555555 and then validated. For users in the USA, using 8005555555 does not give an error. To use a USA phone number from outside the US, the user needs to add +1 before the number; thus, +18005555555 should work without any errors.

Copy link

melvin-bot bot commented Oct 14, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Oct 14, 2024
@rayane-djouah
Copy link
Contributor

@lschurr I think we can close this issue

@melvin-bot melvin-bot bot removed the Overdue label Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

@lschurr, @rayane-djouah Whoops! This issue is 2 days overdue. Let's get this updated quick!

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. Daily KSv2 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
Projects
Archived in project
Development

No branches or pull requests

5 participants