diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 6efa65d8eff7..2d5ea45d44f0 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -5467,7 +5467,16 @@ function canEditFieldOfMoneyRequest({ return false; } - if (isSettled(moneyRequestReport) || isReportApproved({report: moneyRequestReport})) { + // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 + const reportPolicy = policy ?? getPolicy(moneyRequestReport?.policyID); + // Moving an expense to another report weighs these two on their own rather than as part of the combined check. + const isAdmin = isExpenseReport(moneyRequestReport) && reportPolicy?.role === CONST.POLICY.ROLE.ADMIN; + const isManager = isExpenseReport(moneyRequestReport) && deprecatedCurrentUserAccountID === moneyRequestReport?.managerID; + + // Admins can add or replace a receipt on an approved expense so a missing receipt can still be provided after approval. + const isAdminChangingReceiptOnApprovedReport = isAdmin && fieldToEdit === CONST.EDIT_REQUEST_FIELD.RECEIPT && !isDeleteAction && !isSettled(moneyRequestReport); + + if (!isAdminChangingReceiptOnApprovedReport && (isSettled(moneyRequestReport) || isReportApproved({report: moneyRequestReport}))) { return false; } @@ -5479,13 +5488,8 @@ function canEditFieldOfMoneyRequest({ return false; } - // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 - const reportPolicy = policy ?? getPolicy(moneyRequestReport?.policyID); const canEditExpense = canCurrentUserEditExpense(reportAction, moneyRequestReport, reportPolicy); const isRequestor = deprecatedCurrentUserAccountID === reportAction?.actorAccountID; - // Moving an expense to another report weighs these two on their own rather than as part of the combined check. - const isAdmin = isExpenseReport(moneyRequestReport) && reportPolicy?.role === CONST.POLICY.ROLE.ADMIN; - const isManager = isExpenseReport(moneyRequestReport) && deprecatedCurrentUserAccountID === moneyRequestReport?.managerID; if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.REIMBURSABLE) { return canEditExpense; diff --git a/src/pages/inbox/report/ReportActionCompose/ComposerDropZone.tsx b/src/pages/inbox/report/ReportActionCompose/ComposerDropZone.tsx index effe586d8e99..fb03588cdb35 100644 --- a/src/pages/inbox/report/ReportActionCompose/ComposerDropZone.tsx +++ b/src/pages/inbox/report/ReportActionCompose/ComposerDropZone.tsx @@ -12,7 +12,7 @@ import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; -import {isChatRoom, isGroupChat, isInvoiceReport, isReportApproved, isSettled, temporary_getMoneyRequestOptions} from '@libs/ReportUtils'; +import {isChatRoom, isGroupChat, isInvoiceReport, isSettled, temporary_getMoneyRequestOptions} from '@libs/ReportUtils'; import {hasReceipt as hasReceiptTransactionUtils} from '@libs/TransactionUtils'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -78,9 +78,10 @@ function RichDropZone({reportID, shouldAddOrReplaceReceipt, transactionID, onAtt const hasReceipt = hasReceiptTransactionUtils(transaction); - const isSettledOrApproved = isSettled(report) || isSettled(parentReport) || isReportApproved({report}) || isReportApproved({report: parentReport}); + const isSettledReport = isSettled(report) || isSettled(parentReport); const hasMoneyRequestOptions = !!temporary_getMoneyRequestOptions(report, policy, reportParticipantIDs, betas, isReportArchived, isRestrictedToPreferredPolicy).length; - const canModifyReceipt = shouldAddOrReplaceReceipt && !isSettledOrApproved; + // Approved reports are not excluded here because shouldAddOrReplaceReceipt already limits them to admins. + const canModifyReceipt = shouldAddOrReplaceReceipt && !isSettledReport; const shouldDisplayDualDropZone = canModifyReceipt || hasMoneyRequestOptions; if (shouldDisplayDualDropZone) { diff --git a/tests/unit/canEditFieldOfMoneyRequestTest.ts b/tests/unit/canEditFieldOfMoneyRequestTest.ts index fbd5ec88a779..15d44090b5ac 100644 --- a/tests/unit/canEditFieldOfMoneyRequestTest.ts +++ b/tests/unit/canEditFieldOfMoneyRequestTest.ts @@ -4,7 +4,7 @@ import initOnyxDerivedValues from '@userActions/OnyxDerived'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Policy} from '@src/types/onyx'; +import type {Policy, Report} from '@src/types/onyx'; import {toCollectionDataSet} from '@src/types/utils/CollectionDataSet'; import Onyx from 'react-native-onyx'; @@ -539,6 +539,135 @@ describe('canEditFieldOfMoneyRequest', () => { }); }); + describe('receipt on an approved expense', () => { + const APPROVED_POLICY_ID = '55'; + const APPROVED_REPORT_ID = '66'; + const APPROVED_TRANSACTION_ID = '77'; + const EXPENSE_AMOUNT = 500; + + const approvedReportAction = { + ...createRandomReportAction(9), + reportID: APPROVED_REPORT_ID, + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + actorAccountID: currentUserAccountID, + originalMessage: { + IOUTransactionID: APPROVED_TRANSACTION_ID, + type: CONST.IOU.ACTION.CREATE, + amount: EXPENSE_AMOUNT, + currency: CONST.CURRENCY.USD, + }, + }; + + const approvedTransaction = { + ...createRandomTransaction(Number(APPROVED_TRANSACTION_ID)), + transactionID: APPROVED_TRANSACTION_ID, + reportID: APPROVED_REPORT_ID, + amount: EXPENSE_AMOUNT, + }; + + const memberPolicy: Policy = { + ...createRandomPolicy(Number(APPROVED_POLICY_ID), CONST.POLICY.TYPE.CORPORATE), + id: APPROVED_POLICY_ID, + role: CONST.POLICY.ROLE.USER, + }; + + const adminPolicy: Policy = {...memberPolicy, role: CONST.POLICY.ROLE.ADMIN}; + + const approvedReport = { + ...createExpenseReport(Number(APPROVED_REPORT_ID)), + policyID: APPROVED_POLICY_ID, + ownerAccountID: currentUserAccountID, + stateNum: CONST.REPORT.STATE_NUM.APPROVED, + statusNum: CONST.REPORT.STATUS_NUM.APPROVED, + }; + + const setUpOnyx = async (reportPolicy: Policy, report: Report = approvedReport, reportAction = approvedReportAction) => { + const policyCollectionDataSet = toCollectionDataSet(ONYXKEYS.COLLECTION.POLICY, [reportPolicy], (p) => p.id); + await Onyx.multiSet({ + [ONYXKEYS.SESSION]: {email: currentUserEmail, accountID: currentUserAccountID}, + [`${ONYXKEYS.COLLECTION.TRANSACTION}${APPROVED_TRANSACTION_ID}`]: approvedTransaction, + [`${ONYXKEYS.COLLECTION.REPORT}${APPROVED_REPORT_ID}`]: report, + ...policyCollectionDataSet, + }); + await waitForBatchedUpdates(); + return reportAction; + }; + + afterEach(() => { + Onyx.clear(); + return waitForBatchedUpdates(); + }); + + it('should return true for an admin replacing a receipt on an approved report', async () => { + const reportAction = await setUpOnyx(adminPolicy); + + const canEditReceipt = canEditFieldOfMoneyRequest({reportAction, fieldToEdit: CONST.EDIT_REQUEST_FIELD.RECEIPT, transaction: approvedTransaction}); + + expect(canEditReceipt).toBe(true); + }); + + it('should return false for the non-admin submitter replacing a receipt on an approved report', async () => { + const reportAction = await setUpOnyx(memberPolicy); + + const canEditReceipt = canEditFieldOfMoneyRequest({reportAction, fieldToEdit: CONST.EDIT_REQUEST_FIELD.RECEIPT, transaction: approvedTransaction}); + + expect(canEditReceipt).toBe(false); + }); + + it('should return false for a non-admin manager replacing a receipt on an approved report', async () => { + const reportAction = await setUpOnyx(memberPolicy, { + ...approvedReport, + ownerAccountID: secondUserAccountID, + managerID: currentUserAccountID, + }); + + const canEditReceipt = canEditFieldOfMoneyRequest({reportAction, fieldToEdit: CONST.EDIT_REQUEST_FIELD.RECEIPT, transaction: approvedTransaction}); + + expect(canEditReceipt).toBe(false); + }); + + it('should return false for an admin deleting a receipt on an approved report', async () => { + const reportAction = await setUpOnyx(adminPolicy); + + const canDeleteReceipt = canEditFieldOfMoneyRequest({ + reportAction, + fieldToEdit: CONST.EDIT_REQUEST_FIELD.RECEIPT, + isDeleteAction: true, + transaction: approvedTransaction, + }); + + expect(canDeleteReceipt).toBe(false); + }); + + it('should return false for an admin replacing a receipt on a reimbursed report', async () => { + const reportAction = await setUpOnyx(adminPolicy, { + ...approvedReport, + statusNum: CONST.REPORT.STATUS_NUM.REIMBURSED, + }); + + const canEditReceipt = canEditFieldOfMoneyRequest({reportAction, fieldToEdit: CONST.EDIT_REQUEST_FIELD.RECEIPT, transaction: approvedTransaction}); + + expect(canEditReceipt).toBe(false); + }); + + it('should keep the other restricted fields locked for an admin on an approved report', async () => { + const reportAction = await setUpOnyx(adminPolicy); + + const restrictedFields = [ + CONST.EDIT_REQUEST_FIELD.AMOUNT, + CONST.EDIT_REQUEST_FIELD.CURRENCY, + CONST.EDIT_REQUEST_FIELD.MERCHANT, + CONST.EDIT_REQUEST_FIELD.DATE, + CONST.EDIT_REQUEST_FIELD.REIMBURSABLE, + CONST.EDIT_REQUEST_FIELD.BILLABLE, + ]; + + for (const fieldToEdit of restrictedFields) { + expect(canEditFieldOfMoneyRequest({reportAction, fieldToEdit, transaction: approvedTransaction})).toBe(false); + } + }); + }); + describe('legacy unreported expense (no report action)', () => { const LEGACY_TRANSACTION_ID = '777'; const LEGACY_CUSTOM_UNIT_ID = 'legacyPerDiemUnit';