Skip to content

Commit cd840a9

Browse files
GonalsGonals
authored
Merge pull request #89175 from TaduJR/feat-user-pill-workflow-attendees
fix: Attendees are not copied in alphabetical order when they are arranged alphabetically
2 parents 5256586 + 1082c1d commit cd840a9

11 files changed

Lines changed: 499 additions & 105 deletions

File tree

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

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import UserPills from '@components/UserPills';
66
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
77
import useLocalize from '@hooks/useLocalize';
88
import useThemeStyles from '@hooks/useThemeStyles';
9+
import {enrichAndSortAttendees} from '@libs/AttendeeUtils';
910
import Navigation from '@libs/Navigation/Navigation';
10-
import {sortAlphabetically} from '@libs/OptionsListUtils';
11-
import {getAttendees} from '@libs/TransactionUtils';
11+
import {getAttendees, getAttendeesListDisplayString} from '@libs/TransactionUtils';
1212
import CONST from '@src/CONST';
1313
import type {IOUAction, IOUType} from '@src/CONST';
1414
import type {TranslationPaths} from '@src/languages/types';
@@ -33,35 +33,22 @@ 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+
const iouAttendees = enrichAndSortAttendees(rawIouAttendees, personalDetailsList, localeCompare);
3738

3839
return (
3940
<MenuItemWithTopDescription
4041
key="attendees"
4142
shouldShowRightIcon={!isReadOnly}
42-
accessibilityLabel={`${translate('iou.attendees')}, ${iouAttendees?.map((a) => a?.displayName ?? a?.login).join(', ')}`}
43+
accessibilityLabel={`${translate('iou.attendees')}, ${Array.isArray(iouAttendees) ? getAttendeesListDisplayString(iouAttendees) : ''}`}
4344
description={`${translate('iou.attendees')} ${
4445
iouAttendees?.length && iouAttendees.length > 1 && formattedAmountPerAttendee ? `\u00B7 ${formattedAmountPerAttendee} ${translate('common.perPerson')}` : ''
4546
}`}
4647
descriptionTextStyle={styles.textLabelSupportingNormal}
4748
titleComponent={
4849
Array.isArray(iouAttendees) ? (
4950
<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) => ({
51+
users={iouAttendees.map((a) => ({
6552
avatar: a?.avatarUrl,
6653
displayName: a?.displayName ?? a?.login ?? a?.email ?? '',
6754
accountID: a?.accountID,

‎src/components/ReportActionItem/MoneyRequestView.tsx‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ import type {ViolationField} from '@hooks/useViolations';
4141
import useViolations from '@hooks/useViolations';
4242
import {updateMoneyRequestBillable, updateMoneyRequestReimbursable, updateMoneyRequestTaxRate} from '@libs/actions/IOU/UpdateMoneyRequest';
4343
import initSplitExpense from '@libs/actions/SplitExpenses';
44-
import {getIsMissingAttendeesViolation} from '@libs/AttendeeUtils';
44+
import {enrichAndSortAttendees, getIsMissingAttendeesViolation} from '@libs/AttendeeUtils';
4545
import {getBrokenConnectionUrlToFixPersonalCard, getCompanyCardDescription} from '@libs/CardUtils';
4646
import {getDecodedCategoryName, isCategoryMissing} from '@libs/CategoryUtils';
4747
import DistanceRequestUtils from '@libs/DistanceRequestUtils';
4848
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
4949
import {getRateFromMerchant} from '@libs/MergeTransactionUtils';
50-
import {hasEnabledOptions, sortAlphabetically} from '@libs/OptionsListUtils';
50+
import {hasEnabledOptions} from '@libs/OptionsListUtils';
5151
import Parser from '@libs/Parser';
5252
import {
5353
canSubmitPerDiemExpenseFromWorkspace,
@@ -82,6 +82,7 @@ import {
8282
} from '@libs/ReportUtils';
8383
import {hasEnabledTags, shouldShowDependentTagList} from '@libs/TagsOptionsListUtils';
8484
import {
85+
getAttendeesListDisplayString,
8586
getBillable,
8687
getCurrency,
8788
getDescription,
@@ -283,7 +284,8 @@ function MoneyRequestView({
283284
const isTransactionScanning = isScanning(updatedTransaction ?? transaction);
284285
const hasRoute = hasRouteTransactionUtils(transactionBackup ?? transaction, isDistanceRequest);
285286

286-
const actualAttendees = isFromMergeTransaction && updatedTransaction ? updatedTransaction.comment?.attendees : transactionAttendees;
287+
const rawActualAttendees = isFromMergeTransaction && updatedTransaction ? updatedTransaction.comment?.attendees : transactionAttendees;
288+
const actualAttendees = enrichAndSortAttendees(rawActualAttendees, personalDetailsList, localeCompare);
287289

288290
// Use the updated transaction amount in merge flow to have correct positive/negative sign
289291
const actualAmount = isFromMergeTransaction && updatedTransaction ? updatedTransaction.amount : transactionAmount;
@@ -802,7 +804,8 @@ function MoneyRequestView({
802804
const previousTagLength = getLengthOfTag(previousTag ?? '');
803805
const currentTagLength = getLengthOfTag(currentTransactionTag ?? '');
804806

805-
const getAttendeesTitle = Array.isArray(actualAttendees) ? actualAttendees.map((item) => item?.displayName ?? item?.login).join(', ') : '';
807+
// actualAttendees is already sorted by enrichAndSortAttendees above; pass without localeCompare to preserve that order while stripping the SMS domain.
808+
const getAttendeesTitle = Array.isArray(actualAttendees) ? getAttendeesListDisplayString(actualAttendees) : '';
806809
const attendeesCopyValue = !canEdit ? getAttendeesTitle : undefined;
807810

808811
const tagList = policyTagLists.map(({name, orderWeight, tags}, index) => {
@@ -1171,21 +1174,7 @@ function MoneyRequestView({
11711174
titleComponent={
11721175
Array.isArray(actualAttendees) ? (
11731176
<UserPills
1174-
users={sortAlphabetically(
1175-
actualAttendees.map((a) => {
1176-
const pd = a?.accountID ? personalDetailsList?.[a.accountID] : undefined;
1177-
const freshAvatar = typeof pd?.avatar === 'string' ? pd.avatar : undefined;
1178-
return {
1179-
...a,
1180-
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
1181-
displayName: pd?.displayName || a?.displayName,
1182-
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
1183-
avatarUrl: freshAvatar || a?.avatarUrl,
1184-
};
1185-
}),
1186-
'displayName',
1187-
localeCompare,
1188-
).map((a) => ({
1177+
users={actualAttendees.map((a) => ({
11891178
avatar: a?.avatarUrl,
11901179
displayName: a?.displayName ?? a?.login ?? a?.email ?? '',
11911180
accountID: a?.accountID,

‎src/libs/AttendeeUtils.ts‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import type {OnyxEntry} from 'react-native-onyx';
12
import type {LocaleContextProps} from '@components/LocaleContextProvider';
23
import CONST from '@src/CONST';
3-
import type {PolicyCategories, PolicyCategory} from '@src/types/onyx';
4+
import type {PersonalDetailsList, PolicyCategories, PolicyCategory} from '@src/types/onyx';
45
import type {Attendee} from '@src/types/onyx/IOU';
56
import type {CurrentUserPersonalDetails} from '@src/types/onyx/PersonalDetails';
7+
import {sortAlphabetically} from './OptionsListUtils';
68

79
function getNormalizedString(value?: string): string | undefined {
810
const normalizedValue = value?.trim();
@@ -133,4 +135,38 @@ function syncMissingAttendeesViolation<T extends {name: string}>(
133135
return violations;
134136
}
135137

136-
export {formatRequiredFieldsTitle, getIsMissingAttendeesViolation, normalizeAttendee, normalizeAttendees, syncMissingAttendeesViolation};
138+
/**
139+
* Enrich each attendee with live `personalDetails` and return them sorted alphabetically by displayName.
140+
*/
141+
function enrichAndSortAttendees(attendees: Attendee[], personalDetailsList: OnyxEntry<PersonalDetailsList>, localeCompare: LocaleContextProps['localeCompare']): Attendee[];
142+
function enrichAndSortAttendees(
143+
attendees: Attendee[] | string | undefined,
144+
personalDetailsList: OnyxEntry<PersonalDetailsList>,
145+
localeCompare: LocaleContextProps['localeCompare'],
146+
): Attendee[] | string | undefined;
147+
function enrichAndSortAttendees(
148+
attendees: Attendee[] | string | undefined,
149+
personalDetailsList: OnyxEntry<PersonalDetailsList>,
150+
localeCompare: LocaleContextProps['localeCompare'],
151+
): Attendee[] | string | undefined {
152+
if (!Array.isArray(attendees)) {
153+
return attendees;
154+
}
155+
return sortAlphabetically(
156+
attendees.map((a) => {
157+
const pd = a?.accountID ? personalDetailsList?.[a.accountID] : undefined;
158+
const freshAvatar = typeof pd?.avatar === 'string' ? pd.avatar : undefined;
159+
return {
160+
...a,
161+
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- intentional || to fall back when personalDetails has an empty string
162+
displayName: pd?.displayName || a?.displayName,
163+
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- intentional || to fall back when personalDetails has an empty string
164+
avatarUrl: freshAvatar || a?.avatarUrl,
165+
};
166+
}),
167+
'displayName',
168+
localeCompare,
169+
);
170+
}
171+
172+
export {enrichAndSortAttendees, formatRequiredFieldsTitle, getIsMissingAttendeesViolation, normalizeAttendee, normalizeAttendees, syncMissingAttendeesViolation};

‎src/libs/MergeTransactionUtils.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,14 @@ function selectTargetAndSourceTransactionsForMerge(
498498
* @param translate - The translation function
499499
* @returns The formatted display string for the field value
500500
*/
501-
function getDisplayValue(field: MergeFieldKey, transaction: Transaction, policy: Policy | undefined, translate: LocaleContextProps['translate'], reports?: Array<OnyxEntry<Report>>): string {
501+
function getDisplayValue(
502+
field: MergeFieldKey,
503+
transaction: Transaction,
504+
policy: Policy | undefined,
505+
translate: LocaleContextProps['translate'],
506+
localeCompare: LocaleContextProps['localeCompare'],
507+
reports?: Array<OnyxEntry<Report>>,
508+
): string {
502509
const fieldValue = getMergeFieldValue(getTransactionDetails(transaction), transaction, field);
503510

504511
if (isEmptyMergeValue(fieldValue) || fieldValue === undefined) {
@@ -528,7 +535,7 @@ function getDisplayValue(field: MergeFieldKey, transaction: Transaction, policy:
528535
return transaction?.reportName ?? getReportName(getReportOrDraftReport(SafeString(fieldValue), reports));
529536
}
530537
if (field === 'attendees') {
531-
return Array.isArray(fieldValue) ? getAttendeesListDisplayString(fieldValue) : '';
538+
return Array.isArray(fieldValue) ? getAttendeesListDisplayString(fieldValue, localeCompare) : '';
532539
}
533540

534541
if (field === 'taxValue') {
@@ -554,6 +561,7 @@ function buildMergeFieldsData(
554561
targetTransactionPolicy: Policy | undefined,
555562
sourceTransactionPolicy: Policy | undefined,
556563
translate: LocaleContextProps['translate'],
564+
localeCompare: LocaleContextProps['localeCompare'],
557565
reports: Array<OnyxEntry<Report>> = [],
558566
): MergeFieldData[] {
559567
if (!targetTransaction || !sourceTransaction) {
@@ -568,12 +576,12 @@ function buildMergeFieldsData(
568576
const options: MergeFieldOption[] = [
569577
{
570578
transaction: targetTransaction,
571-
displayValue: getDisplayValue(field, targetTransaction, targetTransactionPolicy, translate, reports),
579+
displayValue: getDisplayValue(field, targetTransaction, targetTransactionPolicy, translate, localeCompare, reports),
572580
isSelected: selectedTransactionId === targetTransaction.transactionID,
573581
},
574582
{
575583
transaction: sourceTransaction,
576-
displayValue: getDisplayValue(field, sourceTransaction, sourceTransactionPolicy, translate, reports),
584+
displayValue: getDisplayValue(field, sourceTransaction, sourceTransactionPolicy, translate, localeCompare, reports),
577585
isSelected: selectedTransactionId === sourceTransaction.transactionID,
578586
},
579587
];

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import {format, isValid, parse} from 'date-fns';
2+
import {Str} from 'expensify-common';
23
import {deepEqual} from 'fast-equals';
34
import lodashDeepClone from 'lodash/cloneDeep';
45
import lodashSet from 'lodash/set';
56
import type {NullishDeep, OnyxCollection, OnyxEntry} from 'react-native-onyx';
67
import Onyx from 'react-native-onyx';
78
import type {ValueOf} from 'type-fest';
9+
import type {LocaleContextProps} from '@components/LocaleContextProvider';
810
import type {Coordinate} from '@components/MapView/MapViewTypes';
911
import utils from '@components/MapView/utils';
1012
import type {UnreportedExpenseListItemType} from '@components/Search/SearchList/ListItem/types';
@@ -1204,19 +1206,26 @@ function getAttendees(transaction: OnyxInputOrEntry<Transaction>, currentUserPer
12041206
}
12051207

12061208
/**
1207-
* Return the list of attendees as a string of display names/logins.
1209+
* Returns attendees joined as a display string. Pass `localeCompare` to sort alphabetically (matches the pill sort);
1210+
* omit it to keep insertion order — used by non-React callers that don't want to thread the comparator.
1211+
* Strips the SMS domain so phone-login attendees render the same as in the rendered pills.
12081212
*/
1209-
function getAttendeesListDisplayString(attendees: Attendee[]): string {
1210-
return attendees.map((item) => item.displayName ?? item.login).join(', ');
1213+
function getAttendeesListDisplayString(attendees: Attendee[], localeCompare?: LocaleContextProps['localeCompare']): string {
1214+
const getName = (a: Attendee) => Str.removeSMSDomain(a.displayName ?? a.login ?? '');
1215+
const ordered = localeCompare
1216+
? // Lowercase to match sortAlphabetically (the pill sort) so joined string and pill order never disagree on case.
1217+
[...attendees].sort((a, b) => localeCompare(getName(a).toLowerCase(), getName(b).toLowerCase()))
1218+
: attendees;
1219+
return ordered.map(getName).join(', ');
12111220
}
12121221

12131222
/**
12141223
* Return the list of attendees as a string and modified list of attendees as a string if present.
12151224
*/
1216-
function getFormattedAttendees(modifiedAttendees?: Attendee[], attendees?: Attendee[]): [string, string] {
1225+
function getFormattedAttendees(modifiedAttendees?: Attendee[], attendees?: Attendee[], localeCompare?: LocaleContextProps['localeCompare']): [string, string] {
12171226
const oldAttendees = modifiedAttendees ?? [];
12181227
const newAttendees = attendees ?? [];
1219-
return [getAttendeesListDisplayString(oldAttendees), getAttendeesListDisplayString(newAttendees)];
1228+
return [getAttendeesListDisplayString(oldAttendees, localeCompare), getAttendeesListDisplayString(newAttendees, localeCompare)];
12201229
}
12211230

12221231
/**

‎src/pages/TransactionMerge/DetailsReviewPage.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ function DetailsReviewPage({route}: DetailsReviewPageProps) {
150150
// Build merge fields array with all necessary information
151151
const mergeFields = useMemo(
152152
() =>
153-
buildMergeFieldsData(conflictFields, targetTransaction, sourceTransaction, mergeTransaction, targetTransactionPolicy, sourceTransactionPolicy, translate, [
153+
buildMergeFieldsData(conflictFields, targetTransaction, sourceTransaction, mergeTransaction, targetTransactionPolicy, sourceTransactionPolicy, translate, localeCompare, [
154154
targetTransactionReport,
155155
sourceTransactionReport,
156156
]),
@@ -164,6 +164,7 @@ function DetailsReviewPage({route}: DetailsReviewPageProps) {
164164
targetTransactionPolicy,
165165
sourceTransactionPolicy,
166166
translate,
167+
localeCompare,
167168
],
168169
);
169170

‎tests/perf-test/ModifiedExpenseMessage.perf-test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,13 @@ test('[ModifiedExpenseMessage] getForReportAction on 1k reports and policies', a
6565
});
6666

6767
await waitForBatchedUpdates();
68-
await measureFunction(() => getForReportAction({translate: translateLocal, reportAction, policy: undefined, policyTags: mockedPolicyTags, currentUserLogin: CURRENT_USER_LOGIN}));
68+
await measureFunction(() =>
69+
getForReportAction({
70+
translate: translateLocal,
71+
reportAction,
72+
policy: undefined,
73+
policyTags: mockedPolicyTags,
74+
currentUserLogin: CURRENT_USER_LOGIN,
75+
}),
76+
);
6977
});

0 commit comments

Comments
 (0)