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

store applied filters in onyx #48312

Merged
merged 19 commits into from
Sep 11, 2024
Merged

store applied filters in onyx #48312

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e08f197
add applied filters to onxy upen filters button press
289Adam289 Aug 30, 2024
c265761
Merge branch 'main' into 289Adam289/48156-store-applied-filters
289Adam289 Aug 30, 2024
cebebc8
change toString function
289Adam289 Sep 2, 2024
c55bb3e
Merge branch 'main' into 289Adam289/48156-store-applied-filters
289Adam289 Sep 2, 2024
88e8219
add applied filters on narrow view
289Adam289 Sep 2, 2024
d0028ce
improve function names
289Adam289 Sep 2, 2024
1b2eecf
review suggestions
289Adam289 Sep 2, 2024
2616394
Merge branch 'main' into 289Adam289/48156-store-applied-filters
289Adam289 Sep 2, 2024
3260c11
fix ts and ampersant error
289Adam289 Sep 3, 2024
975349e
review suggestions
289Adam289 Sep 3, 2024
c1d4ca5
Merge branch 'main' into 289Adam289/48156-store-applied-filters
289Adam289 Sep 4, 2024
507fc44
review
289Adam289 Sep 5, 2024
e5fb785
fix function signature
289Adam289 Sep 5, 2024
49119f9
fix string sanitization
289Adam289 Sep 6, 2024
81803b8
Merge branch 'main' into 289Adam289/48156-store-applied-filters
289Adam289 Sep 9, 2024
ad05cf4
remove is and has filters
289Adam289 Sep 10, 2024
b57bbcf
fix typecheck
289Adam289 Sep 10, 2024
e96f20d
fix filters and reset button
289Adam289 Sep 11, 2024
263c291
fix on narrow layout
289Adam289 Sep 11, 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
2 changes: 1 addition & 1 deletion src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const ROUTES = {

SEARCH_CENTRAL_PANE: {
route: 'search',
getRoute: ({query, isCustomQuery = false}: {query: SearchQueryString; isCustomQuery?: boolean}) => `search?q=${query}&isCustomQuery=${isCustomQuery}` as const,
getRoute: ({query, isCustomQuery = false}: {query: SearchQueryString; isCustomQuery?: boolean}) => `search?q=${encodeURIComponent(query)}&isCustomQuery=${isCustomQuery}` as const,
},
SEARCH_ADVANCED_FILTERS: 'search/filters',
SEARCH_ADVANCED_FILTERS_DATE: 'search/filters/date',
Expand Down
7 changes: 6 additions & 1 deletion src/components/Search/SearchPageHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,12 @@ function SearchPageHeader({queryJSON, hash, onSelectDeleteOption, setOfflineModa
<Button
text={translate('search.filtersHeader')}
icon={Expensicons.Filters}
onPress={() => Navigation.navigate(ROUTES.SEARCH_ADVANCED_FILTERS)}
onPress={() => {
const filters = SearchUtils.getFilters(queryJSON);
const values = SearchUtils.getFiltersFormValues(filters, queryJSON);
SearchActions.updateAdvancedFilters(values);
Navigation.navigate(ROUTES.SEARCH_ADVANCED_FILTERS);
}}
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
medium
/>
</HeaderWrapper>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Search/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type QueryFilter = {
value: string | number;
};

type AdvancedFiltersKeys = ValueOf<typeof CONST.SEARCH.SYNTAX_FILTER_KEYS> | typeof CONST.SEARCH.SYNTAX_ROOT_KEYS.TYPE | typeof CONST.SEARCH.SYNTAX_ROOT_KEYS.STATUS;
type AdvancedFiltersKeys = ValueOf<typeof CONST.SEARCH.SYNTAX_FILTER_KEYS>;
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved

type QueryFilters = {
[K in AdvancedFiltersKeys]?: QueryFilter[];
Expand Down
59 changes: 50 additions & 9 deletions src/libs/SearchUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import * as searchParser from './SearchParser/searchParser';
import * as TransactionUtils from './TransactionUtils';
import * as UserUtils from './UserUtils';

type KeysOfFilterKeysObject = keyof typeof CONST.SEARCH.SYNTAX_FILTER_KEYS;
type FilterKeys = keyof typeof CONST.SEARCH.SYNTAX_FILTER_KEYS;

const columnNamesToSortingProperty = {
[CONST.SEARCH.TABLE_COLUMNS.TO]: 'formattedTo' as const,
Expand Down Expand Up @@ -443,18 +443,20 @@ function getChatFiltersTranslationKey(has: ValueOf<typeof CONST.SEARCH.CHAT_TYPE
function buildQueryStringFromFilters(filterValues: Partial<SearchAdvancedFiltersForm>) {
const filtersString = Object.entries(filterValues).map(([filterKey, filterValue]) => {
if ((filterKey === FILTER_KEYS.MERCHANT || filterKey === FILTER_KEYS.DESCRIPTION || filterKey === FILTER_KEYS.REPORT_ID) && filterValue) {
const keyInCorrectForm = (Object.keys(CONST.SEARCH.SYNTAX_FILTER_KEYS) as KeysOfFilterKeysObject[]).find((key) => CONST.SEARCH.SYNTAX_FILTER_KEYS[key] === filterKey);
const keyInCorrectForm = (Object.keys(CONST.SEARCH.SYNTAX_FILTER_KEYS) as FilterKeys[]).find((key) => CONST.SEARCH.SYNTAX_FILTER_KEYS[key] === filterKey);
if (keyInCorrectForm) {
return `${CONST.SEARCH.SYNTAX_FILTER_KEYS[keyInCorrectForm]}:${sanitizeString(filterValue as string)}`;
}
}
if (filterKey === FILTER_KEYS.KEYWORD && filterValue) {
const keyInCorrectForm = (Object.keys(CONST.SEARCH.SYNTAX_FILTER_KEYS) as KeysOfFilterKeysObject[]).find((key) => CONST.SEARCH.SYNTAX_FILTER_KEYS[key] === filterKey);
if (keyInCorrectForm) {
return `${CONST.SEARCH.SYNTAX_FILTER_KEYS[keyInCorrectForm]}:${filterValue as string}`;
}
return `${filterValue as string}`;
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 to sanitize the string for the keyword filter?

Copy link
Contributor Author

@289Adam289 289Adam289 Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

At first I didn't think it was necessary but after some thought if we add operator symbols to sanitizeString function and sanitize every word of an input it should prevent some bugs.

Copy link
Contributor Author

@289Adam289 289Adam289 Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

While working on keyword filter sanitization i came across another problem. Quotation marks are hard to parse. For example category with a single quote mark breaks the parser and even sanitizing it wont help. I suggest we discuss this problem in another issue cause it may need some fundamental changes in grammar itself. cc @luacmartins @Kicu

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that it'd require changes to the grammar and we can handle that as a separate issue.

}
if (filterKey === FILTER_KEYS.TYPE && filterValue) {
return `${CONST.SEARCH.SYNTAX_ROOT_KEYS.TYPE}:${filterValue as string}`;
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
}
if (filterKey === FILTER_KEYS.STATUS && filterValue) {
return `${CONST.SEARCH.SYNTAX_ROOT_KEYS.STATUS}:${filterValue as string}`;
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
}

if (
(filterKey === FILTER_KEYS.CATEGORY ||
filterKey === FILTER_KEYS.CARD_ID ||
Expand All @@ -469,8 +471,8 @@ function buildQueryStringFromFilters(filterValues: Partial<SearchAdvancedFilters
Array.isArray(filterValue) &&
filterValue.length > 0
) {
const filterValueArray = filterValues[filterKey] ?? [];
const keyInCorrectForm = (Object.keys(CONST.SEARCH.SYNTAX_FILTER_KEYS) as KeysOfFilterKeysObject[]).find((key) => CONST.SEARCH.SYNTAX_FILTER_KEYS[key] === filterKey);
const filterValueArray = Array.from(new Set<string>(filterValues[filterKey] ?? []));
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
const keyInCorrectForm = (Object.keys(CONST.SEARCH.SYNTAX_FILTER_KEYS) as FilterKeys[]).find((key) => CONST.SEARCH.SYNTAX_FILTER_KEYS[key] === filterKey);
if (keyInCorrectForm) {
return `${CONST.SEARCH.SYNTAX_FILTER_KEYS[keyInCorrectForm]}:${filterValueArray.map(sanitizeString).join(',')}`;
}
Expand Down Expand Up @@ -529,6 +531,44 @@ function getFilters(queryJSON: SearchQueryJSON) {
return filters;
}

289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
function getFiltersFormValues(filters: QueryFilters, queryJSON: SearchQueryJSON) {
const filterKeys = Object.keys(filters);
const filtersForm = {} as Partial<SearchAdvancedFiltersForm>;
for (const filterKey of filterKeys) {
if (filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.REPORT_ID || filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.MERCHANT || filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.DESCRIPTION) {
filtersForm[filterKey] = filters[filterKey]?.[0]?.value.toString();
}
if (
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.CATEGORY ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.CARD_ID ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.TAX_RATE ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.EXPENSE_TYPE ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.TAG ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.CURRENCY ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM ||
filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.TO
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add In and Has filters keys

Copy link
Contributor

Choose a reason for hiding this comment

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

We can also consider defining a MULTIPLE_VALUES_FILTER_KEYS const and then check for Object.values(MULTIPLE_VALUES_FILTER_KEYS).includes(filterKey) instead. This will simplify the condition if we need it in multiple places

filtersForm[filterKey] = filters[filterKey]?.map((filter) => filter.value.toString());
}
if (filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.KEYWORD) {
filtersForm[filterKey] = filters[filterKey]?.map((filter) => filter.value.toString()).join(' ');
}
if (filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.DATE) {
filtersForm[FILTER_KEYS.DATE_BEFORE] = filters[filterKey]?.find((filter) => filter.operator === 'lt')?.value.toString();
filtersForm[FILTER_KEYS.DATE_AFTER] = filters[filterKey]?.find((filter) => filter.operator === 'gt')?.value.toString();
}
if (filterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.AMOUNT) {
filtersForm[FILTER_KEYS.LESS_THAN] = filters[filterKey]?.find((filter) => filter.operator === 'lt')?.value.toString();
filtersForm[FILTER_KEYS.GREATER_THAN] = filters[filterKey]?.find((filter) => filter.operator === 'gt')?.value.toString();
}
}

filtersForm[FILTER_KEYS.TYPE] = queryJSON.type;
filtersForm[FILTER_KEYS.STATUS] = queryJSON.status;

return filtersForm;
}

/**
* Given a SearchQueryJSON this function will try to find the value of policyID filter saved in query
* and return just the first policyID value from the filter.
Expand Down Expand Up @@ -587,6 +627,7 @@ export {
buildSearchQueryString,
getCurrentSearchParams,
getFilters,
getFiltersFormValues,
getPolicyIDFromSearchQuery,
getListItem,
getSearchHeaderTitle,
Expand Down
5 changes: 2 additions & 3 deletions src/pages/Search/AdvancedSearchFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ function getFilterDisplayTitle(filters: Partial<SearchAdvancedFiltersForm>, fiel
if (greaterThan) {
return translate('search.filters.amount.greaterThan', convertToDisplayStringWithoutCurrency(Number(greaterThan)));
}
// Will never happen
return;
}

if (
Expand All @@ -184,9 +186,6 @@ function getFilterDisplayTitle(filters: Partial<SearchAdvancedFiltersForm>, fiel
return filters[fieldName];
}

// Todo Once all Advanced filters are implemented this line can be cleaned up. See: https://github.com/Expensify/App/issues/45026
// @ts-expect-error this property access is temporarily an error, because not every SYNTAX_FILTER_KEYS is handled by form.
// When all filters are updated here: src/types/form/SearchAdvancedFiltersForm.ts this line comment + type cast can be removed.
const filterValue = filters[fieldName] as string;
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
return filterValue ? Str.recapitalize(filterValue) : undefined;
}
Expand Down
1 change: 1 addition & 0 deletions src/pages/Search/SearchTypeMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ function SearchTypeMenu({queryJSON, isCustomQuery}: SearchTypeMenuProps) {
<SearchTypeMenuNarrow
typeMenuItems={typeMenuItems}
activeItemIndex={activeItemIndex}
queryJSON={queryJSON}
title={title}
/>
);
Expand Down
13 changes: 11 additions & 2 deletions src/pages/Search/SearchTypeMenuNarrow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,27 @@ import Button from '@components/Button';
import Icon from '@components/Icon';
import PopoverMenu from '@components/PopoverMenu';
import PressableWithFeedback from '@components/Pressable/PressableWithFeedback';
import type {SearchQueryJSON} from '@components/Search/types';
import Text from '@components/Text';
import useSingleExecution from '@hooks/useSingleExecution';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import * as SearchActions from '@libs/actions/Search';
import Navigation from '@libs/Navigation/Navigation';
import * as Expensicons from '@src/components/Icon/Expensicons';
import * as SearchUtils from '@src/libs/SearchUtils';
import ROUTES from '@src/ROUTES';
import type {SearchTypeMenuItem} from './SearchTypeMenu';

type SearchTypeMenuNarrowProps = {
typeMenuItems: SearchTypeMenuItem[];
activeItemIndex: number;
queryJSON: SearchQueryJSON;
title?: string;
};

function SearchTypeMenuNarrow({typeMenuItems, activeItemIndex, title}: SearchTypeMenuNarrowProps) {
function SearchTypeMenuNarrow({typeMenuItems, activeItemIndex, queryJSON, title}: SearchTypeMenuNarrowProps) {
const theme = useTheme();
const styles = useThemeStyles();
const {singleExecution} = useSingleExecution();
Expand Down Expand Up @@ -96,7 +100,12 @@ function SearchTypeMenuNarrow({typeMenuItems, activeItemIndex, title}: SearchTyp
</PressableWithFeedback>
<Button
icon={Expensicons.Filters}
onPress={() => Navigation.navigate(ROUTES.SEARCH_ADVANCED_FILTERS)}
onPress={() => {
289Adam289 marked this conversation as resolved.
Show resolved Hide resolved
const filters = SearchUtils.getFilters(queryJSON);
const values = SearchUtils.getFiltersFormValues(filters, queryJSON);
SearchActions.updateAdvancedFilters(values);
Navigation.navigate(ROUTES.SEARCH_ADVANCED_FILTERS);
}}
/>
<PopoverMenu
menuItems={popoverMenuItems}
Expand Down
2 changes: 2 additions & 0 deletions src/types/form/SearchAdvancedFiltersForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type Form from './Form';

const FILTER_KEYS = {
TYPE: 'type',
STATUS: 'status',
DATE_AFTER: 'dateAfter',
DATE_BEFORE: 'dateBefore',
CURRENCY: 'currency',
Expand Down Expand Up @@ -30,6 +31,7 @@ type SearchAdvancedFiltersForm = Form<
InputID,
{
[FILTER_KEYS.TYPE]: string;
[FILTER_KEYS.STATUS]: string;
[FILTER_KEYS.DATE_AFTER]: string;
[FILTER_KEYS.DATE_BEFORE]: string;
[FILTER_KEYS.CURRENCY]: string[];
Expand Down
Loading