diff --git a/src/components/Search/index.tsx b/src/components/Search/index.tsx index 35e108634535..3861ac4074e6 100644 --- a/src/components/Search/index.tsx +++ b/src/components/Search/index.tsx @@ -34,7 +34,7 @@ import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTop import type {PlatformStackNavigationProp} from '@libs/Navigation/PlatformStackNavigation/types'; import Performance from '@libs/Performance'; import {isSplitAction} from '@libs/ReportSecondaryActionUtils'; -import {canAddOrDeleteTransactions, canEditFieldOfMoneyRequest, canHoldUnholdReportAction, isOneTransactionReport, selectFilteredReportActions} from '@libs/ReportUtils'; +import {canEditFieldOfMoneyRequest, canHoldUnholdReportAction, isOneTransactionReport, selectFilteredReportActions} from '@libs/ReportUtils'; import {buildCannedSearchQuery, buildSearchQueryJSON, buildSearchQueryString} from '@libs/SearchQueryUtils'; import { createAndOpenSearchTransactionThread, @@ -103,7 +103,7 @@ function mapTransactionItemToSelectedEntry( item.keyForList, { isSelected: true, - canDelete: canAddOrDeleteTransactions(item.report, item.policy), + canDelete: item.canDelete, canHold: canHoldRequest, isHeld: isOnHold(item), canUnhold: canUnholdRequest, @@ -193,7 +193,7 @@ function prepareTransactionsList( ...selectedTransactions, [item.keyForList]: { isSelected: true, - canDelete: canAddOrDeleteTransactions(item.report, item.policy), + canDelete: item.canDelete, canHold: canHoldRequest, isHeld: isOnHold(item), canUnhold: canUnholdRequest, @@ -535,8 +535,7 @@ function Search({ ), // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing isSelected: areAllMatchingItemsSelected || selectedTransactions[transactionItem.transactionID]?.isSelected || isExpenseReportType, - canDelete: canAddOrDeleteTransactions(transactionItem.report, transactionItem.policy), - + canDelete: transactionItem.canDelete, reportID: transactionItem.reportID, policyID: transactionItem.report?.policyID, amount: transactionItem.modifiedAmount ?? transactionItem.amount, @@ -586,8 +585,7 @@ function Search({ ), // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing isSelected: areAllMatchingItemsSelected || selectedTransactions[transactionItem.transactionID].isSelected, - canDelete: canAddOrDeleteTransactions(transactionItem.report, transactionItem.policy), - + canDelete: transactionItem.canDelete, reportID: transactionItem.reportID, policyID: transactionItem.report?.policyID, amount: transactionItem.modifiedAmount ?? transactionItem.amount, diff --git a/src/hooks/useSelectedTransactionsActions.ts b/src/hooks/useSelectedTransactionsActions.ts index a4b4e0ec106b..9d1f322d1619 100644 --- a/src/hooks/useSelectedTransactionsActions.ts +++ b/src/hooks/useSelectedTransactionsActions.ts @@ -9,8 +9,8 @@ import Navigation from '@libs/Navigation/Navigation'; import {getIOUActionForTransactionID, getReportAction, isDeletedAction} from '@libs/ReportActionsUtils'; import {isMergeAction, isSplitAction} from '@libs/ReportSecondaryActionUtils'; import { - canAddOrDeleteTransactions, canDeleteCardTransactionByLiabilityType, + canDeleteTransaction, canEditFieldOfMoneyRequest, canHoldUnholdReportAction, canUserPerformWriteAction as canUserPerformWriteActionReportUtils, @@ -335,7 +335,7 @@ function useSelectedTransactionsActions({ return canRemoveTransaction && isIOUActionOwner && !isActionDeleted; }); - const canRemoveReportTransaction = canAddOrDeleteTransactions(report, policy, isReportArchived); + const canRemoveReportTransaction = canDeleteTransaction(report, isReportArchived); if (canRemoveReportTransaction && canAllSelectedTransactionsBeRemoved) { options.push({ diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 270739561a2e..6db1882c8728 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -33,7 +33,7 @@ import { hasOnlyNonReimbursableTransactions, hasReportBeenReopened as hasReportBeenReopenedUtils, hasReportBeenRetracted as hasReportBeenRetractedUtils, - isArchivedReport, // eslint-disable-next-line @typescript-eslint/no-deprecated + isArchivedReport, isAwaitingFirstLevelApproval, isClosedReport as isClosedReportUtils, isCurrentUserSubmitter, @@ -125,7 +125,6 @@ function isSplitAction(report: OnyxEntry, reportTransactions: Array | string): boolean { /** * Checks if a report is an IOU report using report */ -function isIOUReportUsingReport(report: OnyxEntry): boolean { +function isIOUReportUsingReport(report: OnyxEntry): report is Report { return report?.type === CONST.REPORT.TYPE.IOU; } @@ -1966,9 +1963,6 @@ function requiresManualSubmission(report: OnyxEntry, policy: OnyxEntry

): boolean { if (!report) { return false; @@ -1981,63 +1975,6 @@ function isAwaitingFirstLevelApproval(report: OnyxEntry): boolean { return isProcessingReport(report) && submitsToAccountID === report.managerID; } -function isAwaitingFirstLevelApprovalNew(report: OnyxEntry, reportActions: ReportAction[], policy: OnyxEntry): boolean { - if (!report) { - return false; - } - - if (!isProcessingReport(report)) { - return false; - } - - if (isIOUReportUsingReport(report)) { - return true; - } - - if (hasDynamicExternalWorkflow(policy)) { - return false; - } - - if (policy?.approvalMode === CONST.POLICY.APPROVAL_MODE.BASIC) { - return true; - } - - // If the report is part of a policy with Instant Submit, this data should be stored in the CREATED action - // as Instant Submit reports do not have a SUBMITTED action. - // For all other cases, use the most recent SUBMITTED action instead. - const usedReportAction = isInstantSubmitEnabled(policy) - ? reportActions?.find((action) => action.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED) - : reportActions - ?.filter((action) => action.actionName === CONST.REPORT.ACTIONS.TYPE.SUBMITTED) - ?.sort((a, b) => { - if (!a.created || !b.created) { - return !a.created ? 1 : -1; - } - return a.created < b.created ? 1 : -1; - }) - ?.at(0); - - const originalMessage = getOriginalMessage(usedReportAction); - if (!originalMessage) { - return false; - } - let submittedTo: number | undefined = 'submittedTo' in originalMessage ? originalMessage?.submittedTo : undefined; - - if (!submittedTo) { - const submittedToLogin = 'to' in originalMessage ? (originalMessage?.to ?? '') : ''; - submittedTo = getAccountIDsByLogins([submittedToLogin])?.at(0); - } - - if (!submittedTo) { - const managerID = 'managerOnVacation' in originalMessage && originalMessage?.managerOnVacation ? originalMessage?.managerOnVacation : report.managerID; - const approverAccountID = - policy?.employeeList?.[getLoginByAccountID(report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID) ?? '']?.submitsTo ?? getAccountIDsByLogins([getDefaultApprover(policy)])?.at(0); - return managerID === approverAccountID; - } - - return report.managerID === submittedTo; -} - /** * Updates optimistic transaction violations to OnyxData for the given policy and categories onyx update. * @@ -2747,17 +2684,21 @@ function getChildReportNotificationPreference(reportAction: OnyxInputOrEntry, policy?: OnyxEntry, isReportArchived = false): boolean { +function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry, isReportArchived = false): boolean { if (!isMoneyRequestReport(moneyRequestReport) || isReportArchived) { return false; } + // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 + // eslint-disable-next-line @typescript-eslint/no-deprecated + const policy = getPolicy(moneyRequestReport?.policyID); + // Adding or deleting transactions is not allowed on a closed report if (moneyRequestReport?.statusNum === CONST.REPORT.STATUS_NUM.CLOSED && !isOpenReport(moneyRequestReport)) { return false; } if (isInstantSubmitEnabled(policy) && isProcessingReport(moneyRequestReport)) { - return isAwaitingFirstLevelApprovalNew(moneyRequestReport, Object.values(allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${moneyRequestReport?.reportID}`] ?? {}), policy); + return isAwaitingFirstLevelApproval(moneyRequestReport); } if (isReportApproved({report: moneyRequestReport}) || isClosedReport(moneyRequestReport) || isSettled(moneyRequestReport?.reportID)) { @@ -2786,7 +2727,17 @@ function canAddTransaction(moneyRequestReport: OnyxEntry, isReportArchiv return false; } - return canAddOrDeleteTransactions(moneyRequestReport, policy, isReportArchived); + return canAddOrDeleteTransactions(moneyRequestReport, isReportArchived); +} + +/** + * Checks whether the supplied report supports deleting more transactions from it. + * Return true if: + * - report is a non-settled IOU + * - report is a non-approved IOU + */ +function canDeleteTransaction(moneyRequestReport: OnyxEntry, isReportArchived = false): boolean { + return canAddOrDeleteTransactions(moneyRequestReport, isReportArchived); } /** @@ -2816,7 +2767,6 @@ function isMoneyRequestReportEligibleForMerge(reportID: string, isAdmin: boolean } if (isSubmitter) { - // eslint-disable-next-line @typescript-eslint/no-deprecated return isOpenReport(report) || (isIOUReport(report) && isProcessingReport(report)) || isAwaitingFirstLevelApproval(report); } @@ -3003,7 +2953,7 @@ function canDeleteReportAction( if (isActionOwner) { if (!isEmptyObject(report) && (isMoneyRequestReport(report) || isInvoiceReport(report))) { - return canAddOrDeleteTransactions(report, policy ?? undefined) && canCardTransactionBeDeleted; + return canDeleteTransaction(report) && canCardTransactionBeDeleted; } if (isTrackExpenseAction(reportAction)) { return canCardTransactionBeDeleted; @@ -4715,7 +4665,6 @@ function canEditReportPolicy(report: OnyxEntry, reportPolicy: OnyxEntry< } if (isSubmitted) { - // eslint-disable-next-line @typescript-eslint/no-deprecated return (isSubmitter && isAwaitingFirstLevelApproval(report)) || isManager || isAdmin; } @@ -13038,7 +12987,7 @@ export { canAccessReport, isReportNotFound, canAddTransaction, - canAddOrDeleteTransactions, + canDeleteTransaction, canBeAutoReimbursed, canCreateRequest, canCreateTaskInReport, @@ -13254,9 +13203,7 @@ export { isOpenReport, requiresManualSubmission, isReportIDApproved, - // eslint-disable-next-line @typescript-eslint/no-deprecated isAwaitingFirstLevelApproval, - isAwaitingFirstLevelApprovalNew, isPublicAnnounceRoom, isPublicRoom, isReportApproved, diff --git a/src/libs/SearchUIUtils.ts b/src/libs/SearchUIUtils.ts index 2eb6de363bbc..860fc1c53051 100644 --- a/src/libs/SearchUIUtils.ts +++ b/src/libs/SearchUIUtils.ts @@ -2659,6 +2659,7 @@ function getTransactionFromTransactionListItem(item: TransactionListItemType): O isTaxAmountColumnWide, violations, hash, + canDelete, accountID, policyID, ...transaction diff --git a/src/pages/ReportDetailsPage.tsx b/src/pages/ReportDetailsPage.tsx index 8c67e7d911a9..7eca9b3f2dce 100644 --- a/src/pages/ReportDetailsPage.tsx +++ b/src/pages/ReportDetailsPage.tsx @@ -49,8 +49,8 @@ import {isPolicyAdmin as isPolicyAdminUtil, isPolicyEmployee as isPolicyEmployee import {getOneTransactionThreadReportID, getOriginalMessage, getTrackExpenseActionableWhisper, isDeletedAction, isMoneyRequestAction, isTrackExpenseAction} from '@libs/ReportActionsUtils'; import {getReportName as getReportNameFromReportNameUtils} from '@libs/ReportNameUtils'; import { - canAddOrDeleteTransactions, canDeleteCardTransactionByLiabilityType, + canDeleteTransaction, canEditReportDescription as canEditReportDescriptionUtil, canJoinChat, canLeaveChat, @@ -291,7 +291,7 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail !isClosedReport(report) && isTaskModifiable && isTaskActionable; - const canDeleteRequest = isActionOwner && (canAddOrDeleteTransactions(moneyRequestReport, policy, isMoneyRequestReportArchived) || isSelfDMTrackExpenseReport) && !isDeletedParentAction; + const canDeleteRequest = isActionOwner && (canDeleteTransaction(moneyRequestReport, isMoneyRequestReportArchived) || isSelfDMTrackExpenseReport) && !isDeletedParentAction; const iouTransactionID = isMoneyRequestAction(requestParentReportAction) ? getOriginalMessage(requestParentReportAction)?.IOUTransactionID : undefined; const [iouTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${iouTransactionID}`, {canBeMissing: true}); const {duplicateTransactions, duplicateTransactionViolations} = useDuplicateTransactionsAndViolations(iouTransactionID ? [iouTransactionID] : []); diff --git a/src/types/onyx/SearchResults.ts b/src/types/onyx/SearchResults.ts index 8b90d653cbbb..08b66a5086af 100644 --- a/src/types/onyx/SearchResults.ts +++ b/src/types/onyx/SearchResults.ts @@ -86,6 +86,9 @@ type SearchTransaction = { /** The transaction amount */ amount: number; + /** If the transaction can be deleted */ + canDelete: boolean; + /** The edited transaction amount */ modifiedAmount: number; diff --git a/tests/unit/MoneyRequestReportUtilsTest.ts b/tests/unit/MoneyRequestReportUtilsTest.ts index 9cae1e021d32..7bfdb2105183 100644 --- a/tests/unit/MoneyRequestReportUtilsTest.ts +++ b/tests/unit/MoneyRequestReportUtilsTest.ts @@ -55,6 +55,7 @@ const transactionItemBaseMock: TransactionListItemType = { policy: policyBaseMock, reportAction: reportActionBaseMock, holdReportAction: undefined, + canDelete: true, cardID: undefined, cardName: undefined, category: '', diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index ea78a030ec28..8d42accd771a 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -30,11 +30,11 @@ import { buildOptimisticReportPreview, buildParticipantsFromAccountIDs, buildTransactionThread, - canAddOrDeleteTransactions, canAddTransaction, canCreateRequest, canDeleteMoneyRequestReport, canDeleteReportAction, + canDeleteTransaction, canEditMoneyRequest, canEditReportDescription, canEditRoomVisibility, @@ -2950,7 +2950,6 @@ describe('ReportUtils', () => { submitsTo: currentUserEmail, }, }, - approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC, }; Promise.all([ Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${paidPolicy.id}`, paidPolicy), @@ -5985,7 +5984,7 @@ describe('ReportUtils', () => { }); }); - describe('canAddOrDeleteTransactions', () => { + describe('canDeleteTransaction', () => { it('should return true for a non-archived report', async () => { // Given a non-archived expense report const report: Report = { @@ -5997,7 +5996,7 @@ describe('ReportUtils', () => { // When it's checked if the transactions can be deleted // Simulate how components determined if a report is archived by using this hook const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.reportID)); - const result = canAddOrDeleteTransactions(report, policy, isReportArchived.current); + const result = canDeleteTransaction(report, isReportArchived.current); // Then the result is true expect(result).toBe(true); @@ -6014,7 +6013,7 @@ describe('ReportUtils', () => { // When it's checked if the transactions can be deleted const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.reportID)); - const result = canAddOrDeleteTransactions(report, policy, isReportArchived.current); + const result = canDeleteTransaction(report, isReportArchived.current); // Then the result is false expect(result).toBe(false); @@ -6063,7 +6062,7 @@ describe('ReportUtils', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${openReport.reportID}`, openReport); - expect(canAddOrDeleteTransactions(openReport, policy, false)).toBe(true); + expect(canDeleteTransaction(openReport, false)).toBe(true); }); it('should return false for closed report when workflow is disabled', async () => { @@ -6076,7 +6075,7 @@ describe('ReportUtils', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${closedReport.reportID}`, closedReport); - expect(canAddOrDeleteTransactions(closedReport, policy, false)).toBe(false); + expect(canDeleteTransaction(closedReport, false)).toBe(false); }); }); }); diff --git a/tests/unit/Search/SearchUIUtilsTest.ts b/tests/unit/Search/SearchUIUtilsTest.ts index aaa53367f92c..37ba61eb501e 100644 --- a/tests/unit/Search/SearchUIUtilsTest.ts +++ b/tests/unit/Search/SearchUIUtilsTest.ts @@ -386,6 +386,7 @@ const searchResults: OnyxTypes.SearchResults = { [`report_${reportID5}`]: report5, [`transactions_${transactionID}`]: { amount: -5000, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -415,6 +416,7 @@ const searchResults: OnyxTypes.SearchResults = { }, [`transactions_${transactionID2}`]: { amount: -5000, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -445,6 +447,7 @@ const searchResults: OnyxTypes.SearchResults = { ...allViolations, [`transactions_${transactionID3}`]: { amount: 1200, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -474,6 +477,7 @@ const searchResults: OnyxTypes.SearchResults = { }, [`transactions_${transactionID4}`]: { amount: 3200, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -759,6 +763,7 @@ const transactionsListItems = [ policy, reportAction: reportAction1, holdReportAction: undefined, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -810,6 +815,7 @@ const transactionsListItems = [ policy, reportAction: reportAction2, holdReportAction: undefined, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -871,6 +877,7 @@ const transactionsListItems = [ policy, reportAction: reportAction3, holdReportAction: undefined, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -927,6 +934,7 @@ const transactionsListItems = [ policy, reportAction: reportAction4, holdReportAction: undefined, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -1020,6 +1028,7 @@ const transactionReportGroupListItems = [ reportAction: reportAction1, holdReportAction: undefined, amount: -5000, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -1114,6 +1123,7 @@ const transactionReportGroupListItems = [ reportAction: reportAction2, holdReportAction: undefined, amount: -5000, + canDelete: true, cardID: undefined, cardName: undefined, category: '', @@ -1696,7 +1706,7 @@ describe('SearchUIUtils', () => { expect(distanceTransaction).toBeDefined(); expect(distanceTransaction?.iouRequestType).toBe(CONST.IOU.REQUEST_TYPE.DISTANCE); - const expectedPropertyCount = 45; + const expectedPropertyCount = 46; expect(Object.keys(distanceTransaction ?? {}).length).toBe(expectedPropertyCount); }); @@ -1729,7 +1739,7 @@ describe('SearchUIUtils', () => { expect(distanceTransaction).toBeDefined(); expect(distanceTransaction?.iouRequestType).toBe(CONST.IOU.REQUEST_TYPE.DISTANCE); - const expectedPropertyCount = 45; + const expectedPropertyCount = 46; expect(Object.keys(distanceTransaction ?? {}).length).toBe(expectedPropertyCount); }); @@ -2321,6 +2331,7 @@ describe('SearchUIUtils', () => { // eslint-disable-next-line @typescript-eslint/naming-convention transactions_1805965960759424086: { amount: 0, + canDelete: false, category: 'Employee Meals Remote (Fringe Benefit)', comment: { comment: '', @@ -2443,6 +2454,7 @@ describe('SearchUIUtils', () => { // eslint-disable-next-line @typescript-eslint/naming-convention transactions_1805965960759424086: { amount: 0, + canDelete: false, cardID: undefined, cardName: undefined, category: 'Employee Meals Remote (Fringe Benefit)', diff --git a/tests/unit/Search/handleActionButtonPressTest.ts b/tests/unit/Search/handleActionButtonPressTest.ts index 5eda3218fc48..4deb193a807c 100644 --- a/tests/unit/Search/handleActionButtonPressTest.ts +++ b/tests/unit/Search/handleActionButtonPressTest.ts @@ -181,6 +181,7 @@ const mockReportItemWithHold = { action: 'view', allActions: ['view'], amount: -12300, + canDelete: true, category: '', comment: { comment: '', diff --git a/tests/unit/TransactionGroupListItemTest.tsx b/tests/unit/TransactionGroupListItemTest.tsx index 05cc0ab57376..6e360fdafeb7 100644 --- a/tests/unit/TransactionGroupListItemTest.tsx +++ b/tests/unit/TransactionGroupListItemTest.tsx @@ -26,6 +26,7 @@ jest.mock('@libs/SearchUIUtils', () => ({ const mockTransaction: TransactionListItemType = { accountID: 1, amount: 0, + canDelete: true, category: '', groupAmount: 1284, groupCurrency: 'USD', diff --git a/tests/unit/hooks/useSelectedTransactionsActions.test.ts b/tests/unit/hooks/useSelectedTransactionsActions.test.ts index 6450e46df997..31738985991b 100644 --- a/tests/unit/hooks/useSelectedTransactionsActions.test.ts +++ b/tests/unit/hooks/useSelectedTransactionsActions.test.ts @@ -292,7 +292,7 @@ describe('useSelectedTransactionsActions', () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction); jest.spyOn(require('@libs/ReportUtils'), 'canDeleteCardTransactionByLiabilityType').mockReturnValue(true); - jest.spyOn(require('@libs/ReportUtils'), 'canAddOrDeleteTransactions').mockReturnValue(true); + jest.spyOn(require('@libs/ReportUtils'), 'canDeleteTransaction').mockReturnValue(true); jest.spyOn(require('@libs/ReportActionsUtils'), 'isDeletedAction').mockReturnValue(false); jest.spyOn(require('@libs/ReportActionsUtils'), 'getIOUActionForTransactionID').mockReturnValue(reportActions.at(0) as OnyxEntry);