[HOLD for payment 2024-04-25] [$500] Back to home leads to recent chat instead of LHN after opening non-existing profile

[izarutskaya]

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: v1.4.52-5
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4424104
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Log in to New Expensify
2. Navigate to non-existing profile, e. g. https://staging.new.expensify.com/a/hello
3. Tap on Go back to home page
4. Tap on Go back to home page again

Expected Result:

User should be redirected back to LHN

Actual Result:

User is redirected to the most recent chat instead of LHN.
Hmm.. it's not here page is displayed twice.

Partially reproducible in Prod:
User is also redirected to the most recent chat instead of LHN, but not here page is shown once.

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

Bug6414170_1710450722340.Record_2024-03-14-21-21-17.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d8b170d82d50a998
• Upwork Job ID: 1775265846262415360
• Last Price Increase: 2024-04-02
• Automatic offers:
• ishpaul777 | Reviewer | 0
• bernhardoj | Contributor | 0

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[izarutskaya]

@anmurali 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.

[izarutskaya]

We think that this bug might be related to #wave-collect - Release 1
CC @trjExpensify

[bernhardoj]

Proposal

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

The most recent chat is shown when going back from not found profile page.

What is the root cause of that problem?

When we open an RHP page, in this case, the profile page, it will try to find the matching bottom tab, matching central pane screen, and the RHP itself and "push" them all to the stack.

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 162 to 166 in e9c112a

	if (rhpNavigator) {
	// Routes
	// - matching bottom tab
	// - matching root route for rhp
	// - found rhp

To find the matching central pane, it will check the mapping here,

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 132 to 144 in e9c112a

	// Check for CentralPaneNavigator
	for (const [centralPaneName, RHPNames] of Object.entries(CENTRAL_PANE_TO_RHP_MAPPING)) {
	if (RHPNames.includes(route.name)) {
	return createCentralPaneNavigator({name: centralPaneName as CentralPaneName, params: route.params});
	}
	}
	// Check for FullScreenNavigator
	for (const [fullScreenName, RHPNames] of Object.entries(FULL_SCREEN_TO_RHP_MAPPING)) {
	if (RHPNames && RHPNames.includes(route.name)) {
	return createFullScreenNavigator({name: fullScreenName as FullScreenName, params: route.params});
	}
	}

but in our case, a profile page doesn't have any mapping. We can't map a profile page to any central pane. Because the matching central pane is undefined, we use a default one and that is a report screen without any reportID which will find the most recent report to show.

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Line 179 in e9c112a

matchingRootRoute = matchingRootRoute ?? createCentralPaneNavigator({name: SCREENS.REPORT});

So, when we press back, we will see the most recent report chat.

This is useful for desktop view since a central pane screen is always visible, but it's not for a small screen.

I can't reproduce the app navigates back to the not found page on dev (only on staging, maybe fixed).

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

For a small screen, if there is no matching central pane, we shouldn't push the default, that is the report screen to the navigation.

matchingRootRoute = matchingRootRoute ?? (!isNarrowLayout ? createCentralPaneNavigator({name: SCREENS.REPORT}) : undefined);
...
const matchingBottomTabRoute = getMatchingBottomTabRouteForState(matchingRootRoute ? {routes: [matchingRootRoute]} : {routes: []});
...
if (matchingRootRoute && (!isNarrowLayout || !isRHPScreenOpenedFromLHN)) {
    routes.push(matchingRootRoute);
}

[dragnoir]

Proposal

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

Profile page, back to home leads to recent chat instead of LHN

What is the root cause of that problem?

We are not setting the value of onLinkPress to FullPageNotFoundView in ProfilePage

App/src/pages/ProfilePage.tsx

Line 131 in 8b54aff

<FullPageNotFoundView shouldShow={shouldShowBlockingView || CONST.RESTRICTED_ACCOUNT_IDS.includes(accountID)}>

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

set onLinkPress value to navigate to LHN when clicked.

<FullPageNotFoundView
                shouldShow={shouldShowBlockingView || CONST.RESTRICTED_ACCOUNT_IDS.includes(accountID)}
                onLinkPress={() => {
                    Navigation.goBack(ROUTES.HOME);
                }}
            >

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[anmurali]

I cannot reproduce on Staging or Production. I see the Go back to home page once and it successfully takes me back to LHN

[bernhardoj]

@anmurali hi, it's still reproducible, can you recheck, please?

Screen.Recording.2024-03-19.at.13.31.36.mov

Notice that it goes back to report/chat screen instead of LHN.

[lanitochka17]

Issue is still reproducible on the latest build 1.4.57-2

0-02-01-5f68f73f9a6f56b253929cf49e651111cdb66963ca6c38c5370a4088f55ebe5c_f9f3703862c082fc.mp4

Record_2024-03-27-20-51-50.mp4

[anmurali]

Ah I see in mWeb what you mean. I missed that.

[melvin-bot]

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

[melvin-bot]

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

[ishpaul777]

reviewing 👀

[ishpaul777]

@bernhardoj RCA makes most sense to me and the solution also works well! Their proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[bernhardoj]

PR is ready

cc: @ishpaul777

[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.4.62-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Don't add report screen by default when visiting RHP in narrow layout #39788

If no regressions arise, payment will be issued on 2024-04-25. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @ishpaul777 requires payment automatic offer (Reviewer)

[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:

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

[anmurali]

@bernhardoj is paid. @ishpaul777 can you complete the checklist so I can release your payment as well?

[deetergp]

Bump @ishpaul777

[ishpaul777]

The PR that introduced the bug has been identified. Link to the PR: #37421

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/37421/files#r1582555183

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: N/A

Determine if we should create a regression test for this bug. - I dont think so.

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. - Incase we do need one, steps to repro and expected result in OP looks good!

[ishpaul777]

Sorry for delay completed checklist!

[deetergp]

Woo! Thanks @ishpaul777. All you @anmurali 🎉

[ishpaul777]

gentle bump @anmurali 😄

[anmurali]

completed all payments!