Skip to content

Show an Approve anyway confirmation for report violations #106274

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
MelvinBot wants to merge 10 commits into
base: main
Choose a base branch
from
+1,099 −215
Open

Show an Approve anyway confirmation for report violations #106274

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a4b5c79
Show a confirmation before approving reports with violations
MelvinBot Nov 7, 2026
c5fa640
Correct approval warning test fixtures and Search mocks
MelvinBot Nov 7, 2026
05fa661
Remove unused transaction row approval confirmation
MelvinBot Nov 7, 2026
7a106c4
Separate hook imports in transaction row regression test
MelvinBot Nov 7, 2026
793e960
Respect approver dismissals in approval warnings
MelvinBot Nov 7, 2026
84ec42d
Document Search approval confirmation callback
MelvinBot Nov 9, 2026
c13ec58
Merge main into approval warning branch
MelvinBot Nov 12, 2026
57a6c9d
Repair approval button test fixtures and apply translations
MelvinBot Nov 12, 2026
cb83f22
Merge main and warn before approving snapshot-only reports
MelvinBot Nov 13, 2026
4860181
Merge latest main into approval warning branch
MelvinBot Nov 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 28 additions & 24 deletions src/components/MoneyReportHeaderPrimaryAction/useConfirmApproval.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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});

@chatgpt-codex-connector chatgpt-codex-connector Bot Nov 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Filter dismissed violations before approval prompts

Pass the user-visible, filtered violation collection to the confirmation instead of the raw Onyx entries. When a violation has been dismissed through transaction.comment.dismissedViolations, getReportSubmitViolationSummary() only applies shouldShowViolation() and therefore adds the dismissed violation to the modal, even though useTransactionsAndViolationsForReport() normally removes it. This makes the report-header and Search approval buttons repeatedly warn about violations the user has already dismissed; the hold-menu and bulk-approval paths similarly pass raw collections and need the same filtering.

Useful? React with 👍 / 👎.

@dukenv0307 dukenv0307 Nov 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make sense. @MelvinBot pls fix

This comment was marked as off-topic.

Sign in to view


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
Expand Down
44 changes: 24 additions & 20 deletions src/components/ReportActionItem/MoneyRequestReportPreview/useConfirmApproveReportAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -34,31 +35,34 @@ function useConfirmApproveReportAction(actionButtonData: ReturnType<typeof useRe
const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE);

const {iouReport, policy, ownerLogin, userBillingGracePeriodEnds, amountOwed, ownerBillingGracePeriodEnd, delegateEmail, delegateAccountID} = actionButtonData;
const confirmApproveReportViolations = useConfirmApproveReportViolations({report: iouReport, policy});

return (full = true) => {
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);
}
};
}
Expand Down
8 changes: 8 additions & 0 deletions src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -219,6 +220,12 @@ function ExpenseReportListItemInner<TItem extends ListItem>({
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,

@Julesssss Julesssss Nov 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The row and 'Approve anyway?' modal can be different. If a report hasn't been opened yet, its expenses only come from the Search results, not from what's loaded locally. The row still shows the violation, but the modal only checks the loaded expenses, finds none, and approves straight away with no warning.

ReportListItemHeader has the same issue. Bulk approve already checks both, so could we do the same here and add a test for that case?

@dukenv0307 dukenv0307 Nov 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make sense. @MelvinBot Merge main + fix

@MelvinBot MelvinBot Nov 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Web test steps verification results: ❌ fail

Merged latest main and pushed the snapshot-warning fix in 4860181d, covering report rows, grouped headers, wide-layout approval, and bulk approval; 162 focused tests and static checks passed. Full Jest timed out, and browser verification was blocked by an unavailable Web session.

Step Status
Snapshot-only report row warns before approvalBrowser blocked; regression tests passed.
 ⚠️
Grouped report header warns before approvalBrowser blocked; regression tests passed.
 ⚠️
Cancel preserves approval state and selectionBrowser blocked; cancellation tests passed.
 ⚠️
Wide and bulk approval warn consistentlyBrowser blocked; regression tests passed.
 ⚠️

view run · no recording available

violationsCollection: reportViolations,
});
const confirmSubmitReportViolations = useConfirmSubmitReportViolations({
reportID: reportItem.reportID,
report: reportForViolations,
Expand Down Expand Up @@ -276,6 +283,7 @@ function ExpenseReportListItemInner<TItem extends ListItem>({
onDelegateAccessRestricted: showDelegateNoAccessModal,
personalPolicyID,
confirmSubmitReportViolations,
confirmApproveReportViolations,
ownerBillingGracePeriodEnd,
openReportSubmitToPopover,
onPendingCardTransactionsBlock: () => showPendingCardTransactionsBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy),
Expand Down
3 changes: 3 additions & 0 deletions src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -289,6 +290,7 @@ function ReportListItemHeaderInner<TItem extends ListItem>({
isTrackIntentUser,
rules,
});
const confirmApproveReportViolations = useConfirmApproveReportViolations({report: parentReport ?? snapshotReport, policy: parentPolicy});
const confirmSubmitReportViolations = useConfirmSubmitReportViolations({
reportID: snapshotReport?.reportID ?? reportItem.reportID,
report: parentReport ?? snapshotReport,
Expand Down Expand Up @@ -340,6 +342,7 @@ function ReportListItemHeaderInner<TItem extends ListItem>({
personalPolicyID,
ownerBillingGracePeriodEnd,
confirmSubmitReportViolations,
confirmApproveReportViolations,
openReportSubmitToPopover,
onPendingCardTransactionsBlock: () => showPendingCardTransactionsBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy),
onAllHeldExpensesBlock: () => showHeldExpensesBlockModal(showConfirmModal, translate, shouldShowMarkAsDoneCopy),
Expand Down
43 changes: 43 additions & 0 deletions src/hooks/useConfirmApproveReportViolations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<Report>;
policy: OnyxEntry<Policy>;
transactions?: Array<OnyxEntry<Transaction>>;
violationsCollection?: OnyxCollection<TransactionViolations>;
};

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;
49 changes: 29 additions & 20 deletions src/hooks/useHoldMenuSubmit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down
67 changes: 36 additions & 31 deletions src/hooks/useLifecycleActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading