From a44f6fe131fe527b661350a12b4d0a5e977ea77f Mon Sep 17 00:00:00 2001 From: "Eugene Voloshchak (via MelvinBot)" Date: Wed, 14 Oct 2026 19:22:42 +0000 Subject: [PATCH] Use a unique id as the React key for search filter chips so a filter and its negation don't collide Co-authored-by: Eugene Voloshchak --- .../SearchPageHeader/SearchFiltersBarNarrow.tsx | 2 +- .../SearchPageHeader/SearchFiltersBarWide.tsx | 2 +- src/libs/SearchUIUtils.ts | 14 +++++++++++--- src/pages/Search/SearchSavePage.tsx | 2 +- tests/unit/Search/SearchUIUtilsTest.ts | 16 ++++++++++++++++ tests/unit/pages/Search/SearchSavePageTest.tsx | 4 ++-- 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/components/Search/SearchPageHeader/SearchFiltersBarNarrow.tsx b/src/components/Search/SearchPageHeader/SearchFiltersBarNarrow.tsx index d96fa0081beb..1d06e72902a8 100644 --- a/src/components/Search/SearchPageHeader/SearchFiltersBarNarrow.tsx +++ b/src/components/Search/SearchPageHeader/SearchFiltersBarNarrow.tsx @@ -51,7 +51,7 @@ function SearchFiltersBarNarrow({queryJSON}: SearchFiltersBarNarrowProps) { ref={scrollRef} showsHorizontalScrollIndicator={false} data={data} - keyExtractor={(item) => item.key} + keyExtractor={(item) => item.id} renderItem={renderFilterItem} onEndReached={adjustScroll} onEndReachedThreshold={0.75} diff --git a/src/components/Search/SearchPageHeader/SearchFiltersBarWide.tsx b/src/components/Search/SearchPageHeader/SearchFiltersBarWide.tsx index e7006ded5896..58131b5eb304 100644 --- a/src/components/Search/SearchPageHeader/SearchFiltersBarWide.tsx +++ b/src/components/Search/SearchPageHeader/SearchFiltersBarWide.tsx @@ -30,7 +30,7 @@ function SearchFiltersBarWideContent({hasErrors, shouldShowFiltersBarLoading, fi return filters.map((item) => ( )); diff --git a/src/libs/SearchUIUtils.ts b/src/libs/SearchUIUtils.ts index 7e529a2bbb8c..a00e90234fc7 100644 --- a/src/libs/SearchUIUtils.ts +++ b/src/libs/SearchUIUtils.ts @@ -6164,6 +6164,8 @@ function getReportFieldTextKey(key: ReportFieldKey): ReportFieldTextKey { type SearchFilter = { key: keyof typeof FILTER_VIEW_MAP; + /** Unique per filter, unlike `key` which is shared by a filter and its negation (e.g. `status` and `statusNot`) */ + id: MappedFilterKey; label: string; value: string | string[]; }; @@ -6236,7 +6238,7 @@ function mapFiltersFormToLabelValueList( if (displayValue && label) { addedGroups.add(syntax); const isDefault = defaultSearchQueryFilterKeys.has(syntax); - (isDefault ? defaultFilters : nonDefaultFilters).push({key: syntax, label: translate(label), value: displayValue, ...mapper?.(syntax, isDefault)}); + (isDefault ? defaultFilters : nonDefaultFilters).push({key: syntax, id: syntax, label: translate(label), value: displayValue, ...mapper?.(syntax, isDefault)}); } continue; } @@ -6252,7 +6254,13 @@ function mapFiltersFormToLabelValueList( addedGroups.add(CONST.SEARCH.REPORT_FIELD.GLOBAL_PREFIX); const isDefault = defaultSearchQueryFilterKeys.has(getReportFieldTextKey(key)); const extra = mapper?.(CONST.SEARCH.SYNTAX_FILTER_KEYS.REPORT_FIELD, isDefault); - (isDefault ? defaultFilters : nonDefaultFilters).push({key: CONST.SEARCH.SYNTAX_FILTER_KEYS.REPORT_FIELD, label: translate('workspace.common.reportField'), value, ...extra}); + (isDefault ? defaultFilters : nonDefaultFilters).push({ + key: CONST.SEARCH.SYNTAX_FILTER_KEYS.REPORT_FIELD, + id: CONST.SEARCH.SYNTAX_FILTER_KEYS.REPORT_FIELD, + label: translate('workspace.common.reportField'), + value, + ...extra, + }); } continue; } @@ -6269,7 +6277,7 @@ function mapFiltersFormToLabelValueList( if (label && value && !(Array.isArray(value) && value.length === 0)) { const isDefault = defaultSearchQueryFilterKeys.has(baseKey); - (isDefault ? defaultFilters : nonDefaultFilters).push({key: baseKey, label, value, ...mapper?.(key, isDefault)}); + (isDefault ? defaultFilters : nonDefaultFilters).push({key: baseKey, id: key, label, value, ...mapper?.(key, isDefault)}); } } diff --git a/src/pages/Search/SearchSavePage.tsx b/src/pages/Search/SearchSavePage.tsx index d8befb33b33e..1da1fdb77773 100644 --- a/src/pages/Search/SearchSavePage.tsx +++ b/src/pages/Search/SearchSavePage.tsx @@ -224,7 +224,7 @@ function SearchSavePage() { appliedFilters.map((filter) => ( {CONST.DOT_SEPARATOR} diff --git a/tests/unit/Search/SearchUIUtilsTest.ts b/tests/unit/Search/SearchUIUtilsTest.ts index 45f1871d8342..e77ba0fb96d4 100644 --- a/tests/unit/Search/SearchUIUtilsTest.ts +++ b/tests/unit/Search/SearchUIUtilsTest.ts @@ -15076,6 +15076,22 @@ describe('SearchUIUtils', () => { expect(result.at(0)?.isDefault).toBe(true); expect(result.at(1)?.isDefault).toBe(false); }); + + it('gives a filter and its negation the same key but different ids', () => { + // Given a query that uses the status filter both normally and negated, e.g. `status:approved -status:drafts,paid` + const form = { + [FILTER_KEYS.TYPE]: CONST.SEARCH.DATA_TYPES.EXPENSE, + [FILTER_KEYS.STATUS]: [CONST.SEARCH.STATUS.EXPENSE.APPROVED], + [FILTER_KEYS.STATUS_NOT]: [CONST.SEARCH.STATUS.EXPENSE.DRAFTS, CONST.SEARCH.STATUS.EXPENSE.PAID], + }; + + // When the form is mapped to filter chips + const result = SearchUIUtils.mapFiltersFormToLabelValueList(form, new Set(), new Set(), translateLocal, undefined, localeCompare, convertToDisplayStringWithoutCurrency); + const statusFilters = result.filter((filter) => filter.key === CONST.SEARCH.SYNTAX_FILTER_KEYS.STATUS); + + // Then both chips share the base key (used to pick the chip's popover), while their ids stay unique so they can be used as React keys + expect(statusFilters.map((filter) => filter.id)).toEqual([FILTER_KEYS.STATUS, FILTER_KEYS.STATUS_NOT]); + }); }); }); diff --git a/tests/unit/pages/Search/SearchSavePageTest.tsx b/tests/unit/pages/Search/SearchSavePageTest.tsx index 258c06a1d59e..af6572fef8e4 100644 --- a/tests/unit/pages/Search/SearchSavePageTest.tsx +++ b/tests/unit/pages/Search/SearchSavePageTest.tsx @@ -75,8 +75,8 @@ it.each([[['12']], [['12', '23']], [['123']]])('renders canonical card selection }); it('renders scalar feed and tax values without calling array display hooks', () => { jest.spyOn(SearchUIUtils, 'mapFiltersFormToLabelValueList').mockReturnValueOnce([ - {key: CONST.SEARCH.SYNTAX_FILTER_KEYS.FEED, label: 'Feed', value: 'feed-scalar'}, - {key: CONST.SEARCH.SYNTAX_FILTER_KEYS.TAX_RATE, label: 'Tax', value: 'tax-scalar'}, + {key: CONST.SEARCH.SYNTAX_FILTER_KEYS.FEED, id: CONST.SEARCH.SYNTAX_FILTER_KEYS.FEED, label: 'Feed', value: 'feed-scalar'}, + {key: CONST.SEARCH.SYNTAX_FILTER_KEYS.TAX_RATE, id: CONST.SEARCH.SYNTAX_FILTER_KEYS.TAX_RATE, label: 'Tax', value: 'tax-scalar'}, ]); const output = JSON.stringify(render().toJSON()); expect(['feed-scalar', 'tax-scalar'].every((text) => output.includes(text))).toBe(true);