[HOLD for payment 2021-10-14] [HOLD for payment 2021-10-14] Prevent inaccurate information by enforcing a character limit during the VBA flow

[SofiedeVreese]

Expected Result:

When you're adding your phone number, tax ID and last 4 of SSN in the add VBA flow, we should enforce a character limit and format at you type.

Actual Result:

We do none of that.

Solution:

Enforce a character limit on these fields such that:

• Phone number is limited to 10*
• Tax ID is limited to 9
• SSN is limited to 4

*The phone number limit is 10, excluding the ( and ) which surround the beginning 3 digits. We should auto-format this as you type. Such that typing 5551234567 auto formats to (555) 123-4567

• Version Number: (For Cash and Mobile issues only) Staging Version 1.0.94-0 (1.0.94-0)

Future Notes:

1. We should also eventually localise the phone number help text and character limit based on your location info, but given these are likely to be US only for now, we can cross that bridge when we get to it.
2. We could also format the EIN such that it matches the IRS letters 12-3456789 but not all Tax ID's are EIN's (some self employed people just use SSNs) and we'd also need to not include the dash when we send it to our validation APIs.

Coming from Expensify/Expensify #176806

[SofiedeVreese]

Triaged & auto-assigned via the external label in #176806.

[SofiedeVreese]

Posted on Upwork!.

Adding exported label.

[MelvinBot]

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

[PrashantMangukiya]

I created workspace, added private domain email as secondary login and verified. Although I am not able to see add bank account related flow. Any suggestion how to activate VBA flow to reproduce this issue as a contributor.

Also in my account only paypal.me shows as payment method, there is no Add Bank Account etc.

[SofiedeVreese]

Hmm @PrashantMangukiya that is indeed odd. Let me chat to our Engineers to double-check and see if this is truly an issue that's Contributor-suitable!

[PrashantMangukiya]

@SofiedeVreese Thank you for updates.

This issue not set as external so consider as my suggestion (not proposal). At present I can not reproduce VBA flow, so suggesting generic sample code to format phone number as user is typing. During form submit we have to remove also remove () and - from phone number, because within db we may not store such formatting chars.

Also sample code given to limit Tax ID to 9 char, and SSN to 4 chars.

  onChangeTaxID(taxId) {
      const taxIdFinal = !taxId ? taxId : taxId.trim().slice(0, 9);
      this.setState({
          taxId: taxIdFinal,
      });
  }

  onChangeSSN(ssn) {
      const ssnFinal = !ssn ? ssn : ssn.trim().slice(0, 4);
      this.setState({
          ssn: ssnFinal,
      });
  }

  onChangePhoneNumber(phone) {
    this.setState({
        phone: this.formatPhoneNumber(phone),
    });
  }

  formatPhoneNumber(phone) {
    if (!phone) { return phone; }
    const phoneNumber = phone.replace(/[^\d]/g, '');  // Cleanup non digit value
    if (phoneNumber.length < 4) { return phoneNumber; }
    if (phoneNumber.length < 7) {
        return `(${phoneNumber.slice(0, 3)}) ${phoneNumber.slice(3)}`;
    }
    return `(${phoneNumber.slice(0, 3)}) ${phoneNumber.slice(3, 6)}-${phoneNumber.slice(6, 10)}`;
  }

  <ExpensiTextInput
      label={this.props.translate('common.phoneNumber')}
      value={this.state.phone}
      onChangeText={this.onChangePhoneNumber}
      keyboardType={CONST.KEYBOARD_TYPE.PHONE_PAD}
      returnKeyType="done"
  />

If this issue labeled as external then consider as proposal. I already submitted proposal on Upwork, you can accept it if my solution suitable for this issue. Thank you.

[akshayasalvi]

Proposal

My suggestion would be to resolve just the length and type checks in this issue. Auto-formatting is at the moment not supported. We should be adding masked input support rather than programmatically adding phone number formatting. For example, http://insin.github.io/react-maskedinput/ .

I am suggesting without adding auto-formatting to the phone number, we should add maxLength to input fields.

In CompanyStep

<ExpensiTextInput
     label={this.props.translate('common.phoneNumber')}
     maxLength={10}
     ...
     ...

<ExpensiTextInput
     label={this.props.translate('companyStep.taxIDNumber')}
     maxLength={9}
     ...
     ...

and in IdentityForm

<ExpensiTextInput
    label={`${translate('common.ssnLast4')}`}
    maxLength={4}

[SofiedeVreese]

Hey @PrashantMangukiya I checked with our engineers and this flow is not hidden, it's sitting on a beta active on dev.

They suggested trying with a fresh private domain account vs secondary login.

[PrashantMangukiya]

@SofiedeVreese I got it, thanks for the updates.

[puneetlath]

Hm, I agree with @akshayasalvi that we should just validate the length/format and not worry about auto-formatting the input.

My suggestion would be to simply:

• Limit phone number field to 15, since that is the length that would cover international, not including dashes and parenthesis
• Validate phone number input for allowable characters
• Limit Tax ID to 9
• Limit SSN to 4

What do you think @SofiedeVreese ?

[SofiedeVreese]

I like that @puneetlath it seems the simplest and cleanest way of tackling this issue.

[SofiedeVreese]

@akshayasalvi do you agree with Puneet's feedback above?

@puneetlath if so, would you like to proceed with @akshayasalvi ?

[akshayasalvi]

Yeah, @SofiedeVreese I agree with @puneetlath's feedback.

[puneetlath]

@akshayasalvi I don't know that simply using maxLength will work for the phone number field, since we want to allow parenthesis and dashes. And we want to disallow characters that can't be in phone numbers. How would you go about handling that?

[akshayasalvi]

I agree. But I also a little unsure if we're allowing parenthesis and dashes. Placement for this might differ based on the locale. I could quickly write a regex to match this, once you confirm the format.

I've also checked some of the codes, it seems we're using Str.isValidPhone function is used for validation. So are we planning to change the phone number formatting across all usages?

[puneetlath]

So perhaps one solution would be to:

• set a max-length that takes into account the possibility of parenthesis/dashes/etc
• only allow numbers and specific symbols
• use the Str.isValidPhone to do a client-side validation that it is a valid phone number

[akshayasalvi]

Sure that could work.

[puneetlath]

Ok great. And can you share your proposal at a high level for how you would handle each of the requirements?

[akshayasalvi]

In extension to what I've mentioned in this comment #5184 (comment)

Proposal

My suggestion would be to resolve just the length and type checks in this issue. Auto-formatting is at the moment not supported. We should be adding masked input support rather than programmatically adding phone number formatting. For example, http://insin.github.io/react-maskedinput/ .

I am suggesting without adding auto-formatting to the phone number, we should add maxLength to input fields.

In CompanyStep

<ExpensiTextInput
     label={this.props.translate('common.phoneNumber')}
     maxLength={10}
     ...
     ...

<ExpensiTextInput
     label={this.props.translate('companyStep.taxIDNumber')}
     maxLength={9}
     ...
     ...

and in IdentityForm

<ExpensiTextInput
    label={`${translate('common.ssnLast4')}`}
    maxLength={4}

Here's what I'll add.

1. We should add keyboardType={CONST.KEYBOARD_TYPE.NUMERIC} to ssn, taxIDNumber.

2. For phone numbers, we'll use keyboardType={CONST.KEYBOARD_TYPE.PHONE_PAD} to allow special symbols.

3. It seems Str.isValidPhone only allows number values and not parenthesis. Hence based on requirements:
   a. Instead of using isValidPhone I'll use a custom regex to allow parenthesis and brackets.
   Example regex,

   isValidPhoneWithSpecialSymbols(phone) {
      return phone.test(`/^[+]*[(]{0,1}[0-9]{1,3}[)]{0,1}[-\s\./0-9]*$/g`);
   }
   

   b. Strip brackets etc. and then pass the value to Str.isValidPhone.

    isValidPhoneWithSpecialSymbols(phone) {
        return Str.isValidPhone(phone.replace(/[^\d\+]/g,""));
    }
   

cc - @puneetlath

[puneetlath]

Sounds good. @SofiedeVreese can you go ahead and hire @akshayasalvi in Upwork please?

[SofiedeVreese]

Hey @akshayasalvi looks like I haven't received a proposal from you in Upwork yet. Would you be able to submit your proposal there too so I can hire you via Upwork? Thanks! 🙌

[akshayasalvi]

@SofiedeVreese Done.

[SofiedeVreese]

Thank you @akshayasalvi I've just hired you through Upwork!

[SofiedeVreese]

PR close to being merged.

[SofiedeVreese]

PR merged, now waiting for deploy.

[SofiedeVreese]

Deployed!

[botify]

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

• https://github.com/Expensify/App/pulls/5531

If no regressions arise, payment will be issued on 2021-10-14. 🎊

[SofiedeVreese]

Issue paid out! As it was sitting on a Newsletter milestone, I've paid out $375.