[$250] mWeb - Workspace - Opens "Hmm it's not here" when tapping system back button

[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: 9.0.40
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5007250
Issue reported by: Applause - Internal Team

Action Performed:

Employee's side

1. Log in to the app as a workspace employee.
2. Navigate to Workspace settings > Profile
3. Tap on the system back button
4. Tap on Members (or Profile again)
5. Tap on the system back button
   Admin's side
6. Log in to the app as a workspace admin.
7. Navigate to Workspace settings
8. Tap on any setting, for instance, Workflows
9. Tap on the system back button
10. Tap on any other setting (or the same tapped in step 3)
11. Tap on the system back button

Expected Result:

The user returns to the Workspace settings page

Actual Result:

The "Hmm... it's not here" page is displayed:
Hmm... it's not here
Oops, this page cannot be found

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

Bug6617565_1727454588873.it_s_not_here-admin_s_side.mp4.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021843764334226859970
• Upwork Job ID: 1843764334226859970
• Last Price Increase: 2024-10-08

Issue Owner

Current Issue Owner: @ntdiary

[melvin-bot]

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

[lanitochka17]

@jliexpensify 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

[jliexpensify]

I can't repro this on 9.0.41-2 (Pixel 6)

[lanitochka17]

Issue is still reproducible on the latest build 9.0.44-10
These steps should help you reproduce the issue:

1. Go to settings click on workspaces and open one
2. Click on members
3. Click on 3 dots at top right corner
4. Click phone back button
5. Observe user is taken to workspace detail page then click members again
6. Click on 3 dots again and then click on the phones back button
7. Observe that hmm not found page is shown

Bug6618542_1727536595923.Screen_Recording_20240928_180655.mp4

[c3024]

Edited by proposal-police: This proposal was edited at 2024-10-05 17:48:57 UTC.

Proposal

Please restate the problem that we are trying to solve in this issue.

Navigating back with the device back button disrupts the URL when navigating from pages like “Members” and “Categories" etc. on narrow layouts.

What is the root cause of that problem?

This happens specifically for the settings/workspaces/:policyID route. On wide layout, this route adds a central pane and navigates to Profile by default and adds profile at the end of the link. Clicking on Members and back takes to Profile page. If we press back button again it takes to the workspaces page.

However, on narrow layouts, the route is settings/workspaces/:policyID and no profile is added to the link. So navigating to members page and then back with device button takes us back to this link.

As mentioned here, workspace routes can give an improper path with getStateFromPath. In this case, the path is settings/workspace/<policyID>. So, we normalize the path and replace this path in state.

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Lines 373 to 383 in 99f280b

	const normalizedPath = !path.startsWith('/') ? `/${path}` : path;
	const pathWithoutPolicyID = getPathWithoutPolicyID(normalizedPath);
	const isAnonymous = isAnonymousUser();
	// Anonymous users don't have access to workspaces
	const policyID = isAnonymous ? undefined : extractPolicyIDFromPath(path);
	const state = getStateFromPath(pathWithoutPolicyID, options) as PartialState<NavigationState<RootStackParamList>>;
	if (shouldReplacePathInNestedState) {
	replacePathInNestedState(state, path);
	}

We replace the nested path by mutating the state directly here.

App/src/libs/Navigation/linkingConfig/replacePathInNestedState.ts

Lines 6 to 14 in 99f280b

	function replacePathInNestedState(state: PartialState<NavigationState<RootStackParamList>>, path: string) {
	const found = findFocusedRoute(state);
	if (!found) {
	return;
	}
	// @ts-expect-error Updating read only property
	found.path = path;
	}

When the path changes with the device back button, React Navigation needs to adjust its internal state. If the state is mutated during this process, like directly setting a route’s value as above, it can lead to unpredictable behaviour including extra popState events. It should not be mutated.

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

We can create a new object and return it from replacePathInNestedState as follows:

function replacePathInNestedState(state: PartialState<NavigationState<RootStackParamList>>, path: string) {
    const found = findFocusedRoute(state);
    if (!found) {
        return;
    }
    const updatedRoute = {
        ...found,
        path: path,
    };
    const newState = {
        ...state,
        routes: state.routes.map(route =>
          route === found ? updatedRoute : route
        ),
    };
    return newState;
}

and use that here:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Line 382 in 99f280b

replacePathInNestedState(state, path);

as

state = replacePathInNestedState(state, path);

What alternative solutions did you explore? (Optional)

As far as I understand, the path issue with the ‘/’ at the start is handled here:

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Line 373 in 99f280b

const normalizedPath = !path.startsWith('/') ? `/${path}` : path;

I have not found any other cases where getPathFromState does not return the correct path. The getPathWithoutPolicyID functions also check with a regex in the format of /w/, which I have not found anywhere in ROUTES. So, we might consider removing this function replacePathInNestedState completely.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[ntdiary]

During the review, the inspection of the above proposal may take another day. 🤔

[ntdiary]

Hi, @adamgrzybowski, do you remember which specific issue the replacePathInNestedState function was meant to address? I only found that you added this function, but couldn't find any related issues, would appreciate if you could provide more context. ❤️
PR: #33280 (comment)

When we open a workspace from the WorkspaceListPage, replacePathInNestedState replaces the path in the state with a path that doesn’t have a / prefix, which is inconsistent with the path stored in memory history. This seems to have caused the url not to display correctly if we open the members page and then click the browser's back button.

[c3024]

Oh, for some reason I misread the path here

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Line 378 in 9d86f30

replacePathInNestedState(state, path);

as normalizedPath.

So, I could not identify where the '/' was being removed.

[ntdiary]

Oh, for some reason I misread the path here

App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts

Line 378 in 9d86f30

replacePathInNestedState(state, path);

as normalizedPath.
So, I could not identify where the '/' was being removed.

I'm still digging in and verifying. 😄

[melvin-bot]

@ntdiary @jliexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[adamgrzybowski]

Hey @ntdiary, sorry for the delay. Unfortunately, I don't remember the exact case. It was probably something like going back (with the in app button) to this page causing a wrong path in the browser's URL bar if the path wasn't corrected in the state.

[ntdiary]

Hey @ntdiary, sorry for the delay. Unfortunately, I don't remember the exact case. It was probably something like going back (with the in app button) to this page causing a wrong path in the browser's URL bar if the path wasn't corrected in the state.

@adamgrzybowski, thank you! :)

[ntdiary]

I have not found any other cases where getPathFromState does not return the correct path. The getPathWithoutPolicyID functions also check with a regex in the format of /w/, which I have not found anywhere in ROUTES. So, we might consider removing this function replacePathInNestedState completely.

@c3024, we have a workspace switcher, and the /w URL is used when switching workspaces.

Based on my investigation, the issue seems more related to a mismatch between two paths in the memory history.

Instead of having replacePathInNestedState return a new state, perhaps I’m leaning more toward avoiding the use of replacePathInNestedState, maybe change something like this:

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

Line 136 in 8ab4792

const {adaptedState, metainfo} = getAdaptedStateFromPath(path, linkingConfig.config);

const {adaptedState, metainfo} = getAdaptedStateFromPath(path, linkingConfig.config, false);

Additionally, I’m working on testing more accurate and comprehensive cases, so that we can move forward with more confidence. :)