Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: app goes to workspace initial page when back from not found page #49226

Merged
merged 19 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
511c9ad
fix: app goes to workspace initial page when back from not found page
dominictb Sep 15, 2024
1df33b4
modify add route condition
dominictb Sep 15, 2024
2e0ee31
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Sep 18, 2024
8842853
merge main
dominictb Sep 24, 2024
e43dcd1
fix: add comment
dominictb Sep 24, 2024
c904709
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Sep 26, 2024
8603898
Merge branch 'fix/46646' of https://github.com/dominictb/epsf-app int…
dominictb Sep 26, 2024
a34be5b
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 3, 2024
4e693e4
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 11, 2024
09e9fb1
do not use hook to get isLoadingReportData
dominictb Oct 11, 2024
b90ad82
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 14, 2024
9e9b2a8
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 15, 2024
d0c483a
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 17, 2024
68e8297
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 21, 2024
d0d5b4c
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 22, 2024
0386167
fix: initial workspace is removed in 3-pane layout
dominictb Oct 22, 2024
33b1993
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 23, 2024
da44573
Merge branch 'main' of https://github.com/dominictb/epsf-app into fix…
dominictb Oct 29, 2024
63029f1
fix: stale isLoadingReportData Onyx value
dominictb Oct 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,19 +1,33 @@
import type {ParamListBase, PartialState, RouterConfigOptions, StackNavigationState} from '@react-navigation/native';
import {StackRouter} from '@react-navigation/native';
import {useOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import * as PolicyUtils from '@libs/PolicyUtils';
import ONYXKEYS from '@src/ONYXKEYS';
import SCREENS from '@src/SCREENS';
import type {FullScreenNavigatorRouterOptions} from './types';

type StackState = StackNavigationState<ParamListBase> | PartialState<StackNavigationState<ParamListBase>>;

const isAtLeastOneInState = (state: StackState, screenName: string): boolean => state.routes.some((route) => route.name === screenName);

function adaptStateIfNecessary(state: StackState) {
function adaptStateIfNecessary(state: StackState, isLoadingReportData: OnyxEntry<boolean>) {
const isNarrowLayout = getIsNarrowLayout();
const workspaceCentralPane = state.routes.at(-1);
const policyID =
workspaceCentralPane?.params && 'policyID' in workspaceCentralPane.params && typeof workspaceCentralPane.params.policyID === 'string'
? workspaceCentralPane.params.policyID
: undefined;

// There should always be WORKSPACE.INITIAL screen in the state to make sure go back works properly if we deeplinkg to a subpage of settings.
dominictb marked this conversation as resolved.
Show resolved Hide resolved
if (!isAtLeastOneInState(state, SCREENS.WORKSPACE.INITIAL)) {
const policy = PolicyUtils.getPolicy(policyID ?? '');
const isPolicyAccessible = PolicyUtils.isPolicyAccessible(policy);
if (!isLoadingReportData && !isPolicyAccessible) {
return;
}

// @ts-expect-error Updating read only property
// noinspection JSConstantReassignment
state.stale = true; // eslint-disable-line
Expand Down Expand Up @@ -51,12 +65,13 @@ function adaptStateIfNecessary(state: StackState) {

function CustomFullScreenRouter(options: FullScreenNavigatorRouterOptions) {
const stackRouter = StackRouter(options);
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA, {initialValue: true});

return {
...stackRouter,
getInitialState({routeNames, routeParamList, routeGetIdList}: RouterConfigOptions) {
const initialState = stackRouter.getInitialState({routeNames, routeParamList, routeGetIdList});
adaptStateIfNecessary(initialState);
adaptStateIfNecessary(initialState, isLoadingReportData);

// If we needed to modify the state we need to rehydrate it to get keys for new routes.
if (initialState.stale) {
Expand All @@ -66,7 +81,7 @@ function CustomFullScreenRouter(options: FullScreenNavigatorRouterOptions) {
return initialState;
},
getRehydratedState(partialState: StackState, {routeNames, routeParamList, routeGetIdList}: RouterConfigOptions): StackNavigationState<ParamListBase> {
adaptStateIfNecessary(partialState);
adaptStateIfNecessary(partialState, isLoadingReportData);
const state = stackRouter.getRehydratedState(partialState, {routeNames, routeParamList, routeGetIdList});
return state;
},
Expand Down
14 changes: 14 additions & 0 deletions src/libs/Navigation/Navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import type {HybridAppRoute, Route} from '@src/ROUTES';
import ROUTES, {HYBRID_APP_ROUTES} from '@src/ROUTES';
import {PROTECTED_SCREENS} from '@src/SCREENS';
import type {Screen} from '@src/SCREENS';
import type {Report} from '@src/types/onyx';
import originalCloseRHPFlow from './closeRHPFlow';
import originalDismissModal from './dismissModal';
Expand Down Expand Up @@ -410,6 +411,18 @@ function getTopMostCentralPaneRouteFromRootState() {
return getTopmostCentralPaneRoute(navigationRef.getRootState() as State<RootStackParamList>);
}

function removeScreenFromNavigationState(screen: Screen) {
navigationRef.dispatch((state) => {
const routes = state.routes?.filter((item) => item.name !== screen);

return CommonActions.reset({
...state,
routes,
index: routes.length < state.routes.length ? state.index - 1 : state.index,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index: routes.length - 1 should be enough. The index for the stack points to the last element of the array.

BTW the current implementation for calculating the index would break if there is more than one screen with given name. It's not possible for the workspace initial but it is possible for other screens.

Maybe we should limit this function to filter out just the last found route with this name? It may make sense even with fixed index implementation to limit this function.

Also maybe we don't have to call reset if we haven't removed any route?

Copy link
Contributor Author

@dominictb dominictb Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing, I found this function does not work properly since the Workspace_Initial is always inside FullScreenNavigator, so we cannot filter out the Workspace_Initial by using state.routes?.filter((item) => item.name !== screen). It prevents the app from working properly when deep linking.

});
});
}

export default {
setShouldPopAllStateOnUP,
navigate,
Expand All @@ -433,6 +446,7 @@ export default {
closeRHPFlow,
setNavigationActionToMicrotaskQueue,
getTopMostCentralPaneRouteFromRootState,
removeScreenFromNavigationState,
};

export {navigationRef};
5 changes: 5 additions & 0 deletions src/libs/PolicyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,10 @@ function getWorkflowApprovalsUnavailable(policy: OnyxEntry<Policy>) {
return policy?.approvalMode === CONST.POLICY.APPROVAL_MODE.OPTIONAL || !!policy?.errorFields?.approvalMode;
}

function isPolicyAccessible(policy: OnyxEntry<Policy>) {
return !isEmptyObject(policy) && (Object.keys(policy).length !== 1 || isEmptyObject(policy.errors)) && policy?.id;
}

export {
canEditTaxRate,
extractPolicyIDFromPath,
Expand Down Expand Up @@ -1105,6 +1109,7 @@ export {
getTagNamesFromTagsLists,
getDomainNameForPolicy,
getWorkflowApprovalsUnavailable,
isPolicyAccessible,
};

export type {MemberEmailsToAccountIDs};
10 changes: 9 additions & 1 deletion src/pages/workspace/AccessOrNotFoundWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {IOUType} from '@src/CONST';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import type {PolicyFeatureName} from '@src/types/onyx/Policy';
import callOrReturn from '@src/types/utils/callOrReturn';
Expand Down Expand Up @@ -132,7 +133,7 @@ function AccessOrNotFoundWrapper({accessVariants = [], fullPageNotFoundViewProps
return acc && accessFunction(policy, login, report, allPolicies ?? null, iouType);
}, true);

const isPolicyNotAccessible = isEmptyObject(policy) || (Object.keys(policy).length === 1 && !isEmptyObject(policy.errors)) || !policy?.id;
const isPolicyNotAccessible = !PolicyUtils.isPolicyAccessible(policy);
const shouldShowNotFoundPage = (!isMoneyRequest && !isFromGlobalCreate && isPolicyNotAccessible) || !isPageAccessible || !isPolicyFeatureEnabled || shouldBeBlocked;

// We only update the feature state if it isn't pending.
Expand All @@ -145,6 +146,13 @@ function AccessOrNotFoundWrapper({accessVariants = [], fullPageNotFoundViewProps
setIsPolicyFeatureEnabled(isFeatureEnabled);
}, [pendingField, isOffline, isFeatureEnabled]);

useEffect(() => {
if (!isPolicyNotAccessible) {
return;
}
Navigation.removeScreenFromNavigationState(SCREENS.WORKSPACE.INITIAL);
}, [isPolicyNotAccessible]);

if (shouldShowFullScreenLoadingIndicator) {
return <FullscreenLoadingIndicator />;
}
Expand Down
Loading