Skip to content

Commit 72d2ae3

Browse files
luacmartinsluacmartins
authored
Merge pull request #77486 from nkdengineer/fix/77148
fix: Delete option is present for approved expense
2 parents 9b7e5d3 + 6be31d1 commit 72d2ae3

3 files changed

Lines changed: 85 additions & 50 deletions

File tree

‎src/libs/ReportUtils.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2725,16 +2725,13 @@ function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry<Report>, isRep
27252725
if (!isMoneyRequestReport(moneyRequestReport) || isReportArchived) {
27262726
return false;
27272727
}
2728-
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
2729-
// eslint-disable-next-line @typescript-eslint/no-deprecated
2730-
const policy = getPolicy(moneyRequestReport?.policyID);
27312728

27322729
// Adding or deleting transactions is not allowed on a closed report
27332730
if (moneyRequestReport?.statusNum === CONST.REPORT.STATUS_NUM.CLOSED && !isOpenReport(moneyRequestReport)) {
27342731
return false;
27352732
}
27362733

2737-
if (isInstantSubmitEnabled(policy) && isProcessingReport(moneyRequestReport)) {
2734+
if (isProcessingReport(moneyRequestReport) && isExpenseReport(moneyRequestReport)) {
27382735
return isAwaitingFirstLevelApproval(moneyRequestReport);
27392736
}
27402737

@@ -2959,7 +2956,7 @@ function canDeleteMoneyRequestReport(report: Report, reportTransactions: Transac
29592956
}
29602957

29612958
const isReportSubmitter = isCurrentUserSubmitter(report);
2962-
return isReportSubmitter && isReportOpenOrProcessing;
2959+
return isReportSubmitter && (isOpenReport(report) || (isProcessingReport(report) && isAwaitingFirstLevelApproval(report)));
29632960
}
29642961

29652962
return false;

‎tests/unit/ReportSecondaryActionUtilsTest.ts‎

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,13 +1398,15 @@ describe('getSecondaryAction', () => {
13981398
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false);
13991399
});
14001400

1401-
it('includes DELETE option for owner of single processing expense transaction', async () => {
1401+
it('includes DELETE option for owner of single processing expense transaction which is not forwarded', async () => {
14021402
const report = {
14031403
reportID: REPORT_ID,
14041404
type: CONST.REPORT.TYPE.EXPENSE,
14051405
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
1406+
managerID: APPROVER_ACCOUNT_ID,
14061407
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
14071408
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
1409+
policyID: POLICY_ID,
14081410
} as unknown as Report;
14091411

14101412
const TRANSACTION_ID = 'TRANSACTION_ID';
@@ -1414,8 +1416,17 @@ describe('getSecondaryAction', () => {
14141416
reportID: REPORT_ID,
14151417
} as unknown as Transaction;
14161418

1417-
const policy = {} as unknown as Policy;
1419+
const policy = {
1420+
id: POLICY_ID,
1421+
employeeList: {
1422+
[EMPLOYEE_EMAIL]: {
1423+
email: EMPLOYEE_EMAIL,
1424+
submitsTo: APPROVER_EMAIL,
1425+
},
1426+
},
1427+
} as unknown as Policy;
14181428
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
1429+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy);
14191430

14201431
const result = getSecondaryReportActions({
14211432
currentUserEmail: EMPLOYEE_EMAIL,
@@ -1430,13 +1441,15 @@ describe('getSecondaryAction', () => {
14301441
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true);
14311442
});
14321443

1433-
it('includes DELETE option for owner of processing expense report', async () => {
1444+
it('includes DELETE option for owner of processing expense report which is not forwarded', async () => {
14341445
const report = {
14351446
reportID: REPORT_ID,
14361447
type: CONST.REPORT.TYPE.EXPENSE,
14371448
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
1449+
managerID: APPROVER_ACCOUNT_ID,
14381450
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
14391451
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
1452+
policyID: POLICY_ID,
14401453
} as unknown as Report;
14411454

14421455
const TRANSACTION_ID = 'TRANSACTION_ID';
@@ -1452,8 +1465,17 @@ describe('getSecondaryAction', () => {
14521465
reportID: REPORT_ID,
14531466
} as unknown as Transaction;
14541467

1455-
const policy = {} as unknown as Policy;
1468+
const policy = {
1469+
id: POLICY_ID,
1470+
employeeList: {
1471+
[EMPLOYEE_EMAIL]: {
1472+
email: EMPLOYEE_EMAIL,
1473+
submitsTo: APPROVER_EMAIL,
1474+
},
1475+
},
1476+
} as unknown as Policy;
14561477
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
1478+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy);
14571479

14581480
const result = getSecondaryReportActions({
14591481
currentUserEmail: EMPLOYEE_EMAIL,
@@ -1468,11 +1490,13 @@ describe('getSecondaryAction', () => {
14681490
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true);
14691491
});
14701492

1471-
it('does not include DELETE option for corporate liability card transaction', async () => {
1493+
it('does not includes DELETE option for report that has been forwarded', async () => {
14721494
const report = {
14731495
reportID: REPORT_ID,
14741496
type: CONST.REPORT.TYPE.EXPENSE,
14751497
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
1498+
managerID: MANAGER_ACCOUNT_ID,
1499+
policyID: POLICY_ID,
14761500
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
14771501
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
14781502
} as unknown as Report;
@@ -1482,14 +1506,16 @@ describe('getSecondaryAction', () => {
14821506
const transaction = {
14831507
transactionID: TRANSACTION_ID,
14841508
reportID: REPORT_ID,
1485-
managedCard: true,
1486-
comment: {
1487-
liabilityType: CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT,
1488-
},
14891509
} as unknown as Transaction;
14901510

1491-
const policy = {} as unknown as Policy;
1511+
const policy = {
1512+
id: POLICY_ID,
1513+
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
1514+
approver: APPROVER_EMAIL,
1515+
} as unknown as Policy;
1516+
14921517
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
1518+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy);
14931519

14941520
const result = getSecondaryReportActions({
14951521
currentUserEmail: EMPLOYEE_EMAIL,
@@ -1504,44 +1530,29 @@ describe('getSecondaryAction', () => {
15041530
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false);
15051531
});
15061532

1507-
it('does not include DELETE option for unreported card expense imported with deleting disabled', async () => {
1508-
// Given the unreported card expense imported with deleting disabled
1533+
it('does not include DELETE option for corporate liability card transaction', async () => {
15091534
const report = {
15101535
reportID: REPORT_ID,
1511-
type: CONST.REPORT.TYPE.CHAT,
1512-
chatType: CONST.REPORT.CHAT_TYPE.SELF_DM,
1536+
type: CONST.REPORT.TYPE.EXPENSE,
1537+
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
15131538
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
15141539
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
1515-
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
15161540
} as unknown as Report;
15171541

15181542
const TRANSACTION_ID = 'TRANSACTION_ID';
15191543

15201544
const transaction = {
15211545
transactionID: TRANSACTION_ID,
1522-
reportID: CONST.REPORT.UNREPORTED_REPORT_ID,
1546+
reportID: REPORT_ID,
15231547
managedCard: true,
15241548
comment: {
15251549
liabilityType: CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT,
15261550
},
15271551
} as unknown as Transaction;
15281552

1529-
const reportActions = [
1530-
{
1531-
reportActionID: '1',
1532-
actorAccountID: EMPLOYEE_ACCOUNT_ID,
1533-
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
1534-
originalMessage: {
1535-
IOUTransactionID: TRANSACTION_ID,
1536-
IOUReportID: CONST.REPORT.UNREPORTED_REPORT_ID,
1537-
},
1538-
},
1539-
] as unknown as ReportAction[];
1540-
15411553
const policy = {} as unknown as Policy;
15421554
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
15431555

1544-
// Then it should return false since the unreported card expense is imported with deleting disabled
15451556
const result = getSecondaryReportActions({
15461557
currentUserEmail: EMPLOYEE_EMAIL,
15471558
currentUserAccountID: EMPLOYEE_ACCOUNT_ID,
@@ -1551,38 +1562,48 @@ describe('getSecondaryAction', () => {
15511562
originalTransaction: {} as Transaction,
15521563
violations: {},
15531564
policy,
1554-
reportActions,
15551565
});
15561566
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false);
15571567
});
15581568

1559-
it('includes DELETE option for report that has been forwarded', async () => {
1569+
it('does not include DELETE option for unreported card expense imported with deleting disabled', async () => {
1570+
// Given the unreported card expense imported with deleting disabled
15601571
const report = {
15611572
reportID: REPORT_ID,
1562-
type: CONST.REPORT.TYPE.EXPENSE,
1563-
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
1564-
managerID: MANAGER_ACCOUNT_ID,
1565-
policyID: POLICY_ID,
1573+
type: CONST.REPORT.TYPE.CHAT,
1574+
chatType: CONST.REPORT.CHAT_TYPE.SELF_DM,
15661575
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
15671576
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
1577+
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
15681578
} as unknown as Report;
15691579

15701580
const TRANSACTION_ID = 'TRANSACTION_ID';
15711581

15721582
const transaction = {
15731583
transactionID: TRANSACTION_ID,
1574-
reportID: REPORT_ID,
1584+
reportID: CONST.REPORT.UNREPORTED_REPORT_ID,
1585+
managedCard: true,
1586+
comment: {
1587+
liabilityType: CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT,
1588+
},
15751589
} as unknown as Transaction;
15761590

1577-
const policy = {
1578-
id: POLICY_ID,
1579-
approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
1580-
approver: APPROVER_EMAIL,
1581-
} as unknown as Policy;
1591+
const reportActions = [
1592+
{
1593+
reportActionID: '1',
1594+
actorAccountID: EMPLOYEE_ACCOUNT_ID,
1595+
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
1596+
originalMessage: {
1597+
IOUTransactionID: TRANSACTION_ID,
1598+
IOUReportID: CONST.REPORT.UNREPORTED_REPORT_ID,
1599+
},
1600+
},
1601+
] as unknown as ReportAction[];
15821602

1603+
const policy = {} as unknown as Policy;
15831604
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
1584-
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, policy);
15851605

1606+
// Then it should return false since the unreported card expense is imported with deleting disabled
15861607
const result = getSecondaryReportActions({
15871608
currentUserEmail: EMPLOYEE_EMAIL,
15881609
currentUserAccountID: EMPLOYEE_ACCOUNT_ID,
@@ -1592,9 +1613,11 @@ describe('getSecondaryAction', () => {
15921613
originalTransaction: {} as Transaction,
15931614
violations: {},
15941615
policy,
1616+
reportActions,
15951617
});
1596-
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(true);
1618+
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.DELETE)).toBe(false);
15971619
});
1620+
15981621
it('include DELETE option for demo transaction', async () => {
15991622
const report = {
16001623
reportID: REPORT_ID,

‎tests/unit/ReportUtilsTest.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3018,7 +3018,7 @@ describe('ReportUtils', () => {
30183018
type: CONST.POLICY.TYPE.TEAM,
30193019
name: '',
30203020
role: 'user',
3021-
owner: '',
3021+
owner: currentUserEmail,
30223022
outputCurrency: '',
30233023
isPolicyExpenseChatEnabled: false,
30243024
};
@@ -3038,6 +3038,7 @@ describe('ReportUtils', () => {
30383038
parentReportID: '101',
30393039
policyID: paidPolicy.id,
30403040
ownerAccountID: currentUserAccountID,
3041+
managerID: currentUserAccountID,
30413042
};
30423043
const moneyRequestOptions = temporary_getMoneyRequestOptions(report, paidPolicy, [currentUserAccountID, participantsAccountIDs.at(0) ?? CONST.DEFAULT_NUMBER_ID]);
30433044
expect(moneyRequestOptions.length).toBe(2);
@@ -3752,13 +3753,26 @@ describe('ReportUtils', () => {
37523753
reportID: '1',
37533754
type: CONST.REPORT.TYPE.EXPENSE,
37543755
ownerAccountID: currentUserAccountID,
3756+
managerID: currentUserAccountID,
37553757
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
37563758
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
37573759
participants: {
37583760
[currentUserAccountID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS},
37593761
},
3762+
policyID: '11',
37603763
};
3761-
// Wait for Onyx to load session data before calling canDeleteMoneyRequestReport,
3764+
3765+
const expenseReportPolicy = {
3766+
id: '11',
3767+
employeeList: {
3768+
[currentUserEmail]: {
3769+
email: currentUserEmail,
3770+
submitsTo: currentUserEmail,
3771+
},
3772+
},
3773+
};
3774+
3775+
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}11`, expenseReportPolicy); // Wait for Onyx to load session data before calling canDeleteMoneyRequestReport,
37623776
// since it relies on the session subscription for currentUserAccountID.
37633777
await new Promise<void>((resolve) => {
37643778
const connection = Onyx.connectWithoutView({
@@ -3769,6 +3783,7 @@ describe('ReportUtils', () => {
37693783
},
37703784
});
37713785
});
3786+
37723787
expect(canDeleteMoneyRequestReport(expenseReport, [], [])).toBe(true);
37733788
});
37743789
});

0 commit comments

Comments
 (0)