Skip to content

Revert "Fix delete conditions" #63294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aldo-expensify wants to merge 1 commit into from
Closed

Revert "Fix delete conditions" #63294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 15 additions & 31 deletions src/libs/ReportSecondaryActionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
hasOnlyHeldExpenses,
hasReportBeenReopened as hasReportBeenReopenedUtils,
isArchivedReport,
isAwaitingFirstLevelApproval,
isClosedReport as isClosedReportUtils,
isCurrentUserSubmitter,
isExpenseReport as isExpenseReportUtils,
Expand All @@ -39,7 +38,6 @@ import {
isProcessingReport as isProcessingReportUtils,
isReportApproved as isReportApprovedUtils,
isReportManager as isReportManagerUtils,
isSelfDM as isSelfDMReportUtils,
isSettled,
isWorkspaceEligibleForReportChange,
} from './ReportUtils';
Expand All @@ -48,7 +46,6 @@ import {
allHavePendingRTERViolation,
getOriginalTransactionWithSplitInfo,
hasReceipt as hasReceiptTransactionUtils,
isCardTransaction as isCardTransactionUtils,
isDuplicate,
isOnHold as isOnHoldTransactionUtils,
isPending,
Expand Down Expand Up @@ -405,42 +402,29 @@ function isMoveTransactionAction(reportTransactions: Transaction[], reportAction
return canMoveExpense;
}

function isDeleteAction(report: Report, reportTransactions: Transaction[], reportActions: ReportAction[], policy?: Policy): boolean {
function isDeleteAction(report: Report): boolean {
const isExpenseReport = isExpenseReportUtils(report);
const isIOUReport = isIOUReportUtils(report);
const isUnreported = isSelfDMReportUtils(report);
const transaction = reportTransactions.at(0);
const transactionID = transaction?.transactionID;
const isOwner = transactionID ? getIOUActionForTransactionID(reportActions, transactionID)?.actorAccountID === getCurrentUserAccountID() : false;
const isReportOpenOrProcessing = isOpenReportUtils(report) || isProcessingReportUtils(report);
const isSingleTransaction = 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 && !isIOUReport) {
return false;
}

if (isExpenseReport) {
const isCardTransactionWithCorporateLiability =
isSingleTransaction && isCardTransactionUtils(transaction) && transaction?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT;
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);

if (isCardTransactionWithCorporateLiability) {
return false;
}
if (!isReportSubmitter) {
return false;
}

const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false;
const isForwarded = isProcessingReportUtils(report) && isApprovalEnabled && !isAwaitingFirstLevelApproval(report);
const isReportOpen = isOpenReportUtils(report);
const isProcessingReport = isProcessingReportUtils(report);
const isReportApproved = isReportApprovedUtils({report});

return isReportSubmitter && isReportOpenOrProcessing && !isForwarded;
if (isReportApproved) {
return false;
}

return false;
return isReportOpen || isProcessingReport;
}

function isRetractAction(report: Report, policy?: Policy): boolean {
Expand Down Expand Up @@ -562,7 +546,7 @@ function getSecondaryReportActions(

options.push(CONST.REPORT.SECONDARY_ACTIONS.VIEW_DETAILS);

if (isDeleteAction(report, reportTransactions, reportActions ?? [], policy)) {
if (isDeleteAction(report)) {
options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE);
}

Expand All @@ -588,7 +572,7 @@ function getSecondaryTransactionThreadActions(

options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.VIEW_DETAILS);

if (isDeleteAction(parentReport, [reportTransaction], reportActions ?? [])) {
if (isDeleteAction(parentReport)) {
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.DELETE);
}

Expand Down
228 changes: 1 addition & 227 deletions tests/unit/ReportSecondaryActionUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, [APPROVER_ACCOUNT_ID]: {accountID: APPROVER_ACCOUNT_ID, login: APPROVER_EMAIL}});
await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS});
});

it('should always return default options', () => {
Expand Down Expand Up @@ -654,232 +654,6 @@ 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 policy = {} as unknown as Policy;

const result = getSecondaryReportActions(report, [transaction], {}, policy, 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);
});

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', () => {
Expand Down