Skip to content

Commit da3a27b

Browse files
TaduJRTaduJR
committed
fix: sort attendees in one canonical order across all consumers
1 parent d91ed96 commit da3a27b

5 files changed

Lines changed: 82 additions & 36 deletions

File tree

‎src/components/MoneyRequestConfirmationList/sections/AttendeeField.tsx‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,25 @@ function AttendeeField({formattedAmountPerAttendee, isReadOnly, transactionID, a
3333
const personalDetailsList = usePersonalDetails();
3434
const shouldDisplayAttendeesError = formError === 'violations.missingAttendees';
3535

36-
const iouAttendees = getAttendees(transaction, currentUserPersonalDetails);
36+
const rawIouAttendees = getAttendees(transaction, currentUserPersonalDetails);
37+
// Enrich + sort once so pills and accessibility label share one canonical order.
38+
const iouAttendees = Array.isArray(rawIouAttendees)
39+
? sortAlphabetically(
40+
rawIouAttendees.map((a) => {
41+
const pd = a?.accountID ? personalDetailsList?.[a.accountID] : undefined;
42+
const freshAvatar = typeof pd?.avatar === 'string' ? pd.avatar : undefined;
43+
return {
44+
...a,
45+
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- intentional || to fall back when personalDetails has an empty string
46+
displayName: pd?.displayName || a?.displayName,
47+
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- intentional || to fall back when personalDetails has an empty string
48+
avatarUrl: freshAvatar || a?.avatarUrl,
49+
};
50+
}),
51+
'displayName',
52+
localeCompare,
53+
)
54+
: rawIouAttendees;
3755

3856
return (
3957
<MenuItemWithTopDescription
@@ -47,21 +65,7 @@ function AttendeeField({formattedAmountPerAttendee, isReadOnly, transactionID, a
4765
titleComponent={
4866
Array.isArray(iouAttendees) ? (
4967
<UserPills
50-
users={sortAlphabetically(
51-
iouAttendees.map((a) => {
52-
const pd = a?.accountID ? personalDetailsList?.[a.accountID] : undefined;
53-
const freshAvatar = typeof pd?.avatar === 'string' ? pd.avatar : undefined;
54-
return {
55-
...a,
56-
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
57-
displayName: pd?.displayName || a?.displayName,
58-
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
59-
avatarUrl: freshAvatar || a?.avatarUrl,
60-
};
61-
}),
62-
'displayName',
63-
localeCompare,
64-
).map((a) => ({
68+
users={iouAttendees.map((a) => ({
6569
avatar: a?.avatarUrl,
6670
displayName: a?.displayName ?? a?.login ?? a?.email ?? '',
6771
accountID: a?.accountID,

‎src/components/ReportActionItem/MoneyRequestView.tsx‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,25 @@ function MoneyRequestView({
284284
const isTransactionScanning = isScanning(updatedTransaction ?? transaction);
285285
const hasRoute = hasRouteTransactionUtils(transactionBackup ?? transaction, isDistanceRequest);
286286

287-
const actualAttendees = isFromMergeTransaction && updatedTransaction ? updatedTransaction.comment?.attendees : transactionAttendees;
287+
const rawActualAttendees = isFromMergeTransaction && updatedTransaction ? updatedTransaction.comment?.attendees : transactionAttendees;
288+
// Enrich + sort once so pills, hover-Copy value, accessibility label, and violation check share one canonical order.
289+
const actualAttendees = Array.isArray(rawActualAttendees)
290+
? sortAlphabetically(
291+
rawActualAttendees.map((a) => {
292+
const pd = a?.accountID ? personalDetailsList?.[a.accountID] : undefined;
293+
const freshAvatar = typeof pd?.avatar === 'string' ? pd.avatar : undefined;
294+
return {
295+
...a,
296+
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- intentional || to fall back when personalDetails has an empty string
297+
displayName: pd?.displayName || a?.displayName,
298+
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- intentional || to fall back when personalDetails has an empty string
299+
avatarUrl: freshAvatar || a?.avatarUrl,
300+
};
301+
}),
302+
'displayName',
303+
localeCompare,
304+
)
305+
: rawActualAttendees;
288306

289307
// Use the updated transaction amount in merge flow to have correct positive/negative sign
290308
const actualAmount = isFromMergeTransaction && updatedTransaction ? updatedTransaction.amount : transactionAmount;
@@ -1178,21 +1196,7 @@ function MoneyRequestView({
11781196
titleComponent={
11791197
Array.isArray(actualAttendees) ? (
11801198
<UserPills
1181-
users={sortAlphabetically(
1182-
actualAttendees.map((a) => {
1183-
const pd = a?.accountID ? personalDetailsList?.[a.accountID] : undefined;
1184-
const freshAvatar = typeof pd?.avatar === 'string' ? pd.avatar : undefined;
1185-
return {
1186-
...a,
1187-
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
1188-
displayName: pd?.displayName || a?.displayName,
1189-
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
1190-
avatarUrl: freshAvatar || a?.avatarUrl,
1191-
};
1192-
}),
1193-
'displayName',
1194-
localeCompare,
1195-
).map((a) => ({
1199+
users={actualAttendees.map((a) => ({
11961200
avatar: a?.avatarUrl,
11971201
displayName: a?.displayName ?? a?.login ?? a?.email ?? '',
11981202
accountID: a?.accountID,

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,11 +1201,17 @@ function getAttendees(transaction: OnyxInputOrEntry<Transaction>, currentUserPer
12011201
return attendees;
12021202
}
12031203

1204+
// Mirrors LocaleContextProvider so output here matches the user-facing pill order.
1205+
const ATTENDEES_DISPLAY_COLLATOR = new Intl.Collator(undefined, {usage: 'sort', sensitivity: 'variant', numeric: true, caseFirst: 'upper'});
1206+
12041207
/**
1205-
* Return the list of attendees as a string of display names/logins.
1208+
* Return the attendees list as an alphabetically sorted display string. Sorting here keeps every consumer in sync.
12061209
*/
12071210
function getAttendeesListDisplayString(attendees: Attendee[]): string {
1208-
return attendees.map((item) => item.displayName ?? item.login).join(', ');
1211+
return [...attendees]
1212+
.sort((a, b) => ATTENDEES_DISPLAY_COLLATOR.compare(a.displayName ?? a.login ?? '', b.displayName ?? b.login ?? ''))
1213+
.map((item) => item.displayName ?? item.login)
1214+
.join(', ');
12091215
}
12101216

12111217
/**

‎tests/unit/MergeTransactionUtilsTest.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,7 @@ describe('MergeTransactionUtils', () => {
893893
expect(result).toBe('Department, Engineering, Frontend');
894894
});
895895

896-
it('should return correct value for attendees field', () => {
896+
it('should return attendees in alphabetical order regardless of insertion order', () => {
897897
const transaction = {
898898
...createRandomTransaction(0),
899899
comment: {
@@ -906,7 +906,7 @@ describe('MergeTransactionUtils', () => {
906906
};
907907
const result = getDisplayValue('attendees', transaction, undefined, translateLocal);
908908

909-
expect(result).toBe('Test User 2, Test User 1');
909+
expect(result).toBe('Test User 1, Test User 2');
910910
});
911911

912912
it('should return string values directly', () => {

‎tests/unit/TransactionUtilsTest.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,6 +1391,38 @@ describe('TransactionUtils', () => {
13911391
});
13921392
});
13931393

1394+
describe('getAttendeesListDisplayString', () => {
1395+
it('returns attendees alphabetically regardless of insertion order (deploy blocker #89130)', () => {
1396+
const attendees: Attendee[] = [
1397+
{email: 'b@x.com', displayName: 'banana', avatarUrl: '', login: 'b@x.com'},
1398+
{email: 'a@x.com', displayName: 'apple', avatarUrl: '', login: 'a@x.com'},
1399+
];
1400+
expect(TransactionUtils.getAttendeesListDisplayString(attendees)).toBe('apple, banana');
1401+
});
1402+
1403+
it('uses numeric-aware sort so "User 9" comes before "User 10"', () => {
1404+
const attendees: Attendee[] = [
1405+
{email: '10@x.com', displayName: 'User 10', avatarUrl: '', login: '10@x.com'},
1406+
{email: '9@x.com', displayName: 'User 9', avatarUrl: '', login: '9@x.com'},
1407+
];
1408+
expect(TransactionUtils.getAttendeesListDisplayString(attendees)).toBe('User 9, User 10');
1409+
});
1410+
1411+
it('returns empty string for empty array', () => {
1412+
expect(TransactionUtils.getAttendeesListDisplayString([])).toBe('');
1413+
});
1414+
1415+
it('does not mutate the input array', () => {
1416+
const attendees: Attendee[] = [
1417+
{email: 'b@x.com', displayName: 'banana', avatarUrl: '', login: 'b@x.com'},
1418+
{email: 'a@x.com', displayName: 'apple', avatarUrl: '', login: 'a@x.com'},
1419+
];
1420+
const snapshot = [...attendees];
1421+
TransactionUtils.getAttendeesListDisplayString(attendees);
1422+
expect(attendees).toEqual(snapshot);
1423+
});
1424+
});
1425+
13941426
describe('isCategoryBeingAnalyzed', () => {
13951427
it('should return false for undefined transaction', () => {
13961428
expect(TransactionUtils.isCategoryBeingAnalyzed(undefined)).toBe(false);

0 commit comments

Comments
 (0)