From 2bafa075bb502280542bc7cfb436c12bce331ab2 Mon Sep 17 00:00:00 2001 From: allgandalf Date: Wed, 23 Jul 2025 16:39:35 +0530 Subject: [PATCH 01/22] fix getEffectiveDisplayName --- src/components/LocaleContextProvider.tsx | 4 +- src/libs/PersonalDetailsUtils.ts | 9 +- src/libs/ReportActionsUtils.ts | 4 +- src/libs/ReportUtils.ts | 2 +- tests/unit/libs/PersonalDetailsUtilsTest.ts | 99 +++++++++++++++++++++ tests/utils/TestHelper.ts | 7 ++ 6 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 tests/unit/libs/PersonalDetailsUtilsTest.ts diff --git a/src/components/LocaleContextProvider.tsx b/src/components/LocaleContextProvider.tsx index 4ef2453a6005..fc4f455a1323 100644 --- a/src/components/LocaleContextProvider.tsx +++ b/src/components/LocaleContextProvider.tsx @@ -53,6 +53,8 @@ type LocaleContextProps = { preferredLocale: Locale | undefined; }; +type FormatPhoneNumberType = LocaleContextProps['formatPhoneNumber']; + const LocaleContext = createContext({ translate: () => '', numberFormat: () => '', @@ -166,4 +168,4 @@ LocaleContextProvider.displayName = 'LocaleContextProvider'; export {LocaleContext, LocaleContextProvider}; -export type {Locale, LocaleContextProps}; +export type {Locale, LocaleContextProps, FormatPhoneNumberType}; diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index 4dfe2bf7f07b..228f5774f9b6 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -1,13 +1,14 @@ import {Str} from 'expensify-common'; import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; +import type {FormatPhoneNumberType} from '@components/LocaleContextProvider'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {OnyxInputOrEntry, PersonalDetails, PersonalDetailsList, PrivatePersonalDetails} from '@src/types/onyx'; import type {Address} from '@src/types/onyx/PrivatePersonalDetails'; import type {OnyxData} from '@src/types/onyx/Request'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; -import {formatPhoneNumber} from './LocalePhoneNumber'; +import {formatPhoneNumber as formatPhoneNumberUtils} from './LocalePhoneNumber'; import {translateLocal} from './Localize'; import {areEmailsFromSamePrivateDomain} from './LoginUtils'; import {parsePhoneNumber} from './PhoneNumber'; @@ -228,7 +229,7 @@ function getPersonalDetailsOnyxDataForOptimisticUsers(newLogins: string[], newAc personalDetailsNew[accountID] = { login, accountID, - displayName: formatPhoneNumber(login), + displayName: formatPhoneNumberUtils(login), isOptimisticPersonalDetail: true, }; @@ -323,7 +324,7 @@ function getFormattedAddress(privatePersonalDetails: OnyxEntry | OnyxInputOrEntry): string { // If we have a number like +15857527441@expensify.sms then let's remove @expensify.sms and format it // so that the option looks cleaner in our UI. - const userLogin = formatPhoneNumber(login); + const userLogin = formatPhoneNumberUtils(login); if (!passedPersonalDetails) { return userLogin; diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 40d12e88f016..7ce9b6e4267e 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -1392,7 +1392,7 @@ function getMemberChangeMessageElements(reportAction: OnyxEntry, g const mentionElements = targetAccountIDs.map((accountID): MemberChangeMessageUserMentionElement => { const personalDetail = personalDetails.find((personal) => personal.accountID === accountID); - const handleText = getEffectiveDisplayName(personalDetail) ?? translateLocal('common.hidden'); + const handleText = getEffectiveDisplayName(formatPhoneNumber, personalDetail) ?? translateLocal('common.hidden'); return { kind: 'userMention', @@ -1856,7 +1856,7 @@ function getActionableMentionWhisperMessage(reportAction: OnyxEntry { const personalDetail = personalDetails.find((personal) => personal.accountID === accountID); - const displayName = getEffectiveDisplayName(personalDetail); + const displayName = getEffectiveDisplayName(formatPhoneNumber, personalDetail); const handleText = isEmpty(displayName) ? translateLocal('common.hidden') : displayName; return `@${handleText}`; }); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 35235ca23893..f16ae4d7d5fb 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4779,7 +4779,7 @@ function getAdminRoomInvitedParticipants(parentReportAction: OnyxEntry { - const name = getEffectiveDisplayName(personalDetail); + const name = getEffectiveDisplayName(formatPhoneNumber, personalDetail); if (name && name?.length > 0) { return name; } diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts new file mode 100644 index 000000000000..d0afc131b712 --- /dev/null +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -0,0 +1,99 @@ +import Onyx from 'react-native-onyx'; +import {getEffectiveDisplayName} from '@libs/PersonalDetailsUtils'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetails} from '@src/types/onyx'; +import {formatPhoneNumber} from '../../utils/TestHelper'; + +describe('PersonalDetailsUtils', () => { + beforeAll(() => { + Onyx.merge(ONYXKEYS.COUNTRY_CODE, 1); + }); + + test('should return undefined when personalDetail is undefined', () => { + const result = getEffectiveDisplayName(formatPhoneNumber, undefined); + expect(result).toBeUndefined(); + expect(result).toBe(undefined); + }); + + test('should return undefined when personalDetail has neither login nor displayName', () => { + const personalDetail: PersonalDetails = {accountID: 123}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBeUndefined(); + expect(result).toBe(undefined); + }); + + test('should return displayName when login is empty or null but displayName exists', () => { + const personalDetail1: PersonalDetails = {accountID: 123, displayName: 'John Doe', login: ''}; + const personalDetail2: PersonalDetails = {accountID: 456, displayName: 'Jane Smith', login: null as unknown as string}; // Simulate null login + + let result = getEffectiveDisplayName(formatPhoneNumber, personalDetail1); + expect(result).toBe('John Doe'); + + result = getEffectiveDisplayName(formatPhoneNumber, personalDetail2); + expect(result).toBe('Jane Smith'); + }); + + test('should return login (email) when only login exists (not a phone number)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: 'john.doe@example.com'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('john.doe@example.com'); + }); + + test('should return national format for phone login if from the same region (US)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '+15551234567'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+1 555-123-4567'); + }); + + test('should return international format for phone login if from a different region (GB)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '+442079460000'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+44 20 7946 0000'); + }); + + test('should return formatted login (email) when both login and displayName exist (login takes precedence)', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: 'john.doe@example.com', + displayName: 'John Doe Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('john.doe@example.com'); + }); + + test('should return formatted login (phone) when both login (same region) and displayName exist', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: '+15551234567', + displayName: 'John Doe Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+1 555-123-4567'); + }); + + test('should return formatted login (phone) when both login (different region) and displayName exist', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: '+442079460000', + displayName: 'Jane Smith Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+44 20 7946 0000'); + }); + + test('should correctly handle login with SMS domain', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: `+18005550000`, + displayName: 'SMS User', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('(800) 555-0000'); + }); + + test('should fall back to displayName if formatted login is an empty string and displayName exists', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '', displayName: 'Fallback Name'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('Fallback Name'); + }); +}); diff --git a/tests/utils/TestHelper.ts b/tests/utils/TestHelper.ts index 549b3ea97851..7018087e9f10 100644 --- a/tests/utils/TestHelper.ts +++ b/tests/utils/TestHelper.ts @@ -4,6 +4,7 @@ import {Linking} from 'react-native'; import Onyx from 'react-native-onyx'; import type {ConnectOptions, OnyxKey} from 'react-native-onyx/dist/types'; import type {ApiCommand, ApiRequestCommandParameters} from '@libs/API/types'; +import {formatPhoneNumberWithCountryCode} from '@libs/LocalePhoneNumber'; import {translateLocal} from '@libs/Localize'; import Pusher from '@libs/Pusher'; import PusherConnectionManager from '@libs/PusherConnectionManager'; @@ -39,6 +40,11 @@ type FormData = { entries: () => Array<[string, string | Blob]>; }; +const countryCodeByIP = 1; +function formatPhoneNumber(phoneNumber: string) { + return formatPhoneNumberWithCountryCode(phoneNumber, countryCodeByIP); +} + function setupApp() { beforeAll(() => { Linking.setInitialURL('https://new.expensify.com/'); @@ -363,4 +369,5 @@ export { navigateToSidebarOption, getOnyxData, getNavigateToChatHintRegex, + formatPhoneNumber, }; From 2ef9333e5f4e920dcdaf0038267b1998564c2cc8 Mon Sep 17 00:00:00 2001 From: Gandalf Date: Wed, 23 Jul 2025 16:41:53 +0530 Subject: [PATCH 02/22] Update tests/unit/libs/PersonalDetailsUtilsTest.ts --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index d0afc131b712..f9d26a65cfb2 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -12,7 +12,6 @@ describe('PersonalDetailsUtils', () => { test('should return undefined when personalDetail is undefined', () => { const result = getEffectiveDisplayName(formatPhoneNumber, undefined); expect(result).toBeUndefined(); - expect(result).toBe(undefined); }); test('should return undefined when personalDetail has neither login nor displayName', () => { From c757a14ad0897ce74f74e67a467ed0c9b6598ab8 Mon Sep 17 00:00:00 2001 From: Gandalf Date: Wed, 23 Jul 2025 16:41:57 +0530 Subject: [PATCH 03/22] Update tests/unit/libs/PersonalDetailsUtilsTest.ts --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index f9d26a65cfb2..cf6d3e1829b0 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -18,7 +18,6 @@ describe('PersonalDetailsUtils', () => { const personalDetail: PersonalDetails = {accountID: 123}; const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); expect(result).toBeUndefined(); - expect(result).toBe(undefined); }); test('should return displayName when login is empty or null but displayName exists', () => { From e1e5367ed85f2c15c78d67b578b8b902786b1def Mon Sep 17 00:00:00 2001 From: allgandalf Date: Wed, 23 Jul 2025 19:37:48 +0530 Subject: [PATCH 04/22] remove unwanted import --- src/components/LocaleContextProvider.tsx | 4 +--- src/libs/PersonalDetailsUtils.ts | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/components/LocaleContextProvider.tsx b/src/components/LocaleContextProvider.tsx index fc4f455a1323..4ef2453a6005 100644 --- a/src/components/LocaleContextProvider.tsx +++ b/src/components/LocaleContextProvider.tsx @@ -53,8 +53,6 @@ type LocaleContextProps = { preferredLocale: Locale | undefined; }; -type FormatPhoneNumberType = LocaleContextProps['formatPhoneNumber']; - const LocaleContext = createContext({ translate: () => '', numberFormat: () => '', @@ -168,4 +166,4 @@ LocaleContextProvider.displayName = 'LocaleContextProvider'; export {LocaleContext, LocaleContextProvider}; -export type {Locale, LocaleContextProps, FormatPhoneNumberType}; +export type {Locale, LocaleContextProps}; diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index 228f5774f9b6..f86a96147d26 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -1,7 +1,7 @@ import {Str} from 'expensify-common'; import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; -import type {FormatPhoneNumberType} from '@components/LocaleContextProvider'; +import type {LocaleContextProps} from '@components/LocaleContextProvider'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {OnyxInputOrEntry, PersonalDetails, PersonalDetailsList, PrivatePersonalDetails} from '@src/types/onyx'; @@ -324,7 +324,7 @@ function getFormattedAddress(privatePersonalDetails: OnyxEntry Date: Wed, 23 Jul 2025 16:39:35 +0530 Subject: [PATCH 05/22] fix getEffectiveDisplayName --- src/components/LocaleContextProvider.tsx | 4 +- src/libs/PersonalDetailsUtils.ts | 4 +- src/libs/ReportActionsUtils.ts | 4 +- src/libs/ReportUtils.ts | 2 +- tests/unit/libs/PersonalDetailsUtilsTest.ts | 93 ++++++++++++++++++++- 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/components/LocaleContextProvider.tsx b/src/components/LocaleContextProvider.tsx index 4ef2453a6005..fc4f455a1323 100644 --- a/src/components/LocaleContextProvider.tsx +++ b/src/components/LocaleContextProvider.tsx @@ -53,6 +53,8 @@ type LocaleContextProps = { preferredLocale: Locale | undefined; }; +type FormatPhoneNumberType = LocaleContextProps['formatPhoneNumber']; + const LocaleContext = createContext({ translate: () => '', numberFormat: () => '', @@ -166,4 +168,4 @@ LocaleContextProvider.displayName = 'LocaleContextProvider'; export {LocaleContext, LocaleContextProvider}; -export type {Locale, LocaleContextProps}; +export type {Locale, LocaleContextProps, FormatPhoneNumberType}; diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index 7568be651762..06d00b844836 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -328,9 +328,9 @@ function getFormattedAddress(privatePersonalDetails: OnyxEntry, g const mentionElements = targetAccountIDs.map((accountID): MemberChangeMessageUserMentionElement => { const personalDetail = personalDetails.find((personal) => personal.accountID === accountID); - const handleText = getEffectiveDisplayName(personalDetail) ?? translateLocal('common.hidden'); + const handleText = getEffectiveDisplayName(formatPhoneNumber, personalDetail) ?? translateLocal('common.hidden'); return { kind: 'userMention', @@ -1856,7 +1856,7 @@ function getActionableMentionWhisperMessage(reportAction: OnyxEntry { const personalDetail = personalDetails.find((personal) => personal.accountID === accountID); - const displayName = getEffectiveDisplayName(personalDetail); + const displayName = getEffectiveDisplayName(formatPhoneNumber, personalDetail); const handleText = isEmpty(displayName) ? translateLocal('common.hidden') : displayName; return `@${handleText}`; }); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 35235ca23893..f16ae4d7d5fb 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4779,7 +4779,7 @@ function getAdminRoomInvitedParticipants(parentReportAction: OnyxEntry { - const name = getEffectiveDisplayName(personalDetail); + const name = getEffectiveDisplayName(formatPhoneNumber, personalDetail); if (name && name?.length > 0) { return name; } diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index 7ee452d5a1a5..f531934ea468 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -1,8 +1,10 @@ -import Onyx from 'react-native-onyx'; -import {getPersonalDetailsOnyxDataForOptimisticUsers} from '@libs/PersonalDetailsUtils'; +import {getEffectiveDisplayName, getPersonalDetailsOnyxDataForOptimisticUsers} from '@libs/PersonalDetailsUtils'; import ONYXKEYS from '@src/ONYXKEYS'; +import Onyx from 'react-native-onyx'; +import type {PersonalDetails} from '@src/types/onyx'; import {formatPhoneNumber} from '../../utils/TestHelper'; + describe('PersonalDetailsUtils', () => { beforeAll(async () => { await Onyx.merge(ONYXKEYS.COUNTRY_CODE, 1); @@ -56,4 +58,91 @@ describe('PersonalDetailsUtils', () => { expect(result).toEqual(expected); }); + test('should return undefined when personalDetail is undefined', () => { + const result = getEffectiveDisplayName(formatPhoneNumber, undefined); + expect(result).toBeUndefined(); + expect(result).toBe(undefined); + }); + + test('should return undefined when personalDetail has neither login nor displayName', () => { + const personalDetail: PersonalDetails = {accountID: 123}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBeUndefined(); + expect(result).toBe(undefined); + }); + + test('should return displayName when login is empty or null but displayName exists', () => { + const personalDetail1: PersonalDetails = {accountID: 123, displayName: 'John Doe', login: ''}; + const personalDetail2: PersonalDetails = {accountID: 456, displayName: 'Jane Smith', login: null as unknown as string}; // Simulate null login + + let result = getEffectiveDisplayName(formatPhoneNumber, personalDetail1); + expect(result).toBe('John Doe'); + + result = getEffectiveDisplayName(formatPhoneNumber, personalDetail2); + expect(result).toBe('Jane Smith'); + }); + + test('should return login (email) when only login exists (not a phone number)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: 'john.doe@example.com'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('john.doe@example.com'); + }); + + test('should return national format for phone login if from the same region (US)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '+15551234567'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+1 555-123-4567'); + }); + + test('should return international format for phone login if from a different region (GB)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '+442079460000'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+44 20 7946 0000'); + }); + + test('should return formatted login (email) when both login and displayName exist (login takes precedence)', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: 'john.doe@example.com', + displayName: 'John Doe Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('john.doe@example.com'); + }); + + test('should return formatted login (phone) when both login (same region) and displayName exist', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: '+15551234567', + displayName: 'John Doe Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+1 555-123-4567'); + }); + + test('should return formatted login (phone) when both login (different region) and displayName exist', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: '+442079460000', + displayName: 'Jane Smith Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+44 20 7946 0000'); + }); + + test('should correctly handle login with SMS domain', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: `+18005550000`, + displayName: 'SMS User', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('(800) 555-0000'); + }); + + test('should fall back to displayName if formatted login is an empty string and displayName exists', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '', displayName: 'Fallback Name'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('Fallback Name'); + }); }); From 996055437994b74e99e5126782dda6094ea9093b Mon Sep 17 00:00:00 2001 From: Gandalf Date: Wed, 23 Jul 2025 16:41:53 +0530 Subject: [PATCH 06/22] Update tests/unit/libs/PersonalDetailsUtilsTest.ts --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index f531934ea468..8a53fde352aa 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -61,7 +61,6 @@ describe('PersonalDetailsUtils', () => { test('should return undefined when personalDetail is undefined', () => { const result = getEffectiveDisplayName(formatPhoneNumber, undefined); expect(result).toBeUndefined(); - expect(result).toBe(undefined); }); test('should return undefined when personalDetail has neither login nor displayName', () => { From b76bc9f3e5695a8ebf04d72fb6df15692dc28a4c Mon Sep 17 00:00:00 2001 From: Gandalf Date: Wed, 23 Jul 2025 16:41:57 +0530 Subject: [PATCH 07/22] Update tests/unit/libs/PersonalDetailsUtilsTest.ts --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index 8a53fde352aa..8d444c1275d1 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -67,7 +67,6 @@ describe('PersonalDetailsUtils', () => { const personalDetail: PersonalDetails = {accountID: 123}; const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); expect(result).toBeUndefined(); - expect(result).toBe(undefined); }); test('should return displayName when login is empty or null but displayName exists', () => { From c6a74ece91ab96b5d98f989f8aaef6e2f5dd6a3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Muzyk?= Date: Wed, 16 Jul 2025 11:25:17 +0200 Subject: [PATCH 08/22] feat: 65634 Reduce amount of items perg page --- src/CONST/index.ts | 2 +- .../SelectionList/BaseSelectionList.tsx | 33 +++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/CONST/index.ts b/src/CONST/index.ts index b6d990ffe148..f4e3aab4d037 100755 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -5206,7 +5206,7 @@ const CONST = { * The maximum count of items per page for SelectionList. * When paginate, it multiplies by page number. */ - MAX_SELECTION_LIST_PAGE_LENGTH: 500, + MAX_SELECTION_LIST_PAGE_LENGTH: 50, /** * Bank account names diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 1ea9636fec34..347461e94794 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -167,7 +167,29 @@ function BaseSelectionList( const {isKeyboardShown} = useKeyboardState(); const [itemsToHighlight, setItemsToHighlight] = useState | null>(null); const itemFocusTimeoutRef = useRef(null); - const [currentPage, setCurrentPage] = useState(1); + const isItemSelected = useCallback( + (item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple), + [isSelected, selectedItems, canSelectMultiple], + ); + const calculateInitialCurrentPage = useCallback(() => { + if (canSelectMultiple || sections.length === 0) { + return 1; + } + + let currentIndex = 0; + for (const section of sections) { + if (section.data) { + for (const item of section.data) { + if (isItemSelected(item)) { + return Math.floor(currentIndex / CONST.MAX_SELECTION_LIST_PAGE_LENGTH) + 1; + } + currentIndex++; + } + } + } + return 1; + }, [canSelectMultiple, isItemSelected, sections]); + const [currentPage, setCurrentPage] = useState(() => calculateInitialCurrentPage()); const isTextInputFocusedRef = useRef(false); const {singleExecution} = useSingleExecution(); const [itemHeights, setItemHeights] = useState>({}); @@ -187,11 +209,6 @@ function BaseSelectionList( const incrementPage = () => setCurrentPage((prev) => prev + 1); - const isItemSelected = useCallback( - (item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple), - [isSelected, selectedItems, canSelectMultiple], - ); - const canShowProductTrainingTooltipMemo = useMemo(() => { return canShowProductTrainingTooltip && isFocused; }, [canShowProductTrainingTooltip, isFocused]); @@ -779,7 +796,9 @@ function BaseSelectionList( : 0; // Reset the current page to 1 when the user types something - setCurrentPage(1); + if (prevTextInputValue !== textInputValue) { + setCurrentPage(1); + } updateAndScrollToFocusedIndex(newSelectedIndex); }, [ From 9821e7d4f03925108326f60e2244f0d0b235050f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Muzyk?= Date: Fri, 18 Jul 2025 08:06:45 +0200 Subject: [PATCH 09/22] fix: add comment --- src/components/SelectionList/BaseSelectionList.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 347461e94794..7384e37e6dc7 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -171,6 +171,7 @@ function BaseSelectionList( (item: TItem) => item.isSelected ?? ((isSelected?.(item) ?? selectedItems.includes(item.keyForList ?? '')) && canSelectMultiple), [isSelected, selectedItems, canSelectMultiple], ); + /** Calculates on which page is selected item so we can scroll to it on first render */ const calculateInitialCurrentPage = useCallback(() => { if (canSelectMultiple || sections.length === 0) { return 1; From 060315da64e5e3bc0860b6d99d02ac5d445c915a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Muzyk?= Date: Mon, 21 Jul 2025 14:50:18 +0200 Subject: [PATCH 10/22] fix: search cases --- .../SelectionList/BaseSelectionList.tsx | 52 ++++++++++++++++--- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 7384e37e6dc7..25a7b1a162db 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -194,6 +194,7 @@ function BaseSelectionList( const isTextInputFocusedRef = useRef(false); const {singleExecution} = useSingleExecution(); const [itemHeights, setItemHeights] = useState>({}); + const pendingScrollIndexRef = useRef(null); const onItemLayout = (event: LayoutChangeEvent, itemKey: string | null | undefined) => { if (!itemKey) { @@ -342,6 +343,16 @@ function BaseSelectionList( return; } + const requiredPage = Math.floor((index + 1) / CONST.MAX_SELECTION_LIST_PAGE_LENGTH); + + // If the required page is beyond the current page, load all pages up to it, + // then return early and let the scroll happen after the page update + if (requiredPage > currentPage) { + pendingScrollIndexRef.current = index; + setCurrentPage(requiredPage); + return; + } + const itemIndex = item.index ?? -1; const sectionIndex = item.sectionIndex ?? -1; let viewOffsetToKeepFocusedItemAtTopOfViewableArea = 0; @@ -358,10 +369,11 @@ function BaseSelectionList( } listRef.current.scrollToLocation({sectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight - viewOffsetToKeepFocusedItemAtTopOfViewableArea}); + pendingScrollIndexRef.current = null; }, // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - [flattenedSections.allOptions], + [flattenedSections.allOptions, currentPage], ); const [disabledArrowKeyIndexes, setDisabledArrowKeyIndexes] = useState(flattenedSections.disabledArrowKeyOptionsIndexes); @@ -374,6 +386,21 @@ function BaseSelectionList( // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, [flattenedSections.disabledArrowKeyOptionsIndexes]); + /** Check whether there is a need to scroll to an item and if all items are loaded */ + useEffect(() => { + if (pendingScrollIndexRef.current === null) { + return; + } + + const indexToScroll = pendingScrollIndexRef.current; + const targetItem = flattenedSections.allOptions.at(indexToScroll); + + if (targetItem && indexToScroll < CONST.MAX_SELECTION_LIST_PAGE_LENGTH * currentPage) { + pendingScrollIndexRef.current = null; + scrollToIndex(indexToScroll, true); + } + }, [currentPage, scrollToIndex, flattenedSections.allOptions]); + const debouncedScrollToIndex = useMemo(() => lodashDebounce(scrollToIndex, CONST.TIMING.LIST_SCROLLING_DEBOUNCE_TIME, {leading: true, trailing: true}), [scrollToIndex]); const setHasKeyBeenPressed = useCallback(() => { @@ -788,19 +815,28 @@ function BaseSelectionList( ) { return; } + // Reset the current page to 1 when the user types something + if (prevTextInputValue !== textInputValue) { + setCurrentPage(1); + } + + // When clearing the search, scroll to the selected item if one exists + if (prevTextInputValue !== '' && textInputValue === '') { + const foundSelectedItemIndex = flattenedSections.allOptions.findIndex(isItemSelected); + if (foundSelectedItemIndex !== -1) { + updateAndScrollToFocusedIndex(foundSelectedItemIndex); + return; + } + } + // Remove the focus if the search input is empty and prev search input not empty or selected options length is changed (and allOptions length remains the same) // else focus on the first non disabled item const newSelectedIndex = - (isEmpty(prevTextInputValue) && textInputValue === '') || + (prevTextInputValue !== '' && textInputValue === '') || (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length) ? -1 : 0; - // Reset the current page to 1 when the user types something - if (prevTextInputValue !== textInputValue) { - setCurrentPage(1); - } - updateAndScrollToFocusedIndex(newSelectedIndex); }, [ canSelectMultiple, @@ -812,6 +848,8 @@ function BaseSelectionList( prevSelectedOptionsLength, prevAllOptionsLength, shouldUpdateFocusedIndex, + flattenedSections.allOptions, + isItemSelected, ]); useEffect( From 5c47c1a0ef71fa0f1778ef0e98f8eef28db27ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Muzyk?= Date: Tue, 22 Jul 2025 08:57:28 +0200 Subject: [PATCH 11/22] fix: go back to ceil --- src/components/SelectionList/BaseSelectionList.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 25a7b1a162db..488087253fef 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -343,7 +343,8 @@ function BaseSelectionList( return; } - const requiredPage = Math.floor((index + 1) / CONST.MAX_SELECTION_LIST_PAGE_LENGTH); + // Calculate which page is needed to show this index + const requiredPage = Math.ceil((index + 1) / CONST.MAX_SELECTION_LIST_PAGE_LENGTH); // If the required page is beyond the current page, load all pages up to it, // then return early and let the scroll happen after the page update From c46ea820e7f0afb79c9f68da99a323dc4c7253d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Muzyk?= Date: Tue, 22 Jul 2025 14:32:21 +0200 Subject: [PATCH 12/22] feat: More tests for BaseSelectionList --- tests/unit/BaseSelectionListTest.tsx | 127 ++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/tests/unit/BaseSelectionListTest.tsx b/tests/unit/BaseSelectionListTest.tsx index 575476b354c7..997b679e4546 100644 --- a/tests/unit/BaseSelectionListTest.tsx +++ b/tests/unit/BaseSelectionListTest.tsx @@ -1,5 +1,6 @@ import * as NativeNavigation from '@react-navigation/native'; import {fireEvent, render, screen} from '@testing-library/react-native'; +import {useState} from 'react'; import {SectionList} from 'react-native'; import BaseSelectionList from '@components/SelectionList/BaseSelectionList'; import RadioListItem from '@components/SelectionList/RadioListItem'; @@ -10,6 +11,9 @@ import CONST from '@src/CONST'; type BaseSelectionListSections = { sections: SelectionListProps['sections']; canSelectMultiple?: boolean; + initialNumToRender?: number; + searchText?: string; + setSearchText?: (searchText: string) => void; }; const mockSections = Array.from({length: 10}, (_, index) => ({ @@ -18,6 +22,18 @@ const mockSections = Array.from({length: 10}, (_, index) => ({ isSelected: index === 1, })); +const largeMockSections = Array.from({length: 100}, (_, index) => ({ + text: `Item ${index}`, + keyForList: `${index}`, + isSelected: index === 1, +})); + +const largeMockSectionsWithSelectedItemFromSecondPage = Array.from({length: 100}, (_, index) => ({ + text: `Item ${index}`, + keyForList: `${index}`, + isSelected: index === 70, +})); + jest.mock('@src/components/ConfirmedRoute.tsx'); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); @@ -28,20 +44,31 @@ jest.mock('@react-navigation/native', () => { }; }); +jest.mock('@hooks/useLocalize', () => + jest.fn(() => ({ + translate: jest.fn((key: string) => key), + numberFormat: jest.fn((num: number) => num.toString()), + })), +); + describe('BaseSelectionList', () => { const onSelectRowMock = jest.fn(); function BaseListItemRenderer(props: BaseSelectionListSections) { - const {sections, canSelectMultiple} = props; + const {sections, canSelectMultiple, initialNumToRender, setSearchText, searchText} = props; const focusedKey = sections[0].data.find((item) => item.isSelected)?.keyForList; return ( ); } @@ -87,4 +114,102 @@ describe('BaseSelectionList', () => { fireEvent.press(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)); expect(spy).toHaveBeenCalledWith(expect.objectContaining({itemIndex: 0})); }); + + it('should show only elements from first page and Show More button when items exceed page limit', () => { + render( + , + ); + + // Should render exactly first page (50 items) + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}49`)).toBeTruthy(); + + // Should NOT render items from second page + expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeFalsy(); + expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeFalsy(); + + expect(screen.getByText('common.showMore')).toBeTruthy(); + expect(screen.getByText('50')).toBeTruthy(); + expect(screen.getByText('100')).toBeTruthy(); + }); + + it('should hide Show More button when items fit on one page', () => { + render( + , + ); + + expect(screen.queryByText('common.showMore')).toBeFalsy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}9`)).toBeTruthy(); + }); + + it('should load more items when Show More button is clicked', () => { + render( + , + ); + + // Click Show More button + fireEvent.press(screen.getByText('common.showMore')); + + // Should now show items from second page + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}50`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}99`)).toBeTruthy(); + + // Should not show, Show more button as we rendered whole list + expect(screen.queryByText('common.showMore')).toBeFalsy(); + }); + + it('should search for first item then scroll back to preselected item when search is cleared', () => { + function SearchableListWrapper() { + const [searchText, setSearchText] = useState(''); + + // Filter sections based on search text + const filteredSections = searchText + ? largeMockSectionsWithSelectedItemFromSecondPage.filter((item) => item.text.toLowerCase().includes(searchText.toLowerCase())) + : largeMockSectionsWithSelectedItemFromSecondPage; + + return ( + + ); + } + + render(); + + // Initially should show item 70 as selected and visible + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected(); + + // Search for "Item 0" + fireEvent.changeText(screen.getByTestId('selection-list-text-input'), 'Item 0'); + + // Should show only the first item (Item 0) in search results + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}0`)).toBeTruthy(); + expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeFalsy(); + expect(screen.queryByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}1`)).toBeFalsy(); + + // Clear the search text + fireEvent.changeText(screen.getByTestId('selection-list-text-input'), ''); + + // Should scroll back to and show the preselected item 70 + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeTruthy(); + expect(screen.getByTestId(`${CONST.BASE_LIST_ITEM_TEST_ID}70`)).toBeSelected(); + }); }); From 429b2a4416f7063e0ddbea73f1a2d406881ae069 Mon Sep 17 00:00:00 2001 From: Vit Horacek Date: Wed, 23 Jul 2025 13:33:08 +0200 Subject: [PATCH 13/22] Add word to cspell --- cspell.json | 1 + 1 file changed, 1 insertion(+) diff --git a/cspell.json b/cspell.json index 94144d242d8a..b669599be3e6 100644 --- a/cspell.json +++ b/cspell.json @@ -8,6 +8,7 @@ "ADDCOMMENT", "Addendums", "ADFS", + "Aeroplan", "águero", "Aircall", "airshipconfig", From 553285d9460f27eb952971c4a61ff89306a6b34c Mon Sep 17 00:00:00 2001 From: allgandalf Date: Wed, 23 Jul 2025 19:37:48 +0530 Subject: [PATCH 14/22] remove unwanted import --- src/components/LocaleContextProvider.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/LocaleContextProvider.tsx b/src/components/LocaleContextProvider.tsx index fc4f455a1323..4ef2453a6005 100644 --- a/src/components/LocaleContextProvider.tsx +++ b/src/components/LocaleContextProvider.tsx @@ -53,8 +53,6 @@ type LocaleContextProps = { preferredLocale: Locale | undefined; }; -type FormatPhoneNumberType = LocaleContextProps['formatPhoneNumber']; - const LocaleContext = createContext({ translate: () => '', numberFormat: () => '', @@ -168,4 +166,4 @@ LocaleContextProvider.displayName = 'LocaleContextProvider'; export {LocaleContext, LocaleContextProvider}; -export type {Locale, LocaleContextProps, FormatPhoneNumberType}; +export type {Locale, LocaleContextProps}; From cc2726bd60b7350b2bd751f9a27ed918d79b033c Mon Sep 17 00:00:00 2001 From: allgandalf Date: Thu, 24 Jul 2025 12:33:11 +0530 Subject: [PATCH 15/22] fix prettier --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index f51cdea8917d..d3145f155921 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -1,10 +1,9 @@ -import {getEffectiveDisplayName, getPersonalDetailsOnyxDataForOptimisticUsers} from '@libs/PersonalDetailsUtils'; import Onyx from 'react-native-onyx'; +import {getEffectiveDisplayName, getPersonalDetailsOnyxDataForOptimisticUsers} from '@libs/PersonalDetailsUtils'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PersonalDetails} from '@src/types/onyx'; import {formatPhoneNumber} from '../../utils/TestHelper'; - describe('PersonalDetailsUtils', () => { beforeAll(async () => { await Onyx.merge(ONYXKEYS.COUNTRY_CODE, 1); From 5eb09cfd5c72097b3ad3f108fb4729c5f6a308f9 Mon Sep 17 00:00:00 2001 From: allgandalf Date: Thu, 24 Jul 2025 14:19:16 +0530 Subject: [PATCH 16/22] fix lint --- src/libs/PersonalDetailsUtils.ts | 2 +- src/pages/InviteReportParticipantsPage.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index 87cb1a72a11c..06d00b844836 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -233,7 +233,7 @@ function getPersonalDetailsOnyxDataForOptimisticUsers( personalDetailsNew[accountID] = { login, accountID, - displayName: formatPhoneNumberUtils(login), + displayName: formatPhoneNumber(login), isOptimisticPersonalDetail: true, }; diff --git a/src/pages/InviteReportParticipantsPage.tsx b/src/pages/InviteReportParticipantsPage.tsx index 85f1e613e454..bbe83f58ee16 100644 --- a/src/pages/InviteReportParticipantsPage.tsx +++ b/src/pages/InviteReportParticipantsPage.tsx @@ -200,7 +200,7 @@ function InviteReportParticipantsPage({betas, report, didScreenTransitionEnd}: I }); inviteToGroupChat(reportID, invitedEmailsToAccountIDs, formatPhoneNumber); goBack(); - }, [selectedOptions, goBack, reportID, validate]); + }, [selectedOptions, goBack, reportID, validate, formatPhoneNumber]); const headerMessage = useMemo(() => { const processedLogin = debouncedSearchTerm.trim().toLowerCase(); From 57d810a32c5d5df555011c15b9beace926d93bed Mon Sep 17 00:00:00 2001 From: allgandalf Date: Wed, 30 Jul 2025 23:08:34 +0530 Subject: [PATCH 17/22] format unit test --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 195 ++++++++------------ 1 file changed, 74 insertions(+), 121 deletions(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index a2c2f96b0023..253b401f1347 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -9,139 +9,92 @@ describe('PersonalDetailsUtils', () => { await Onyx.merge(ONYXKEYS.COUNTRY_CODE, 1); }); - test('getPersonalDetailsOnyxDataForOptimisticUsers should return correct optimistic and finally data', () => { - const newLogins = ['3322076524', 'test2@test.com']; - const newAccountIDs = [1, 2]; - - // Call the function with the mock formatPhoneNumber - const result = getPersonalDetailsOnyxDataForOptimisticUsers(newLogins, newAccountIDs, formatPhoneNumber); - - // Construct the expected output based on the function's logic and the mock behavior. - const expected = { - optimisticData: [ - { - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - onyxMethod: 'merge', - value: { - // eslint-disable-next-line @typescript-eslint/naming-convention - '1': { - accountID: 1, - displayName: '3322076524', - isOptimisticPersonalDetail: true, - login: '3322076524', - }, - // eslint-disable-next-line @typescript-eslint/naming-convention - '2': { - accountID: 2, - // This comes from mockFormatPhoneNumber('test2@test.com') - displayName: 'test2@test.com', - isOptimisticPersonalDetail: true, - login: 'test2@test.com', - }, - }, - }, - ], - finallyData: [ - { - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - onyxMethod: Onyx.METHOD.MERGE, - value: { - // eslint-disable-next-line @typescript-eslint/naming-convention - '1': null, - // eslint-disable-next-line @typescript-eslint/naming-convention - '2': null, - }, - }, - ], - }; - - expect(result).toEqual(expected); - }); - - test('should return undefined when personalDetail is undefined', () => { - const result = getEffectiveDisplayName(formatPhoneNumber, undefined); - expect(result).toBeUndefined(); - }); + describe('getEffectiveDisplayName', () => { + test('should return undefined when personalDetail is undefined', () => { + const result = getEffectiveDisplayName(formatPhoneNumber, undefined); + expect(result).toBeUndefined(); + }); - test('should return undefined when personalDetail has neither login nor displayName', () => { - const personalDetail: PersonalDetails = {accountID: 123}; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBeUndefined(); - }); + test('should return undefined when personalDetail has neither login nor displayName', () => { + const personalDetail: PersonalDetails = {accountID: 123}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBeUndefined(); + }); - test('should return displayName when login is empty or null but displayName exists', () => { - const personalDetail1: PersonalDetails = {accountID: 123, displayName: 'John Doe', login: ''}; - const personalDetail2: PersonalDetails = {accountID: 456, displayName: 'Jane Smith', login: null as unknown as string}; // Simulate null login + test('should return displayName when login is empty or null but displayName exists', () => { + const personalDetail1: PersonalDetails = {accountID: 123, displayName: 'John Doe', login: ''}; + const personalDetail2: PersonalDetails = {accountID: 456, displayName: 'Jane Smith', login: null as unknown as string}; // Simulate null login - let result = getEffectiveDisplayName(formatPhoneNumber, personalDetail1); - expect(result).toBe('John Doe'); + let result = getEffectiveDisplayName(formatPhoneNumber, personalDetail1); + expect(result).toBe('John Doe'); - result = getEffectiveDisplayName(formatPhoneNumber, personalDetail2); - expect(result).toBe('Jane Smith'); - }); + result = getEffectiveDisplayName(formatPhoneNumber, personalDetail2); + expect(result).toBe('Jane Smith'); + }); - test('should return login (email) when only login exists (not a phone number)', () => { - const personalDetail: PersonalDetails = {accountID: 123, login: 'john.doe@example.com'}; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('john.doe@example.com'); - }); + test('should return login (email) when only login exists (not a phone number)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: 'john.doe@example.com'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('john.doe@example.com'); + }); - test('should return national format for phone login if from the same region (US)', () => { - const personalDetail: PersonalDetails = {accountID: 123, login: '+15551234567'}; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('+1 555-123-4567'); - }); + test('should return national format for phone login if from the same region (US)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '+15551234567'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+1 555-123-4567'); + }); - test('should return international format for phone login if from a different region (GB)', () => { - const personalDetail: PersonalDetails = {accountID: 123, login: '+442079460000'}; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('+44 20 7946 0000'); - }); + test('should return international format for phone login if from a different region (GB)', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '+442079460000'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+44 20 7946 0000'); + }); - test('should return formatted login (email) when both login and displayName exist (login takes precedence)', () => { - const personalDetail: PersonalDetails = { - accountID: 123, - login: 'john.doe@example.com', - displayName: 'John Doe Full Name', - }; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('john.doe@example.com'); - }); + test('should return formatted login (email) when both login and displayName exist (login takes precedence)', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: 'john.doe@example.com', + displayName: 'John Doe Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('john.doe@example.com'); + }); - test('should return formatted login (phone) when both login (same region) and displayName exist', () => { - const personalDetail: PersonalDetails = { - accountID: 123, - login: '+15551234567', - displayName: 'John Doe Full Name', - }; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('+1 555-123-4567'); - }); + test('should return formatted login (phone) when both login (same region) and displayName exist', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: '+15551234567', + displayName: 'John Doe Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+1 555-123-4567'); + }); - test('should return formatted login (phone) when both login (different region) and displayName exist', () => { - const personalDetail: PersonalDetails = { - accountID: 123, - login: '+442079460000', - displayName: 'Jane Smith Full Name', - }; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('+44 20 7946 0000'); - }); + test('should return formatted login (phone) when both login (different region) and displayName exist', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: '+442079460000', + displayName: 'Jane Smith Full Name', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('+44 20 7946 0000'); + }); - test('should correctly handle login with SMS domain', () => { - const personalDetail: PersonalDetails = { - accountID: 123, - login: `+18005550000`, - displayName: 'SMS User', - }; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('(800) 555-0000'); - }); + test('should correctly handle login with SMS domain', () => { + const personalDetail: PersonalDetails = { + accountID: 123, + login: `+18005550000`, + displayName: 'SMS User', + }; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('(800) 555-0000'); + }); - test('should fall back to displayName if formatted login is an empty string and displayName exists', () => { - const personalDetail: PersonalDetails = {accountID: 123, login: '', displayName: 'Fallback Name'}; - const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('Fallback Name'); + test('should fall back to displayName if formatted login is an empty string and displayName exists', () => { + const personalDetail: PersonalDetails = {accountID: 123, login: '', displayName: 'Fallback Name'}; + const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); + expect(result).toBe('Fallback Name'); + }); }); describe('getPersonalDetailsOnyxDataForOptimisticUsers', () => { From e446f57b52ff462e0cb3dbc5989c58ce1e28f329 Mon Sep 17 00:00:00 2001 From: Gandalf Date: Wed, 30 Jul 2025 23:11:35 +0530 Subject: [PATCH 18/22] Update tests/unit/libs/PersonalDetailsUtilsTest.ts Co-authored-by: Shubham Agrawal <58412969+shubham1206agra@users.noreply.github.com> --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index 253b401f1347..8d206b9f5f42 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -5,9 +5,6 @@ import type {PersonalDetails} from '@src/types/onyx'; import {formatPhoneNumber} from '../../utils/TestHelper'; describe('PersonalDetailsUtils', () => { - beforeAll(async () => { - await Onyx.merge(ONYXKEYS.COUNTRY_CODE, 1); - }); describe('getEffectiveDisplayName', () => { test('should return undefined when personalDetail is undefined', () => { From 34c1870b357231b93d65f34bdd787ceeb1502d7d Mon Sep 17 00:00:00 2001 From: allgandalf Date: Wed, 30 Jul 2025 23:12:23 +0530 Subject: [PATCH 19/22] fix prettier --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index 8d206b9f5f42..5511066460b3 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -5,7 +5,6 @@ import type {PersonalDetails} from '@src/types/onyx'; import {formatPhoneNumber} from '../../utils/TestHelper'; describe('PersonalDetailsUtils', () => { - describe('getEffectiveDisplayName', () => { test('should return undefined when personalDetail is undefined', () => { const result = getEffectiveDisplayName(formatPhoneNumber, undefined); From fba80219305ba03a60cf6157278da268a01aa918 Mon Sep 17 00:00:00 2001 From: allgandalf Date: Wed, 30 Jul 2025 23:17:11 +0530 Subject: [PATCH 20/22] fix failing test --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index 5511066460b3..7ca3b21c8161 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -83,7 +83,7 @@ describe('PersonalDetailsUtils', () => { displayName: 'SMS User', }; const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('(800) 555-0000'); + expect(result).toBe('+1 800-555-0000'); }); test('should fall back to displayName if formatted login is an empty string and displayName exists', () => { From 192b2a6bafd5dd9c31137262d97a2e0dd1068912 Mon Sep 17 00:00:00 2001 From: Gandalf Date: Wed, 30 Jul 2025 23:19:24 +0530 Subject: [PATCH 21/22] Apply suggestion from @shubham1206agra Co-authored-by: Shubham Agrawal <58412969+shubham1206agra@users.noreply.github.com> --- src/libs/PersonalDetailsUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index 441ccf0f98a2..06d00b844836 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -330,7 +330,7 @@ function getFormattedAddress(privatePersonalDetails: OnyxEntry Date: Wed, 30 Jul 2025 23:23:38 +0530 Subject: [PATCH 22/22] fix unit test --- tests/unit/libs/PersonalDetailsUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/libs/PersonalDetailsUtilsTest.ts b/tests/unit/libs/PersonalDetailsUtilsTest.ts index 7ca3b21c8161..5511066460b3 100644 --- a/tests/unit/libs/PersonalDetailsUtilsTest.ts +++ b/tests/unit/libs/PersonalDetailsUtilsTest.ts @@ -83,7 +83,7 @@ describe('PersonalDetailsUtils', () => { displayName: 'SMS User', }; const result = getEffectiveDisplayName(formatPhoneNumber, personalDetail); - expect(result).toBe('+1 800-555-0000'); + expect(result).toBe('(800) 555-0000'); }); test('should fall back to displayName if formatted login is an empty string and displayName exists', () => {