[$500] Bank account - Able to save phone number starting from = not + via WS Bank account connection

[lanitochka17]

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

---

Version Number: 1.4.27-1 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 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Navigate to the add bank account modal in Workspace settings
3. Click on the Connect online with Plaid
4. Select Chase bank
5. Enter the test credentials
6. Choose the savings account "Plaid Saving11122XXXXXX111"
7. Save Chase bank account and go to the Company information
8. In a Company find phone number and enter number starting not from + but from = (=15123456789)
9. Fill out the fields with the required test credentials
10. Click Save and continue button.

Expected Result:

User is able to save phone number starting from + via WS Bank account connection

Actual Result:

User is able to save phone number starting from = not + via WS Bank account connection

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/8a797403-d194-4d32-b184-41dc1ce1fcae

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012299eb9b4898ac21
• Upwork Job ID: 1748071347542683648
• Last Price Increase: 2024-01-18
• Automatic offers:
• FitseTLT | Contributor | 28116411

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[FitseTLT]

Proposal

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

Bank account - Able to save phone number starting from = not + via WS Bank account connection

What is the root cause of that problem?

We are checking if it is valid US phone number here https://github.com/Expensify/App/blob/599c005849cd065a53d35e015991b637c768f2da/src/pages/ReimbursementAccount/CompanyStep.js#L107-L108 But here https://github.com/Expensify/App/blob/599c005849cd065a53d35e015991b637c768f2da/src/libs/ValidationUtils.ts#L268-L269 b/c we pass it regionCode parsePhoneNumber returns valid (possible as true) although it starts with # (it will return invalid if you pass the same string without region code ) can be considered a bug from parsePhoneNumber and hence it not indicating the error message

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

We should validate the number with Str.isValidPhone before we pass it to isValidUSPhone change it to

            if (values.companyPhone && (!Str.isValidPhone(values.companyPhone) || !ValidationUtils.isValidUSPhone(values.companyPhone, true))) {
            errors.companyPhone = 'bankAccount.error.phoneNumber';

or We can move the validating code inside ValidationUtils.isValidUSPhone as we use it in different places

What alternative solutions did you explore? (Optional)

the validation we can even implement it in our parsePhoneNumber

[ghost]

Proposal

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

Bank account - Able to save phone number starting from = not + via WS Bank account connection

What is the root cause of that problem?

I think the root cause is : We are checking if it is valid US phone number here

https://github.com/Expensify/App/blob/31b19eaaf00c23492c6e5132b9501abdeb210e78/src/pages/ReimbursementAccount/CompanyStep.js#L107-L109

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

We can do to fix it is in ValidationUtils, we can enhance the isValidUSPhone function to include a stricter validation for the phone number format before it gets parsed and checked by parsePhoneNumber. Specifically, you can add a regex check to ensure that the phone number starts with "+" when it includes a country code, or adheres to a valid US phone number format.

something like this in parsePhoneNumber

const regex = isCountryCodeOptional ? /^(\+\d{1,2})?\d{10}$/ : /^\+\d{1,2}\d{10}$/;

    if (!regex.test(phone)) {
        return false;
    }

What alternative solutions did you explore? (Optional)

Another solution would be in CompanyStep file:

• Update the validation functions to allow "+" only, not "=".
• Preprocess input to convert any "=" to "+" before validating.

if (values.companyPhone) {

  // Preprocess phone number
  values.companyPhone = values.companyPhone.replace(/^=/, '+');

  // Validate
  if (!Str.isValidPhone(values.companyPhone) || !ValidationUtils.isValidUSPhone(values.companyPhone, true)) {
    errors.companyPhone = 'bankAccount.error.phoneNumber'; 
  }

}

[ghost]

Updated Proposal

[dukenv0307]

Proposal

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

User is able to save phone number starting from = not + via WS Bank account connection

What is the root cause of that problem?

We validate phoneNumber using isValidUSPhone https://github.com/Expensify/App/blob/991a4e213d6cb0af42b62b3e9f9c6b95b0317bea/src/pages/ReimbursementAccount/CompanyStep.js#L107

In this function, parsePhoneNumberreturn possible as true because it will remove the special character and check the number. So the US phone number starting with = still pass the validate function. https://github.com/Expensify/App/blob/991a4e213d6cb0af42b62b3e9f9c6b95b0317bea/src/libs/ValidationUtils.ts#L268-L269

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

In isValidUSPhone function, we should add a check using Str.isValidPhone function to make sure it starts with + and possibly is the phone number. It's the same way as we check here and some other places when we check for the valid phone number

return parsedPhoneNumber.possible && parsedPhoneNumber.regionCode === CONST.COUNTRY.US && Str.isValidPhone(phoneNumber);

https://github.com/Expensify/App/blob/991a4e213d6cb0af42b62b3e9f9c6b95b0317bea/src/libs/ValidationUtils.ts#L268-L269

*Note: I saw that we're using isValidUSPhone so we should check this function so all places in the app using it can work correctly.

What alternative solutions did you explore? (Optional)

We can create a util to check for valid phoneNumber and use it in all related places of the app to make the app consistent as well

[mananjadhav]

I think it's best we call the isValidPhone inside isValidUSPhone. Hence @FitseTLT's proposal looks good to me.

🎀 👀 🎀 C+ reviewed.

[melvin-bot[bot]]

Triggered auto assignment to @AndrewGable, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot[bot]]

📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[sonialiap]

Not overdue, a proposal was just accepted and contributor assigned

[mananjadhav]

PR was just raised few hours ago. Will review this PR today.

[mananjadhav]

@sonialiap This was deployed to production yesterday but the payout date wasn't updated.

[mananjadhav]

@sonialiap this should be ready for the payout today. I don't have any offending PR, we fixed this in the isValidUSPhone, we could've added when the method was added, not sure?

But I think it makes sense to add a regression test for this one. Here's the regression test steps:

1. Open the App
2. Navigate to the add bank account modal in Workspace settings
3. Add a bank account.
4. Go to the company information
5. In a Company find phone number and enter number starting not from + but from = (=15123456789)
6. And focus on the next input and Verify that Please enter a valid phone validation message is displayed.

[mallenexpensify]

Contributor: @FitseTLT paid $500 via Upwork Contributor+: @mananjadhav due $500 via NewDot

TR GH - https://github.com/Expensify/Expensify/issues/369254

Thanks!!

[JmillsExpensify]

$500 approved for @mananjadhav based on summary above.