[Due for payment 2025-05-27] [$250] Delete & Hold are present when there are two expense previews, app crashes with Hold option

[mitarachim]

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: 9.1.40-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp
Email or phone of affected tester (no customers): applausetester+smidod11s1j@applause.expensifail.com
Issue reported by: Applause Internal Team
Device used: Mac 15.3 / Chrome
App Component: Money Requests

Action Performed:

Precondition:

• Log in with Expensifail account.

1. Go to staging.new.expensify.com
2. Go to 1:1 DM.
3. Submit two expenses to th euser.
4. Click on the expense preview.
5. Click on any expense.
6. Send a message in the transaction thread.
7. Click on the report header.
8. Click Delete expense.
9. Delete the expense.
10. Close the RHP.
11. Now back in the expense report. click More.
12. Click Delete.
13. Note that nothing happens after tapping Delete.
14. Click More.
15. Click Hold.
16. Now app crashes with the Hold option.

Expected Result:

In Step 13 and 16, Hold and Delete option should not appear in More options because the expense report has two expense previews - one of the expense previews is present as "Deleted expense" because the expense has a comment.

Actual Result:

In Step 13 and 16, Hold and Delete option are present in More options when the expense report has two expense previews.
In Step 13, nothing happens with Delete option.
In Step 16, app crashes with Hold option.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6822777_1746511204158.20250506_135306.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021919796257257842653
• Upwork Job ID: 1919796257257842653
• Last Price Increase: 2025-05-06
• Automatic offers:
• brunovjk | Reviewer | 107218744
• thelullabyy | Contributor | 107218746

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

In Step 13 and 16, Hold and Delete option are present in More options when the expense report has two expense previews.
In Step 13, nothing happens with Delete option.
In Step 16, app crashes with Hold option.

What is the root cause of that problem?

After we delete the transaction, it will set the transaction to null

It leads to reportTransactions has only 1 transaction => isHoldAction and isDeleteAction will be true

App/src/libs/ReportSecondaryActionUtils.ts

Lines 442 to 444 in 0601503

	if (isHoldAction(report, reportTransactions)) {
	options.push(CONST.REPORT.SECONDARY_ACTIONS.HOLD);
	}

App/src/libs/ReportSecondaryActionUtils.ts

Lines 454 to 456 in 0601503

	if (isDeleteAction(report, reportTransactions)) {
	options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE);
	}

App/src/libs/ReportSecondaryActionUtils.ts

Line 275 in 0601503

const isOneExpenseReport = reportTransactions.length === 1;

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

We should add the condition like we did with ContextMenuActions component

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 372 to 375 in 0601503

	textTranslateKey: 'iou.hold',
	icon: Expensicons.Stopwatch,
	shouldShow: ({type, moneyRequestAction, areHoldRequirementsMet}) =>
	type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && areHoldRequirementsMet && canHoldUnholdReportAction(moneyRequestAction).canHoldRequest,

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 778 to 784 in 0601503

	textTranslateKey: 'reportActionContextMenu.deleteAction',
	icon: Expensicons.Trashcan,
	shouldShow: ({type, reportAction, isArchivedRoom, isChronosReport, reportID, moneyRequestAction}) =>
	// Until deleting parent threads is supported in FE, we will prevent the user from deleting a thread parent
	!!reportID &&
	type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION &&
	canDeleteReportAction(moneyRequestAction ?? reportAction, isMoneyRequestAction(moneyRequestAction) ? getOriginalMessage(moneyRequestAction)?.IOUReportID : reportID) &&

1. Add a new variable parentAction to getSecondaryReportActions function here and pass requestParentReportAction

App/src/components/MoneyReportHeader.tsx

Line 489 in 0601503

return getSecondaryReportActions(moneyRequestReport, transactions, violations, policy);

2. Using canHoldUnholdReportAction instead of isHoldAction and use canDeleteReportAction instead of isDeleteAction

Note: we can do the same with other action

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None

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.

[melvin-bot]

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

[melvin-bot]

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

[thelullabyy]

Proposal

Please re-state the problem that we are trying to solve in this issue.

In Step 13, nothing happens with Delete option.
In Step 16, app crashes with Hold option.

What is the root cause of that problem?

App/src/libs/ReportSecondaryActionUtils.ts

Lines 442 to 444 in fdc8093

	if (isHoldAction(report, reportTransactions)) {
	options.push(CONST.REPORT.SECONDARY_ACTIONS.HOLD);
	}

App/src/libs/ReportSecondaryActionUtils.ts

Lines 454 to 456 in fdc8093

	if (isDeleteAction(report, reportTransactions)) {
	options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE);
	}

Both isHoldAction and isDeleteAction action don't cover this case because the deleted transaction will be removed

App/src/libs/ReportSecondaryActionUtils.ts

Line 383 in fdc8093

const isSingleTransaction = reportTransactions.length === 1;

App/src/libs/ReportSecondaryActionUtils.ts

Line 275 in fdc8093

const isOneExpenseReport = reportTransactions.length === 1;

--> Both variables will be true

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

We need to implement another way to verify that there is only one transaction. We must use both transactions and reportAction to detect it.

App/src/libs/ReportSecondaryActionUtils.ts

Line 274 in fdc8093

function isHoldAction(report: Report, reportTransactions: Transaction[]): boolean {

// reportActionscould be added to isHoldAction as a new param
    const transactionThreadReportID = getOneTransactionThreadReportID(report.reportID, reportActions ?? []);

    const isOneExpenseReport = reportTransactions.length === 1;
    const transaction = reportTransactions.at(0);

    if (!transactionThreadReportID || !isOneExpenseReport || !transaction) {
        return false;
    }

Also, apply the same way to other actions

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

I suggest adding UTs for both isHoldAction and isDeleteAction function and covering 3 main cases

• There are two transactions
• There is one transaction
• there are two transactions but one of them has been removed

What alternative solutions did you explore? (Optional)

NA

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.

[brunovjk]

Thanks everyone for the proposals! 🙏 Both solutions worked in my initial tests, and the RCAs are quite similar.

I think we can move forward with @nkdengineer's proposal — it aligns well with the current code structure and makes the dependency on parentReportID explicit, improving clarity and maintainability.

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.