ikevin127 / expensify-proposal-testing

MIT License
0 stars 0 forks source link

[MOCK] BA - On the phone number page, user can not enter data as the placeholder shows #2

Closed ikevin127 closed 1 month ago

ikevin127 commented 8 months ago

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: v1.4.47-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+vd_web030424@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team

Action Performed: Pre-requisite: user must be logged in and have created a workspace.

  1. Go to Workspace > Bank account.
  2. Initiate the add BA flow with the testing credentials.
  3. On Company information section, reach the Phone number page.
  4. Verify the placeholder is displayed as "(xxx)xxx-xxxx".
  5. Enter a random US number in that same format, like "(440)458-9784".
  6. Click on "Next".
  7. Verify an error appears.

Expected Result: The user should be able to enter the phone number as the placeholder shows.

Actual Result: The user can not enter the phone number as the placeholder shows.

ikevin127 commented 8 months ago

Edited by proposal-police: This proposal was edited at 2024-03-07 16:41:05 UTC.

tienifr's proposal

Proposal

Please re-state the problem that we are trying to solve in this issue. The user can not enter the phone number as the placeholder shows.

What is the root cause of that problem?

We already have the logic to check the phone number is valid or not in here

but it's not really correct since it can cause some confused cases: https://github.com/Expensify/App/issues/34766

so we added the additional logic to check phone number in

https://github.com/Expensify/App/blob/ffa731a28a2d1402148eef54e51d86fbb64321fe/src/libs/ValidationUtils.ts#L287

but isValidPhone just use the simple e164 regex

https://github.com/Expensify/expensify-common/blob/77d0b150ba6bfbe7a64b3c3e30b65592b2e58c4a/lib/CONST.jsx#L571-L572

and that doesn't cover other format like (440)458-9784

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

Here's the possible format:

significant: 4404589784 international: +1 440-458-9784 e164: +14404589784 national: (440) 458-978 123.456.7890

so we can use the following regex:

const phoneNumberRegex = /(\+\d{1,2}\s?)?(\(\d{3}\)|\d{3})[\s.-]?\d{3}[\s.-]?\d{4}/;

What alternative solutions did you explore? (Optional)

NA

ikevin127 commented 8 months ago

Edited by proposal-police: This proposal was edited at 2024-03-07 16:31:55 UTC.

dragnoir's proposal

Proposal

Please re-state the problem that we are trying to solve in this issue. Phone number in the format (440)458-9784 is marked as invalid

What is the root cause of that problem?

Issue is within the function isValidUSPhone https://github.com/Expensify/App/blob/578769197ddd4039f46b949d83a7e85ad2476abc/src/libs/ValidationUtils.ts#L281C10-L293

When the input is in phormat like: (440)458-9784

The condition below: https://github.com/Expensify/App/blob/578769197ddd4039f46b949d83a7e85ad2476abc/src/libs/ValidationUtils.ts#L287-L289 will turn: regionCode: US and !Str.isValidPhone(phone): true Then the execution will exit without approving the phone number

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

We need to fix this condition https://github.com/Expensify/App/blob/578769197ddd4039f46b949d83a7e85ad2476abc/src/libs/ValidationUtils.ts#L285-L289

It's built to fix an issue as mentioned in the comment:

// When we pass regionCode as an option to parsePhoneNumber it wrongly assumes inputs like '=15123456789' as valid

Currently we use Str.isValidPhone(phone) to check if the input is a valid phone number. isValidPhone is a function from expensify-common. We can replace it with another function we create to check all possible format and return a true for valid US phone numbers.

What alternative solutions did you explore? (Optional)

We can fix the function isValidPhone from expensify-common to consider the format (xxx)xxx-xxxx as a valid phone number https://github.com/Expensify/expensify-common/blob/77d0b150ba6bfbe7a64b3c3e30b65592b2e58c4a/lib/str.js#L934-L936