[HOLD for payment 2024-07-17] Workspace switcher - Report from another workspace persists after switching workspace

@mallenexpensify

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: 9.0-3.2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

Account has several workspaces

Go to staging.new.expensify.com
Select Expensify in the workspace switcher (if not yet)
Open workspace chat from workspace A
Open workspace switcher
Select Workspace B

Expected Result:

After selecting Workspace B from the switcher, the currently opening report should switch to some report from Workspace B (production behavior)

Actual Result:

After selecting Workspace B from the switcher, the currently opening report is still the workspace chat from Workspace A. It does not switch to report in Workspace B

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

Bug6527328_1719575851910.20240628_195143.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @mallenexpensify

@pecanoro

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

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

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

@pecanoro

@pecanoro FYI 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

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

Hmm, switching workspaces gets me a crash every time:

Ok, I can reproduce the bug with my normal expensify account since the other one is giving me that error.

@lanitochka17

@lanitochka17 Does it only happen if the chat was selected already? I can only reproduce if I had that chat opened

Also, I am not fully sure this is on purpose or not

Proposal

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

The report from another workspace is still shown when switching to another workspace.

What is the root cause of that problem?

When we switch workspace, we push a new report screen with an empty reportID param so the report screen will handle populating the reportID itself from the findLastAccessedReport.

App/src/libs/Navigation/switchPolicyID.ts

Lines 120 to 132 in 4622e19

    
           // If the user is on the home page and changes the current workspace, then should be displayed a report from the selected workspace. 
        
           // To achieve that, it's necessary to navigate without the reportID param. 
        
           if (checkIfActionPayloadNameIsEqual(actionForBottomTabNavigator, SCREENS.HOME)) { 
        
               delete params.reportID; 
        
           } 
        
           root.dispatch({ 
        
               type: CONST.NAVIGATION.ACTION_TYPE.PUSH, 
        
               payload: { 
        
                   name: topmostCentralPaneRoute?.name, 
        
                   params, 
        
               }, 
        
           });

This is the new logic from #44559.

App/src/pages/home/ReportScreen.tsx

Lines 163 to 184 in 4622e19

    
           // Check if there's a reportID in the route. If not, set it to the last accessed reportID 
        
           const lastAccessedReportID = useLastAccessedReportID(!!route.params.openOnAdminRoom); 
        
           useEffect(() => { 
        
               // Don't update if there is a reportID in the params already 
        
               if (route.params.reportID) { 
        
                   const reportActionID = route?.params?.reportActionID; 
        
                   const isValidReportActionID = ValidationUtils.isNumeric(reportActionID); 
        
                   if (reportActionID && !isValidReportActionID) { 
        
                       navigation.setParams({reportActionID: ''}); 
        
                   } 
        
                   return; 
        
               } 
        
               // It's possible that reports aren't fully loaded yet 
        
               // in that case the reportID is undefined 
        
               if (!lastAccessedReportID) { 
        
                   return; 
        
               } 
        
               Log.info(`[ReportScreen] no reportID found in params, setting it to lastAccessedReportID: ${lastAccessedReportID}`); 
        
               navigation.setParams({reportID: lastAccessedReportID}); 
        
           }, [lastAccessedReportID, navigation, route]);

However, the reportID is never empty, so we never use the reportID returned from findLastAccessedReport which do a filtering based on the selected workspace.

App/src/libs/ReportUtils.ts

Lines 1178 to 1180 in 4622e19

    
           if (!!policyID || policyMemberAccountIDs.length > 0) { 
        
               reportsValues = filterReportsByPolicyIDAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID); 
        
           }

The reason it's not empty is because we pass a reportID to the initial params of the report screen.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 201 in 4622e19

const lastAccessedReportID = useLastAccessedReportID(shouldOpenOnAdminRoom());

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 86 to 96 in 4622e19

    
           function getCentralPaneScreenInitialParams(screenName: CentralPaneName, lastAccessedReportID?: string): Partial<ValueOf<CentralPaneScreensParamList>> { 
        
               if (screenName === SCREENS.SEARCH.CENTRAL_PANE) { 
        
                   return {sortBy: CONST.SEARCH.TABLE_COLUMNS.DATE, sortOrder: CONST.SEARCH.SORT_ORDER.DESC}; 
        
               } 
        
               if (screenName === SCREENS.REPORT) { 
        
                   return { 
        
                       openOnAdminRoom: shouldOpenOnAdminRoom() ? true : undefined, 
        
                       reportID: lastAccessedReportID, 
        
                   }; 
        
               }

useLastAccessedReportID recalculates when activeWorkspaceID is updated, but in our case, when the new report screen is added, the value isn't updated yet, so we pass the old last accessed report, which is the current report view.

App/src/hooks/useLastAccessedReportID.ts

Lines 131 to 143 in 4622e19

    
           const getSnapshot = useCallback(() => { 
        
               const policyMemberAccountIDs = getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID); 
        
               return ReportUtils.findLastAccessedReport( 
        
                   reports, 
        
                   !canUseDefaultRooms, 
        
                   policies, 
        
                   isFirstTimeNewExpensifyUser, 
        
                   shouldOpenOnAdminRoom, 
        
                   reportMetadata, 
        
                   activeWorkspaceID, 
        
                   policyMemberAccountIDs, 
        
               )?.reportID; 
        
           }, [activeWorkspaceID, canUseDefaultRooms, shouldOpenOnAdminRoom]);

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

We can remove the report screen initial params. We already have useLastAccessedReportID inside the report screen.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 91 to 96 in 4622e19

    
           if (screenName === SCREENS.REPORT) { 
        
               return { 
        
                   openOnAdminRoom: shouldOpenOnAdminRoom() ? true : undefined, 
        
                   reportID: lastAccessedReportID, 
        
               }; 
        
           }

@c3024

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

The bot keeps delaying the payment date everytime we deploy, can we use the 07-17 as the payment date?

@mallenexpensify

That sounds fair to me. @mallenexpensify what do you think?

Yeah, updated in title, lotta payments due tomorrow. Addressed internally in #bug-zero too since it looks like we have 79 issues with double payment dates of tomorrow and the 22nd :/
We also have this internal issue for trying to figure out why it's happening.

Thanks! Requested in ND.

Still need a payment summary with amounts.

@bernhardoj

Payment Summary

Upwork Job

Reviewer: @bernhardoj owed $250 via NewDot
ROLE: @c3024 paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@mallenexpensify)

I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
I have paid out the Upwork contracts or cancelled the ones that are incorrect
I have verified the payment summary above is correct

@bernhardoj

Contributor: @bernhardoj due $250 via NewDot
Contributor+: @c3024 paid $250 via Upwork.

@c3024 , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01403d04f9b0f49fc0

Can you also complete the BZ checklist? Thx

@c3024

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: [CP Stg] Replace ReportScreenIDSetter with useLastAccessedReportID #44559
[@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: [HOLD for payment 2024-07-17] Workspace switcher - Report from another workspace persists after switching workspace #44619 (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: No, I don't think any specific steps can be taken that could have helped to catch this bug earlier.
[@c3024] Determine if we should create a regression test for this bug. No, this was caught in staging. So another regression test is not required.
[@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. NA

@c3024

@c3024 do we want a regression test here? I'd think we do unless this was a regression.

This is what's in the PR

Prerequisite: have at least 2 workspaces

(web/desktop)

Open the first workspace chat (#admins, #announce, etc.)
Open the workspace switcher page
Switch to the second workspace
Verify the first workspace chat doesn't show anymore
(android/iOS/mWeb)
can't switch WS while a report is shown, so just make sure the WS switching still works

@bernhardoj

$250 approved for @bernhardoj

This is a regression identified during staging for a PR I reviewed. So, I think no regression test is required.

@c3024

Updated the payment breakdown to denote @c3024 was paid. Thanks!

Navigation Menu

Repository navigation

[HOLD for payment 2024-07-17] Workspace switcher - Report from another workspace persists after switching workspace #44619

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

41 remaining items

Payment Summary

BugZero Checklist (@mallenexpensify)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// If the user is on the home page and changes the current workspace, then should be displayed a report from the selected workspace.
	// To achieve that, it's necessary to navigate without the reportID param.
	if (checkIfActionPayloadNameIsEqual(actionForBottomTabNavigator, SCREENS.HOME)) {
	delete params.reportID;
	}

	root.dispatch({
	type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
	payload: {
	name: topmostCentralPaneRoute?.name,
	params,
	},
	});

	// Check if there's a reportID in the route. If not, set it to the last accessed reportID
	const lastAccessedReportID = useLastAccessedReportID(!!route.params.openOnAdminRoom);
	useEffect(() => {
	// Don't update if there is a reportID in the params already
	if (route.params.reportID) {
	const reportActionID = route?.params?.reportActionID;
	const isValidReportActionID = ValidationUtils.isNumeric(reportActionID);
	if (reportActionID && !isValidReportActionID) {
	navigation.setParams({reportActionID: ''});
	}
	return;
	}

	// It's possible that reports aren't fully loaded yet
	// in that case the reportID is undefined
	if (!lastAccessedReportID) {
	return;
	}

	Log.info(`[ReportScreen] no reportID found in params, setting it to lastAccessedReportID: ${lastAccessedReportID}`);
	navigation.setParams({reportID: lastAccessedReportID});
	}, [lastAccessedReportID, navigation, route]);

	if (!!policyID \|\| policyMemberAccountIDs.length > 0) {
	reportsValues = filterReportsByPolicyIDAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID);
	}

	function getCentralPaneScreenInitialParams(screenName: CentralPaneName, lastAccessedReportID?: string): Partial<ValueOf<CentralPaneScreensParamList>> {
	if (screenName === SCREENS.SEARCH.CENTRAL_PANE) {
	return {sortBy: CONST.SEARCH.TABLE_COLUMNS.DATE, sortOrder: CONST.SEARCH.SORT_ORDER.DESC};
	}

	if (screenName === SCREENS.REPORT) {
	return {
	openOnAdminRoom: shouldOpenOnAdminRoom() ? true : undefined,
	reportID: lastAccessedReportID,
	};
	}

	const getSnapshot = useCallback(() => {
	const policyMemberAccountIDs = getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID);
	return ReportUtils.findLastAccessedReport(
	reports,
	!canUseDefaultRooms,
	policies,
	isFirstTimeNewExpensifyUser,
	shouldOpenOnAdminRoom,
	reportMetadata,
	activeWorkspaceID,
	policyMemberAccountIDs,
	)?.reportID;
	}, [activeWorkspaceID, canUseDefaultRooms, shouldOpenOnAdminRoom]);

[HOLD for payment 2024-07-17] Workspace switcher - Report from another workspace persists after switching workspace #44619

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Jun 28, 2024

github-actions commented on Jun 28, 2024

lanitochka17 commented on Jun 28, 2024

lanitochka17 commented on Jun 28, 2024

pecanoro commented on Jun 28, 2024

pecanoro commented on Jun 28, 2024

pecanoro commented on Jun 28, 2024

pecanoro commented on Jun 28, 2024

bernhardoj commented on Jun 29, 2024

Proposal

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

What is the root cause of that problem?

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

41 remaining items

melvin-bot commented on Jul 15, 2024

bernhardoj commented on Jul 16, 2024

JmillsExpensify commented on Jul 16, 2024

mallenexpensify commented on Jul 16, 2024

bernhardoj commented on Jul 17, 2024

JmillsExpensify commented on Jul 17, 2024

melvin-bot commented on Jul 17, 2024

Payment Summary

BugZero Checklist (@mallenexpensify)

mallenexpensify commented on Jul 17, 2024

c3024 commented on Jul 18, 2024

mallenexpensify commented on Jul 19, 2024

JmillsExpensify commented on Jul 19, 2024

c3024 commented on Jul 22, 2024

mallenexpensify commented on Jul 22, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions