From 651bcadd2a72035ca8a85ae45b2c7e76ab80efaa Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 13:55:32 -0600 Subject: [PATCH 01/10] fix delete condition --- src/libs/ReportSecondaryActionUtils.ts | 37 +++++++++++++------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index f33fbb110fa2..11ba2ed8c980 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -16,7 +16,7 @@ import { isPreferredExporter, isSubmitAndClose as isSubmitAndCloseUtils, } from './PolicyUtils'; -import {getIOUActionForReportID, getIOUActionForTransactionID, getOneTransactionThreadReportID, isPayAction} from './ReportActionsUtils'; +import {getIOUActionForReportID, getIOUActionForTransactionID, getOneTransactionThreadReportID, isPayAction, isReversedTransaction} from './ReportActionsUtils'; import {isPrimaryPayAction} from './ReportPrimaryActionUtils'; import { canAddTransaction, @@ -38,6 +38,7 @@ import { isReportManager as isReportManagerUtils, isSettled, isWorkspaceEligibleForReportChange, + isSelfDM as isSelfDMReportUtils, } from './ReportUtils'; import {getSession} from './SessionUtils'; import {allHavePendingRTERViolation, isDuplicate, isOnHold as isOnHoldTransactionUtils, shouldShowBrokenConnectionViolationForMultipleTransactions} from './TransactionUtils'; @@ -349,29 +350,29 @@ function isMoveTransactionAction(reportTransactions: Transaction[], reportAction return canMoveExpense; } -function isDeleteAction(report: Report): boolean { +function isDeleteAction(report: Report, reportTransactions: Transaction[], reportActions: ReportAction[]): boolean { const isExpenseReport = isExpenseReportUtils(report); const isIOUReport = isIOUReportUtils(report); + const isUnreported = isSelfDMReportUtils(report); - if (!isExpenseReport && !isIOUReport) { - return false; - } + if (isUnreported ||isIOUReport) { + const transactionID = reportTransactions?.at(0)?.transactionID ?? '0'; + const action = getIOUActionForTransactionID(reportActions, transactionID); + const isOwner = action?.actorAccountID === getCurrentUserAccountID(); - const isReportSubmitter = isCurrentUserSubmitter(report.reportID); - - if (!isReportSubmitter) { - return false; + // Users cannot delete a report in the unrepeorted or IOU cases, but they can delete individual transactions. + // So we check if the reportTransactions length is 1 which means they're viewing a single transaction and thus can delete it. + return isOwner && reportTransactions.length === 1; } - const isReportOpen = isOpenReportUtils(report); - const isProcessingReport = isProcessingReportUtils(report); - const isReportApproved = isReportApprovedUtils({report}); - - if (isReportApproved) { - return false; + if (isExpenseReport) { + const isReportSubmitter = isCurrentUserSubmitter(report.reportID); + const isReportOpen = isOpenReportUtils(report); + const isProcessingReport = isProcessingReportUtils(report); + return isReportSubmitter && (isReportOpen || isProcessingReport); } - return isReportOpen || isProcessingReport; + return false; } function isRetractAction(report: Report, policy?: Policy): boolean { @@ -488,7 +489,7 @@ function getSecondaryReportActions( options.push(CONST.REPORT.SECONDARY_ACTIONS.VIEW_DETAILS); - if (isDeleteAction(report)) { + if (isDeleteAction(report, reportTransactions, reportActions ?? [])) { options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE); } @@ -508,7 +509,7 @@ function getSecondaryTransactionThreadActions( options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.VIEW_DETAILS); - if (isDeleteAction(parentReport)) { + if (isDeleteAction(parentReport, [reportTransaction], reportActions ?? [])) { options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.DELETE); } From 13bad70d2da20635ce2e2ac5fa5b0b958040f07c Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 14:08:34 -0600 Subject: [PATCH 02/10] add corporate liability case --- src/libs/ReportSecondaryActionUtils.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 11ba2ed8c980..fcaf9d34b5bf 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -41,7 +41,7 @@ import { isSelfDM as isSelfDMReportUtils, } from './ReportUtils'; import {getSession} from './SessionUtils'; -import {allHavePendingRTERViolation, isDuplicate, isOnHold as isOnHoldTransactionUtils, shouldShowBrokenConnectionViolationForMultipleTransactions} from './TransactionUtils'; +import {allHavePendingRTERViolation, isCardTransaction as isCardTransactionUtils, isDuplicate, isOnHold as isOnHoldTransactionUtils, shouldShowBrokenConnectionViolationForMultipleTransactions} from './TransactionUtils'; function isAddExpenseAction(report: Report, reportTransactions: Transaction[]) { const isReportSubmitter = isCurrentUserSubmitter(report.reportID); @@ -367,9 +367,10 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor if (isExpenseReport) { const isReportSubmitter = isCurrentUserSubmitter(report.reportID); - const isReportOpen = isOpenReportUtils(report); - const isProcessingReport = isProcessingReportUtils(report); - return isReportSubmitter && (isReportOpen || isProcessingReport); + const isReportOpenOrProcessing = isOpenReportUtils(report) || isProcessingReportUtils(report); + const isCardTransactionWithCorporateLiability = reportTransactions.length === 1 && isCardTransactionUtils(reportTransactions.at(0)) && reportTransactions.at(0)?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; + + return isReportSubmitter && isReportOpenOrProcessing && !isCardTransactionWithCorporateLiability; } return false; From 9308c3b1e3efd12beb527917a26e7d4bb16caea5 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 14:20:36 -0600 Subject: [PATCH 03/10] refactor --- src/libs/ReportSecondaryActionUtils.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index fcaf9d34b5bf..b3836c253151 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -354,20 +354,25 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor const isExpenseReport = isExpenseReportUtils(report); const isIOUReport = isIOUReportUtils(report); const isUnreported = isSelfDMReportUtils(report); + const transaction = reportTransactions.at(0); + const transactionID = transaction?.transactionID ?? '0'; + const action = getIOUActionForTransactionID(reportActions, transactionID); + const isOwner = action?.actorAccountID === getCurrentUserAccountID(); + const isReportOpenOrProcessing = isOpenReportUtils(report) || isProcessingReportUtils(report); + const isSingleTransaction = reportTransactions.length === 1; - if (isUnreported ||isIOUReport) { - const transactionID = reportTransactions?.at(0)?.transactionID ?? '0'; - const action = getIOUActionForTransactionID(reportActions, transactionID); - const isOwner = action?.actorAccountID === getCurrentUserAccountID(); - - // Users cannot delete a report in the unrepeorted or IOU cases, but they can delete individual transactions. - // So we check if the reportTransactions length is 1 which means they're viewing a single transaction and thus can delete it. - return isOwner && reportTransactions.length === 1; + if (isUnreported) { + return isOwner; + } + + // Users cannot delete a report in the unrepeorted or IOU cases, but they can delete individual transactions. + // So we check if the reportTransactions length is 1 which means they're viewing a single transaction and thus can delete it. + if (isIOUReport) { + return isSingleTransaction && isOwner && isReportOpenOrProcessing; } if (isExpenseReport) { const isReportSubmitter = isCurrentUserSubmitter(report.reportID); - const isReportOpenOrProcessing = isOpenReportUtils(report) || isProcessingReportUtils(report); const isCardTransactionWithCorporateLiability = reportTransactions.length === 1 && isCardTransactionUtils(reportTransactions.at(0)) && reportTransactions.at(0)?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; return isReportSubmitter && isReportOpenOrProcessing && !isCardTransactionWithCorporateLiability; From ddccb32b18f6574dfebb375b38a6ebce6e1f63cb Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 14:25:40 -0600 Subject: [PATCH 04/10] refactor some more --- src/libs/ReportSecondaryActionUtils.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index b3836c253151..08d2eb52303f 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -372,10 +372,14 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor } if (isExpenseReport) { - const isReportSubmitter = isCurrentUserSubmitter(report.reportID); - const isCardTransactionWithCorporateLiability = reportTransactions.length === 1 && isCardTransactionUtils(reportTransactions.at(0)) && reportTransactions.at(0)?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; + const isCardTransactionWithCorporateLiability = isSingleTransaction && isCardTransactionUtils(transaction) && transaction?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; + + if (isCardTransactionWithCorporateLiability) { + return false; + } - return isReportSubmitter && isReportOpenOrProcessing && !isCardTransactionWithCorporateLiability; + const isReportSubmitter = isCurrentUserSubmitter(report.reportID); + return isReportSubmitter && isReportOpenOrProcessing; } return false; From 6e31d5683a9de10c83af4bf39b190828cc009596 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 14:48:01 -0600 Subject: [PATCH 05/10] fix lint --- src/libs/ReportSecondaryActionUtils.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 08d2eb52303f..1a4928760651 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -16,7 +16,7 @@ import { isPreferredExporter, isSubmitAndClose as isSubmitAndCloseUtils, } from './PolicyUtils'; -import {getIOUActionForReportID, getIOUActionForTransactionID, getOneTransactionThreadReportID, isPayAction, isReversedTransaction} from './ReportActionsUtils'; +import {getIOUActionForReportID, getIOUActionForTransactionID, getOneTransactionThreadReportID, isPayAction} from './ReportActionsUtils'; import {isPrimaryPayAction} from './ReportPrimaryActionUtils'; import { canAddTransaction, @@ -36,12 +36,18 @@ import { isProcessingReport as isProcessingReportUtils, isReportApproved as isReportApprovedUtils, isReportManager as isReportManagerUtils, + isSelfDM as isSelfDMReportUtils, isSettled, isWorkspaceEligibleForReportChange, - isSelfDM as isSelfDMReportUtils, } from './ReportUtils'; import {getSession} from './SessionUtils'; -import {allHavePendingRTERViolation, isCardTransaction as isCardTransactionUtils, isDuplicate, isOnHold as isOnHoldTransactionUtils, shouldShowBrokenConnectionViolationForMultipleTransactions} from './TransactionUtils'; +import { + allHavePendingRTERViolation, + isCardTransaction as isCardTransactionUtils, + isDuplicate, + isOnHold as isOnHoldTransactionUtils, + shouldShowBrokenConnectionViolationForMultipleTransactions, +} from './TransactionUtils'; function isAddExpenseAction(report: Report, reportTransactions: Transaction[]) { const isReportSubmitter = isCurrentUserSubmitter(report.reportID); @@ -364,7 +370,7 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor if (isUnreported) { return isOwner; } - + // Users cannot delete a report in the unrepeorted or IOU cases, but they can delete individual transactions. // So we check if the reportTransactions length is 1 which means they're viewing a single transaction and thus can delete it. if (isIOUReport) { @@ -372,7 +378,8 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor } if (isExpenseReport) { - const isCardTransactionWithCorporateLiability = isSingleTransaction && isCardTransactionUtils(transaction) && transaction?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; + const isCardTransactionWithCorporateLiability = + isSingleTransaction && isCardTransactionUtils(transaction) && transaction?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; if (isCardTransactionWithCorporateLiability) { return false; From 23f8ee62bb9012b5701a221b1e7c7c0fa4bfc636 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 15:02:40 -0600 Subject: [PATCH 06/10] add tets --- tests/unit/ReportSecondaryActionUtilsTest.ts | 193 +++++++++++++++++++ 1 file changed, 193 insertions(+) diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index f63e88e19627..67b5d6cf0fe1 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -654,6 +654,199 @@ describe('getSecondaryAction', () => { const result = getSecondaryReportActions(report, [{} as Transaction], {}, policy); expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); }); + + it('includes DELETE option for owner of unreported transaction', () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.CHAT, + chatType: CONST.REPORT.CHAT_TYPE.SELF_DM, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + + const transaction = { + transactionID: TRANSACTION_ID, + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + } as unknown as Transaction; + + const reportActions = [ + { + reportActionID: '1', + actorAccountID: EMPLOYEE_ACCOUNT_ID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + originalMessage: { + IOUTransactionID: TRANSACTION_ID, + IOUReportID: CONST.REPORT.UNREPORTED_REPORT_ID, + }, + }, + ] as unknown as ReportAction[]; + + const result = getSecondaryReportActions(report, [transaction], {}, {}, undefined, reportActions); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); + }); + + it('includes DELETE option for owner of single processing IOU transaction', () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + + const transaction = { + transactionID: TRANSACTION_ID, + reportID: REPORT_ID, + } as unknown as Transaction; + + const reportActions = [ + { + reportActionID: '1', + actorAccountID: EMPLOYEE_ACCOUNT_ID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + originalMessage: { + IOUTransactionID: TRANSACTION_ID, + IOUReportID: REPORT_ID, + }, + }, + ] as unknown as ReportAction[]; + + const policy = {} as unknown as Policy; + + const result = getSecondaryReportActions(report, [transaction], {}, policy, undefined, reportActions); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); + }); + + it('does not include DELETE option for IOU report', () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + const TRANSACTION_ID_2 = 'TRANSACTION_ID_2'; + + const transaction1 = { + transactionID: TRANSACTION_ID, + reportID: REPORT_ID, + } as unknown as Transaction; + + const transaction2 = { + transactionID: TRANSACTION_ID_2, + reportID: REPORT_ID, + } as unknown as Transaction; + + const reportActions = [ + { + reportActionID: '1', + actorAccountID: EMPLOYEE_ACCOUNT_ID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + originalMessage: { + IOUTransactionID: TRANSACTION_ID, + IOUReportID: REPORT_ID, + }, + }, + { + reportActionID: '2', + actorAccountID: EMPLOYEE_ACCOUNT_ID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + originalMessage: { + IOUTransactionID: TRANSACTION_ID_2, + IOUReportID: REPORT_ID, + }, + }, + ] as unknown as ReportAction[]; + + const policy = {} as unknown as Policy; + + const result = getSecondaryReportActions(report, [transaction1, transaction2], {}, policy, undefined, reportActions); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); + }); + + it('includes DELETE option for owner of single processing expense transaction', async () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + + const transaction = { + transactionID: TRANSACTION_ID, + reportID: REPORT_ID, + } as unknown as Transaction; + + const policy = {} as unknown as Policy; + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report); + + const result = getSecondaryReportActions(report, [transaction], {}, policy); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); + }); + + it('includes DELETE option for owner of processing expense report', async () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + const TRANSACTION_ID_2 = 'TRANSACTION_ID_2'; + + const transaction1 = { + transactionID: TRANSACTION_ID, + reportID: REPORT_ID, + } as unknown as Transaction; + + const transaction2 = { + transactionID: TRANSACTION_ID_2, + reportID: REPORT_ID, + } as unknown as Transaction; + + const policy = {} as unknown as Policy; + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report); + + const result = getSecondaryReportActions(report, [transaction1, transaction2], {}, policy); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); + }); + + it('does not include DELETE option for corporate liability card transaction', async () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + + const transaction = { + transactionID: TRANSACTION_ID, + reportID: REPORT_ID, + managedCard: true, + comment: { + liabilityType: CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT, + }, + } as unknown as Transaction; + + const policy = {} as unknown as Policy; + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report); + + const result = getSecondaryReportActions(report, [transaction], {}, policy); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); + }); }); describe('getSecondaryTransactionThreadActions', () => { From ad1fe6931005806e03c57a3224aea16c1e131691 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 15:42:13 -0600 Subject: [PATCH 07/10] update conditional --- src/libs/ReportSecondaryActionUtils.ts | 8 +++++--- tests/unit/ReportSecondaryActionUtilsTest.ts | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 1a4928760651..93f57c5e6e97 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -25,6 +25,7 @@ import { canHoldUnholdReportAction, hasOnlyHeldExpenses, isArchivedReport, + isAwaitingFirstLevelApproval, isClosedReport as isClosedReportUtils, isCurrentUserSubmitter, isExpenseReport as isExpenseReportUtils, @@ -361,8 +362,8 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor const isIOUReport = isIOUReportUtils(report); const isUnreported = isSelfDMReportUtils(report); const transaction = reportTransactions.at(0); - const transactionID = transaction?.transactionID ?? '0'; - const action = getIOUActionForTransactionID(reportActions, transactionID); + const transactionID = transaction?.transactionID; + const action = getIOUActionForTransactionID(reportActions, transactionID ?? ''); const isOwner = action?.actorAccountID === getCurrentUserAccountID(); const isReportOpenOrProcessing = isOpenReportUtils(report) || isProcessingReportUtils(report); const isSingleTransaction = reportTransactions.length === 1; @@ -386,7 +387,8 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor } const isReportSubmitter = isCurrentUserSubmitter(report.reportID); - return isReportSubmitter && isReportOpenOrProcessing; + const isForwarded = isProcessingReportUtils(report) && !isAwaitingFirstLevelApproval(report); + return isReportSubmitter && isReportOpenOrProcessing && !isForwarded; } return false; diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index 67b5d6cf0fe1..b25d3b7101d8 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -682,7 +682,9 @@ describe('getSecondaryAction', () => { }, ] as unknown as ReportAction[]; - const result = getSecondaryReportActions(report, [transaction], {}, {}, undefined, reportActions); + const policy = {} as unknown as Policy; + + const result = getSecondaryReportActions(report, [transaction], {}, policy, undefined, reportActions); expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); }); From 95286ac0a2ad1d0f420363155b5248b6685b9724 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 16:28:42 -0600 Subject: [PATCH 08/10] add test case --- src/libs/ReportSecondaryActionUtils.ts | 8 +++-- tests/unit/ReportSecondaryActionUtilsTest.ts | 33 +++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 93f57c5e6e97..06eaecee946f 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -357,7 +357,7 @@ function isMoveTransactionAction(reportTransactions: Transaction[], reportAction return canMoveExpense; } -function isDeleteAction(report: Report, reportTransactions: Transaction[], reportActions: ReportAction[]): boolean { +function isDeleteAction(report: Report, reportTransactions: Transaction[], reportActions: ReportAction[], policy?: Policy): boolean { const isExpenseReport = isExpenseReportUtils(report); const isIOUReport = isIOUReportUtils(report); const isUnreported = isSelfDMReportUtils(report); @@ -387,7 +387,9 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor } const isReportSubmitter = isCurrentUserSubmitter(report.reportID); - const isForwarded = isProcessingReportUtils(report) && !isAwaitingFirstLevelApproval(report); + const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false; + const isForwarded = isProcessingReportUtils(report) && isApprovalEnabled && !isAwaitingFirstLevelApproval(report); + return isReportSubmitter && isReportOpenOrProcessing && !isForwarded; } @@ -508,7 +510,7 @@ function getSecondaryReportActions( options.push(CONST.REPORT.SECONDARY_ACTIONS.VIEW_DETAILS); - if (isDeleteAction(report, reportTransactions, reportActions ?? [])) { + if (isDeleteAction(report, reportTransactions, reportActions ?? [], policy)) { options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE); } diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index b25d3b7101d8..73c183f4daa8 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -41,7 +41,7 @@ describe('getSecondaryAction', () => { jest.clearAllMocks(); Onyx.clear(); await Onyx.merge(ONYXKEYS.SESSION, SESSION); - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS}); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS, [APPROVER_ACCOUNT_ID]: {accountID: APPROVER_ACCOUNT_ID, login: APPROVER_EMAIL}}); }); it('should always return default options', () => { @@ -849,6 +849,37 @@ describe('getSecondaryAction', () => { const result = getSecondaryReportActions(report, [transaction], {}, policy); expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); }); + + it('does not include DELETE option for report that has been forwarded', async () => { + const report = { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: EMPLOYEE_ACCOUNT_ID, + managerID: MANAGER_ACCOUNT_ID, + policyID: POLICY_ID, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + } as unknown as Report; + + const TRANSACTION_ID = 'TRANSACTION_ID'; + + const transaction = { + transactionID: TRANSACTION_ID, + reportID: REPORT_ID, + } as unknown as Transaction; + + const policy = { + id: POLICY_ID, + approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, + approver: APPROVER_EMAIL, + } as unknown as Policy; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy); + + const result = getSecondaryReportActions(report, [transaction], {}, policy); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); + }); }); describe('getSecondaryTransactionThreadActions', () => { From 1ba2cc03eea463c5ef8f53f0f14adba1ff8c3668 Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 28 May 2025 16:35:08 -0600 Subject: [PATCH 09/10] fix eslint --- src/libs/ReportSecondaryActionUtils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 06eaecee946f..1c66ed34aa21 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -363,8 +363,7 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor const isUnreported = isSelfDMReportUtils(report); const transaction = reportTransactions.at(0); const transactionID = transaction?.transactionID; - const action = getIOUActionForTransactionID(reportActions, transactionID ?? ''); - const isOwner = action?.actorAccountID === getCurrentUserAccountID(); + const isOwner = transactionID ? getIOUActionForTransactionID(reportActions, transactionID)?.actorAccountID === getCurrentUserAccountID() : false; const isReportOpenOrProcessing = isOpenReportUtils(report) || isProcessingReportUtils(report); const isSingleTransaction = reportTransactions.length === 1; From 5e526215b67e6cf91c19e12529257d7f109632ec Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Thu, 29 May 2025 11:26:27 -0600 Subject: [PATCH 10/10] fix lint --- src/libs/ReportSecondaryActionUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 9ce64bdad2f3..1d7aa33f30e0 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -45,11 +45,11 @@ import { import {getSession} from './SessionUtils'; import { allHavePendingRTERViolation, + getOriginalTransactionWithSplitInfo, + hasReceipt as hasReceiptTransactionUtils, isCardTransaction as isCardTransactionUtils, isDuplicate, isOnHold as isOnHoldTransactionUtils, - getOriginalTransactionWithSplitInfo, - hasReceipt as hasReceiptTransactionUtils, isPending, isReceiptBeingScanned, shouldShowBrokenConnectionViolationForMultipleTransactions,