Skip to content

Fix stale expense report totals after offline mixed-currency moves #89564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nabi-ebrahimi wants to merge 1 commit into
base: main
Choose a base branch
from
+111 −15
Open

Fix stale expense report totals after offline mixed-currency moves #89564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 40 additions & 9 deletions src/libs/actions/Transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,19 @@ function changeTransactionsReport({
...pendingFields,
};
};
const clearOptimisticPendingFields = (reportIDToUpdate: string | undefined, fieldNames: Array<keyof NonNullable<Report['pendingFields']>>) => {
if (!reportIDToUpdate || !optimisticPendingFieldsByReport[reportIDToUpdate]) {
return;
}

for (const fieldName of fieldNames) {
delete optimisticPendingFieldsByReport[reportIDToUpdate][fieldName];
}

if (Object.keys(optimisticPendingFieldsByReport[reportIDToUpdate]).length === 0) {
delete optimisticPendingFieldsByReport[reportIDToUpdate];
}
};
const clearAccumulatedReportTotals = (reportIDToUpdate: string) => {
delete updatedReportTotals[reportIDToUpdate];
delete updatedReportNonReimbursableTotals[reportIDToUpdate];
Expand All @@ -1014,6 +1027,14 @@ function changeTransactionsReport({
total: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
});
};
const clearStaleReportState = (reportIDToUpdate: string | undefined) => {
if (!reportIDToUpdate) {
return;
}

staleReportIDs.delete(reportIDToUpdate);
clearOptimisticPendingFields(reportIDToUpdate, ['total']);
};
const getTargetReportCurrencies = (targetReportID: string) => {
if (!targetReportCurrenciesByReport[targetReportID]) {
targetReportCurrenciesByReport[targetReportID] = new Set(
Expand Down Expand Up @@ -1204,22 +1225,32 @@ function changeTransactionsReport({
const {amount: transactionAmount = 0, currency: transactionCurrency} = getTransactionDetails(transaction, undefined, undefined, allowNegative) ?? {};
const resolvedTransactionCurrency = transactionCurrency ?? transaction.currency;
const oldReportTotal = oldReport?.total ?? 0;
const updatedReportTotal = transactionAmount < 0 ? oldReportTotal - transactionAmount : oldReportTotal + transactionAmount;

if (oldReport) {
const oldReportTransactionCount = updatedReportTransactionCounts[oldReportID] ?? oldReport.transactionCount ?? 0;
updatedReportTransactionCounts[oldReportID] = Math.max(0, oldReportTransactionCount - 1);

if (staleReportIDs.has(oldReportID) || oldReport.pendingFields?.total) {
const remainingTransactions = getReportTransactions(oldReportID).filter(
(reportTransaction) => reportTransaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !transactionIDs.includes(reportTransaction.transactionID),
);
const willBeEmpty = remainingTransactions.length === 0;
Comment on lines +1233 to +1236
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard empty-report reset with authoritative transaction count

willBeEmpty is computed from getReportTransactions(oldReportID), which can be a partial local subset (for example, search move flows only pass selected transactions in allTransactions from src/pages/Search/SearchTransactionsChangeReport.tsx). In that case this condition can evaluate to empty even when oldReport.transactionCount indicates remaining expenses, and the new branch then force-resets total, nonReimbursableTotal, and unheldNonReimbursableTotal to 0. This produces inconsistent optimistic state (e.g., nonzero transactionCount with zero totals) and incorrect amounts until a server refresh.

Useful? React with 👍 / 👎.


if (willBeEmpty) {
clearStaleReportState(oldReportID);
updatedReportTotals[oldReportID] = 0;
updatedReportNonReimbursableTotals[oldReportID] = 0;
updatedReportUnheldNonReimbursableTotals[oldReportID] = 0;
} else if (staleReportIDs.has(oldReportID) || oldReport.pendingFields?.total) {
markReportTotalAsStale(oldReportID);
} else if (oldReport.currency === transactionCurrency) {
updatedReportTotals[oldReportID] = updatedReportTotals[oldReportID] ? updatedReportTotals[oldReportID] : updatedReportTotal;
updatedReportNonReimbursableTotals[oldReportID] =
(updatedReportNonReimbursableTotals[oldReportID] ? updatedReportNonReimbursableTotals[oldReportID] : (oldReport?.nonReimbursableTotal ?? 0)) +
(transaction?.reimbursable ? 0 : transactionAmount);
updatedReportUnheldNonReimbursableTotals[oldReportID] =
(updatedReportUnheldNonReimbursableTotals[oldReportID] ? updatedReportUnheldNonReimbursableTotals[oldReportID] : (oldReport?.unheldNonReimbursableTotal ?? 0)) +
(transaction?.reimbursable && !isOnHold(transaction) ? 0 : transactionAmount);
const currentTotal = updatedReportTotals[oldReportID] ?? oldReportTotal;
updatedReportTotals[oldReportID] = currentTotal + transactionAmount;

const currentNonReimbursableTotal = updatedReportNonReimbursableTotals[oldReportID] ?? oldReport?.nonReimbursableTotal ?? 0;
updatedReportNonReimbursableTotals[oldReportID] = currentNonReimbursableTotal + (transaction?.reimbursable ? 0 : transactionAmount);

const currentUnheldNonReimbursableTotal = updatedReportUnheldNonReimbursableTotals[oldReportID] ?? oldReport?.unheldNonReimbursableTotal ?? 0;
updatedReportUnheldNonReimbursableTotals[oldReportID] = currentUnheldNonReimbursableTotal + (transaction?.reimbursable && !isOnHold(transaction) ? 0 : transactionAmount);
} else {
markReportTotalAsStale(oldReportID);
}
Expand Down
77 changes: 71 additions & 6 deletions tests/unit/TransactionTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ describe('Transaction', () => {
expect(report?.total).toBe(0);
});

it('should update the old report total when the currency is the same', async () => {
it('should reset the old report total to 0 when moving the last same-currency expense', async () => {
const oldExpenseReport = {
...createRandomReport(1, undefined),
total: -200,
Expand Down Expand Up @@ -738,11 +738,11 @@ describe('Transaction', () => {
});
});

expect(report?.total).toBe(oldExpenseReport.total - transaction.amount);
expect(report?.nonReimbursableTotal).toBe(oldExpenseReport.nonReimbursableTotal - transaction.amount);
expect(report?.total).toBe(0);
expect(report?.nonReimbursableTotal).toBe(0);
});

it('should not update the old report total when the currency is different', async () => {
it('should reset the old report total to 0 when no expenses remain, even if the currency is different', async () => {
const oldExpenseReport = {
...createRandomReport(1, undefined),
total: -200,
Expand Down Expand Up @@ -787,8 +787,73 @@ describe('Transaction', () => {
});
});

expect(report?.total).toBe(oldExpenseReport.total);
expect(report?.nonReimbursableTotal).toBe(oldExpenseReport.nonReimbursableTotal);
expect(report?.total).toBe(0);
expect(report?.nonReimbursableTotal).toBe(0);
});

it('should reset the old report total to 0 after moving all same-currency expenses to a new report', async () => {
const oldExpenseReport = {
...createRandomReport(1, undefined),
total: -300,
nonReimbursableTotal: -300,
currency: CONST.CURRENCY.USD,
transactionCount: 2,
};
const firstTransaction = {
...generateTransaction({
reportID: oldExpenseReport.reportID,
}),
amount: -100,
reimbursable: false,
currency: CONST.CURRENCY.USD,
};
const secondTransaction = {
...generateTransaction({
reportID: oldExpenseReport.reportID,
}),
amount: -200,
reimbursable: false,
currency: CONST.CURRENCY.USD,
};
const firstIOUAction = createIOUAction(firstTransaction, FAKE_OLD_REPORT_ID);
const secondIOUAction = createIOUAction(secondTransaction, FAKE_OLD_REPORT_ID);
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${firstTransaction.transactionID}`, firstTransaction);
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${secondTransaction.transactionID}`, secondTransaction);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${oldExpenseReport.reportID}`, oldExpenseReport);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oldExpenseReport.reportID}`, {
[firstIOUAction.reportActionID]: firstIOUAction,
[secondIOUAction.reportActionID]: secondIOUAction,
});

const fakeReport = await getReportFromUseOnyx(FAKE_NEW_REPORT_ID);
const allTransactions = {
[`${ONYXKEYS.COLLECTION.TRANSACTION}${firstTransaction.transactionID}`]: firstTransaction,
[`${ONYXKEYS.COLLECTION.TRANSACTION}${secondTransaction.transactionID}`]: secondTransaction,
};
changeTransactionsReport({
transactionIDs: [firstTransaction.transactionID, secondTransaction.transactionID],
isASAPSubmitBetaEnabled: false,
accountID: CURRENT_USER_ID,
email: 'test@example.com',
newReport: fakeReport,
policy: undefined,
allTransactions,
policyTagList: undefined,
});
await waitForBatchedUpdates();

const report = await new Promise<OnyxEntry<Report>>((resolve) => {
const connection = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.REPORT}${oldExpenseReport.reportID}`,
callback: (value) => {
Onyx.disconnect(connection);
resolve(value);
},
});
});

expect(report?.total).toBe(0);
expect(report?.nonReimbursableTotal).toBe(0);
});

it('should keep both reports stale and preserve the displayed totals for mixed-currency partial moves', async () => {
Expand Down
Loading