From a73854b5f7319d5ef89cc54af5dc8f181f4b08d8 Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 20 Oct 2026 15:54:04 +0700 Subject: [PATCH 1/4] refactor: remove allPolicies read from getParentNavigationSubtitle --- src/components/AvatarWithDisplayName.tsx | 14 +++- src/libs/ReportUtils.ts | 6 +- .../ReportDetailsTitleSection.tsx | 14 +++- src/pages/ShareCodePage.tsx | 8 +- src/pages/inbox/HeaderView.tsx | 4 + tests/unit/ReportUtilsTest.ts | 84 +++++++++++++++++-- 6 files changed, 115 insertions(+), 15 deletions(-) diff --git a/src/components/AvatarWithDisplayName.tsx b/src/components/AvatarWithDisplayName.tsx index 3ec898ae2a38..5e09ed63a477 100644 --- a/src/components/AvatarWithDisplayName.tsx +++ b/src/components/AvatarWithDisplayName.tsx @@ -22,6 +22,7 @@ import { canEditReportTitle, getChatRoomSubtitle, getDisplayNamesWithTooltips, + getInvoiceReceiverPolicyID, getParentNavigationSubtitle, getReportStatusColorStyle, getReportStatusTooltipTranslation, @@ -209,10 +210,12 @@ function AvatarWithDisplayName({ const isInSidePanel = useIsInSidePanel(); const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`); + const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`); const [personalDetails] = useAllPersonalDetails() ?? CONST.EMPTY_OBJECT; const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const policy = usePolicy(report?.policyID); + const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentReport)); const theme = useTheme(); const styles = useThemeStyles(); // We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to display the edit button only on large screens @@ -225,7 +228,16 @@ function AvatarWithDisplayName({ const title = getReportName(report, getReportNameFromNames(derivedReportNames, report?.reportID)); const isParentReportArchived = useReportIsArchived(report?.parentReportID); const subtitle = getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, true, isReportArchived); - const parentNavigationSubtitleData = getParentNavigationSubtitle(report, policy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived); + const parentNavigationSubtitleData = getParentNavigationSubtitle( + report, + policy, + invoiceReceiverPolicy, + conciergeReportID, + translate, + formatPhoneNumber, + derivedParentReportName, + isParentReportArchived, + ); const isMoneyRequestOrReport = isMoneyRequestReport(report) || isMoneyRequest(report) || isTrackExpenseReport(report) || isInvoiceReport(report); const ownerPersonalDetails = getPersonalDetailsForAccountIDs(report?.ownerAccountID ? [report.ownerAccountID] : [], personalDetails); const displayNamesWithTooltips = getDisplayNamesWithTooltips(Object.values(ownerPersonalDetails), false, localeCompare, formatPhoneNumber, translate); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 061db3d917ff..f67780239906 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7295,10 +7295,14 @@ function getPendingDeleteMemberAccountIDs(pendingChatMembers: PendingChatMember[ /** * Gets the parent navigation subtitle for the report + * + * @param invoiceReceiverPolicy - The policy of the business receiving the invoice, i.e. the policy whose ID + * `getInvoiceReceiverPolicyID` reads off the parent report. Only used for invoice reports and invoice rooms. */ function getParentNavigationSubtitle( report: OnyxEntry, policy: OnyxEntry, + invoiceReceiverPolicy: OnyxEntry, conciergeReportID: string | undefined, translate: LocalizedTranslate, formatPhoneNumber: LocaleContextProps['formatPhoneNumber'], @@ -7341,8 +7345,6 @@ function getParentNavigationSubtitle( if (isInvoiceReport(report) || isInvoiceRoom(parentReport)) { const senderWorkspaceName = getPolicyName({report: parentReport, policy, unavailableTranslation: translate('workspace.common.unavailable')}); - const invoiceReceiverPolicyID = getInvoiceReceiverPolicyID(parentReport); - const invoiceReceiverPolicy = invoiceReceiverPolicyID ? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${invoiceReceiverPolicyID}`] : undefined; const isCurrentUserReceiver = isCurrentUserInvoiceReceiver(parentReport); const invoiceReceiverPersonalDetail = getInvoiceReceiverPersonalDetail(parentReport, getAllPersonalDetails()); const invoicePayerName = getInvoicePayerName(parentReport, translate, formatPhoneNumber, invoiceReceiverPersonalDetail, invoiceReceiverPolicy); diff --git a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx index 45942fe1a3b5..53f07e7869ca 100644 --- a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx +++ b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx @@ -8,6 +8,7 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import useParentReportAction from '@hooks/useParentReportAction'; +import usePolicy from '@hooks/usePolicy'; import useReportIsArchived from '@hooks/useReportIsArchived'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -16,6 +17,7 @@ import Navigation from '@libs/Navigation/Navigation'; import { canEditReportTitle, getAvailableReportFields, + getInvoiceReceiverPolicyID, getParentNavigationSubtitle, getReportFieldKey, isInvoiceReport as isInvoiceReportUtil, @@ -73,7 +75,17 @@ function ReportDetailsTitleSectionContent({report}: ReportDetailsTitleSectionCon isMoneyRequest, isTrackExpenseReport: isTrackExpenseReportUtil(report, parentReport, parentReportAction), }); - const parentNavigationSubtitleData = getParentNavigationSubtitle(report, policy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived); + const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentReport)); + const parentNavigationSubtitleData = getParentNavigationSubtitle( + report, + policy, + invoiceReceiverPolicy, + conciergeReportID, + translate, + formatPhoneNumber, + derivedParentReportName, + isParentReportArchived, + ); const titleField: OnyxTypes.PolicyReportField | undefined = (() => { const fields = getAvailableReportFields(report, Object.values(policy?.fieldList ?? {})); diff --git a/src/pages/ShareCodePage.tsx b/src/pages/ShareCodePage.tsx index e1c1eda00954..8146ee1660dc 100644 --- a/src/pages/ShareCodePage.tsx +++ b/src/pages/ShareCodePage.tsx @@ -14,6 +14,7 @@ import useEnvironment from '@hooks/useEnvironment'; import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import usePolicy from '@hooks/usePolicy'; import {useDerivedReportNamesByReportIDs} from '@hooks/useReportAttributes'; import useReportIsArchived from '@hooks/useReportIsArchived'; import useStyleUtils from '@hooks/useStyleUtils'; @@ -30,6 +31,7 @@ import { getChatRoomSubtitle, getDefaultWorkspaceAvatar, getDisplayNameForParticipant, + getInvoiceReceiverPolicyID, getParentNavigationSubtitle, getParticipantsAccountIDsForDisplay, getPolicyName, @@ -113,6 +115,7 @@ function ShareCodePage({report, policy, backTo, shouldShowQRCodeOnly = false}: S const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`); + const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentReport)); const reportForTitle = useMemo(() => getReportForHeader(report, parentReport), [report, parentReport]); const derivedReportNames = useDerivedReportNamesByReportIDs([report?.parentReportID, reportForTitle?.reportID]); const derivedParentReportName = getReportNameFromNames(derivedReportNames, report?.parentReportID); @@ -135,8 +138,8 @@ function ShareCodePage({report, policy, backTo, shouldShowQRCodeOnly = false}: S } return ( - getParentNavigationSubtitle(report, policy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived).workspaceName ?? - getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, false, isReportArchived) + getParentNavigationSubtitle(report, policy, invoiceReceiverPolicy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived) + .workspaceName ?? getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, false, isReportArchived) ); } @@ -144,6 +147,7 @@ function ShareCodePage({report, policy, backTo, shouldShowQRCodeOnly = false}: S }, [ report, policy, + invoiceReceiverPolicy, currentUserPersonalDetails.login, isReport, isReportArchived, diff --git a/src/pages/inbox/HeaderView.tsx b/src/pages/inbox/HeaderView.tsx index a5b3595cf035..c91dd6d5da8c 100644 --- a/src/pages/inbox/HeaderView.tsx +++ b/src/pages/inbox/HeaderView.tsx @@ -50,6 +50,7 @@ import { chatIncludesChronos, getChatRoomSubtitle, getDisplayNamesWithTooltips, + getInvoiceReceiverPolicyID, getParentNavigationSubtitle, getParticipantsAccountIDsForDisplay, getPolicyDescriptionText, @@ -178,6 +179,8 @@ function HeaderView({onNavigationMenuButtonClicked, reportID}: HeaderViewProps) const reportHeaderData = (!isTaskReport && !isSupportTicketReport && !isChatThread && report?.parentReportID) || isParentInvoiceAndIsTransactionThread ? parentReport : report; const isParentOneTransactionThread = isOneTransactionThread(parentReport, grandParentReport, grandParentReportAction, undefined, grandParentReportActions); const parentNavigationReport = isParentOneTransactionThread ? parentReport : reportHeaderData; + const [parentNavigationParentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(parentNavigationReport?.parentReportID)}`); + const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentNavigationParentReport)); const derivedNames = useDerivedReportNamesByReportIDs([parentNavigationReport?.parentReportID, reportHeaderData?.reportID]); const derivedParentReportName = getReportNameFromNames(derivedNames, parentNavigationReport?.parentReportID); const derivedReportHeaderName = getReportNameFromNames(derivedNames, reportHeaderData?.reportID); @@ -199,6 +202,7 @@ function HeaderView({onNavigationMenuButtonClicked, reportID}: HeaderViewProps) const parentNavigationSubtitleData = getParentNavigationSubtitle( parentNavigationReport, policy, + invoiceReceiverPolicy, conciergeReportID, translate, formatPhoneNumber, diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 550153d198d6..2805e9f39ea9 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -3934,13 +3934,13 @@ describe('ReportUtils', () => { }); it('should return the correct parent navigation subtitle for the archived invoice report', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, translateLocal, formatPhoneNumber, undefined, true); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined, true); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual).toEqual({reportName: 'A workspace & Ragnar Lothbrok (archived)'}); }); it('should return the correct parent navigation subtitle for the non archived invoice report', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, translateLocal, formatPhoneNumber, undefined, false); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined, false); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual).toEqual({reportName: 'A workspace & Ragnar Lothbrok'}); }); @@ -3959,7 +3959,7 @@ describe('ReportUtils', () => { role: CONST.POLICY.ROLE.ADMIN, }); - const actual = getParentNavigationSubtitle(expenseReport, testPolicy, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle(expenseReport, testPolicy, undefined, undefined, translateLocal, formatPhoneNumber, undefined); expect(actual.workspaceName).toBe('Direct Policy Name'); }); @@ -3987,14 +3987,14 @@ describe('ReportUtils', () => { }; return Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}200`, parentInvoiceRoom).then(() => { - const actual = getParentNavigationSubtitle(invoiceReport, testPolicy, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle(invoiceReport, testPolicy, undefined, undefined, translateLocal, formatPhoneNumber, undefined); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual.reportName).toContain('Invoice Policy'); }); }); it('should fall back to allPolicies when policy parameter is undefined', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; // Should still resolve via Onyx-connected allPolicies or report.policyName expect(normalizedActual.reportName).toContain('A workspace'); @@ -4007,7 +4007,7 @@ describe('ReportUtils', () => { reportName: 'Chat Report', type: CONST.REPORT.TYPE.CHAT, }; - const actual = getParentNavigationSubtitle(chatReport, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle(chatReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); expect(actual).toEqual({}); }); @@ -4034,13 +4034,13 @@ describe('ReportUtils', () => { }) .then(waitForBatchedUpdates) .then(() => { - const actual = getParentNavigationSubtitle(childReport, undefined, conciergeReportID, translateLocal, formatPhoneNumber, 'Concierge'); + const actual = getParentNavigationSubtitle(childReport, undefined, undefined, conciergeReportID, translateLocal, formatPhoneNumber, 'Concierge'); expect(actual.reportName).toBe('Concierge'); }); }); it('should return reportName and workspaceName when parent report exists and conciergeReportID is undefined', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); expect(actual).toHaveProperty('reportName'); }); @@ -4052,9 +4052,75 @@ describe('ReportUtils', () => { const expenseReport = {reportID: '780051', type: CONST.REPORT.TYPE.EXPENSE, ownerAccountID: hiddenOwnerAccountID}; const translateWithHiddenMarker: LocalizedTranslate = (path, ...parameters) => (path === 'common.hidden' ? 'HiddenMarker' : translateLocal(path, ...parameters)); - const actual = getParentNavigationSubtitle(expenseReport, undefined, undefined, translateWithHiddenMarker, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle(expenseReport, undefined, undefined, undefined, translateWithHiddenMarker, formatPhoneNumber, undefined); expect(actual.reportName).toContain('HiddenMarker'); }); + + /** + * The invoice receiver's policy must come from the caller rather than the module-level `allPolicies` + * Onyx cache that ReportUtils used to read. For a business receiver the payer name is just that + * policy's name, so a stale cached policy would render the wrong workspace in the subtitle. + * See https://github.com/Expensify/App/issues/66415. + */ + describe('reads the invoice receiver policy from the caller, not the Onyx cache', () => { + const RECEIVER_POLICY_ID = 'invoice-receiver-policy'; + const invoiceRoomID = '990001'; + const invoiceReportID = '990002'; + + const invoiceRoom: Report = { + reportID: invoiceRoomID, + reportName: 'Invoice Room', + policyName: 'Sender Workspace', + invoiceReceiver: {type: CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS, policyID: RECEIVER_POLICY_ID}, + }; + + const invoiceReport: Report = { + reportID: invoiceReportID, + parentReportID: invoiceRoomID, + parentReportActionID: '1', + reportName: 'Invoice', + type: CONST.REPORT.TYPE.INVOICE, + }; + + beforeEach(async () => { + // The cached policy carries a name that must never reach the subtitle. + await Promise.all([ + Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${invoiceRoomID}`, invoiceRoom), + Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${invoiceReportID}`, invoiceReport), + Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${RECEIVER_POLICY_ID}`, createMock({id: RECEIVER_POLICY_ID, name: 'Cached Receiver Name'})), + ]); + return waitForBatchedUpdates(); + }); + + afterEach(async () => { + await Promise.all([ + Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${invoiceRoomID}`, null), + Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${invoiceReportID}`, null), + Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${RECEIVER_POLICY_ID}`, null), + ]); + return waitForBatchedUpdates(); + }); + + it('renders the payer name from the passed receiver policy, not the cached one', () => { + // Given an invoice whose business receiver resolves to a policy that is also sitting in the Onyx cache + const passedReceiverPolicy = createMock({id: RECEIVER_POLICY_ID, name: 'Passed Receiver Name'}); + + // When the caller passes the receiver policy it actually holds + const actual = getParentNavigationSubtitle(invoiceReport, undefined, passedReceiverPolicy, undefined, translateLocal, formatPhoneNumber, undefined); + + // Then the subtitle names the passed workspace, so a stale cache can never mislabel the payer + expect(actual.reportName?.replaceAll('\u00A0', ' ')).toBe('Sender Workspace & Passed Receiver Name'); + }); + + it('falls back to the unavailable label when the caller has no receiver policy', () => { + // Given the same invoice, but a caller that has not loaded the receiver policy yet + // When the subtitle is built with no receiver policy + const actual = getParentNavigationSubtitle(invoiceReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + + // Then the cached policy name is still not used, because the lookup no longer reads the cache + expect(actual.reportName).not.toContain('Cached Receiver Name'); + }); + }); }); describe('getWhisperDisplayNames', () => { From 6b50c7970351c5f0f5b5c2c70313fbf91cecaf37 Mon Sep 17 00:00:00 2001 From: daledah Date: Wed, 21 Oct 2026 01:42:59 +0700 Subject: [PATCH 2/4] address comment --- tests/unit/ReportUtilsTest.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 2805e9f39ea9..fd9ed5d1b37b 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -4112,13 +4112,15 @@ describe('ReportUtils', () => { expect(actual.reportName?.replaceAll('\u00A0', ' ')).toBe('Sender Workspace & Passed Receiver Name'); }); - it('falls back to the unavailable label when the caller has no receiver policy', () => { + it('never falls back to the cached policy when the caller has no receiver policy', () => { // Given the same invoice, but a caller that has not loaded the receiver policy yet // When the subtitle is built with no receiver policy const actual = getParentNavigationSubtitle(invoiceReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); - // Then the cached policy name is still not used, because the lookup no longer reads the cache - expect(actual.reportName).not.toContain('Cached Receiver Name'); + // Then the payer falls back to the invoice room's own `policyName`, which is how `getInvoicePayerName` + // has always resolved a business receiver without a policy. What matters here is that the cached + // 'Cached Receiver Name' never appears: the subtitle is built purely from what the caller passed in. + expect(actual.reportName?.replaceAll('\u00A0', ' ')).toBe('Sender Workspace & Sender Workspace'); }); }); }); From a9a32592659f8a76850933510bf601b05180e411 Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 27 Oct 2026 11:10:02 +0700 Subject: [PATCH 3/4] address comments --- src/components/AvatarWithDisplayName.tsx | 14 +- src/libs/ReportUtils.ts | 44 ++++-- .../ReportDetailsTitleSection.tsx | 7 +- src/pages/ShareCodePage.tsx | 11 +- src/pages/inbox/HeaderView.tsx | 17 +-- tests/unit/ReportUtilsTest.ts | 131 +++++++++++++++--- 6 files changed, 170 insertions(+), 54 deletions(-) diff --git a/src/components/AvatarWithDisplayName.tsx b/src/components/AvatarWithDisplayName.tsx index c6f0c8b3c551..c982994f66d3 100644 --- a/src/components/AvatarWithDisplayName.tsx +++ b/src/components/AvatarWithDisplayName.tsx @@ -11,6 +11,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; import Navigation from '@libs/Navigation/Navigation'; import {getPersonalDetailsForAccountIDs} from '@libs/PersonalDetailsUtils'; @@ -211,12 +212,14 @@ function AvatarWithDisplayName({ const isInSidePanel = useIsInSidePanel(); const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`); - const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`); + const [invoiceReceiverPolicyID] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.parentReportID)}`, { + selector: getInvoiceReceiverPolicyID, + }); const [personalDetails] = useAllPersonalDetails() ?? CONST.EMPTY_OBJECT; const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const policy = usePolicy(report?.policyID); - const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentReport)); + const invoiceReceiverPolicy = usePolicy(invoiceReceiverPolicyID); const theme = useTheme(); const styles = useThemeStyles(); // We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to display the edit button only on large screens @@ -252,16 +255,15 @@ function AvatarWithDisplayName({ const derivedParentReportName = getReportNameFromNames(derivedReportNames, report?.parentReportID); const title = getReportName(report, derivedReportName); const subtitle = getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, true, isReportArchived); - const parentNavigationSubtitleData = getParentNavigationSubtitle( + const parentNavigationSubtitleData = getParentNavigationSubtitle({ report, policy, invoiceReceiverPolicy, - conciergeReportID, translate, formatPhoneNumber, - derivedParentReportName, + parentReportName: derivedParentReportName, isParentReportArchived, - ); + }); const isMoneyRequestOrReport = isMoneyRequestReport(report) || isMoneyRequest(report) || isTrackExpenseReport(report) || isInvoiceReport(report); const ownerPersonalDetails = getPersonalDetailsForAccountIDs(report?.ownerAccountID ? [report.ownerAccountID] : [], personalDetails); const displayNamesWithTooltips = getDisplayNamesWithTooltips(Object.values(ownerPersonalDetails), false, localeCompare, formatPhoneNumber, translate); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 143069245723..fff9d7b8afa8 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7229,22 +7229,42 @@ function getPendingDeleteMemberAccountIDs(pendingChatMembers: PendingChatMember[ return pendingChatMembers.filter((member) => member.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).map((member) => member.accountID); } +type GetParentNavigationSubtitleParams = { + /** The report whose parent is being described */ + report: OnyxEntry; + + /** The workspace policy of `report`, used to name the workspace its parent chat belongs to */ + policy: OnyxEntry; + + /** + * The policy of the business receiving the invoice, i.e. the policy whose ID `getInvoiceReceiverPolicyID` + * reads off the parent report. Only used for invoice reports and invoice rooms. + */ + invoiceReceiverPolicy: OnyxEntry; + + translate: LocalizedTranslate; + + formatPhoneNumber: LocaleContextProps['formatPhoneNumber']; + + /** Pre-resolved name of the parent report, so this function does not have to derive it */ + parentReportName: string | undefined; + + /** Whether the parent report is archived */ + isParentReportArchived?: boolean; +}; + /** * Gets the parent navigation subtitle for the report - * - * @param invoiceReceiverPolicy - The policy of the business receiving the invoice, i.e. the policy whose ID - * `getInvoiceReceiverPolicyID` reads off the parent report. Only used for invoice reports and invoice rooms. */ -function getParentNavigationSubtitle( - report: OnyxEntry, - policy: OnyxEntry, - invoiceReceiverPolicy: OnyxEntry, - conciergeReportID: string | undefined, - translate: LocalizedTranslate, - formatPhoneNumber: LocaleContextProps['formatPhoneNumber'], - parentReportName: string | undefined, +function getParentNavigationSubtitle({ + report, + policy, + invoiceReceiverPolicy, + translate, + formatPhoneNumber, + parentReportName, isParentReportArchived = false, -): ParentNavigationSummaryParams { +}: GetParentNavigationSubtitleParams): ParentNavigationSummaryParams { const parentReport = getParentReport(report); if (isEmptyObject(parentReport)) { diff --git a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx index 53f07e7869ca..8122cb38c4d0 100644 --- a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx +++ b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx @@ -76,16 +76,15 @@ function ReportDetailsTitleSectionContent({report}: ReportDetailsTitleSectionCon isTrackExpenseReport: isTrackExpenseReportUtil(report, parentReport, parentReportAction), }); const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentReport)); - const parentNavigationSubtitleData = getParentNavigationSubtitle( + const parentNavigationSubtitleData = getParentNavigationSubtitle({ report, policy, invoiceReceiverPolicy, - conciergeReportID, translate, formatPhoneNumber, - derivedParentReportName, + parentReportName: derivedParentReportName, isParentReportArchived, - ); + }); const titleField: OnyxTypes.PolicyReportField | undefined = (() => { const fields = getAvailableReportFields(report, Object.values(policy?.fieldList ?? {})); diff --git a/src/pages/ShareCodePage.tsx b/src/pages/ShareCodePage.tsx index 9b008d153be7..4e4ee8cad111 100644 --- a/src/pages/ShareCodePage.tsx +++ b/src/pages/ShareCodePage.tsx @@ -145,8 +145,15 @@ function ShareCodePage({report, policy, backTo, shouldShowQRCodeOnly = false}: S .join(' & '); } else { subtitle = - getParentNavigationSubtitle(report, policy, invoiceReceiverPolicy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived) - .workspaceName ?? getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, false, isReportArchived); + getParentNavigationSubtitle({ + report, + policy, + invoiceReceiverPolicy, + translate, + formatPhoneNumber, + parentReportName: derivedParentReportName, + isParentReportArchived, + }).workspaceName ?? getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, false, isReportArchived); } } else { subtitle = currentUserPersonalDetails.login; diff --git a/src/pages/inbox/HeaderView.tsx b/src/pages/inbox/HeaderView.tsx index 182874d61997..cc05f411e289 100644 --- a/src/pages/inbox/HeaderView.tsx +++ b/src/pages/inbox/HeaderView.tsx @@ -179,8 +179,10 @@ function HeaderView({onNavigationMenuButtonClicked, reportID}: HeaderViewProps) const reportHeaderData = (!isTaskReport && !isSupportTicketReport && !isChatThread && report?.parentReportID) || isParentInvoiceAndIsTransactionThread ? parentReport : report; const isParentOneTransactionThread = isOneTransactionThread(parentReport, grandParentReport, grandParentReportAction, undefined, grandParentReportActions); const parentNavigationReport = isParentOneTransactionThread ? parentReport : reportHeaderData; - const [parentNavigationParentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(parentNavigationReport?.parentReportID)}`); - const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentNavigationParentReport)); + const [invoiceReceiverPolicyID] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(parentNavigationReport?.parentReportID)}`, { + selector: getInvoiceReceiverPolicyID, + }); + const invoiceReceiverPolicy = usePolicy(invoiceReceiverPolicyID); const derivedNames = useDerivedReportNamesByReportIDs([parentNavigationReport?.parentReportID, reportHeaderData?.reportID]); const derivedParentReportName = getReportNameFromNames(derivedNames, parentNavigationReport?.parentReportID); const derivedReportHeaderName = getReportNameFromNames(derivedNames, reportHeaderData?.reportID); @@ -199,16 +201,15 @@ function HeaderView({onNavigationMenuButtonClicked, reportID}: HeaderViewProps) ? getReportStatusTooltipTranslation({stateNum: reportHeaderData?.stateNum, statusNum: reportHeaderData?.statusNum, translate}) : undefined; const isParentReportHeaderDataArchived = useReportIsArchived(reportHeaderData?.parentReportID); - const parentNavigationSubtitleData = getParentNavigationSubtitle( - parentNavigationReport, + const parentNavigationSubtitleData = getParentNavigationSubtitle({ + report: parentNavigationReport, policy, invoiceReceiverPolicy, - conciergeReportID, translate, formatPhoneNumber, - derivedParentReportName, - isParentReportHeaderDataArchived, - ); + parentReportName: derivedParentReportName, + isParentReportArchived: isParentReportHeaderDataArchived, + }); const humanAgentAccountID = getHumanAgentAccountIDFromReportAction(parentReportAction); const humanAgentName = getHumanAgentFirstName(parentReportAction, personalDetails); const reportDescription = StringUtils.lineBreaksToSpaces(Parser.htmlToText(getReportDescription(report))); diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 5712cbe877c9..71a06f9adbac 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -4010,13 +4010,29 @@ describe('ReportUtils', () => { }); it('should return the correct parent navigation subtitle for the archived invoice report', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined, true); + const actual = getParentNavigationSubtitle({ + report: baseArchivedPolicyExpenseChat, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + isParentReportArchived: true, + }); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual).toEqual({reportName: 'A workspace & Ragnar Lothbrok (archived)'}); }); it('should return the correct parent navigation subtitle for the non archived invoice report', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined, false); + const actual = getParentNavigationSubtitle({ + report: baseArchivedPolicyExpenseChat, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + isParentReportArchived: false, + }); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual).toEqual({reportName: 'A workspace & Ragnar Lothbrok'}); }); @@ -4035,7 +4051,14 @@ describe('ReportUtils', () => { role: CONST.POLICY.ROLE.ADMIN, }); - const actual = getParentNavigationSubtitle(expenseReport, testPolicy, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: expenseReport, + policy: testPolicy, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); expect(actual.workspaceName).toBe('Direct Policy Name'); }); @@ -4063,14 +4086,28 @@ describe('ReportUtils', () => { }; return Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}200`, parentInvoiceRoom).then(() => { - const actual = getParentNavigationSubtitle(invoiceReport, testPolicy, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: invoiceReport, + policy: testPolicy, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual.reportName).toContain('Invoice Policy'); }); }); it('should fall back to allPolicies when policy parameter is undefined', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: baseArchivedPolicyExpenseChat, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; // Should still resolve via Onyx-connected allPolicies or report.policyName expect(normalizedActual.reportName).toContain('A workspace'); @@ -4083,14 +4120,22 @@ describe('ReportUtils', () => { reportName: 'Chat Report', type: CONST.REPORT.TYPE.CHAT, }; - const actual = getParentNavigationSubtitle(chatReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: chatReport, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); expect(actual).toEqual({}); }); - it('should pass conciergeReportID through to getReportName for parent report name resolution', () => { - const conciergeReportID = '999'; + it('should use the parent report name the caller resolved rather than deriving it again', () => { + // Given a chat thread hanging off the Concierge chat + const parentReportID = '999'; const conciergeParentReport: Report = { - reportID: conciergeReportID, + reportID: parentReportID, lastReadTime: '2024-02-01 04:56:47.233', reportName: 'Concierge', type: CONST.REPORT.TYPE.CHAT, @@ -4098,7 +4143,7 @@ describe('ReportUtils', () => { const childReport: Report = { reportID: '1000', lastReadTime: '2024-02-01 04:56:47.233', - parentReportID: conciergeReportID, + parentReportID, parentReportActionID: '1', reportName: 'Child Thread', type: CONST.REPORT.TYPE.CHAT, @@ -4110,13 +4155,30 @@ describe('ReportUtils', () => { }) .then(waitForBatchedUpdates) .then(() => { - const actual = getParentNavigationSubtitle(childReport, undefined, undefined, conciergeReportID, translateLocal, formatPhoneNumber, 'Concierge'); + // When the caller hands in the parent report name it already resolved + const actual = getParentNavigationSubtitle({ + report: childReport, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: 'Concierge', + }); + + // Then that name is what the subtitle shows, so the function never has to look the parent up itself expect(actual.reportName).toBe('Concierge'); }); }); - it('should return reportName and workspaceName when parent report exists and conciergeReportID is undefined', () => { - const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + it('should return reportName and workspaceName when the parent report exists', () => { + const actual = getParentNavigationSubtitle({ + report: baseArchivedPolicyExpenseChat, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); expect(actual).toHaveProperty('reportName'); }); @@ -4128,14 +4190,22 @@ describe('ReportUtils', () => { const expenseReport = {reportID: '780051', type: CONST.REPORT.TYPE.EXPENSE, ownerAccountID: hiddenOwnerAccountID}; const translateWithHiddenMarker: LocalizedTranslate = (path, ...parameters) => (path === 'common.hidden' ? 'HiddenMarker' : translateLocal(path, ...parameters)); - const actual = getParentNavigationSubtitle(expenseReport, undefined, undefined, undefined, translateWithHiddenMarker, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: expenseReport, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateWithHiddenMarker, + formatPhoneNumber, + parentReportName: undefined, + }); expect(actual.reportName).toContain('HiddenMarker'); }); /** * The invoice receiver's policy must come from the caller rather than the module-level `allPolicies` * Onyx cache that ReportUtils used to read. For a business receiver the payer name is just that - * policy's name, so a stale cached policy would render the wrong workspace in the subtitle. + * policy's name, so these tests pin the payer to whatever the caller hands in and prove the cache + * is never consulted, whatever it happens to hold. * See https://github.com/Expensify/App/issues/66415. */ describe('reads the invoice receiver policy from the caller, not the Onyx cache', () => { @@ -4182,21 +4252,38 @@ describe('ReportUtils', () => { const passedReceiverPolicy = createMock({id: RECEIVER_POLICY_ID, name: 'Passed Receiver Name'}); // When the caller passes the receiver policy it actually holds - const actual = getParentNavigationSubtitle(invoiceReport, undefined, passedReceiverPolicy, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: invoiceReport, + policy: undefined, + invoiceReceiverPolicy: passedReceiverPolicy, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); - // Then the subtitle names the passed workspace, so a stale cache can never mislabel the payer + // Then the subtitle names the passed workspace, proving the payer is read off the argument and + // not off the policy that `RECEIVER_POLICY_ID` resolves to in Onyx expect(actual.reportName?.replaceAll('\u00A0', ' ')).toBe('Sender Workspace & Passed Receiver Name'); }); it('never falls back to the cached policy when the caller has no receiver policy', () => { // Given the same invoice, but a caller that has not loaded the receiver policy yet // When the subtitle is built with no receiver policy - const actual = getParentNavigationSubtitle(invoiceReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); + const actual = getParentNavigationSubtitle({ + report: invoiceReport, + policy: undefined, + invoiceReceiverPolicy: undefined, + translate: translateLocal, + formatPhoneNumber, + parentReportName: undefined, + }); - // Then the payer falls back to the invoice room's own `policyName`, which is how `getInvoicePayerName` - // has always resolved a business receiver without a policy. What matters here is that the cached - // 'Cached Receiver Name' never appears: the subtitle is built purely from what the caller passed in. - expect(actual.reportName?.replaceAll('\u00A0', ' ')).toBe('Sender Workspace & Sender Workspace'); + // Then the cached name never reaches the subtitle. Without a receiver policy `getInvoicePayerName` + // falls back to the invoice room's own `policyName`, exactly as it did before this refactor, so the + // assertion states that intent rather than pinning the full string and breaking if the fallback improves. + const reportName = actual.reportName?.replaceAll('\u00A0', ' '); + expect(reportName).not.toContain('Cached Receiver Name'); + expect(reportName).toContain('Sender Workspace'); }); }); }); From a55869f6443b1e787baa1699f8246337316c2e2f Mon Sep 17 00:00:00 2001 From: daledah Date: Tue, 27 Oct 2026 11:14:10 +0700 Subject: [PATCH 4/4] address comments --- src/components/AvatarWithDisplayName.tsx | 7 +- src/libs/ReportUtils.ts | 44 ++----- .../ReportDetailsTitleSection.tsx | 7 +- src/pages/ShareCodePage.tsx | 11 +- src/pages/inbox/HeaderView.tsx | 11 +- tests/unit/ReportUtilsTest.ts | 115 +++--------------- 6 files changed, 44 insertions(+), 151 deletions(-) diff --git a/src/components/AvatarWithDisplayName.tsx b/src/components/AvatarWithDisplayName.tsx index c982994f66d3..bac91876f5c8 100644 --- a/src/components/AvatarWithDisplayName.tsx +++ b/src/components/AvatarWithDisplayName.tsx @@ -255,15 +255,16 @@ function AvatarWithDisplayName({ const derivedParentReportName = getReportNameFromNames(derivedReportNames, report?.parentReportID); const title = getReportName(report, derivedReportName); const subtitle = getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, true, isReportArchived); - const parentNavigationSubtitleData = getParentNavigationSubtitle({ + const parentNavigationSubtitleData = getParentNavigationSubtitle( report, policy, invoiceReceiverPolicy, + conciergeReportID, translate, formatPhoneNumber, - parentReportName: derivedParentReportName, + derivedParentReportName, isParentReportArchived, - }); + ); const isMoneyRequestOrReport = isMoneyRequestReport(report) || isMoneyRequest(report) || isTrackExpenseReport(report) || isInvoiceReport(report); const ownerPersonalDetails = getPersonalDetailsForAccountIDs(report?.ownerAccountID ? [report.ownerAccountID] : [], personalDetails); const displayNamesWithTooltips = getDisplayNamesWithTooltips(Object.values(ownerPersonalDetails), false, localeCompare, formatPhoneNumber, translate); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index fff9d7b8afa8..143069245723 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7229,42 +7229,22 @@ function getPendingDeleteMemberAccountIDs(pendingChatMembers: PendingChatMember[ return pendingChatMembers.filter((member) => member.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE).map((member) => member.accountID); } -type GetParentNavigationSubtitleParams = { - /** The report whose parent is being described */ - report: OnyxEntry; - - /** The workspace policy of `report`, used to name the workspace its parent chat belongs to */ - policy: OnyxEntry; - - /** - * The policy of the business receiving the invoice, i.e. the policy whose ID `getInvoiceReceiverPolicyID` - * reads off the parent report. Only used for invoice reports and invoice rooms. - */ - invoiceReceiverPolicy: OnyxEntry; - - translate: LocalizedTranslate; - - formatPhoneNumber: LocaleContextProps['formatPhoneNumber']; - - /** Pre-resolved name of the parent report, so this function does not have to derive it */ - parentReportName: string | undefined; - - /** Whether the parent report is archived */ - isParentReportArchived?: boolean; -}; - /** * Gets the parent navigation subtitle for the report + * + * @param invoiceReceiverPolicy - The policy of the business receiving the invoice, i.e. the policy whose ID + * `getInvoiceReceiverPolicyID` reads off the parent report. Only used for invoice reports and invoice rooms. */ -function getParentNavigationSubtitle({ - report, - policy, - invoiceReceiverPolicy, - translate, - formatPhoneNumber, - parentReportName, +function getParentNavigationSubtitle( + report: OnyxEntry, + policy: OnyxEntry, + invoiceReceiverPolicy: OnyxEntry, + conciergeReportID: string | undefined, + translate: LocalizedTranslate, + formatPhoneNumber: LocaleContextProps['formatPhoneNumber'], + parentReportName: string | undefined, isParentReportArchived = false, -}: GetParentNavigationSubtitleParams): ParentNavigationSummaryParams { +): ParentNavigationSummaryParams { const parentReport = getParentReport(report); if (isEmptyObject(parentReport)) { diff --git a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx index 8122cb38c4d0..53f07e7869ca 100644 --- a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx +++ b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx @@ -76,15 +76,16 @@ function ReportDetailsTitleSectionContent({report}: ReportDetailsTitleSectionCon isTrackExpenseReport: isTrackExpenseReportUtil(report, parentReport, parentReportAction), }); const invoiceReceiverPolicy = usePolicy(getInvoiceReceiverPolicyID(parentReport)); - const parentNavigationSubtitleData = getParentNavigationSubtitle({ + const parentNavigationSubtitleData = getParentNavigationSubtitle( report, policy, invoiceReceiverPolicy, + conciergeReportID, translate, formatPhoneNumber, - parentReportName: derivedParentReportName, + derivedParentReportName, isParentReportArchived, - }); + ); const titleField: OnyxTypes.PolicyReportField | undefined = (() => { const fields = getAvailableReportFields(report, Object.values(policy?.fieldList ?? {})); diff --git a/src/pages/ShareCodePage.tsx b/src/pages/ShareCodePage.tsx index 4e4ee8cad111..9b008d153be7 100644 --- a/src/pages/ShareCodePage.tsx +++ b/src/pages/ShareCodePage.tsx @@ -145,15 +145,8 @@ function ShareCodePage({report, policy, backTo, shouldShowQRCodeOnly = false}: S .join(' & '); } else { subtitle = - getParentNavigationSubtitle({ - report, - policy, - invoiceReceiverPolicy, - translate, - formatPhoneNumber, - parentReportName: derivedParentReportName, - isParentReportArchived, - }).workspaceName ?? getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, false, isReportArchived); + getParentNavigationSubtitle(report, policy, invoiceReceiverPolicy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived) + .workspaceName ?? getChatRoomSubtitle(report, policy, conciergeReportID, translate, rules, false, isReportArchived); } } else { subtitle = currentUserPersonalDetails.login; diff --git a/src/pages/inbox/HeaderView.tsx b/src/pages/inbox/HeaderView.tsx index cc05f411e289..34de7d142afa 100644 --- a/src/pages/inbox/HeaderView.tsx +++ b/src/pages/inbox/HeaderView.tsx @@ -201,15 +201,16 @@ function HeaderView({onNavigationMenuButtonClicked, reportID}: HeaderViewProps) ? getReportStatusTooltipTranslation({stateNum: reportHeaderData?.stateNum, statusNum: reportHeaderData?.statusNum, translate}) : undefined; const isParentReportHeaderDataArchived = useReportIsArchived(reportHeaderData?.parentReportID); - const parentNavigationSubtitleData = getParentNavigationSubtitle({ - report: parentNavigationReport, + const parentNavigationSubtitleData = getParentNavigationSubtitle( + parentNavigationReport, policy, invoiceReceiverPolicy, + conciergeReportID, translate, formatPhoneNumber, - parentReportName: derivedParentReportName, - isParentReportArchived: isParentReportHeaderDataArchived, - }); + derivedParentReportName, + isParentReportHeaderDataArchived, + ); const humanAgentAccountID = getHumanAgentAccountIDFromReportAction(parentReportAction); const humanAgentName = getHumanAgentFirstName(parentReportAction, personalDetails); const reportDescription = StringUtils.lineBreaksToSpaces(Parser.htmlToText(getReportDescription(report))); diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 71a06f9adbac..47b8a5c75cb6 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -4010,29 +4010,13 @@ describe('ReportUtils', () => { }); it('should return the correct parent navigation subtitle for the archived invoice report', () => { - const actual = getParentNavigationSubtitle({ - report: baseArchivedPolicyExpenseChat, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - isParentReportArchived: true, - }); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined, true); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual).toEqual({reportName: 'A workspace & Ragnar Lothbrok (archived)'}); }); it('should return the correct parent navigation subtitle for the non archived invoice report', () => { - const actual = getParentNavigationSubtitle({ - report: baseArchivedPolicyExpenseChat, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - isParentReportArchived: false, - }); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined, false); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual).toEqual({reportName: 'A workspace & Ragnar Lothbrok'}); }); @@ -4051,14 +4035,7 @@ describe('ReportUtils', () => { role: CONST.POLICY.ROLE.ADMIN, }); - const actual = getParentNavigationSubtitle({ - report: expenseReport, - policy: testPolicy, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(expenseReport, testPolicy, undefined, undefined, translateLocal, formatPhoneNumber, undefined); expect(actual.workspaceName).toBe('Direct Policy Name'); }); @@ -4086,28 +4063,14 @@ describe('ReportUtils', () => { }; return Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}200`, parentInvoiceRoom).then(() => { - const actual = getParentNavigationSubtitle({ - report: invoiceReport, - policy: testPolicy, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(invoiceReport, testPolicy, undefined, undefined, translateLocal, formatPhoneNumber, undefined); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; expect(normalizedActual.reportName).toContain('Invoice Policy'); }); }); it('should fall back to allPolicies when policy parameter is undefined', () => { - const actual = getParentNavigationSubtitle({ - report: baseArchivedPolicyExpenseChat, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); const normalizedActual = {...actual, reportName: actual.reportName?.replaceAll('\u00A0', ' ')}; // Should still resolve via Onyx-connected allPolicies or report.policyName expect(normalizedActual.reportName).toContain('A workspace'); @@ -4120,22 +4083,14 @@ describe('ReportUtils', () => { reportName: 'Chat Report', type: CONST.REPORT.TYPE.CHAT, }; - const actual = getParentNavigationSubtitle({ - report: chatReport, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(chatReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); expect(actual).toEqual({}); }); - it('should use the parent report name the caller resolved rather than deriving it again', () => { - // Given a chat thread hanging off the Concierge chat - const parentReportID = '999'; + it('should pass conciergeReportID through to getReportName for parent report name resolution', () => { + const conciergeReportID = '999'; const conciergeParentReport: Report = { - reportID: parentReportID, + reportID: conciergeReportID, lastReadTime: '2024-02-01 04:56:47.233', reportName: 'Concierge', type: CONST.REPORT.TYPE.CHAT, @@ -4143,7 +4098,7 @@ describe('ReportUtils', () => { const childReport: Report = { reportID: '1000', lastReadTime: '2024-02-01 04:56:47.233', - parentReportID, + parentReportID: conciergeReportID, parentReportActionID: '1', reportName: 'Child Thread', type: CONST.REPORT.TYPE.CHAT, @@ -4155,30 +4110,13 @@ describe('ReportUtils', () => { }) .then(waitForBatchedUpdates) .then(() => { - // When the caller hands in the parent report name it already resolved - const actual = getParentNavigationSubtitle({ - report: childReport, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: 'Concierge', - }); - - // Then that name is what the subtitle shows, so the function never has to look the parent up itself + const actual = getParentNavigationSubtitle(childReport, undefined, undefined, conciergeReportID, translateLocal, formatPhoneNumber, 'Concierge'); expect(actual.reportName).toBe('Concierge'); }); }); - it('should return reportName and workspaceName when the parent report exists', () => { - const actual = getParentNavigationSubtitle({ - report: baseArchivedPolicyExpenseChat, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + it('should return reportName and workspaceName when parent report exists and conciergeReportID is undefined', () => { + const actual = getParentNavigationSubtitle(baseArchivedPolicyExpenseChat, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); expect(actual).toHaveProperty('reportName'); }); @@ -4190,14 +4128,7 @@ describe('ReportUtils', () => { const expenseReport = {reportID: '780051', type: CONST.REPORT.TYPE.EXPENSE, ownerAccountID: hiddenOwnerAccountID}; const translateWithHiddenMarker: LocalizedTranslate = (path, ...parameters) => (path === 'common.hidden' ? 'HiddenMarker' : translateLocal(path, ...parameters)); - const actual = getParentNavigationSubtitle({ - report: expenseReport, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateWithHiddenMarker, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(expenseReport, undefined, undefined, undefined, translateWithHiddenMarker, formatPhoneNumber, undefined); expect(actual.reportName).toContain('HiddenMarker'); }); @@ -4252,14 +4183,7 @@ describe('ReportUtils', () => { const passedReceiverPolicy = createMock({id: RECEIVER_POLICY_ID, name: 'Passed Receiver Name'}); // When the caller passes the receiver policy it actually holds - const actual = getParentNavigationSubtitle({ - report: invoiceReport, - policy: undefined, - invoiceReceiverPolicy: passedReceiverPolicy, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(invoiceReport, undefined, passedReceiverPolicy, undefined, translateLocal, formatPhoneNumber, undefined); // Then the subtitle names the passed workspace, proving the payer is read off the argument and // not off the policy that `RECEIVER_POLICY_ID` resolves to in Onyx @@ -4269,14 +4193,7 @@ describe('ReportUtils', () => { it('never falls back to the cached policy when the caller has no receiver policy', () => { // Given the same invoice, but a caller that has not loaded the receiver policy yet // When the subtitle is built with no receiver policy - const actual = getParentNavigationSubtitle({ - report: invoiceReport, - policy: undefined, - invoiceReceiverPolicy: undefined, - translate: translateLocal, - formatPhoneNumber, - parentReportName: undefined, - }); + const actual = getParentNavigationSubtitle(invoiceReport, undefined, undefined, undefined, translateLocal, formatPhoneNumber, undefined); // Then the cached name never reaches the subtitle. Without a receiver policy `getInvoicePayerName` // falls back to the invoice room's own `policyName`, exactly as it did before this refactor, so the