Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/libs/SuggestionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ function trimLeadingSpace(str: string): string {
return str.startsWith(' ') ? str.slice(1) : str;
}

function getDisplayName(details: PersonalDetails) {
const displayNameFromAccountID = getDisplayNameForParticipant({accountID: details.accountID, formatPhoneNumber: formatPhoneNumberPhoneUtils});
function getDisplayName(details: PersonalDetails, translate: LocaleContextProps['translate']) {
const displayNameFromAccountID = getDisplayNameForParticipant({accountID: details.accountID, formatPhoneNumber: formatPhoneNumberPhoneUtils, translate});
if (!displayNameFromAccountID) {
return details.login?.length ? details.login : '';
}
Expand All @@ -23,13 +23,17 @@ function getDisplayName(details: PersonalDetails) {
/**
* Function to sort users. It compares weights, display names, and accountIDs in that order
*/
function getSortedPersonalDetails(personalDetails: Array<PersonalDetails & {weight: number}>, localeCompare: LocaleContextProps['localeCompare']) {
function getSortedPersonalDetails(
personalDetails: Array<PersonalDetails & {weight: number}>,
localeCompare: LocaleContextProps['localeCompare'],
translate: LocaleContextProps['translate'],
) {
return personalDetails.sort((first, second) => {
if (first.weight !== second.weight) {
return first.weight - second.weight;
}

const displayNameLoginOrder = localeCompare(getDisplayName(first), getDisplayName(second));
const displayNameLoginOrder = localeCompare(getDisplayName(first, translate), getDisplayName(second, translate));
if (displayNameLoginOrder !== 0) {
return displayNameLoginOrder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,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 = getSortedPersonalDetails(filteredPersonalDetails, localeCompare);
const sortedPersonalDetails = getSortedPersonalDetails(filteredPersonalDetails, localeCompare, translate);

for (const detail of sortedPersonalDetails.slice(0, CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_SUGGESTIONS - suggestions.length)) {
suggestions.push({
Expand Down
12 changes: 6 additions & 6 deletions tests/unit/SuggestionUtils.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
import {getSortedPersonalDetails} from '@libs/SuggestionUtils';

import {localeCompare} from '../utils/TestHelper';
import {localeCompare, translateLocal} 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]);
expect(getSortedPersonalDetails([second, first], localeCompare, translateLocal)).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]);
expect(getSortedPersonalDetails([second, first, third], localeCompare, translateLocal)).toEqual([first, second, third]);
expect(getSortedPersonalDetails([third, second, first], localeCompare, translateLocal)).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]);
expect(getSortedPersonalDetails([second, first], localeCompare, translateLocal)).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]);
expect(getSortedPersonalDetails([second, first], localeCompare, translateLocal)).toEqual([second, first]);
});
});
});
53 changes: 53 additions & 0 deletions tests/unit/SuggestionUtilsTranslateTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import type {LocalizedTranslate} from '@components/LocaleContextProvider';

import {getDisplayNameForParticipant} from '@libs/ReportUtils';
import {getSortedPersonalDetails} from '@libs/SuggestionUtils';

import type {PersonalDetails} from '@src/types/onyx';

import {localeCompare, translateLocal} from '../utils/TestHelper';

jest.mock('@libs/ReportUtils', () => {
Comment thread
cretadn22 marked this conversation as resolved.
// jest.requireActual is typed as returning `any`, so this assignment is unavoidably unsafe.
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const actual = jest.requireActual('@libs/ReportUtils');
Comment thread
cretadn22 marked this conversation as resolved.
// Spreading the actual (untyped `any`) module into the mock makes the return intentionally unsafe.
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
...actual,
__esModule: true,
getDisplayNameForParticipant: jest.fn(({accountID}: {accountID: number}) => `Name ${accountID}`),
};
});

const mockGetDisplayNameForParticipant = jest.mocked(getDisplayNameForParticipant);

describe('SuggestionUtils - getSortedPersonalDetails translate threading', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('passes the provided translate function to getDisplayNameForParticipant', () => {
const first = {login: 'first@test.com', weight: 2, accountID: 801} as PersonalDetails & {weight: number};
const second = {login: 'second@test.com', weight: 2, accountID: 802} as PersonalDetails & {weight: number};

getSortedPersonalDetails([second, first], localeCompare, translateLocal);

// The sort comparator resolves each display name via getDisplayNameForParticipant, which must receive the provided translate.
expect(mockGetDisplayNameForParticipant).toHaveBeenCalledWith(expect.objectContaining({translate: translateLocal}));
});

it('sorts by the display name resolved through the provided translate function', () => {
// Same weight forces the comparator to fall back to display names, which come from the provided translate.
const alpha = {login: 'zzz@test.com', weight: 5, accountID: 811} as PersonalDetails & {weight: number};
const beta = {login: 'aaa@test.com', weight: 5, accountID: 812} as PersonalDetails & {weight: number};
// translate drives the resolved name: account 811 -> 'Aaa', account 812 -> 'Zzz'.
const translateWithNames: LocalizedTranslate = translateLocal;
mockGetDisplayNameForParticipant.mockImplementation(({accountID}) => (accountID === 811 ? 'Aaa' : 'Zzz'));

const sorted = getSortedPersonalDetails([beta, alpha], localeCompare, translateWithNames);

// Despite beta being passed first, account 811 ('Aaa') sorts ahead of account 812 ('Zzz').
expect(sorted.map((detail) => detail.accountID)).toEqual([811, 812]);
});
});
Loading