[HOLD for payment 2024-06-20] [$250] Workspace switcher resets to All (Expensify) after page refresh

[m-natarajan]

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.73-4
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1715648972336829

Action Performed:

1. Go the staging.new.expensify.com and login
2. Click the dropdown on the top left and choose different workspace
3. Refresh the page

Expected Result:

The selected workspace should remain the same

Actual Result:

The workspace switcher changes back to all (still filtering based on the workspace picked, but the icon and title resets back to Expensify)

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

WS.Filter.mp4

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016d7c9a80aa03fdc3
• Upwork Job ID: 1792505861453303808
• Last Price Increase: 2024-05-20
• Automatic offers:
• c3024 | Reviewer | 102452994
• abzokhattab | Contributor | 102452997

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

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

[shawnborton]

FWIW, this was originally reported by @GandalfGwaihir

[m-natarajan]

Solution from reporter @GandalfGwaihir : We need to subscribe to the active policy using Onyx in the WorkspaceSwitcherButton component, we used to do this but in Ideal nav 2 PR, we removed it and shifted the subscription to the TopBar component, this caused the bug.

[allgandalf]

Proposal

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

Switcher resets to Expensify icon on page refresh

What is the root cause of that problem?

When we refresh the Page, the page remounts and the Policy prop passed becomes undefined for the Switcher Icon component:

App/src/components/WorkspaceSwitcherButton.tsx

Line 23 in a6cb60c

function WorkspaceSwitcherButton({policy}: WorkspaceSwitcherButtonProps) {

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

We should subscribe to the active Policy using Onyx in the component WorkspaceSwitcherButton, this will make sure that every time the page remounts, we are able to access the latest policy information.

We should also get the activeWorkspaceID from the Topbar and pass it down to the WorkspaceSwitcherButton this will help us in getting the policy details from Onyx.

Result:

Screen.Recording.2024-05-14.at.7.43.13.PM.mov

[melvin-bot]

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

[trjExpensify]

Interesting! @GandalfGwaihir @shawnborton I can reproduce this one, but only in this condition:

1. Click on the chats tab
2. Switch to a different workspace
3. Click the avatar in the bottom tab bar
4. Refresh the page
5. Click the chats tab in the bottom tab
6. Observe the workspace has reverted back to the Expensify "All" view

2024-05-20_11-35-52.mp4

As you can see in the video above, if I just refresh while on the Chats tab, it doesn't revert back to the All view.

CC: @adamgrzybowski @WojtekBoman @mountiny for vis on this as well.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016d7c9a80aa03fdc3

[melvin-bot]

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

[c3024]

OP here says

Actual Result:
The workspace switcher changes back to all (still filtering based on the workspace picked, but the icon and title resets back to Expensify)

so, there is clear deviation from header to LHN here in the Actual Result description.

But here,

Observe the workspace has reverted back to the Expensify "All" view

Here, header and LHN are consistent. Is this a bug?

cc: @GandalfGwaihir, does your proposal fix this case as well? ^

[trjExpensify]

@c3024 have you tested this on the latest staging? This was a repro video from a few mins ago. The only case I can see of reverting back to "All" is when you're on the settings bottom tab, not the Chats tab.

[abzokhattab]

Proposal

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

on settings refresh then navigating back to chats, the app resets to Expensify

What is the root cause of that problem?

1. When refreshing the app on the settings page, the activeWorkspaceID is cleared because it is not saved as an Onyx or route parameter.
2. Then, upon pressing the chats icon, the app navigates to the HOME endpoint.
3. On navigating to the home page, the current activeWorkspaceID is undefined, thus it falls back to the default switcher.

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

To preserve the activeWorkspaceID, we have two options:

1. Either to save the active workspace id in an oynx where it will be updated whenever the user changes the switcher. then on navigating to the chats, we should check the active workspace id using oynx and navigate to it.
2. Or we can add a new URL param activeWorkspaceID and set this param whenever the user navigates to the settings page, then inside the BottomTabBar, instead of navigating to HOME, the app should navigate to the active policy (using the useActiveRoute context) if the value is found; if not, it should navigate to HOME.

[c3024]

@trjExpensify , yes I tested on main and saw the same behaviour as you described.

@abzokhattab Is your video of before on latest main? I don't see the workspace avatar or name resetting to Expensify and All when testing on main.

workspaceSwitcher.mp4

[WojtekBoman]

The fix for that has been already merged (#40016). We don't handle this case on the settings page, because we don't store a policyID in the url there.

Screen.Recording.2024-05-20.at.14.14.52.mov

[abzokhattab]

@c3024 you are right, i updated latest to main and i couldn't reproduce the original issue.

Updated my proposal to fix the second issue

[c3024]

@abzokhattab I think we prefer saving to Onyx over using a URL param. URL params are not frequently used.

For the first option, could you share a bit more about the code changes or a branch with the changes required?

[abzokhattab]

There we go, here are the changes: main...abzokhattab:App:fix-chat-navigation

POC:

Screen.Recording.2024-05-21.at.12.31.14.PM.mov

[c3024]

@abzokhattab, thanks!

@abzokhattab's proposal here looks good to me.

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

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

[trjExpensify]

We don't handle this case on the settings page, because we don't store a policyID in the url there.

Screen.Recording.2024-05-20.at.14.14.52.mov

Thanks, @WojtekBoman. Are we cool with the proposed solution to fix this problem of going chats > switch > settings > chats > revert back to home? I'm just kinda' curious why we didn't broach this with the ideal nav v2 changes that removed the workspace from the URL path for settings. CC: @mountiny for vis

[mountiny]

I feel like both options work well, but we would have to add this to many URLs as you loose this policyID refreshing on all pages where its not in url (workspace settings).

Discussed with @WojtekBoman IRL and he will look into potential problems with the Onyx approach as then we are creating situation where there are two sources of truth and could lead to some other unintended behaviour if not done well.

If the proposal then works well, we would choose the contributor to implement it

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

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

@trjExpensify, @NikkiWines, @abzokhattab, @c3024 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[c3024]

@abzokhattab update on PR?

[abzokhattab]

Working together on it here: #42684

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

• Fix chat navigation #42684

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

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

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

[trjExpensify]

👋 checklist time, please!

[c3024]

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

#33280

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:

abzokhattab@7c7f8fa#r143387608

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 discussion has been started because this is not a bug that could be identified earlier.

Determine if we should create a regression test for this bug.

Yes

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.

Regression Test Proposal

1. Click on the Inbox in the bottom tab bar.
2. Switch to a different workspace with the workspace switcher on top left of LHN.
3. Click the avatar (Account Settings) in the bottom tab bar.
4. Refresh the page.
5. Click on the Inbox in the bottom tab bar.
6. Verify that workspace switcher shows the correct selected workspace selected in Step 2.

👍 or 👎

[trjExpensify]

Regression test issue created here.

Payment summary as follows:

• $250 to @c3024 for the C+ review
• $250 to @abzokhattab for the fix

Paid, closing!