[Paid] [$250] [Held requests] Back button on expense report reopens same report after returning from Hold page

[lanitochka17]

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: 9.0.19-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify app
2. Go to workspace chat
3. Submit two expenses
4. Go to expense report
5. Long tap on the expense preview
6. Select Hold
7. Tap Back button
8. Tap Back button again

Expected Result:

App will return to main chat when tapping back button on expense report after returning from hold expense page

Actual Result:

App returns to the same expense report when tapping back button on expense report after returning from hold expense page

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/71c53d4a-4d0d-4768-bac7-6f7f8993aa86

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cb01ad1f22f574aa
• Upwork Job ID: 1823810865100514650
• Last Price Increase: 2024-08-14
• Automatic offers:
• ZhenjaHorbach | Reviewer | 103543616
• nkdengineer | Contributor | 103543617

Issue Owner

Current Issue Owner: @ZhenjaHorbach

[melvin-bot[bot]]

Triggered auto assignment to @OfstadC (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@OfstadC FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-08-12 20:11:40 UTC.

Proposal

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

Back button on expense report reopens same report after returning from Hold page

What is the root cause of that problem?

We are passing a fallbackroute here backTo. This is the problem https://github.com/Expensify/App/blob/5d57ae84466a52dd25b12e9b0e76eda26281ae96/src/pages/iou/HoldReasonFormView.tsx#L42

When we press back button. This condition becomes true so instead of going back we are adding a route in the navigator https://github.com/Expensify/App/blob/8ddece233f57d2240fbf981c5fa07220b8a31a29/src/libs/Navigation/Navigation.ts#L222-L225

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

Remove the backTo from here https://github.com/Expensify/App/blob/5d57ae84466a52dd25b12e9b0e76eda26281ae96/src/pages/iou/HoldReasonFormView.tsx#L42

We should also handle the cleanup after removing backTo.

What alternative solutions did you explore? (Optional)

[nkdengineer]

Proposal

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

App returns to the same expense report when tapping back button on expense report after returning from hold expense page

What is the root cause of that problem?

We navigate back to the backTo param when clicking on the back button

https://github.com/Expensify/App/blob/9819f378685b4eea4f533624d8df7c8b60f58fc9/src/pages/iou/HoldReasonFormView.tsx#L42

Then since it's the first screen in RHP, we navigate with type as UP https://github.com/Expensify/App/blob/9819f378685b4eea4f533624d8df7c8b60f58fc9/src/libs/Navigation/Navigation.ts#L222-L224

When we are on the hold reason page the stack navigator is Bottom tab bar --> Chat report --> Expense report --> Hold reason page, and here the action type is REPLACE since we pass the type as UP. So the navigation will replace the last route with the target report which is the expense report and now the stack navigator becomes Bottom tab bar --> Chat report --> Expense report --> Expense report.

https://github.com/Expensify/App/blob/9819f378685b4eea4f533624d8df7c8b60f58fc9/src/libs/Navigation/linkTo/index.ts#L125-L127

That is the reason the expense report displays twice when we click on the back button after opening the hold reason page

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

Since the hold reason page can be opened from the expense report via context menu action or transaction thread report via the report detail page, we should keep the backTo param and fix this issue.

To fix this issue, we should check here if the top center panel is the target screen and the last route is the side navigator we will dismiss the modal and return early otherwise update action.type to REPLACE to replace the current route with the target screen.

if (type === CONST.NAVIGATION.TYPE.UP) {
    if (!areParamsDifferent && isSideModalNavigator(lastRoute?.name) && topmostCentralPaneRoute?.name === targetName) {
        dismissModal(navigation);
        return;
    }
    action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE;
}

https://github.com/Expensify/App/blob/9819f378685b4eea4f533624d8df7c8b60f58fc9/src/libs/Navigation/linkTo/index.ts#L125-L127

What alternative solutions did you explore? (Optional)

[OfstadC]

Was able to reproduce If selected twice, it goes back to chat, but first "back" shows the same report.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ZhenjaHorbach]

@Nodebrute @nkdengineer Thanks for your proposals

@Nodebrute I'm not certain it's a good idea to remove backTo, as we access the HoldReasonScreen from various screens, and we need this param to keep track of the previous screen (especially after reloading the page)

@nkdengineer Your proposal looks promising And I think it makes sense Especially considering that it will help fix potential similar issues So I don't mind going with this proposal

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[NikkiWines]

yep, agreed @nkdengineer's proposal looks good 👍

[melvin-bot[bot]]

📣 @ZhenjaHorbach 🎉 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[bot]]

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

[OfstadC]

Deployed 7 days ago - automation didn't work. Processing payments now

[OfstadC]

Payment Summary

• @ZhenjaHorbach paid $250 via Upwork
• @nkdengineer paid $250 via Upwork

[OfstadC]

@ZhenjaHorbach Any regression tests to propose here? 😃

[ZhenjaHorbach]

I will add regression tests today or tomorrow !

[ZhenjaHorbach]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [x] [@ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:

https://github.com/Expensify/App/pull/43086

• [x] [@ZhenjaHorbach] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/App/pull/43086/files#r1734155815

• [x] [@ZhenjaHorbach] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

NA

• [x] [@ZhenjaHorbach] Determine if we should create a regression test for this bug.
• [x] [@ZhenjaHorbach] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

• Launch App
• Go to any workspace chat
• Submit two expenses
• Go to expense report
• Long tap on the expense preview
• Select Hold
• Tap Back button
• Tap Back button again
• Verify that: App will return to main chat when tapping back button on expense report after returning from hold expense page

Do we agree 👍 or 👎

[OfstadC]

Thanks @ZhenjaHorbach !

https://github.com/Expensify/Expensify/issues/423933