diff --git a/src/components/AvatarWithDisplayName.tsx b/src/components/AvatarWithDisplayName.tsx index 80d28f64cc33..bac91876f5c8 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'; @@ -22,6 +23,7 @@ import { canEditReportTitle, getChatRoomSubtitle, getDisplayNamesWithTooltips, + getInvoiceReceiverPolicyID, getParentNavigationSubtitle, getReportStatusColorStyle, getReportStatusTooltipTranslation, @@ -210,10 +212,14 @@ function AvatarWithDisplayName({ const isInSidePanel = useIsInSidePanel(); const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${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(invoiceReceiverPolicyID); const theme = useTheme(); const styles = useThemeStyles(); // We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to display the edit button only on large screens @@ -249,7 +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(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 dd12bfd49699..143069245723 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7231,10 +7231,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'], @@ -7277,8 +7281,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 44b422f97263..9b008d153be7 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 = getReportForHeader(report, parentReport); const derivedReportNames = useDerivedReportNamesByReportIDs([report?.parentReportID, reportForTitle?.reportID]); const derivedParentReportName = getReportNameFromNames(derivedReportNames, report?.parentReportID); @@ -142,8 +145,8 @@ function ShareCodePage({report, policy, backTo, shouldShowQRCodeOnly = false}: S .join(' & '); } else { subtitle = - 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); } } else { subtitle = currentUserPersonalDetails.login; diff --git a/src/pages/inbox/HeaderView.tsx b/src/pages/inbox/HeaderView.tsx index aba31bda77b9..34de7d142afa 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,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 [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,6 +204,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 43f7c1f81f1e..47b8a5c75cb6 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -4010,13 +4010,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'}); }); @@ -4035,7 +4035,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'); }); @@ -4063,14 +4063,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'); @@ -4083,7 +4083,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({}); }); @@ -4110,13 +4110,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'); }); @@ -4128,9 +4128,81 @@ 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 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', () => { + 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, 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); + + // 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'); + }); + }); }); describe('getWhisperDisplayNames', () => {