PAY on 06/03 - [$500] [Wave Collect] [Ideal Nav] Workspace - Workspace is not clickable after returning from error page

[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: .1.4-36.0
Reproducible in staging?: Y
Reproducible in production?: N
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: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: User is invited to Collect workspace

1. Go to staging.new.expensify.com
2. Click Settings from the bottom
3. Click on the Collect workspace
4. Add /members to the end of URL and navigate to it
5. Click the back button on the top left of right hand page
6. Click on the Collect workspace

Expected Result:

The workspace page for Collect workspace will open

Actual Result:

The Collect workspace in Workspaces overview is no longer clickable

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

Add any screenshot/video evidence

Bug6365128_1706896531177.20240203_000901.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0159986817ac8b64d5
• Upwork Job ID: 1753494361146109952
• Last Price Increase: 2024-02-02
• Automatic offers:
• shubham1206agra | Reviewer | 28146520
• bernhardoj | Contributor | 28146521

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @blimpich (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

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

[blimpich]

This is probably ideal nav related #33280. Going to follow hayata's instructions as per this slack thread (internal Expensify Slack channel).

[blimpich]

Also not a blocker, at least not worth reverting ideal nav over.

[Krishna2323]

@blimpich, is this open for external proposals?

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0159986817ac8b64d5

[melvin-bot]

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

[blimpich]

@blimpich, is this open for external proposals?

@Krishna2323 It is now 🙂. Thanks for the reminder

[bernhardoj]

Proposal

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

Can't navigate to the same workspace after accessing the not found page of a workspace.

What is the root cause of that problem?

When we press the go back button of the not found page, it will navigate to /settings/workspace.

App/src/pages/workspace/WorkspaceMembersPage.js

Line 428 in 3ab4e6e

onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WORKSPACES)}

It will update the central pane navigator screen with the workspace list, but notice that the bottom tab navigator on the left still shows the previous workspace settings.

That is because the bottom tab navigator screen is not updated. When we click the same workspace, it will try to navigate to the workspace detail, but it will return early here because the workspace detail that we want to navigate is the same workspace that is currently shown on the bottom tab navigator on the left.

App/src/libs/Navigation/linkTo.ts

Lines 205 to 210 in 3ab4e6e

	const shouldNavigate = !!extractedPolicyID;
	const actionForBottomTabNavigator = getActionForBottomTabNavigator(action, rootState, policyID, shouldNavigate);
	if (!actionForBottomTabNavigator) {
	return;
	}

App/src/libs/Navigation/linkTo.ts

Lines 98 to 102 in 3ab4e6e

	// Verify if the policyID is different than the one we are currently on. If it is, we need to navigate to the new policyID.
	const isNewPolicy = bottomTabParams?.policyID !== payloadParams?.policyID;
	if (bottomTabCurrentTab?.name === screen && !shouldNavigate && !isNewPolicy) {
	return;
	}

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

We need to update the bottom tab navigator too. We already did this in WorkspacePageWithSections

App/src/pages/workspace/WorkspacePageWithSections.tsx

Lines 116 to 121 in 3ab4e6e

	const goBack = () => {
	Navigation.goBack(ROUTES.SETTINGS_WORKSPACES);
	// Needed when workspace with given policyID does not exist
	Navigation.navigateWithSwitchPolicyID({route: ROUTES.ALL_SETTINGS});
	};

where we use this component to wrap many workspace pages, like the workspace overview page.

We can either copy the go back code or wrap the members page with WorkspacePageWithSections too

But I see that WorkspacePageWithSections is used on the workspace page that needs bank account data which we don't care in members page, so I'm not sure if we want to use it here.

Btw, we can improve the navigation to navigate back to the workspace overview if the workspace exists.

// This empty policy check is taken from other parts of the code, but there are some cases where an invalid/not found workspace returns an object with an error instead of empty, so we should add an error check here too (see image below).
if (_.isEmpty(props.policy) || Object.values(props.policy?.errors ?? {})[0] === 'Invalid Policy ID') {
    Navigation.goBack(ROUTES.SETTINGS_WORKSPACES);
    Navigation.navigateWithSwitchPolicyID({route: ROUTES.ALL_SETTINGS});
} else {
    Navigation.goBack(ROUTES.WORKSPACE_INITIAL.getRoute(policyID));
}

What alternative solutions did you explore? (Optional)

Or we can simply just call goBack without any fallback route, so it will just go back to the previous screen on the stack, only if the workspace exists. (but currently looks like it needs 2 times to press the go back for it to works correctly, I guess another bug)

[melvin-bot]

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

[bfitzexpensify]

How does @bernhardoj's proposal look @shubham1206agra?

[shubham1206agra]

@bernhardoj's proposal looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @blimpich is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

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

[bernhardoj]

PR is ready

cc: @shubham1206agra

[bfitzexpensify]

Bump on this one @shubham1206agra - thank you!

[bfitzexpensify]

Manually bringing the BZ checklist across:

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

• Fix can't open the workspace after closing the workspace not found (unaccessible) page #35980

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

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

• @bernhardoj requires payment via Upwork - https://www.upwork.com/ab/applicants/1753494361146109952/offers
• @shubham1206agra requires payment via Upwork - https://www.upwork.com/ab/applicants/1753494361146109952/offers - @shubham1206agra, can you please accept the Upwork offer?

[bfitzexpensify]

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:

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

[shubham1206agra]

@bfitzexpensify Accepted

[shubham1206agra]

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:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: [Wave 8] Ideal nav  #33280
• [@shubham1206agra] 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: [Wave 8] Ideal nav  #33280 (comment)
• [@shubham1206agra ] 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: Not Required
• [@shubham1206agra ] Determine if we should create a regression test for this bug. No
• [@shubham1206agra ] 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.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bfitzexpensify]

Contracts paid out, BZ checklist complete, no need for a regression test - we're all done here.