[$500] Bank Account - Navigation Problem in "Get assistance" page after a reload

[izarutskaya]

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

---

Version Number: v1.4.3-6
Reproducible in staging?: Y
Reproducible in production?: N
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:
Slack conversation: @

Action Performed:

1. Go to settings > Workspace > choose a works space(currency should be USD)
2. Bank account > Connect Manually
3. Click on the Question mark at the top.
4. Reload the Get Assistance page
5. Click on the back button.

Expected Result:

page navigates to the "Connect bank account" page

Actual Result:

page navigates to the "Workspace" page

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

Bug6290112_1700900010646.test18_Navigation.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a9f115ad31668583
• Upwork Job ID: 1728910206074769408
• Last Price Increase: 2023-11-26
• Automatic offers:
• ntdiary | Reviewer | 27890114
• tienifr | Contributor | 27920721

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @slafortune (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 - @ntdiary (External)

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @cead22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot]

📣 @github-actions[bot]! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[izarutskaya]

Production - different behavior.

Recording.1938.mp4

[abzokhattab]

Proposal

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

Back from assistance page after reload doesnt go back to the connect bank page

What is the root cause of that problem?

the back navigation is set to the workspace settings url:

App/src/pages/GetAssistancePage.js

Line 85 in d521dd6

onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WORKSPACES)}

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

since the url of the get assistance page doesnt contain the workspace id

https://staging.new.expensify.com/get-assistance/WorkspaceBankAccount

we need to add backTo param when clicking on the get assistance url to the save the prev route and go back to it on clicking the back button

to do that first we need to modify the GET_ASSISTANCE route to accept the back to param:
getRoute: (taskID: string, backTo?: string) => getUrlWithBackToParam(get-assistance/${taskID}, backTo),

then here we need to change it to:

                                onPress={singleExecution(waitForNavigate(() => Navigation.navigate(ROUTES.GET_ASSISTANCE.getRoute(guidesCallTaskID, activeRoute))))}

where the activeRoute is const activeRoute = Navigation.getActiveRoute();

and finally inside the getAssistancePage:

we create a handleBack function:

    const backTo = lodashGet(props.route, 'params.backTo', '');
   const handleNavigateBack = () => {
        if (!_.isEmpty(backTo)) {
            Navigation.goBack(backTo);
        } else {
            Navigation.goBack(ROUTES.SETTINGS_WORKSPACES);
        }
    };

and call this function here:

                onBackButtonPress={handleNavigateBack}

POC:

Screen.Recording.2023-11-27.at.10.01.03.AM.mov

[tienifr]

Proposal

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

Navigate back from Get assistance page leads to Workspace list page after reloading while it should be Connect bank account page.

What is the root cause of that problem?

When we get to the GetAssistancePage we do not save the history of previous screens
And since it is used in many other places, if you tried to go back, we would get to the fallback screen, which is Workspace list page:

App/src/pages/GetAssistancePage.js

Line 85 in d985a0e

onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WORKSPACES)}

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

Because GetAssistancePage is wrapped inside HeaderWithBackButton and it is used in a lot of places, we should add backTo param as the route of current page that uses HeaderWithBackButton (i.e. the active route).

1. Update getRoute in ROUTES to allow backTo param:

GET_ASSISTANCE: {
    route: 'get-assistance/:taskID',
    getRoute: (taskID: string, backTo: string) => getUrlWithBackToParam(`get-assistance/${taskID}`, backTo),
},

2. Update HeaderWithBackButton to pass backTo to GetAssistancePage as the current active route here:

Navigation.navigate(ROUTES.GET_ASSISTANCE.getRoute(
    guidesCallTaskID,
    Navigation.getActiveRoute()
))

3. In GetAssistancePage, we retrieve the backTo param and set it as fallback URL for goBack function here:

const navigateBackTo = lodashGet(props.route, 'params.backTo', ROUTES.SETTINGS_WORKSPACES);

onBackButtonPress={() => Navigation.goBack(navigateBackTo)}

What alternative solutions did you explore? (Optional)

NA

[mountiny]

I dont think this is a deploy blocker, fwiw it seems to work better than production 😂

Demoting

[tienifr]

@abzokhattab If you have major changes, please update that in a separated proposal. Your initial proposal did not cover the main ideas, also implementation details can always be handled in the PR.

[abzokhattab]

Hi @tienifr , I disagree with you the main idea was already mentioned in the first version (which is the same as yours). in the second version, i added the code changes for the proposed solution.

There is no breaking change in the proposal, both versions have the same RCA and solution approach but the second one i added the code changes.

[melvin-bot]

📣 @ntdiary 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @allroundexperts Please request via NewDot manual requests for the Contributor role ($500)

[cead22]

Should I have unassigned @ntdiary before assigning @allroundexperts?

[ntdiary]

Sure, I have just unassigned myself. 😂

[allroundexperts]

Thanks for the proposals everyone!

@abzokhattab Your initial proposal was too generic and lacked any details on how you were to implement the persistence of previous route. While concrete implementation details are not necessarily required for a proposal to be accepted, a general high level explanation of the how and why is a must.

@tienifr's proposal explains the root cause and proposes a clear solution as well. I think we should go with them.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @cead22 is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[abzokhattab]

Thanks @allroundexperts for your response ..

"how you implement the persistence of the previous route" --> i mentioned the high-level solution by stating that we can save the current URL in the backTo param and navigate to this URL when clicking on back ..
I kept it brief initially because the backTo is already used widely in our project, so I think its quite understandable when i mention that we need to the the backTo param and i already explained how it works

again i still see that both versions go in the same direction and i didn't introduce a breaking change in my proposal, i just added the changes.

Thanks

[cead22]

@abzokhattab thanks for the note, I definitely understand where you're coming from, since we try to balance succinctness with clarity, and I think in this case a little more detail would've made your proposal more immediately obvious as to what it was doing.

I hope this doesn't discourage you from making other proposals, but in order to move this forward, I'm going to assign @tienifr to this issue

[melvin-bot]

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

[melvin-bot]

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

[tienifr]

PR ready for review #32466.

[melvin-bot]

This issue has not been updated in over 15 days. @cead22, @slafortune, @allroundexperts, @tienifr 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!

[tienifr]

@slafortune I think we're good to proceed with payments here, the PR was deployed to production 3 weeks ago

[tienifr]

@slafortune I think we're good to proceed with payments here, the PR was deployed to production 3 weeks ago

@slafortune friendly bump on this, thanks!

[slafortune]

I am so sorry for the delay with this @tienifr ! I hadn't noticed this once it moved to monthly and didn't have the payment date in the title 😭

Paid!

[slafortune]

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

[JmillsExpensify]

$500 approved for @allroundexperts based on comment above.