Skip to content

Commit f01edb1

Browse files
authored
Merge pull request Expensify#87057 from TaduJR/feat-Insights-Release-1-Top-Categories-Add-a-limit-filter-to-search
fix: Reports - Limit filter is reapplied after resetting filters
2 parents e245656 + 4e69b8d commit f01edb1

7 files changed

Lines changed: 24 additions & 18 deletions

File tree

src/components/Search/FilterDropdowns/DisplayPopup.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ function DisplayPopup({queryJSON, searchResults, closeOverlay, onSort}: DisplayP
125125
buildFilterQueryWithSortDefaults(
126126
updatedFilterFormValues,
127127
{view: searchAdvancedFilters.view, groupBy: searchAdvancedFilters.groupBy},
128-
{sortBy: queryJSON.sortBy, sortOrder: queryJSON.sortOrder, limit: queryJSON.limit},
128+
{sortBy: queryJSON.sortBy, sortOrder: queryJSON.sortOrder},
129129
) ?? '';
130130
if (!queryString) {
131131
return;

src/components/Search/SearchPageHeader/useSearchActionsBar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ function useSearchActionsBar(queryJSON: SearchQueryJSON, isMobileSelectionModeEn
242242
buildFilterQueryWithSortDefaults(
243243
updatedFilterFormValues,
244244
{view: searchAdvancedFiltersForm.view, groupBy: searchAdvancedFiltersForm.groupBy},
245-
{sortBy: queryJSON.sortBy, sortOrder: queryJSON.sortOrder, limit: queryJSON.limit},
245+
{sortBy: queryJSON.sortBy, sortOrder: queryJSON.sortOrder},
246246
) ?? '';
247247
if (!queryString) {
248248
return;

src/components/Search/SearchPageHeader/useSearchFiltersBar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ function useSearchFiltersBar(queryJSON: SearchQueryJSON, isMobileSelectionModeEn
287287
buildFilterQueryWithSortDefaults(
288288
updatedFilterFormValues,
289289
{view: searchAdvancedFiltersForm.view, groupBy: searchAdvancedFiltersForm.groupBy},
290-
{sortBy: queryJSON.sortBy, sortOrder: queryJSON.sortOrder, limit: queryJSON.limit},
290+
{sortBy: queryJSON.sortBy, sortOrder: queryJSON.sortOrder},
291291
) ?? '';
292292
if (!queryString) {
293293
return;

src/libs/SearchQueryUtils.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,6 @@ function getSanitizedRawFilters(queryJSON: SearchQueryJSON): RawQueryFilter[] |
739739
type BuildQueryStringOptions = {
740740
sortBy?: string;
741741
sortOrder?: string;
742-
limit?: number;
743742
};
744743

745744
/**
@@ -960,9 +959,8 @@ function buildQueryStringFromFilterFormValues(filterValues: Partial<SearchAdvanc
960959
filtersString.push(amountFilter);
961960
}
962961

963-
const limitValue = limit ?? options?.limit;
964-
if (limitValue) {
965-
const num = Number(limitValue);
962+
if (limit) {
963+
const num = Number(limit);
966964
if (Number.isInteger(num) && num > 0) {
967965
filtersString.push(`${CONST.SEARCH.SYNTAX_ROOT_KEYS.LIMIT}:${num}`);
968966
}
@@ -1891,7 +1889,7 @@ function shouldResetSortForViewChange({newView, oldView, groupBy}: {newView: str
18911889
function buildFilterQueryWithSortDefaults(
18921890
filterValues: Partial<SearchAdvancedFiltersForm>,
18931891
previousState: {view?: string; groupBy?: string},
1894-
currentQueryOptions: {sortBy?: string; sortOrder?: string; limit?: number},
1892+
currentQueryOptions: {sortBy?: string; sortOrder?: string},
18951893
): string | undefined {
18961894
const resetSort = shouldResetSort({
18971895
newGroupBy: filterValues.groupBy,
@@ -1909,7 +1907,6 @@ function buildFilterQueryWithSortDefaults(
19091907
const queryString = buildQueryStringFromFilterFormValues(filterValues, {
19101908
sortBy: resetSort || resetSortForViewChange ? undefined : currentQueryOptions.sortBy,
19111909
sortOrder: resetSort || resetSortForViewChange ? undefined : currentQueryOptions.sortOrder,
1912-
limit: currentQueryOptions.limit,
19131910
});
19141911

19151912
if (!resetSort && !resetSortForViewChange) {

src/pages/Search/AdvancedSearchFilters.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ function AdvancedSearchFilters() {
634634
buildFilterQueryWithSortDefaults(
635635
searchAdvancedFilters,
636636
{view: currentQueryJSON?.view, groupBy: currentQueryJSON?.groupBy},
637-
{sortBy: currentQueryJSON?.sortBy, sortOrder: currentQueryJSON?.sortOrder, limit: currentQueryJSON?.limit},
637+
{sortBy: currentQueryJSON?.sortBy, sortOrder: currentQueryJSON?.sortOrder},
638638
) ?? ''
639639
);
640640
}, [searchAdvancedFilters]);

src/pages/Search/SearchColumnsPage.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ function SearchColumnsPage() {
5050
const queryString = buildQueryStringFromFilterFormValues(updatedAdvancedFilters, {
5151
sortBy: currentQueryJSON?.sortBy,
5252
sortOrder: currentQueryJSON?.sortOrder,
53-
limit: currentQueryJSON?.limit,
5453
});
5554

5655
Navigation.navigate(ROUTES.SEARCH_ROOT.getRoute({query: queryString}), {forceReplace: true});

tests/unit/Search/SearchQueryUtilsTest.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -425,12 +425,13 @@ describe('SearchQueryUtils', () => {
425425
});
426426

427427
describe('limit option', () => {
428-
test('includes limit in query string when provided', () => {
428+
test('includes limit in query string when provided in form values', () => {
429429
const filterValues: Partial<SearchAdvancedFiltersForm> = {
430430
type: 'expense',
431+
limit: '10',
431432
};
432433

433-
const result = buildQueryStringFromFilterFormValues(filterValues, {limit: 10});
434+
const result = buildQueryStringFromFilterFormValues(filterValues);
434435

435436
expect(result).toEqual('type:expense limit:10');
436437
});
@@ -439,9 +440,10 @@ describe('SearchQueryUtils', () => {
439440
const filterValues: Partial<SearchAdvancedFiltersForm> = {
440441
type: 'expense',
441442
merchant: 'Amazon',
443+
limit: '25',
442444
};
443445

444-
const result = buildQueryStringFromFilterFormValues(filterValues, {sortBy: 'amount', sortOrder: 'asc', limit: 25});
446+
const result = buildQueryStringFromFilterFormValues(filterValues, {sortBy: 'amount', sortOrder: 'asc'});
445447

446448
expect(result).toEqual('sortBy:amount sortOrder:asc type:expense merchant:Amazon limit:25');
447449
});
@@ -462,7 +464,7 @@ describe('SearchQueryUtils', () => {
462464
limit: '',
463465
};
464466

465-
const result = buildQueryStringFromFilterFormValues(filterValues, {limit: 10});
467+
const result = buildQueryStringFromFilterFormValues(filterValues);
466468

467469
expect(result).not.toContain('limit:');
468470
});
@@ -530,15 +532,23 @@ describe('SearchQueryUtils', () => {
530532
expect(keywordFilter?.filters.at(0)?.value).toBe('hello');
531533
});
532534

533-
test('form limit takes priority over options limit', () => {
535+
test('limit only comes from form values, not options', () => {
534536
const filterValues: Partial<SearchAdvancedFiltersForm> = {
535537
type: 'expense',
536538
limit: '30',
537539
};
538540

539-
const result = buildQueryStringFromFilterFormValues(filterValues, {limit: 10});
541+
const result = buildQueryStringFromFilterFormValues(filterValues);
540542
expect(result).toContain('limit:30');
541-
expect(result).not.toContain('limit:10');
543+
});
544+
545+
test('omits limit when form has no limit key (reset filters scenario)', () => {
546+
const filterValues: Partial<SearchAdvancedFiltersForm> = {
547+
type: 'expense',
548+
};
549+
550+
const result = buildQueryStringFromFilterFormValues(filterValues, {sortBy: 'date', sortOrder: 'desc'});
551+
expect(result).not.toContain('limit');
542552
});
543553
});
544554

0 commit comments

Comments
 (0)