[$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?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6346953_1705607179949.Able_to_save_a_phone_number_starting_from.mp4

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]

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

[melvin-bot]

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

[melvin-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

App/src/pages/ReimbursementAccount/CompanyStep.js

Lines 107 to 108 in 599c005

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

But here

App/src/libs/ValidationUtils.ts

Lines 268 to 269 in 599c005

	const parsedPhoneNumber = parsePhoneNumber(phone, {regionCode});
	return parsedPhoneNumber.possible && parsedPhoneNumber.regionCode === CONST.COUNTRY.US;

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

App/src/pages/ReimbursementAccount/CompanyStep.js

Lines 107 to 109 in 31b19ea

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

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

App/src/pages/ReimbursementAccount/CompanyStep.js

Line 107 in 991a4e2

if (values.companyPhone && !ValidationUtils.isValidUSPhone(values.companyPhone, true)) {

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.

App/src/libs/ValidationUtils.ts

Lines 268 to 269 in 991a4e2

	const parsedPhoneNumber = parsePhoneNumber(phone, {regionCode});
	return parsedPhoneNumber.possible && parsedPhoneNumber.regionCode === CONST.COUNTRY.US;

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);

App/src/libs/ValidationUtils.ts

Lines 268 to 269 in 991a4e2

	const parsedPhoneNumber = parsePhoneNumber(phone, {regionCode});
	return parsedPhoneNumber.possible && parsedPhoneNumber.regionCode === CONST.COUNTRY.US;

*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]

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

[melvin-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.