From 09c6534823ccfff543604af624d7b751659876e1 Mon Sep 17 00:00:00 2001 From: Linh Date: Sat, 3 May 2025 10:08:15 +0700 Subject: [PATCH 1/3] fix: Approve button not visible on first time expense page load --- src/components/MoneyReportHeader.tsx | 4 ++-- src/components/MoneyReportHeaderOld.tsx | 5 +++-- .../MoneyRequestReportPreviewContent.tsx | 3 ++- src/components/ReportActionItem/ReportPreviewOld.tsx | 2 +- src/libs/actions/IOU.ts | 9 ++++++--- tests/actions/IOUTest.ts | 10 ++++++++++ 6 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index 351a86b81727f..966d56a90d14b 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -209,8 +209,8 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea const shouldShowPayButton = isPaidAnimationRunning || canIOUBePaid || onlyShowPayElsewhere; const shouldShowApproveButton = useMemo( - () => (canApproveIOU(moneyRequestReport, policy) && !hasOnlyPendingTransactions) || isApprovedAnimationRunning, - [moneyRequestReport, policy, hasOnlyPendingTransactions, isApprovedAnimationRunning], + () => (canApproveIOU(moneyRequestReport, policy, transactions) && !hasOnlyPendingTransactions) || isApprovedAnimationRunning, + [moneyRequestReport, policy, transactions, hasOnlyPendingTransactions, isApprovedAnimationRunning], ); const shouldDisableApproveButton = shouldShowApproveButton && !isAllowedToApproveExpenseReport(moneyRequestReport); diff --git a/src/components/MoneyReportHeaderOld.tsx b/src/components/MoneyReportHeaderOld.tsx index f4ced95e35078..3cc286ac2b5ac 100644 --- a/src/components/MoneyReportHeaderOld.tsx +++ b/src/components/MoneyReportHeaderOld.tsx @@ -211,8 +211,8 @@ function MoneyReportHeaderOld({policy, report: moneyRequestReport, transactionTh const shouldShowPayButton = isPaidAnimationRunning || canIOUBePaid || onlyShowPayElsewhere; const shouldShowApproveButton = useMemo( - () => (canApproveIOU(moneyRequestReport, policy) && !hasOnlyPendingTransactions) || isApprovedAnimationRunning, - [moneyRequestReport, policy, hasOnlyPendingTransactions, isApprovedAnimationRunning], + () => (canApproveIOU(moneyRequestReport, policy, transactions) && !hasOnlyPendingTransactions) || isApprovedAnimationRunning, + [moneyRequestReport, policy, transactions, hasOnlyPendingTransactions, isApprovedAnimationRunning], ); const shouldDisableApproveButton = shouldShowApproveButton && !isAllowedToApproveExpenseReport(moneyRequestReport); @@ -220,6 +220,7 @@ function MoneyReportHeaderOld({policy, report: moneyRequestReport, transactionTh const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN; const filteredTransactions = transactions?.filter((t) => t) ?? []; + const shouldShowSubmitButton = canSubmitReport(moneyRequestReport, policy, filteredTransactions, violations); const shouldShowExportIntegrationButton = !shouldShowPayButton && !shouldShowSubmitButton && !!connectedIntegration && isAdmin && canBeExported(moneyRequestReport); diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx index 8b11a3615275e..71f4e52e12043 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx @@ -229,7 +229,8 @@ function MoneyRequestReportPreviewContent({ } }; - const shouldShowApproveButton = useMemo(() => canApproveIOU(iouReport, policy), [iouReport, policy]) || isApprovedAnimationRunning; + const shouldShowApproveButton = useMemo(() => canApproveIOU(iouReport, policy, transactions), [iouReport, policy, transactions]) || isApprovedAnimationRunning; + const shouldShowSubmitButton = canSubmitReport(iouReport, policy, filteredTransactions, violations); const shouldShowSettlementButton = !shouldShowSubmitButton && (shouldShowPayButton || shouldShowApproveButton) && !shouldShowRTERViolationMessage && !shouldShowBrokenConnectionViolation; diff --git a/src/components/ReportActionItem/ReportPreviewOld.tsx b/src/components/ReportActionItem/ReportPreviewOld.tsx index deef6a1a99d3f..c5d759ca417c2 100644 --- a/src/components/ReportActionItem/ReportPreviewOld.tsx +++ b/src/components/ReportActionItem/ReportPreviewOld.tsx @@ -200,7 +200,7 @@ function ReportPreviewOld({ const canIOUBePaidAndApproved = useMemo(() => getCanIOUBePaid(false, false), [getCanIOUBePaid]); const onlyShowPayElsewhere = useMemo(() => !canIOUBePaid && getCanIOUBePaid(true), [canIOUBePaid, getCanIOUBePaid]); const shouldShowPayButton = isPaidAnimationRunning || canIOUBePaid || onlyShowPayElsewhere; - const shouldShowApproveButton = useMemo(() => canApproveIOU(iouReport, policy), [iouReport, policy]) || isApprovedAnimationRunning; + const shouldShowApproveButton = useMemo(() => canApproveIOU(iouReport, policy, transactions), [iouReport, policy, transactions]) || isApprovedAnimationRunning; const shouldDisableApproveButton = shouldShowApproveButton && !isAllowedToApproveExpenseReport(iouReport); diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index f4dfac0404cb6..a4b88f253e4f4 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -8750,7 +8750,11 @@ function sendMoneyWithWallet(report: OnyxEntry, amount: number notifyNewAction(params.chatReportID, managerID); } -function canApproveIOU(iouReport: OnyxTypes.OnyxInputOrEntry | SearchReport, policy: OnyxTypes.OnyxInputOrEntry | SearchPolicy) { +function canApproveIOU( + iouReport: OnyxTypes.OnyxInputOrEntry | SearchReport, + policy: OnyxTypes.OnyxInputOrEntry | SearchPolicy, + iouTransactions?: OnyxTypes.Transaction[], +) { // Only expense reports can be approved if (!isExpenseReport(iouReport) || !(policy && isPaidGroupPolicy(policy))) { return false; @@ -8768,7 +8772,7 @@ function canApproveIOU(iouReport: OnyxTypes.OnyxInputOrEntry | const iouSettled = isSettled(iouReport); const reportNameValuePairs = allReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${iouReport?.reportID}`]; const isArchivedExpenseReport = isArchivedReport(reportNameValuePairs); - const reportTransactions = getReportTransactions(iouReport?.reportID); + const reportTransactions = iouTransactions ?? getReportTransactions(iouReport?.reportID); const hasOnlyPendingCardOrScanningTransactions = reportTransactions.length > 0 && reportTransactions.every(isPendingCardOrScanningTransaction); if (hasOnlyPendingCardOrScanningTransactions) { return false; @@ -8843,7 +8847,6 @@ function canIOUBePaid( const {reimbursableSpend} = getMoneyRequestSpendBreakdown(iouReport); const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : canBeAutoReimbursed(iouReport, policy); const shouldBeApproved = canApproveIOU(iouReport, policy); - const isPayAtEndExpenseReport = isPayAtEndExpenseReportReportUtils(iouReport?.reportID, transactions); return ( isPayer && diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 55adfdf78c496..4ddec49fec004 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -5031,6 +5031,8 @@ describe('actions/IOU', () => { await waitForBatchedUpdates(); expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); + // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data + expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2])).toBeFalsy(); }); it('should return false if we have only scan failure transactions', async () => { const policyID = '2'; @@ -5083,6 +5085,8 @@ describe('actions/IOU', () => { await waitForBatchedUpdates(); expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); + // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data + expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2])).toBeFalsy(); }); it('should return false if all transactions are pending card or scan failure transaction', async () => { const policyID = '2'; @@ -5126,6 +5130,8 @@ describe('actions/IOU', () => { await waitForBatchedUpdates(); expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); + // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data + expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2])).toBeFalsy(); }); it('should return true if at least one transactions is not pending card or scan failure transaction', async () => { const policyID = '2'; @@ -5174,6 +5180,8 @@ describe('actions/IOU', () => { await waitForBatchedUpdates(); expect(canApproveIOU(fakeReport, fakePolicy)).toBeTruthy(); + // Then should return true when passing transactions directly as the third parameter instead of relying on Onyx data + expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2, fakeTransaction3])).toBeTruthy(); }); it('should return false if the report is closed', async () => { @@ -5203,6 +5211,8 @@ describe('actions/IOU', () => { await waitForBatchedUpdates(); // Then, canApproveIOU should return false since the report is closed expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); + // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data + expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction])).toBeFalsy(); }); }); From b37394df0cf2d758e2b62a3eb8e7fb01248f0f03 Mon Sep 17 00:00:00 2001 From: Linh Date: Sat, 3 May 2025 10:35:28 +0700 Subject: [PATCH 2/3] fix: revert space change --- src/components/MoneyReportHeaderOld.tsx | 1 - .../MoneyRequestReportPreviewContent.tsx | 1 - src/libs/actions/IOU.ts | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/MoneyReportHeaderOld.tsx b/src/components/MoneyReportHeaderOld.tsx index 3cc286ac2b5ac..18753c72cfec6 100644 --- a/src/components/MoneyReportHeaderOld.tsx +++ b/src/components/MoneyReportHeaderOld.tsx @@ -220,7 +220,6 @@ function MoneyReportHeaderOld({policy, report: moneyRequestReport, transactionTh const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN; const filteredTransactions = transactions?.filter((t) => t) ?? []; - const shouldShowSubmitButton = canSubmitReport(moneyRequestReport, policy, filteredTransactions, violations); const shouldShowExportIntegrationButton = !shouldShowPayButton && !shouldShowSubmitButton && !!connectedIntegration && isAdmin && canBeExported(moneyRequestReport); diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx index 71f4e52e12043..8c4bd87454dfe 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx @@ -230,7 +230,6 @@ function MoneyRequestReportPreviewContent({ }; const shouldShowApproveButton = useMemo(() => canApproveIOU(iouReport, policy, transactions), [iouReport, policy, transactions]) || isApprovedAnimationRunning; - const shouldShowSubmitButton = canSubmitReport(iouReport, policy, filteredTransactions, violations); const shouldShowSettlementButton = !shouldShowSubmitButton && (shouldShowPayButton || shouldShowApproveButton) && !shouldShowRTERViolationMessage && !shouldShowBrokenConnectionViolation; diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index a4b88f253e4f4..445c7c42b4818 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -8847,6 +8847,7 @@ function canIOUBePaid( const {reimbursableSpend} = getMoneyRequestSpendBreakdown(iouReport); const isAutoReimbursable = policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES ? false : canBeAutoReimbursed(iouReport, policy); const shouldBeApproved = canApproveIOU(iouReport, policy); + const isPayAtEndExpenseReport = isPayAtEndExpenseReportReportUtils(iouReport?.reportID, transactions); return ( isPayer && From 7f3a0bd8f0c82f0060a031398c62e3afee28f781 Mon Sep 17 00:00:00 2001 From: Linh Date: Sat, 3 May 2025 15:48:47 +0700 Subject: [PATCH 3/3] update unitest to use data from hook --- tests/actions/IOUTest.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 4ddec49fec004..33e6d68a27cc9 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -1,7 +1,9 @@ +import {renderHook} from '@testing-library/react-native'; import {format} from 'date-fns'; import isEqual from 'lodash/isEqual'; import type {OnyxCollection, OnyxEntry, OnyxInputValue} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; +import useReportWithTransactionsAndViolations from '@hooks/useReportWithTransactionsAndViolations'; import { calculateDiffAmount, canApproveIOU, @@ -109,6 +111,9 @@ jest.mock('@src/libs/actions/Report', () => { }); jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn()); +// This keeps the error "@rnmapbox/maps native code not available." from causing the tests to fail +jest.mock('@components/ConfirmedRoute.tsx'); + const CARLOS_EMAIL = 'cmartins@expensifail.com'; const CARLOS_ACCOUNT_ID = 1; const CARLOS_PARTICIPANT: Participant = {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, role: 'member'}; @@ -5032,7 +5037,8 @@ describe('actions/IOU', () => { expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data - expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2])).toBeFalsy(); + const {result} = renderHook(() => useReportWithTransactionsAndViolations(reportID)); + expect(canApproveIOU(result.current.at(0) as Report, fakePolicy, result.current.at(1) as Transaction[])).toBeFalsy(); }); it('should return false if we have only scan failure transactions', async () => { const policyID = '2'; @@ -5086,7 +5092,8 @@ describe('actions/IOU', () => { expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data - expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2])).toBeFalsy(); + const {result} = renderHook(() => useReportWithTransactionsAndViolations(reportID)); + expect(canApproveIOU(result.current.at(0) as Report, fakePolicy, result.current.at(1) as Transaction[])).toBeFalsy(); }); it('should return false if all transactions are pending card or scan failure transaction', async () => { const policyID = '2'; @@ -5131,7 +5138,8 @@ describe('actions/IOU', () => { expect(canApproveIOU(fakeReport, fakePolicy)).toBeFalsy(); // Then should return false when passing transactions directly as the third parameter instead of relying on Onyx data - expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2])).toBeFalsy(); + const {result} = renderHook(() => useReportWithTransactionsAndViolations(reportID)); + expect(canApproveIOU(result.current.at(0) as Report, fakePolicy, result.current.at(1) as Transaction[])).toBeFalsy(); }); it('should return true if at least one transactions is not pending card or scan failure transaction', async () => { const policyID = '2'; @@ -5181,7 +5189,8 @@ describe('actions/IOU', () => { expect(canApproveIOU(fakeReport, fakePolicy)).toBeTruthy(); // Then should return true when passing transactions directly as the third parameter instead of relying on Onyx data - expect(canApproveIOU(fakeReport, fakePolicy, [fakeTransaction1, fakeTransaction2, fakeTransaction3])).toBeTruthy(); + const {result} = renderHook(() => useReportWithTransactionsAndViolations(reportID)); + expect(canApproveIOU(result.current.at(0) as Report, fakePolicy, result.current.at(1) as Transaction[])).toBeTruthy(); }); it('should return false if the report is closed', async () => {