Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use localeCompare to sort in mention list #45324

Merged
merged 8 commits into from
Jul 31, 2024
29 changes: 29 additions & 0 deletions src/libs/compareUserInList.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import localeCompare from './LocaleCompare';

/**
* the comparison function used to determine which user will come first in the sorted list
* generally, smaller weight means will come first, and if the weight is the same, we'll sort based on displayName/login and accountID
*/

type UserDetailsWithWeight = {
displayName: string;
weight: number;
accountID: number;
};

function compareUserInList(first: UserDetailsWithWeight, second: UserDetailsWithWeight) {
// first, we should sort by weight
if (first.weight !== second.weight) {
return first.weight - second.weight;
}

const displayNameLoginOrder = localeCompare(first.displayName, second.displayName);
if (displayNameLoginOrder !== 0) {
return displayNameLoginOrder;
}

// Then fallback on accountID as the final sorting criteria.
return first.accountID - second.accountID;
}

export default compareUserInList;
17 changes: 14 additions & 3 deletions src/pages/home/report/ReportActionCompose/SuggestionMention.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import lodashMapValues from 'lodash/mapValues';
import lodashSortBy from 'lodash/sortBy';
import type {ForwardedRef} from 'react';
import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
import {useOnyx} from 'react-native-onyx';
import type {OnyxCollection} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import * as Expensicons from '@components/Icon/Expensicons';
import type {Mention} from '@components/MentionSuggestions';
import MentionSuggestions from '@components/MentionSuggestions';
Expand All @@ -14,6 +14,7 @@ import useCurrentReportID from '@hooks/useCurrentReportID';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useDebounce from '@hooks/useDebounce';
import useLocalize from '@hooks/useLocalize';
import compareUserInList from '@libs/compareUserInList';
import * as LoginUtils from '@libs/LoginUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import getPolicyEmployeeAccountIDs from '@libs/PolicyEmployeeListUtils';
Expand Down Expand Up @@ -270,9 +271,19 @@ function SuggestionMention(
}

return true;
});
}) as Array<PersonalDetails & {weight: number}>; // at this point we are sure that the details are not null

const sortedPersonalDetails = filteredPersonalDetails
.map((user) => ({
originalDetail: user,
weight: user.weight,
accountID: user.accountID,
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
displayName: ReportUtils.getDisplayNameForParticipant(user.accountID) || user.login || '',
}))
.sort((first, second) => compareUserInList(first, second))
.map((userAfterCompare) => userAfterCompare.originalDetail);

const sortedPersonalDetails = lodashSortBy(filteredPersonalDetails, ['weight', 'displayName', 'login']);
sortedPersonalDetails.slice(0, CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_SUGGESTIONS - suggestions.length).forEach((detail) => {
suggestions.push({
text: formatLoginPrivateDomain(PersonalDetailsUtils.getDisplayNameOrDefault(detail), detail?.login),
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/compareUserInListTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import compareUserInList from '@libs/compareUserInList';

describe('compareUserInList', () => {
it('should compare the weight if the weight is difference', () => {
const first = {displayName: 'John Doe', weight: 1, accountID: 1};
const second = {displayName: 'Jane Doe', weight: 2, accountID: 2};
expect(compareUserInList(first, second)).toBe(-1);
});

it('Should compare the displayName if the weight is the same', () => {
const first = {displayName: 'águero', weight: 2, accountID: 1};
const second = {displayName: 'Bronn', weight: 2, accountID: 1};
const third = {displayName: 'Carol', weight: 2, accountID: 1};
expect(compareUserInList(first, second)).toBe(-1);
expect(compareUserInList(first, third)).toBe(-1);
expect(compareUserInList(second, third)).toBe(-1);
});

it('Should compare the accountID if both the weight and displayName are the same', () => {
const first = {displayName: 'águero', weight: 2, accountID: 1};
const second = {displayName: 'aguero', weight: 2, accountID: 2};
expect(compareUserInList(first, second)).toBe(-1);
});
});
Loading