diff --git a/src/hooks/useSearchBulkActions.ts b/src/hooks/useSearchBulkActions.ts index ce43f5f9e5e8..3b21a7697606 100644 --- a/src/hooks/useSearchBulkActions.ts +++ b/src/hooks/useSearchBulkActions.ts @@ -2060,7 +2060,7 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) { const firstTransactionKey = selectedTransactionsKeys.at(0); const firstTransactionMeta = firstTransactionKey ? selectedTransactions[firstTransactionKey] : undefined; - const canShowDeleteAction = shouldShowDeleteOption(selectedTransactions, currentSearchResults?.data, selectedReports, queryJSON?.type); + const canShowDeleteAction = shouldShowDeleteOption(selectedTransactions, currentSearchResults?.data, accountID, selectedReports, queryJSON?.type); const isSplittable = !!firstTransactionMeta?.canSplit; const isAlreadySplit = !!firstTransactionMeta?.hasBeenSplit; diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 494ddf6f7549..eaa22f17504c 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -678,8 +678,8 @@ function isChangeWorkspaceAction(report: Report, policies: OnyxCollection, isProduction: boolean, + currentUserAccountID: number, ): boolean { if (reportTransactions.length !== 1) { return false; @@ -699,7 +700,10 @@ function shouldShowEditSplitInDeleteAction( } const isSelfDMSplit = isSelfDMReportUtils(report); - return shouldRedirectDeleteToSplitExpenseEdit(reportTransaction, originalTransaction, isSelfDMSplit, isProduction) && isDeleteAction(report, reportTransactions, reportActions); + return ( + shouldRedirectDeleteToSplitExpenseEdit(reportTransaction, originalTransaction, isSelfDMSplit, isProduction) && + isDeleteAction(report, reportTransactions, currentUserAccountID, reportActions) + ); } function isRetractAction(report: Report, policy?: Policy): boolean { @@ -1051,7 +1055,7 @@ function getSecondaryReportActions({ if ( isSplitAction(report, reportTransactions, originalTransaction, currentUserLogin, currentUserAccountID, policy, parentReport, isProduction) && - !shouldShowEditSplitInDeleteAction(report, reportTransactions, reportActions, originalTransaction, isProduction) + !shouldShowEditSplitInDeleteAction(report, reportTransactions, reportActions, originalTransaction, isProduction, currentUserAccountID) ) { options.push(CONST.REPORT.SECONDARY_ACTIONS.SPLIT); } @@ -1106,7 +1110,7 @@ function getSecondaryReportActions({ options.push(CONST.REPORT.SECONDARY_ACTIONS.VIEW_DETAILS); - if (isDeleteAction(report, reportTransactions, reportActions ?? [])) { + if (isDeleteAction(report, reportTransactions, currentUserAccountID, reportActions ?? [])) { options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE); } @@ -1185,7 +1189,7 @@ function getSecondaryTransactionThreadActions({ if ( isSplitAction(parentReport, [reportTransaction], originalTransaction, currentUserLogin, currentUserAccountID, policy, grandParentReport, isProduction) && - !shouldShowEditSplitInDeleteAction(parentReport, [reportTransaction], reportAction ? [reportAction] : [], originalTransaction, isProduction) + !shouldShowEditSplitInDeleteAction(parentReport, [reportTransaction], reportAction ? [reportAction] : [], originalTransaction, isProduction, currentUserAccountID) ) { options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.SPLIT); } @@ -1216,7 +1220,7 @@ function getSecondaryTransactionThreadActions({ options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.VIEW_DETAILS); - if (isDeleteAction(parentReport, [reportTransaction], reportAction ? [reportAction] : [])) { + if (isDeleteAction(parentReport, [reportTransaction], currentUserAccountID, reportAction ? [reportAction] : [])) { options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.DELETE); } diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index f12653caf56f..d70f5c975b93 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3207,10 +3207,10 @@ function canDeleteCardTransactionByLiabilityType(transaction: OnyxEntry, reportTransactions: Transaction[], reportActions: ReportAction[]): boolean { +function canDeleteMoneyRequestReport(report: OnyxEntry, reportTransactions: Transaction[], reportActions: ReportAction[], currentUserAccountID: number): boolean { const transaction = reportTransactions.at(0); const transactionID = transaction?.transactionID; - const isOwner = transactionID ? getIOUActionForTransactionID(reportActions, transactionID)?.actorAccountID === deprecatedCurrentUserAccountID : false; + const isOwner = transactionID ? getIOUActionForTransactionID(reportActions, transactionID)?.actorAccountID === currentUserAccountID : false; const isReportOpenOrProcessing = isOpenReport(report) || isProcessingReport(report); const isSingleTransaction = reportTransactions.length === 1; @@ -3225,7 +3225,7 @@ function canDeleteMoneyRequestReport(report: OnyxEntry, reportTransactio } if (isInvoiceReport(report)) { - return report?.ownerAccountID === deprecatedCurrentUserAccountID && isReportOpenOrProcessing; + return report?.ownerAccountID === currentUserAccountID && isReportOpenOrProcessing; } // Users cannot delete a report in the unreported or IOU cases, but they can delete individual transactions. @@ -3239,7 +3239,7 @@ function canDeleteMoneyRequestReport(report: OnyxEntry, reportTransactio return false; } - const isReportSubmitter = isCurrentUserSubmitter(report); + const isReportSubmitter = isCurrentUserSubmitter(report, currentUserAccountID); return isReportSubmitter && (isOpenReport(report) || (isProcessingReport(report) && isAwaitingFirstLevelApproval(report))); } @@ -3256,9 +3256,10 @@ function canDeleteReportAction( transaction: OnyxEntry | undefined, transactions: OnyxCollection, childReportActions: OnyxCollection, + currentUserAccountID: number, ): boolean { const report = getReportOrDraftReport(reportID); - const isActionOwner = reportAction?.actorAccountID === deprecatedCurrentUserAccountID; + const isActionOwner = reportAction?.actorAccountID === currentUserAccountID; const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`] ?? null; if (isDemoTransaction(transaction)) { @@ -3290,6 +3291,7 @@ function canDeleteReportAction( report, Object.values(transactions ?? {}).filter((t): t is Transaction => !!t), Object.values(childReportActions ?? {}).filter((action): action is ReportAction => !!action), + currentUserAccountID, ); } diff --git a/src/libs/SearchUIUtils.ts b/src/libs/SearchUIUtils.ts index 15df21a1bd70..cd640158c9ae 100644 --- a/src/libs/SearchUIUtils.ts +++ b/src/libs/SearchUIUtils.ts @@ -6370,6 +6370,7 @@ function navigateToSearchRHP(route: {route: string; getRoute: (backTo?: string) function shouldShowDeleteOption( selectedTransactions: Record, currentSearchResults: SearchResults['data'] | undefined, + currentUserAccountID: number, selectedReports: SelectedReports[] = [], searchDataType?: SearchDataTypes, ) { @@ -6394,7 +6395,7 @@ function shouldShowDeleteOption( reportTransactions.push(item); } } - return canDeleteMoneyRequestReport(fullReport, reportTransactions, reportActionsArray); + return canDeleteMoneyRequestReport(fullReport, reportTransactions, reportActionsArray, currentUserAccountID); }) : selectedTransactionsKeys.every((id) => { const transaction = currentSearchResults?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`] ?? selectedTransactions[id]?.transaction; @@ -6408,7 +6409,7 @@ function shouldShowDeleteOption( Object.values(reportActions ?? {}).find((action) => (isMoneyRequestAction(action) ? getOriginalMessage(action)?.IOUTransactionID : undefined) === id) ?? selectedTransactions[id].reportAction; - return canDeleteMoneyRequestReport(parentReport, [transaction], parentReportAction ? [parentReportAction] : []); + return canDeleteMoneyRequestReport(parentReport, [transaction], parentReportAction ? [parentReportAction] : [], currentUserAccountID); }); } diff --git a/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx b/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx index a78ff43799c1..67e4ef6861ee 100644 --- a/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx +++ b/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx @@ -1520,7 +1520,19 @@ const ContextMenuActions: ContextMenuAction[] = [ isAnonymousAction: false, textTranslateKey: 'common.delete', icon: 'Trashcan', - shouldShow: ({type, reportAction, isArchivedRoom, isChronosReport, reportID: reportIDParam, moneyRequestAction, iouTransaction, transactions, childReportActions, isProduction}) => { + shouldShow: ({ + type, + reportAction, + isArchivedRoom, + isChronosReport, + reportID: reportIDParam, + moneyRequestAction, + iouTransaction, + transactions, + childReportActions, + isProduction, + currentUserAccountID, + }) => { // Until deleting parent threads is supported in FE, we will prevent the user from deleting a thread parent let reportID = reportIDParam; @@ -1545,7 +1557,7 @@ const ContextMenuActions: ContextMenuAction[] = [ return ( !!reportIDParam && type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && - canDeleteReportAction(moneyRequestAction ?? reportAction, reportID, iouTransaction, transactions, childReportActions) && + canDeleteReportAction(moneyRequestAction ?? reportAction, reportID, iouTransaction, transactions, childReportActions, currentUserAccountID) && !isArchivedRoom && !isChronosReport && !isMessageDeleted(reportAction) diff --git a/tests/perf-test/ReportUtils.perf-test.ts b/tests/perf-test/ReportUtils.perf-test.ts index d25c0392fe42..06c5e5ff007a 100644 --- a/tests/perf-test/ReportUtils.perf-test.ts +++ b/tests/perf-test/ReportUtils.perf-test.ts @@ -104,7 +104,7 @@ describe('ReportUtils', () => { const reportAction = {...createRandomReportAction(1), actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT} as unknown as ReportAction; await waitForBatchedUpdates(); - await measureFunction(() => canDeleteReportAction(reportAction, reportID, transaction, undefined, undefined)); + await measureFunction(() => canDeleteReportAction(reportAction, reportID, transaction, undefined, undefined, 1)); }); test('[ReportUtils] getReportRecipientAccountID on 1k participants', async () => { diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index eef4a79c44c2..be5571f354cc 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -5418,7 +5418,7 @@ describe('ReportUtils', () => { }, }); }); - expect(canDeleteMoneyRequestReport(invoiceReport, [], [])).toBe(true); + expect(canDeleteMoneyRequestReport(invoiceReport, [], [], currentUserAccountID)).toBe(true); }); it('should allow deletion if the expense report is submitted but not yet approved by anyone', async () => { @@ -5457,7 +5457,7 @@ describe('ReportUtils', () => { }); }); - expect(canDeleteMoneyRequestReport(expenseReport, [], [])).toBe(true); + expect(canDeleteMoneyRequestReport(expenseReport, [], [], currentUserAccountID)).toBe(true); }); }); @@ -8589,35 +8589,21 @@ describe('ReportUtils', () => { }); describe('canDeleteReportAction', () => { - it('should return false for delete button visibility if transaction is not allowed to be deleted', () => { - const parentReport = LHNTestUtils.getFakeReport(); - const report = LHNTestUtils.getFakeReport(); - const parentReportAction: ReportAction = { - ...LHNTestUtils.getFakeReportAction(), - message: [ - { - type: 'COMMENT', - html: 'hey', - text: 'hey', - isEdited: false, - whisperedTo: [], - isDeletedParentAction: false, - moderationDecision: { - decision: CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE, - }, - }, - ], - childReportID: report.reportID, + it('should return false for delete button visibility if transaction is not allowed to be deleted', async () => { + // Given a restricted managed-card expense on an open expense report owned by the current user + const expenseReport = { + ...LHNTestUtils.getFakeReport(), + type: CONST.REPORT.TYPE.EXPENSE, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + ownerAccountID: currentUserAccountID, }; - report.parentReportID = parentReport.reportID; - report.parentReportActionID = parentReportAction.reportActionID; - const currentReportId = ''; const transactionID = 1; const moneyRequestAction = { - ...parentReportAction, + ...LHNTestUtils.getFakeReportAction(), actorAccountID: currentUserAccountID, actionName: CONST.REPORT.ACTIONS.TYPE.IOU, - reportID: '1', + reportID: expenseReport.reportID, originalMessage: { IOUTransactionID: '1', amount: 100, @@ -8626,6 +8612,16 @@ describe('ReportUtils', () => { type: CONST.IOU.REPORT_ACTION_TYPE.PAY, paymentType: CONST.IOU.PAYMENT_TYPE.EXPENSIFY, }, + message: [ + { + type: 'COMMENT', + html: 'hey', + text: 'hey', + isEdited: false, + whisperedTo: [], + isDeletedParentAction: false, + }, + ], }; const transaction: Transaction = { @@ -8633,14 +8629,17 @@ describe('ReportUtils', () => { category: '', tag: '', created: testDate, - reportID: currentReportId, + reportID: expenseReport.reportID, managedCard: true, comment: { liabilityType: CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT, }, }; - expect(canDeleteReportAction(moneyRequestAction, currentReportId, transaction, undefined, undefined)).toBe(false); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport); + + // Then the owner cannot delete it because the card transaction's liability type restricts deletion + expect(canDeleteReportAction(moneyRequestAction, expenseReport.reportID, transaction, undefined, undefined, currentUserAccountID)).toBe(false); }); it('should return true for demo transaction', () => { @@ -8684,7 +8683,7 @@ describe('ReportUtils', () => { }, }; - expect(canDeleteReportAction(moneyRequestAction, '1', transaction, undefined, undefined)).toBe(true); + expect(canDeleteReportAction(moneyRequestAction, '1', transaction, undefined, undefined, currentUserAccountID)).toBe(true); }); it('should return false for unreported card expense imported with deleting disabled', async () => { @@ -8730,7 +8729,7 @@ describe('ReportUtils', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); // Then it should return false since the unreported card expense is imported with deleting disabled - expect(canDeleteReportAction(trackExpenseAction, selfDMReport.reportID, transaction, undefined, undefined)).toBe(false); + expect(canDeleteReportAction(trackExpenseAction, selfDMReport.reportID, transaction, undefined, undefined, currentUserAccountID)).toBe(false); }); it("should return false for ADD_COMMENT report action the current user (admin of the personal policy) didn't comment", async () => { @@ -8757,7 +8756,7 @@ describe('ReportUtils', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${adminPolicy.id}`, adminPolicy); - expect(canDeleteReportAction(reportAction, report.reportID, undefined, undefined, undefined)).toBe(false); + expect(canDeleteReportAction(reportAction, report.reportID, undefined, undefined, undefined, currentUserAccountID)).toBe(false); }); }); diff --git a/tests/unit/Search/SearchUIUtilsTest.ts b/tests/unit/Search/SearchUIUtilsTest.ts index 85b4616c4a52..dac5bba9384f 100644 --- a/tests/unit/Search/SearchUIUtilsTest.ts +++ b/tests/unit/Search/SearchUIUtilsTest.ts @@ -10603,7 +10603,7 @@ describe('SearchUIUtils', () => { await Onyx.merge(ONYXKEYS.SESSION, {accountID: TEST_ACCOUNT_ID}); - expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults)).toBe(true); + expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults, TEST_ACCOUNT_ID)).toBe(true); }); it('should show delete option for unreported expense which can be deleted', async () => { @@ -10789,7 +10789,7 @@ describe('SearchUIUtils', () => { await Onyx.merge(ONYXKEYS.SESSION, {accountID: TEST_ACCOUNT_ID}); - expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults)).toBe(true); + expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults, TEST_ACCOUNT_ID)).toBe(true); }); }); describe('getToFieldValueForTransaction', () => {