Navigation - Repeating navigation after submitting expense and returning via device gesture

[vincdargento]

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: 9.1.6-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
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): [email protected]
Issue reported by: Applause Internal Team
Device used: iPhone 15 Pro Max / iOS 18.3
App Component: Money Requests

Action Performed:

1. Launch ND or hybrid app.
2. Go to workspace chat that has no unsettled expense.
3. Submit an expense to the workspace chat.
4. Tap on the expense preview to go to expense report.
5. Tap + > Create expense.
6. Submit another expense in the expense report.
7. Swipe to right to go back (iOS gesture) - Result 1.
8. Swipe to right to go back (iOS gesture) - Result 2.
9. Swipe to right to go back (iOS gesture) - Result 3.
10. Swipe to right to go back (iOS gesture) - Result 4.
11. Swipe to right to go back (iOS gesture) - Result 5.
12. Swipe to right to go back (iOS gesture) - Result 6.
13. Swipe to right to go back (iOS gesture) - Result 7.
14. Swipe to right to go back (iOS gesture) - Result 8.

Expected Result:

In Step 7, app will return to main workspace chat.
In Step 8, app will return to LHN.

Actual Result:

Result 1 (Step 7) - App returns to LHN.
Result 2 (Step 8) - App returns to expense report.
Result 3 (Step 9) - App returns to LHN.
Result 4 (Step 10) - App returns to expense report.
Result 5 (Step 11) - App returns to Create expense page.
Result 6 (Step 12) - App returns to expense report.
Result 7 (Step 13) - App returns to main workspace chat.
Result 8 (Step 14) - App returns to LHN.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @WojtekBoman

[melvin-bot]

Triggered auto assignment to @trjExpensify (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.

[trjExpensify]

@mountiny who would be good to look at this, and also is it worth it if we're making changes? 🤔

[mountiny]

@WojtekBoman @adamgrzybowski would you be able to look into this one?

[mountiny]

I think it is worth checking this one out

[WojtekBoman]

Hi, I've already looked into this and I think I've found the root cause of this problem.

It's caused by the race condition that appears when we're requesting money second time from the expense report.
In this case the app tries to navigate to the report twice from:

1. the scrollToBottomForCurrentUserAction method from src/pages/home/report/ReportActionsList.tsx it's triggered and Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID)); is called.
2. the requestMoney function from src/libs/actions/IOU.ts tries to dismiss the rhp and then navigate, but scrollToBottomForCurrentUserAction is called earlier. As a result, the RHP is not dismissed and we navigate to the same report second time.

I'm wondering if Navigation.navigate should be called in this case from scrollToBottomForCurrentUserAction. This issue appears only on native platforms as this method is not called on web.

@rinej Could you take a look at it? I see that you worked on scrollToBottomForCurrentUserAction and know how it should work. Should this method be called in this case? I'd like to fix it properly without causing new bugs :D

[trjExpensify]

Excellent. @WojtekBoman, I've assigned you for now. 👍

[mountiny]

Raised in here as well for a faster resolution

[trjExpensify]

What are the next steps here?

[WojtekBoman]

I'd like to get some feedback about the scrollToBottomForCurrentUserAction method. This info will be helpful to fix the issue properly.

cc: @rinej
Could you take a look at the question above when you have some time? I'd like to make sure how this method is supposed to work :)

[mountiny]

Bumped the thread, Tomasz is on a sick leave

[TMisiukiewicz]

I just took a look on this since @rinej is on sick leave but I am not very much into details. Looks like the flow works properly when I don't call scrollToBottomForCurrentUserAction so I think it shouldn't be called. Also it is not very clear to me why scroll function is responsible for navigating at all 🤔

[rinej]

Hello, sorry for the delayed response, I was out of the office.

Based on my investigation, the scrollToBottomForCurrentUserAction should only be triggered when the user receives a message via Pusher. It doesn’t seem necessary to trigger the navigation after requesting money action made by the user — I couldn’t find a valid reason for that (though there may be some edge cases I’m not currently aware of).

I’ve reviewed the issue today and I agree with @WojtekBoman proposed approach, I think that fixing the race condition is a good way to resolve this issue 👍

[WojtekBoman]

@rinej @TMisiukiewicz Thanks for your feedback!
I'll try to figure out a good solution for this issue on Monday

[melvin-bot]

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

[melvin-bot]

@trjExpensify @mountiny @WojtekBoman this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[WojtekBoman]

I'm currently involved in Improve Desktop Navigation topic, someone from SWM will take it over

[trjExpensify]

Nice, any idea who yet? Thanks!

[mountiny]

@adamgrzybowski is back :)

[WojtekBoman]

I will work on that :)

[mountiny]

🎉

[melvin-bot]

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

[mountiny]

@WojtekBoman what is your eta for a pr?

[WojtekBoman]

It should be ready by the end of the week

[WojtekBoman]

I believe I know how to fix this, but I would wait for that PR that is waiting to be merged as its logic is closely related to the code I need to touch to fix this issue

cc @mountiny

[mountiny]

Merging, lets also hope its not going to be reverted due to some blockers

[mountiny]

I think we might be good to go here, nobody external to pay on this issue