From 3c917ddbb3b02045ec9c4eb6d323fb2358034782 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Tue, 28 Apr 2026 13:31:31 +0300 Subject: [PATCH 1/2] fix: filter ineligible reports in useOutstandingReports with regression guards Restores #70423's intent (don't show non-reimbursable-only reports in the move expenses list when the workspace has instant submit + no approvers) while guarding against the three deploy blockers that defeated the previous attempts: - #81608: filter lives in the hook so ReportField on the confirmation page sees the same filtered list and correctly disables itself when no destination is eligible, instead of opening a blank picker. - #88424: the currently-selected source report always survives the filter so outstandingReports.length stays >= 1, shouldShowNotFoundPage stays false, and the "Remove from report" footer renders for retracted reports. - #88425: optimistically created reports (pendingFields.createReport === ADD) bypass isReportIneligibleForMoveExpenses so the new report doesn't flicker in/out during the create -> deferred-microtask move window. --- src/hooks/useOutstandingReports.ts | 29 +- src/libs/ReportUtils.ts | 7 + tests/unit/ReportUtilsTest.ts | 32 +++ .../unit/hooks/useOutstandingReports.test.ts | 250 ++++++++++++++++++ 4 files changed, 314 insertions(+), 4 deletions(-) create mode 100644 tests/unit/hooks/useOutstandingReports.test.ts diff --git a/src/hooks/useOutstandingReports.ts b/src/hooks/useOutstandingReports.ts index b23b255bf3b7..1c911249087b 100644 --- a/src/hooks/useOutstandingReports.ts +++ b/src/hooks/useOutstandingReports.ts @@ -1,8 +1,8 @@ import type {OnyxEntry} from 'react-native-onyx'; -import {getOutstandingReportsForUser, isSelfDM} from '@libs/ReportUtils'; +import {getOutstandingReportsForUser, isReportIneligibleForMoveExpenses, isSelfDM} from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Policy} from '@src/types/onyx'; +import type {Policy, Report} from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import useMappedPolicies from './useMappedPolicies'; import useOnyx from './useOnyx'; @@ -13,6 +13,7 @@ export default function useOutstandingReports(selectedReportID: string | undefin const [outstandingReportsByPolicyID] = useOnyx(ONYXKEYS.DERIVED.OUTSTANDING_REPORTS_BY_POLICY_ID); const [personalPolicyID] = useOnyx(ONYXKEYS.PERSONAL_POLICY_ID); const [allPoliciesID] = useMappedPolicies(policyIdMapper); + const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY); const [reportNameValuePairs] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS); const [selectedReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${selectedReportID}`); @@ -21,6 +22,24 @@ export default function useOutstandingReports(selectedReportID: string | undefin return []; } + // Hide reports the backend will reject as move-expense destinations (instant submit + + // submit-and-close + only non-reimbursable transactions returns a 403 — issue #70423). The + // currently-selected source report is always kept so the picker still renders for in-place + // editing — e.g. the "Remove from report" footer for retracted reports (deploy blocker + // #88424). Optimistic create reports are guarded inside isReportIneligibleForMoveExpenses + // itself to handle the create→move microtask race (deploy blocker #88425). + const filterEligibleReports = (reports: Array>) => + reports.filter((report) => { + if (!report) { + return false; + } + if (selectedReportID && report.reportID === selectedReportID) { + return true; + } + const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`]; + return !isReportIneligibleForMoveExpenses(report, policy); + }); + if (!selectedPolicyID || selectedPolicyID === personalPolicyID || isSelfDM(selectedReport)) { const result = []; for (const policyID of Object.values(allPoliciesID ?? {})) { @@ -31,8 +50,10 @@ export default function useOutstandingReports(selectedReportID: string | undefin const reports = getOutstandingReportsForUser(policyID, ownerAccountID, outstandingReportsByPolicyID[policyID] ?? {}, reportNameValuePairs, isEditing); result.push(...reports); } - return result; + return filterEligibleReports(result); } - return getOutstandingReportsForUser(selectedPolicyID, ownerAccountID, outstandingReportsByPolicyID?.[selectedPolicyID ?? CONST.DEFAULT_NUMBER_ID] ?? {}, reportNameValuePairs, isEditing); + return filterEligibleReports( + getOutstandingReportsForUser(selectedPolicyID, ownerAccountID, outstandingReportsByPolicyID?.[selectedPolicyID ?? CONST.DEFAULT_NUMBER_ID] ?? {}, reportNameValuePairs, isEditing), + ); } diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 4b5e6b2182f0..041faa1e650e 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -2731,6 +2731,13 @@ function isReportIneligibleForMoveExpenses(moneyRequestReport: OnyxEntry if (isDraftReport(moneyRequestReport?.reportID)) { return false; } + // Optimistically created reports go through a transient window where the report exists in + // Onyx but the transaction hasn't been associated yet. During that window + // hasOnlyNonReimbursableTransactions flips from false to true, which would otherwise cause + // the new report to flicker into and out of the picker (deploy blocker #88425). + if (moneyRequestReport?.pendingFields?.createReport === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { + return false; + } return isInstantSubmitEnabled(policy) && isSubmitAndClose(policy) && hasOnlyNonReimbursableTransactions(moneyRequestReport?.reportID); } diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 0860f8729c4c..9d39e45c9dec 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -8835,6 +8835,38 @@ describe('ReportUtils', () => { expect(result).toBe(false); }); + + // Regression test for deploy blocker https://github.com/Expensify/App/issues/88425. + // Optimistically created reports must not be flagged as ineligible during the brief + // window between report creation and the deferred transaction-move microtask, otherwise + // the new report flickers into and out of the picker. + it('should return false for a report that is still optimistically being created (pendingFields.createReport === ADD)', async () => { + const testPolicy: Policy = { + ...createRandomPolicy(3007), + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, + }; + const report: Report = { + ...createRandomReport(30008, undefined), + type: CONST.REPORT.TYPE.EXPENSE, + policyID: testPolicy.id, + ownerAccountID: currentUserAccountID, + pendingFields: {createReport: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}, + }; + const transaction = { + transactionID: '30008', + reportID: report.reportID, + reimbursable: false, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); + + const result = isReportIneligibleForMoveExpenses(report, testPolicy); + + expect(result).toBe(false); + }); }); describe('canDeleteTransaction', () => { diff --git a/tests/unit/hooks/useOutstandingReports.test.ts b/tests/unit/hooks/useOutstandingReports.test.ts new file mode 100644 index 000000000000..60c759319898 --- /dev/null +++ b/tests/unit/hooks/useOutstandingReports.test.ts @@ -0,0 +1,250 @@ +import {act, renderHook, waitFor} from '@testing-library/react-native'; +import Onyx from 'react-native-onyx'; +import useOutstandingReports from '@hooks/useOutstandingReports'; +import initOnyxDerivedValues from '@userActions/OnyxDerived'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy, Report} from '@src/types/onyx'; +import createRandomPolicy from '../../utils/collections/policies'; +import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; + +const POLICY_ID = 'policy1'; +const ACCOUNT_ID = 100; + +function buildPolicy(overrides: Partial = {}): Policy { + return { + ...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM), + id: POLICY_ID, + pendingAction: undefined, + ...overrides, + }; +} + +function buildExpenseReport(reportID: string, overrides: Partial = {}): Report { + return { + reportID, + policyID: POLICY_ID, + ownerAccountID: ACCOUNT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + reportName: `Report ${reportID}`, + ...overrides, + }; +} + +function buildInstantSubmitNoApproversPolicy(): Policy { + return buildPolicy({ + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL, + }); +} + +async function setupOnyxData(policy: Policy, reports: Report[], transactions: Array<{transactionID: string; reportID: string; reimbursable: boolean}>) { + await Onyx.merge(ONYXKEYS.SESSION, {accountID: ACCOUNT_ID}); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy); + + for (const report of reports) { + // eslint-disable-next-line no-await-in-loop + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + } + + for (const txn of transactions) { + // eslint-disable-next-line no-await-in-loop + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${txn.transactionID}`, txn); + } + + await waitForBatchedUpdates(); +} + +describe('useOutstandingReports', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + initOnyxDerivedValues(); + return waitForBatchedUpdates(); + }); + + beforeEach(async () => { + await act(async () => { + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + }); + + afterEach(async () => { + await act(async () => { + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + }); + + it('returns reports when policy does not have instant submit with no approvers', async () => { + // Given a workspace without instant submit and a report containing a non-reimbursable expense + await act(async () => { + await setupOnyxData(buildPolicy({autoReporting: false}), [buildExpenseReport('report1')], [{transactionID: 'txn1', reportID: 'report1', reimbursable: false}]); + }); + + // When the hook computes outstanding reports for that workspace + const {result} = renderHook(() => useOutstandingReports(undefined, POLICY_ID, ACCOUNT_ID, false)); + + // Then the report should be included because the policy doesn't trigger the ineligibility filter + await waitFor(() => { + expect(result.current.length).toBe(1); + expect(result.current.at(0)?.reportID).toBe('report1'); + }); + }); + + it('filters out reports with only non-reimbursable transactions when policy has instant submit and submit & close', async () => { + // Given a workspace with instant submit and no approvers, and a report containing only non-reimbursable expenses + await act(async () => { + await setupOnyxData(buildInstantSubmitNoApproversPolicy(), [buildExpenseReport('report1')], [{transactionID: 'txn1', reportID: 'report1', reimbursable: false}]); + }); + + // When the hook computes outstanding reports for that workspace + const {result} = renderHook(() => useOutstandingReports(undefined, POLICY_ID, ACCOUNT_ID, false)); + + // Then the report should be excluded because moving expenses to it would fail server-side with a 403 (issue #70423) + await waitFor(() => { + expect(result.current.length).toBe(0); + }); + }); + + it('keeps reports with reimbursable transactions even with instant submit and submit & close', async () => { + // Given a workspace with instant submit and no approvers, but a report that has reimbursable expenses + await act(async () => { + await setupOnyxData(buildInstantSubmitNoApproversPolicy(), [buildExpenseReport('report1')], [{transactionID: 'txn1', reportID: 'report1', reimbursable: true}]); + }); + + // When the hook computes outstanding reports for that workspace + const {result} = renderHook(() => useOutstandingReports(undefined, POLICY_ID, ACCOUNT_ID, false)); + + // Then the report should be included because it contains reimbursable transactions that keep the report open + await waitFor(() => { + expect(result.current.length).toBe(1); + expect(result.current.at(0)?.reportID).toBe('report1'); + }); + }); + + it('returns empty array when all reports are ineligible so the confirmation Report field can be disabled', async () => { + // Given a workspace with instant submit and no approvers, where every report has only non-reimbursable expenses + await act(async () => { + await setupOnyxData( + buildInstantSubmitNoApproversPolicy(), + [buildExpenseReport('report1'), buildExpenseReport('report2')], + [ + {transactionID: 'txn1', reportID: 'report1', reimbursable: false}, + {transactionID: 'txn2', reportID: 'report2', reimbursable: false}, + ], + ); + }); + + // When the hook computes outstanding reports for that workspace, with no selected report (the confirmation page case) + const {result} = renderHook(() => useOutstandingReports(undefined, POLICY_ID, ACCOUNT_ID, false)); + + // Then no reports should be returned. ReportField uses outstandingReports.length to decide whether the field is interactive, + // so an empty list correctly disables the field instead of opening a blank picker (deploy blocker #81608) + await waitFor(() => { + expect(result.current.length).toBe(0); + }); + }); + + it('filters only ineligible reports and keeps eligible ones', async () => { + // Given a workspace with instant submit and no approvers, one report with only non-reimbursable expenses and another with reimbursable expenses + await act(async () => { + await setupOnyxData( + buildInstantSubmitNoApproversPolicy(), + [buildExpenseReport('reportIneligible'), buildExpenseReport('reportEligible')], + [ + {transactionID: 'txnIneligible', reportID: 'reportIneligible', reimbursable: false}, + {transactionID: 'txnEligible', reportID: 'reportEligible', reimbursable: true}, + ], + ); + }); + + // When the hook computes outstanding reports for that workspace + const {result} = renderHook(() => useOutstandingReports(undefined, POLICY_ID, ACCOUNT_ID, false)); + + // Then only the eligible report should remain since the ineligible one would cause a 403 if expenses were moved to it + await waitFor(() => { + expect(result.current.length).toBe(1); + expect(result.current.at(0)?.reportID).toBe('reportEligible'); + }); + }); + + // Regression test for deploy blocker https://github.com/Expensify/App/issues/88424. + // The currently selected (source) report must always remain in the outstanding list. Otherwise + // outstandingReports.length === 0 in IOURequestEditReportCommon, which makes + // shouldShowNotFoundPage true and unmounts the SelectionList, hiding the "Remove from report" + // footer that lives inside it. + it('keeps the currently selected source report in the list even when it is otherwise ineligible', async () => { + // Given a retracted report containing only a non-reimbursable expense on a workspace + // with instant submit and no approvers — the same conditions that flag a destination as ineligible + await act(async () => { + await setupOnyxData(buildInstantSubmitNoApproversPolicy(), [buildExpenseReport('sourceReport')], [{transactionID: 'txnSource', reportID: 'sourceReport', reimbursable: false}]); + }); + + // When the picker is rendered with that report as the selected source + const {result} = renderHook(() => useOutstandingReports('sourceReport', POLICY_ID, ACCOUNT_ID, true)); + + // Then the source report must still be returned so the picker renders and the "Remove from report" footer remains accessible + await waitFor(() => { + expect(result.current.length).toBe(1); + expect(result.current.at(0)?.reportID).toBe('sourceReport'); + }); + }); + + it('still filters out an ineligible non-source report even when a different source report is selected', async () => { + // Given a selected source report and another report that is ineligible as a destination + await act(async () => { + await setupOnyxData( + buildInstantSubmitNoApproversPolicy(), + [buildExpenseReport('sourceReport'), buildExpenseReport('ineligibleDestination')], + [ + {transactionID: 'txnSource', reportID: 'sourceReport', reimbursable: true}, + {transactionID: 'txnIneligible', reportID: 'ineligibleDestination', reimbursable: false}, + ], + ); + }); + + // When the hook is invoked with sourceReport as the selected report + const {result} = renderHook(() => useOutstandingReports('sourceReport', POLICY_ID, ACCOUNT_ID, true)); + + // Then only the source remains; the ineligible destination is filtered out as before + await waitFor(() => { + expect(result.current.length).toBe(1); + expect(result.current.at(0)?.reportID).toBe('sourceReport'); + }); + }); + + // Regression test for deploy blocker https://github.com/Expensify/App/issues/88425. + // When "Create report" runs, an optimistic empty report is written to Onyx with + // pendingFields.createReport === ADD, then the transaction is moved to it on a deferred + // microtask. During that gap, hasOnlyNonReimbursableTransactions flips from false (0 txns) + // to true (1 non-reimbursable txn) and would otherwise cause the new report to be filtered + // out, making it appear briefly and then disappear from the picker. + it('keeps optimistically created reports (pendingFields.createReport === ADD) in the list', async () => { + // Given an optimistically created empty report on a workspace with instant submit and no approvers, + // plus a non-reimbursable transaction that has just been associated with it + await act(async () => { + await setupOnyxData( + buildInstantSubmitNoApproversPolicy(), + [ + buildExpenseReport('optimisticReport', { + pendingFields: {createReport: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}, + }), + ], + [{transactionID: 'txnOptimistic', reportID: 'optimisticReport', reimbursable: false}], + ); + }); + + // When the hook computes outstanding reports + const {result} = renderHook(() => useOutstandingReports(undefined, POLICY_ID, ACCOUNT_ID, false)); + + // Then the optimistic report remains visible until the server confirms creation and clears pendingFields + await waitFor(() => { + expect(result.current.length).toBe(1); + expect(result.current.at(0)?.reportID).toBe('optimisticReport'); + }); + }); +}); From 6de392f31dea19d4fbebf3347f2c006248f3f7ff Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Tue, 28 Apr 2026 13:36:51 +0300 Subject: [PATCH 2/2] fix: remove abbreviation flagged by cspell in regression-test comment --- tests/unit/hooks/useOutstandingReports.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/hooks/useOutstandingReports.test.ts b/tests/unit/hooks/useOutstandingReports.test.ts index 60c759319898..f00a19ed9037 100644 --- a/tests/unit/hooks/useOutstandingReports.test.ts +++ b/tests/unit/hooks/useOutstandingReports.test.ts @@ -220,9 +220,9 @@ describe('useOutstandingReports', () => { // Regression test for deploy blocker https://github.com/Expensify/App/issues/88425. // When "Create report" runs, an optimistic empty report is written to Onyx with // pendingFields.createReport === ADD, then the transaction is moved to it on a deferred - // microtask. During that gap, hasOnlyNonReimbursableTransactions flips from false (0 txns) - // to true (1 non-reimbursable txn) and would otherwise cause the new report to be filtered - // out, making it appear briefly and then disappear from the picker. + // microtask. During that gap, hasOnlyNonReimbursableTransactions flips from false (zero + // transactions) to true (one non-reimbursable transaction) and would otherwise cause the + // new report to be filtered out, making it appear briefly and then disappear from the picker. it('keeps optimistically created reports (pendingFields.createReport === ADD) in the list', async () => { // Given an optimistically created empty report on a workspace with instant submit and no approvers, // plus a non-reimbursable transaction that has just been associated with it