Skip to content

Commit

Permalink
Merge pull request #47787 from software-mansion-labs/kicu/policy-ids-…
Browse files Browse the repository at this point in the history
…query

[Search] Move policyID into Search query syntax and remove policyIDs param
  • Loading branch information
luacmartins authored Aug 27, 2024
2 parents b5af0e8 + 4207b25 commit 9dc5b65
Show file tree
Hide file tree
Showing 22 changed files with 263 additions and 133 deletions.
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5292,6 +5292,7 @@ const CONST = {
STATUS: 'status',
SORT_BY: 'sortBy',
SORT_ORDER: 'sortOrder',
POLICY_ID: 'policyID',
},
SYNTAX_FILTER_KEYS: {
DATE: 'date',
Expand Down
3 changes: 1 addition & 2 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ const ROUTES = {

SEARCH_CENTRAL_PANE: {
route: 'search',
getRoute: ({query, isCustomQuery = false, policyIDs}: {query: SearchQueryString; isCustomQuery?: boolean; policyIDs?: string}) =>
`search?q=${query}&isCustomQuery=${isCustomQuery}${policyIDs ? `&policyIDs=${policyIDs}` : ''}` as const,
getRoute: ({query, isCustomQuery = false}: {query: SearchQueryString; isCustomQuery?: boolean}) => `search?q=${query}&isCustomQuery=${isCustomQuery}` as const,
},
SEARCH_ADVANCED_FILTERS: 'search/filters',
SEARCH_ADVANCED_FILTERS_DATE: 'search/filters/date',
Expand Down
5 changes: 2 additions & 3 deletions src/components/Search/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import type {SearchColumnType, SearchQueryJSON, SearchStatus, SelectedTransactio
type SearchProps = {
queryJSON: SearchQueryJSON;
isCustomQuery: boolean;
policyIDs?: string;
};

const transactionItemMobileHeight = 100;
Expand Down Expand Up @@ -74,7 +73,7 @@ function prepareTransactionsList(item: TransactionListItemType, selectedTransact
return {...selectedTransactions, [item.keyForList]: {isSelected: true, canDelete: item.canDelete, canHold: item.canHold, canUnhold: item.canUnhold, action: item.action}};
}

function Search({queryJSON, policyIDs, isCustomQuery}: SearchProps) {
function Search({queryJSON, isCustomQuery}: SearchProps) {
const {isOffline} = useNetwork();
const {translate} = useLocalize();
const styles = useThemeStyles();
Expand Down Expand Up @@ -118,7 +117,7 @@ function Search({queryJSON, policyIDs, isCustomQuery}: SearchProps) {
return;
}

SearchActions.search({queryJSON, offset, policyIDs});
SearchActions.search({queryJSON, offset});
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [isOffline, offset, queryJSON]);

Expand Down
1 change: 1 addition & 0 deletions src/components/Search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type SearchQueryAST = {
sortBy: SearchColumnType;
sortOrder: SortOrder;
filters: ASTNode;
policyID?: string;
};

type SearchQueryJSON = {
Expand Down
12 changes: 5 additions & 7 deletions src/hooks/useActiveWorkspaceFromNavigationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,20 @@ import SCREENS from '@src/SCREENS';
*/
function useActiveWorkspaceFromNavigationState() {
// The last policyID value is always stored in the last route in BottomTabNavigator.
const activeWorkpsaceID = useNavigationState<BottomTabNavigatorParamList, string | undefined>((state) => {
const activeWorkspaceID = useNavigationState<BottomTabNavigatorParamList, string | undefined>((state) => {
// SCREENS.HOME is a screen located in the BottomTabNavigator, if it's not in state.routeNames it means that this hook was called from a screen in another navigator.
if (!state.routeNames.includes(SCREENS.HOME)) {
Log.warn('useActiveWorkspaceFromNavigationState should be called only from BottomTab screens');
}

const policyID = state.routes.at(-1)?.params?.policyID;
const params = state.routes.at(-1)?.params ?? {};

if (!policyID) {
return undefined;
if ('policyID' in params) {
return params?.policyID;
}

return policyID;
});

return activeWorkpsaceID;
return activeWorkspaceID;
}

export default useActiveWorkspaceFromNavigationState;
2 changes: 0 additions & 2 deletions src/libs/API/parameters/Search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import type {SearchQueryString} from '@components/Search/types';
type SearchParams = {
hash: number;
jsonQuery: SearchQueryString;
// Tod this is temporary, remove top level policyIDs as part of: https://github.com/Expensify/App/issues/46592
policyIDs?: string;
};

export default SearchParams;
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {useOnyx} from 'react-native-onyx';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import {PressableWithFeedback} from '@components/Pressable';
import type {SearchQueryString} from '@components/Search/types';
import Tooltip from '@components/Tooltip';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useLocalize from '@hooks/useLocalize';
Expand Down Expand Up @@ -36,6 +37,34 @@ type BottomTabBarProps = {
selectedTab: string | undefined;
};

/**
* Returns SearchQueryString that has policyID correctly set.
*
* When we're coming back to Search Screen we might have pre-existing policyID inside SearchQuery.
* There are 2 cases when we might want to remove this `policyID`:
* - if Policy was removed in another screen
* - if WorkspaceSwitcher was used to globally unset a policyID
* Otherwise policyID will be inserted into query
*/
function handleQueryWithPolicyID(query: SearchQueryString, activePolicyID?: string): SearchQueryString {
const queryJSON = SearchUtils.buildSearchQueryJSON(query);
if (!queryJSON) {
return query;
}

const policyID = queryJSON.policyID ?? activePolicyID;
const policy = PolicyUtils.getPolicy(policyID);

// In case policy is missing or there is no policy currently selected via WorkspaceSwitcher we remove it
if (!activePolicyID || !policy) {
delete queryJSON.policyID;
} else {
queryJSON.policyID = policyID;
}

return SearchUtils.buildSearchQueryString(queryJSON);
}

function BottomTabBar({selectedTab}: BottomTabBarProps) {
const theme = useTheme();
const styles = useThemeStyles();
Expand Down Expand Up @@ -91,13 +120,23 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
const currentSearchParams = SearchUtils.getCurrentSearchParams();
if (currentSearchParams) {
const {q, ...rest} = currentSearchParams;
const policy = PolicyUtils.getPolicy(currentSearchParams?.policyIDs);
Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: q, ...rest, policyIDs: policy ? currentSearchParams?.policyIDs : undefined}));
const cleanedQuery = handleQueryWithPolicyID(q, activeWorkspaceID);

Navigation.navigate(
ROUTES.SEARCH_CENTRAL_PANE.getRoute({
query: cleanedQuery,
...rest,
}),
);
return;
}
Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: SearchUtils.buildCannedSearchQuery()}));

const defaultCannedQuery = SearchUtils.buildCannedSearchQuery();
// when navigating to search we might have an activePolicyID set from workspace switcher
const query = activeWorkspaceID ? `${defaultCannedQuery} ${CONST.SEARCH.SYNTAX_ROOT_KEYS.POLICY_ID}:${activeWorkspaceID}` : defaultCannedQuery;
Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query}));
});
}, [selectedTab]);
}, [activeWorkspaceID, selectedTab]);

return (
<View style={styles.bottomTabBarContainer}>
Expand Down
22 changes: 22 additions & 0 deletions src/libs/Navigation/extractPolicyIDFromQuery.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import * as SearchUtils from '@libs/SearchUtils';
import type {NavigationPartialRoute} from './types';

function extractPolicyIDFromQuery(route?: NavigationPartialRoute<string>) {
if (!route?.params) {
return undefined;
}

if (!('q' in route.params)) {
return undefined;
}

const queryString = route.params.q as string;
const queryJSON = SearchUtils.buildSearchQueryJSON(queryString);
if (!queryJSON) {
return undefined;
}

return SearchUtils.getPolicyIDFromSearchQuery(queryJSON);
}

export default extractPolicyIDFromQuery;
17 changes: 12 additions & 5 deletions src/libs/Navigation/getPolicyIDFromState.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import extractPolicyIDFromQuery from './extractPolicyIDFromQuery';
import getTopmostBottomTabRoute from './getTopmostBottomTabRoute';
import type {RootStackParamList, State} from './types';

/**
* returns policyID value if one exists in navigation state
*
* PolicyID in this app can be stored in two ways:
* - on most screens but NOT Search as `policyID` param
* - on Search related screens as policyID filter inside `q` (SearchQuery) param
*/
const getPolicyIDFromState = (state: State<RootStackParamList>): string | undefined => {
const topmostBottomTabRoute = getTopmostBottomTabRoute(state);

const shouldAddPolicyIDToUrl = !!topmostBottomTabRoute && !!topmostBottomTabRoute.params && 'policyID' in topmostBottomTabRoute.params && !!topmostBottomTabRoute.params?.policyID;

if (!shouldAddPolicyIDToUrl) {
return undefined;
const policyID = topmostBottomTabRoute && topmostBottomTabRoute.params && 'policyID' in topmostBottomTabRoute.params && topmostBottomTabRoute.params?.policyID;
if (policyID) {
return topmostBottomTabRoute.params?.policyID as string;
}

return topmostBottomTabRoute.params?.policyID as string;
return extractPolicyIDFromQuery(topmostBottomTabRoute);
};

export default getPolicyIDFromState;
21 changes: 9 additions & 12 deletions src/libs/Navigation/linkTo/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import {findFocusedRoute} from '@react-navigation/native';
import {omitBy} from 'lodash';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import isReportOpenInRHP from '@libs/Navigation/isReportOpenInRHP';
import extractPolicyIDsFromState from '@libs/Navigation/linkingConfig/extractPolicyIDsFromState';
import {isCentralPaneName} from '@libs/NavigationUtils';
import shallowCompare from '@libs/ObjectUtils';
import {extractPolicyIDFromPath, getPathWithoutPolicyID} from '@libs/PolicyUtils';
import getActionsFromPartialDiff from '@navigation/AppNavigator/getActionsFromPartialDiff';
import getPartialStateDiff from '@navigation/AppNavigator/getPartialStateDiff';
import dismissModal from '@navigation/dismissModal';
import extractPolicyIDFromQuery from '@navigation/extractPolicyIDFromQuery';
import extrapolateStateFromParams from '@navigation/extrapolateStateFromParams';
import getPolicyIDFromState from '@navigation/getPolicyIDFromState';
import getStateFromPath from '@navigation/getStateFromPath';
Expand Down Expand Up @@ -49,18 +49,18 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam
const stateFromPath = getStateFromPath(pathWithoutPolicyID) as PartialState<NavigationState<RootStackParamList>>;
// Creating path with /w/ included if necessary.
const topmostCentralPaneRoute = getTopmostCentralPaneRoute(rootState);
const policyIDs = !!topmostCentralPaneRoute?.params && 'policyIDs' in topmostCentralPaneRoute.params ? (topmostCentralPaneRoute?.params?.policyIDs as string) : '';

const extractedPolicyID = extractPolicyIDFromPath(`/${path}`);
const policyIDFromState = getPolicyIDFromState(rootState);
const policyID = extractedPolicyID ?? policyIDFromState ?? policyIDs;
const policyID = extractedPolicyID ?? policyIDFromState;
const lastRoute = rootState?.routes?.at(-1);

const isNarrowLayout = getIsNarrowLayout();

const isFullScreenOnTop = lastRoute?.name === NAVIGATORS.FULL_SCREEN_NAVIGATOR;

// policyIDs is present only on SCREENS.SEARCH.CENTRAL_PANE and it's displayed in the url as a query param, on the other pages this parameter is called policyID and it's shown in the url in the format: /w/:policyID
if (policyID && !isFullScreenOnTop && !policyIDs) {
// policyID on SCREENS.SEARCH.CENTRAL_PANE can be present only as part of SearchQuery, while on other pages it's stored in the url in the format: /w/:policyID/
if (policyID && !isFullScreenOnTop && !policyIDFromState) {
// The stateFromPath doesn't include proper path if there is a policy passed with /w/id.
// We need to replace the path in the state with the proper one.
// To avoid this hacky solution we may want to create custom getActionFromState function in the future.
Expand Down Expand Up @@ -95,8 +95,10 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam
if (isCentralPaneName(action.payload.name) && (isTargetScreenDifferentThanCurrent || areParamsDifferent)) {
// We need to push a tab if the tab doesn't match the central pane route that we are going to push.
const topmostBottomTabRoute = getTopmostBottomTabRoute(rootState);
const policyIDsFromState = extractPolicyIDsFromState(stateFromPath);
const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateFromPath, policyID || policyIDsFromState);

const focusedRoute = findFocusedRoute(stateFromPath);
const policyIDFromQuery = extractPolicyIDFromQuery(focusedRoute);
const matchingBottomTabRoute = getMatchingBottomTabRouteForState(stateFromPath, policyID ?? policyIDFromQuery);
const isOpeningSearch = matchingBottomTabRoute.name === SCREENS.SEARCH.BOTTOM_TAB;
const isNewPolicyID =
((topmostBottomTabRoute?.params as Record<string, string | undefined>)?.policyID ?? '') !==
Expand All @@ -115,11 +117,6 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam
action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH;
}

// If we navigate to SCREENS.SEARCH.CENTRAL_PANE, it's necessary to pass the current policyID, but we have to remember that this param is called policyIDs on this page
if (targetName === SCREENS.SEARCH.CENTRAL_PANE && targetParams && policyID) {
(targetParams as Record<string, string | undefined>).policyIDs = policyID;
}

// If the type is UP, we deeplinked into one of the RHP flows and we want to replace the current screen with the previous one in the flow
// and at the same time we want the back button to go to the page we were before the deeplink
} else if (type === CONST.NAVIGATION.TYPE.UP) {
Expand Down
13 changes: 0 additions & 13 deletions src/libs/Navigation/linkingConfig/extractPolicyIDsFromState.ts

This file was deleted.

9 changes: 5 additions & 4 deletions src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import type {BottomTabName, CentralPaneName, FullScreenName, NavigationPartialRo
import {isCentralPaneName} from '@libs/NavigationUtils';
import {extractPolicyIDFromPath, getPathWithoutPolicyID} from '@libs/PolicyUtils';
import * as ReportConnection from '@libs/ReportConnection';
import extractPolicyIDFromQuery from '@navigation/extractPolicyIDFromQuery';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Screen} from '@src/SCREENS';
import SCREENS from '@src/SCREENS';
import CENTRAL_PANE_TO_RHP_MAPPING from './CENTRAL_PANE_TO_RHP_MAPPING';
import config, {normalizedConfigs} from './config';
import extractPolicyIDsFromState from './extractPolicyIDsFromState';
import FULL_SCREEN_TO_RHP_MAPPING from './FULL_SCREEN_TO_RHP_MAPPING';
import getMatchingBottomTabRouteForState from './getMatchingBottomTabRouteForState';
import getMatchingCentralPaneRouteForState from './getMatchingCentralPaneRouteForState';
Expand Down Expand Up @@ -379,10 +379,11 @@ const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options) => {
throw new Error('Unable to parse path');
}

// Only on SCREENS.SEARCH.CENTRAL_PANE policyID is stored differently as "policyIDs" param, so we're handling this case here
const policyIDs = extractPolicyIDsFromState(state);
// On SCREENS.SEARCH.CENTRAL_PANE policyID is stored differently inside search query ("q" param), so we're handling this case
const focusedRoute = findFocusedRoute(state);
const policyIDFromQuery = extractPolicyIDFromQuery(focusedRoute);

return getAdaptedState(state, policyID ?? policyIDs);
return getAdaptedState(state, policyID ?? policyIDFromQuery);
};

export default getAdaptedStateFromPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,9 @@ function getMatchingBottomTabRouteForState(state: State<RootStackParamList>, pol

if (tabName === SCREENS.SEARCH.BOTTOM_TAB) {
const topmostCentralPaneRouteParams = {...topmostCentralPaneRoute.params} as Record<string, string | undefined>;
delete topmostCentralPaneRouteParams?.policyIDs;
if (policyID) {
topmostCentralPaneRouteParams.policyID = policyID;
}
return {name: tabName, params: topmostCentralPaneRouteParams};
}

return {name: tabName, params: paramsWithPolicyID};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function setupCustomAndroidBackHandler() {
const bottomTabRoutes = bottomTabRoute?.state?.routes;
const focusedRoute = findFocusedRoute(rootState);

// Shoudn't happen but for type safety.
// Shouldn't happen but for type safety.
if (!bottomTabRoutes) {
return false;
}
Expand All @@ -38,27 +38,27 @@ function setupCustomAndroidBackHandler() {
const bottomTabRouteAfterPop = bottomTabRoutes.at(-2);

// It's possible that central pane search is desynchronized with the bottom tab search.
// e.g. opening a tab different than search will wipe out central pane screens.
// e.g. opening a tab different from search will wipe out central pane screens.
// In that case we have to push the proper one.
if (
bottomTabRouteAfterPop &&
bottomTabRouteAfterPop.name === SCREENS.SEARCH.BOTTOM_TAB &&
(!centralPaneRouteAfterPop || centralPaneRouteAfterPop.name !== SCREENS.SEARCH.CENTRAL_PANE)
) {
const {policyID, ...restParams} = bottomTabRoutes[bottomTabRoutes.length - 2].params as SearchPageProps['route']['params'];
navigationRef.dispatch({...StackActions.push(SCREENS.SEARCH.CENTRAL_PANE, {...restParams, policyIDs: policyID})});
const searchParams = bottomTabRoutes[bottomTabRoutes.length - 2].params as SearchPageProps['route']['params'];
navigationRef.dispatch({...StackActions.push(SCREENS.SEARCH.CENTRAL_PANE, searchParams)});
}

return true;
}

// Handle back press to go back to the search page.
// It's possible that central pane search is desynchronized with the bottom tab search.
// e.g. opening a tab different than search will wipe out central pane screens.
// e.g. opening a tab different from search will wipe out central pane screens.
// In that case we have to push the proper one.
if (bottomTabRoutes && bottomTabRoutes?.length >= 2 && bottomTabRoutes[bottomTabRoutes.length - 2].name === SCREENS.SEARCH.BOTTOM_TAB && rootState?.routes?.length === 1) {
const {policyID, ...restParams} = bottomTabRoutes[bottomTabRoutes.length - 2].params as SearchPageProps['route']['params'];
navigationRef.dispatch({...StackActions.push(SCREENS.SEARCH.CENTRAL_PANE, {...restParams, policyIDs: policyID})});
const searchParams = bottomTabRoutes[bottomTabRoutes.length - 2].params as SearchPageProps['route']['params'];
navigationRef.dispatch({...StackActions.push(SCREENS.SEARCH.CENTRAL_PANE, searchParams)});
navigationRef.dispatch({...StackActions.pop(), target: bottomTabRoute?.state?.key});
return true;
}
Expand Down
Loading

0 comments on commit 9dc5b65

Please sign in to comment.