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

[Search] Move policyID into Search query syntax and remove policyIDs param #47787

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5340,6 +5340,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
2 changes: 1 addition & 1 deletion src/components/Search/SearchPageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function SearchPageHeader({queryJSON, hash, onSelectDeleteOption, setOfflineModa
[data, selectedTransactions],
);
const {status} = queryJSON;
const headerSubtitle = isCustomQuery ? SearchUtils.getSearchHeaderTitle(queryJSON) : translate(headerContent[status]?.titleText);
const headerSubtitle = isCustomQuery ? SearchUtils.getSearchHeaderTitle(queryJSON) : translate(headerContent[status]?.titleText ?? '');
const headerTitle = isCustomQuery ? translate('search.filtersHeader') : '';
const headerIcon = isCustomQuery ? Illustrations.Filters : headerContent[status]?.icon;

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 @@ -36,7 +36,6 @@ import type {SearchColumnType, SearchQueryJSON, SearchStatus, SelectedTransactio
type SearchProps = {
queryJSON: SearchQueryJSON;
isCustomQuery: boolean;
policyIDs?: string;
};

const transactionItemMobileHeight = 100;
Expand Down Expand Up @@ -73,7 +72,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 @@ -117,7 +116,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 @@ -60,6 +60,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) => {
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
// 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 All @@ -18,7 +19,7 @@ import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import type {RootStackParamList, State} from '@libs/Navigation/types';
import {isCentralPaneName} from '@libs/NavigationUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import {getCurrentSearchParams} from '@libs/SearchUtils';
import * as SearchUtils from '@libs/SearchUtils';
import type {BrickRoad} from '@libs/WorkspacesSettingsUtils';
import {getChatTabBrickRoad} from '@libs/WorkspacesSettingsUtils';
import BottomTabAvatar from '@pages/home/sidebar/BottomTabAvatar';
Expand All @@ -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 @@ -88,16 +117,24 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
return;
}
interceptAnonymousUser(() => {
const currentSearchParams = getCurrentSearchParams();
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: CONST.SEARCH.TAB.EXPENSE.ALL}));

const query = activeWorkspaceID ? `${CONST.SEARCH.TAB.EXPENSE.ALL} policyID:${activeWorkspaceID}` : CONST.SEARCH.TAB.EXPENSE.ALL;
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) {
// Policy on SCREENS.SEARCH.CENTRAL_PANE can be present only as part of SearchQuery, while on other pages it's called `policyID` and stored in the url in the format: /w/:policyID/
Kicu marked this conversation as resolved.
Show resolved Hide resolved
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.

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
Loading
Loading