[HOLD for payment 2024-08-09] [$250] Workspace switcher - WS switcher and LHN are not reset to Expensify after leaving workspace

[izarutskaya]

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: 1.4.85-4
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

• User is invited to a workspace.

1. Go to staging.new.expensify.com
2. Open workspace switcher.
3. Select the invited workspace.
4. Go to Account settings > Workspaces.
5. Click 3-dot menu on the invited workspace > Leave.
6. Return to LHN.

Expected Result:

• Workspace switcher will switch to Expensify.
• LHN will show all the chats because Expensify is selected.

Actual Result:

• Workspace switcher displays Expensify, but when opening the workspace list, Expensify is not selected.
• LHN still shows the filtered chats.

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

20240619_131605.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018dbe15d720ffa01b
• Upwork Job ID: 1804259959268757378
• Last Price Increase: 2024-06-21
• Automatic offers:
• truph01 | Contributor | 103068211

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #vip-vsb

[tsa321]

Proposal

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

Workspace switcher - WS switcher and LHN are not reset to Expensify after leaving workspace

What is the root cause of that problem?

We don't have check on currently accessed policy, whether the user is still member of the policy or the policy is not deleted, Inside callback of Onyx connect to policy collection.

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

We can make a callback in onyx policy collection, then check if currently accessed policy is deleted or the user is not member anymore, then updateLastAccessedWorkspace(undefined);

Is this expected result? Or should we show modal if the remove member or delete workspace action are from other user?
the confirm modal could be. You are no longer member of ..... Then ask Do you want to change the workspace Expensify? ` if user is in report screen.

[truph01]

Proposal

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

Workspace switcher displays Expensify, but when opening the workspace list, Expensify is not selected.
LHN still shows the filtered chats.

What is the root cause of that problem?

In deleteWorkspace function

App/src/libs/actions/Policy/Policy.ts

Lines 335 to 340 in d938880

	// Reset the lastAccessedWorkspacePolicyID
	if (policyID === lastAccessedWorkspacePolicyID) {
	updateLastAccessedWorkspace(undefined);
	}
	}

We have logic to update the lastAccessedWorkspacePolicyID, but we missed it in the leaveWorkspace function

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

• We can add a new listener to BaseSidebarScreen component:

    const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
    const selectedPolicy = Object.values(policies ?? {}).find((o) => o?.id === activeWorkspaceID);
    const prevSelectedPolicy = usePrevious(selectedPolicy);
    useEffect(() => {
        if (!selectedPolicy && !prevSelectedPolicy) {
            return;
        }
        if (prevSelectedPolicy && !selectedPolicy && activeWorkspaceID !== undefined) {
            Navigation.navigateWithSwitchPolicyID({policyID: undefined});
            updateLastAccessedWorkspace(undefined);
        }
    }, [prevSelectedPolicy, selectedPolicy, activeWorkspaceID]);

• So once the selected policy is undefined, it will switch to the default policy.

What alternative solutions did you explore? (Optional)

App/src/libs/actions/Policy/Policy.ts

Line 716 in d938880

API.write(WRITE_COMMANDS.LEAVE_POLICY, params, {optimisticData, successData, failureData});

here, we also should add

if (policyID === lastAccessedWorkspacePolicyID) {
        updateLastAccessedWorkspace(undefined);
    }

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018dbe15d720ffa01b

[melvin-bot]

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

[s77rt]

@tsa321

In Policy.leaveWorkspace we don't have logic to check current workspace is leaved workspace, then navigate to all expensify chats

This is not the root cause. I don't see why such logic should be a part of the leaveWorkspace call. Also this will not fix the problem if the admin removed you (as a member) from the workspace

[s77rt]

@truph01 Same note as above ^ Adding the check to the leaveWorkspace will only fix the problem if you leave the workspace but what about if another admin removes you from that workspace?

[tsa321]

@s77rt How about we make a callback in onyx policy collection, then check if currently accessed policy is deleted or user is not member of the currently accessed policy anymore, then updateLastAccessedWorkspace(undefined);

Is this expected result? Or should we show modal if the remove member or delete workspace action are from other user?
the confirm modal could be. You are no longer member of ..... Then ask Do you want to change the workspace Expensify? ` if user is in report screen.

Proposal

updated

[s77rt]

@tsa321 I'd prefer not to relay on Onyx callbacks. Can we go with the same approach as reports? We have a useEffect on ReportScreen that handles report deletion

[truph01]

Proposal updated

• @s77rt I updated my main solution based on your suggestion above.

[s77rt]

@truph01 I noticed that If I select a workspace that I created and then delete it, the bug does not occur. Can you investigate that and why the bug only occurs on invited workspaces? That should help us find a better solution

[truph01]

@s77rt

• When deleting the workspace, we call:
  

  App/src/pages/workspace/WorkspacesListPage.tsx

  Lines 141 to 144 in 060160e

  	if (activeWorkspaceID === policyIDToDelete) {
  	setActiveWorkspaceID(undefined);
  	Navigation.navigateWithSwitchPolicyID({policyID: undefined});
  	}

  
  it is the same as what we do when switching to "Expensify" workspace. So the bug does not occur when we delete our workspaces that we are owner. Below is the detail.

• There are two ways to get the current active workspace, one is from ACTIVE_WORKSPACE_ID in session storage and one is from the navigation state.

• setActiveWorkspaceID(undefined): It will set the ACTIVE_WORKSPACE_ID in session storage to undefined. With this data, in workspace switcher page, the "Expensify" is displayed with "ticked" icon.

• Navigation.navigateWithSwitchPolicyID({policyID: undefined}): It will update the navigation state. With this data, in LHN, we display all the reports without filtering.

[truph01]

Update comment

[s77rt]

@truph01 Ah right. In your proposal we are not calling setActiveWorkspaceID though but updateLastAccessedWorkspace. Won't this make the workspace switcher page has not selected option?

[truph01]

@s77rt Yes, we do not need to call setActiveWorkspaceID because we already call it in:

App/src/libs/Navigation/NavigationRoot.tsx

Lines 127 to 141 in ab2426c

	const handleStateChange = (state: NavigationState | undefined) => {
	if (!state) {
	return;
	}
	const activeWorkspaceID = getPolicyIDFromState(state as NavigationState<RootStackParamList>);
	// Performance optimization to avoid context consumers to delay first render
	setTimeout(() => {
	currentReportIDValue?.updateCurrentReportID(state);
	setActiveWorkspaceID(activeWorkspaceID);
	}, 0);
	parseAndLogRoute(state);
	// We want to clean saved scroll offsets for screens that aren't anymore in the state.
	cleanStaleScrollOffsets(state);
	};

[s77rt]

@truph01 Makes sense! Overall the proposal looks good to me

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[s77rt]

Same ^

[melvin-bot]

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

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

[melvin-bot]

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

• Fix: WS switcher and LHN are not reset to Expensify after leaving #45285

If no regressions arise, payment will be issued on 2024-08-09. 🎊

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

• @s77rt requires payment through NewDot Manual Requests
• @truph01 requires payment automatic offer (Contributor)

[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.
• [@johncschuster] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: More like a missed case [Wave 8] Ideal nav  #33280
• The offending PR has been commented on: [Wave 8] Ideal nav  #33280 (comment)
• A discussion in #expensify-bugs has been started: Not needed, one of a kind bug
• Determine if we should create a regression test for this bug: No, low priority

[johncschuster]

Acknowledged! I'll issue payment after the regression threshold is cleared.

[johncschuster]

Payment Summary

Contributor: @truph01 paid $250 via Upwork (paid on 2024-08-09)
Contributor+: @s77rt owed $250 via NewDot

[johncschuster]

Payment has been issued via Upwork to @truph01. Go ahead and request payment on ND, @s77rt! Thank you both for your contributions! 🎉

[JmillsExpensify]

$250 approved for @s77rt