Skip to content

Commit 3c963f8

Browse files
authored
Merge pull request Expensify#91073 from mukhrr/fix/89548-blockers
[CP Staging] fixed no-imbursable row deploy blockers
2 parents 27ed226 + 9ba7886 commit 3c963f8

4 files changed

Lines changed: 160 additions & 9 deletions

File tree

src/components/ReportActionItem/MoneyReportView.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {
4141
shouldHideSingleReportField,
4242
} from '@libs/ReportUtils';
4343
import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan';
44-
import {getTransactionPendingAction} from '@libs/TransactionUtils';
44+
import {getTransactionPendingAction, isTransactionPendingDelete} from '@libs/TransactionUtils';
4545
import AnimatedEmptyStateBackground from '@pages/inbox/report/AnimatedEmptyStateBackground';
4646
import variables from '@styles/variables';
4747
import CONST from '@src/CONST';
@@ -106,8 +106,15 @@ function MoneyReportView({
106106
const {billableTotal, taxTotal} = getBillableAndTaxTotal(report, transactions);
107107

108108
const isTaxEnabled = isPolicyTaxEnabled(policy);
109-
const isSingleNonReimbursableExpense = isSingleTransactionReport(report, transactions) && transactions.at(0)?.reimbursable === false;
110-
const shouldShowBreakdown = !isSingleNonReimbursableExpense && (!!nonReimbursableSpend || !!billableTotal || (!!taxTotal && isTaxEnabled));
109+
// Exclude transactions pending deletion so a report being reduced to a single expense (e.g. deleting one of two) is treated as single immediately,
110+
// instead of waiting for the optimistic delete to be removed from Onyx.
111+
// While offline the deleted expense is still rendered, so keep counting it to stay consistent with the visible transaction list.
112+
const visibleTransactions = transactions.filter((transaction) => isOffline || !isTransactionPendingDelete(transaction));
113+
const isSingleNonReimbursableExpense = isSingleTransactionReport(report, visibleTransactions) && visibleTransactions.at(0)?.reimbursable === false;
114+
// The reimbursable/non-reimbursable rows duplicate the Total for a single non-reimbursable expense, so suppress only those rows.
115+
// Billable and tax rows convey distinct information and must still show.
116+
const shouldShowReimbursabilityBreakdown = !isSingleNonReimbursableExpense && !!nonReimbursableSpend;
117+
const shouldShowBreakdown = shouldShowReimbursabilityBreakdown || !!billableTotal || (!!taxTotal && isTaxEnabled);
111118
const formattedTotalAmount = convertToDisplayString(totalDisplaySpend, report?.currency);
112119
const formattedOutOfPocketAmount = convertToDisplayString(reimbursableSpend, report?.currency);
113120
const formattedCompanySpendAmount = convertToDisplayString(nonReimbursableSpend, report?.currency);
@@ -261,8 +268,8 @@ function MoneyReportView({
261268
{!!shouldShowBreakdown && (
262269
<>
263270
{[
264-
{label: 'cardTransactions.outOfPocket', value: formattedOutOfPocketAmount, show: !!nonReimbursableSpend},
265-
{label: 'cardTransactions.companySpend', value: formattedCompanySpendAmount, show: !!nonReimbursableSpend},
271+
{label: 'cardTransactions.outOfPocket', value: formattedOutOfPocketAmount, show: shouldShowReimbursabilityBreakdown},
272+
{label: 'cardTransactions.companySpend', value: formattedCompanySpendAmount, show: shouldShowReimbursabilityBreakdown},
266273
{label: 'common.billable', value: formattedBillableAmount, show: !!billableTotal},
267274
{label: 'common.tax', value: formattedTaxAmount, show: !!taxTotal && isTaxEnabled},
268275
]

src/components/ReportActionItem/MoneyRequestView.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ import {
113113
isPerDiemRequest as isPerDiemRequestTransactionUtils,
114114
isScanning,
115115
isTimeRequest as isTimeRequestTransactionUtils,
116+
isTransactionPendingDelete,
116117
shouldShowAttendees as shouldShowAttendeesTransactionUtils,
117118
} from '@libs/TransactionUtils';
118119
import {isInvalidMerchantValue} from '@libs/ValidationUtils';
@@ -252,6 +253,10 @@ function MoneyRequestView({
252253
const isP2PDistanceRequest = isCustomUnitRateIDForP2P(transaction);
253254
const moneyRequestReport = parentReport;
254255
const parentReportTransactions = useReportTransactions(moneyRequestReport?.reportID);
256+
// Exclude transactions pending deletion so the report is recognized as single-expense immediately after deleting one of its expenses,
257+
// instead of waiting for the optimistic delete to be removed from Onyx (https://github.com/Expensify/App/issues/91058).
258+
// While offline the deleted expense is still rendered, so keep counting it to stay consistent with the visible transaction list.
259+
const visibleParentReportTransactions = parentReportTransactions.filter((t) => isOffline || !isTransactionPendingDelete(t));
255260
const isApproved = isReportApproved({report: moneyRequestReport});
256261
const isInvoice = isInvoiceReport(moneyRequestReport);
257262
const isTrackExpense = !mergeTransactionID && isTrackExpenseReportNew(transactionThreadReport, moneyRequestReport, parentReportAction);
@@ -570,7 +575,7 @@ function MoneyRequestView({
570575
amountDescription += ` ${CONST.DOT_SEPARATOR} ${translate('common.converted')} ${convertToDisplayString(transactionConvertedAmount, moneyRequestReport?.currency)}`;
571576
}
572577
const isCurrentTransactionReimbursable = updatedTransaction?.reimbursable ?? !!transactionReimbursable;
573-
if (!isCurrentTransactionReimbursable && isSingleTransactionReport(moneyRequestReport, parentReportTransactions)) {
578+
if (!isCurrentTransactionReimbursable && isSingleTransactionReport(moneyRequestReport, visibleParentReportTransactions)) {
574579
amountDescription += ` ${CONST.DOT_SEPARATOR} ${Str.UCFirst(translate('iou.nonReimbursable'))}`;
575580
}
576581

tests/ui/MoneyReportViewTest.tsx

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ const buildExpenseReport = (overrides: Partial<OnyxTypes.Report> = {}): OnyxType
4747
...overrides,
4848
});
4949

50-
const buildTransaction = (id: string, amount: number, reimbursable: boolean | undefined): OnyxTypes.Transaction =>
50+
const buildTransaction = (id: string, amount: number, reimbursable: boolean | undefined, billable = false, taxAmount = 0): OnyxTypes.Transaction =>
5151
({
5252
transactionID: id,
5353
reportID,
@@ -57,6 +57,8 @@ const buildTransaction = (id: string, amount: number, reimbursable: boolean | un
5757
merchant: 'Merchant',
5858
comment: {},
5959
reimbursable,
60+
billable,
61+
taxAmount,
6062
}) as OnyxTypes.Transaction;
6163

6264
const seedReportAndTransactions = async (transactions: OnyxTypes.Transaction[], reportOverrides: Partial<OnyxTypes.Report> = {}) => {
@@ -76,12 +78,12 @@ const seedReportAndTransactions = async (transactions: OnyxTypes.Transaction[],
7678
await waitForBatchedUpdatesWithAct();
7779
};
7880

79-
const renderMoneyReportView = (report: OnyxTypes.Report) =>
81+
const renderMoneyReportView = (report: OnyxTypes.Report, policy: OnyxTypes.Policy | undefined = undefined) =>
8082
render(
8183
<ComposeProviders components={[OnyxListItemProvider]}>
8284
<MoneyReportView
8385
report={report}
84-
policy={undefined}
86+
policy={policy}
8587
shouldHideThreadDividerLine={false}
8688
shouldShowAnimatedBackground={false}
8789
/>
@@ -125,6 +127,42 @@ describe('MoneyReportView reimbursable/non-reimbursable breakdown rows', () => {
125127
});
126128
});
127129

130+
it('shows the billable row but still hides the redundant rows for a single non-reimbursable billable expense', async () => {
131+
const transactions = [buildTransaction('t1', 5000, false, true)];
132+
await seedReportAndTransactions(transactions, {nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000});
133+
134+
renderMoneyReportView(buildExpenseReport({nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000}));
135+
await waitForBatchedUpdatesWithAct();
136+
137+
await waitFor(() => {
138+
expect(screen.getByText('common.billable')).toBeOnTheScreen();
139+
expect(screen.queryByText('cardTransactions.outOfPocket')).not.toBeOnTheScreen();
140+
expect(screen.queryByText('cardTransactions.companySpend')).not.toBeOnTheScreen();
141+
});
142+
});
143+
144+
it('shows the tax row but still hides the redundant rows for a single non-reimbursable taxed expense', async () => {
145+
const policy = {
146+
id: policyID,
147+
type: CONST.POLICY.TYPE.TEAM,
148+
role: CONST.POLICY.ROLE.ADMIN,
149+
name: 'Policy',
150+
outputCurrency: CONST.CURRENCY.USD,
151+
tax: {trackingEnabled: true},
152+
} as OnyxTypes.Policy;
153+
const transactions = [buildTransaction('t1', 5000, false, false, 500)];
154+
await seedReportAndTransactions(transactions, {nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000});
155+
156+
renderMoneyReportView(buildExpenseReport({nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000}), policy);
157+
await waitForBatchedUpdatesWithAct();
158+
159+
await waitFor(() => {
160+
expect(screen.getByText('common.tax')).toBeOnTheScreen();
161+
expect(screen.queryByText('cardTransactions.outOfPocket')).not.toBeOnTheScreen();
162+
expect(screen.queryByText('cardTransactions.companySpend')).not.toBeOnTheScreen();
163+
});
164+
});
165+
128166
it('still shows both breakdown rows when reimbursable expenses + credits net to zero alongside non-reimbursable spend', async () => {
129167
const transactions = [buildTransaction('t1', 5000, true), buildTransaction('t2', -5000, true), buildTransaction('t3', 3000, false)];
130168
await seedReportAndTransactions(transactions, {nonReimbursableTotal: -3000, unheldNonReimbursableTotal: -3000, unheldTotal: -3000, total: -3000});
@@ -137,4 +175,39 @@ describe('MoneyReportView reimbursable/non-reimbursable breakdown rows', () => {
137175
expect(screen.getByText('cardTransactions.companySpend')).toBeOnTheScreen();
138176
});
139177
});
178+
179+
it('treats the report as a single non-reimbursable expense (hides redundant rows) while the deleted expense is still pending removal', async () => {
180+
const transactions = [
181+
buildTransaction('t1', 5000, false),
182+
{...buildTransaction('t2', 3000, false), pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE} as OnyxTypes.Transaction,
183+
];
184+
await seedReportAndTransactions(transactions, {nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000});
185+
186+
renderMoneyReportView(buildExpenseReport({nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000}));
187+
await waitForBatchedUpdatesWithAct();
188+
189+
await waitFor(() => {
190+
expect(screen.queryByText('cardTransactions.outOfPocket')).not.toBeOnTheScreen();
191+
expect(screen.queryByText('cardTransactions.companySpend')).not.toBeOnTheScreen();
192+
});
193+
});
194+
195+
it('keeps the breakdown rows while offline because the pending-deleted expense is still rendered', async () => {
196+
const transactions = [
197+
buildTransaction('t1', 5000, false),
198+
{...buildTransaction('t2', 3000, false), pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE} as OnyxTypes.Transaction,
199+
];
200+
await seedReportAndTransactions(transactions, {nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000});
201+
await act(async () => {
202+
await Onyx.merge(ONYXKEYS.NETWORK, {shouldForceOffline: true});
203+
});
204+
205+
renderMoneyReportView(buildExpenseReport({nonReimbursableTotal: -5000, unheldNonReimbursableTotal: -5000}));
206+
await waitForBatchedUpdatesWithAct();
207+
208+
await waitFor(() => {
209+
expect(screen.getByText('cardTransactions.outOfPocket')).toBeOnTheScreen();
210+
expect(screen.getByText('cardTransactions.companySpend')).toBeOnTheScreen();
211+
});
212+
});
140213
});

tests/ui/MoneyRequestViewTest.tsx

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,4 +334,70 @@ describe('MoneyRequestView edit fields', () => {
334334
expect(screen.queryByTestId(/^menu-item-iou\.amount.*iou\.nonReimbursable/i)).not.toBeOnTheScreen();
335335
});
336336
});
337+
338+
it('should append "Non-reimbursable" immediately when the only other expense is pending deletion', async () => {
339+
const threadReport = {
340+
...LHNTestUtils.getFakeReport(),
341+
parentReportID: expenseReportID,
342+
parentReportActionID,
343+
};
344+
345+
await setupTestData();
346+
await act(async () => {
347+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {reimbursable: false});
348+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}_sibling`, {
349+
transactionID: `${transactionID}_sibling`,
350+
reportID: expenseReportID,
351+
amount: 2500,
352+
currency: CONST.CURRENCY.USD,
353+
created: '2025-06-02',
354+
merchant: 'Sibling',
355+
comment: {},
356+
reimbursable: false,
357+
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
358+
});
359+
});
360+
await waitForBatchedUpdatesWithAct();
361+
362+
renderMoneyRequestView(threadReport);
363+
await waitForBatchedUpdatesWithAct();
364+
365+
await waitFor(() => {
366+
expect(screen.getByTestId(/^menu-item-iou\.amount.*iou\.nonReimbursable/i)).toBeOnTheScreen();
367+
});
368+
});
369+
370+
it('should NOT append "Non-reimbursable" while offline because the pending-deleted expense is still rendered', async () => {
371+
const threadReport = {
372+
...LHNTestUtils.getFakeReport(),
373+
parentReportID: expenseReportID,
374+
parentReportActionID,
375+
};
376+
377+
await setupTestData();
378+
await act(async () => {
379+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {reimbursable: false});
380+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}_sibling`, {
381+
transactionID: `${transactionID}_sibling`,
382+
reportID: expenseReportID,
383+
amount: 2500,
384+
currency: CONST.CURRENCY.USD,
385+
created: '2025-06-02',
386+
merchant: 'Sibling',
387+
comment: {},
388+
reimbursable: false,
389+
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
390+
});
391+
await Onyx.merge(ONYXKEYS.NETWORK, {shouldForceOffline: true});
392+
});
393+
await waitForBatchedUpdatesWithAct();
394+
395+
renderMoneyRequestView(threadReport);
396+
await waitForBatchedUpdatesWithAct();
397+
398+
await waitFor(() => {
399+
expect(screen.getByTestId(/^menu-item-iou\.amount/)).toBeOnTheScreen();
400+
expect(screen.queryByTestId(/^menu-item-iou\.amount.*iou\.nonReimbursable/i)).not.toBeOnTheScreen();
401+
});
402+
});
337403
});

0 commit comments

Comments
 (0)