Skip to content

Commit ba92595

Browse files
committed
only show remove hold in transaction thread secondary actions for admin if he is not the holder
1 parent f031c0d commit ba92595

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

src/libs/ReportSecondaryActionUtils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,10 @@ function isRemoveHoldAction(report: Report, chatReport: OnyxEntry<Report>, repor
546546
return policy?.role === CONST.POLICY.ROLE.ADMIN;
547547
}
548548

549+
function isRemoveHoldActionForTransaction(report: Report, reportTransaction: Transaction, policy?: Policy): boolean {
550+
return isOnHoldTransactionUtils(reportTransaction) && policy?.role === CONST.POLICY.ROLE.ADMIN && !isHoldCreator(reportTransaction, report.reportID);
551+
}
552+
549553
function getSecondaryReportActions({
550554
report,
551555
chatReport,
@@ -670,6 +674,10 @@ function getSecondaryTransactionThreadActions(
670674
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.HOLD);
671675
}
672676

677+
if (isRemoveHoldActionForTransaction(parentReport, reportTransaction, policy)) {
678+
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.REMOVE_HOLD);
679+
}
680+
673681
if (isSplitAction(parentReport, [reportTransaction], policy)) {
674682
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.SPLIT);
675683
}

tests/unit/ReportSecondaryActionUtilsTest.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1247,7 +1247,7 @@ describe('getSecondaryExportReportActions', () => {
12471247
expect(result.includes(CONST.REPORT.EXPORT_OPTIONS.MARK_AS_EXPORTED)).toBe(true);
12481248
});
12491249

1250-
it('includes REMOVE HOLD option for admin in expense report but not in transaction thread report', () => {
1250+
it('includes REMOVE HOLD option for expense report admin', () => {
12511251
const report = {} as unknown as Report;
12521252
const policy = {
12531253
role: CONST.POLICY.ROLE.ADMIN,
@@ -1266,6 +1266,27 @@ describe('getSecondaryExportReportActions', () => {
12661266
const result = getSecondaryReportActions({report, chatReport, reportTransactions, violations: {}, policy});
12671267
expect(result).toContain(CONST.REPORT.SECONDARY_ACTIONS.REMOVE_HOLD);
12681268
});
1269+
1270+
it('includes REMOVE HOLD option for transaction thread report admin if he is not the holder', () => {
1271+
const report = {} as unknown as Report;
1272+
const policy = {
1273+
role: CONST.POLICY.ROLE.ADMIN,
1274+
} as unknown as Policy;
1275+
const transaction = {
1276+
comment: {
1277+
hold: 'REPORT_ACTION_ID',
1278+
},
1279+
} as unknown as Transaction;
1280+
1281+
jest.spyOn(ReportUtils, 'isHoldCreator').mockReturnValue(false);
1282+
const result = getSecondaryTransactionThreadActions(report, transaction, [], policy);
1283+
expect(result).toContain(CONST.REPORT.SECONDARY_ACTIONS.REMOVE_HOLD);
1284+
1285+
// Do not show if admin is the holder
1286+
jest.spyOn(ReportUtils, 'isHoldCreator').mockReturnValue(true);
1287+
const result2 = getSecondaryTransactionThreadActions(report, transaction, [], policy);
1288+
expect(result2).not.toContain(CONST.REPORT.SECONDARY_ACTIONS.REMOVE_HOLD);
1289+
});
12691290
});
12701291

12711292
describe('getSecondaryTransactionThreadActions', () => {

0 commit comments

Comments
 (0)