From a88cc24041207e8cf7f597e2eb0b447f05ec92a8 Mon Sep 17 00:00:00 2001 From: Brandon Stites <42391420+stitesExpensify@users.noreply.github.com> Date: Thu, 18 Dec 2025 11:25:26 -0500 Subject: [PATCH] Revert "fix: Delete option is present for approved expense" --- src/libs/ReportUtils.ts | 7 +- tests/unit/ReportSecondaryActionUtilsTest.ts | 109 ++++++++----------- tests/unit/ReportUtilsTest.ts | 19 +--- 3 files changed, 50 insertions(+), 85 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 621ffaa2ce728..b7234382a6bef 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -2725,13 +2725,16 @@ function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry, isRep 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 (isProcessingReport(moneyRequestReport) && isExpenseReport(moneyRequestReport)) { + if (isInstantSubmitEnabled(policy) && isProcessingReport(moneyRequestReport)) { return isAwaitingFirstLevelApproval(moneyRequestReport); } @@ -2959,7 +2962,7 @@ function canDeleteMoneyRequestReport(report: Report, reportTransactions: Transac } const isReportSubmitter = isCurrentUserSubmitter(report); - return isReportSubmitter && (isOpenReport(report) || (isProcessingReport(report) && isAwaitingFirstLevelApproval(report))); + return isReportSubmitter && isReportOpenOrProcessing; } return false; diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index 0510967f30cf5..ff9e0f3d60a6a 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -1398,15 +1398,13 @@ describe('getSecondaryAction', () => { expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); }); - it('includes DELETE option for owner of single processing expense transaction which is not forwarded', async () => { + 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, - managerID: APPROVER_ACCOUNT_ID, statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, - policyID: POLICY_ID, } as unknown as Report; const TRANSACTION_ID = 'TRANSACTION_ID'; @@ -1416,17 +1414,8 @@ describe('getSecondaryAction', () => { reportID: REPORT_ID, } as unknown as Transaction; - const policy = { - id: POLICY_ID, - employeeList: { - [EMPLOYEE_EMAIL]: { - email: EMPLOYEE_EMAIL, - submitsTo: APPROVER_EMAIL, - }, - }, - } as unknown as Policy; + const policy = {} 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({ currentUserEmail: EMPLOYEE_EMAIL, @@ -1441,15 +1430,13 @@ describe('getSecondaryAction', () => { expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); }); - it('includes DELETE option for owner of processing expense report which is not forwarded', async () => { + 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, - managerID: APPROVER_ACCOUNT_ID, statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, - policyID: POLICY_ID, } as unknown as Report; const TRANSACTION_ID = 'TRANSACTION_ID'; @@ -1465,17 +1452,8 @@ describe('getSecondaryAction', () => { reportID: REPORT_ID, } as unknown as Transaction; - const policy = { - id: POLICY_ID, - employeeList: { - [EMPLOYEE_EMAIL]: { - email: EMPLOYEE_EMAIL, - submitsTo: APPROVER_EMAIL, - }, - }, - } as unknown as Policy; + const policy = {} 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({ currentUserEmail: EMPLOYEE_EMAIL, @@ -1490,46 +1468,6 @@ describe('getSecondaryAction', () => { expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); }); - it('does not includes 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({ - currentUserEmail: EMPLOYEE_EMAIL, - currentUserAccountID: EMPLOYEE_ACCOUNT_ID, - report, - chatReport, - reportTransactions: [transaction], - originalTransaction: {} as Transaction, - violations: {}, - policy, - }); - expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); - }); - it('does not include DELETE option for corporate liability card transaction', async () => { const report = { reportID: REPORT_ID, @@ -1618,6 +1556,45 @@ describe('getSecondaryAction', () => { expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false); }); + it('includes 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({ + currentUserEmail: EMPLOYEE_EMAIL, + currentUserAccountID: EMPLOYEE_ACCOUNT_ID, + report, + chatReport, + reportTransactions: [transaction], + originalTransaction: {} as Transaction, + violations: {}, + policy, + }); + expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true); + }); it('include DELETE option for demo transaction', async () => { const report = { reportID: REPORT_ID, diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 06aea16993800..7daaa295ff9d3 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -3018,7 +3018,7 @@ describe('ReportUtils', () => { type: CONST.POLICY.TYPE.TEAM, name: '', role: 'user', - owner: currentUserEmail, + owner: '', outputCurrency: '', isPolicyExpenseChatEnabled: false, }; @@ -3038,7 +3038,6 @@ describe('ReportUtils', () => { parentReportID: '101', policyID: paidPolicy.id, ownerAccountID: currentUserAccountID, - managerID: currentUserAccountID, }; const moneyRequestOptions = temporary_getMoneyRequestOptions(report, paidPolicy, [currentUserAccountID, participantsAccountIDs.at(0) ?? CONST.DEFAULT_NUMBER_ID]); expect(moneyRequestOptions.length).toBe(2); @@ -3776,26 +3775,13 @@ describe('ReportUtils', () => { reportID: '1', type: CONST.REPORT.TYPE.EXPENSE, ownerAccountID: currentUserAccountID, - managerID: currentUserAccountID, stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, participants: { [currentUserAccountID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS}, }, - policyID: '11', - }; - - const expenseReportPolicy = { - id: '11', - employeeList: { - [currentUserEmail]: { - email: currentUserEmail, - submitsTo: currentUserEmail, - }, - }, }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}11`, expenseReportPolicy); // Wait for Onyx to load session data before calling canDeleteMoneyRequestReport, + // Wait for Onyx to load session data before calling canDeleteMoneyRequestReport, // since it relies on the session subscription for currentUserAccountID. await new Promise((resolve) => { const connection = Onyx.connectWithoutView({ @@ -3806,7 +3792,6 @@ describe('ReportUtils', () => { }, }); }); - expect(canDeleteMoneyRequestReport(expenseReport, [], [])).toBe(true); }); });