[$500] Connect bank account - User can't continue / receives error if returns to entering account page and tap Continue

[izarutskaya]

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.3.94-0
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 staging.new.expensify.com or open Expensify App and login with any account
2. Go to existing workspace or create new one
3. Go to Workspace > Connect bank account
4. Tap 'Connect with Plaid' and select any bank (eq. Chase)
5. Follow the flow until 'Personal Information' page
6. Click 'Back' (<) button twice to return to the page for entering routing and account numbers
7. Click/Tap 'Continue'

Expected Result:

User goes to the page 'Company Information'

Actual Result:

User receives an error message and cannot continue

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

Bug6258972_1698815006327.BA-Chase-error-after-return-account-numbers-page.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011f6c8b5dad4aad8a
• Upwork Job ID: 1719693003927179264
• Last Price Increase: 2023-11-08
• Automatic offers:
• dukenv0307 | Contributor | 27590431

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011f6c8b5dad4aad8a

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[yh-0218]

Proposal

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

Connect bank account - User can't continue / receives error if returns to entering account page and tap Continue

What is the root cause of that problem?

Because we got validation about masked accountNumber here.

App/src/pages/ReimbursementAccount/BankAccountManualStep.js

Lines 40 to 44 in a31933a

	if (
	values.accountNumber &&
	!CONST.BANK_ACCOUNT.REGEX.US_ACCOUNT_NUMBER.test(values.accountNumber.trim()) &&
	!CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim())
	) {

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

We don't need to validate about masked accountNumber because that is saved after validation on step3.
so we need to change validation
We need to update like this.

if (
        values.accountNumber &&
        !shouldDisableInputs &&
        !CONST.BANK_ACCOUNT.REGEX.US_ACCOUNT_NUMBER.test(values.accountNumber.trim()) &&
        !CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim())
) {

What alternative solutions did you explore? (Optional)

We can didn't mask about account number

Screen.Recording.2023-11-01.at.3.44.51.PM.mov

[dukenv0307]

Proposal

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

1. User receives an error message and cannot continue if they enter a 16-digit account number, go all the way to the Personal information page and go back to edit.

2. There's also another problem: When starting the Connecting bank account flow, if we enter a masked account number like XXXX1234, it will still let us through

What is the root cause of that problem?

1. The back-end is always leaving 4 last digits and mask the rest, while the front-end is expecting account number longer than 13 digits to be masked differently here. So the front-end regex will throw error if a more-than-13-digits account number has 4 last digits unmasked and the rest masked.
2. Even in the initial bank account flow, we're allowing masked number to go through here. Masked number should only be allowed when coming back the bank account page, where the account number was already replaced by back-end with masked number.

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

1. Fix the front-end to match the masked account number rule of back-end, or fix back-end to align with front-end
2. Allow masked bank account number only if shouldDisableInputs is true (that means the bank account number is already saved and is replaced by BE by masked one). To do this, we can update this line to:

!(shouldDisableInputs && CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim()))

What alternative solutions did you explore? (Optional)

NA

Result

poc-resize.mov

[melvin-bot]

@CortneyOfstad, @sobitneupane Huh... This is 4 days overdue. Who can take care of this?

[CortneyOfstad]

@sobitneupane thoughts on the proposals above? TIA!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sobitneupane]

Sorry for the delay. Will review the proposals shortly.

[sobitneupane]

Thanks for the proposal everyone.

@dukenv0307 Thanks for the detailed explanation on root cause.

The back-end is always leaving 4 last digits and mask the rest, while the front-end is expecting account number longer than 13 digits to be masked differently here. So the front-end regex will throw error if a more-than-13-digits account number has 4 last digits unmasked and the rest masked.

As mentioned in the proposal, the format in which backend returns the account detail is different than expected in the frontend. So, to solve the issue we should either update frontend to expect only last 4 digits for masked account number. Or update backend to return account number with first 6 digit and last 4 digit if the length of account number is greater than 13.

Proposal from @dukenv0307 looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @sobitneupane Please request via NewDot manual requests for the Reviewer role ($500)

[lakchote]

@dukenv0307 proposal LGTM 👍

[melvin-bot]

📣 @dukenv0307 🎉 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 📖

[dukenv0307]

@sobitneupane @lakchote I want to double-confirm here

1. So, to solve the issue we should either update frontend to expect only last 4 digits for masked account number.
   Or
2. update backend to return account number with first 6 digit and last 4 digit if the length of account number is greater than 13.

Which option that we gonna choose?

[lakchote]

@dukenv0307 In my opinion, the backend is the source of truth. I would go with #1. @sobitneupane what's your opinion?

[dukenv0307]

@sobitneupane The PR is ready for review

[sobitneupane]

Yeah. Good to go with first option.

[melvin-bot]

This issue has not been updated in over 15 days. @CortneyOfstad, @lakchote, @sobitneupane, @dukenv0307 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[sobitneupane]

PR was deployed to production two weeks back. Ready for payment.

cc: @CortneyOfstad

[CortneyOfstad]

Payment Summary

• @dukenv0307 paid $500 via Upwork as contributor
• @sobitneupane to be paid $500 via NewDot as C+ (reviewer)

[sobitneupane]

Requested payment on newDot.

[JmillsExpensify]

$500 payment approved for @sobitneupane based on comment above.