[HOLD for payment 2023-08-28] [HOLD #20908] [$1000] [mWeb] Redirects to Assign task instead of LHN on press back button of device

[kavimuru]

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:

1. click + icon
2. select Assign task option
3. enter title and click on next button
4. select Assignee and share somewhere
5. click on confirm task button to finish creating the task
6. press back button of device

Expected Result:

should navigate to LHN

Actual Result:

navigates to Assign task screen

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.26-1
Reproducible in staging?: Needs reproduction (For me it takes to LHN)
Reproducible in production?: Needs reproduction
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_20230619_145614_Chrome.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ab0e974992d7d84a
• Upwork Job ID: 1672317686491295744
• Last Price Increase: 2023-07-07

[melvin-bot]

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

[joekaufmanexpensify]

Clarifying if this bug is dev only

[gadhiyamanan]

@joekaufmanexpensify I think it is reproducible in staging and production but not sure because I have no access to task in staging or production. i tested on dev and reproducible in dev

[joekaufmanexpensify]

I can reproduce this. And then see on the native android app, clicking the device back button after you finish creating the task brings you back to LHN.

[melvin-bot]

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

[melvin-bot]

Current assignee @joekaufmanexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[ygshbht]

Proposal

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

The application navigates back to the "Assign task" screen when the back button of the device or browser is pressed, even after a task is successfully assigned. This occurs both on mobile and desktop platforms. The expected behavior in mobiles is for the application to navigate to the Left Hand Navigation (LHN) screen.

What is the root cause of that problem?

This could be due to how the navigation stack or the webpage history is being managed. The "Assign task" screen might not be properly removed from the navigation history or stack after a task has been assigned, causing the application to navigate back to it when the device back button is pressed or the browser's back button is clicked.

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

A potential solution to this issue could be ensuring the "Assign task" screen is properly removed from the navigation history or stack once a task is successfully assigned. For web applications, manipulating the browser's history object can effectively achieve this – for instance, using the replaceState() method instead of pushState() can overwrite the current history entry instead of creating a new one. For a mobile application, invoking a navigation.popToTop() or a specific navigation.goBack() method might be effective.

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[alitoshmatov]

Proposal

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

[mWeb] Redirects to Assign task instead of LHN on press back button of device

What is the root cause of that problem?

In the case of assign task there is two modals that is recorded in stack but when we create task we call dismisModal which removes one modal from stack history

App/src/libs/actions/Task.js

Line 184 in 116abf5

Navigation.dismissModal(optimisticTaskReport.reportID);

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

We can modify dismissModal to specify how many times it should pop the stack, like for this case we know that task creation is using two modals. We can pass second argument to dismissModal and based on this argument we can call

            navigationRef.current.dispatch({...StackActions.pop(), target: rootState.key});

multiple times.

What alternative solutions did you explore? (Optional)

[s77rt]

@ygshbht Thanks for the proposal. Although your RCA could be correct the writing is questionable, please re-write the RCA in clear statements describing the exact root cause instead of providing assumptions. Then please write a proper solution that actually reflects our code base.

[s77rt]

@alitoshmatov Thanks for the proposal. Can you please elaborate how we have two modals in this case?

BTW, we don't want to call navigationRef.current.dispatch({...StackActions.pop(), target: rootState.key}); this will not navigate to the task report after being created.

[ygshbht]

@s77rt I appreciate your feedback. Let me present a clearer version of the Root Cause Analysis and the Proposed Solution:

Root Cause Analysis:

The application redirects to the "Assign task" screen when the back button is pressed, due to the "Assign task" screen persisting in the navigation stack.

Proposed Solution:

Currently, the code is set up to remove only one screen (the "Confirm task" screen) from the navigation stack after task creation. However, we should adjust this to also remove the "Assign task" screen. To accomplish this, we can utilize a function named dismissNumModals, which can remove a specific number of screens from the navigation stack:

function dismissNumModals(num=1, targetReportID){
    const rootState = navigationRef.getRootState();

    Array(num).fill(0).forEach(repetition=>{
        navigationRef.current.dispatch({...StackActions.pop(), target: rootState.key});
    })

    if (targetReportID && targetReportID !== getTopmostReportId(rootState)) {
        const state = getStateFromPath(ROUTES.getReportRoute(targetReportID));

        const action = getActionFromState(state, linkingConfig.config);
        action.type = 'NAVIGATE';
        navigationRef.current.dispatch(action);
    }
}

By using this function, we will be able to remove both the "Confirm task" and "Assign task" screens, which should result in a proper redirect upon pressing the back button. I am confident that this solution will effectively address the issue. You are welcome to test this approach to verify its effectiveness.

@ygshbht Thanks for the proposal. Although your RCA could be correct the writing is questionable, please re-write the RCA in clear statements describing the exact root cause instead of providing assumptions. Then please write a proper solution that actually reflects our code base.

[s77rt]

@ygshbht Thanks, that's much better. Can you explain why this bug only occurs on Web (not reproducible on Native)?

[ygshbht]

@s77rt Thank you for your query.

The observed discrepancy in behavior stems from the divergent navigation mechanisms in web browsers and native apps.

In web browsers, activating the back button on the browser or invoking the back functionality propels the user to the preceding URL in the browser history stack. Consequently, as the "Assign task" screen has a corresponding state in the stack, pressing the back button will navigate back to this screen.

Contrastingly, native apps generally lack a universal "back" functionality akin to that in web browsers, excluding the back button present in the app's user interface or the system back button on Android devices. These buttons typically adhere to the application's navigation stack rather than the browser's history stack.

Similarly, when using the in-app UI to navigate backwards on the web app, the issue is not present, echoing the behavior of native apps. This again emphasizes that the issue lies in how the web browser handles the back action in contrast to the in-app UI or native apps.

Therefore, it's this variation in the navigation model that prevents the issue from manifesting on native platforms.

[alitoshmatov]

@s77rt
I meant adding this to the begining of dismissModal and not change other parts, and it does navigate to report.
for (let i = 0; i < additionalPopCount; i++) { navigationRef.current.dispatch({...StackActions.pop(), target: rootState.key}); }

Can you please elaborate how we have two modals in this case?

There are two modals, New task and Confirm Task

Result:

Screen.Recording.2023-06-26.at.1.57.37.PM.mov

This issue is also present in request and send money processes.

[s77rt]

@ygshbht Can we get more details where this difference is coming from? Is it related to react-navigation?

[s77rt]

@alitoshmatov Thanks. I got it

[ygshbht]

@s77rt Thank you for your response.

In essence, the difference arises from how back navigation is handled in different environments. When we talk about native apps, the 'back' action typically uses the app's own navigation stack to determine where to go back to. The app directly manages this stack.

However, in a web browser, the back button on the toolbar or the system's back is used, which are managed differently than in-app navigation. The browser's back button navigates through the browser's history stack, not the app's navigation stack.

This is not related to the UI or react-navigation as the in-app navigation works consistently across web and native environments. It's the external back button on web browsers causing the difference.

I hope this helps to clarify. Let me know if there are still any areas of confusion!

@ygshbht Can we get more details where this difference is coming from? Is it related to react-navigation?

[joekaufmanexpensify]

Same

[joekaufmanexpensify]

Same.

[joekaufmanexpensify]

Same

[joekaufmanexpensify]

Held

[adamgrzybowski]

Hey, this should be fixed by this PR

[mountiny]

@joekaufmanexpensify could you please confirm this issue is resolved in staging? If yes, then we can close it

[joekaufmanexpensify]

Sure, I'll test now.

[joekaufmanexpensify]

Confirmed this was fixed by the above PR. Closing, as this is all set!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.55-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [navigation-refactor] Fix browser history for complex navigation actions #24165

If no regressions arise, payment will be issued on 2023-08-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment
• @mananjadhav does not require payment (Eligible for Manual Requests)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] 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:
• [@s77rt] 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:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] 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.
• [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

I don't think a checklist is needed here. The bug seems upstream (or at least it was introduced after navigation reboot). If anything we can go with whatever checklist is submitted here #23796 (comment) (the PR reviewer is assigned to that issue).

No payment is due here I think, @mananjadhav (bug reporter) can request from ND.

Nothing to do. ✔️

[gadhiyamanan]

I think this is eligible for reporting bonus because the PR was created after issue reported. The issue was reported by me not @mananjadhav
Cc: @joekaufmanexpensify @mountiny

[joekaufmanexpensify]

My thinking was no reporting bonus would be issued here since my understanding of this comment was that the RCA of this issue was the same as other navigation initiatives we already planned to fix. LMK if that's not correct though @mountiny

[mountiny]

I agree with Joe here, basically this is not a unique case of the issue, but one of the symptoms/ one of many flows where you could run into it, creating money request, leaving a thread, creating a split, task all of those are same from fix perspective so it does not make sense to pay for each of them being reported. Sorry.