Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/components/AvatarWithDisplayName.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -22,6 +23,7 @@ import {
canEditReportTitle,
getChatRoomSubtitle,
getDisplayNamesWithTooltips,
getInvoiceReceiverPolicyID,
getParentNavigationSubtitle,
getReportStatusColorStyle,
getReportStatusTooltipTranslation,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Report>,
policy: OnyxEntry<Policy>,
invoiceReceiverPolicy: OnyxEntry<Policy>,
conciergeReportID: string | undefined,
translate: LocalizedTranslate,
formatPhoneNumber: LocaleContextProps['formatPhoneNumber'],
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -16,6 +17,7 @@ import Navigation from '@libs/Navigation/Navigation';
import {
canEditReportTitle,
getAvailableReportFields,
getInvoiceReceiverPolicyID,
getParentNavigationSubtitle,
getReportFieldKey,
isInvoiceReport as isInvoiceReportUtil,
Expand Down Expand Up @@ -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 ?? {}));
Expand Down
7 changes: 5 additions & 2 deletions src/pages/ShareCodePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -30,6 +31,7 @@ import {
getChatRoomSubtitle,
getDefaultWorkspaceAvatar,
getDisplayNameForParticipant,
getInvoiceReceiverPolicyID,
getParentNavigationSubtitle,
getParticipantsAccountIDsForDisplay,
getPolicyName,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/pages/inbox/HeaderView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
chatIncludesChronos,
getChatRoomSubtitle,
getDisplayNamesWithTooltips,
getInvoiceReceiverPolicyID,
getParentNavigationSubtitle,
getParticipantsAccountIDsForDisplay,
getPolicyDescriptionText,
Expand Down Expand Up @@ -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);
Expand All @@ -199,6 +204,7 @@ function HeaderView({onNavigationMenuButtonClicked, reportID}: HeaderViewProps)
const parentNavigationSubtitleData = getParentNavigationSubtitle(
parentNavigationReport,
policy,
invoiceReceiverPolicy,
conciergeReportID,
translate,
formatPhoneNumber,
Expand Down
90 changes: 81 additions & 9 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'});
});
Expand All @@ -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');
});

Expand Down Expand Up @@ -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');
Expand All @@ -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({});
});

Expand All @@ -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');
});

Expand All @@ -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<Policy>({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<Policy>({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', () => {
Expand Down
Loading