Merge pull request Expensify#67591 from dominictb/fix/67586

[CP Staging] only show remove hold in transaction thread secondary actions for admin if he is not the holder

Author: mollfpr

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<ValueOf<typeof CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS>, DropdownOption<ValueOf<typeof CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS>>> = {
         [CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.HOLD]: {

src/libs/ReportSecondaryActionUtils.ts

@@ -547,11 +547,7 @@ function isRemoveHoldAction(report: Report, chatReport: OnyxEntry<Report>, 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,14 +667,15 @@ function getSecondaryTransactionThreadActions(
     reportTransaction: Transaction,
     reportActions: ReportAction[],
     policy: OnyxEntry<Policy>,
+    transactionThreadReport?: OnyxEntry<Report>,
 ): Array<ValueOf<typeof CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS>> {
     const options: Array<ValueOf<typeof CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS>> = [];
 
     if (isHoldActionForTransaction(parentReport, reportTransaction, reportActions)) {
         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);
     }
 

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 () => {