Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mollfpr merged 6 commits into from
Aug 1, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
76d7696
do not show remove hold in transaction thread secondary actions
dominictb Jul 31, 2025
f031c0d
remove redundant test case
dominictb Jul 31, 2025
ba92595
only show remove hold in transaction thread secondary actions for adm…
dominictb Jul 31, 2025
bc735ed
rearrange test cases
dominictb Jul 31, 2025
d3de207
pass transaction thread report param
dominictb Jul 31, 2025
14116f3
update test case
dominictb Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/MoneyRequestHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]: {
Expand Down
9 changes: 3 additions & 6 deletions src/libs/ReportSecondaryActionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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);
}

Expand Down
15 changes: 11 additions & 4 deletions tests/unit/ReportSecondaryActionUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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 () => {
Expand Down