App: Routing number error does not drop you back into connect manually flow

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:

Navigate to /bank-account/
Click “Connect manually”
Input an invalid routing number, i.e. 12312312344
Input any bank number, i.e. 123
Click “Save & continue” button

Note: The incorrect behaviour only happens the first time we get the error, to replicate it again, reload the page.

Expected Result:

See the loading transition, show again the same form highlighting the error in the routing number

Actual Result:

After the backend validation fails, the user is taken back to the step before choosing “Connect manually”

https://user-images.githubusercontent.com/87341702/134405155-5164d428-2e20-40ad-bd96-18b2d9c05ff1.mov

Workaround:

The user can just click again “Connect manually” and proceed.

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

View all open jobs on GitHub

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 17 (12 by maintainers)

Most upvoted comments

It was a regression caused by this code:

https://github.com/Expensify/App/blob/main/src/libs/actions/BankAccounts.js#L648-L657

This was introduced in this PR

Basically it causes the BankAccountStep component to be created before we update the withdrawal account data here. At this point achData is equal to {subStep: 'manual'}, which in the past made it work.

When we create the component BankAccountStep before that last update, subStep in Onyx is null when it gets copied in the constructor.

Reverting this change to:

    API.BankAccount_SetupWithdrawal(newACHData)
        .then((response) => {
            Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {loading: false, achData: {...newACHData}});

Fixes the problem with the bug bouncing the user back to the previous view, and the bug described here doesn’t seem to be happening anymore. Maybe it got fix some other way? fixed in Onyx? fixed by constantly saving the draft values? 🤷

How would we prefer to proceed?

Revert as I’m pointing out here
Use my PR about reading the subStep directly form Onyx always?
Both

I would choose both, because I don’t see any value in the complexity introduced here anymore, so I would revert that. I also still suggest reading the subStep directly from Onyx and avoid double fetching.

But if we wanted to go with the simplest solution, it would be just reverting!

aldo-expensify on Sep 10, 2021

Thanks @Jag96 , I’ll revert that part and test that everything is working fine then!

aldo-expensify on Sep 10, 2021

@aldo-expensify I think you hit the nail on the head w/ the investigation.

The reason for that change was to fix an issue where Onyx.merge was overwriting data unexpectedly, more info here. The changes in that PR were to enable us to save previously entered account/routing numbers (issue), but most of that functionality was replaced by https://github.com/Expensify/App/pull/5114 recently.

If you revert the code in question and you can successfully pass the tests from https://github.com/Expensify/App/pull/5114 and https://github.com/Expensify/App/pull/4477 then I think it’s ok to revert.

Jag96 on Sep 10, 2021

Thanks for looking into that 🙇

I agree we should still make subStep the single source of truth as you’ve called out. That seems like a better solution long term to me.

Looked at the code you mention, but I’m not too familiar with why those changes were made. Maybe we can get @Jag96 to weigh in before proceeding?

marcaaron on Sep 10, 2021