From 3efef0859f8cc0d69848ada3c6cfdd838e8d724d Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:38:19 +0500 Subject: [PATCH 1/2] Filter DELETE-pending transactions out of LHN RBR aggregation Reverting a split leaves the split child transaction in Onyx marked pendingAction DELETE until the server confirms. getViolatingReportIDForRBRInLHN counted that lingering transaction's violation and kept the report's red-dot lit, while the opened report (which filters DELETE-pending transactions) looked clean. Filter DELETE-pending transactions from the RBR aggregation, mirroring the empty-report check in the same file. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libs/ReportUtils.ts | 4 +- tests/unit/ReportUtilsTest.ts | 90 +++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index ddb6f6a77685..2a0455319747 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -9451,7 +9451,9 @@ function getViolatingReportIDForRBRInLHN(report: OnyxEntry, transactionV return false; } const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${potentialReport.policyID}`]; - const transactions = getReportTransactions(potentialReport.reportID); + // Ignore transactions that are already pending deletion (e.g. a reverted split child) so the LHN RBR stays + // consistent with what the opened report renders, which also filters out DELETE-pending transactions. + const transactions = getReportTransactions(potentialReport.reportID).filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); // Allow both open and processing reports to show RBR for violations if (!isOpenOrProcessingReport(potentialReport)) { diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 8d612b2320dd..62623f33f8d8 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -15383,6 +15383,96 @@ describe('ReportUtils', () => { await Onyx.clear(); }); + it('should return null when the only violating transaction is pending deletion (e.g. a reverted split child)', async () => { + await Onyx.clear(); + + const policyID = 'policy-rbr-deleting'; + const chatReportID = 'chat-rbr-deleting'; + const expenseReportID = 'expense-rbr-deleting'; + const transactionID = 'transaction-rbr-deleting'; + + const policyData: Policy = { + id: policyID, + name: 'RBR Pending Delete Test Workspace', + type: CONST.POLICY.TYPE.TEAM, + role: CONST.POLICY.ROLE.ADMIN, + outputCurrency: CONST.CURRENCY.USD, + reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES, + approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, + employeeList: { + [currentUserEmail]: { + role: CONST.POLICY.ROLE.ADMIN, + }, + }, + owner: currentUserEmail, + isPolicyExpenseChatEnabled: true, + }; + + const chatReport: Report = { + ...createPolicyExpenseChat(807), + reportID: chatReportID, + ownerAccountID: currentUserAccountID, + policyID, + iouReportID: expenseReportID, + hasOutstandingChildRequest: true, + }; + + const expenseReport: Report = { + ...createExpenseReport(808), + reportID: expenseReportID, + chatReportID, + ownerAccountID: currentUserAccountID, + managerID: 42, + policyID, + type: CONST.REPORT.TYPE.EXPENSE, + currency: CONST.CURRENCY.USD, + total: 5000, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + }; + + const baseTransaction = createRandomTransaction(807); + // The reverted split child stays in Onyx marked for deletion until the server confirms. + const transaction: Transaction = { + ...baseTransaction, + transactionID, + reportID: expenseReportID, + amount: 5000, + currency: CONST.CURRENCY.USD, + status: CONST.TRANSACTION.STATUS.POSTED, + reimbursable: true, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + }; + + const transactionViolationsKey = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}` as OnyxKey; + const transactionViolationsCollection: OnyxCollection = { + [transactionViolationsKey]: [ + { + name: CONST.VIOLATIONS.MISSING_CATEGORY, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + }, + ], + }; + + await Onyx.merge(ONYXKEYS.SESSION, {accountID: currentUserAccountID, email: currentUserEmail}); + await waitForBatchedUpdates(); + + await Promise.all([ + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyData), + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`, chatReport), + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport), + Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction), + Onyx.merge(transactionViolationsKey, transactionViolationsCollection[transactionViolationsKey]), + ]); + await waitForBatchedUpdates(); + + const result = getViolatingReportIDForRBRInLHN(chatReport, transactionViolationsCollection); + expect(result).toBeNull(); + + await Onyx.clear(); + }); + it('should return null when all expense reports in the policy are closed', async () => { await Onyx.clear(); From 529cdb6fa73fb83b4628291ba1e823c7e3500375 Mon Sep 17 00:00:00 2001 From: dilshodmackbook-sketch <279628751+dilshodmackbook-sketch@users.noreply.github.com> Date: Tue, 4 Aug 2026 09:50:30 +0500 Subject: [PATCH 2/2] Address review: reuse isTransactionPendingDelete, DRY, reorder filter, cover mixed case - Swap the raw pendingAction !== DELETE comparisons for the existing isTransactionPendingDelete predicate across all three call sites in ReportUtils. - Move the transaction filter below the isOpenOrProcessingReport early-return so it only runs for reports that survive the guard. - Add a positive control to the pending-delete RBR test and a mixed live + DELETE-pending test proving the filter is per-transaction. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libs/ReportUtils.ts | 17 +++-- tests/unit/ReportUtilsTest.ts | 117 ++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 9 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 2a0455319747..386bbb4e6da0 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -297,6 +297,7 @@ import { isReceiptBeingScanned, isScanning, isScanRequest as isScanRequestTransactionUtils, + isTransactionPendingDelete, } from './TransactionUtils'; import addTrailingForwardSlash from './UrlUtils'; import {getDefaultAvatarURL} from './UserAvatarUtils'; @@ -9280,7 +9281,7 @@ function getPolicyIDsWithEmptyReportsForAccount( } // Ignore transactions that are already pending deletion so we treat the report as empty once the removal is queued. - const transactions = (reportsTransactionsParam[report.reportID] ?? []).filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + const transactions = (reportsTransactionsParam[report.reportID] ?? []).filter((transaction) => !isTransactionPendingDelete(transaction)); if (transactions.length === 0) { policyLookup[report.policyID] = true; } @@ -9450,16 +9451,16 @@ function getViolatingReportIDForRBRInLHN(report: OnyxEntry, transactionV if (!potentialReport) { return false; } - const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${potentialReport.policyID}`]; - // Ignore transactions that are already pending deletion (e.g. a reverted split child) so the LHN RBR stays - // consistent with what the opened report renders, which also filters out DELETE-pending transactions. - const transactions = getReportTransactions(potentialReport.reportID).filter((transaction) => transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); - // Allow both open and processing reports to show RBR for violations if (!isOpenOrProcessingReport(potentialReport)) { return false; } + const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${potentialReport.policyID}`]; + // Ignore transactions that are already pending deletion (e.g. a reverted split child) so the LHN RBR stays + // consistent with what the opened report renders, which also filters out DELETE-pending transactions. + const transactions = getReportTransactions(potentialReport.reportID).filter((transaction) => !isTransactionPendingDelete(transaction)); + const excludedNoticeNamesForLHN = isProcessingReport(potentialReport) ? [CONST.VIOLATIONS.MODIFIED_AMOUNT] : []; return ( @@ -13014,9 +13015,7 @@ function hasExportError(reportActions: OnyxEntry | ReportAction[] } function doesReportContainRequestsFromMultipleUsers(iouReport: OnyxEntry, shouldExcludeDeletedTransactions = false): boolean { - const transactions = getReportTransactions(iouReport?.reportID).filter( - (transaction) => !shouldExcludeDeletedTransactions || transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - ); + const transactions = getReportTransactions(iouReport?.reportID).filter((transaction) => !shouldExcludeDeletedTransactions || !isTransactionPendingDelete(transaction)); return isIOUReport(iouReport) && transactions.some((transaction) => (Number(transaction?.modifiedAmount) || transaction?.amount) <= 0); } diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 62623f33f8d8..1670f07066b4 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -15470,6 +15470,123 @@ describe('ReportUtils', () => { const result = getViolatingReportIDForRBRInLHN(chatReport, transactionViolationsCollection); expect(result).toBeNull(); + // Positive control: the same violating transaction lights the RBR when it isn't pending deletion. + // This proves the null above comes from the DELETE filter and not from some unrelated gate. + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {pendingAction: null}); + await waitForBatchedUpdates(); + expect(getViolatingReportIDForRBRInLHN(chatReport, transactionViolationsCollection)).toBe(expenseReportID); + + await Onyx.clear(); + }); + + it('should still surface RBR when a report has a live violating transaction alongside a DELETE-pending one', async () => { + await Onyx.clear(); + + const policyID = 'policy-rbr-mixed'; + const chatReportID = 'chat-rbr-mixed'; + const expenseReportID = 'expense-rbr-mixed'; + const deletedTransactionID = 'transaction-rbr-mixed-deleted'; + const liveTransactionID = 'transaction-rbr-mixed-live'; + + const policyData: Policy = { + id: policyID, + name: 'RBR Mixed Transactions Test Workspace', + type: CONST.POLICY.TYPE.TEAM, + role: CONST.POLICY.ROLE.ADMIN, + outputCurrency: CONST.CURRENCY.USD, + reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES, + approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, + employeeList: { + [currentUserEmail]: { + role: CONST.POLICY.ROLE.ADMIN, + }, + }, + owner: currentUserEmail, + isPolicyExpenseChatEnabled: true, + }; + + const chatReport: Report = { + ...createPolicyExpenseChat(807), + reportID: chatReportID, + ownerAccountID: currentUserAccountID, + policyID, + iouReportID: expenseReportID, + hasOutstandingChildRequest: true, + }; + + const expenseReport: Report = { + ...createExpenseReport(808), + reportID: expenseReportID, + chatReportID, + ownerAccountID: currentUserAccountID, + managerID: 42, + policyID, + type: CONST.REPORT.TYPE.EXPENSE, + currency: CONST.CURRENCY.USD, + total: 10000, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + }; + + // Two violating transactions on the SAME expense report: a reverted split child (DELETE-pending) + // and one that is still live. The report's RBR must survive because the live child still violates. + const deletedTransaction: Transaction = { + ...createRandomTransaction(809), + transactionID: deletedTransactionID, + reportID: expenseReportID, + amount: 5000, + currency: CONST.CURRENCY.USD, + status: CONST.TRANSACTION.STATUS.POSTED, + reimbursable: true, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + }; + const liveTransaction: Transaction = { + ...createRandomTransaction(810), + transactionID: liveTransactionID, + reportID: expenseReportID, + amount: 5000, + currency: CONST.CURRENCY.USD, + status: CONST.TRANSACTION.STATUS.POSTED, + reimbursable: true, + pendingAction: null, + }; + + const deletedViolationsKey = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${deletedTransactionID}` as OnyxKey; + const liveViolationsKey = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${liveTransactionID}` as OnyxKey; + const transactionViolationsCollection: OnyxCollection = { + [deletedViolationsKey]: [ + { + name: CONST.VIOLATIONS.MISSING_CATEGORY, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + }, + ], + [liveViolationsKey]: [ + { + name: CONST.VIOLATIONS.MISSING_CATEGORY, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + }, + ], + }; + + await Onyx.merge(ONYXKEYS.SESSION, {accountID: currentUserAccountID, email: currentUserEmail}); + await waitForBatchedUpdates(); + + await Promise.all([ + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyData), + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`, chatReport), + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport), + Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${deletedTransaction.transactionID}`, deletedTransaction), + Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${liveTransaction.transactionID}`, liveTransaction), + Onyx.merge(deletedViolationsKey, transactionViolationsCollection[deletedViolationsKey]), + Onyx.merge(liveViolationsKey, transactionViolationsCollection[liveViolationsKey]), + ]); + await waitForBatchedUpdates(); + + const result = getViolatingReportIDForRBRInLHN(chatReport, transactionViolationsCollection); + expect(result).toBe(expenseReportID); + await Onyx.clear(); });