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
35 changes: 32 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 localeCompare from '@libs/LocaleCompare';
import * as LoginUtils from '@libs/LoginUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import getPolicyEmployeeAccountIDs from '@libs/PolicyEmployeeListUtils';
Expand Down Expand Up @@ -56,6 +57,30 @@ type SuggestionPersonalDetailsList = Record<
| null
>;

function getDisplayName(details: PersonalDetails) {
const displayNameFromAccountID = ReportUtils.getDisplayNameForParticipant(details.accountID);
if (!displayNameFromAccountID) {
return details.login?.length ? details.login : '';
}
return displayNameFromAccountID;
}

/**
* Comparison 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;
}

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

return first.accountID - second.accountID;
}

function SuggestionMention(
{value, selection, setSelection, updateComment, isAutoSuggestionPickerLarge, measureParentContainerAndReportCursor, isComposerFocused, isGroupPolicyReport, policyID}: SuggestionProps,
ref: ForwardedRef<SuggestionsRef>,
Expand Down Expand Up @@ -270,9 +295,11 @@ function SuggestionMention(
}

return true;
});
}) 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 = 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 Expand Up @@ -443,3 +470,5 @@ function SuggestionMention(
SuggestionMention.displayName = 'SuggestionMention';

export default forwardRef(SuggestionMention);

export {compareUserInList};
30 changes: 30 additions & 0 deletions tests/unit/compareUserInListTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import {compareUserInList} from '@pages/home/report/ReportActionCompose/SuggestionMention';

describe('compareUserInList', () => {
it('Should compare 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(compareUserInList(first, second)).toBe(-1);
dominictb marked this conversation as resolved.
Show resolved Hide resolved
expect(compareUserInList(second, first)).toBe(1);
});

it('Should compare 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(compareUserInList(first, second)).toBe(-1);
dominictb marked this conversation as resolved.
Show resolved Hide resolved
expect(compareUserInList(first, third)).toBe(-1);
expect(compareUserInList(second, third)).toBe(-1);

expect(compareUserInList(second, first)).toBe(1);
expect(compareUserInList(third, first)).toBe(1);
expect(compareUserInList(third, second)).toBe(1);
});

it('Should compare the accountID if both the weight and displayName are the same', () => {
const first = {login: 'águero', weight: 2, accountID: 6};
const second = {login: 'aguero', weight: 2, accountID: 7};
expect(compareUserInList(first, second)).toBe(-1);
dominictb marked this conversation as resolved.
Show resolved Hide resolved
expect(compareUserInList(second, first)).toBe(1);
});
});
Loading