diff --git a/src/components/Search/SearchRouter/SearchRouter.tsx b/src/components/Search/SearchRouter/SearchRouter.tsx index 3a4ebdd856cd..cc30e67cc29f 100644 --- a/src/components/Search/SearchRouter/SearchRouter.tsx +++ b/src/components/Search/SearchRouter/SearchRouter.tsx @@ -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'; @@ -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, {}]; } diff --git a/src/libs/SearchParser/searchParser.js b/src/libs/SearchParser/searchParser.js index ac786bf9fe6f..13d82261892e 100644 --- a/src/libs/SearchParser/searchParser.js +++ b/src/libs/SearchParser/searchParser.js @@ -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}; }; diff --git a/src/libs/SearchParser/searchParser.peggy b/src/libs/SearchParser/searchParser.peggy index 75238c3cbcfc..7813844c9eac 100644 --- a/src/libs/SearchParser/searchParser.peggy +++ b/src/libs/SearchParser/searchParser.peggy @@ -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}; } diff --git a/tests/unit/Search/SearchQueryUtilsTest.ts b/tests/unit/Search/SearchQueryUtilsTest.ts index 9e80544b68e4..40e50e4e8608 100644 --- a/tests/unit/Search/SearchQueryUtilsTest.ts +++ b/tests/unit/Search/SearchQueryUtilsTest.ts @@ -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', () => { @@ -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', () => {