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.

Sign up for GitHub

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 v2] [App] Use new query syntax #45617

Merged
merged 16 commits into from
Jul 25, 2024
+231 −198
Merged

[Search v2] [App] Use new query syntax #45617

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
25acc96
use new query syntax
adamgrzybowski Jul 19, 2024
fe0ec54
Merge branch 'main' into adamgrzybowski/use-new-query-syntax
adamgrzybowski Jul 19, 2024
6f9ba6d
fix problems after merge
adamgrzybowski Jul 19, 2024
0a8edf0
Merge branch 'main' into adamgrzybowski/use-new-query-syntax
adamgrzybowski Jul 19, 2024
9eceb03
remove default query
adamgrzybowski Jul 19, 2024
1c7e908
improve handling undefined queryJSON
adamgrzybowski Jul 22, 2024
0987879
fix imports for SearchStatusMenu
adamgrzybowski Jul 22, 2024
a4d185a
add review suggestions
adamgrzybowski Jul 23, 2024
ddd1b91
remove unecessary comment
adamgrzybowski Jul 23, 2024
fb83ab1
remove cq and add isCustomQuery
adamgrzybowski Jul 23, 2024
ab849e0
fix searchV2
adamgrzybowski Jul 24, 2024
c7af40c
fix offset and remove it from query
adamgrzybowski Jul 24, 2024
f758cc4
Merge branch 'main' into adamgrzybowski/use-new-query-syntax
Kicu Jul 25, 2024
84ee1f9
Add fixes after review
Kicu Jul 25, 2024
4741f7a
Fix search hash not reacting to policyIDs change
Kicu Jul 25, 2024
1fc7be1
Fix lint
Kicu Jul 25, 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
14 changes: 13 additions & 1 deletion src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5210,12 +5210,20 @@ const CONST = {
ASC: 'asc',
DESC: 'desc',
},
TAB: {
STATUS: {
ALL: 'all',
SHARED: 'shared',
DRAFTS: 'drafts',
FINISHED: 'finished',
},
TAB: {
EXPENSE: {
ALL: 'type:expense status:all',
SHARED: 'type:expense status:shared',
DRAFTS: 'type:expense status:drafts',
FINISHED: 'type:expense status:finished',
},
},
TABLE_COLUMNS: {
RECEIPT: 'receipt',
DATE: 'date',
Expand Down Expand Up @@ -5263,6 +5271,10 @@ const CONST = {
REPORT_ID: 'reportID',
KEYWORD: 'keyword',
},
QUERY_KIND: {
luacmartins marked this conversation as resolved.
Show resolved Hide resolved
CANNED_QUERY: 'cannedQuery',
CUSTOM_QUERY: 'customQuery',
},
},

REFERRER: {
Expand Down
27 changes: 8 additions & 19 deletions src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type {TupleToUnion, ValueOf} from 'type-fest';
import type CONST from './CONST';
import type {QueryKind, SearchQueryString} from './components/Search/types';
import CONST from './CONST';
import type {IOUAction, IOUType} from './CONST';
import type {IOURequestType} from './libs/actions/IOU';
import type {AuthScreensParamList} from './libs/Navigation/types';
import type {ConnectionName, SageIntacctMappingName} from './types/onyx/Policy';
import type {SearchQuery} from './types/onyx/SearchResults';
import type AssertTypesNotEqual from './types/utils/AssertTypesNotEqual';

// This is a file containing constants for all the routes we want to be able to go to
Expand Down Expand Up @@ -37,16 +36,9 @@ const ROUTES = {
ALL_SETTINGS: 'all-settings',

SEARCH_CENTRAL_PANE: {
route: '/search/:query',
getRoute: (searchQuery: SearchQuery, queryParams?: AuthScreensParamList['Search_Central_Pane']) => {
const {sortBy, sortOrder} = queryParams ?? {};

if (!sortBy && !sortOrder) {
return `search/${searchQuery}` as const;
}

return `search/${searchQuery}?sortBy=${sortBy}&sortOrder=${sortOrder}` as const;
},
route: 'search',
getRoute: ({query, queryKind = CONST.SEARCH.QUERY_KIND.CANNED_QUERY, policyIDs}: {query: SearchQueryString; queryKind?: QueryKind; policyIDs?: string}) =>
`search?${queryKind === CONST.SEARCH.QUERY_KIND.CANNED_QUERY ? 'q' : 'cq'}=${query}${policyIDs ? `&policyIDs=${policyIDs}` : ''}` as const,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to include the policyID in the query as well so that the AST includes it in the filters. Any reason to treat it differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a transitional step.

BTW will current grammar handle policyID? I don't see this keyword in the .peggy file

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah makes sense that we do it like this for now. We should eventually move it to the AST and include it in the peggy grammar too

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a separate issue to migrate the policyID param to the AST?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it should be in the root of QueryJSON? So something like

type JSONQuery = {
    input: string;
    hash: number;
    type: string;
    status: string;
    sortBy: string;
    sortOrder: string;
    offset: number;
    filters: ASTNode;
    policyID: string;
};

Also currently it is called policyIDs, as the assumption was that we wanted to be able to use multiple policyIDs in the future. Is this still the case?

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll add it to filters as policyID. So the AST will look like this:

{filters: {
    "operator": "eq",
    "left": "policyID",
    "right": "<policyID>"

I think we should use policyID singular from now on since we have a bunch of other filters, e.g. category, tag, etc that accepts multiple values but are all singular too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hope I understand this correctly. I will add policyID to grammar and keep it inside AST, while removing it from URL

Copy link
Contributor

Choose a reason for hiding this comment

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

read my comment here: #45617 (comment)

},

SEARCH_ADVANCED_FILTERS: 'search/filters',
Expand All @@ -56,14 +48,11 @@ const ROUTES = {
SEARCH_ADVANCED_FILTERS_TYPE: 'search/filters/type',

SEARCH_REPORT: {
route: 'search/:query/view/:reportID',
getRoute: (query: string, reportID: string) => `search/${query}/view/${reportID}` as const,
route: 'search/view/:reportID',
getRoute: (reportID: string) => `search/view/${reportID}` as const,
},

TRANSACTION_HOLD_REASON_RHP: {
route: 'search/:query/hold',
getRoute: (query: string) => `search/${query}/hold` as const,
},
TRANSACTION_HOLD_REASON_RHP: 'search/hold',

// This is a utility route used to go to the user's concierge chat, or the sign-in page if the user's not authenticated
CONCIERGE: 'concierge',
Expand Down
5 changes: 2 additions & 3 deletions src/components/PromotedActionsBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as HeaderUtils from '@libs/HeaderUtils';
import * as Localize from '@libs/Localize';
import getTopmostCentralPaneRoute from '@libs/Navigation/getTopmostCentralPaneRoute';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import type {AuthScreensParamList, RootStackParamList, State} from '@libs/Navigation/types';
import type {RootStackParamList, State} from '@libs/Navigation/types';
import * as ReportUtils from '@libs/ReportUtils';
import * as ReportActions from '@userActions/Report';
import * as Session from '@userActions/Session';
Expand Down Expand Up @@ -80,8 +80,7 @@ const PromotedActions = {
return;
}

const currentQuery = topmostCentralPaneRoute?.params as AuthScreensParamList['Search_Central_Pane'];
ReportUtils.changeMoneyRequestHoldStatus(reportAction, ROUTES.SEARCH_REPORT.getRoute(currentQuery?.query ?? CONST.SEARCH.TAB.ALL, reportAction?.childReportID ?? ''));
ReportUtils.changeMoneyRequestHoldStatus(reportAction, ROUTES.SEARCH_REPORT.getRoute(reportAction?.childReportID ?? ''));
},
}),
} satisfies PromotedActionsType;
Expand Down
10 changes: 5 additions & 5 deletions src/components/Search/SearchListWithHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import useWindowDimensions from '@hooks/useWindowDimensions';
import * as SearchActions from '@libs/actions/Search';
import * as SearchUtils from '@libs/SearchUtils';
import CONST from '@src/CONST';
import type {SearchDataTypes, SearchQuery, SearchReport} from '@src/types/onyx/SearchResults';
import type {SearchDataTypes, SearchReport} from '@src/types/onyx/SearchResults';
import SearchPageHeader from './SearchPageHeader';
import type {SelectedTransactionInfo, SelectedTransactions} from './types';
import type {SearchStatus, SelectedTransactionInfo, SelectedTransactions} from './types';

type SearchListWithHeaderProps = Omit<BaseSelectionListProps<ReportListItemType | TransactionListItemType>, 'onSelectAll' | 'onCheckboxPress' | 'sections'> & {
query: SearchQuery;
status: SearchStatus;
hash: number;
data: TransactionListItemType[] | ReportListItemType[];
searchType: SearchDataTypes;
Expand All @@ -44,7 +44,7 @@ function mapToItemWithSelectionInfo(item: TransactionListItemType | ReportListIt
}

function SearchListWithHeader(
{ListItem, onSelectRow, query, hash, data, searchType, isMobileSelectionModeActive, setIsMobileSelectionModeActive, ...props}: SearchListWithHeaderProps,
{ListItem, onSelectRow, status, hash, data, searchType, isMobileSelectionModeActive, setIsMobileSelectionModeActive, ...props}: SearchListWithHeaderProps,
ref: ForwardedRef<SelectionListHandle>,
) {
const {isSmallScreenWidth} = useWindowDimensions();
Expand Down Expand Up @@ -188,7 +188,7 @@ function SearchListWithHeader(
<SearchPageHeader
selectedTransactions={selectedTransactions}
clearSelectedItems={clearSelectedItems}
query={query}
status={status}
hash={hash}
onSelectDeleteOption={handleOnSelectDeleteOption}
isMobileSelectionModeActive={isMobileSelectionModeActive}
Expand Down
20 changes: 10 additions & 10 deletions src/components/Search/SearchPageHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ import SearchSelectedNarrow from '@pages/Search/SearchSelectedNarrow';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type {SearchQuery, SearchReport} from '@src/types/onyx/SearchResults';
import type {SearchReport} from '@src/types/onyx/SearchResults';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
import type IconAsset from '@src/types/utils/IconAsset';
import getDownloadOption from './SearchActionOptionsUtils';
import {useSearchContext} from './SearchContext';
import type {SelectedTransactions} from './types';
import type {SearchStatus, SelectedTransactions} from './types';

type SearchPageHeaderProps = {
query: SearchQuery;
status: SearchStatus;
selectedTransactions?: SelectedTransactions;
selectedReports?: Array<SearchReport['reportID']>;
clearSelectedItems?: () => void;
Expand All @@ -39,7 +39,7 @@ type SearchPageHeaderProps = {
type SearchHeaderOptionValue = DeepValueOf<typeof CONST.SEARCH.BULK_ACTION_TYPES> | undefined;

function SearchPageHeader({
query,
status,
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
selectedTransactions = {},
hash,
clearSelectedItems,
Expand All @@ -58,7 +58,7 @@ function SearchPageHeader({
const {isSmallScreenWidth} = useResponsiveLayout();
const {setSelectedTransactionIDs} = useSearchContext();

const headerContent: {[key in SearchQuery]: {icon: IconAsset; title: string}} = {
const headerContent: {[key in SearchStatus]: {icon: IconAsset; title: string}} = {
all: {icon: Illustrations.MoneyReceipts, title: translate('common.expenses')},
shared: {icon: Illustrations.SendMoney, title: translate('common.shared')},
drafts: {icon: Illustrations.Pencil, title: translate('common.drafts')},
Expand All @@ -81,7 +81,7 @@ function SearchPageHeader({
return;
}

SearchActions.exportSearchItemsToCSV(query, selectedReports, selectedTransactionsKeys, [activeWorkspaceID ?? ''], () => {
SearchActions.exportSearchItemsToCSV(status, selectedReports, selectedTransactionsKeys, [activeWorkspaceID ?? ''], () => {
setDownloadErrorModalOpen?.();
});
});
Expand Down Expand Up @@ -109,7 +109,7 @@ function SearchPageHeader({
setIsMobileSelectionModeActive?.(false);
}
setSelectedTransactionIDs(selectedTransactionsKeys);
Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP.getRoute(query));
Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP);
},
});
}
Expand Down Expand Up @@ -176,6 +176,7 @@ function SearchPageHeader({

return options;
}, [
status,
selectedTransactionsKeys,
selectedTransactions,
translate,
Expand All @@ -187,7 +188,6 @@ function SearchPageHeader({
theme.icon,
styles.colorMuted,
styles.fontWeightNormal,
query,
isOffline,
setOfflineModalOpen,
setDownloadErrorModalOpen,
Expand All @@ -211,8 +211,8 @@ function SearchPageHeader({

return (
<HeaderWithBackButton
title={headerContent[query]?.title}
icon={headerContent[query]?.icon}
title={headerContent[status]?.title}
icon={headerContent[status]?.icon}
shouldShowBackButton={false}
>
{headerButtonsOptions.length > 0 && (
Expand Down
56 changes: 36 additions & 20 deletions src/components/Search/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,34 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SearchResults from '@src/types/onyx/SearchResults';
import type {SearchDataTypes, SearchQuery} from '@src/types/onyx/SearchResults';
import type {SearchDataTypes} from '@src/types/onyx/SearchResults';
import {useSearchContext} from './SearchContext';
import SearchListWithHeader from './SearchListWithHeader';
import SearchPageHeader from './SearchPageHeader';
import type {SearchColumnType, SortOrder} from './types';
import type {SearchColumnType, SearchQueryJSON, SearchStatus, SortOrder} from './types';

type SearchProps = {
query: SearchQuery;
queryJSON: SearchQueryJSON;
policyIDs?: string;
sortBy?: SearchColumnType;
sortOrder?: SortOrder;
isMobileSelectionModeActive?: boolean;
setIsMobileSelectionModeActive?: (isMobileSelectionModeActive: boolean) => void;
};

const sortableSearchTabs: SearchQuery[] = [CONST.SEARCH.TAB.ALL];
const sortableSearchTabs: SearchStatus[] = [CONST.SEARCH.STATUS.ALL];
const transactionItemMobileHeight = 100;
const reportItemTransactionHeight = 52;
const listItemPadding = 12; // this is equivalent to 'mb3' on every transaction/report list item
const searchHeaderHeight = 54;
function Search({query, policyIDs, sortBy, sortOrder, isMobileSelectionModeActive, setIsMobileSelectionModeActive}: SearchProps) {
function Search({queryJSON, policyIDs, isMobileSelectionModeActive, setIsMobileSelectionModeActive}: SearchProps) {
const {isOffline} = useNetwork();
const styles = useThemeStyles();
const {isLargeScreenWidth, isSmallScreenWidth} = useWindowDimensions();
const navigation = useNavigation<StackNavigationProp<AuthScreensParamList>>();
const lastSearchResultsRef = useRef<OnyxEntry<SearchResults>>();
const {setCurrentSearchHash} = useSearchContext();

const hash = SearchUtils.getQueryHash(query, policyIDs, sortBy, sortOrder);
const {status, sortBy, sortOrder, hash} = queryJSON;

const [currentSearchResults] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`);

const getItemHeight = useCallback(
Expand Down Expand Up @@ -97,7 +96,9 @@ function Search({query, policyIDs, sortBy, sortOrder, isMobileSelectionModeActiv
}

setCurrentSearchHash(hash);
SearchActions.search({hash, query, policyIDs, offset: 0, sortBy, sortOrder});

// TODO_SEARCH: Function below will be deprecated soon. No point in refactoring to use status instead of query.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
SearchActions.search({hash, query: status, policyIDs, offset: 0, sortBy, sortOrder});
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [hash, isOffline]);

Expand All @@ -109,7 +110,7 @@ function Search({query, policyIDs, sortBy, sortOrder, isMobileSelectionModeActiv
return (
<>
<SearchPageHeader
query={query}
status={status}
hash={hash}
/>
<SearchRowSkeleton shouldAnimate />
Expand All @@ -123,7 +124,7 @@ function Search({query, policyIDs, sortBy, sortOrder, isMobileSelectionModeActiv
return (
<>
<SearchPageHeader
query={query}
status={status}
hash={hash}
/>
<EmptySearchView />
Expand All @@ -144,15 +145,27 @@ function Search({query, policyIDs, sortBy, sortOrder, isMobileSelectionModeActiv
SearchActions.createTransactionThread(hash, item.transactionID, reportID, item.moneyRequestReportActionID);
}

Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute(query, reportID));
Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute(reportID));
};

const fetchMoreResults = () => {
if (!searchResults?.search?.hasMoreResults || shouldShowLoadingState || shouldShowLoadingMoreItems) {
return;
}
const currentOffset = searchResults?.search?.offset ?? 0;
SearchActions.search({hash, query, offset: currentOffset + CONST.SEARCH.RESULTS_PAGE_SIZE, sortBy, sortOrder});

const currentSearchParams = SearchUtils.getCurrentSearchParams();
const currentQueryString = SearchUtils.getQueryStringFromParams(currentSearchParams);
const isCustomQuery = SearchUtils.isCustomQueryFromParams(currentSearchParams);
const currentQueryJSON = SearchUtils.buildSearchQueryJSON(currentQueryString);
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved

if (!currentQueryJSON) {
return;
}

// TODO_SEARCH: offset should be a number but it is a string.
adamgrzybowski marked this conversation as resolved.
Show resolved Hide resolved
const newQuery = SearchUtils.buildSearchQueryString({...currentQueryJSON, offset: Number(currentQueryJSON.offset) + CONST.SEARCH.RESULTS_PAGE_SIZE});

navigation.setParams(isCustomQuery ? {cq: newQuery} : {q: newQuery});
};

const type = SearchUtils.getSearchType(searchResults?.search);
Expand All @@ -168,21 +181,24 @@ function Search({query, policyIDs, sortBy, sortOrder, isMobileSelectionModeActiv
const sortedData = SearchUtils.getSortedSections(type, data, sortBy, sortOrder);

const onSortPress = (column: SearchColumnType, order: SortOrder) => {
navigation.setParams({
sortBy: column,
sortOrder: order,
});
const currentSearchParams = SearchUtils.getCurrentSearchParams();
const currentQueryString = SearchUtils.getQueryStringFromParams(currentSearchParams);
const isCustomQuery = SearchUtils.isCustomQueryFromParams(currentSearchParams);
const currentQueryJSON = SearchUtils.buildSearchQueryJSON(currentQueryString);

const newQuery = SearchUtils.buildSearchQueryString({...currentQueryJSON, sortBy: column, sortOrder: order});
navigation.setParams(isCustomQuery ? {cq: newQuery} : {q: newQuery});
};

const isSortingAllowed = sortableSearchTabs.includes(query);
const isSortingAllowed = sortableSearchTabs.includes(status);

const shouldShowYear = SearchUtils.shouldShowYear(searchResults?.data);

const canSelectMultiple = isSmallScreenWidth ? isMobileSelectionModeActive : true;

return (
<SearchListWithHeader
query={query}
status={status}
hash={hash}
data={sortedData}
searchType={searchResults?.search?.type as SearchDataTypes}
Expand Down
36 changes: 35 additions & 1 deletion src/components/Search/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type SelectedTransactions = Record<string, SelectedTransactionInfo>;

type SortOrder = ValueOf<typeof CONST.SEARCH.SORT_ORDER>;
type SearchColumnType = ValueOf<typeof CONST.SEARCH.TABLE_COLUMNS>;
type SearchStatus = ValueOf<typeof CONST.SEARCH.STATUS>;

type SearchContext = {
currentSearchHash: number;
Expand All @@ -49,4 +50,37 @@ type QueryFilters = {
[K in AllFieldKeys]: QueryFilter | QueryFilter[];
};

export type {SelectedTransactionInfo, SelectedTransactions, SearchColumnType, SortOrder, SearchContext, ASTNode, QueryFilter, QueryFilters, AllFieldKeys};
type QueryKind = ValueOf<typeof CONST.SEARCH.QUERY_KIND>;

type SearchQueryString = string;

type SearchQueryAST = {
type: string;
status: SearchStatus;
sortBy: SearchColumnType;
sortOrder: SortOrder;
offset: number;
filters: ASTNode;
};

type SearchQueryJSON = {
input: string;
hash: number;
} & SearchQueryAST;

export type {
QueryKind,
SelectedTransactionInfo,
SelectedTransactions,
SearchColumnType,
SearchStatus,
SearchQueryAST,
SearchQueryJSON,
SearchQueryString,
SortOrder,
SearchContext,
ASTNode,
QueryFilter,
QueryFilters,
AllFieldKeys,
};
Loading
Loading