[HOLD for payment 2024-06-20] [$500] [Search v1] LHN shows Not found when navigated from global create menu

[luacmartins]

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:
Reproducible in staging?:
Reproducible in production?:
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
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

Action Performed:

There are two issues here:

Issue 1

1. From the Search page, click on a result
2. Click on the header link
3. Notice that the LHN shows not found

Issue 2

1. While viewing the Search page, click on the global create menu
2. Submit and expese
3. Notice that the LHN shows not found

Expected Result:

Navigation should work

Actual Result:

LHN shows not found

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: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015dea8a0a2e4109e9
• Upwork Job ID: 1797769446176616448
• Last Price Increase: 2024-06-04
• Automatic offers:
• c3024 | Reviewer | 102591587
• bernhardoj | Contributor | 102591588

Issue Owner

Current Issue Owner: @alexpensify

[luacmartins]

cc @adamgrzybowski can you take a look please?

[melvin-bot]

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

[dragnoir]

Proposal

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

Non supported currency turns error

What is the root cause of that problem?

On the Search page, when an expense or any other type of report is submitted, or when the header link in a report is clicked, the action performed is as follows:

App/src/libs/actions/IOU.ts

Line 3498 in e6c89d9

Navigation.dismissModal(activeReportID);

The dismissModal function opens the report view but does not change the left view from SearchPageBottomTab to SidebarScreen. The issue lies within the dismissModalWithReport function:

The propblem is within dismissModalWithReport

App/src/libs/Navigation/dismissModalWithReport.ts

Lines 53 to 63 in 9a63031

	if (shouldOpenAllWorkspace) {
	switchPolicyID(navigationRef, {route: ROUTES.HOME});
	} else {
	switchPolicyID(navigationRef, {policyID, route: ROUTES.HOME});
	}
	const action: StackNavigationAction = getActionFromState(reportState, linkingConfig.config);
	if (action) {
	action.type = 'REPLACE';
	navigationRef.dispatch(action);
	}

When switchPolicyID is performed, subsequent actions like navigationRef.dispatch(action) are lost in the memory queue.

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

We should change the order and prioritize navigationRef.dispatch(action)

const  action:  StackNavigationAction  =  getActionFromState(reportState, linkingConfig.config);

if (action) {
action.type  =  'REPLACE';
  navigationRef.dispatch(action);
}

if (shouldOpenAllWorkspace) {
  switchPolicyID(navigationRef, {route:  ROUTES.HOME});
} else {
  switchPolicyID(navigationRef, {policyID, route:  ROUTES.HOME});
}

POC video:

20240603_225950.mp4

What alternative solutions did you explore?

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015dea8a0a2e4109e9

[melvin-bot]

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

[luacmartins]

@c3024 @adamgrzybowski let me know what you think of the proposal above

[bernhardoj]

Proposal

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

LHN shows not found when pressing the report parent navigation or submitting an expense from global create while on the search page.

What is the root cause of that problem?

When pressing the parent navigation link, we use goBack with the parent report as the fallback to go to the parent report.

App/src/components/ParentNavigationSubtitle.tsx

Lines 39 to 48 in 1d5f83d

	<PressableWithoutFeedback
	onPress={() => {
	const parentAction = ReportActionsUtils.getReportAction(parentReportID, parentReportActionID ?? '');
	const isVisibleAction = ReportActionsUtils.shouldReportActionBeVisible(parentAction, parentAction?.reportActionID ?? '');
	// Pop the thread report screen before navigating to the chat report.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID));
	if (isVisibleAction && !isOffline) {
	// Pop the chat report screen before navigating to the linked report action.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID, parentReportActionID));
	}

Because the report screen is in RHP and it's the only screen, we navigate with the UP navigation type, which will replace the current screen with the report screen,

App/src/libs/Navigation/Navigation.ts

Lines 219 to 222 in 1d5f83d

	if (shouldEnforceFallback || (isFirstRouteInNavigator && fallbackRoute)) {
	navigate(fallbackRoute, CONST.NAVIGATION.TYPE.UP);
	return;
	}

App/src/libs/Navigation/linkTo/index.ts

Lines 85 to 86 in 1d5f83d

	if (type === CONST.NAVIGATION.TYPE.UP) {
	action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE;

but the bottom tab navigator screen isn't changed. The bottom tab navigator screen should be updated if the navigation target is a central pane screen, which is true for our case, however, the UP type case is prioritized as you can see from the if condition below.

App/src/libs/Navigation/linkTo/index.ts

Lines 85 to 101 in 1d5f83d

	if (type === CONST.NAVIGATION.TYPE.UP) {
	action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE;
	// If this action is navigating to the report screen and the top most navigator is different from the one we want to navigate - PUSH the new screen to the top of the stack
	} else if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && (isTargetScreenDifferentThanCurrent || areParamsDifferent)) {
	// We need to push a tab if the tab doesn't match the central pane route that we are going to push.
	const topmostBottomTabRoute = getTopmostBottomTabRoute(rootState);
	const policyIDsFromState = extractPolicyIDsFromState(stateFromPath);
	const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateFromPath, policyID || policyIDsFromState);
	const isNewPolicyID =
	(topmostBottomTabRoute?.params as Record<string, string | undefined>)?.policyID !== (matchingBottomTabRoute?.params as Record<string, string | undefined>)?.policyID;
	if (topmostBottomTabRoute && (topmostBottomTabRoute.name !== matchingBottomTabRoute.name || isNewPolicyID)) {
	root.dispatch({
	type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
	payload: matchingBottomTabRoute,
	});
	}

Previously, we had a FORCED_UP type which is always prioritized first and then the UP type will be checked after checking the central pane screen,

if (type == FORCED_UP) ...
else if (target == CENTRAL_PANE_NAVIGATOR) ...
else if (type == UP)

but now, the FORCED_UP is removed and replaced with UP in #42335.

For the second issue, after submitting an expense, we call dismissModal.

App/src/libs/actions/IOU.ts

Line 3498 in 1d5f83d

Navigation.dismissModal(activeReportID);

What it will do is it will replace the current screen with a report screen and also update the bottom tab screen by switchPolicyID.

App/src/libs/Navigation/dismissModalWithReport.ts

Lines 51 to 63 in 1d5f83d

	const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);
	if (shouldOpenAllWorkspace) {
	switchPolicyID(navigationRef, {route: ROUTES.HOME});
	} else {
	switchPolicyID(navigationRef, {policyID, route: ROUTES.HOME});
	}
	const action: StackNavigationAction = getActionFromState(reportState, linkingConfig.config);
	if (action) {
	action.type = 'REPLACE';
	navigationRef.dispatch(action);
	}

App/src/libs/Navigation/switchPolicyID.ts

Lines 85 to 91 in 1d5f83d

	const actionForBottomTabNavigator = getActionForBottomTabNavigator(action, rootState, policyID);
	if (!actionForBottomTabNavigator) {
	return;
	}
	root.dispatch(actionForBottomTabNavigator);

The bottom tab screen will be based on the route that we pass, in our case, it's HOME, so it should show the HOME LHN. If we don't pass route, then it will use the currently focused bottom tab screen (this is the newPath that will be explained below). However, there is a logic here that will always set the LHN to be the search bottom tab if we execute the navigation while on the search page, even if we pass a route to navigate to,

App/src/libs/Navigation/switchPolicyID.ts

Lines 78 to 81 in 1d5f83d

	const isOpeningSearchFromBottomTab = topmostCentralPaneRoute?.name === SCREENS.SEARCH.CENTRAL_PANE;
	if (isOpeningSearchFromBottomTab) {
	newPath = ROUTES.SEARCH.getRoute(CONST.TAB_SEARCH.ALL);
	}

Previously, we checked whether the newPath is the search bottom or not before setting the LHN to be the search bottom tab.

But it's changed in #42335 too because the const is now removed.

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

To fix the 2nd issue, we can use the old approach by adding back the const,

const isOpeningSearchFromBottomTab = newPath.startsWith(CONST.SEARCH_BOTTOM_TAB_URL);

or update isOpeningSearchFromBottomTab condition so it's true only if we don't pass a route.

const isOpeningSearchFromBottomTab = !route && topmostCentralPaneRoute?.name === SCREENS.SEARCH.CENTRAL_PANE;

To fix the 1st issue, we can either:

1. Add back FORCED_UP and it's logic like before
2. Copy the bottom tab update logic from this if block,
   

   App/src/libs/Navigation/linkTo/index.ts

   Lines 89 to 101 in 1d5f83d

   	} else if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && (isTargetScreenDifferentThanCurrent || areParamsDifferent)) {
   	// We need to push a tab if the tab doesn't match the central pane route that we are going to push.
   	const topmostBottomTabRoute = getTopmostBottomTabRoute(rootState);
   	const policyIDsFromState = extractPolicyIDsFromState(stateFromPath);
   	const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateFromPath, policyID || policyIDsFromState);
   	const isNewPolicyID =
   	(topmostBottomTabRoute?.params as Record<string, string | undefined>)?.policyID !== (matchingBottomTabRoute?.params as Record<string, string | undefined>)?.policyID;
   	if (topmostBottomTabRoute && (topmostBottomTabRoute.name !== matchingBottomTabRoute.name || isNewPolicyID)) {
   	root.dispatch({
   	type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
   	payload: matchingBottomTabRoute,
   	});
   	}

   
   to the type is UP logic if the target is the central pane screen

if (type === CONST.NAVIGATION.TYPE.UP) {
    if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR) // bottom tab screen update goes here
    action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE;
}

3. Rearrange the if such as the central pane screen condition is prioritized first,

if (target == CENTRAL_PANE_NAVIGATOR) ...
else if (type == UP)

and add another type check in the first if to change the action type.

App/src/libs/Navigation/linkTo/index.ts

Line 103 in 1d5f83d

action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH;

if (target == CENTRAL_PANE_NAVIGATOR) {
    ... (bottom tab screen update)

    if (type  === UP) {
        action.type = CONST.NAVIGATION.ACTION_TYPE.REPLACE
    } else {
        action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH;
    }

   ...
} else if (type === UP) ...

4. Use dismissModal instead of goBack when pressing the parent navigation link while in a search page (or in report RHP), but we also need to pass the reportActionID to dismissModal so it can shows the correct linked message.

[adamgrzybowski]

@bernhardoj Thanks for this great proposal!

For the issue with dismiss modal I personally prefer this solution:
const isOpeningSearchFromBottomTab = !route && topmostCentralPaneRoute?.name === SCREENS.SEARCH.CENTRAL_PANE;. I like the fact that we don't operate on the url but rather on the state.

For the issue in the linkTo file I like the 3rd option. It looks simple and doesn't use FORCED_UP which may be confusing.

[luacmartins]

I agree with @bernhardoj's proposal since it covers both nav issues. Let's implement @adamgrzybowski's suggestion though.

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

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

[JmillsExpensify]

@bernhardoj Quick note for visibility: This PR is quite high value, so I'm going to double the value to reflect that. Can you make this your top priority? Thanks!

[melvin-bot]

Upwork job price has been updated to $500

[bernhardoj]

@JmillsExpensify hi, it's late for me now, this will be my first thing to work tomorrow.

[JmillsExpensify]

Thank you for confirming! Sounds good

[alexpensify]

Awesome, looks like this PR is moving forward. Thanks!

[luacmartins]

PR is merged.

[alexpensify]

Weekly Update: Waiting for this PR to go to prod

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

• Fix LHN shows Not found when navigated from global create menu or report parent navigation link #43086

If no regressions arise, payment will be issued on 2024-06-18. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @c3024 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:

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

[melvin-bot]

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

• Fix LHN shows Not found when navigated from global create menu or report parent navigation link #43086

If no regressions arise, payment will be issued on 2024-06-20. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @c3024 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:

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

[awongCM]

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: Reproducible in staging?: Reproducible in production?: 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 Expensify/Expensify Issue URL: Issue reported by: Slack conversation:

Action Performed:

There are two issues here:

Issue 1

1. From the Search page, click on a result
2. Click on the header link
3. Notice that the LHN shows not found

Issue 2

1. While viewing the Search page, click on the global create menu
2. Submit and expese
3. Notice that the LHN shows not found

Expected Result:

Navigation should work

Actual Result:

LHN shows not found

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: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
Issue Owner

Hi - is this issue going to remain open for a while? I'm interested to contribute to the problem statement.

[melvin-bot]

📣 @awongCM! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[c3024]

@awongCM

Issues with the External and Help Wanted labels are open to external contributors. So, this issue is no more open for contribution.

You can check the complete guidelines here.

[awongCM]

here

Thanks. I'll take a look in my own spare time. Cheers.

[alexpensify]

Flagging that the payment day is on June 20

[awongCM]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01076ffa2e22c46ed8

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[alexpensify]

Payouts due: 2024-06-20

• Contributor: $500 @bernhardoj
• Contributor+: $500 @c3024

Upwork job is here.

Closing, everyone here has been paid and I've closed the job in Expensify. I've also manually corrected the amount due in Upwork as a bonus.