Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
8 changes: 5 additions & 3 deletions src/components/Search/SearchRouter/SearchRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {getReportAction} from '@libs/ReportActionsUtils';
import {isHiddenForCurrentUser, isOneOnOneChat} from '@libs/ReportUtils';
import type {OptionData} from '@libs/ReportUtils';
import {getAutocompleteQueryWithComma, getTrimmedUserSearchQueryPreservingComma} from '@libs/SearchAutocompleteUtils';
import {searchKeyToSavedSearchID} from '@libs/SearchKeyUtils';
import {buildSearchQueryJSON, buildUserReadableQueryString, getQueryWithUpdatedValues, sanitizeSearchValue} from '@libs/SearchQueryUtils';
import StringUtils from '@libs/StringUtils';

Expand Down Expand Up @@ -132,13 +133,14 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla
const reportAttributes = useReportAttributes();

// Seed the input on open. When the SearchRouter is opened from the search button on the search page
// and `shouldShowSearchQuery` is true, we build a user-readable query string from the current search
// query (showing names instead of IDs) along with the substitutions map needed to map those names
// and `shouldShowSearchQuery` is true or a saved search is active, we build a user-readable query
// from the current search (showing names instead of IDs) along with the substitutions map needed to map those names
// back to IDs when submitting. Otherwise we fall back to the explicit pending query (e.g. from
// ExpenseReportSearchHandler). Computed once via a lazy initializer so the query string and its
// substitutions stay consistent.
const [[initialQuery, initialSubstitutions]] = useState<[string, SubstitutionMap]>(() => {
if (!currentSearchQueryJSON || !isFromSearchPageSearchButton || !searchContext?.shouldShowSearchQuery) {
const isSavedSearch = !!searchKeyToSavedSearchID(currentSearchKey);
if (!currentSearchQueryJSON || !isFromSearchPageSearchButton || (!searchContext?.shouldShowSearchQuery && !isSavedSearch)) {
return [pendingInitialQuery, {}];
}

Expand Down
4 changes: 4 additions & 0 deletions src/libs/SearchParser/searchParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,10 @@ function peg$parse(input, options) {
if (op === "contains") {
return buildFilter("eq", "keyword", text().trim());
}
// Ignore unsupported groupings before they override a valid grouping or affect sort and view defaults.
if (key === "groupBy" && !Object.hasOwn(GROUP_BY_DEFAULT_SORT, value)) {
return null;
}
updateDefaultValues(key, value);
return {...buildFilter(op, key, value), isDefault: true};
};
Expand Down
4 changes: 4 additions & 0 deletions src/libs/SearchParser/searchParser.peggy
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ defaultFilter
if (op === "contains") {
return buildFilter("eq", "keyword", text().trim());
}
// Ignore unsupported groupings before they override a valid grouping or affect sort and view defaults.
if (key === "groupBy" && !Object.hasOwn(GROUP_BY_DEFAULT_SORT, value)) {
return null;
}
updateDefaultValues(key, value);
return {...buildFilter(op, key, value), isDefault: true};
}
Expand Down
22 changes: 18 additions & 4 deletions tests/unit/Search/SearchQueryUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,12 +516,26 @@ describe('SearchQueryUtils', () => {
expect(result).toEqual(`${defaultQuery} from:9876,87654 to:78901 amount:15000 hello test`);
});

test('returns query with updated groupBy', () => {
const userQuery = 'from:12345 groupBy:reports';
test('ignores unsupported groupBy and preserves other filters', () => {
// Given a saved expense query with a From filter and an unsupported grouping
const userQuery = 'type:expense from:12345 group-by: reports';

// When the router normalizes the submitted query
const result = getQueryWithUpdatedValues(userQuery);

expect(result).toEqual(`${defaultQuery} groupBy:reports from:12345`);
// Then the unsupported grouping is omitted and the From filter remains
expect(result).toEqual(`${defaultQuery} from:12345`);
});

test('keeps a supported groupBy when followed by an unsupported groupBy', () => {
// Given a query with a valid grouping followed by an unsupported grouping
const userQuery = 'type:expense from:12345 group-by:merchant group-by:reports';

// When the router normalizes the submitted query
const result = getQueryWithUpdatedValues(userQuery);

// Then the valid grouping and From filter remain applied
expect(result).toEqual('type:expense sortBy:groupMerchant sortOrder:asc groupBy:merchant from:12345');
});

test('returns query with updated view', () => {
Expand Down Expand Up @@ -2888,7 +2902,7 @@ describe('SearchQueryUtils', () => {
expect(isDefaultExpenses('type:expense footerTotal:billable footerCurrency:EUR')).toBe(true);
expect(isDefaultExpenses('type:expense footerTotal:billable category:Travel')).toBe(false);
expect(isDefaultExpenseReports('type:expense-report footerCount:expenses')).toBe(true);
expect(isDefaultExpenseReports('type:expense-report groupBy:report')).toBe(false);
expect(isDefaultExpenseReports('type:expense-report groupBy:merchant')).toBe(false);
});

it('keeps every footer selection out of the recent and similar search hashes, since none of them changes a result', () => {
Expand Down
Loading