From 27e8a35c65cae981dd84dfc87aa6f623e1a5bbeb Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 27 Jun 2026 13:11:33 +0800 Subject: [PATCH 1/2] Refactor getLoginsByAccountIDs to accept personalDetailsList and update related usages --- src/libs/PersonalDetailsUtils.ts | 4 +-- .../DynamicReportParticipantsInvitePage.tsx | 2 +- src/pages/DynamicRoomInvitePage.tsx | 2 +- .../domain/Admins/DomainAddAdminPage.tsx | 5 +-- src/selectors/PersonalDetails.ts | 12 ++++++- .../actions/IOUTest/DeleteMoneyRequestTest.ts | 24 ++++++------- tests/actions/IOUTest/DuplicateTest.ts | 4 +-- tests/actions/IOUTest/TrackExpenseTest.ts | 8 ++--- tests/actions/MergeTransactionTest.ts | 8 ++--- tests/unit/PersonalDetailsSelectorTest.ts | 34 +++++++++++++++++++ 10 files changed, 74 insertions(+), 29 deletions(-) diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index bbaf4084ad2e..e17b50619f5d 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -287,9 +287,9 @@ function getLoginByAccountID(accountID: number | undefined, personalDetails: Ony * @param accountIDs Array of user accountIDs * @returns Array of logins according to passed accountIDs */ -function getLoginsByAccountIDs(accountIDs: number[]): string[] { +function getLoginsByAccountIDs(accountIDs: number[], personalDetailsList: OnyxEntry = allPersonalDetails): string[] { return accountIDs.reduce((foundLogins: string[], accountID) => { - const currentLogin = getLoginByAccountID(accountID); + const currentLogin = getLoginByAccountID(accountID, personalDetailsList); if (currentLogin) { foundLogins.push(currentLogin); } diff --git a/src/pages/DynamicReportParticipantsInvitePage.tsx b/src/pages/DynamicReportParticipantsInvitePage.tsx index 57d33f24788e..64ee80eb7078 100644 --- a/src/pages/DynamicReportParticipantsInvitePage.tsx +++ b/src/pages/DynamicReportParticipantsInvitePage.tsx @@ -47,7 +47,7 @@ function DynamicReportParticipantsInvitePage({report}: DynamicReportParticipants ...CONST.EXPENSIFY_EMAILS_OBJECT, }; const participantsAccountIDs = getParticipantsAccountIDsForDisplay(report, false, true); - const loginsByAccountIDs = getLoginsByAccountIDs(participantsAccountIDs); + const loginsByAccountIDs = getLoginsByAccountIDs(participantsAccountIDs, personalDetailsList); for (const login of loginsByAccountIDs) { excludedUsers[login] = true; } diff --git a/src/pages/DynamicRoomInvitePage.tsx b/src/pages/DynamicRoomInvitePage.tsx index a40b6b1fa51e..457322ac3061 100644 --- a/src/pages/DynamicRoomInvitePage.tsx +++ b/src/pages/DynamicRoomInvitePage.tsx @@ -71,7 +71,7 @@ function DynamicRoomInvitePage({report, policy, didScreenTransitionEnd}: Dynamic ...CONST.EXPENSIFY_EMAILS_OBJECT, }; const participantsAccountIDs = getParticipantsAccountIDsForDisplay(report, false, true, undefined, reportMetadata); - const loginsByAccountIDs = getLoginsByAccountIDs(participantsAccountIDs); + const loginsByAccountIDs = getLoginsByAccountIDs(participantsAccountIDs, personalDetailsList); for (const login of loginsByAccountIDs) { const smsDomain = addSMSDomainIfPhoneNumber(login); excludedUsers[smsDomain] = true; diff --git a/src/pages/domain/Admins/DomainAddAdminPage.tsx b/src/pages/domain/Admins/DomainAddAdminPage.tsx index bdb9b7d50d4b..036267ad7f94 100644 --- a/src/pages/domain/Admins/DomainAddAdminPage.tsx +++ b/src/pages/domain/Admins/DomainAddAdminPage.tsx @@ -18,7 +18,6 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import {getHeaderMessage} from '@libs/PersonalDetailOptionsListUtils'; import type {OptionData} from '@libs/PersonalDetailOptionsListUtils'; -import {getLoginsByAccountIDs} from '@libs/PersonalDetailsUtils'; import {addSMSDomainIfPhoneNumber, parsePhoneNumber} from '@libs/PhoneNumber'; import type {SettingsNavigatorParamList} from '@navigation/types'; import DomainNotFoundPageWrapper from '@pages/domain/DomainNotFoundPageWrapper'; @@ -27,6 +26,8 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; +import {personalDetailsLoginsSelector} from '@src/selectors/PersonalDetails'; +import getEmptyArray from '@src/types/utils/getEmptyArray'; type Sections = Section; @@ -46,6 +47,7 @@ function DomainAddAdminPage({route}: DomainAddAdminProps) { const [adminIDs] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, { selector: adminAccountIDsSelector, }); + const [adminLoginsByAccountIDs = getEmptyArray()] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsLoginsSelector(adminIDs)}); const [didScreenTransitionEnd, setDidScreenTransitionEnd] = useState(false); const didInvite = useRef(false); @@ -56,7 +58,6 @@ function DomainAddAdminPage({route}: DomainAddAdminProps) { const excludedUsers: Record = { ...CONST.EXPENSIFY_EMAILS_OBJECT, }; - const adminLoginsByAccountIDs = getLoginsByAccountIDs(adminIDs ?? []); for (const login of adminLoginsByAccountIDs) { const smsDomain = addSMSDomainIfPhoneNumber(login); excludedUsers[smsDomain] = true; diff --git a/src/selectors/PersonalDetails.ts b/src/selectors/PersonalDetails.ts index 26871b14dbdd..03963075e527 100644 --- a/src/selectors/PersonalDetails.ts +++ b/src/selectors/PersonalDetails.ts @@ -1,5 +1,12 @@ import type {OnyxEntry} from 'react-native-onyx'; -import {getDisplayNameOrDefault, getLoginByAccountID, getPersonalDetailsByID, getPersonalDetailsListByIDs, newGetPersonalDetailsByIDs} from '@libs/PersonalDetailsUtils'; +import { + getDisplayNameOrDefault, + getLoginByAccountID, + getLoginsByAccountIDs, + getPersonalDetailsByID, + getPersonalDetailsListByIDs, + newGetPersonalDetailsByIDs, +} from '@libs/PersonalDetailsUtils'; import CONST from '@src/CONST'; import type {PersonalDetails, PersonalDetailsList, Report} from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; @@ -13,6 +20,8 @@ const personalDetailsListSelector = (accountIDs: Array | und const personalDetailsLoginSelector = (accountID: number | undefined) => (personalDetailsList: OnyxEntry) => getLoginByAccountID(accountID, personalDetailsList); +const personalDetailsLoginsSelector = (accountIDs: number[] | undefined) => (personalDetailsList: OnyxEntry) => getLoginsByAccountIDs(accountIDs, personalDetailsList); + const personalDetailsDisplayNameSelector = (accountID: number) => (personalDetails: OnyxEntry) => getDisplayNameOrDefault(personalDetails?.[accountID]); const conciergePersonalDetailSelector = personalDetailsSelector(CONST.ACCOUNT_ID.CONCIERGE); @@ -72,6 +81,7 @@ export { personalDetailsListSelector, personalDetailsDisplayNameSelector, personalDetailsLoginSelector, + personalDetailsLoginsSelector, conciergePersonalDetailSelector, doesPersonalDetailExistSelector, accountIDToLoginSelector, diff --git a/tests/actions/IOUTest/DeleteMoneyRequestTest.ts b/tests/actions/IOUTest/DeleteMoneyRequestTest.ts index d2dffff6d6a0..a23208082d0e 100644 --- a/tests/actions/IOUTest/DeleteMoneyRequestTest.ts +++ b/tests/actions/IOUTest/DeleteMoneyRequestTest.ts @@ -522,9 +522,9 @@ describe('actions/IOU/DeleteMoneyRequest', () => { jest.advanceTimersByTime(10); // Given User logins from the participant accounts - const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); + const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -629,9 +629,9 @@ describe('actions/IOU/DeleteMoneyRequest', () => { jest.advanceTimersByTime(10); // Given User logins from the participant accounts - const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); + const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -762,10 +762,10 @@ describe('actions/IOU/DeleteMoneyRequest', () => { expect(thread.participants).toEqual(expectedTransactionThreadParticipants); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -909,9 +909,9 @@ describe('actions/IOU/DeleteMoneyRequest', () => { await waitForBatchedUpdates(); jest.advanceTimersByTime(10); - const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); + const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -1225,9 +1225,9 @@ describe('actions/IOU/DeleteMoneyRequest', () => { expect(thread.participants).toStrictEqual(expectedTransactionThreadParticipants); jest.advanceTimersByTime(10); - const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); + const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -1405,10 +1405,10 @@ describe('actions/IOU/DeleteMoneyRequest', () => { expect(thread.participants).toEqual(expectedTransactionThreadParticipants); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), diff --git a/tests/actions/IOUTest/DuplicateTest.ts b/tests/actions/IOUTest/DuplicateTest.ts index 708fe829d850..8bc22494650e 100644 --- a/tests/actions/IOUTest/DuplicateTest.ts +++ b/tests/actions/IOUTest/DuplicateTest.ts @@ -448,10 +448,10 @@ describe('actions/Duplicate', () => { [RORY_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, role: CONST.REPORT.ROLE.ADMIN}, }); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(transactionThreadReport1.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), diff --git a/tests/actions/IOUTest/TrackExpenseTest.ts b/tests/actions/IOUTest/TrackExpenseTest.ts index 7737edf98d6e..e33e1765d641 100644 --- a/tests/actions/IOUTest/TrackExpenseTest.ts +++ b/tests/actions/IOUTest/TrackExpenseTest.ts @@ -2326,10 +2326,10 @@ describe('actions/IOU/TrackExpense', () => { [CARLOS_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, role: CONST.REPORT.ROLE.MEMBER}, }); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -2437,10 +2437,10 @@ describe('actions/IOU/TrackExpense', () => { [CARLOS_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, role: CONST.REPORT.ROLE.MEMBER}, }); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 002899d4a071..d3320ad947ea 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1131,10 +1131,10 @@ describe('mergeTransactionRequest', () => { [TEST_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, role: CONST.REPORT.ROLE.ADMIN}, }); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), @@ -1315,10 +1315,10 @@ describe('mergeTransactionRequest', () => { [TEST_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, role: CONST.REPORT.ROLE.ADMIN}, }); + const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participantAccountIDs = Object.keys(thread.participants ?? {}).map(Number); - const userLogins = getLoginsByAccountIDs(participantAccountIDs); + const userLogins = getLoginsByAccountIDs(participantAccountIDs, allPersonalDetails); jest.advanceTimersByTime(10); - const allPersonalDetails = await getOnyxValue(ONYXKEYS.PERSONAL_DETAILS_LIST); const participants = userLogins.map((login, index) => ({ login, accountID: participantAccountIDs.at(index), diff --git a/tests/unit/PersonalDetailsSelectorTest.ts b/tests/unit/PersonalDetailsSelectorTest.ts index 3c404b91d1c3..6516405c91c2 100644 --- a/tests/unit/PersonalDetailsSelectorTest.ts +++ b/tests/unit/PersonalDetailsSelectorTest.ts @@ -4,6 +4,7 @@ import { personalDetailsDisplayNameSelector, personalDetailsListSelector, personalDetailsLoginSelector, + personalDetailsLoginsSelector, personalDetailsSelector, } from '@selectors/PersonalDetails'; import {getDisplayNameOrDefault} from '@libs/PersonalDetailsUtils'; @@ -98,6 +99,39 @@ describe('PersonalDetailsSelector', () => { }); }); + describe('personalDetailsLoginsSelector', () => { + const secondAccountID = 456; + const secondPersonalDetails = { + accountID: secondAccountID, + displayName: 'Second User', + login: 'second@user.com', + }; + const multiPersonalDetailsList = { + [accountID]: personalDetails, + [secondAccountID]: secondPersonalDetails, + } as unknown as PersonalDetailsList; + + it('should return the logins for the given accountIDs', () => { + const result = personalDetailsLoginsSelector([accountID, secondAccountID])(multiPersonalDetailsList); + expect(result).toEqual([personalDetails.login, secondPersonalDetails.login]); + }); + + it('should filter out accountIDs that do not exist in the list', () => { + const result = personalDetailsLoginsSelector([accountID, 999])(multiPersonalDetailsList); + expect(result).toEqual([personalDetails.login]); + }); + + it('should return an empty array if accountIDs is empty', () => { + const result = personalDetailsLoginsSelector([])(multiPersonalDetailsList); + expect(result).toEqual([]); + }); + + it('should return an empty array if none of the accountIDs exist in the list', () => { + const result = personalDetailsLoginsSelector([888, 999])(multiPersonalDetailsList); + expect(result).toEqual([]); + }); + }); + describe('multiPersonalDetailsSelector', () => { it('should return the personal details for the given accountIDs', () => { const result = multiPersonalDetailsSelector([accountID])(personalDetailsList); From 2b1eb15fe07d0f5a2aa919d0c2da1e5cd4302d00 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 27 Jun 2026 13:29:26 +0800 Subject: [PATCH 2/2] accept undefined --- src/libs/PersonalDetailsUtils.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index e17b50619f5d..ffbfd367ca60 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -287,14 +287,16 @@ function getLoginByAccountID(accountID: number | undefined, personalDetails: Ony * @param accountIDs Array of user accountIDs * @returns Array of logins according to passed accountIDs */ -function getLoginsByAccountIDs(accountIDs: number[], personalDetailsList: OnyxEntry = allPersonalDetails): string[] { - return accountIDs.reduce((foundLogins: string[], accountID) => { - const currentLogin = getLoginByAccountID(accountID, personalDetailsList); - if (currentLogin) { - foundLogins.push(currentLogin); - } - return foundLogins; - }, []); +function getLoginsByAccountIDs(accountIDs: number[] | undefined, personalDetailsList: OnyxEntry = allPersonalDetails): string[] { + return ( + accountIDs?.reduce((foundLogins: string[], accountID) => { + const currentLogin = getLoginByAccountID(accountID, personalDetailsList); + if (currentLogin) { + foundLogins.push(currentLogin); + } + return foundLogins; + }, []) ?? [] + ); } /**