diff --git a/src/components/MoneyRequestHeader.tsx b/src/components/MoneyRequestHeader.tsx index 7c1aef460e77..d34935401487 100644 --- a/src/components/MoneyRequestHeader.tsx +++ b/src/components/MoneyRequestHeader.tsx @@ -202,8 +202,8 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre if (!transaction || !reportActions) { return []; } - return getSecondaryTransactionThreadActions(parentReport, transaction, Object.values(reportActions), policy); - }, [parentReport, policy, transaction]); + return getSecondaryTransactionThreadActions(parentReport, transaction, Object.values(reportActions), policy, report); + }, [report, parentReport, policy, transaction]); const secondaryActionsImplementation: Record, DropdownOption>> = { [CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.HOLD]: { diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index ae5844024496..4785cce7ab18 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -547,11 +547,7 @@ function isRemoveHoldAction(report: Report, chatReport: OnyxEntry, repor } function isRemoveHoldActionForTransaction(report: Report, reportTransaction: Transaction, policy?: Policy): boolean { - if (!isOnHoldTransactionUtils(reportTransaction)) { - return false; - } - - return policy?.role === CONST.POLICY.ROLE.ADMIN; + return isOnHoldTransactionUtils(reportTransaction) && policy?.role === CONST.POLICY.ROLE.ADMIN && !isHoldCreator(reportTransaction, report.reportID); } function getSecondaryReportActions({ @@ -671,6 +667,7 @@ function getSecondaryTransactionThreadActions( reportTransaction: Transaction, reportActions: ReportAction[], policy: OnyxEntry, + transactionThreadReport?: OnyxEntry, ): Array> { const options: Array> = []; @@ -678,7 +675,7 @@ function getSecondaryTransactionThreadActions( options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.HOLD); } - if (isRemoveHoldActionForTransaction(parentReport, reportTransaction, policy)) { + if (transactionThreadReport && isRemoveHoldActionForTransaction(transactionThreadReport, reportTransaction, policy)) { options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.REMOVE_HOLD); } diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index 13ea1b36b60b..aae9c3dcdf28 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -1247,7 +1247,7 @@ describe('getSecondaryExportReportActions', () => { expect(result.includes(CONST.REPORT.EXPORT_OPTIONS.MARK_AS_EXPORTED)).toBe(true); }); - it('includes REMOVE HOLD option for admin', () => { + it('includes REMOVE HOLD option for admin if he is not the holder', () => { const report = {} as unknown as Report; const policy = { role: CONST.POLICY.ROLE.ADMIN, @@ -1310,8 +1310,9 @@ describe('getSecondaryTransactionThreadActions', () => { expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.HOLD)).toBe(true); }); - it('includes REMOVE HOLD option for admin', () => { + it('includes REMOVE HOLD option for transaction thread report admin if he is not the holder', () => { const report = {} as unknown as Report; + const transactionThreadReport = {} as unknown as Report; const policy = { role: CONST.POLICY.ROLE.ADMIN, } as unknown as Policy; @@ -1321,8 +1322,14 @@ describe('getSecondaryTransactionThreadActions', () => { }, } as unknown as Transaction; - const result = getSecondaryTransactionThreadActions(report, transaction, [], policy); - expect(result).toContain(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.REMOVE_HOLD); + jest.spyOn(ReportUtils, 'isHoldCreator').mockReturnValue(false); + const result = getSecondaryTransactionThreadActions(report, transaction, [], policy, transactionThreadReport); + expect(result).toContain(CONST.REPORT.SECONDARY_ACTIONS.REMOVE_HOLD); + + // Do not show if admin is the holder + jest.spyOn(ReportUtils, 'isHoldCreator').mockReturnValue(true); + const result2 = getSecondaryTransactionThreadActions(report, transaction, [], policy, transactionThreadReport); + expect(result2).not.toContain(CONST.REPORT.SECONDARY_ACTIONS.REMOVE_HOLD); }); it('includes DELETE option for expense report submitter', async () => {