[Due for payment 2025-03-24] [$250] Expense - In delete expense details page, on tapping header "hold" option is displayed

[lanitochka17]

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.8-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Tap fab- create expense
3. Select a user
4. Complete the flow
5. Tap plus icon & create second expense
6. Open the second expense details page
7. Enter merchant and description
8. Send few comments
9. Rename merchant and description
10. Send few comments
11. Tap header - delete expense
12. Tap header

Expected Result:

In delete expense details page, on tapping header "hold" option must not be displayed

Actual Result:

In delete expense details page, on tapping header "hold" option is displayed

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6760267_1741034396453.Screenrecorder-2025-03-04-02-00-26-748.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021897001007484700253
• Upwork Job ID: 1897001007484700253
• Last Price Increase: 2025-03-04
• Automatic offers:
• DylanDylann | Contributor | 106433335
• daledah | Contributor | 106463957

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

Triggered auto assignment to @aldo-expensify (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

Production:

bandicam.2025-03-03.23-45-58-083.mp4

[samranahm]

🚨 Edited by proposal-police: This proposal was edited at 2025-03-03 21:22:21 UTC.

Proposal

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

Expense - In delete expense details page, on tapping header "hold" option is displayed

What is the root cause of that problem?

App/src/libs/ReportUtils.ts

Line 3683 in add7b6e

function canHoldUnholdReportAction(reportAction: OnyxInputOrEntry<ReportAction>): {canHoldRequest: boolean; canUnholdRequest: boolean} {

In canHoldUnholdReportAction.canholdRequest currently we are not explicitly checking if the ParentAction has been deleted.

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

App/src/libs/ReportUtils.ts

Line 3730 in add7b6e

const canHoldRequest = canHoldOrUnholdRequest && !isOnHold && (isRequestIOU || canModifyStatus) && !isScanning;

We should add another check here !isDeletedParentAction

const isDeletedParentAction = isDeletedAction(reportAction);
const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed && !isDeletedParentAction;

Edit: If the parent reportAction was deleted, we can't Hold or Unhold the expense

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

In our
tests/unit/ReportUtilsTest.ts

1. Add new unit test to confirm false from canHoldOrUnholdRequest is any condition is false

!isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed && !isDeletedParentAction

2. Hold Scenarios:
   Not currently on hold, Should allow canHoldRequest
   Currently on hold & not a duplicate, Should allow canUnholdRequest

What alternative solutions did you explore? (Optional)

Results

Screen.Recording.2025-03-04.at.5.31.18.AM.mov

[aldo-expensify]

Reproduced...

[aldo-expensify]

I was struggling to get my local dev environment to work, but now I'm back.

[aldo-expensify]

This was caused by this: #57219

Thanks @samranahm for the investigation / proposal here: #57702 (comment), I'm just not sure if this is the right way of solving it or if the right way would be to calculate properly the canHoldRequest variable here:

App/src/libs/ReportUtils.ts

Line 3710 in 295127d

const canHoldRequest = canHoldOrUnholdRequest && !isOnHold && (isRequestIOU || canModifyStatus) && !isScanning;

I'll wait for some feedback from @nkdengineer or @DylanDylann since they worked in this PR: #57219

[DylanDylann]

@nkdengineer @aldo-expensify I don't think this is a regression

In #57219, we remove !!transaction?.reimbursable as an intention because we don't need reimbursable check anymore.

The reason why this bug happens after #57219 is merged: the transaction is an empty object when removed, incidentally canHoldOrUnholdRequest is true after #57219 was merged because previously if the transaction is empty, transaction?.reimbursable will be undefined --> canHoldOrUnholdRequest is false

The root cause analysis of this issue indicates that the isClosed variable fails to update accurately, leading to the discrepancy in canHoldOrUnholdRequest

App/src/libs/ReportUtils.ts

Line 3703 in b649380

const isClosed = isClosedReport(moneyRequestReport);

--> canHoldOrUnholdRequest get a wrong value

App/src/libs/ReportUtils.ts

Line 3709 in b649380

const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed;

[daledah]

🚨 Edited by proposal-police: This proposal was edited at 2025-03-10 03:35:11 UTC.

Proposal

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

In delete expense details page, on tapping header "hold" option is displayed

What is the root cause of that problem?

After deleting expense then transaction = {} and the condition canHoldOrUnholdRequest here will be true

App/src/libs/ReportUtils.ts

Line 3709 in b649380

const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed;

I agree with Dylan here, we are not handling deleted expense case properly. We are checking the isClosed condition for moneyRequestReport but when deleting expense, moneyRequestReport still has statusNum=0 which means it is open => canHoldOrUnholdRequest has a wrong value.

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

We should add !isEmptyObject(transaction) to this condition to handle case delete expense

    const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed && !isEmptyObject(transaction);

App/src/libs/ReportUtils.ts

Line 3709 in b649380

const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed;

OR we can add a condition to canHoldOrUnholdRequest to check if the transaction report is closed

    const isDeletedParentAction = isDeletedAction(reportAction);
    const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed && !isDeletedParentAction;

App/src/libs/ReportUtils.ts

Line 3709 in b649380

const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentActionLocal && !isClosed;

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

We can add test for the function canHoldUnholdReportAction, in case deleted we should return canHoldRequest and canUnholdRequest as false

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.