[HELD #26538] [$1000] Web - Can access company info in bank account page without typing routing and account number

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

IMPORTANT: If there are bank account details saved already. Please do start over on the connect bank account page.

1. Settings -> workspaces -> choose any workspace
2. Go to bank account
3. If there is start over option, click on it and start over.
4. Now on the URL, ../bankaccount/new?...
   replace 'new' with 'company'
5. Notice that company page opens, & on clicking back notice that routing and account numbers are empty.
   However, even if 'new' is changed to 'personal-information' , it doesn't show Personal information page.

Expected Result:

Even when changing the url parameter 'new' to 'company' , it should redirect to 'new' i.e connect bank account page.

Actual Result:

After changing url parameter, company information page opens leaving the routing and account number empty.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.69.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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-09-11.at.7.03.41.PM.mov

Recording.4453.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ashimsharma10
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694439235983099

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c9f6719a38b4191b
• Upwork Job ID: 1702031275629907968
• Last Price Increase: 2023-10-27

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01c9f6719a38b4191b

[melvin-bot]

Triggered auto assignment to @kevinksullivan (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 @flaviadefaria (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[parasharrajat]

I am wondering do we have to worry about this issue. I am sure that we discussed this many times in the past but I don't remember the correct behaviour.

Going to open a slack discussion.

[akinwale]

Proposal

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

It is possible to access company info and other steps with a direct URL without providing a routing or account number in the connect bank account flow.

What is the root cause of that problem?

There are no checks in subsequent steps of the ReimbursementAccountPage component to verify whether or not the routing and account numbers have been provided in Step 1.

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

We are obtaining the currentStep in the componentDidUpdate method. With this, we can perform some new checks to see if the flow should be redirected to the first step.

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.js

Line 150 in d68263a

const currentStep = lodashGet(this.props.reimbursementAccount, 'achData.currentStep') || CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT;

Add the following lines in the componentDidUpdate method after obtaining the currentStep to check the values in the reimbursementAccountDraft prop and redirect accordingly if neither the account number nor the routing number has been provided. In addition to null / undefined validation, we can also add empty string checks on the values.

if (this.state.hasACHDataBeenLoaded) {
    const {reimbursementAccountDraft} = this.props;
    const {accountNumber, routingNumber} = reimbursementAccountDraft;
    if ((!accountNumber || !routingNumber) && currentStep !== CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT) {
        BankAccounts.goToWithdrawalAccountSetupStep(CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT, {subStep: CONST.BANK_ACCOUNT.SUBSTEP.MANUAL});
        return;
    }
}

What alternative solutions did you explore? (Optional)

None.

[dukenv0307]

Proposal

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

Web - Can access company info in bank account page without typing routing and account number

What is the root cause of that problem?

We pass the stepToOpen open that is gotten in the parm to openReimbursementAccountPage and then the BE returns the currentStep in data as the stepToOpen param. So the next step can be accessed without complete the previous step

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.js

Line 272 in d68263a

BankAccounts.openReimbursementAccountPage(stepToOpen, subStep, ignoreLocalCurrentStep ? '' : localCurrentStep);

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

I think we should pass the last step which is completed if the stepToOpen is the next step of the last step completed

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.js

Line 272 in d68263a

BankAccounts.openReimbursementAccountPage(stepToOpen, subStep, ignoreLocalCurrentStep ? '' : localCurrentStep);

1. BE should return an extra field in achData of reimbursementAccount like lastStepCompleted. This field will be the last step that the user completed

2. Create a function to compare the stepToOpen and lastStepCompleted. If the stepToOpen is the step immediately after the lastStepCompleted or before we pass it into openReimbursementAccountPage API otherwise pass the next step of lastStepCompleted to the API

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.js

Line 272 in d68263a

BankAccounts.openReimbursementAccountPage(stepToOpen, subStep, ignoreLocalCurrentStep ? '' : localCurrentStep);

What alternative solutions did you explore? (Optional)

[flaviadefaria]

@kevinksullivan Looks like I was incorrectly double assigned here by the external label. We are discussing fixing this here. In the meantime, going to un-assign as this doesn't need two BZ assignees.

[kevinksullivan]

sounds good 👍 . @parasharrajat let us know what you think about the proposals above

[parasharrajat]

@kevinksullivan Still waiting on the slack discussion, feel free to push it forward. expensify.slack.com/archives/C049HHMV9SM/p1694439235983099

[parasharrajat]

Based on the Slack thread, it has been decided to solve this issue. However we are interested in a generic solution that targets the scenario, not the screen. The same scenario can be present on many screens.

How can we create a solution that allows us to solve this problem on multiple screens?

[akinwale]

Based on the Slack thread, it has been decided to solve this issue. However we are interested in a generic solution that targets the scenario, not the screen. The same scenario can be present on many screens.

How can we create a solution that allows us to solve this problem on multiple screens?

The solution for this issue in particular would be fundamentally different from the solution to the other cases due to the implementation approach.

The connect bank account flow screens are all rendered by a single component, ReimbursementAccountPage, using conditional checks. In the other related issue, the different screens in the flow are being displayed after navigating to them using the Navigator.

If the goal is to achieve a unified implementation with a one-size-fits-all solution, the connect bank account flow would have to be refactored prior.

[dukenv0307]

@parasharrajat I think the bank account flow is different from the money request flow. The money request flow has already the logic to reset to the first page. The bug you mention above is only a special case of this flow.

[parasharrajat]

I have a crazy idea. What if creating a route guard that works at react-navigation screen level or we create a wrapper component like ScreenWrapper whose job will be to verify some details on page load and take action when fails otherwise let the page be as it is?

[melvin-bot]

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

[melvin-bot]

@kevinksullivan, @parasharrajat Whoops! This issue is 2 days overdue. Let's get this updated quick!

[parasharrajat]

There is no new update on the last Slack post. I looked into the mentioned PR and it seems are refactoring the step pages into a single page so that the intermediate page is not navigable.

It is being attempted at #26538. So we are kind of holding on to that. Similiar pattern can be adopted here.

[melvin-bot]

@kevinksullivan, @parasharrajat Eep! 4 days overdue now. Issues have feelings too...

[kevinksullivan]

got it, so I'll hold on that issue then @parasharrajat ?

[melvin-bot]

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

[kevinksullivan]

held

[melvin-bot]

@kevinksullivan, @parasharrajat Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[kevinksullivan]

held

[kevinksullivan]

held

[melvin-bot]

This issue has not been updated in over 15 days. @kevinksullivan, @parasharrajat 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!

[kevinksullivan]

same

[kevinksullivan]

can't reproduce this one anymore.