[$250] Expense - Unhold option is available for user who does not hold the expense which causes error

@Ollyws

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.32-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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:

Precondition:

Log in with Expensifail account

Go to staging.new.expensify.com
[User A] Submit two expenses to User B
[User A] Go to expense report
[User A] Select one of the expenses via checkbox
[User A] Click on the dropdown > Hold
[User A] Enter a reason and save it
[User B] Open chat with User A
[User B] Go to expense report
[User B] Click on the held expense, then click on the report header
[User B} Note that there is no Unhold option because only the person who holds the expense can unhold it in p2p expense
[User B] Close the RHP
[User B].Select the held expense via checkbox
[User B] Click on the dropdown > Unhold
[User B] Close the RHP and click on the expense which is now unheld
[User B] Note that it throws error "Unexpected error taking this expense off hold. Please try again later."

Expected Result:

In Step 13, the receiver should not have the option to unhold the expense because only the person who holds the expense can unhold it in p2p expense

Actual Result:

In Step 13, the receiver is given the option to unhold the expense when they are not the one who holds the expense
In Step 15, when the receiver unholds the expense, it throws error "Unexpected error taking this expense off hold. Please try again later."

Workaround:

Unknown

Platforms:

Select the officially supported platforms where the issue was reproduced:

Platforms Tested:

On which of our officially supported platforms was this issue tested:

Screenshots/Videos

Add any screenshot/video evidence

Bug6811753_1745506943148.20250424_225148.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021915823501257164210
Upwork Job ID: 1915823501257164210
Last Price Increase: 2025-05-02

Issue Owner

Current Issue Owner: @Ollyws

@OfstadC

Triggered auto assignment to @OfstadC (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.

🚨 Edited by proposal-police: This proposal was edited at 2025-04-24 16:37:40 UTC.

Proposal

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

Unhold option is available for the user that doesn't do the hold when selecting from checkbox.

What is the root cause of that problem?

Currently, we show the UNHOLD option without checking who hold it, or whether the user can UNHOLD it or not. We only check if it's currently on hold.

App/src/hooks/useSelectedTransactionsActions.ts

Lines 48 to 52 in f020d19

    
           if (allTransactionOnHold && selectedTransactions.length === 1) { 
        
               options.push({ 
        
                   text: translate('iou.unhold'), 
        
                   icon: Expensicons.Stopwatch, 
        
                   value: UNHOLD,

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

We need to follow the same approach in report details page by checking if the user can unhold or not using canHoldUnholdReportAction util.

App/src/pages/ReportDetailsPage.tsx

Line 781 in f020d19

    
           const canHoldUnholdReportAction = canHoldUnholdReportActionUtil(moneyRequestAction);

We only need to get the first transactionID since we only allow to hold/unhold a single expense.

const action = getIOUActionForTransactionID(reportActions, selectedTransactionsID.at(0) ?? '');
const {canHoldRequest, canUnholdRequest} = canHoldUnholdReportAction(action);

if (allTransactionOnHold && selectedTransactions.length === 1 && canUnholdRequest) {
    ...
}

We can apply the same to HOLD option if needed.

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

Test useSelectedTransactionsActions hook and verify it returns the correct options.

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

@Ollyws

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

Should be fixed by #60436.

Proposal

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

Unhold option is available for the user that doesn't do the hold when selecting from checkbox.

What is the root cause of that problem?

Currently, we show the UNHOLD option without checking who hold it, or whether the user can UNHOLD it or not. We only check if it's currently on hold.

App/src/hooks/useSelectedTransactionsActions.ts
Lines 48 to 52 in f020d19

if (allTransactionOnHold && selectedTransactions.length === 1) { 
    options.push({ 
        text: translate('iou.unhold'), 
        icon: Expensicons.Stopwatch, 
        value: UNHOLD,

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

We need to follow the same approach in report details page by checking if the user can unhold or not using canHoldUnholdReportActionUtil.

First, we need to import the utility:

import {canDeleteCardTransactionByLiabilityType, canDeleteTransaction, canHoldUnholdReportAction as canHoldUnholdReportActionUtil} from '@libs/ReportUtils';

Then we need to modify the code to check permissions before showing the unhold option:

if (allTransactionOnHold && selectedTransactions.length === 1) {
    // Get the action for the selected transaction
    const action = getIOUActionForTransactionID(reportActions, selectedTransactionsID[0]);
    
    // Check if this user can unhold the expense using the utility function
    const {canUnholdRequest} = canHoldUnholdReportActionUtil(action);
    
    // Only add the unhold option if the user has permission to unhold
    if (canUnholdRequest) {
        options.push({
            text: translate('iou.unhold'),
            icon: Expensicons.Stopwatch,
            value: UNHOLD,
            onSelected: () => {
                // ... existing code ...
            },
        });
    }
}

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

Test useSelectedTransactionsActions hook and verify it returns the correct options.

Can help with internal review on this when it's ready for it!

@Tony-MK

@Tony-MK can you confirm if your PR will fix this issue? If not we'll fix it here.

@Ollyws

Hey @Ollyws, the PR will fix this bug.

Ok let's hold for that then.

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@Ollyws

@Ollyws Whoops! This issue is 2 days overdue. Let's get this updated quick!

That PR is on staging - can we retest/investigate this? Thanks!

@Ollyws

@Ollyws Eep! 4 days overdue now. Issues have feelings too...

https://expensify.slack.com/archives/C9YU7BX5M/p1746720632531669

Not reproducible

20250509_012024.mp4

Since this can no longer be reproduced - closing

Navigation Menu

Repository navigation

[$250] Expense - Unhold option is available for user who does not hold the expense which causes error #60845

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

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

Proposal

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

What is the root cause of that problem?

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

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (allTransactionOnHold && selectedTransactions.length === 1) {
	options.push({
	text: translate('iou.unhold'),
	icon: Expensicons.Stopwatch,
	value: UNHOLD,

[$250] Expense - Unhold option is available for user who does not hold the expense which causes error #60845

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Apr 24, 2025

bernhardoj commented on Apr 24, 2025

Proposal

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

What is the root cause of that problem?

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

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

melvin-bot commented on Apr 25, 2025

melvin-bot commented on Apr 25, 2025

Tony-MK commented on Apr 26, 2025

Nick4dawin commented on Apr 28, 2025

Proposal

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

What is the root cause of that problem?

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

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

dangrous commented on Apr 28, 2025

Ollyws commented on Apr 30, 2025

Tony-MK commented on Apr 30, 2025

Ollyws commented on May 2, 2025

melvin-bot commented on May 2, 2025

melvin-bot commented on May 5, 2025

dangrous commented on May 5, 2025

melvin-bot commented on May 7, 2025

OfstadC commented on May 8, 2025

izarutskaya commented on May 8, 2025

OfstadC commented on May 8, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions