From e410acf7073bc237d554863a71ce093034b39276 Mon Sep 17 00:00:00 2001 From: Shubham Agrawal Date: Wed, 23 Jul 2025 23:17:02 +0530 Subject: [PATCH 1/2] Refactored localeCompare in SearchQueryUtils --- .../Search/SearchMultipleSelectionPicker.tsx | 8 +++--- src/libs/SearchQueryUtils.ts | 14 +++++++--- src/pages/Search/AdvancedSearchFilters.tsx | 4 +-- tests/unit/Search/SearchQueryUtilsTest.ts | 26 ++++++++++++++++++- tests/utils/TestHelper.ts | 11 ++++++++ 5 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/components/Search/SearchMultipleSelectionPicker.tsx b/src/components/Search/SearchMultipleSelectionPicker.tsx index b26c2a62fcef..8a2431a30d32 100644 --- a/src/components/Search/SearchMultipleSelectionPicker.tsx +++ b/src/components/Search/SearchMultipleSelectionPicker.tsx @@ -23,7 +23,7 @@ type SearchMultipleSelectionPickerProps = { }; function SearchMultipleSelectionPicker({items, initiallySelectedItems, pickerTitle, onSaveSelection, shouldShowTextInput = true}: SearchMultipleSelectionPickerProps) { - const {translate} = useLocalize(); + const {translate, localeCompare} = useLocalize(); const [searchTerm, debouncedSearchTerm, setSearchTerm] = useDebouncedState(''); const [selectedItems, setSelectedItems] = useState(initiallySelectedItems ?? []); @@ -35,7 +35,7 @@ function SearchMultipleSelectionPicker({items, initiallySelectedItems, pickerTit const {sections, noResultsFound} = useMemo(() => { const selectedItemsSection = selectedItems .filter((item) => item?.name.toLowerCase().includes(debouncedSearchTerm?.toLowerCase())) - .sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString())) + .sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString(), localeCompare)) .map((item) => ({ text: item.name, keyForList: item.name, @@ -47,7 +47,7 @@ function SearchMultipleSelectionPicker({items, initiallySelectedItems, pickerTit (item) => !selectedItems.some((selectedItem) => selectedItem.value.toString() === item.value.toString()) && item?.name?.toLowerCase().includes(debouncedSearchTerm?.toLowerCase()), ) - .sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString())) + .sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString(), localeCompare)) .map((item) => ({ text: item.name, keyForList: item.name, @@ -72,7 +72,7 @@ function SearchMultipleSelectionPicker({items, initiallySelectedItems, pickerTit ], noResultsFound: isEmpty, }; - }, [selectedItems, items, pickerTitle, debouncedSearchTerm]); + }, [selectedItems, items, pickerTitle, debouncedSearchTerm, localeCompare]); const onSelectItem = useCallback( (item: Partial) => { diff --git a/src/libs/SearchQueryUtils.ts b/src/libs/SearchQueryUtils.ts index ee2aa13f3774..5394931d819c 100644 --- a/src/libs/SearchQueryUtils.ts +++ b/src/libs/SearchQueryUtils.ts @@ -1,6 +1,7 @@ import cloneDeep from 'lodash/cloneDeep'; import type {OnyxCollection} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; +import type {LocaleContextProps} from '@components/LocaleContextProvider'; import type { ASTNode, QueryFilter, @@ -26,7 +27,6 @@ import type {SearchDataTypes} from '@src/types/onyx/SearchResults'; import {getCardFeedsForDisplay} from './CardFeedUtils'; import {getCardDescription} from './CardUtils'; import {convertToBackendAmount, convertToFrontendAmountAsInteger} from './CurrencyUtils'; -import localeCompare from './LocaleCompare'; import Log from './Log'; import {validateAmount} from './MoneyRequestUtils'; import navigationRef from './Navigation/navigationRef'; @@ -273,6 +273,14 @@ function getUpdatedFilterValue(filterName: ValueOf { const filters = cloneDeep(filter.filters); - filters.sort((a, b) => localeCompare(a.value.toString(), b.value.toString())); + filters.sort((a, b) => customCollator.compare(a.value.toString(), b.value.toString())); return buildFilterValuesString(filter.key, filters); }) .sort() @@ -883,7 +891,7 @@ function isDefaultExpensesQuery(queryJSON: SearchQueryJSON) { /** * Always show `No category` and `No tag` as the first option */ -const sortOptionsWithEmptyValue = (a: string, b: string) => { +const sortOptionsWithEmptyValue = (a: string, b: string, localeCompare: LocaleContextProps['localeCompare']) => { if (a === CONST.SEARCH.CATEGORY_EMPTY_VALUE || a === CONST.SEARCH.TAG_EMPTY_VALUE) { return -1; } diff --git a/src/pages/Search/AdvancedSearchFilters.tsx b/src/pages/Search/AdvancedSearchFilters.tsx index 76a1991be19b..c734ac3183f1 100644 --- a/src/pages/Search/AdvancedSearchFilters.tsx +++ b/src/pages/Search/AdvancedSearchFilters.tsx @@ -436,7 +436,7 @@ function getFilterDisplayTitle( if (nonDateFilterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.CATEGORY && filters[nonDateFilterKey]) { const filterArray = filters[nonDateFilterKey] ?? []; return filterArray - .sort(sortOptionsWithEmptyValue) + .sort((a, b) => sortOptionsWithEmptyValue(a, b, localeCompare)) .map((value) => (value === CONST.SEARCH.CATEGORY_EMPTY_VALUE ? translate('search.noCategory') : value)) .join(', '); } @@ -444,7 +444,7 @@ function getFilterDisplayTitle( if (nonDateFilterKey === CONST.SEARCH.SYNTAX_FILTER_KEYS.TAG && filters[nonDateFilterKey]) { const filterArray = filters[nonDateFilterKey] ?? []; return filterArray - .sort(sortOptionsWithEmptyValue) + .sort((a, b) => sortOptionsWithEmptyValue(a, b, localeCompare)) .map((value) => (value === CONST.SEARCH.TAG_EMPTY_VALUE ? translate('search.noTag') : getCleanedTagName(value))) .join(', '); } diff --git a/tests/unit/Search/SearchQueryUtilsTest.ts b/tests/unit/Search/SearchQueryUtilsTest.ts index 08fa0b24a7d2..2ec6343be4b8 100644 --- a/tests/unit/Search/SearchQueryUtilsTest.ts +++ b/tests/unit/Search/SearchQueryUtilsTest.ts @@ -2,9 +2,17 @@ // we need "dirty" object key names in these tests import {generatePolicyID} from '@libs/actions/Policy/Policy'; import CONST from '@src/CONST'; -import {buildFilterFormValuesFromQuery, buildQueryStringFromFilterFormValues, buildSearchQueryJSON, getQueryWithUpdatedValues, shouldHighlight} from '@src/libs/SearchQueryUtils'; +import { + buildFilterFormValuesFromQuery, + buildQueryStringFromFilterFormValues, + buildSearchQueryJSON, + getQueryWithUpdatedValues, + shouldHighlight, + sortOptionsWithEmptyValue, +} from '@src/libs/SearchQueryUtils'; import ONYXKEYS from '@src/ONYXKEYS'; import type {SearchAdvancedFiltersForm} from '@src/types/form'; +import {localeCompare} from '../../utils/TestHelper'; const personalDetailsFakeData = { 'johndoe@example.com': { @@ -268,4 +276,20 @@ describe('SearchQueryUtils', () => { expect(shouldHighlight('Take a 2-minute tour', 'tour 2-minute')).toBe(false); }); }); + + describe('sortOptionsWithEmptyValue', () => { + it('should prioritize empty values at the start', () => { + const options = ['B', 'A', CONST.SEARCH.CATEGORY_EMPTY_VALUE, 'C']; + const sortedOptions = options.sort((a, b) => sortOptionsWithEmptyValue(a, b, localeCompare)); + + expect(sortedOptions).toEqual([CONST.SEARCH.CATEGORY_EMPTY_VALUE, 'A', 'B', 'C']); + }); + + it('should not change order of non-empty values', () => { + const options = ['B', 'A', 'C']; + const sortedOptions = options.sort((a, b) => sortOptionsWithEmptyValue(a, b, localeCompare)); + + expect(sortedOptions).toEqual(['A', 'B', 'C']); + }); + }); }); diff --git a/tests/utils/TestHelper.ts b/tests/utils/TestHelper.ts index 549b3ea97851..9e62dbe495e1 100644 --- a/tests/utils/TestHelper.ts +++ b/tests/utils/TestHelper.ts @@ -346,6 +346,16 @@ async function navigateToSidebarOption(index: number): Promise { await waitForBatchedUpdatesWithAct(); } +/** + * @private + * This is a custom collator only for testing purposes. + */ +const customCollator = new Intl.Collator('en', {usage: 'sort', sensitivity: 'variant', numeric: true, caseFirst: 'upper'}); + +function localeCompare(a: string, b: string): number { + return customCollator.compare(a, b); +} + export type {MockFetch, FormData}; export { assertFormDataMatchesObject, @@ -363,4 +373,5 @@ export { navigateToSidebarOption, getOnyxData, getNavigateToChatHintRegex, + localeCompare, }; From be83bc73725045ba08ff23c0cf6974551895c65f Mon Sep 17 00:00:00 2001 From: Shubham Agrawal <58412969+shubham1206agra@users.noreply.github.com> Date: Wed, 30 Jul 2025 20:46:10 +0530 Subject: [PATCH 2/2] Update tests/unit/Search/SearchQueryUtilsTest.ts --- tests/unit/Search/SearchQueryUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/Search/SearchQueryUtilsTest.ts b/tests/unit/Search/SearchQueryUtilsTest.ts index 2ec6343be4b8..f74e3fbc6918 100644 --- a/tests/unit/Search/SearchQueryUtilsTest.ts +++ b/tests/unit/Search/SearchQueryUtilsTest.ts @@ -285,7 +285,7 @@ describe('SearchQueryUtils', () => { expect(sortedOptions).toEqual([CONST.SEARCH.CATEGORY_EMPTY_VALUE, 'A', 'B', 'C']); }); - it('should not change order of non-empty values', () => { + it('should sort non-empty values properly', () => { const options = ['B', 'A', 'C']; const sortedOptions = options.sort((a, b) => sortOptionsWithEmptyValue(a, b, localeCompare));