[Due for payment 2026-05-26] [$250] Held Expense with Multiple Violations Is Automatically Unheld and Approved Without Prompt

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number:
Reproducible in staging?: Needs Reproduction (Unable to reproduce)
Reproducible in production?: Needs reproduction
If this was caught during regression testing, add the test name, ID and link from BrowserStack:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @rafecolton
Slack conversation (hyperlinked to channel name): #Product

Action Performed:

1. Create a report with multiple card expenses
2. Ensure at least one expense has multiple violations
3. Put that expense on hold
4. Approve the report

Expected Result:

User should be prompted with options:
Approve all expenses, OR
Approve only unheld expenses and move held expenses to a new report

Actual Result:

Held expense is:
Automatically unheld
Approved without prompt

MelvinBot response:

I investigated the approval flow for held expenses. Here's what I found:

TL;DR:​ The most likely cause is a client-side race condition where another violation action on the same expense overwrites the hold field in Onyx state before the approval check runs. When the client doesn't detect the hold, it sends full=true to the server, which auto-unholds everything — no prompt shown.

How the flow works:

1.  When you hold an expense, putOnHold sets transaction.comment.hold via a safe Onyx deep-merge
2.  When you approve, the client checks hasHeldExpenses() which reads transaction.comment.hold from Onyx
3.  If held expenses are detected → hold menu modal shown (approve non-held only vs approve all)
4.  If ​not​ detected → approveMoneyRequest is called with full=true hardcoded
5.  Server receives full=true → handleHeldTransactions auto-unholds everything. The server defaults full to true when not explicitly set.

Why "multiple violations on the same expense" matters:
Several code paths that deal with violations spread the full transaction.comment object into optimistic Onyx updates (e.g., dismissDuplicateTransactionViolation). This captures a ​snapshot​ of the comment at call time. If the hold was set concurrently or the snapshot was taken before the hold Onyx merge was processed, the spread overwrites comment.hold with a stale value (null/empty), making hasHeldExpenses() return false on the next check.

Card vs manual:​ No difference — the hold/approval logic treats both identically.

To confirm:​ We'd need the specific reportID to check server logs for whether ApproveMoneyRequest received full=true and whether "Held transactions found, unholding them" was logged. If rafecolton can share the reportID, I can check VictoriaLogs to verify the exact sequence of events.

Workaround:

Unknown

Platforms:

Select the officially supported platforms where the issue was reproduced:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome / Safari

Screenshots/Videos

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @huult

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022050604818847892860
• Upwork Job ID: 2050604818847892860
• Last Price Increase: 2026-05-09

[exfy-zapier]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[melvin-bot]

📣 @exfy-zapier! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[MelvinBot]

Proposal

What is the root cause of that problem?

When a user approves a report, the client checks hasHeldExpenses which reads transaction.comment.hold from Onyx via isOnHold. If no held expenses are detected, useConfirmApproval skips the hold prompt and calls approveMoneyRequest with full: true, which tells the server to auto-unhold everything.

The root cause is that dismissDuplicateTransactionViolation spreads the entire transaction object (including comment) into an optimistic Onyx update:

value: {
    ...transaction,
    comment: {
        ...transaction?.comment,
        dismissedViolations: { ... },
    },
},

The transaction variable comes from a React state snapshot (allTransactions via useOnyx) captured at render time. If the user put an expense on hold and then a violation action fires (e.g. dismissing a duplicate violation) before the Onyx state snapshot refreshes, the spread writes back a stale comment object without the hold field — effectively removing the hold silently. The same issue exists in the failure data path which also spreads the stale transaction.

This is why "multiple violations" is the trigger — it increases the chance of a violation-related optimistic update racing with the hold.

What changes do you think we should make in order to solve the problem?

Stop spreading the full transaction object in optimistic updates. In dismissDuplicateTransactionViolation (Transaction.ts:611-625), the optimistic update should only write the fields it intends to change (comment.dismissedViolations) rather than spreading the entire transaction and comment. This prevents overwriting unrelated fields like hold:

value: {
    comment: {
        dismissedViolations: {
            duplicatedTransaction: {
                [login]: getUnixTime(new Date()),
            },
        },
    },
},

The same fix should be applied to the failure data (Transaction.ts:635-641) — either remove the stale spread or only restore the specific fields that need reverting.

What alternative solutions did you explore? (Optional)

1. Deep-merge in Onyx — Changing Onyx.METHOD.MERGE to deep-merge nested objects would also prevent the overwrite, but this has broader implications across the codebase and could introduce other subtle behavior changes.
2. Server-side guard — The server could reject full=true when held transactions exist, but this would just mask the client-side data integrity issue and could break legitimate "approve all" flows.

Investigation details

Evidence chain:

1. Hold is set via putOnHold writing {comment: {hold: reportActionID}} to Onyx
2. dismissDuplicateTransactionViolation receives allTransactions from a React state snapshot — if captured before hold was applied, the snapshot is stale
3. The stale spread overwrites comment in Onyx, removing the hold field
4. useConfirmApproval reads isAnyTransactionOnHold (line 32), finds false, and sends full: true (line 57)
5. Server receives full=true and auto-unholds all transactions

Related open issues:

• Expensify/App#88355 — Rejected and Held Expenses Not Handled Correctly During Approval Flow
• Expensify/App#87971 — Approval Modal Shows Incorrect Amounts and Inconsistent State for Reports with Held Expenses

Additional affected code paths: changeTransactionsReport also spreads the full transaction into optimistic updates, which could cause the same class of bug.

---

Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, @MelvinBot <your feedback> to refine this analysis, or explain why you are rejecting Melvin's proposal.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @huult (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022050604818847892860

[samranahm]

Proposal

What is the root cause of that problem?

In dismissDuplicateTransactionViolation (Transaction.ts), the optimistic Onyx MERGE for each transaction spreads the entire ...transaction snapshot into the update value:

value: { ...transaction, comment: { ...transaction?.comment, dismissedViolations: {...} } }

This means ALL transaction fields from the snapshot are included in the MERGE, including comment.hold. If the snapshot was captured before the expense was put on hold (or when hold was null), the MERGE will explicitly write comment.hold = null to Onyx, overwriting the actual hold state. The failure/rollback data has the same issue: value: { ...transaction } also spreads the stale snapshot, so even after a failure rollback, the hold gets cleared.

Once comment.hold is set to null in Onyx, hasHeldExpenses(reportID) returns false, useConfirmApproval computes isAnyTransactionOnHold = false, and the approval skips the hold prompt — calling approveMoneyRequest with full: true. Inside that function, the optimistic data also clears comment.hold = '' for all "held" transactions (but since none are seen as held, this is a no-op). The server receives full=true and approves everything without respecting the hold.

What changes do you think we should make in order to solve the problem?

In dismissDuplicateTransactionViolation in Transaction.ts, change both the optimistic and failure Onyx updates for transactions to only include the specific field being changed (comment.dismissedViolations) instead of spreading the entire transaction snapshot.

Optimistic update should only set comment.dismissedViolations, not spread ...transaction. This way Onyx MERGE will only update that one nested field and will leave comment.hold (and all other fields) untouched.

const optimisticDataTransactions: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.TRANSACTION>> = currentTransactions.map((transaction) => ({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction?.transactionID}`,
    value: {
        comment: {
            dismissedViolations: {
                duplicatedTransaction: {
                    [dismissedPersonalDetails.login ?? '']: getUnixTime(new Date()),
                },
            },
        },
    },
}));

Failure/rollback update should only revert comment.dismissedViolations back to the pre-dismissal value (from transaction?.comment?.dismissedViolations ?? null), again without touching comment.hold.

const failureDataTransaction: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.TRANSACTION>> = currentTransactions.map((transaction) => ({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction?.transactionID}`,
    value: {
        comment: {
            dismissedViolations: transaction?.comment?.dismissedViolations ?? null,
        },
    },
}));

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[github-actions]

⚠️ @samranahm Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).