Merge pull request Expensify#67068 from shubham1206agra/refactor-onyx-7

Refactored localeCompare in SuggestionUtils

Author: tgolen

src/libs/SuggestionUtils.ts

@@ -1,6 +1,6 @@
+import type {LocaleContextProps} from '@components/LocaleContextProvider';
 import CONST from '@src/CONST';
 import type {PersonalDetails} from '@src/types/onyx';
-import localeCompare from './LocaleCompare';
 import {getDisplayNameForParticipant} from './ReportUtils';
 
 /**
@@ -32,19 +32,21 @@ function getDisplayName(details: PersonalDetails) {
 }
 
 /**
- * Comparison function to sort users. It compares weights, display names, and accountIDs in that order
+ * Function to sort users. It compares weights, display names, and accountIDs in that order
  */
-function compareUserInList(first: PersonalDetails & {weight: number}, second: PersonalDetails & {weight: number}) {
-    if (first.weight !== second.weight) {
-        return first.weight - second.weight;
-    }
+function getSortedPersonalDetails(personalDetails: Array<PersonalDetails & {weight: number}>, localeCompare: LocaleContextProps['localeCompare']) {
+    return personalDetails.sort((first, second) => {
+        if (first.weight !== second.weight) {
+            return first.weight - second.weight;
+        }
 
-    const displayNameLoginOrder = localeCompare(getDisplayName(first), getDisplayName(second));
-    if (displayNameLoginOrder !== 0) {
-        return displayNameLoginOrder;
-    }
+        const displayNameLoginOrder = localeCompare(getDisplayName(first), getDisplayName(second));
+        if (displayNameLoginOrder !== 0) {
+            return displayNameLoginOrder;
+        }
 
-    return first.accountID - second.accountID;
+        return first.accountID - second.accountID;
+    });
 }
 
-export {trimLeadingSpace, hasEnoughSpaceForLargeSuggestionMenu, compareUserInList};
+export {trimLeadingSpace, hasEnoughSpaceForLargeSuggestionMenu, getSortedPersonalDetails};

src/pages/home/report/ReportActionCompose/SuggestionMention.tsx

@@ -19,7 +19,7 @@ import {getDisplayNameOrDefault} from '@libs/PersonalDetailsUtils';
 import getPolicyEmployeeAccountIDs from '@libs/PolicyEmployeeListUtils';
 import {canReportBeMentionedWithinPolicy, doesReportBelongToWorkspace, isGroupChat, isReportParticipant} from '@libs/ReportUtils';
 import StringUtils from '@libs/StringUtils';
-import {compareUserInList, trimLeadingSpace} from '@libs/SuggestionUtils';
+import {getSortedPersonalDetails, trimLeadingSpace} from '@libs/SuggestionUtils';
 import {isValidRoomName} from '@libs/ValidationUtils';
 import {searchInServer} from '@userActions/Report';
 import CONST from '@src/CONST';
@@ -62,7 +62,7 @@ function SuggestionMention(
     ref: ForwardedRef<SuggestionsRef>,
 ) {
     const personalDetails = usePersonalDetails();
-    const {translate, formatPhoneNumber} = useLocalize();
+    const {translate, formatPhoneNumber, localeCompare} = useLocalize();
     const [suggestionValues, setSuggestionValues] = useState(defaultSuggestionsValues);
     const suggestionValuesRef = useRef(suggestionValues);
     // eslint-disable-next-line react-compiler/react-compiler
@@ -303,7 +303,7 @@ function SuggestionMention(
             }) as Array<PersonalDetails & {weight: number}>;
 
             // At this point we are sure that the details are not null, since empty user details have been filtered in the previous step
-            const sortedPersonalDetails = filteredPersonalDetails.sort(compareUserInList);
+            const sortedPersonalDetails = getSortedPersonalDetails(filteredPersonalDetails, localeCompare);
 
             sortedPersonalDetails.slice(0, CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_SUGGESTIONS - suggestions.length).forEach((detail) => {
                 suggestions.push({
@@ -324,7 +324,7 @@ function SuggestionMention(
 
             return suggestions;
         },
-        [translate, formatPhoneNumber, formatLoginPrivateDomain],
+        [translate, formatPhoneNumber, formatLoginPrivateDomain, localeCompare],
     );
 
     const getRoomMentionOptions = useCallback(

tests/unit/SuggestionUtils.ts

@@ -0,0 +1,32 @@
+import {getSortedPersonalDetails} from '@libs/SuggestionUtils';
+import {localeCompare} from '../utils/TestHelper';
+
+describe('SuggestionUtils', () => {
+    describe('getSortedPersonalDetails', () => {
+        it('Should sort using the weight if the weight is different', () => {
+            const first = {login: 'John Doe', weight: 1, accountID: 1};
+            const second = {login: 'Jane Doe', weight: 2, accountID: 2};
+            expect(getSortedPersonalDetails([second, first], localeCompare)).toEqual([first, second]);
+        });
+
+        it('Should sort using the displayName if the weight is the same', () => {
+            const first = {login: 'águero', weight: 2, accountID: 3};
+            const second = {login: 'Bronn', weight: 2, accountID: 4};
+            const third = {login: 'Carol', weight: 2, accountID: 5};
+            expect(getSortedPersonalDetails([second, first, third], localeCompare)).toEqual([first, second, third]);
+            expect(getSortedPersonalDetails([third, second, first], localeCompare)).toEqual([first, second, third]);
+        });
+
+        it('Should sort using the accountID if both the weight and displayName are the same', () => {
+            const first = {login: 'aguero', weight: 2, accountID: 6};
+            const second = {login: 'aguero', weight: 2, accountID: 7};
+            expect(getSortedPersonalDetails([second, first], localeCompare)).toEqual([first, second]);
+        });
+
+        it('Should sort using the displayName with different diacritics if the weight is the same', () => {
+            const first = {login: 'águero', weight: 2, accountID: 8};
+            const second = {login: 'aguero', weight: 2, accountID: 8};
+            expect(getSortedPersonalDetails([second, first], localeCompare)).toEqual([second, first]);
+        });
+    });
+});