[HOLD for payment 2021-12-06] [$250] When creating new accounts, allow users to enter their phone numbers with - and/or ( ) and spaces

[mallenexpensify]

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

---

Action Performed:

Flow 1:

1. Navigate to NewDot
2. Input a phone number as login using any of symbols -, (, ) or spaces (e.g. +1 (222) 222 2222)

Flow 2:

1. Open concierge chat in NewDot
2. Click the call icon on the top right
3. Input a phone number as login using any of symbols -, (, ) or spaces (e.g. +1 (222) 222 2222)

Expected Result:

The user should be able to sign in or request a call from concierge just fine . The number input by the user should be cleaned before being sent to the back-end, stripping -, (, ) and spaces.

Actual Result:

We get validation errors

## Platform: Where is this issue occurring? ALL

Version Number:
Reproducible in staging?:
Reproducible in production?:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/183582#
Issue reported by:
Slack conversation:

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[mallenexpensify]

Upwork Job posted. cc @aldo-expensify in case you want to snag this, auto-assigning via Exported tag now.
https://www.upwork.com/jobs/~01e3a9fa28fc98c96a

[MelvinBot]

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

[Bojan996]

After following the code, I see that the main issue is the regex that is used to validate the phone number.
Because you are using the expensify-common git repo for validation you will either have to change the code directly in the repo or add a custom regex to your project (for this specific case only). I would suggest changing the code on expensify-common (I see it's the same company name so I guess you have access), because this will improve future use.

The function that you are using for validation can be found in expensify-common/lib/str.js 921
This uses the regex found in expensify-common/lib/CONST.jsx 518

This can be improved, I have a regex that will solve it but I need to know are you restricting the users to follow a country specific phone pattern, would you allow users to not put in spaces, parenthesis and a plus sign or do they have to?

If you provide some more details about what kind of patterns would you allow, I am more than happy to solve the issue for you.

[parasharrajat]

@mallenexpensify I added this as a feature @mountiny do you have any comments? Oops didn't see that we want to add it.

Proposal

Two approaches

1

I allowed the bracket (( )) and - in the login in the following commit. 500ba78.

1. So we will update the replacement regex here allow spaces as well. So that validation will pass for Logic form.

/^\+?\d*$/.test(input.replace(/((?!\n)[()-\s\t])/g, ''))

2. We just run this replacement before matching the input in Str.isvalidPhone or before passing to backend.

   App/src/pages/signin/LoginForm.js

   Line 82 in b12eb5f

   if (!Str.isValidEmail(this.state.login) && !Str.isValidPhone(this.state.login)) {

   .

   App/src/pages/signin/LoginForm.js

   Line 96 in b12eb5f

   fetchAccountDetails(this.state.login);

---

2

OR we can just use the CONST.REGEX.PHONE_WITH_SPECIAL_CHARS regex instead of Str.isValidPhonein Login Page validations which works well. And remove these values before sending them to the backend using the above replacement.

[mountiny]

@parasharrajat is right, this has been introduced when we wanted to unify the error messages in this PR #5864.

We should be able to just revert the commit Rajat mentioned to allow the (, ) and - characters.

[parasharrajat]

Sorry, @mountiny for tagging you. I didn't correctly see the issue. We want to add it not remove it so I have updated the proposal. Now we want to extend that change and allow spaces as well.

[akshayasalvi]

Proposal

I had changed the phone number regex in the company VBA flow. I would want to change the validation check to isValidPhoneWithSpecialChars in LoginForm.validateAndSubmitForm

if (!Str.isValidEmail(this.state.login) && !isValidPhoneWithSpecialChars(this.state.login)) { // updated func here
            if (isNumericWithSpecialChars(this.state.login)) {
        

[mallenexpensify]

We are placing all non-critical issues on hold while we're on a company offsite. The hold is expected to be lifted on 11/19 (but could go longer). For any PRs that are submitted before or during the hold, we will add a $250 bonus payment.

[chiragsalian]

I'm going to go with @parasharrajat because he captured the importance of replacing the character values as well before sending it to the backend. If we send the characters to the backend I'm pretty sure it will error out.

As for your 1 and 2 proposal choices @parasharrajat. I think 1 would be easier and cleaner. I don't mind solution 2 but in my limited testing CONST.REGEX.PHONE_WITH_SPECIAL_CHARS or isValidPhoneWithSpecialChars didn't work as well as I expected. For example, take this number without special characters +15857527441, with special characters it would be +1 (585) 752-7441 and the regex fails and the length checks in isValidPhoneWithSpecialChars also fail for it. Something to keep in mind if you want to pursue solution 2.

Anyway feel free to create a PR @parasharrajat and @mallenexpensify feel free to hire Rajath in Upwork.

[mallenexpensify]

@parasharrajat , can you apply here please? https://www.upwork.com/jobs/~01e3a9fa28fc98c96a

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.16-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Allow spaces and special chars in phone number username #6293

If no regressions arise, payment will be issued on 2021-12-06. 🎊

[parasharrajat]

Ping for

@mallenexpensify

[mallenexpensify]

@parasharrajat Can you accept the offer here please and confirm when you have?
https://www.upwork.com/jobs/~01b50ba5b54d346056

[parasharrajat]

Accepted it @mallenexpensify

[mallenexpensify]

Paid @parasharrajat $250 for the job and $250 for company offsite hold bonus.
Thanks!