[Hold for payment: 2024-09-20] [$250] Track expense - Clicking back navigates user back to the share with my accountant flow

[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.9-1 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 Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Go to your chat and create a track expense
3. Click on "Share with my accountant" on the actionable whisper for the track expense
4. Go with the flow until the end
5. Click on the browser back button

Expected Result:

App should navigate user back to the previously opened chat report

Actual Result:

App navigates user back to the "Share with my accountant" flow and user can go through the previously done flow again

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] 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/87e6a369-1fdc-4cd0-a55e-2dac469c459d

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b38481aa14324c79
• Upwork Job ID: 1815753645056507384
• Last Price Increase: 2024-08-27
• Automatic offers:
• c3024 | Reviewer | 103784904
• shubham1206agra | Contributor | 103784908

Issue Owner

Current Issue Owner: @c3024

[melvin-bot[bot]]

Triggered auto assignment to @dylanexpensify (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]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@dylanexpensify 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

[c3024]

Proposal

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

Clicking back on clicking the browser back button after completing the Share with my accountant flow opens the Share with accountant flow RHP

What is the root cause of that problem?

When we complete the share to my accountant flow, we open the invite members page here https://github.com/Expensify/App/blob/c2ded1dc91252803795948fa97888e420b0e648f/src/libs/actions/IOU.ts#L3807-L3811 but we cannot ensure this exact order of dismissing modal and then opening the invite room page. This must be causing a mismatch between device history and the navigation state.

So, when we click back at the invite page (or after completing inviting a member) the navigation state does not change as expected.

This is how the navigation state changes when clicking back on the invite page (this will be similar in the flow after completing inviting a member as well)

When we click back it removes the RHP and a report (WS chat here) and adds a RHP. The correct thing to happen here is to only remove the RHP and add the invite room RHP.

I think this is due to browser history mismatch with the navigation state because we cannot ensure the dismissModal and navigate order actions correctly in the trackExpense flow.

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

I think we should have a promise based dismissModal for situations like these. We can add a state listener to identify state change of dismissModal action and resolve the promise after the dismissModal navigation is complete.

Here is the sample test branch with detailed changes.

What alternative solutions did you explore? (Optional)

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[dylanexpensify]

reviewed

[Zakpak0]

Proposal

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

When using the browser's back button to navigate after sharing an expense there share modal needs to be closed.

What is the root cause of that problem?

There is no action being made to close the modal after sharing the expense and navigating to the next page.

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

We can dismiss the modal, then we need to wait for it to be removed from our navigation state before we move forward. Then we can navigate to our workspace and finally open the invite panel. src/libs/actions/IOU.ts

    switch (action) {
        case CONST.IOU.ACTION.SHARE: {
            Navigation.dismissModal(linkedTrackedExpenseReportID)
            Navigation.isNavigationReady().then(() => { 
                Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(activeReportID ?? "-1"))
                Navigation.isNavigationReady().then(() => {
                Navigation.navigate(ROUTES.ROOM_INVITE.getRoute(activeReportID ?? '-1', CONST.IOU.SHARE.ROLE.ACCOUNTANT));
                })      
            })
            break
        }
        default: {
            Navigation.dismissModal(activeReportID);
        }
    }

Additionally I noticed the same issue with the "Invite to workspace" modal. It will cause the same problem so we should dismiss that as well. src/pages/RoomInvitePage.tsx

        Navigation.dismissModal(reportID ?? "-1")
        Navigation.navigate(backRoute);

Here is a test branch with my changes

What alternative solutions did you explore? (Optional)

N/A

[c3024]

Thanks for your proposal @Zakpak0 .

When the right hand panel opens with the Invite Page, the Centre Pane should show the workspace chat. I tested your branch. It shows the selfDM chat in the Center Pane when the Invite Page appears in the Share to accountant flow.

Could you fix that?

[Zakpak0]

Proposal

Updated No problem @c3024 , for that I needed to change the order of operations. I updated the branch with the change as well

[c3024]

Browser back button on Invite Page in Right Hand Panel (RHP) takes the user to Share RHP with the latest branch. This should not happen.

https://github.com/user-attachments/assets/c5849b75-350c-4404-8dae-47caf84d0164

[Zakpak0]

Proposal

Updated @c3024 I updated it so that it handles pressing back from the RHP back button. One thing I noticed though was that you must directly navigate to the workspace before you can handle the navigation to the RHP invite overlay. This RHP navigation route being independent of the report we have open seems to be intentional? Another thing is we must wait for the navigation state to be updated before we move on to our next navigation command. Otherwise we won't be editing the state correctly.

https://github.com/user-attachments/assets/1a96c67b-f054-4d25-a369-0446b7ca16d8

[melvin-bot[bot]]

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

[c3024]

@Zakpak0 That branch works, but it seems more like a workaround to delay navigation to the room invite page. Navigation is ready all the time in this case. isNavigationReady is always true once the NavigationContainer is mounted. So, checking if it is ready only delays the navigation to the invite page slightly because it’s being done in the .then chained to the isNavigationReady promise.

I think the current bug is due to navigating to the room invite page before dismissModal is fully complete. We need something like a dismissModalWithPromise, as seen in this branch, to ensure that navigation to the room invite page happens only after dismissModal is fully complete. This also allows for reuse of this new function in other cases where navigation might occur before dismissModal is complete.

[melvin-bot[bot]]

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

[Zakpak0]

@c3024 thanks, this makes a lot of sense. I'm going to check this out right now!

[melvin-bot[bot]]

@dylanexpensify @c3024 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

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

[c3024]

Waiting for proposals.

[melvin-bot[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[bot]]

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

[c3024]

Waiting for proposals!

[melvin-bot[bot]]

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

[c3024]

Waiting for proposals.

[melvin-bot[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[bot]]

@dylanexpensify @c3024 this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

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

[c3024]

Waiting for proposals!

[melvin-bot[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[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@dylanexpensify, @c3024 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[wildan-m]

Proposal

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

When perform Track expense clicking back navigates user back to the share with my accountant flow

What is the root cause of that problem?

The activeReportID in this code points to the destination report, not the previous one.

https://github.com/Expensify/App/blob/d5cfecfc53a2abf926a1dd9be8bc72c0b373ff81/src/libs/actions/IOU.ts#L3815

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

I think share actions will only occur in selfDM. If that's the case, using linkedTrackedExpenseReportID is appropriate. Another option is to use ReportUtils.findSelfDMReportID() or include a new optional parameter prevActiveReportID in trackExpense report.

Change:

https://github.com/Expensify/App/blob/d5cfecfc53a2abf926a1dd9be8bc72c0b373ff81/src/libs/actions/IOU.ts#L3815-L3819

To

    const isShareAction = action === CONST.IOU.ACTION.SHARE;
    const modalReportID = isShareAction ? linkedTrackedExpenseReportID : (isSearchTopmostCentralPane() ? undefined : activeReportID);

    Navigation.dismissModal(modalReportID);

    if (isShareAction) {
        const inviteRoute = ROUTES.ROOM_INVITE.getRoute(activeReportID ?? '-1', CONST.IOU.SHARE.ROLE.ACCOUNTANT);
        Navigation.navigate(inviteRoute);
    }    

What alternative solutions did you explore? (Optional)

N/A

[WojtekBoman]

We should wrap the navigation action with Navigation.setNavigationActionToMicrotaskQueue:

    Navigation.dismissModal(isSearchTopmostCentralPane() ? undefined : activeReportID);
    if (action === CONST.IOU.ACTION.SHARE) {
        Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.navigate(ROUTES.ROOM_INVITE.getRoute(activeReportID ?? '-1', CONST.IOU.SHARE.ROLE.ACCOUNTANT)));
    }

Thanks to this, we will be sure that the first modal has already been closed and we can perform the next navigation action.

Here's the video with the fixed version:

https://github.com/user-attachments/assets/b2cedb6e-1360-4bf0-9447-ef6509a0a09f

[shubham1206agra]

Proposal

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

Track expense - Clicking back navigates user back to the share with my accountant flow

What is the root cause of that problem?

In https://github.com/Expensify/App/blob/d5cfecfc53a2abf926a1dd9be8bc72c0b373ff81/src/libs/actions/IOU.ts#L3815-L3819

This is happening due to race condition between dismissModal and navigate since they execute almost parallel. This makes navigate execute before dismissModal execution finishes.

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

Replace https://github.com/Expensify/App/blob/d5cfecfc53a2abf926a1dd9be8bc72c0b373ff81/src/libs/actions/IOU.ts#L3815-L3819 with

Navigation.dismissModal(isSearchTopmostCentralPane() ? undefined : activeReportID);
if (action === CONST.IOU.ACTION.SHARE) {
    Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.navigate(ROUTES.ROOM_INVITE.getRoute(activeReportID ?? '-1', CONST.IOU.SHARE.ROLE.ACCOUNTANT)));
}

to fix the race condition

What alternative solutions did you explore? (Optional)

NA

[shubham1206agra]

@c3024 I asked @WojtekBoman to look into this here https://expensify.slack.com/archives/C05LX9D6E07/p1724738243842929?thread_ts=1724226268.839299&cid=C05LX9D6E07

[melvin-bot[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[bot]]

@dylanexpensify, @c3024 10 days overdue. Is anyone even seeing these? Hello?

[c3024]

I'll update.

[c3024]

I am not sure if the task queued in the microtask queue always gets executed after the dismiss modal action is complete. The dismiss modal action might take a bit longer, and since the navigate action is in a different promise chain, it might get executed earlier.

Ideally, we should chain the navigate action to a dismiss modal action function returning a promise, like this.

However, in practice, the delay caused by adding the navigate action to the microtask queue seems to be sufficient for the dismiss modal action to complete. Therefore, the added verbosity and complexity of creating another dismissModal function that returns a promise might not be worth it.

@WojtekBoman and @shubham1206agra's proposals here and here LGTM!

🎀 👀 🎀 C+ Reviewed

[melvin-bot[bot]]

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

[cead22]

@WojtekBoman and @shubham1206agra's proposals here and here LGTM!

@c3024 sorry, can you clarify which one you selected?

[c3024]

Both proposals are the same. @WojtekBoman is an expert agency member. I think @shubham1206agra asked him for the fix for this on Slack here and he posted a proposal in the standard format with the same solution. So, I guess Shubham will work on it if he is assigned.

[melvin-bot[bot]]

📣 @c3024 🎉 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]]

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

[melvin-bot[bot]]

@cead22, @shubham1206agra, @dylanexpensify, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[shubham1206agra]

I will raise the PR toady.

[c3024]

Automation failed here. Deployed to production on 13-Sep. I think this should be on HOLD for payment till 20-Sep or 21-Sep.

cc: @dylanexpensify

[dylanexpensify]

Payment summary:

Contributor: @shubham1206agra $250 Contributor+: @c3024 $250

Please apply/request!