Skip to content

Commit 2583d8a

Browse files
iwizniaiwiznia
authored
Merge pull request Expensify#65530 from nkdengineer/fix/65256
2 parents d212634 + b59b9e2 commit 2583d8a

8 files changed

Lines changed: 79 additions & 49 deletions

File tree

‎src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ function MoneyRequestReportPreviewContent({
456456
if (isPaidAnimationRunning) {
457457
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY;
458458
}
459-
return getReportPreviewAction(violations, iouReport, policy, transactions, isIouReportArchived || isChatReportArchived, reportActions, invoiceReceiverPolicy);
459+
return getReportPreviewAction(violations, isIouReportArchived || isChatReportArchived, iouReport, policy, transactions, reportActions, invoiceReceiverPolicy);
460460
}, [isPaidAnimationRunning, violations, iouReport, policy, transactions, isIouReportArchived, reportActions, invoiceReceiverPolicy, isChatReportArchived]);
461461

462462
const addExpenseDropdownOptions = useMemo(

‎src/libs/ReportPreviewActionUtils.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ import {allHavePendingRTERViolation, isPending, isScanning, shouldShowBrokenConn
4343
function canSubmit(
4444
report: Report,
4545
violations: OnyxCollection<TransactionViolation[]>,
46+
isReportArchived: boolean,
4647
reportActions?: OnyxEntry<ReportActions> | ReportAction[],
4748
policy?: Policy,
4849
transactions?: Transaction[],
49-
isReportArchived = false,
5050
) {
5151
if (isReportArchived) {
5252
return false;
@@ -123,8 +123,8 @@ function canApprove(report: Report, violations: OnyxCollection<TransactionViolat
123123
function canPay(
124124
report: Report,
125125
violations: OnyxCollection<TransactionViolation[]>,
126+
isReportArchived: boolean,
126127
policy?: Policy,
127-
isReportArchived = false,
128128
invoiceReceiverPolicy?: Policy,
129129
shouldConsiderViolations = true,
130130
) {
@@ -207,7 +207,7 @@ function canExport(report: Report, violations: OnyxCollection<TransactionViolati
207207
return (isApproved || isReimbursed || isClosed) && !hasAnyViolations;
208208
}
209209

210-
function canReview(report: Report, violations: OnyxCollection<TransactionViolation[]>, policy?: Policy, transactions?: Transaction[], isReportArchived = false) {
210+
function canReview(report: Report, violations: OnyxCollection<TransactionViolation[]>, isReportArchived: boolean, policy?: Policy, transactions?: Transaction[]) {
211211
const hasAnyViolations =
212212
hasMissingSmartscanFields(report.reportID, transactions) ||
213213
hasViolations(report.reportID, violations) ||
@@ -222,7 +222,7 @@ function canReview(report: Report, violations: OnyxCollection<TransactionViolati
222222
isReimbursed ||
223223
(!(isSubmitter && isOpen && policy?.areWorkflowsEnabled) &&
224224
!canApprove(report, violations, policy, transactions, false) &&
225-
!canPay(report, violations, policy, isReportArchived, policy, false))
225+
!canPay(report, violations, isReportArchived, policy, policy, false))
226226
) {
227227
return false;
228228
}
@@ -246,10 +246,10 @@ function canReview(report: Report, violations: OnyxCollection<TransactionViolati
246246

247247
function getReportPreviewAction(
248248
violations: OnyxCollection<TransactionViolation[]>,
249+
isReportArchived: boolean,
249250
report?: Report,
250251
policy?: Policy,
251252
transactions?: Transaction[],
252-
isReportArchived = false,
253253
reportActions?: OnyxEntry<ReportActions> | ReportAction[],
254254
invoiceReceiverPolicy?: Policy,
255255
): ValueOf<typeof CONST.REPORT.REPORT_PREVIEW_ACTIONS> {
@@ -259,19 +259,19 @@ function getReportPreviewAction(
259259
if (isAddExpenseAction(report, transactions ?? [], isReportArchived)) {
260260
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE;
261261
}
262-
if (canSubmit(report, violations, reportActions, policy, transactions, isReportArchived)) {
262+
if (canSubmit(report, violations, isReportArchived, reportActions, policy, transactions)) {
263263
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT;
264264
}
265265
if (canApprove(report, violations, policy, transactions)) {
266266
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE;
267267
}
268-
if (canPay(report, violations, policy, isReportArchived, invoiceReceiverPolicy)) {
268+
if (canPay(report, violations, isReportArchived, policy, invoiceReceiverPolicy)) {
269269
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY;
270270
}
271271
if (canExport(report, violations, policy, reportActions)) {
272272
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.EXPORT_TO_ACCOUNTING;
273273
}
274-
if (canReview(report, violations, policy, transactions, isReportArchived)) {
274+
if (canReview(report, violations, isReportArchived, policy, transactions)) {
275275
return CONST.REPORT.REPORT_PREVIEW_ACTIONS.REVIEW;
276276
}
277277

‎src/libs/ReportPrimaryActionUtils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ type GetReportPrimaryActionParams = {
5555
policy?: Policy;
5656
reportNameValuePairs?: ReportNameValuePairs;
5757
reportActions?: ReportAction[];
58-
isChatReportArchived?: boolean;
58+
isChatReportArchived: boolean;
5959
invoiceReceiverPolicy?: Policy;
6060
};
6161

62-
function isAddExpenseAction(report: Report, reportTransactions: Transaction[], isChatReportArchived = false) {
62+
function isAddExpenseAction(report: Report, reportTransactions: Transaction[], isChatReportArchived: boolean) {
6363
if (isChatReportArchived) {
6464
return false;
6565
}

‎src/libs/SearchUIUtils.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -717,9 +717,15 @@ function getAction(data: OnyxTypes.SearchResults['data'], allViolations: OnyxCol
717717
const policy = getPolicyFromKey(data, report) as OnyxTypes.Policy;
718718
const {isSubmitter, isAdmin, isApprover} = getReviewerPermissionFlags(report, policy);
719719

720+
const reportNVP = getReportNameValuePairsFromKey(data, report);
721+
const isIOUReportArchived = isArchivedReport(reportNVP);
722+
723+
const chatReportRNVP = data[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.chatReportID}`] ?? undefined;
724+
const isChatReportArchived = isArchivedReport(chatReportRNVP);
725+
720726
// Only check for violations if we need to (when user has permission to review)
721727
if ((isSubmitter || isApprover || isAdmin) && hasViolations(report.reportID, allViolations, undefined, allReportTransactions)) {
722-
if (isSubmitter && !isApprover && !isAdmin && !canReview(report, allViolations, policy, allReportTransactions)) {
728+
if (isSubmitter && !isApprover && !isAdmin && !canReview(report, allViolations, isIOUReportArchived || isChatReportArchived, policy, allReportTransactions)) {
723729
return CONST.SEARCH.ACTION_TYPES.VIEW;
724730
}
725731
return CONST.SEARCH.ACTION_TYPES.REVIEW;
@@ -736,7 +742,6 @@ function getAction(data: OnyxTypes.SearchResults['data'], allViolations: OnyxCol
736742
: undefined;
737743

738744
const chatReport = getChatReport(data, report);
739-
const chatReportRNVP = data[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.chatReportID}`] ?? undefined;
740745
const canBePaid = canIOUBePaid(report, chatReport, policy, allReportTransactions, false, chatReportRNVP, invoiceReceiverPolicy);
741746

742747
if (canBePaid && !hasOnlyHeldExpenses(report.reportID, allReportTransactions)) {
@@ -759,11 +764,8 @@ function getAction(data: OnyxTypes.SearchResults['data'], allViolations: OnyxCol
759764
return CONST.SEARCH.ACTION_TYPES.APPROVE;
760765
}
761766

762-
const reportNVP = getReportNameValuePairsFromKey(data, report);
763-
const isArchived = isArchivedReport(reportNVP);
764-
765767
// We check for isAllowedToApproveExpenseReport because if the policy has preventSelfApprovals enabled, we disable the Submit action and in that case we want to show the View action instead
766-
if (canSubmitReport(report, policy, allReportTransactions, allViolations, isArchived) && isAllowedToApproveExpenseReport) {
768+
if (canSubmitReport(report, policy, allReportTransactions, allViolations, isIOUReportArchived || isChatReportArchived) && isAllowedToApproveExpenseReport) {
767769
return CONST.SEARCH.ACTION_TYPES.SUBMIT;
768770
}
769771

‎src/libs/actions/IOU.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9397,7 +9397,7 @@ function canSubmitReport(
93979397
policy: OnyxEntry<OnyxTypes.Policy> | SearchPolicy,
93989398
transactions: OnyxTypes.Transaction[] | SearchTransaction[],
93999399
allViolations: OnyxCollection<OnyxTypes.TransactionViolations> | undefined,
9400-
isReportArchived = false,
9400+
isReportArchived: boolean,
94019401
) {
94029402
const currentUserAccountID = getCurrentUserAccountID();
94039403
const isOpenExpenseReport = isOpenExpenseReportReportUtils(report);

‎tests/actions/ReportPreviewActionUtilsTest.ts‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ describe('getReportPreviewAction', () => {
6868
} as unknown as Report;
6969
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
7070

71-
expect(getReportPreviewAction(VIOLATIONS, report, undefined, [])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE);
71+
expect(getReportPreviewAction(VIOLATIONS, false, report, undefined, [])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE);
7272
});
7373

7474
it('canSubmit should return true for expense preview report with manual submit', async () => {
@@ -94,7 +94,7 @@ describe('getReportPreviewAction', () => {
9494

9595
// Simulate how components use a hook to pass the isReportArchived parameter
9696
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
97-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT);
97+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT);
9898
});
9999

100100
it('canSubmit should return false for expense preview report with only pending transactions', async () => {
@@ -124,7 +124,7 @@ describe('getReportPreviewAction', () => {
124124

125125
// Simulate how components use a hook to pass the isReportArchived parameter
126126
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
127-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
127+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
128128
});
129129

130130
describe('canApprove', () => {
@@ -151,7 +151,7 @@ describe('getReportPreviewAction', () => {
151151
} as unknown as Transaction;
152152

153153
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
154-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE);
154+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE);
155155
});
156156

157157
it('should return false for report with scanning expenses', async () => {
@@ -179,7 +179,7 @@ describe('getReportPreviewAction', () => {
179179
},
180180
} as unknown as Transaction;
181181

182-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], false)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
182+
expect(getReportPreviewAction(VIOLATIONS, false, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
183183
});
184184

185185
it('should return false for report with pending expenses', async () => {
@@ -208,7 +208,7 @@ describe('getReportPreviewAction', () => {
208208
date: '2025-01-01',
209209
} as unknown as Transaction;
210210

211-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], false)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
211+
expect(getReportPreviewAction(VIOLATIONS, false, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
212212
});
213213
});
214214

@@ -235,7 +235,7 @@ describe('getReportPreviewAction', () => {
235235
} as unknown as Transaction;
236236

237237
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
238-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE);
238+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE);
239239
});
240240

241241
it('canPay should return true for expense report with payments enabled', async () => {
@@ -259,7 +259,7 @@ describe('getReportPreviewAction', () => {
259259
} as unknown as Transaction;
260260

261261
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
262-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY);
262+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY);
263263
});
264264

265265
it('canPay should return true for submitted invoice', async () => {
@@ -287,7 +287,7 @@ describe('getReportPreviewAction', () => {
287287
} as unknown as Transaction;
288288

289289
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
290-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current, undefined, invoiceReceiverPolicy)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY);
290+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction], undefined, invoiceReceiverPolicy)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY);
291291
});
292292

293293
it('getReportPreviewAction should return VIEW action for zero value invoice', async () => {
@@ -332,7 +332,7 @@ describe('getReportPreviewAction', () => {
332332

333333
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report.parentReportID));
334334

335-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current, undefined, invoiceReceiverPolicy)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
335+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction], undefined, invoiceReceiverPolicy)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
336336
});
337337

338338
it('canPay should return false for archived invoice', async () => {
@@ -364,7 +364,7 @@ describe('getReportPreviewAction', () => {
364364
reportID: `${REPORT_ID}`,
365365
} as unknown as Transaction;
366366
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
367-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current, undefined, invoiceReceiverPolicy)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY);
367+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction], undefined, invoiceReceiverPolicy)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY);
368368
});
369369

370370
it('getReportPreviewAction should return VIEW action for invoice when the chat report is archived', async () => {
@@ -392,7 +392,7 @@ describe('getReportPreviewAction', () => {
392392
const {result: isChatReportArchived} = renderHook(() => useReportIsArchived(report?.chatReportID));
393393

394394
// Then the getReportPreviewAction should return the View action
395-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isChatReportArchived.current, undefined)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
395+
expect(getReportPreviewAction(VIOLATIONS, isChatReportArchived.current, report, policy, [transaction], undefined)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
396396
});
397397

398398
it('canExport should return true for finished reports', async () => {
@@ -414,7 +414,7 @@ describe('getReportPreviewAction', () => {
414414
} as unknown as Transaction;
415415

416416
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
417-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.EXPORT_TO_ACCOUNTING);
417+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.EXPORT_TO_ACCOUNTING);
418418
});
419419

420420
it('canReview should return true for reports where there are violations, user is submitter or approver and Workflows are enabled', async () => {
@@ -448,7 +448,7 @@ describe('getReportPreviewAction', () => {
448448
} as unknown as Transaction;
449449

450450
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
451-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.REVIEW);
451+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.REVIEW);
452452
});
453453

454454
it('canReview should return true for reports with RTER violations regardless of workspace workflow configuration', async () => {
@@ -485,7 +485,7 @@ describe('getReportPreviewAction', () => {
485485
reportID: `${REPORT_ID}`,
486486
} as unknown as Transaction;
487487

488-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.REVIEW);
488+
expect(getReportPreviewAction(VIOLATIONS, false, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.REVIEW);
489489
});
490490

491491
it('canView should return true for reports in which we are waiting for user to add a bank account', async () => {
@@ -507,6 +507,6 @@ describe('getReportPreviewAction', () => {
507507
} as unknown as Transaction;
508508

509509
const {result: isReportArchived} = renderHook(() => useReportIsArchived(report?.parentReportID));
510-
expect(getReportPreviewAction(VIOLATIONS, report, policy, [transaction], isReportArchived.current)).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
510+
expect(getReportPreviewAction(VIOLATIONS, isReportArchived.current, report, policy, [transaction])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
511511
});
512512
});

‎tests/unit/IOUUtilsTest.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ describe('canSubmitReport', () => {
309309

310310
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDWithViolation}`, transactionWithViolation);
311311
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDWithoutViolation}`, transactionWithoutViolation);
312-
expect(canSubmitReport(expenseReport, fakePolicy, [transactionWithViolation, transactionWithoutViolation], violations)).toBe(true);
312+
expect(canSubmitReport(expenseReport, fakePolicy, [transactionWithViolation, transactionWithoutViolation], violations, false)).toBe(true);
313313
});
314314

315315
test('Return false if report can not be submitted', async () => {
@@ -328,7 +328,7 @@ describe('canSubmitReport', () => {
328328
policyID: fakePolicy.id,
329329
};
330330

331-
expect(canSubmitReport(expenseReport, fakePolicy, [], undefined)).toBe(false);
331+
expect(canSubmitReport(expenseReport, fakePolicy, [], undefined, false)).toBe(false);
332332
});
333333

334334
it('returns false if the report is archived', async () => {

0 commit comments

Comments
 (0)