From a4b5c79f960f141f6409a87afc3c6a0808649ada Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Sat, 7 Nov 2026 02:21:26 +0000 Subject: [PATCH 1/7] Show a confirmation before approving reports with violations Co-authored-by: Nguyen Van Duc --- .../useConfirmApproval.ts | 52 +++--- .../useConfirmApproveReportAction.ts | 44 ++--- .../ListItem/ExpenseReportListItem.tsx | 8 + .../ListItem/ReportListItemHeader.tsx | 3 + .../ListItem/TransactionListItem/index.tsx | 3 + .../useConfirmApproveReportViolations.ts | 43 +++++ src/hooks/useHoldMenuSubmit.ts | 49 +++--- src/hooks/useLifecycleActions.tsx | 67 +++---- src/hooks/useSearchBulkActions.ts | 163 ++++++++++-------- src/languages/de.ts | 5 + src/languages/el.ts | 5 + src/languages/en.ts | 5 + src/languages/es.ts | 5 + src/languages/fr.ts | 5 + src/languages/it.ts | 5 + src/languages/ja.ts | 5 + src/languages/nl.ts | 5 + src/languages/pl.ts | 5 + src/languages/pt-BR.ts | 5 + src/languages/zh-hans.ts | 5 + .../confirmApproveViolationsThenProceed.tsx | 48 ++++++ .../getReportApproveViolationSummary.ts | 33 ++++ .../getReportSubmitViolationSummary.ts | 14 +- src/libs/actions/Search.ts | 48 +++--- .../Search/handleActionButtonPressTest.ts | 3 + ...confirmApproveViolationsThenProceedTest.ts | 99 +++++++++++ .../getReportSubmitViolationSummaryTest.ts | 46 +++++ 27 files changed, 590 insertions(+), 188 deletions(-) create mode 100644 src/hooks/useConfirmApproveReportViolations.ts create mode 100644 src/libs/Violations/confirmApproveViolationsThenProceed.tsx create mode 100644 src/libs/Violations/getReportApproveViolationSummary.ts create mode 100644 tests/unit/confirmApproveViolationsThenProceedTest.ts diff --git a/src/components/MoneyReportHeaderPrimaryAction/useConfirmApproval.ts b/src/components/MoneyReportHeaderPrimaryAction/useConfirmApproval.ts index aa497df33633..483111e6dd03 100644 --- a/src/components/MoneyReportHeaderPrimaryAction/useConfirmApproval.ts +++ b/src/components/MoneyReportHeaderPrimaryAction/useConfirmApproval.ts @@ -1,5 +1,6 @@ import {useDelegateNoAccessActions, useDelegateNoAccessState} from '@components/DelegateNoAccessModalProvider'; +import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useDelegateAccountID from '@hooks/useDelegateAccountID'; @@ -73,36 +74,39 @@ function useConfirmApproval( const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); const hasViolations = hasViolationsReportUtils(moneyRequestReport?.reportID, transactionViolations, accountID, email ?? '', undefined, transactions); const isAnyTransactionOnHold = hasHeldExpensesReportUtils(transactions); + const confirmApproveReportViolations = useConfirmApproveReportViolations({report: moneyRequestReport, policy, transactions, violationsCollection: transactionViolations}); const onApprove = (full: boolean) => { if (isDelegateAccessRestricted) { showDelegateNoAccessModal(); return; } - if (!isSubmitPolicy(policy)) { - startApprovedAnimation(); - } - approveMoneyRequest({ - personalDetailsByLogins: getPersonalDetailsByLogin(), - getCurrencyDecimals, - expenseReport: moneyRequestReport, - expenseReportPolicy: policy, - rules, - currentUserAccountIDParam: accountID, - currentUserEmailParam: email ?? '', - hasViolations, - isASAPSubmitBetaEnabled, - userBillingGracePeriodEnds, - amountOwed, - ownerBillingGracePeriodEnd, - ownerLogin, - full, - onApproved: startApprovedAnimation, - delegateEmail, - delegateAccountID, - isTrackIntentUser, - additionalOnyxData: getAdditionalOnyxData?.(), - }); + confirmApproveReportViolations(() => { + if (!isSubmitPolicy(policy)) { + startApprovedAnimation(); + } + approveMoneyRequest({ + personalDetailsByLogins: getPersonalDetailsByLogin(), + getCurrencyDecimals, + expenseReport: moneyRequestReport, + expenseReportPolicy: policy, + rules, + currentUserAccountIDParam: accountID, + currentUserEmailParam: email ?? '', + hasViolations, + isASAPSubmitBetaEnabled, + userBillingGracePeriodEnds, + amountOwed, + ownerBillingGracePeriodEnd, + ownerLogin, + full, + onApproved: startApprovedAnimation, + delegateEmail, + delegateAccountID, + isTrackIntentUser, + additionalOnyxData: getAdditionalOnyxData?.(), + }); + }, full); }; // When there are held expenses the partial/full choice is surfaced up front via the settlement button's approve diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/useConfirmApproveReportAction.ts b/src/components/ReportActionItem/MoneyRequestReportPreview/useConfirmApproveReportAction.ts index 039dc010c6c2..6211f11562ba 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/useConfirmApproveReportAction.ts +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/useConfirmApproveReportAction.ts @@ -5,6 +5,7 @@ */ import {useDelegateNoAccessActions, useDelegateNoAccessState} from '@components/DelegateNoAccessModalProvider'; +import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useOnyx from '@hooks/useOnyx'; @@ -34,31 +35,34 @@ function useConfirmApproveReportAction(actionButtonData: ReturnType { if (isDelegateAccessRestricted) { showDelegateNoAccessModal(); } else { - approveMoneyRequest({ - personalDetailsByLogins: getPersonalDetailsByLogin(), - getCurrencyDecimals, - expenseReport: iouReport, - expenseReportPolicy: policy, - rules, - currentUserAccountIDParam: currentUserDetails.accountID, - currentUserEmailParam: currentUserDetails.email ?? '', - hasViolations, - isASAPSubmitBetaEnabled: isBetaEnabled(CONST.BETAS.ASAP_SUBMIT), - userBillingGracePeriodEnds, - amountOwed, - ownerBillingGracePeriodEnd, - ownerLogin, - full, - onApproved: startApprovedAnimation, - delegateEmail, - delegateAccountID, - isTrackIntentUser, - }); + confirmApproveReportViolations(() => { + approveMoneyRequest({ + personalDetailsByLogins: getPersonalDetailsByLogin(), + getCurrencyDecimals, + expenseReport: iouReport, + expenseReportPolicy: policy, + rules, + currentUserAccountIDParam: currentUserDetails.accountID, + currentUserEmailParam: currentUserDetails.email ?? '', + hasViolations, + isASAPSubmitBetaEnabled: isBetaEnabled(CONST.BETAS.ASAP_SUBMIT), + userBillingGracePeriodEnds, + amountOwed, + ownerBillingGracePeriodEnd, + ownerLogin, + full, + onApproved: startApprovedAnimation, + delegateEmail, + delegateAccountID, + isTrackIntentUser, + }); + }, full); } }; } diff --git a/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx b/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx index 106c65a92dd6..4ea84621d037 100644 --- a/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx +++ b/src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx @@ -13,6 +13,7 @@ import ListItemComposed from '@components/SelectionList/ListItemComposed'; import type {ListItem} from '@components/SelectionList/types'; import Text from '@components/Text'; +import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import useConfirmModal from '@hooks/useConfirmModal'; import useConfirmSubmitReportViolations from '@hooks/useConfirmSubmitReportViolations'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; @@ -219,6 +220,12 @@ function ExpenseReportListItemInner({ const beginSearchActionPress = useSearchActionPress(); const {transactions: reportTransactions, violations: reportViolations} = useTransactionsAndViolationsForReport(reportItem.reportID); const liveReportTransactions = Object.values(reportTransactions); + const confirmApproveReportViolations = useConfirmApproveReportViolations({ + report: reportForViolations, + policy: policyForViolations, + transactions: liveReportTransactions, + violationsCollection: reportViolations, + }); const confirmSubmitReportViolations = useConfirmSubmitReportViolations({ reportID: reportItem.reportID, report: reportForViolations, @@ -276,6 +283,7 @@ function ExpenseReportListItemInner({ onDelegateAccessRestricted: showDelegateNoAccessModal, personalPolicyID, confirmSubmitReportViolations, + confirmApproveReportViolations, ownerBillingGracePeriodEnd, openReportSubmitToPopover, onPendingCardTransactionsBlock: () => showPendingCardTransactionsBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), diff --git a/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx b/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx index 6f53c845f239..ed09ee5cc18c 100644 --- a/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx +++ b/src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx @@ -13,6 +13,7 @@ import {useSearchQueryContext, useSearchResultsContext} from '@components/Search import {useRowSelection} from '@components/Search/SearchSelectionProvider'; import type {ListItem} from '@components/SelectionList/types'; +import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import useConfirmModal from '@hooks/useConfirmModal'; import useConfirmSubmitReportViolations from '@hooks/useConfirmSubmitReportViolations'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; @@ -289,6 +290,7 @@ function ReportListItemHeaderInner({ isTrackIntentUser, rules, }); + const confirmApproveReportViolations = useConfirmApproveReportViolations({report: parentReport ?? snapshotReport, policy: parentPolicy}); const confirmSubmitReportViolations = useConfirmSubmitReportViolations({ reportID: snapshotReport?.reportID ?? reportItem.reportID, report: parentReport ?? snapshotReport, @@ -340,6 +342,7 @@ function ReportListItemHeaderInner({ personalPolicyID, ownerBillingGracePeriodEnd, confirmSubmitReportViolations, + confirmApproveReportViolations, openReportSubmitToPopover, onPendingCardTransactionsBlock: () => showPendingCardTransactionsBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), onAllHeldExpensesBlock: () => showHeldExpensesBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), diff --git a/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx b/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx index 4aec63f0b8ed..e6899a262773 100644 --- a/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx +++ b/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx @@ -11,6 +11,7 @@ import type {TransactionListItemProps, TransactionListItemType} from '@component import useLiveRowCapabilities from '@components/Search/SearchList/ListItem/useLiveRowCapabilities'; import type {ListItem} from '@components/SelectionList/types'; +import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import useConfirmModal from '@hooks/useConfirmModal'; import useConfirmSubmitReportViolations from '@hooks/useConfirmSubmitReportViolations'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; @@ -224,6 +225,7 @@ function TransactionListItemInner({ const transactionViolations = mergeProhibitedViolations(rateOnyxViolations); const filteredViolationsCollection: OnyxCollection = {[transactionViolationsKey]: transactionViolations}; + const confirmApproveReportViolations = useConfirmApproveReportViolations({report: reportForViolations, policy: policyForViolations}); const confirmSubmitReportViolations = useConfirmSubmitReportViolations({ reportID: transactionItem.reportID, report: reportForViolations, @@ -270,6 +272,7 @@ function TransactionListItemInner({ onPendingCardTransactionsBlock: () => showPendingCardTransactionsBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), onAllHeldExpensesBlock: () => showHeldExpensesBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), confirmSubmitReportViolations, + confirmApproveReportViolations, currentUserAccountID, currentUserLogin, activePolicy, diff --git a/src/hooks/useConfirmApproveReportViolations.ts b/src/hooks/useConfirmApproveReportViolations.ts new file mode 100644 index 000000000000..094923c1135b --- /dev/null +++ b/src/hooks/useConfirmApproveReportViolations.ts @@ -0,0 +1,43 @@ +/** Confirms locally available violations before approval, preserving offline behavior. */ +import confirmApproveViolationsThenProceed from '@libs/Violations/confirmApproveViolationsThenProceed'; +import getReportApproveViolationSummary from '@libs/Violations/getReportApproveViolationSummary'; + +import type {Policy, Report, Transaction, TransactionViolations} from '@src/types/onyx'; + +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; + +import useConfirmModal from './useConfirmModal'; +import {useCurrencyListActions} from './useCurrencyList'; +import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; +import useLocalize from './useLocalize'; +import useTransactionsAndViolationsForReport from './useTransactionsAndViolationsForReport'; + +type UseConfirmApproveReportViolationsParams = { + report: OnyxEntry; + policy: OnyxEntry; + transactions?: Array>; + violationsCollection?: OnyxCollection; +}; + +function useConfirmApproveReportViolations({report, policy, transactions: transactionsOverride, violationsCollection: violationsOverride}: UseConfirmApproveReportViolationsParams) { + const {showConfirmModal} = useConfirmModal(); + const {translate, dateFnsLocale} = useLocalize(); + const {convertToDisplayString} = useCurrencyListActions(); + const {accountID, email} = useCurrentUserPersonalDetails(); + const {transactions: ownedTransactions, violations: ownedViolations} = useTransactionsAndViolationsForReport(transactionsOverride ? undefined : report?.reportID); + + return (onProceed: () => void, full = true) => { + const summary = getReportApproveViolationSummary( + transactionsOverride ?? Object.values(ownedTransactions), + violationsOverride ?? ownedViolations, + report, + policy, + email ?? '', + accountID, + full, + ); + confirmApproveViolationsThenProceed({summary, showConfirmModal, translate, dateFnsLocale, convertToDisplayString, onProceed}); + }; +} + +export default useConfirmApproveReportViolations; diff --git a/src/hooks/useHoldMenuSubmit.ts b/src/hooks/useHoldMenuSubmit.ts index a37dbd8bfdc1..526aef26fb8e 100644 --- a/src/hooks/useHoldMenuSubmit.ts +++ b/src/hooks/useHoldMenuSubmit.ts @@ -17,6 +17,7 @@ import {delegateEmailSelector} from '@selectors/Account'; import {hasSeenTourSelector} from '@selectors/Onboarding'; import {loginSelector} from '@selectors/PersonalDetails'; +import useConfirmApproveReportViolations from './useConfirmApproveReportViolations'; import {useCurrencyListActions} from './useCurrencyList'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; import useDelegateAccountID from './useDelegateAccountID'; @@ -74,6 +75,12 @@ function useHoldMenuSubmit({moneyRequestReport, chatReport, requestType, payment const {showDelegateNoAccessModal} = useDelegateNoAccessActions(); const isApprove = requestType === CONST.IOU.REPORT_ACTION_TYPE.APPROVE; + const confirmApproveReportViolations = useConfirmApproveReportViolations({ + report: moneyRequestReport, + policy, + transactions: moneyRequestReportTransactions, + violationsCollection: transactionViolations, + }); const onSubmit = (full: boolean) => { if (isDelegateAccessRestricted) { @@ -91,26 +98,28 @@ function useHoldMenuSubmit({moneyRequestReport, chatReport, requestType, payment const currentChatReport = getReportOrDraftReport({reportID: chatReport?.reportID}) ?? chatReport; if (isApprove) { - approveMoneyRequest({ - personalDetailsByLogins: getPersonalDetailsByLogin(), - getCurrencyDecimals, - expenseReport: currentMoneyRequestReport, - rules, - currentUserAccountIDParam: currentUserDetails.accountID, - currentUserEmailParam: currentUserDetails.email ?? '', - hasViolations, - isASAPSubmitBetaEnabled, - userBillingGracePeriodEnds, - amountOwed, - ownerBillingGracePeriodEnd, - ownerLogin, - full, - onApproved: animationCallback, - expenseReportPolicy: policy, - delegateEmail, - delegateAccountID, - isTrackIntentUser, - }); + confirmApproveReportViolations(() => { + approveMoneyRequest({ + personalDetailsByLogins: getPersonalDetailsByLogin(), + getCurrencyDecimals, + expenseReport: currentMoneyRequestReport, + rules, + currentUserAccountIDParam: currentUserDetails.accountID, + currentUserEmailParam: currentUserDetails.email ?? '', + hasViolations, + isASAPSubmitBetaEnabled, + userBillingGracePeriodEnds, + amountOwed, + ownerBillingGracePeriodEnd, + ownerLogin, + full, + onApproved: animationCallback, + expenseReportPolicy: policy, + delegateEmail, + delegateAccountID, + isTrackIntentUser, + }); + }, full); } else if (currentChatReport && paymentType) { payWithWiseQuote({ bankAccountList, diff --git a/src/hooks/useLifecycleActions.tsx b/src/hooks/useLifecycleActions.tsx index 798c89a96736..fdc520f4fefc 100644 --- a/src/hooks/useLifecycleActions.tsx +++ b/src/hooks/useLifecycleActions.tsx @@ -40,6 +40,7 @@ import React, {useEffect, useRef} from 'react'; import type {ActionHandledType} from './useHoldMenuSubmit'; +import useConfirmApproveReportViolations from './useConfirmApproveReportViolations'; import useConfirmModal from './useConfirmModal'; import useConfirmSubmitReportViolations from './useConfirmSubmitReportViolations'; import {useCurrencyListActions} from './useCurrencyList'; @@ -195,43 +196,47 @@ function useLifecycleActions({reportID, startApprovedAnimation, startAnimation, shouldShowMarkAsDoneCopy, }); + const confirmApproveReportViolations = useConfirmApproveReportViolations({report: moneyRequestReport, policy, transactions, violationsCollection: violations}); + const onApprove = (isFullApproval: boolean, skipAnimation = false) => { if (isDelegateAccessRestricted) { showDelegateNoAccessModal(); return; } - if (!skipAnimation && !isSubmitPolicy(policy)) { - startApprovedAnimation(); - } - approveMoneyRequest({ - personalDetailsByLogins: getPersonalDetailsByLogin(), - getCurrencyDecimals, - expenseReport: moneyRequestReport, - currentUserAccountIDParam: accountID, - currentUserEmailParam: email ?? '', - hasViolations, - isASAPSubmitBetaEnabled, - full: isFullApproval, - expenseReportPolicy: policy, - userBillingGracePeriodEnds, - ownerBillingGracePeriodEnd, - ownerLogin: submitterLogin, - amountOwed, - onApproved: () => { - if (skipAnimation) { - return; - } + confirmApproveReportViolations(() => { + if (!skipAnimation && !isSubmitPolicy(policy)) { startApprovedAnimation(); - }, - delegateEmail, - delegateAccountID, - isTrackIntentUser, - rules, - }); - if (skipAnimation) { - clearSelectedTransactions(true); - onCleanup?.(); - } + } + approveMoneyRequest({ + personalDetailsByLogins: getPersonalDetailsByLogin(), + getCurrencyDecimals, + expenseReport: moneyRequestReport, + currentUserAccountIDParam: accountID, + currentUserEmailParam: email ?? '', + hasViolations, + isASAPSubmitBetaEnabled, + full: isFullApproval, + expenseReportPolicy: policy, + userBillingGracePeriodEnds, + ownerBillingGracePeriodEnd, + ownerLogin: submitterLogin, + amountOwed, + onApproved: () => { + if (skipAnimation) { + return; + } + startApprovedAnimation(); + }, + delegateEmail, + delegateAccountID, + isTrackIntentUser, + rules, + }); + if (skipAnimation) { + clearSelectedTransactions(true); + onCleanup?.(); + } + }, isFullApproval); }; const isIOUPayable = canIOUBePaid(moneyRequestReport, chatReport, policy, bankAccountList, currentUserPersonalDetails.login ?? '', accountID, undefined, false, isChatReportArchived); diff --git a/src/hooks/useSearchBulkActions.ts b/src/hooks/useSearchBulkActions.ts index aec3a17bc34c..559724fba89c 100644 --- a/src/hooks/useSearchBulkActions.ts +++ b/src/hooks/useSearchBulkActions.ts @@ -120,7 +120,9 @@ import { isPerDiemRequest, isScanning, } from '@libs/TransactionUtils'; +import confirmApproveViolationsThenProceed from '@libs/Violations/confirmApproveViolationsThenProceed'; import confirmSubmitViolationsThenProceed from '@libs/Violations/confirmSubmitViolationsThenProceed'; +import getReportApproveViolationSummary from '@libs/Violations/getReportApproveViolationSummary'; import {getReportSubmitViolationSummary, mergeReportSubmitViolationSummaries} from '@libs/Violations/getReportSubmitViolationSummary'; import variables from '@styles/variables'; @@ -1423,80 +1425,100 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) { return; } - // "Select all" can cover more reports than are loaded, so hand the search query to the backend to approve every match. - if (areAllMatchingItemsSelected) { - if (!queryJSON) { - Log.info('[BulkApprove] Dropping bulk approve: no search query'); - return; - } - const allMatchingQuery = getAllMatchingReportQuery(queryJSON, excludedTransactions); - if (!allMatchingQuery) { - Log.info('[BulkApprove] Dropping bulk approve: report exclusion has no reportID'); - return; - } - queueBulkApproveReports(serializeQueryJSONForBackend(allMatchingQuery)); - playSound(SOUNDS.SUCCESS); - clearSelectedTransactions(); - return; - } - - const reportIDList = !selectedReports.length - ? Object.values(selectedTransactions).map((transaction) => transaction.reportID) - : (selectedReports?.filter((report) => !!report).map((report) => report.reportID) ?? []); + const reportIDList = !selectedReports.length ? Object.values(selectedTransactions).map((transaction) => transaction.reportID) : selectedReports.map((report) => report.reportID); const uniqueReportIDs = [...new Set(reportIDList.filter((reportID): reportID is string => reportID !== undefined))]; const searchData = searchResults?.data; - const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); - - let approvedReportCount = 0; - for (const reportID of uniqueReportIDs) { + const snapshotTransactionsByReportID = getTransactionsByReportID(searchData ?? {}); + const perReportSummaries = uniqueReportIDs.map((reportID) => { const expenseReport = getReportFromSearchSnapshot(reportID, searchData, allReports); - if (!expenseReport) { - Log.info('[BulkApprove] Skipping report: expense report not found in the search snapshot or Onyx', false, {reportID}); - continue; - } + const reportPolicy = getPolicyFromSearchSnapshot(expenseReport?.policyID, searchData, policies); + const liveTransactions = Object.values(allReportsTransactionsAndViolations?.[reportID]?.transactions ?? {}); + const reportTransactions = [ + ...new Map([...(snapshotTransactionsByReportID.get(reportID) ?? []), ...liveTransactions].map((transaction) => [transaction.transactionID, transaction])).values(), + ]; + return getReportApproveViolationSummary(reportTransactions, allTransactionViolations, expenseReport, reportPolicy, email ?? '', accountID); + }); + const summary = mergeReportSubmitViolationSummaries(perReportSummaries); - const reportPolicy = getPolicyFromSearchSnapshot(expenseReport.policyID, searchData, policies); - const hasViolations = hasViolationsReportUtils(reportID, allTransactionViolations, accountID, email ?? ''); - const policyToUpgrade = reportPolicy; - const wouldNavigateToUpgrade = isSubmitPolicy(policyToUpgrade) && !!policyToUpgrade?.id; - const wouldNavigateToRestricted = - !!expenseReport.policyID && shouldRestrictUserBillableActions(reportPolicy, ownerBillingGracePeriodEnd, userBillingGracePeriodEnds, amountOwed, accountID); - - approveMoneyRequest({ - personalDetailsByLogins: getPersonalDetailsByLogin(), - getCurrencyDecimals, - expenseReport, - expenseReportPolicy: reportPolicy, - rules, - currentUserAccountIDParam: accountID, - currentUserEmailParam: email ?? '', - hasViolations, - isASAPSubmitBetaEnabled, - userBillingGracePeriodEnds, - amountOwed, - ownerBillingGracePeriodEnd, - ownerLogin: getLoginByAccountID(expenseReport.ownerAccountID, personalDetails), - delegateEmail, - delegateAccountID, - full: true, - additionalOnyxData: getSearchApproveOnyxData(hash, reportID, currentSearchKey), - shouldPlaySuccessSound: false, - isTrackIntentUser, - }); + // As with submit, warnings use local violations; reports not loaded by Select all cannot contribute bullets. + confirmApproveViolationsThenProceed({ + summary, + showConfirmModal, + translate, + dateFnsLocale, + convertToDisplayString, + onProceed: () => { + // "Select all" can cover more reports than are loaded, so hand the search query to the backend to approve every match. + if (areAllMatchingItemsSelected) { + if (!queryJSON) { + Log.info('[BulkApprove] Dropping bulk approve: no search query'); + return; + } + const allMatchingQuery = getAllMatchingReportQuery(queryJSON, excludedTransactions); + if (!allMatchingQuery) { + Log.info('[BulkApprove] Dropping bulk approve: report exclusion has no reportID'); + return; + } + queueBulkApproveReports(serializeQueryJSONForBackend(allMatchingQuery)); + playSound(SOUNDS.SUCCESS); + clearSelectedTransactions(); + return; + } - if (!wouldNavigateToUpgrade && !wouldNavigateToRestricted) { - approvedReportCount += 1; - } - } + const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); - if (approvedReportCount > 0) { - playSound(SOUNDS.SUCCESS); - TransitionTracker.runAfterTransitions({ - callback: () => { - clearSelectedTransactions(); - }, - }); - } + let approvedReportCount = 0; + for (const reportID of uniqueReportIDs) { + const expenseReport = getReportFromSearchSnapshot(reportID, searchData, allReports); + if (!expenseReport) { + Log.info('[BulkApprove] Skipping report: expense report not found in the search snapshot or Onyx', false, {reportID}); + continue; + } + + const reportPolicy = getPolicyFromSearchSnapshot(expenseReport.policyID, searchData, policies); + const hasViolations = hasViolationsReportUtils(reportID, allTransactionViolations, accountID, email ?? ''); + const policyToUpgrade = reportPolicy; + const wouldNavigateToUpgrade = isSubmitPolicy(policyToUpgrade) && !!policyToUpgrade?.id; + const wouldNavigateToRestricted = + !!expenseReport.policyID && shouldRestrictUserBillableActions(reportPolicy, ownerBillingGracePeriodEnd, userBillingGracePeriodEnds, amountOwed, accountID); + + approveMoneyRequest({ + personalDetailsByLogins: getPersonalDetailsByLogin(), + getCurrencyDecimals, + expenseReport, + expenseReportPolicy: reportPolicy, + rules, + currentUserAccountIDParam: accountID, + currentUserEmailParam: email ?? '', + hasViolations, + isASAPSubmitBetaEnabled, + userBillingGracePeriodEnds, + amountOwed, + ownerBillingGracePeriodEnd, + ownerLogin: getLoginByAccountID(expenseReport.ownerAccountID, personalDetails), + delegateEmail, + delegateAccountID, + full: true, + additionalOnyxData: getSearchApproveOnyxData(hash, reportID, currentSearchKey), + shouldPlaySuccessSound: false, + isTrackIntentUser, + }); + + if (!wouldNavigateToUpgrade && !wouldNavigateToRestricted) { + approvedReportCount += 1; + } + } + + if (approvedReportCount > 0) { + playSound(SOUNDS.SUCCESS); + TransitionTracker.runAfterTransitions({ + callback: () => { + clearSelectedTransactions(); + }, + }); + } + }, + }); }, [ isOffline, showOfflineModal, @@ -1527,6 +1549,11 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) { delegateAccountID, getCurrencyDecimals, rules, + allReportsTransactionsAndViolations, + showConfirmModal, + translate, + dateFnsLocale, + convertToDisplayString, ]); const {expenseCount, uniqueReportCount} = useMemo(() => { diff --git a/src/languages/de.ts b/src/languages/de.ts index 791961c64f0f..42c61480d178 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Änderungen speichern', submit: 'Senden', submitAnyway: 'Trotzdem senden', + approveAnyway: 'Trotzdem genehmigen', markAsDone: 'Als erledigt markieren', markAsDoneAnyway: 'Trotzdem als erledigt markieren', submitted: 'Übermittelt', @@ -1511,6 +1512,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Ausstehende Zuordnung', pendingMatchWithCreditCardDescription: 'Beleg wartet auf Abgleich mit Kartenumsatz. Als Barzahlung markieren, um abzubrechen.', markAsCash: 'Als Bar markieren', + confirmApproveReportViolations: { + title: 'Trotzdem genehmigen?', + description: 'Dieser Bericht enthält Richtlinienverstöße. Möchtest du ihn wirklich ohne Änderungen genehmigen?', + }, confirmSubmitReportViolations: { title: 'Bericht einreichen?', description: 'Dieser Bericht enthält Richtlinienverstöße. Möchten Sie ihn wirklich ohne Änderungen einreichen?', diff --git a/src/languages/el.ts b/src/languages/el.ts index 1c8d30ddf11c..79863ffb6af0 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -106,6 +106,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Αποθήκευση αλλαγών', submit: 'Υποβολή', submitAnyway: 'Υποβολή ούτως ή άλλως', + approveAnyway: 'Έγκριση ούτως ή άλλως', markAsDone: 'Σήμανση ως ολοκληρωμένο', markAsDoneAnyway: 'Σήμανση ως ολοκληρωμένο ούτως ή άλλως', submitted: 'Υποβλήθηκε', @@ -1558,6 +1559,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Εκκρεμής αντιστοίχιση', pendingMatchWithCreditCardDescription: 'Η απόδειξη εκκρεμεί για αντιστοίχιση με συναλλαγή κάρτας. Επισημάνετε την ως μετρητά για ακύρωση.', markAsCash: 'Σήμανση ως μετρητά', + confirmApproveReportViolations: { + title: 'Έγκριση ούτως ή άλλως;', + description: 'Αυτή η αναφορά έχει παραβιάσεις πολιτικής. Θέλετε σίγουρα να την εγκρίνετε χωρίς αλλαγές;', + }, confirmSubmitReportViolations: { title: 'Υποβολή αναφοράς;', description: 'Αυτή η αναφορά έχει παραβιάσεις πολιτικής. Είστε σίγουροι ότι θέλετε να την υποβάλετε χωρίς αλλαγές;', diff --git a/src/languages/en.ts b/src/languages/en.ts index 932bb45c8792..3b182fcfe580 100644 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -160,6 +160,7 @@ const translations = { saveChanges: 'Save changes', submit: 'Submit', submitAnyway: 'Submit anyway', + approveAnyway: 'Approve anyway', markAsDone: 'Mark as done', markAsDoneAnyway: 'Mark as done anyway', // @context Status label meaning an item has already been sent or submitted (e.g., a form or report). Not the action “to submit.” @@ -1651,6 +1652,10 @@ const translations = { pendingMatch: 'Pending match', pendingMatchWithCreditCardDescription: 'Receipt pending match with card transaction. Mark as cash to cancel.', markAsCash: 'Mark as cash', + confirmApproveReportViolations: { + title: 'Approve anyway?', + description: 'This report has policy violations. Are you sure you want to approve it without making changes?', + }, confirmSubmitReportViolations: { title: 'Submit report?', description: 'This report has policy violations. Are you sure you want to submit it without making changes?', diff --git a/src/languages/es.ts b/src/languages/es.ts index 3ee2b056b96b..3173a9b141e1 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -95,6 +95,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Guardar cambios', submit: 'Enviar', submitAnyway: 'Enviar de todos modos', + approveAnyway: 'Aprobar de todos modos', markAsDone: 'Marcar como listo', markAsDoneAnyway: 'Marcar como listo de todos modos', submitted: 'Enviado', @@ -1486,6 +1487,10 @@ const translations: TranslationDeepObject = { pendingMatchWithCreditCard: 'Recibo pendiente de adjuntar con la transacción de la tarjeta', pendingMatchWithCreditCardDescription: 'Recibo pendiente de adjuntar con la transacción de la tarjeta. Márcalo como efectivo para cancelar.', markAsCash: 'Marcar como efectivo', + confirmApproveReportViolations: { + title: '¿Aprobar de todos modos?', + description: 'Este informe tiene infracciones de la política. ¿Seguro que quieres aprobarlo sin realizar cambios?', + }, confirmSubmitReportViolations: { title: '¿Enviar informe?', description: 'Este informe tiene infracciones de política. ¿Seguro que quieres enviarlo sin hacer cambios?', diff --git a/src/languages/fr.ts b/src/languages/fr.ts index c6f82b46e368..91205a570725 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Enregistrer les modifications', submit: 'Soumettre', submitAnyway: 'Soumettre quand même', + approveAnyway: 'Approuver quand même', markAsDone: 'Marquer comme terminé', markAsDoneAnyway: 'Marquer comme terminé quand même', submitted: 'Soumis', @@ -1519,6 +1520,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Correspondance en attente', pendingMatchWithCreditCardDescription: 'Reçu en attente de rapprochement avec une transaction par carte. Marquez comme paiement en espèces pour annuler.', markAsCash: 'Marquer comme espèces', + confirmApproveReportViolations: { + title: 'Approuver quand même ?', + description: 'Ce rapport contient des violations de la politique. Voulez-vous vraiment approuver ce rapport sans le modifier ?', + }, confirmSubmitReportViolations: { title: 'Soumettre le rapport ?', description: 'Ce rapport contient des violations de politique. Voulez-vous vraiment le soumettre sans apporter de modifications ?', diff --git a/src/languages/it.ts b/src/languages/it.ts index 4059cea007d2..69de42724f16 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Salva modifiche', submit: 'Invia', submitAnyway: 'Invia comunque', + approveAnyway: 'Approva comunque', markAsDone: 'Segna come completata', markAsDoneAnyway: 'Segna comunque come completata', submitted: 'Inviato', @@ -1510,6 +1511,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Corrispondenza in sospeso', pendingMatchWithCreditCardDescription: 'Ricevuta in attesa di abbinamento con la transazione della carta. Contrassegna come contante per annullare.', markAsCash: 'Segna come contante', + confirmApproveReportViolations: { + title: 'Approvare comunque?', + description: 'Questo report presenta violazioni delle regole. Vuoi davvero approvarlo senza apportare modifiche?', + }, confirmSubmitReportViolations: { title: 'Invia il rapporto?', description: 'Questo rapporto presenta violazioni delle norme. Sei sicuro di volerlo inviare senza apportare modifiche?', diff --git a/src/languages/ja.ts b/src/languages/ja.ts index 146a330cd1eb..08d1ab940eab 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: '変更を保存', submit: '送信', submitAnyway: 'とにかく送信', + approveAnyway: 'それでも承認', markAsDone: '完了にする', markAsDoneAnyway: 'とにかく完了にする', submitted: '送信済み', @@ -1493,6 +1494,10 @@ const translations: TranslationDeepObject = { pendingMatch: '保留中の照合', pendingMatchWithCreditCardDescription: 'レシートはカード取引との照合待ちです。現金としてマークしてキャンセルします。', markAsCash: '現金としてマーク', + confirmApproveReportViolations: { + title: 'それでも承認しますか？', + description: 'このレポートにはポリシー違反があります。変更せずに承認してもよろしいですか？', + }, confirmSubmitReportViolations: { title: 'レポートを送信しますか？', description: 'このレポートにはポリシー違反があります。変更を加えずに送信してもよろしいですか？', diff --git a/src/languages/nl.ts b/src/languages/nl.ts index 754f602e1430..b4f6d12317ba 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Wijzigingen opslaan', submit: 'Verzenden', submitAnyway: 'Toch verzenden', + approveAnyway: 'Toch goedkeuren', markAsDone: 'Markeren als voltooid', markAsDoneAnyway: 'Toch markeren als voltooid', submitted: 'Ingediend', @@ -1508,6 +1509,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Overeenkomst in behandeling', pendingMatchWithCreditCardDescription: 'Bon wordt nog gekoppeld aan kaarttransactie. Markeer als contant om te annuleren.', markAsCash: 'Markeren als contant', + confirmApproveReportViolations: { + title: 'Toch goedkeuren?', + description: 'Dit rapport bevat beleidsovertredingen. Weet je zeker dat je het zonder wijzigingen wilt goedkeuren?', + }, confirmSubmitReportViolations: { title: 'Rapport verzenden?', description: 'Dit rapport heeft beleidsschendingen. Weet u zeker dat u het wilt verzenden zonder wijzigingen aan te brengen?', diff --git a/src/languages/pl.ts b/src/languages/pl.ts index fbc988b6c0a3..716339a31ddb 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -106,6 +106,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Zapisz zmiany', submit: 'Wyślij', submitAnyway: 'Wyślij mimo to', + approveAnyway: 'Zatwierdź mimo to', markAsDone: 'Oznacz jako wykonane', markAsDoneAnyway: 'Oznacz jako wykonane mimo to', submitted: 'Przesłano', @@ -1548,6 +1549,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Oczekujące dopasowanie', pendingMatchWithCreditCardDescription: 'Oczekuje na dopasowanie paragonu do transakcji kartą. Oznacz jako gotówkę, aby anulować.', markAsCash: 'Oznacz jako gotówkę', + confirmApproveReportViolations: { + title: 'Zatwierdzić mimo to?', + description: 'Ten raport zawiera naruszenia zasad. Czy na pewno chcesz go zatwierdzić bez wprowadzania zmian?', + }, confirmSubmitReportViolations: { title: 'Wysłać raport?', description: 'Ten raport zawiera naruszenia zasad. Czy na pewno chcesz go wysłać bez wprowadzania zmian?', diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index f2f1dcbdbf96..e8283992c1c8 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: 'Salvar alterações', submit: 'Enviar', submitAnyway: 'Enviar mesmo assim', + approveAnyway: 'Aprovar mesmo assim', markAsDone: 'Marcar como concluído', markAsDoneAnyway: 'Marcar como concluído mesmo assim', submitted: 'Enviado', @@ -1510,6 +1511,10 @@ const translations: TranslationDeepObject = { pendingMatch: 'Correspondência pendente', pendingMatchWithCreditCardDescription: 'Recibo aguardando correspondência com transação do cartão. Marque como dinheiro para cancelar.', markAsCash: 'Marcar como dinheiro', + confirmApproveReportViolations: { + title: 'Aprovar mesmo assim?', + description: 'Este relatório tem violações da política. Tem certeza de que deseja aprová-lo sem fazer alterações?', + }, confirmSubmitReportViolations: { title: 'Enviar relatório?', description: 'Este relatório tem violações de política. Tem certeza de que deseja enviá-lo sem fazer alterações?', diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index 62724d2c98c0..fc005dfc47a9 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -102,6 +102,7 @@ const translations: TranslationDeepObject = { saveChanges: '保存更改', submit: '提交', submitAnyway: '仍然提交', + approveAnyway: '仍然批准', markAsDone: '标记为完成', markAsDoneAnyway: '仍然标记为完成', submitted: '已提交', @@ -1454,6 +1455,10 @@ const translations: TranslationDeepObject = { pendingMatch: '待匹配', pendingMatchWithCreditCardDescription: '收据正在等待与卡片交易匹配。将其标记为现金以取消。', markAsCash: '标记为现金', + confirmApproveReportViolations: { + title: '仍然批准？', + description: '此报告存在违反政策的情况。确定要在不作更改的情况下批准吗？', + }, confirmSubmitReportViolations: { title: '提交报告？', description: '此报告存在违反政策的情况。您确定要不做任何更改就提交吗？', diff --git a/src/libs/Violations/confirmApproveViolationsThenProceed.tsx b/src/libs/Violations/confirmApproveViolationsThenProceed.tsx new file mode 100644 index 000000000000..52149a84d318 --- /dev/null +++ b/src/libs/Violations/confirmApproveViolationsThenProceed.tsx @@ -0,0 +1,48 @@ +/** Reuses the submit warning's bullet list without resolving violations when approving. */ +import type {LocaleContextProps} from '@components/LocaleContextProvider'; +import {ModalActions} from '@components/Modal/Global/ModalContext'; +import SubmitViolationsList from '@components/SubmitViolationsList'; + +import type {CurrencyListActionsContextType} from '@hooks/useCurrencyList'; + +import showConfirmModalAfterMoreMenuDismiss from '@libs/showConfirmModalAfterMoreMenuDismiss'; + +import CONST from '@src/CONST'; + +import type getReportApproveViolationSummary from './getReportApproveViolationSummary'; + +import {buildSubmitViolationBullets, hasAnySubmitViolation} from './getReportSubmitViolationSummary'; + +type ConfirmApproveViolationsThenProceedParams = { + summary: ReturnType; + showConfirmModal: Parameters[0]; + translate: LocaleContextProps['translate']; + dateFnsLocale: LocaleContextProps['dateFnsLocale']; + convertToDisplayString: CurrencyListActionsContextType['convertToDisplayString']; + onProceed: () => void; +}; + +function confirmApproveViolationsThenProceed({summary, showConfirmModal, translate, dateFnsLocale, convertToDisplayString, onProceed}: ConfirmApproveViolationsThenProceedParams) { + if (!hasAnySubmitViolation(summary)) { + onProceed(); + return; + } + + const bullets = buildSubmitViolationBullets({summary, translate, dateFnsLocale, convertToDisplayString}); + showConfirmModalAfterMoreMenuDismiss(showConfirmModal, { + title: translate('iou.confirmApproveReportViolations.title'), + subtitle: translate('iou.confirmApproveReportViolations.description'), + prompt: , + confirmText: translate('common.approveAnyway'), + cancelText: translate('common.cancel'), + buttonVariant: CONST.BUTTON_VARIANT.DANGER, + shouldEnablePromptScroll: true, + }).then((result) => { + if (result.action !== ModalActions.CONFIRM) { + return; + } + onProceed(); + }); +} + +export default confirmApproveViolationsThenProceed; diff --git a/src/libs/Violations/getReportApproveViolationSummary.ts b/src/libs/Violations/getReportApproveViolationSummary.ts new file mode 100644 index 000000000000..0690c5532ded --- /dev/null +++ b/src/libs/Violations/getReportApproveViolationSummary.ts @@ -0,0 +1,33 @@ +/** Builds approval warnings from local data without the submitter's rejection or card-match warnings. */ +import {isOnHold} from '@libs/TransactionUtils'; + +import type {Policy, Report, Transaction, TransactionViolations} from '@src/types/onyx'; + +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; + +import {getReportSubmitViolationSummary} from './getReportSubmitViolationSummary'; + +function getReportApproveViolationSummary( + transactions: Array>, + violationsCollection: OnyxCollection, + report: OnyxEntry, + policy: OnyxEntry, + currentUserEmail: string, + currentUserAccountID: number, + full = true, +) { + const summary = getReportSubmitViolationSummary( + transactions.filter((transaction) => transaction && (full || !isOnHold(transaction))), + violationsCollection, + report, + policy, + currentUserEmail, + currentUserAccountID, + full, + ); + + // These warnings require acknowledgement by the submitter, not the approver. + return {...summary, hasRejectedExpense: false, hasReportBeenRejected: false, hasPendingCardMatch: false}; +} + +export default getReportApproveViolationSummary; diff --git a/src/libs/Violations/getReportSubmitViolationSummary.ts b/src/libs/Violations/getReportSubmitViolationSummary.ts index c0dd445cbef6..cf72e230e086 100644 --- a/src/libs/Violations/getReportSubmitViolationSummary.ts +++ b/src/libs/Violations/getReportSubmitViolationSummary.ts @@ -40,14 +40,20 @@ type ReportSubmitViolationSummary = { }; /** Whether the "Submit report?" modal should warn about the required fields this expense has left empty. */ -function shouldWarnAboutMissingFields(transaction: Transaction, transactionViolations: OnyxEntry, report: OnyxEntry, policy: OnyxEntry): boolean { +function shouldWarnAboutMissingFields( + transaction: Transaction, + transactionViolations: OnyxEntry, + report: OnyxEntry, + policy: OnyxEntry, + shouldIncludeHeldExpenses: boolean, +): boolean { // These warnings are only for expense reports, where the merchant is required. if (!isExpenseReport(report)) { return false; } // Submitting moves a held expense to a new draft report, except on a dynamic external workflow, so it isn't part of this submission. - if (isOnHold(transaction) && !hasDynamicExternalWorkflow(policy)) { + if (!shouldIncludeHeldExpenses && isOnHold(transaction) && !hasDynamicExternalWorkflow(policy)) { return false; } @@ -71,6 +77,8 @@ function getReportSubmitViolationSummary( policy: OnyxEntry, currentUserEmail: string, currentUserAccountID: number, + // Full approval includes held expenses, whereas submission normally moves them to another report. + shouldIncludeHeldExpenses = false, ): ReportSubmitViolationSummary { let hasRejectedExpense = false; let hasPendingCardMatch = false; @@ -85,7 +93,7 @@ function getReportSubmitViolationSummary( const transactionViolations = violationsCollection?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`]; // These fields have no violation, so this has to run before the check below skips expenses without violations. - if (shouldWarnAboutMissingFields(transaction, transactionViolations, report, policy)) { + if (shouldWarnAboutMissingFields(transaction, transactionViolations, report, policy, shouldIncludeHeldExpenses)) { hasMissingMerchant ||= isMerchantMissing(transaction); // Expense reports allow a zero amount, so only a failed scan's placeholder counts as a missing amount. hasMissingAmount ||= isFailedScanAmountPlaceholder(transaction); diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index 73feb18f8cdd..1dc2d9244702 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -277,6 +277,7 @@ type HandleActionButtonPressParams = { onAllHeldExpensesBlock?: () => void; /** Confirms any unacknowledged report violations before proceeding with the SUBMIT action. */ confirmSubmitReportViolations: (onProceed: ConfirmSubmitReportViolationsOnProceed) => void; + confirmApproveReportViolations: (onProceed: () => void) => void; openReportSubmitToPopover?: (options?: ReportSubmitToPopoverOpenOptions) => void; currentUserAccountID: number; currentUserLogin?: string; @@ -324,6 +325,7 @@ function handleActionButtonPress({ onUndelete, onAllHeldExpensesBlock, confirmSubmitReportViolations, + confirmApproveReportViolations, currentUserAccountID, openReportSubmitToPopover, currentUserLogin, @@ -408,28 +410,30 @@ function handleActionButtonPress({ Navigation.navigate(ROUTES.RESTRICTED_ACTION.getRoute(snapshotReport.policyID)); return; } - getApproveActionCallback({ - hash, - item, - snapshotReport, - snapshotPolicy, - policy, - currentSearchKey, - currentUserAccountID, - currentUserLogin, - isASAPSubmitBetaEnabled, - userBillingGracePeriodEnds, - ownerBillingGracePeriodEnd, - amountOwed, - delegateEmail, - delegateAccountID, - isTrackIntentUser, - ownerLogin: submitterLogin, - allViolations, - rules, - getCurrencyDecimals, - personalDetailsByLogins, - }); + confirmApproveReportViolations(() => + getApproveActionCallback({ + hash, + item, + snapshotReport, + snapshotPolicy, + policy, + currentSearchKey, + currentUserAccountID, + currentUserLogin, + isASAPSubmitBetaEnabled, + userBillingGracePeriodEnds, + ownerBillingGracePeriodEnd, + amountOwed, + delegateEmail, + delegateAccountID, + isTrackIntentUser, + ownerLogin: submitterLogin, + allViolations, + rules, + getCurrencyDecimals, + personalDetailsByLogins, + }), + ); return; case CONST.SEARCH.ACTION_TYPES.SUBMIT: { if (snapshotReport.policyID && shouldRestrictUserBillableActions(policy, ownerBillingGracePeriodEnd, userBillingGracePeriodEnds, amountOwed, currentUserAccountID)) { diff --git a/tests/unit/Search/handleActionButtonPressTest.ts b/tests/unit/Search/handleActionButtonPressTest.ts index fbd2d3536cce..ab65ece13e10 100644 --- a/tests/unit/Search/handleActionButtonPressTest.ts +++ b/tests/unit/Search/handleActionButtonPressTest.ts @@ -368,6 +368,7 @@ describe('handleActionButtonPress', () => { rules: undefined, bankAccountList: undefined, confirmSubmitReportViolations: jest.fn(), + confirmApproveReportViolations: (onProceed) => onProceed(), personalDetailsByLogins: {}, }); @@ -404,6 +405,7 @@ describe('handleActionButtonPress', () => { rules: undefined, bankAccountList: undefined, confirmSubmitReportViolations: jest.fn(), + confirmApproveReportViolations: (onProceed) => onProceed(), personalDetailsByLogins: {}, }); expect(goToItem).toHaveBeenCalledTimes(0); @@ -446,6 +448,7 @@ describe('handleActionButtonPress', () => { getCurrencyDecimals: getCurrencyDecimalsLocal, bankAccountList: undefined, confirmSubmitReportViolations: jest.fn(), + confirmApproveReportViolations: (onProceed) => onProceed(), }); // Then: hasViolations is evaluated against the passed collection, proving the deprecated global getter is no longer used, diff --git a/tests/unit/confirmApproveViolationsThenProceedTest.ts b/tests/unit/confirmApproveViolationsThenProceedTest.ts new file mode 100644 index 000000000000..69d4e5f2f792 --- /dev/null +++ b/tests/unit/confirmApproveViolationsThenProceedTest.ts @@ -0,0 +1,99 @@ +import type {LocaleContextProps} from '@components/LocaleContextProvider'; +import {ModalActions} from '@components/Modal/Global/ModalContext'; + +import showConfirmModalAfterMoreMenuDismiss from '@libs/showConfirmModalAfterMoreMenuDismiss'; +import confirmApproveViolationsThenProceed from '@libs/Violations/confirmApproveViolationsThenProceed'; +import type getReportApproveViolationSummary from '@libs/Violations/getReportApproveViolationSummary'; + +import CONST from '@src/CONST'; + +import createMock from '../utils/createMock'; + +jest.mock('@components/Modal/Global/ModalContext', () => ({ModalActions: {CONFIRM: 'CONFIRM', CLOSE: 'CLOSE'}})); +jest.mock('@components/SubmitViolationsList', () => 'SubmitViolationsList'); +jest.mock('@libs/showConfirmModalAfterMoreMenuDismiss', () => ({__esModule: true, default: jest.fn()})); +jest.mock('@libs/Violations/getReportSubmitViolationSummary', () => ({ + hasAnySubmitViolation: (summary: {otherViolations: Map}) => summary.otherViolations.size > 0, + buildSubmitViolationBullets: () => ['Receipt required'], +})); + +type ApprovalSummary = ReturnType; + +const translate = createMock(jest.fn((key: string) => key)); +const dateFnsLocale = createMock({}); + +function createSummary(hasViolations = false): ApprovalSummary { + return { + hasRejectedExpense: false, + hasReportBeenRejected: false, + hasPendingCardMatch: false, + hasMissingMerchant: false, + hasMissingAmount: false, + otherViolations: hasViolations ? new Map([[CONST.VIOLATIONS.RECEIPT_REQUIRED, {name: CONST.VIOLATIONS.RECEIPT_REQUIRED}]]) : new Map(), + }; +} + +function confirmApproval(summary: ApprovalSummary, onProceed: () => void) { + confirmApproveViolationsThenProceed({summary, onProceed, showConfirmModal: jest.fn(), translate, dateFnsLocale, convertToDisplayString: jest.fn()}); +} + +describe('confirmApproveViolationsThenProceed', () => { + beforeEach(() => jest.clearAllMocks()); + + it('approves immediately without a warning for a clean report', () => { + // Given a report without locally available violations + const onProceed = jest.fn(); + + // When approval is requested + confirmApproval(createSummary(), onProceed); + + // Then no confirmation is necessary + expect(onProceed).toHaveBeenCalledWith(); + expect(showConfirmModalAfterMoreMenuDismiss).not.toHaveBeenCalled(); + }); + + it('waits for explicit confirmation and leaves the violations unchanged', async () => { + // Given a report with a policy violation and a modal the approver has not answered + const summary = createSummary(true); + const onProceed = jest.fn(); + let confirm: (result: {action: typeof ModalActions.CONFIRM}) => void = () => {}; + jest.mocked(showConfirmModalAfterMoreMenuDismiss).mockReturnValue( + new Promise((resolve) => { + confirm = resolve; + }), + ); + + // When the approver requests approval, then confirms the warning + confirmApproval(summary, onProceed); + expect(onProceed).not.toHaveBeenCalled(); + expect(showConfirmModalAfterMoreMenuDismiss).toHaveBeenCalledWith( + expect.any(Function), + expect.objectContaining({ + title: 'iou.confirmApproveReportViolations.title', + subtitle: 'iou.confirmApproveReportViolations.description', + confirmText: 'common.approveAnyway', + cancelText: 'common.cancel', + }), + ); + confirm({action: ModalActions.CONFIRM}); + await Promise.resolve(); + + // Then approval proceeds once without a flag to resolve violations + expect(onProceed).toHaveBeenCalledTimes(1); + expect(onProceed).toHaveBeenCalledWith(); + expect(summary.otherViolations.size).toBe(1); + }); + + it('does not approve when the warning is cancelled or dismissed', async () => { + // Given a report with violations and a cancelled modal + const onProceed = jest.fn(); + jest.mocked(showConfirmModalAfterMoreMenuDismiss).mockResolvedValue(createMock({action: ModalActions.CLOSE})); + + // When approval is requested + confirmApproval(createSummary(true), onProceed); + await Promise.resolve(); + + // Then no approval side effects run + expect(onProceed).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/getReportSubmitViolationSummaryTest.ts b/tests/unit/getReportSubmitViolationSummaryTest.ts index a83a1732305a..a7690a9e10ce 100644 --- a/tests/unit/getReportSubmitViolationSummaryTest.ts +++ b/tests/unit/getReportSubmitViolationSummaryTest.ts @@ -1,5 +1,6 @@ import type {LocaleContextProps} from '@components/LocaleContextProvider'; +import getReportApproveViolationSummary from '@libs/Violations/getReportApproveViolationSummary'; import { buildSubmitViolationBullets, getReportSubmitViolationSummary, @@ -69,6 +70,51 @@ function createMockRejectedReport(): Report { }); } +describe('getReportApproveViolationSummary', () => { + it('warns about a held expense with a missing merchant when approving everything', () => { + // Given a held expense whose missing merchant is not represented by a server violation + const transaction = createMockTransactionWithMerchant(CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, {comment: {hold: 'hold-action'}}); + const report = createMockExpenseReport(); + + // When choosing full or partial approval + const full = getReportApproveViolationSummary([transaction], {}, report, undefined, '', 0, true); + const partial = getReportApproveViolationSummary([transaction], {}, report, undefined, '', 0, false); + + // Then only full approval includes the held expense's missing field warning + expect(full.hasMissingMerchant).toBe(true); + expect(hasAnySubmitViolation(partial)).toBe(false); + }); + + it('does not show submitter-only rejection or pending card-match warnings', () => { + // Given a rejected report with rejected expenses and a pending card match + const violations = { + [getViolationsKey('1')]: [createMockViolation(CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE), createMockViolation(CONST.VIOLATIONS.RTER, {pendingPattern: true})], + }; + + // When an approver reviews its violations + const summary = getReportApproveViolationSummary([createMockTransaction('1')], violations, createMockRejectedReport(), undefined, '', 0); + + // Then the submitter's acknowledgement warnings are not shown to the approver + expect(hasAnySubmitViolation(summary)).toBe(false); + }); + + it('includes held expenses only when approving the full report', () => { + // Given a held expense with a policy violation + const transaction = createMock({transactionID: '1', comment: {hold: 'hold-action'}}); + const overLimit = createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT); + const violations = {[getViolationsKey('1')]: [overLimit]}; + + // When choosing full or partial approval + const full = getReportApproveViolationSummary([transaction], violations, undefined, undefined, '', 0, true); + const partial = getReportApproveViolationSummary([transaction], violations, undefined, undefined, '', 0, false); + + // Then only the full approval warns about this expense, without mutating its violations + expect(full.otherViolations.get(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT)).toBe(overLimit); + expect(hasAnySubmitViolation(partial)).toBe(false); + expect(violations[getViolationsKey('1')]).toEqual([overLimit]); + }); +}); + describe('getReportSubmitViolationSummary', () => { it('returns all-false/empty when there are no violations', () => { // Given a transaction with an empty violations array (the shape Onyx reports once violations have been computed and none apply) From c5fa640a9c5378f5f4779bb52024afd0f07d2b95 Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Sat, 7 Nov 2026 02:39:41 +0000 Subject: [PATCH 2/7] Correct approval warning test fixtures and Search mocks Co-authored-by: Nguyen Van Duc --- tests/unit/confirmApproveViolationsThenProceedTest.ts | 9 +++++++-- tests/unit/hooks/useSearchBulkActionsTest.ts | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/unit/confirmApproveViolationsThenProceedTest.ts b/tests/unit/confirmApproveViolationsThenProceedTest.ts index 69d4e5f2f792..08e45f9a7e53 100644 --- a/tests/unit/confirmApproveViolationsThenProceedTest.ts +++ b/tests/unit/confirmApproveViolationsThenProceedTest.ts @@ -19,17 +19,22 @@ jest.mock('@libs/Violations/getReportSubmitViolationSummary', () => ({ type ApprovalSummary = ReturnType; -const translate = createMock(jest.fn((key: string) => key)); +const translate = createMock(jest.fn((...args: [string, ...unknown[]]) => args[0])); const dateFnsLocale = createMock({}); function createSummary(hasViolations = false): ApprovalSummary { + const otherViolations: ApprovalSummary['otherViolations'] = new Map(); + if (hasViolations) { + otherViolations.set(CONST.VIOLATIONS.RECEIPT_REQUIRED, {name: CONST.VIOLATIONS.RECEIPT_REQUIRED, type: CONST.VIOLATION_TYPES.WARNING}); + } + return { hasRejectedExpense: false, hasReportBeenRejected: false, hasPendingCardMatch: false, hasMissingMerchant: false, hasMissingAmount: false, - otherViolations: hasViolations ? new Map([[CONST.VIOLATIONS.RECEIPT_REQUIRED, {name: CONST.VIOLATIONS.RECEIPT_REQUIRED}]]) : new Map(), + otherViolations, }; } diff --git a/tests/unit/hooks/useSearchBulkActionsTest.ts b/tests/unit/hooks/useSearchBulkActionsTest.ts index 4b304ada370f..d25b32beca38 100644 --- a/tests/unit/hooks/useSearchBulkActionsTest.ts +++ b/tests/unit/hooks/useSearchBulkActionsTest.ts @@ -40,6 +40,8 @@ jest.mock('@libs/actions/Export', () => ({ jest.mock('@libs/actions/Search', () => ({ // The hook and shouldShowDeleteOption resolve report actions through this helper, so the real implementation has to stay available getReportActionsFromSearchSnapshot: jest.requireActual('@libs/actions/Search').getReportActionsFromSearchSnapshot, + getReportFromSearchSnapshot: jest.requireActual('@libs/actions/Search').getReportFromSearchSnapshot, + getPolicyFromSearchSnapshot: jest.requireActual('@libs/actions/Search').getPolicyFromSearchSnapshot, getExportTemplates: jest.fn(() => ({customTemplates: [], defaultTemplates: []})), exportSearchItemsToCSV: jest.fn(), queueExportSearchItemsToCSV: jest.fn(() => 'mock-export-id'), From 05fa6616eb2553b16f220385ece5d7784065ef9f Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Sat, 7 Nov 2026 04:21:08 +0000 Subject: [PATCH 3/7] Remove unused transaction row approval confirmation Co-authored-by: Nguyen Van Duc --- .../ListItem/TransactionListItem/index.tsx | 3 --- src/libs/actions/Search.ts | 4 ++-- .../TransactionListItemViolationsTest.tsx | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx b/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx index e6899a262773..4aec63f0b8ed 100644 --- a/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx +++ b/src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx @@ -11,7 +11,6 @@ import type {TransactionListItemProps, TransactionListItemType} from '@component import useLiveRowCapabilities from '@components/Search/SearchList/ListItem/useLiveRowCapabilities'; import type {ListItem} from '@components/SelectionList/types'; -import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import useConfirmModal from '@hooks/useConfirmModal'; import useConfirmSubmitReportViolations from '@hooks/useConfirmSubmitReportViolations'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; @@ -225,7 +224,6 @@ function TransactionListItemInner({ const transactionViolations = mergeProhibitedViolations(rateOnyxViolations); const filteredViolationsCollection: OnyxCollection = {[transactionViolationsKey]: transactionViolations}; - const confirmApproveReportViolations = useConfirmApproveReportViolations({report: reportForViolations, policy: policyForViolations}); const confirmSubmitReportViolations = useConfirmSubmitReportViolations({ reportID: transactionItem.reportID, report: reportForViolations, @@ -272,7 +270,6 @@ function TransactionListItemInner({ onPendingCardTransactionsBlock: () => showPendingCardTransactionsBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), onAllHeldExpensesBlock: () => showHeldExpensesBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy), confirmSubmitReportViolations, - confirmApproveReportViolations, currentUserAccountID, currentUserLogin, activePolicy, diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index 1dc2d9244702..80c290f8a745 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -277,7 +277,7 @@ type HandleActionButtonPressParams = { onAllHeldExpensesBlock?: () => void; /** Confirms any unacknowledged report violations before proceeding with the SUBMIT action. */ confirmSubmitReportViolations: (onProceed: ConfirmSubmitReportViolationsOnProceed) => void; - confirmApproveReportViolations: (onProceed: () => void) => void; + confirmApproveReportViolations?: (onProceed: () => void) => void; openReportSubmitToPopover?: (options?: ReportSubmitToPopoverOpenOptions) => void; currentUserAccountID: number; currentUserLogin?: string; @@ -410,7 +410,7 @@ function handleActionButtonPress({ Navigation.navigate(ROUTES.RESTRICTED_ACTION.getRoute(snapshotReport.policyID)); return; } - confirmApproveReportViolations(() => + confirmApproveReportViolations?.(() => getApproveActionCallback({ hash, item, diff --git a/tests/unit/Search/TransactionListItemViolationsTest.tsx b/tests/unit/Search/TransactionListItemViolationsTest.tsx index 06dca1f672b5..613743886530 100644 --- a/tests/unit/Search/TransactionListItemViolationsTest.tsx +++ b/tests/unit/Search/TransactionListItemViolationsTest.tsx @@ -6,6 +6,7 @@ import OnyxListItemProvider from '@components/OnyxListItemProvider'; import type * as SearchContextModule from '@components/Search/SearchContext'; import TransactionListItem from '@components/Search/SearchList/ListItem/TransactionListItem'; import type {TransactionListItemType} from '@components/Search/SearchList/ListItem/types'; +import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -25,6 +26,11 @@ const TRANSACTION_VIOLATIONS_KEY = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS const mockSearchResults: {current: {data: Record} | undefined} = {current: undefined}; +jest.mock('@hooks/useConfirmApproveReportViolations', () => ({ + __esModule: true, + default: jest.fn(), +})); + jest.mock('@components/Search/SearchContext', () => ({ ...jest.requireActual('@components/Search/SearchContext'), useSearchQueryContext: () => ({currentSearchHash: 1, currentSearchKey: undefined}), @@ -125,4 +131,17 @@ describe('TransactionListItem violations', () => { expect(screen.getByText(CONST.VIOLATIONS.OVER_LIMIT)).toBeTruthy(); expect(screen.queryByText(CONST.VIOLATIONS.MISSING_TAG)).toBeNull(); }); + + it('does not initialize report approval confirmation for a transaction row', async () => { + // Given transaction rows only offer View, so report approval data is unnecessary + jest.mocked(useConfirmApproveReportViolations).mockClear(); + + // When a transaction row with violations renders + renderRow(); + await waitForBatchedUpdatesWithAct(); + + // Then the row does not initialize the report-wide approval hook + expect(useConfirmApproveReportViolations).not.toHaveBeenCalled(); + expect(screen.getByText(CONST.VIOLATIONS.MISSING_TAG)).toBeTruthy(); + }); }); From 7a106c45f1f28959a3a119136864f1fc278d261d Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Sat, 7 Nov 2026 04:37:12 +0000 Subject: [PATCH 4/7] Separate hook imports in transaction row regression test Co-authored-by: Nguyen Van Duc --- tests/unit/Search/TransactionListItemViolationsTest.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/Search/TransactionListItemViolationsTest.tsx b/tests/unit/Search/TransactionListItemViolationsTest.tsx index 613743886530..9f28678ab789 100644 --- a/tests/unit/Search/TransactionListItemViolationsTest.tsx +++ b/tests/unit/Search/TransactionListItemViolationsTest.tsx @@ -6,6 +6,7 @@ import OnyxListItemProvider from '@components/OnyxListItemProvider'; import type * as SearchContextModule from '@components/Search/SearchContext'; import TransactionListItem from '@components/Search/SearchList/ListItem/TransactionListItem'; import type {TransactionListItemType} from '@components/Search/SearchList/ListItem/types'; + import useConfirmApproveReportViolations from '@hooks/useConfirmApproveReportViolations'; import CONST from '@src/CONST'; From 793e96045dfa77401d4dd84f06b8daf69007837b Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Sat, 7 Nov 2026 06:20:36 +0000 Subject: [PATCH 5/7] Respect approver dismissals in approval warnings Co-authored-by: Nguyen Van Duc --- .../getReportApproveViolationSummary.ts | 32 +++++-- .../getReportSubmitViolationSummaryTest.ts | 96 +++++++++++++++++++ 2 files changed, 118 insertions(+), 10 deletions(-) diff --git a/src/libs/Violations/getReportApproveViolationSummary.ts b/src/libs/Violations/getReportApproveViolationSummary.ts index 0690c5532ded..89a91099f507 100644 --- a/src/libs/Violations/getReportApproveViolationSummary.ts +++ b/src/libs/Violations/getReportApproveViolationSummary.ts @@ -1,6 +1,7 @@ /** Builds approval warnings from local data without the submitter's rejection or card-match warnings. */ -import {isOnHold} from '@libs/TransactionUtils'; +import {getTransactionViolations, isOnHold} from '@libs/TransactionUtils'; +import ONYXKEYS from '@src/ONYXKEYS'; import type {Policy, Report, Transaction, TransactionViolations} from '@src/types/onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; @@ -16,15 +17,26 @@ function getReportApproveViolationSummary( currentUserAccountID: number, full = true, ) { - const summary = getReportSubmitViolationSummary( - transactions.filter((transaction) => transaction && (full || !isOnHold(transaction))), - violationsCollection, - report, - policy, - currentUserEmail, - currentUserAccountID, - full, - ); + const approvalTransactions: Transaction[] = []; + const approvalViolations: OnyxCollection = {}; + for (const transaction of transactions) { + if (!transaction?.transactionID || (!full && isOnHold(transaction))) { + continue; + } + approvalTransactions.push(transaction); + // Raw overrides must respect the approver's dismissals before warnings are classified or deduplicated. + approvalViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`] = getTransactionViolations( + transaction, + violationsCollection, + currentUserEmail, + currentUserAccountID, + report, + undefined, + policy, + ); + } + + const summary = getReportSubmitViolationSummary(approvalTransactions, approvalViolations, report, policy, currentUserEmail, currentUserAccountID, full); // These warnings require acknowledgement by the submitter, not the approver. return {...summary, hasRejectedExpense: false, hasReportBeenRejected: false, hasPendingCardMatch: false}; diff --git a/tests/unit/getReportSubmitViolationSummaryTest.ts b/tests/unit/getReportSubmitViolationSummaryTest.ts index a7690a9e10ce..240e2d64c772 100644 --- a/tests/unit/getReportSubmitViolationSummaryTest.ts +++ b/tests/unit/getReportSubmitViolationSummaryTest.ts @@ -22,6 +22,8 @@ const translate = jest.fn((key: string, ...params: unknown[]) => { }) as unknown as LocaleContextProps['translate']; const convertToDisplayString = jest.fn((amount: number | undefined, currency: string | undefined) => `${amount} ${currency}`); +const APPROVER_EMAIL = 'approver@example.com'; +const SUBMITTER_EMAIL = 'submitter@example.com'; // Not exercised by the violation types covered here (overCategoryLimit doesn't need a formatted date), but // required by buildSubmitViolationBullets's params shape. @@ -71,6 +73,100 @@ function createMockRejectedReport(): Report { } describe('getReportApproveViolationSummary', () => { + it('omits raw violations dismissed by the approver without modifying the source data', () => { + // Given raw violations from a header, hold menu or bulk approval input + const transaction = createMockTransactionWithMerchant('Merchant', { + comment: {dismissedViolations: {[CONST.VIOLATIONS.OVER_CATEGORY_LIMIT]: {[APPROVER_EMAIL]: '2026-11-07 00:00:00'}}}, + }); + const violation = createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT); + const violations = {[getViolationsKey('1')]: [violation]}; + + // When the current approver has already dismissed the only warning + const summary = getReportApproveViolationSummary([transaction], violations, undefined, undefined, APPROVER_EMAIL, 1); + + // Then approval needs no warning and the dismissal remains local to this summary + expect(hasAnySubmitViolation(summary)).toBe(false); + expect(violations[getViolationsKey('1')]).toEqual([violation]); + expect(transaction.comment?.dismissedViolations?.[CONST.VIOLATIONS.OVER_CATEGORY_LIMIT]).toEqual({[APPROVER_EMAIL]: '2026-11-07 00:00:00'}); + }); + + it('still warns the approver about violations dismissed only by the submitter', () => { + // Given a submitted report whose owner dismissed a violation, but its approver did not + const transaction = createMockTransactionWithMerchant('Merchant', { + comment: {dismissedViolations: {[CONST.VIOLATIONS.OVER_CATEGORY_LIMIT]: {[SUBMITTER_EMAIL]: '2026-11-07 00:00:00'}}}, + }); + const violation = createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT); + const report = createMock({...createMockExpenseReport(), stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, ownerAccountID: 2, managerID: 1}); + + // When a different user approves the report + const summary = getReportApproveViolationSummary([transaction], {[getViolationsKey('1')]: [violation]}, report, undefined, APPROVER_EMAIL, 1); + + // Then the submitter's dismissal does not hide the warning from the approver + expect(summary.otherViolations.get(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT)).toBe(violation); + }); + + it('deduplicates only after filtering each transaction dismissal', () => { + // Given two violations with the same name, only the first expense's instance is dismissed + const dismissedTransaction = createMockTransactionWithMerchant('Merchant', { + comment: {dismissedViolations: {[CONST.VIOLATIONS.OVER_CATEGORY_LIMIT]: {[APPROVER_EMAIL]: '2026-11-07 00:00:00'}}}, + }); + const visibleTransaction = createMockTransactionWithMerchant('Merchant', {transactionID: '2'}); + const dismissedViolation = createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT, {amount: 1000, currency: 'USD'}); + const visibleViolation = createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT, {amount: 2000, currency: 'USD'}); + + // When both expenses are summarized in report order + const summary = getReportApproveViolationSummary( + [dismissedTransaction, visibleTransaction], + {[getViolationsKey('1')]: [dismissedViolation], [getViolationsKey('2')]: [visibleViolation]}, + undefined, + undefined, + APPROVER_EMAIL, + 1, + ); + + // Then the remaining warning retains the second expense's message data + expect(summary.otherViolations.size).toBe(1); + expect(summary.otherViolations.get(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT)).toBe(visibleViolation); + }); + + it('keeps missing-field warnings when an unrelated server violation is dismissed', () => { + // Given an expense missing its merchant with a dismissed policy violation + const transaction = createMockTransactionWithMerchant(CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, { + comment: {dismissedViolations: {[CONST.VIOLATIONS.OVER_CATEGORY_LIMIT]: {[APPROVER_EMAIL]: '2026-11-07 00:00:00'}}}, + }); + + // When the approver reviews the raw violations + const summary = getReportApproveViolationSummary( + [transaction], + {[getViolationsKey('1')]: [createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT)]}, + createMockExpenseReport(), + undefined, + APPROVER_EMAIL, + 1, + ); + + // Then only the independent missing merchant warning remains + expect(summary.otherViolations.size).toBe(0); + expect(summary.hasMissingMerchant).toBe(true); + expect(hasAnySubmitViolation(summary)).toBe(true); + }); + + it('excludes dismissed violations during both full and partial approval', () => { + // Given a held expense whose only violation the approver dismissed + const transaction = createMockTransactionWithMerchant('Merchant', { + comment: {hold: 'hold-action', dismissedViolations: {[CONST.VIOLATIONS.OVER_CATEGORY_LIMIT]: {[APPROVER_EMAIL]: '2026-11-07 00:00:00'}}}, + }); + const violations = {[getViolationsKey('1')]: [createMockViolation(CONST.VIOLATIONS.OVER_CATEGORY_LIMIT)]}; + + // When full approval includes the held expense and partial approval excludes it + const full = getReportApproveViolationSummary([transaction], violations, undefined, undefined, APPROVER_EMAIL, 1, true); + const partial = getReportApproveViolationSummary([transaction], violations, undefined, undefined, APPROVER_EMAIL, 1, false); + + // Then neither mode warns about the dismissed violation + expect(hasAnySubmitViolation(full)).toBe(false); + expect(hasAnySubmitViolation(partial)).toBe(false); + }); + it('warns about a held expense with a missing merchant when approving everything', () => { // Given a held expense whose missing merchant is not represented by a server violation const transaction = createMockTransactionWithMerchant(CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, {comment: {hold: 'hold-action'}}); From 84ec42d92e55f7b0d019d993af22ca9044826afa Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Mon, 9 Nov 2026 01:57:29 +0000 Subject: [PATCH 6/7] Document Search approval confirmation callback Co-authored-by: Nguyen Van Duc --- src/libs/actions/Search.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/Search.ts b/src/libs/actions/Search.ts index 80c290f8a745..43e2128897ec 100644 --- a/src/libs/actions/Search.ts +++ b/src/libs/actions/Search.ts @@ -277,6 +277,7 @@ type HandleActionButtonPressParams = { onAllHeldExpensesBlock?: () => void; /** Confirms any unacknowledged report violations before proceeding with the SUBMIT action. */ confirmSubmitReportViolations: (onProceed: ConfirmSubmitReportViolationsOnProceed) => void; + /** Confirms report violations before the APPROVE action proceeds; approval is skipped if omitted. */ confirmApproveReportViolations?: (onProceed: () => void) => void; openReportSubmitToPopover?: (options?: ReportSubmitToPopoverOpenOptions) => void; currentUserAccountID: number; From 57a6c9d4a0e0b0ac15c3749b06e273f7214d9aee Mon Sep 17 00:00:00 2001 From: "Nguyen Van Duc (via MelvinBot)" Date: Thu, 12 Nov 2026 02:59:23 +0000 Subject: [PATCH 7/7] Repair approval button test fixtures and apply translations Co-authored-by: Nguyen Van Duc --- src/languages/de.ts | 2 +- src/languages/el.ts | 2 +- src/languages/es.ts | 4 +- src/languages/fr.ts | 2 +- src/languages/it.ts | 4 +- src/languages/ja.ts | 4 +- src/languages/nl.ts | 2 +- src/languages/pl.ts | 2 +- src/languages/pt-BR.ts | 2 +- src/languages/zh-hans.ts | 2 +- .../ui/components/ApproveActionButtonTest.tsx | 64 ++++++++++++++++++- 11 files changed, 76 insertions(+), 14 deletions(-) diff --git a/src/languages/de.ts b/src/languages/de.ts index b5307bd73274..b9b97d99d7e6 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -1539,7 +1539,7 @@ const translations: TranslationDeepObject = { markAsCash: 'Als Bar markieren', confirmApproveReportViolations: { title: 'Trotzdem genehmigen?', - description: 'Dieser Bericht enthält Richtlinienverstöße. Möchtest du ihn wirklich ohne Änderungen genehmigen?', + description: 'Dieser Bericht enthält Richtlinienverstöße. Sind Sie sicher, dass Sie ihn ohne Änderungen genehmigen möchten?', }, confirmSubmitReportViolations: { title: 'Bericht einreichen?', diff --git a/src/languages/el.ts b/src/languages/el.ts index 093ca1fd5d6c..07856fc5dc5a 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -1585,7 +1585,7 @@ const translations: TranslationDeepObject = { markAsCash: 'Σήμανση ως μετρητά', confirmApproveReportViolations: { title: 'Έγκριση ούτως ή άλλως;', - description: 'Αυτή η αναφορά έχει παραβιάσεις πολιτικής. Θέλετε σίγουρα να την εγκρίνετε χωρίς αλλαγές;', + description: 'Αυτή η αναφορά έχει παραβιάσεις πολιτικής. Είστε βέβαιοι ότι θέλετε να την εγκρίνετε χωρίς να κάνετε αλλαγές;', }, confirmSubmitReportViolations: { title: 'Υποβολή αναφοράς;', diff --git a/src/languages/es.ts b/src/languages/es.ts index d97d8caa2d98..951826149658 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -1512,8 +1512,8 @@ const translations: TranslationDeepObject = { pendingMatchWithCreditCardDescription: 'Recibo pendiente de adjuntar con la transacción de la tarjeta. Márcalo como efectivo para cancelar.', markAsCash: 'Marcar como efectivo', confirmApproveReportViolations: { - title: '¿Aprobar de todos modos?', - description: 'Este informe tiene infracciones de la política. ¿Seguro que quieres aprobarlo sin realizar cambios?', + title: '¿Aprobar de todas formas?', + description: 'Este informe tiene infracciones de la política. ¿Seguro que quieres aprobarlo sin hacer cambios?', }, confirmSubmitReportViolations: { title: '¿Enviar informe?', diff --git a/src/languages/fr.ts b/src/languages/fr.ts index ec6aa1690e73..1bb98cb1337d 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -1547,7 +1547,7 @@ const translations: TranslationDeepObject = { markAsCash: 'Marquer comme espèces', confirmApproveReportViolations: { title: 'Approuver quand même ?', - description: 'Ce rapport contient des violations de la politique. Voulez-vous vraiment approuver ce rapport sans le modifier ?', + description: 'Cette note de frais comporte des violations de politique. Voulez-vous vraiment l’approuver sans y apporter de modifications ?', }, confirmSubmitReportViolations: { title: 'Soumettre le rapport ?', diff --git a/src/languages/it.ts b/src/languages/it.ts index 63961583b716..82881a14d7d0 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -1537,8 +1537,8 @@ const translations: TranslationDeepObject = { pendingMatchWithCreditCardDescription: 'Ricevuta in attesa di abbinamento con la transazione della carta. Contrassegna come contante per annullare.', markAsCash: 'Segna come contante', confirmApproveReportViolations: { - title: 'Approvare comunque?', - description: 'Questo report presenta violazioni delle regole. Vuoi davvero approvarlo senza apportare modifiche?', + title: 'Approvi comunque?', + description: 'Questo report presenta violazioni della policy. Vuoi davvero approvarlo senza apportare modifiche?', }, confirmSubmitReportViolations: { title: 'Invia il rapporto?', diff --git a/src/languages/ja.ts b/src/languages/ja.ts index d6b181f2e333..8ca36d64e0cf 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -103,7 +103,7 @@ const translations: TranslationDeepObject = { saveChanges: '変更を保存', submit: '送信', submitAnyway: 'とにかく送信', - approveAnyway: 'それでも承認', + approveAnyway: 'このまま承認する', markAsDone: '完了にする', markAsDoneAnyway: 'とにかく完了にする', submitted: '送信済み', @@ -1521,7 +1521,7 @@ const translations: TranslationDeepObject = { markAsCash: '現金としてマーク', confirmApproveReportViolations: { title: 'それでも承認しますか？', - description: 'このレポートにはポリシー違反があります。変更せずに承認してもよろしいですか？', + description: 'このレポートにはポリシー違反があります。変更を行わずに承認してもよろしいですか？', }, confirmSubmitReportViolations: { title: 'レポートを送信しますか？', diff --git a/src/languages/nl.ts b/src/languages/nl.ts index c7181797ba98..f47725847ba3 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -1536,7 +1536,7 @@ const translations: TranslationDeepObject = { markAsCash: 'Markeren als contant', confirmApproveReportViolations: { title: 'Toch goedkeuren?', - description: 'Dit rapport bevat beleidsovertredingen. Weet je zeker dat je het zonder wijzigingen wilt goedkeuren?', + description: 'Dit rapport bevat beleidsinbreuken. Weet je zeker dat je het wilt goedkeuren zonder wijzigingen aan te brengen?', }, confirmSubmitReportViolations: { title: 'Rapport verzenden?', diff --git a/src/languages/pl.ts b/src/languages/pl.ts index 020a0123e2a2..1ede60c63771 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -1576,7 +1576,7 @@ const translations: TranslationDeepObject = { markAsCash: 'Oznacz jako gotówkę', confirmApproveReportViolations: { title: 'Zatwierdzić mimo to?', - description: 'Ten raport zawiera naruszenia zasad. Czy na pewno chcesz go zatwierdzić bez wprowadzania zmian?', + description: 'Ten raport zawiera naruszenia zasad. Na pewno chcesz go zatwierdzić bez wprowadzania zmian?', }, confirmSubmitReportViolations: { title: 'Wysłać raport?', diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index c067baf40e15..e9b9862ab29b 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -1538,7 +1538,7 @@ const translations: TranslationDeepObject = { markAsCash: 'Marcar como dinheiro', confirmApproveReportViolations: { title: 'Aprovar mesmo assim?', - description: 'Este relatório tem violações da política. Tem certeza de que deseja aprová-lo sem fazer alterações?', + description: 'Este relatório tem violações da política. Tem certeza de que quer aprová‑lo sem fazer alterações?', }, confirmSubmitReportViolations: { title: 'Enviar relatório?', diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index dd8eab370c9e..f23d912e44ec 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -1474,7 +1474,7 @@ const translations: TranslationDeepObject = { markAsCash: '标记为现金', confirmApproveReportViolations: { title: '仍然批准？', - description: '此报告存在违反政策的情况。确定要在不作更改的情况下批准吗？', + description: '此报表存在策略违规。确定要在不进行任何更改的情况下批准吗？', }, confirmSubmitReportViolations: { title: '提交报告？', diff --git a/tests/ui/components/ApproveActionButtonTest.tsx b/tests/ui/components/ApproveActionButtonTest.tsx index cef504960335..7dff002cd4e2 100644 --- a/tests/ui/components/ApproveActionButtonTest.tsx +++ b/tests/ui/components/ApproveActionButtonTest.tsx @@ -1,8 +1,11 @@ import {act, render} from '@testing-library/react-native'; +import {ModalActions} from '@components/Modal/Global/ModalContext'; import ApproveActionButton from '@components/ReportActionItem/MoneyRequestReportPreview/ApproveActionButton'; +import type useConfirmModal from '@hooks/useConfirmModal'; import useOnyx from '@hooks/useOnyx'; +import useTransactionsAndViolationsForReport from '@hooks/useTransactionsAndViolationsForReport'; import {hasHeldExpensesFromTransactions} from '@libs/ReportUtils'; @@ -10,13 +13,18 @@ import {approveMoneyRequest} from '@userActions/IOU/ReportWorkflow'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Report, TransactionViolations} from '@src/types/onyx'; +import type {Report, Transaction, TransactionViolations} from '@src/types/onyx'; import type {OnyxCollection, UseOnyxResult} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; import React from 'react'; +import createMock from '../../utils/createMock'; + +type ShowConfirmModal = ReturnType['showConfirmModal']; +type ModalResult = Awaited>; + const TEST_IOU_REPORT_ID = '1001'; const iouReport = { @@ -108,14 +116,32 @@ jest.mock('@hooks/useReportTransactionsCollection', () => ({ default: jest.fn(() => ({})), })); jest.mock('@hooks/useOnyx', () => jest.fn()); +jest.mock('@hooks/useTransactionsAndViolationsForReport', () => ({ + __esModule: true, + default: jest.fn(), +})); + +const mockShowConfirmModal = jest.fn, Parameters>(); +jest.mock('@hooks/useConfirmModal', () => ({ + __esModule: true, + default: () => ({showConfirmModal: mockShowConfirmModal}), +})); +// Keep modal presentation synchronous here; transition timing has separate coverage. +jest.mock('@libs/showConfirmModalAfterMoreMenuDismiss', () => ({ + __esModule: true, + default: (showConfirmModal: ShowConfirmModal, options: Parameters[0]) => showConfirmModal(options), +})); const mockedUseOnyx = jest.mocked(useOnyx); +const mockedUseTransactionsAndViolationsForReport = jest.mocked(useTransactionsAndViolationsForReport); const mockedApproveMoneyRequest = jest.mocked(approveMoneyRequest); const mockedHasHeldExpenses = jest.mocked(hasHeldExpensesFromTransactions); describe('ApproveActionButton', () => { beforeEach(() => { jest.clearAllMocks(); + mockedUseTransactionsAndViolationsForReport.mockReturnValue({transactions: {}, violations: {}, isLoaded: true}); + mockShowConfirmModal.mockReset().mockResolvedValue({action: ModalActions.CLOSE}); mockApprovalButtonProps.current = undefined; mockReportPreviewAction = CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE; mockIsDelegateAccessRestricted = false; @@ -211,4 +237,40 @@ describe('ApproveActionButton', () => { expect(mockShowDelegateNoAccessModal).toHaveBeenCalled(); expect(mockedApproveMoneyRequest).not.toHaveBeenCalled(); }); + + it.each([ModalActions.CLOSE, ModalActions.CONFIRM])('waits for the violation modal and handles %s without bypassing confirmation', async (action) => { + // Given a report with a visible warning and a modal that has not been answered yet + const transaction = createMock({transactionID: '1', reportID: TEST_IOU_REPORT_ID, merchant: 'Merchant', amount: 1000, currency: 'USD'}); + mockedUseTransactionsAndViolationsForReport.mockReturnValue({ + transactions: {[`${ONYXKEYS.COLLECTION.TRANSACTION}1`]: transaction}, + violations: { + [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}1`]: [{name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION, type: CONST.VIOLATION_TYPES.WARNING}], + }, + isLoaded: true, + }); + let resolveModal: ((result: ModalResult) => void) | undefined; + mockShowConfirmModal.mockReturnValue( + new Promise((resolve) => { + resolveModal = resolve; + }), + ); + render(); + + // When approval is requested, it must wait for the user's response + act(() => mockApprovalButtonProps.current?.onApprove(true)); + expect(mockShowConfirmModal).toHaveBeenCalledWith(expect.objectContaining({confirmText: 'common.approveAnyway'})); + expect(mockedApproveMoneyRequest).not.toHaveBeenCalled(); + await act(async () => { + resolveModal?.({action}); + }); + + // Then closing the warning cancels approval, and explicit confirmation approves exactly once + if (action === ModalActions.CONFIRM) { + expect(mockedApproveMoneyRequest).toHaveBeenCalledTimes(1); + expect(mockedApproveMoneyRequest).toHaveBeenCalledWith(expect.objectContaining({expenseReport: iouReport, full: true, onApproved: mockStartApprovedAnimation})); + } else { + expect(mockedApproveMoneyRequest).not.toHaveBeenCalled(); + expect(mockStartApprovedAnimation).not.toHaveBeenCalled(); + } + }); });