From 981b9b07c28533d80e0d3d7594413a69430141f5 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Mon, 10 Nov 2025 21:16:22 +0100 Subject: [PATCH 01/25] fix approve button not shown because of invisible brokenCardConnection violation --- src/libs/ReportPreviewActionUtils.ts | 37 +++++++++++++++------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/libs/ReportPreviewActionUtils.ts b/src/libs/ReportPreviewActionUtils.ts index 93052e34ad9a..5febc9597aad 100644 --- a/src/libs/ReportPreviewActionUtils.ts +++ b/src/libs/ReportPreviewActionUtils.ts @@ -29,7 +29,7 @@ import {getSession} from './SessionUtils'; import {allHavePendingRTERViolation, isPending, isScanning, shouldShowBrokenConnectionViolationForMultipleTransactions} from './TransactionUtils'; import ViolationsUtils from './Violations/ViolationsUtils'; -function canSubmit(report: Report, violations: OnyxCollection, isReportArchived: boolean, policy?: Policy, transactions?: Transaction[]) { +function canSubmit(report: Report, violations: OnyxCollection, isReportArchived: boolean, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { if (isReportArchived) { return false; } @@ -44,7 +44,8 @@ function canSubmit(report: Report, violations: OnyxCollection isScanning(transaction)); const submitToAccountID = getSubmitToAccountID(policy, report); @@ -53,17 +54,17 @@ function canSubmit(report: Report, violations: OnyxCollection 0; + return isExpense && (isSubmitter || isManager || isAdmin) && isOpen && !hasAnyVisibleViolations && !isAnyReceiptBeingScanned && !!transactions && transactions.length > 0; } -function canApprove(report: Report, violations: OnyxCollection, policy?: Policy, transactions?: Transaction[], shouldConsiderViolations = true) { +function canApprove(report: Report, violations: OnyxCollection, currentUserEmail: string, policy?: Policy, transactions?: Transaction[], shouldConsiderViolations = true) { const currentUserID = getCurrentUserAccountID(); const isExpense = isExpenseReport(report); const isProcessing = isProcessingReport(report); const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false; const managerID = report?.managerID ?? CONST.DEFAULT_NUMBER_ID; const isCurrentUserManager = managerID === currentUserID; - const hasAnyViolations = hasMissingSmartscanFields(report.reportID, transactions) || hasAnyViolationsUtil(report.reportID, violations); + const hasAnyVisibleViolations = hasMissingSmartscanFields(report.reportID, transactions) || (hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy)); const reportTransactions = transactions ?? getReportTransactions(report?.reportID); const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction)); @@ -82,13 +83,15 @@ function canApprove(report: Report, violations: OnyxCollection 0 && isCurrentUserManager; + return isExpense && isProcessing && !!isApprovalEnabled && (!hasAnyVisibleViolations || !shouldConsiderViolations) && reportTransactions.length > 0 && isCurrentUserManager; } function canPay( report: Report, violations: OnyxCollection, isReportArchived: boolean, + currentUserEmail: string, + transactions?: Transaction[], policy?: Policy, invoiceReceiverPolicy?: Policy, shouldConsiderViolations = true, @@ -109,9 +112,9 @@ function canPay( const {reimbursableSpend} = getMoneyRequestSpendBreakdown(report); const isReimbursed = isSettled(report); - const hasAnyViolations = hasAnyViolationsUtil(report.reportID, violations); + const hasAnyVisibleViolations = hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, transactions); - if (isExpense && isReportPayer && isPaymentsEnabled && isReportFinished && (!hasAnyViolations || !shouldConsiderViolations) && reimbursableSpend !== 0) { + if (isExpense && isReportPayer && isPaymentsEnabled && isReportFinished && (!hasAnyVisibleViolations || !shouldConsiderViolations) && reimbursableSpend !== 0) { return true; } @@ -139,7 +142,7 @@ function canPay( return invoiceReceiverPolicy?.role === CONST.POLICY.ROLE.ADMIN && reimbursableSpend > 0; } -function canExport(report: Report, violations: OnyxCollection, policy?: Policy) { +function canExport(report: Report, violations: OnyxCollection, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { const isExpense = isExpenseReport(report); const isExporter = policy ? isPreferredExporter(policy) : false; const isReimbursed = isSettled(report); @@ -147,7 +150,7 @@ function canExport(report: Report, violations: OnyxCollection, isReportArchived: boolean, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { @@ -181,8 +184,8 @@ function canReview(report: Report, violations: OnyxCollection Date: Mon, 10 Nov 2025 21:16:55 +0100 Subject: [PATCH 02/25] fix empty report next step shown because of invisible brokenCardConnection violation --- src/libs/TransactionUtils/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index da4424cb4280..757c313b463a 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1120,7 +1120,7 @@ function shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations: return false; } - if (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report)) { + if (isCurrentUserSubmitter(report)) { return true; } From 5bb1c925c7c4a39421b772aaca5e939e375a2b70 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 13:29:03 +0100 Subject: [PATCH 03/25] Just check if the violation has been dismissed by anyone If we don't provide an email to isViolationDismissed --- src/libs/TransactionUtils/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 757c313b463a..c485acccb348 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1334,12 +1334,18 @@ function isOnHold(transaction: OnyxEntry): boolean { } /** - * Checks if a violation is dismissed for the given transaction + * Checks if a violation is dismissed for the given transaction, optionally by a particular email address */ function isViolationDismissed(transaction: OnyxEntry, violation: TransactionViolation | undefined, currentUserEmail?: string): boolean { if (!transaction || !violation) { return false; } + + // If we don't provide an email, we just check if the violation has been dismissed by anyone + if (!currentUserEmail) { + return !!transaction?.comment?.dismissedViolations?.[violation.name]; + } + return !!transaction?.comment?.dismissedViolations?.[violation.name]?.[currentUserEmail ?? deprecatedCurrentUserEmail]; } From e3d73cb90e61efb46e079f23cc8b9e44597f668e Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 14:43:46 +0100 Subject: [PATCH 04/25] Fix empty report next step message when the broken connection next step is dismissed --- src/components/MoneyReportHeader.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index cfc61aef9196..ca68aa48bed9 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -543,7 +543,7 @@ function MoneyReportHeader({ return {icon: getStatusIcon(Expensicons.Flag), description: translate('iou.duplicateTransaction', {isSubmitted: isProcessingReport(moneyRequestReport)})}; } - if (!!transaction?.transactionID && shouldShowBrokenConnectionViolation) { + if (!!transaction?.transactionID && !!transactionViolations.length && shouldShowBrokenConnectionViolation) { return { icon: getStatusIcon(Expensicons.Hourglass), description: ( From b03bdb1c1f67172d37c4e46f9e6fdb53937f8f15 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 14:44:24 +0100 Subject: [PATCH 05/25] revert a change --- src/libs/TransactionUtils/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index c485acccb348..3fedfcd30d38 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1120,7 +1120,7 @@ function shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations: return false; } - if (isCurrentUserSubmitter(report)) { + if (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report)) { return true; } From b5bf0ca5639feaee3b193e22e900dfa8e2e4af33 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 14:55:15 +0100 Subject: [PATCH 06/25] fix empty report next step when the BrokenConnection violation is dismissed --- src/components/MoneyRequestHeader.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MoneyRequestHeader.tsx b/src/components/MoneyRequestHeader.tsx index 49dd550bd631..6c051f9f22d2 100644 --- a/src/components/MoneyRequestHeader.tsx +++ b/src/components/MoneyRequestHeader.tsx @@ -162,7 +162,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre if (isExpensifyCardTransaction(transaction) && isPending(transaction)) { return {icon: getStatusIcon(Expensicons.CreditCardHourglass), description: translate('iou.transactionPendingDescription')}; } - if (shouldShowBrokenConnectionViolation) { + if (!!transaction?.transactionID && !!transactionViolations.length && shouldShowBrokenConnectionViolation) { return { icon: getStatusIcon(Expensicons.Hourglass), description: ( From 2f7c119dc1f7d024eda710d76d6060fdab67ace5 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 17:14:05 +0100 Subject: [PATCH 07/25] run prettier --- src/libs/ReportPreviewActionUtils.ts | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libs/ReportPreviewActionUtils.ts b/src/libs/ReportPreviewActionUtils.ts index 5febc9597aad..f4fdd2039ac0 100644 --- a/src/libs/ReportPreviewActionUtils.ts +++ b/src/libs/ReportPreviewActionUtils.ts @@ -44,7 +44,9 @@ function canSubmit(report: Report, violations: OnyxCollection isScanning(transaction)); @@ -57,14 +59,23 @@ function canSubmit(report: Report, violations: OnyxCollection 0; } -function canApprove(report: Report, violations: OnyxCollection, currentUserEmail: string, policy?: Policy, transactions?: Transaction[], shouldConsiderViolations = true) { +function canApprove( + report: Report, + violations: OnyxCollection, + currentUserEmail: string, + policy?: Policy, + transactions?: Transaction[], + shouldConsiderViolations = true, +) { const currentUserID = getCurrentUserAccountID(); const isExpense = isExpenseReport(report); const isProcessing = isProcessingReport(report); const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false; const managerID = report?.managerID ?? CONST.DEFAULT_NUMBER_ID; const isCurrentUserManager = managerID === currentUserID; - const hasAnyVisibleViolations = hasMissingSmartscanFields(report.reportID, transactions) || (hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy)); + const hasAnyVisibleViolations = + hasMissingSmartscanFields(report.reportID, transactions) || + (hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy)); const reportTransactions = transactions ?? getReportTransactions(report?.reportID); const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction)); @@ -112,7 +123,8 @@ function canPay( const {reimbursableSpend} = getMoneyRequestSpendBreakdown(report); const isReimbursed = isSettled(report); - const hasAnyVisibleViolations = hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, transactions); + const hasAnyVisibleViolations = + hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, transactions); if (isExpense && isReportPayer && isPaymentsEnabled && isReportFinished && (!hasAnyVisibleViolations || !shouldConsiderViolations) && reimbursableSpend !== 0) { return true; @@ -150,7 +162,8 @@ function canExport(report: Report, violations: OnyxCollection Date: Tue, 11 Nov 2025 17:17:53 +0100 Subject: [PATCH 08/25] add automated tests for isViolationDismissed --- tests/unit/TransactionUtilsTest.ts | 55 ++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index 0eb538d2ea26..5713cf139b35 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -37,6 +37,7 @@ function generateTransaction(values: Partial = {}): Transaction { const CURRENT_USER_ID = 1; const CURRENT_USER_EMAIL = 'test@example.com'; +const OTHER_USER_EMAIL = 'other@example.com'; const SECOND_USER_ID = 2; const FAKE_OPEN_REPORT_ID = 'FAKE_OPEN_REPORT_ID'; const FAKE_OPEN_REPORT_SECOND_USER_ID = 'FAKE_OPEN_REPORT_SECOND_USER_ID'; @@ -727,6 +728,60 @@ describe('TransactionUtils', () => { const result = TransactionUtils.isViolationDismissed(transaction, violation); expect(result).toBe(true); }); + + it('should return true when violation is dismissed by another user and no email is provided', () => { + // Given a transaction with a violation dismissed by another user + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When checking if the violation is dismissed without providing a user email + const result = TransactionUtils.isViolationDismissed(transaction, violation, undefined); + + // Then it should return true since the violation has been dismissed by someone + expect(result).toBe(true); + }); + + it('should return false when violation is not dismissed and no email is provided', () => { + // Given a transaction with no dismissed violations + const transaction = generateTransaction({ + comment: {}, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When checking if the violation is dismissed without providing a user email + const result = TransactionUtils.isViolationDismissed(transaction, violation, undefined); + + // Then it should return false since the violation has not been dismissed + expect(result).toBe(false); + }); + + it('should return false when violation is dismissed by different user and email is provided', () => { + // Given a transaction with a violation dismissed by a different user + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When checking if the violation is dismissed for the current user + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL); + + // Then it should return false since the current user has not dismissed it + expect(result).toBe(false); + }); }); describe('shouldShowViolation', () => { From 4871b1f18172618c4f2dbc936163221ea2c7d5b6 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 21:46:38 +0100 Subject: [PATCH 09/25] pass current user email to isViolationDismissed in isDuplicate. because for duplicatedExpense violation, if the submitter dismisses it, the approver should still see it, and vice versa --- src/components/MoneyReportHeader.tsx | 4 ++-- src/libs/ReportPrimaryActionUtils.ts | 8 ++++---- src/libs/ReportSecondaryActionUtils.ts | 2 +- src/libs/TransactionUtils/index.ts | 4 ++-- src/libs/actions/IOU.ts | 2 +- tests/unit/ReportPrimaryActionUtilsTest.ts | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index ca68aa48bed9..dddd0fa7a187 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -567,7 +567,7 @@ function MoneyReportHeader({ }; const getFirstDuplicateThreadID = (transactionsList: OnyxTypes.Transaction[], allReportActions: OnyxTypes.ReportAction[]) => { - const duplicateTransaction = transactionsList.find((reportTransaction) => isDuplicate(reportTransaction)); + const duplicateTransaction = transactionsList.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '')); if (!duplicateTransaction) { return null; } @@ -850,7 +850,7 @@ function MoneyReportHeader({ onPress={() => { let threadID = transactionThreadReportID ?? getFirstDuplicateThreadID(transactions, reportActions); if (!threadID) { - const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction)); + const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '')); const transactionID = duplicateTransaction?.transactionID; const iouAction = getIOUActionForReportID(moneyRequestReport?.reportID, transactionID); const createdTransactionThreadReport = createTransactionThreadReport(moneyRequestReport, iouAction); diff --git a/src/libs/ReportPrimaryActionUtils.ts b/src/libs/ReportPrimaryActionUtils.ts index 88119fe66bb8..ef33fc0821e3 100644 --- a/src/libs/ReportPrimaryActionUtils.ts +++ b/src/libs/ReportPrimaryActionUtils.ts @@ -245,8 +245,8 @@ function isRemoveHoldAction(report: Report, chatReport: OnyxEntry, repor return isHolder; } -function isReviewDuplicatesAction(report: Report, reportTransactions: Transaction[]) { - const hasDuplicates = reportTransactions.some((transaction) => isDuplicate(transaction)); +function isReviewDuplicatesAction(report: Report, reportTransactions: Transaction[], currentUserEmail: string) { + const hasDuplicates = reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail)); if (!hasDuplicates) { return false; @@ -347,7 +347,7 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf isDuplicate(transaction)); + const reportHasDuplicatedTransactions = reportTransactions.some((transaction) => isDuplicate(transaction, currentUserLogin)); if (isExpenseReport && isProcessingReport && reportHasDuplicatedTransactions) { return true; diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 3fedfcd30d38..29228064ba8b 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1309,7 +1309,7 @@ function getRecentTransactions(transactions: Record, size = 2): * Check if transaction has duplicatedTransaction violation. * @param transactionID - the transaction to check */ -function isDuplicate(transaction: OnyxEntry): boolean { +function isDuplicate(transaction: OnyxEntry, currentUserEmail: string): boolean { if (!transaction) { return false; } @@ -1317,7 +1317,7 @@ function isDuplicate(transaction: OnyxEntry): boolean { (violation: TransactionViolation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION, ); const hasDuplicatedTransactionViolation = !!duplicatedTransactionViolation; - const isDuplicatedTransactionViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation); + const isDuplicatedTransactionViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation, currentUserEmail); return hasDuplicatedTransactionViolation && !isDuplicatedTransactionViolationDismissed; } diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index b1d6c6d53b6d..623dd6b01bcb 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10364,7 +10364,7 @@ function approveMoneyRequest( // Remove duplicates violations if we approve the report if (hasDuplicates) { - const transactions = getReportTransactions(expenseReport.reportID).filter((transaction) => isDuplicate(transaction)); + const transactions = getReportTransactions(expenseReport.reportID).filter((transaction) => isDuplicate(transaction, currentUserEmailParam)); if (!full) { transactions.filter((transaction) => !isOnHold(transaction)); } diff --git a/tests/unit/ReportPrimaryActionUtilsTest.ts b/tests/unit/ReportPrimaryActionUtilsTest.ts index 435efee1c68a..c389f1b1b0fc 100644 --- a/tests/unit/ReportPrimaryActionUtilsTest.ts +++ b/tests/unit/ReportPrimaryActionUtilsTest.ts @@ -752,7 +752,7 @@ describe('isReviewDuplicatesAction', () => { } as TransactionViolation, ]); - expect(isReviewDuplicatesAction(report, [transaction])).toBe(true); + expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL)).toBe(true); }); it('should return false when report approver has no duplicated transactions', async () => { @@ -772,7 +772,7 @@ describe('isReviewDuplicatesAction', () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction); - expect(isReviewDuplicatesAction(report, [transaction])).toBe(false); + expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL)).toBe(false); }); it('should return false when current user is neither the report submitter nor approver', async () => { @@ -797,7 +797,7 @@ describe('isReviewDuplicatesAction', () => { } as TransactionViolation, ]); - expect(isReviewDuplicatesAction(report, [transaction])).toBe(false); + expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL)).toBe(false); }); }); From 6924d6b46e5ca0f0ed503f81932b215b3c118c13 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 22:03:23 +0100 Subject: [PATCH 10/25] fix failing test --- src/libs/ReportPreviewActionUtils.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportPreviewActionUtils.ts b/src/libs/ReportPreviewActionUtils.ts index f4fdd2039ac0..47bb2c55c2fe 100644 --- a/src/libs/ReportPreviewActionUtils.ts +++ b/src/libs/ReportPreviewActionUtils.ts @@ -29,7 +29,7 @@ import {getSession} from './SessionUtils'; import {allHavePendingRTERViolation, isPending, isScanning, shouldShowBrokenConnectionViolationForMultipleTransactions} from './TransactionUtils'; import ViolationsUtils from './Violations/ViolationsUtils'; -function canSubmit(report: Report, violations: OnyxCollection, isReportArchived: boolean, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { +function canSubmit(report: Report, violations: OnyxCollection, isReportArchived: boolean, policy?: Policy, transactions?: Transaction[]) { if (isReportArchived) { return false; } @@ -44,9 +44,7 @@ function canSubmit(report: Report, violations: OnyxCollection isScanning(transaction)); @@ -56,7 +54,7 @@ function canSubmit(report: Report, violations: OnyxCollection 0; + return isExpense && (isSubmitter || isManager || isAdmin) && isOpen && !hasAnyViolations && !isAnyReceiptBeingScanned && !!transactions && transactions.length > 0; } function canApprove( @@ -257,7 +255,7 @@ function getReportPreviewAction( return CONST.REPORT.REPORT_PREVIEW_ACTIONS.REVIEW; } - if (canSubmit(report, violations, isReportArchived, currentUserEmail, policy, transactions)) { + if (canSubmit(report, violations, isReportArchived, policy, transactions)) { return CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT; } if (canApprove(report, violations, currentUserEmail, policy, transactions)) { From 9cc99ea52a960f364fb23b8f932c23ac69319e12 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 22:13:21 +0100 Subject: [PATCH 11/25] fix type errors --- src/components/MoneyReportHeader.tsx | 2 +- src/components/MoneyRequestHeader.tsx | 4 ++-- src/libs/TransactionUtils/index.ts | 4 ++-- src/libs/actions/IOU.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index dddd0fa7a187..69a013be8145 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -385,7 +385,7 @@ function MoneyReportHeader({ const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE; - const hasDuplicates = hasDuplicateTransactions(moneyRequestReport?.reportID); + const hasDuplicates = hasDuplicateTransactions(email ?? '', moneyRequestReport?.reportID); const shouldShowMarkAsResolved = isMarkAsResolvedAction(moneyRequestReport, transactionViolations); const shouldShowStatusBar = hasAllPendingRTERViolations || diff --git a/src/components/MoneyRequestHeader.tsx b/src/components/MoneyRequestHeader.tsx index 6c051f9f22d2..056009c16047 100644 --- a/src/components/MoneyRequestHeader.tsx +++ b/src/components/MoneyRequestHeader.tsx @@ -108,9 +108,9 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre const styles = useThemeStyles(); const theme = useTheme(); const {translate} = useLocalize(); - const {login: currentUserLogin} = useCurrentUserPersonalDetails(); + const {login: currentUserLogin, email} = useCurrentUserPersonalDetails(); const isOnHold = isOnHoldTransactionUtils(transaction); - const isDuplicate = isDuplicateTransactionUtils(transaction); + const isDuplicate = isDuplicateTransactionUtils(transaction, email ?? ''); const reportID = report?.reportID; const {removeTransaction, currentSearchHash} = useSearchContext(); const {isExpenseSplit} = getOriginalTransactionWithSplitInfo(transaction); diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 29228064ba8b..d5025213fdcb 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1376,11 +1376,11 @@ function hasViolation(transaction: Transaction | undefined, transactionViolation ); } -function hasDuplicateTransactions(iouReportID?: string, allReportTransactions?: SearchTransaction[]): boolean { +function hasDuplicateTransactions(currentUserEmail: string, iouReportID?: string, allReportTransactions?: SearchTransaction[]): boolean { const transactionsByIouReportID = getReportTransactions(iouReportID); const reportTransactions = allReportTransactions ?? transactionsByIouReportID; - return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction)); + return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail)); } /** diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 623dd6b01bcb..8b42d61a0df3 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10206,7 +10206,7 @@ function approveMoneyRequest( const currentNextStep = allNextSteps[`${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`] ?? null; let total = expenseReport.total ?? 0; const hasHeldExpenses = hasHeldExpensesReportUtils(expenseReport.reportID); - const hasDuplicates = hasDuplicateTransactions(expenseReport.reportID); + const hasDuplicates = hasDuplicateTransactions(currentUserEmailParam, expenseReport.reportID); if (hasHeldExpenses && !full && !!expenseReport.unheldTotal) { total = expenseReport.unheldTotal; } From 3da5117db6995631b72ce9fed9e46e0e75c6eff6 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 11 Nov 2025 22:19:08 +0100 Subject: [PATCH 12/25] remove deprecatedCurrentUserEmail --- src/libs/TransactionUtils/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index d5025213fdcb..e313293c766a 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -145,12 +145,10 @@ Onyx.connect({ callback: (value) => (allTransactionViolations = value), }); -let deprecatedCurrentUserEmail = ''; let deprecatedCurrentUserAccountID = -1; Onyx.connect({ key: ONYXKEYS.SESSION, callback: (val) => { - deprecatedCurrentUserEmail = val?.email ?? ''; deprecatedCurrentUserAccountID = val?.accountID ?? CONST.DEFAULT_NUMBER_ID; }, }); @@ -1346,7 +1344,7 @@ function isViolationDismissed(transaction: OnyxEntry, violation: Tr return !!transaction?.comment?.dismissedViolations?.[violation.name]; } - return !!transaction?.comment?.dismissedViolations?.[violation.name]?.[currentUserEmail ?? deprecatedCurrentUserEmail]; + return !!transaction?.comment?.dismissedViolations?.[violation.name]?.[currentUserEmail]; } /** From de6486da581ed1504e2d48044c005c35037a0f1f Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Mon, 17 Nov 2025 20:58:24 +0100 Subject: [PATCH 13/25] Address review comments --- src/components/MoneyReportHeader.tsx | 9 +- src/components/MoneyRequestHeader.tsx | 2 +- .../Search/TransactionGroupListExpanded.tsx | 2 +- .../Search/TransactionListItem.tsx | 2 +- src/hooks/useTransactionViolations.ts | 2 +- .../useTransactionsAndViolationsForReport.ts | 7 +- src/libs/ReportPreviewActionUtils.ts | 28 +- src/libs/ReportPrimaryActionUtils.ts | 16 +- src/libs/ReportSecondaryActionUtils.ts | 4 +- src/libs/ReportUtils.ts | 42 ++- src/libs/SearchUIUtils.ts | 10 +- src/libs/TransactionPreviewUtils.ts | 19 +- src/libs/TransactionUtils/index.ts | 121 +++++-- src/libs/actions/IOU.ts | 9 +- tests/unit/IOUUtilsTest.ts | 4 +- tests/unit/ReportPrimaryActionUtilsTest.ts | 14 +- tests/unit/TransactionUtilsTest.ts | 326 +++++++++++++++--- tests/unit/ViolationUtilsTest.ts | 8 +- 18 files changed, 483 insertions(+), 142 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index b1ec2d546e39..e8ac972b59f4 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -298,7 +298,10 @@ function MoneyReportHeader({ // Check if there is pending rter violation in all transactionViolations with given transactionIDs. // wrapped in useMemo to avoid unnecessary re-renders and for better performance (array operation inside of function) - const hasAllPendingRTERViolations = useMemo(() => allHavePendingRTERViolation(transactions, violations), [transactions, violations]); + const hasAllPendingRTERViolations = useMemo( + () => allHavePendingRTERViolation(transactions, violations, email ?? '', moneyRequestReport, policy), + [transactions, violations, email, moneyRequestReport, policy], + ); // Check if user should see broken connection violation warning. const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, moneyRequestReport, policy, violations); const hasOnlyHeldExpenses = hasOnlyHeldExpensesReportUtils(moneyRequestReport?.reportID); @@ -573,7 +576,7 @@ function MoneyReportHeader({ }; const getFirstDuplicateThreadID = (transactionsList: OnyxTypes.Transaction[], allReportActions: OnyxTypes.ReportAction[]) => { - const duplicateTransaction = transactionsList.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '')); + const duplicateTransaction = transactionsList.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '', moneyRequestReport, policy)); if (!duplicateTransaction) { return null; } @@ -864,7 +867,7 @@ function MoneyReportHeader({ onPress={() => { let threadID = transactionThreadReportID ?? getFirstDuplicateThreadID(transactions, reportActions); if (!threadID) { - const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '')); + const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction, email ?? '', moneyRequestReport, policy)); const transactionID = duplicateTransaction?.transactionID; const iouAction = getIOUActionForReportID(moneyRequestReport?.reportID, transactionID); const createdTransactionThreadReport = createTransactionThreadReport(moneyRequestReport, iouAction); diff --git a/src/components/MoneyRequestHeader.tsx b/src/components/MoneyRequestHeader.tsx index ac361a2fce89..3dfa712473ea 100644 --- a/src/components/MoneyRequestHeader.tsx +++ b/src/components/MoneyRequestHeader.tsx @@ -113,7 +113,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre const {translate} = useLocalize(); const {login: currentUserLogin, email} = useCurrentUserPersonalDetails(); const isOnHold = isOnHoldTransactionUtils(transaction); - const isDuplicate = isDuplicateTransactionUtils(transaction, email ?? ''); + const isDuplicate = isDuplicateTransactionUtils(transaction, email ?? '', report, policy); const reportID = report?.reportID; const {removeTransaction, currentSearchHash} = useSearchContext(); const {isExpenseSplit} = getOriginalTransactionWithSplitInfo(transaction); diff --git a/src/components/SelectionListWithSections/Search/TransactionGroupListExpanded.tsx b/src/components/SelectionListWithSections/Search/TransactionGroupListExpanded.tsx index d78ab2c80762..348c9675d469 100644 --- a/src/components/SelectionListWithSections/Search/TransactionGroupListExpanded.tsx +++ b/src/components/SelectionListWithSections/Search/TransactionGroupListExpanded.tsx @@ -190,7 +190,7 @@ function TransactionGroupListExpanded({ ({ const transactionViolations = useMemo(() => { return (violations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionItem.transactionID}`] ?? []).filter( (violation: TransactionViolation) => - !isViolationDismissed(transactionItem, violation, currentUserDetails.email ?? '') && + !isViolationDismissed(transactionItem, violation, currentUserDetails.email ?? '', snapshotReport, snapshotPolicy) && shouldShowViolation(snapshotReport, snapshotPolicy, violation.name, currentUserDetails.email ?? '', false), ); }, [snapshotPolicy, snapshotReport, transactionItem, violations, currentUserDetails.email]); diff --git a/src/hooks/useTransactionViolations.ts b/src/hooks/useTransactionViolations.ts index 724ca3945dcc..f5854f182ec1 100644 --- a/src/hooks/useTransactionViolations.ts +++ b/src/hooks/useTransactionViolations.ts @@ -27,7 +27,7 @@ function useTransactionViolations(transactionID?: string, shouldShowRterForSettl mergeProhibitedViolations( transactionViolations.filter( (violation: TransactionViolation) => - !isViolationDismissed(transaction, violation, currentUserDetails.email ?? '') && + !isViolationDismissed(transaction, violation, currentUserDetails.email ?? '', iouReport, policy) && shouldShowViolation(iouReport, policy, violation.name, currentUserDetails.email ?? '', shouldShowRterForSettledReport), ), ), diff --git a/src/hooks/useTransactionsAndViolationsForReport.ts b/src/hooks/useTransactionsAndViolationsForReport.ts index a2e8ff0f91d9..c25ca002d4c3 100644 --- a/src/hooks/useTransactionsAndViolationsForReport.ts +++ b/src/hooks/useTransactionsAndViolationsForReport.ts @@ -5,12 +5,15 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {TransactionViolations} from '@src/types/onyx'; import type {ReportTransactionsAndViolations} from '@src/types/onyx/DerivedValues'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; +import useOnyx from './useOnyx'; const DEFAULT_RETURN_VALUE: ReportTransactionsAndViolations = {transactions: {}, violations: {}}; function useTransactionsAndViolationsForReport(reportID?: string) { const allReportsTransactionsAndViolations = useAllReportsTransactionsAndViolations(); const currentUserDetails = useCurrentUserPersonalDetails(); + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {canBeMissing: true}); + const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`, {canBeMissing: true}); const {transactions, violations} = reportID ? (allReportsTransactionsAndViolations?.[reportID] ?? DEFAULT_RETURN_VALUE) : DEFAULT_RETURN_VALUE; @@ -22,14 +25,14 @@ function useTransactionsAndViolationsForReport(reportID?: string) { // This is our accumulator, it's okay to reassign // eslint-disable-next-line no-param-reassign - filteredTransactionViolations[transactionViolationKey] = getTransactionViolations(transaction, violations, currentUserDetails.email ?? '') ?? []; + filteredTransactionViolations[transactionViolationKey] = getTransactionViolations(transaction, violations, currentUserDetails.email ?? '', report, policy) ?? []; return filteredTransactionViolations; }, {} as Record, ); return {transactions, violations: filteredViolations}; - }, [transactions, violations, currentUserDetails?.email]); + }, [transactions, violations, currentUserDetails?.email, report, policy]); return transactionsAndViolations; } diff --git a/src/libs/ReportPreviewActionUtils.ts b/src/libs/ReportPreviewActionUtils.ts index 47bb2c55c2fe..4addb80fb0e4 100644 --- a/src/libs/ReportPreviewActionUtils.ts +++ b/src/libs/ReportPreviewActionUtils.ts @@ -71,9 +71,9 @@ function canApprove( const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false; const managerID = report?.managerID ?? CONST.DEFAULT_NUMBER_ID; const isCurrentUserManager = managerID === currentUserID; - const hasAnyVisibleViolations = - hasMissingSmartscanFields(report.reportID, transactions) || - (hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy)); + + // We should consider only visible violations for the approver, invisible violations should not block approval + const hasAnyVisibleViolations = hasMissingSmartscanFields(report.reportID, transactions) || ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy); const reportTransactions = transactions ?? getReportTransactions(report?.reportID); const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction)); @@ -99,8 +99,6 @@ function canPay( report: Report, violations: OnyxCollection, isReportArchived: boolean, - currentUserEmail: string, - transactions?: Transaction[], policy?: Policy, invoiceReceiverPolicy?: Policy, shouldConsiderViolations = true, @@ -121,10 +119,9 @@ function canPay( const {reimbursableSpend} = getMoneyRequestSpendBreakdown(report); const isReimbursed = isSettled(report); - const hasAnyVisibleViolations = - hasAnyViolationsUtil(report.reportID, violations) && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, transactions); + const hasAnyViolations = hasAnyViolationsUtil(report.reportID, violations); - if (isExpense && isReportPayer && isPaymentsEnabled && isReportFinished && (!hasAnyVisibleViolations || !shouldConsiderViolations) && reimbursableSpend !== 0) { + if (isExpense && isReportPayer && isPaymentsEnabled && isReportFinished && (!hasAnyViolations || !shouldConsiderViolations) && reimbursableSpend !== 0) { return true; } @@ -152,7 +149,7 @@ function canPay( return invoiceReceiverPolicy?.role === CONST.POLICY.ROLE.ADMIN && reimbursableSpend > 0; } -function canExport(report: Report, violations: OnyxCollection, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { +function canExport(report: Report, violations: OnyxCollection, policy?: Policy) { const isExpense = isExpenseReport(report); const isExporter = policy ? isPreferredExporter(policy) : false; const isReimbursed = isSettled(report); @@ -160,8 +157,7 @@ function canExport(report: Report, violations: OnyxCollection, isReportArchived: boolean, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { @@ -196,7 +192,7 @@ function canReview(report: Report, violations: OnyxCollection transaction.transactionID) ?? []; - const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, violations); + const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, violations, currentUserEmail, report, policy); const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, report, policy, violations); if (hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!isAdmin || isSubmitter) && !isReportApproved({report}) && !isReportManuallyReimbursed(report))) { @@ -261,10 +257,10 @@ function getReportPreviewAction( if (canApprove(report, violations, currentUserEmail, policy, transactions)) { return CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE; } - if (canPay(report, violations, isReportArchived, currentUserEmail, transactions, policy, invoiceReceiverPolicy)) { + if (canPay(report, violations, isReportArchived, policy, invoiceReceiverPolicy)) { return CONST.REPORT.REPORT_PREVIEW_ACTIONS.PAY; } - if (canExport(report, violations, currentUserEmail, policy, transactions)) { + if (canExport(report, violations, policy)) { return CONST.REPORT.REPORT_PREVIEW_ACTIONS.EXPORT_TO_ACCOUNTING; } if (canReview(report, violations, isReportArchived, currentUserEmail, policy, transactions)) { diff --git a/src/libs/ReportPrimaryActionUtils.ts b/src/libs/ReportPrimaryActionUtils.ts index 68fe97047218..69dd230d109b 100644 --- a/src/libs/ReportPrimaryActionUtils.ts +++ b/src/libs/ReportPrimaryActionUtils.ts @@ -246,8 +246,8 @@ function isRemoveHoldAction(report: Report, chatReport: OnyxEntry, repor return isHolder; } -function isReviewDuplicatesAction(report: Report, reportTransactions: Transaction[], currentUserEmail: string) { - const hasDuplicates = reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail)); +function isReviewDuplicatesAction(report: Report, reportTransactions: Transaction[], currentUserEmail: string, policy: Policy | undefined) { + const hasDuplicates = reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail, report, policy)); if (!hasDuplicates) { return false; @@ -276,7 +276,7 @@ function isMarkAsCashAction(currentUserEmail: string, report: Report, reportTran } const transactionIDs = reportTransactions.map((t) => t.transactionID); - const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations); + const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations, currentUserEmail, report, policy); if (hasAllPendingRTERViolations) { return true; @@ -305,12 +305,12 @@ function isMarkAsResolvedAction(report?: Report, violations?: TransactionViolati return violations?.some((violation) => violation.name === CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE); } -function isPrimaryMarkAsResolvedAction(report?: Report, reportTransactions?: Transaction[], violations?: OnyxCollection, policy?: Policy) { +function isPrimaryMarkAsResolvedAction(currentUserEmail: string, report?: Report, reportTransactions?: Transaction[], violations?: OnyxCollection, policy?: Policy) { if (!reportTransactions || reportTransactions.length !== 1) { return false; } - const transactionViolations = getTransactionViolations(reportTransactions.at(0), violations); + const transactionViolations = getTransactionViolations(reportTransactions.at(0), violations, currentUserEmail, report, policy); return isExpenseReportUtils(report) && isMarkAsResolvedAction(report, transactionViolations, policy); } @@ -357,7 +357,7 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf isDuplicate(transaction, currentUserLogin)); + const reportHasDuplicatedTransactions = reportTransactions.some((transaction) => isDuplicate(transaction, currentUserLogin, report, policy)); if (isExpenseReport && isProcessingReport && reportHasDuplicatedTransactions) { return true; @@ -237,7 +237,7 @@ function isApproveAction(currentUserLogin: string, report: Report, reportTransac const transactionIDs = reportTransactions.map((t) => t.transactionID); - const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations); + const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations, currentUserLogin, report, policy); if (hasAllPendingRTERViolations) { return true; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 105d3c5a628c..4f5aa823e1fa 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -8921,9 +8921,9 @@ function shouldDisplayViolationsRBRInLHN(report: OnyxEntry, transactionV return ( !isInvoiceReport(potentialReport) && ViolationsUtils.hasVisibleViolationsForUser(potentialReport, transactionViolations, currentUserEmail ?? '', policy, transactions) && - (hasViolations(potentialReport.reportID, transactionViolations, true) || - hasWarningTypeViolations(potentialReport.reportID, transactionViolations, true) || - hasNoticeTypeViolations(potentialReport.reportID, transactionViolations, true)) + (hasViolations(potentialReport.reportID, transactionViolations, true, transactions, currentUserEmail, potentialReport, policy) || + hasWarningTypeViolations(potentialReport.reportID, transactionViolations, true, transactions, currentUserEmail, potentialReport, policy) || + hasNoticeTypeViolations(potentialReport.reportID, transactionViolations, true, transactions, currentUserEmail, potentialReport, policy)) ); }); } @@ -8935,10 +8935,13 @@ function hasViolations( reportID: string | undefined, transactionViolations: OnyxCollection, shouldShowInReview?: boolean, - reportTransactions?: SearchTransaction[], + reportTransactions?: Transaction[] | SearchTransaction[], + currentUserEmailParam?: string, + report?: OnyxEntry, + policy?: OnyxEntry, ): boolean { const transactions = reportTransactions ?? getReportTransactions(reportID); - return transactions.some((transaction) => hasViolation(transaction, transactionViolations, shouldShowInReview)); + return transactions.some((transaction) => hasViolation(transaction, transactionViolations, currentUserEmailParam ?? '', report, policy, shouldShowInReview)); } /** @@ -8948,10 +8951,13 @@ function hasWarningTypeViolations( reportID: string | undefined, transactionViolations: OnyxCollection, shouldShowInReview?: boolean, - reportTransactions?: SearchTransaction[], + reportTransactions?: Transaction[] | SearchTransaction[], + currentUserEmailParam?: string, + report?: OnyxEntry, + policy?: OnyxEntry, ): boolean { const transactions = reportTransactions ?? getReportTransactions(reportID); - return transactions.some((transaction) => hasWarningTypeViolation(transaction, transactionViolations, shouldShowInReview)); + return transactions.some((transaction) => hasWarningTypeViolation(transaction, transactionViolations, currentUserEmailParam ?? '', report, policy, shouldShowInReview)); } /** @@ -8981,20 +8987,30 @@ function hasNoticeTypeViolations( reportID: string | undefined, transactionViolations: OnyxCollection, shouldShowInReview?: boolean, - reportTransactions?: SearchTransaction[], + reportTransactions?: Transaction[] | SearchTransaction[], + currentUserEmailParam?: string, + report?: OnyxEntry, + policy?: OnyxEntry, ): boolean { const transactions = reportTransactions ?? getReportTransactions(reportID); - return transactions.some((transaction) => hasNoticeTypeViolation(transaction, transactionViolations, shouldShowInReview)); + return transactions.some((transaction) => hasNoticeTypeViolation(transaction, transactionViolations, currentUserEmailParam ?? '', report, policy, shouldShowInReview)); } /** * Checks to see if a report contains any type of violation */ -function hasAnyViolations(reportID: string | undefined, transactionViolations: OnyxCollection, reportTransactions?: SearchTransaction[]) { +function hasAnyViolations( + reportID: string | undefined, + transactionViolations: OnyxCollection, + reportTransactions?: Transaction[] | SearchTransaction[], + currentUserEmailParam?: string, + report?: OnyxEntry, + policy?: OnyxEntry, +) { return ( - hasViolations(reportID, transactionViolations, undefined, reportTransactions) || - hasNoticeTypeViolations(reportID, transactionViolations, true, reportTransactions) || - hasWarningTypeViolations(reportID, transactionViolations, true, reportTransactions) + hasViolations(reportID, transactionViolations, undefined, reportTransactions, currentUserEmailParam, report, policy) || + hasNoticeTypeViolations(reportID, transactionViolations, true, reportTransactions, currentUserEmailParam, report, policy) || + hasWarningTypeViolations(reportID, transactionViolations, true, reportTransactions, currentUserEmailParam, report, policy) ); } diff --git a/src/libs/SearchUIUtils.ts b/src/libs/SearchUIUtils.ts index 7d5a10d55bfc..f5a17542fc34 100644 --- a/src/libs/SearchUIUtils.ts +++ b/src/libs/SearchUIUtils.ts @@ -909,12 +909,14 @@ function getTransactionViolations( allViolations: OnyxCollection, transaction: SearchTransaction, currentUserEmail: string, + report: OnyxEntry, + policy: OnyxEntry, ): OnyxTypes.TransactionViolation[] { const transactionViolations = allViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`]; if (!transactionViolations) { return []; } - return transactionViolations.filter((violation) => !isViolationDismissed(transaction, violation, currentUserEmail)); + return transactionViolations.filter((violation) => !isViolationDismissed(transaction, violation, currentUserEmail, report, policy)); } /** @@ -1033,7 +1035,7 @@ function getTransactionsSections( const reportAction = reportActionsByTransactionIDMap.get(transactionItem.transactionID); const policy = data[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]; const shouldShowBlankTo = !report || isOpenExpenseReport(report); - const transactionViolations = getTransactionViolations(allViolations, transactionItem, currentUserEmail); + const transactionViolations = getTransactionViolations(allViolations, transactionItem, currentUserEmail, report, policy); // Use Map.get() for faster lookups with default values const from = reportAction?.actorAccountID ? (personalDetailsMap.get(reportAction.actorAccountID.toString()) ?? emptyPersonalDetails) : emptyPersonalDetails; const to = getToFieldValueForTransaction(transactionItem, report, data.personalDetailsList, reportAction); @@ -1213,7 +1215,7 @@ function getActions( const chatReportRNVP = data[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.chatReportID}`] ?? undefined; const isChatReportArchived = isArchivedReport(chatReportRNVP); - const hasAnyViolationsForReport = hasAnyViolations(report.reportID, allViolations, allReportTransactions); + const hasAnyViolationsForReport = hasAnyViolations(report.reportID, allViolations, allReportTransactions, currentUserEmail, report, policy); const hasVisibleViolationsForReport = hasAnyViolationsForReport && ViolationsUtils.hasVisibleViolationsForUser(report, allViolations, currentUserEmail, policy, allReportTransactions); // Only check for violations if we need to (when user has permission to review) @@ -1510,7 +1512,7 @@ function getReportSections( const report = data[`${ONYXKEYS.COLLECTION.REPORT}${transactionItem.reportID}`] as SearchReport | undefined; const policy = data[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]; const shouldShowBlankTo = !report || isOpenExpenseReport(report); - const transactionViolations = getTransactionViolations(allViolations, transactionItem, currentUserEmail); + const transactionViolations = getTransactionViolations(allViolations, transactionItem, currentUserEmail, report, policy); const actions = Object.values(reportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionItem.reportID}`] ?? {}); const from = reportAction?.actorAccountID ? (data.personalDetailsList?.[reportAction.actorAccountID] ?? emptyPersonalDetails) : emptyPersonalDetails; const to = getToFieldValueForTransaction(transactionItem, report, data.personalDetailsList, reportAction); diff --git a/src/libs/TransactionPreviewUtils.ts b/src/libs/TransactionPreviewUtils.ts index cdd98a05ab43..087f617216b7 100644 --- a/src/libs/TransactionPreviewUtils.ts +++ b/src/libs/TransactionPreviewUtils.ts @@ -10,6 +10,8 @@ import {abandonReviewDuplicateTransactions, setReviewDuplicatesKey} from './acti import {isCategoryMissing} from './CategoryUtils'; import {convertToDisplayString} from './CurrencyUtils'; import DateUtils from './DateUtils'; +import {getCurrentUserEmail} from './Network/NetworkStore'; +import {getPolicy} from './PolicyUtils'; import {getOriginalMessage, isMessageDeleted, isMoneyRequestAction} from './ReportActionsUtils'; import { hasActionWithErrorsForTransaction, @@ -211,7 +213,11 @@ function getTransactionPreviewTextAndTranslationPaths({ const isTransactionScanning = isScanning(transaction); const hasFieldErrors = hasMissingSmartscanFields(transaction); const isPaidGroupPolicy = isPaidGroupPolicyUtil(iouReport); - const hasViolationsOfTypeNotice = hasNoticeTypeViolation(transaction, violations, true) && isPaidGroupPolicy; + const currentUserEmail = getCurrentUserEmail(); + // 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(iouReport?.policyID); + const hasViolationsOfTypeNotice = hasNoticeTypeViolation(transaction, violations, currentUserEmail ?? '', iouReport, policy, true) && isPaidGroupPolicy; const hasActionWithErrors = hasActionWithErrorsForTransaction(iouReport?.reportID, transaction); const {amount: requestAmount, currency: requestCurrency} = transactionDetails; @@ -358,7 +364,12 @@ function createTransactionPreviewConditionals({ const isApproved = isReportApproved({report: iouReport}); const isSettlementOrApprovalPartial = !!iouReport?.pendingFields?.partial; - const hasViolationsOfTypeNotice = hasNoticeTypeViolation(transaction, violations) && iouReport && isPaidGroupPolicyUtil(iouReport); + const currentUserEmail = getCurrentUserEmail(); + // 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(iouReport?.policyID); + const hasViolationsOfTypeNotice = + hasNoticeTypeViolation(transaction, violations, currentUserEmail ?? '', iouReport ?? undefined, policy, true) && iouReport && isPaidGroupPolicyUtil(iouReport); const hasFieldErrors = hasMissingSmartscanFields(transaction); const isFetchingWaypoints = isFetchingWaypointsFromServer(transaction); @@ -378,8 +389,8 @@ function createTransactionPreviewConditionals({ isUnreportedAndHasInvalidDistanceRateTransaction(transaction) || // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing hasViolationsOfTypeNotice || - hasWarningTypeViolation(transaction, violations) || - hasViolation(transaction, violations, true) || + hasWarningTypeViolation(transaction, violations, currentUserEmail ?? '', iouReport ?? undefined, policy) || + hasViolation(transaction, violations, currentUserEmail ?? '', iouReport ?? undefined, policy, true) || (isDistanceRequest(transaction) && violations?.some( (violation) => violation.name === CONST.VIOLATIONS.MODIFIED_AMOUNT && (violation.type === CONST.VIOLATION_TYPES.VIOLATION || violation.type === CONST.VIOLATION_TYPES.NOTICE), diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index fdea8dbfd485..2b7d527af738 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -18,7 +18,7 @@ import {toLocaleDigit} from '@libs/LocaleDigitUtils'; import {translateLocal} from '@libs/Localize'; import Log from '@libs/Log'; import {rand64, roundToTwoDecimalPlaces} from '@libs/NumberUtils'; -import {getPersonalDetailsByIDs} from '@libs/PersonalDetailsUtils'; +import {getLoginsByAccountIDs, getPersonalDetailsByIDs} from '@libs/PersonalDetailsUtils'; import { getCommaSeparatedTagNameWithSanitizedColons, getDistanceRateCustomUnit, @@ -145,10 +145,12 @@ Onyx.connect({ callback: (value) => (allTransactionViolations = value), }); +let deprecatedCurrentUserEmail = ''; let deprecatedCurrentUserAccountID = -1; Onyx.connect({ key: ONYXKEYS.SESSION, callback: (val) => { + deprecatedCurrentUserEmail = val?.email ?? ''; deprecatedCurrentUserAccountID = val?.accountID ?? CONST.DEFAULT_NUMBER_ID; }, }); @@ -1073,14 +1075,16 @@ function hasMissingSmartscanFields(transaction: OnyxInputOrEntry, r function getTransactionViolations( transaction: OnyxEntry, transactionViolations: OnyxCollection, - currentUserEmail?: string, + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, ): TransactionViolations | undefined { if (!transaction || !transactionViolations) { return undefined; } return transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transaction.transactionID]?.filter( - (violation) => !isViolationDismissed(transaction, violation, currentUserEmail), + (violation) => !isViolationDismissed(transaction, violation, currentUserEmail, iouReport, policy), ); } @@ -1104,8 +1108,14 @@ function hasPendingRTERViolation(transactionViolations?: TransactionViolations | /** * Check if there is broken connection violation. */ -function hasBrokenConnectionViolation(transaction: Transaction | SearchTransaction, transactionViolations: OnyxCollection | undefined): boolean { - const violations = getTransactionViolations(transaction, transactionViolations); +function hasBrokenConnectionViolation( + transaction: Transaction | SearchTransaction, + transactionViolations: OnyxCollection | undefined, + currentUserEmail: string, + report: OnyxEntry, + policy: OnyxEntry, +): boolean { + const violations = getTransactionViolations(transaction, transactionViolations, currentUserEmail, report, policy); return !!violations?.find((violation) => isBrokenConnectionViolation(violation)); } @@ -1219,13 +1229,22 @@ function shouldShowViolation( /** * Check if there is pending rter violation in all transactionViolations with given transactionIDs. */ -function allHavePendingRTERViolation(transactions: OnyxEntry, transactionViolations: OnyxCollection | undefined): boolean { +function allHavePendingRTERViolation( + transactions: OnyxEntry, + transactionViolations: OnyxCollection | undefined, + currentUserEmail: string, + report: OnyxEntry, + policy: OnyxEntry, +): boolean { if (!transactions) { return false; } const transactionsWithRTERViolations = transactions.map((transaction) => { - const filteredTransactionViolations = getTransactionViolations(transaction, transactionViolations); + const filteredTransactionViolations = getTransactionViolations(transaction, transactionViolations, currentUserEmail, report, policy)?.filter((violation) => + // We should only consider RTER violations when they are visible to the current user + shouldShowViolation(report, policy, violation.name, currentUserEmail), + ); return hasPendingRTERViolation(filteredTransactionViolations); }); return transactionsWithRTERViolations.length > 0 && transactionsWithRTERViolations.every((value) => value === true); @@ -1241,11 +1260,17 @@ function checkIfShouldShowMarkAsCashButton(hasRTERPendingViolation: boolean, sho /** * Check if there is any transaction without RTER violation within the given transactionIDs. */ -function hasAnyTransactionWithoutRTERViolation(transactions: Transaction[] | SearchTransaction[], transactionViolations: OnyxCollection | undefined): boolean { +function hasAnyTransactionWithoutRTERViolation( + transactions: Transaction[] | SearchTransaction[], + transactionViolations: OnyxCollection | undefined, + currentUserEmail: string, + report: OnyxEntry, + policy: OnyxEntry, +): boolean { return ( transactions.length > 0 && transactions.some((transaction) => { - return !hasBrokenConnectionViolation(transaction, transactionViolations); + return !hasBrokenConnectionViolation(transaction, transactionViolations, currentUserEmail, report, policy); }) ); } @@ -1333,7 +1358,7 @@ function getRecentTransactions(transactions: Record, size = 2): * Check if transaction has duplicatedTransaction violation. * @param transactionID - the transaction to check */ -function isDuplicate(transaction: OnyxEntry, currentUserEmail: string): boolean { +function isDuplicate(transaction: OnyxEntry, currentUserEmail: string, iouReport: OnyxEntry, policy: OnyxEntry): boolean { if (!transaction) { return false; } @@ -1341,7 +1366,7 @@ function isDuplicate(transaction: OnyxEntry, currentUserEmail: stri (violation: TransactionViolation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION, ); const hasDuplicatedTransactionViolation = !!duplicatedTransactionViolation; - const isDuplicatedTransactionViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation, currentUserEmail); + const isDuplicatedTransactionViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation, currentUserEmail, iouReport, policy); return hasDuplicatedTransactionViolation && !isDuplicatedTransactionViolationDismissed; } @@ -1358,19 +1383,53 @@ function isOnHold(transaction: OnyxEntry): boolean { } /** - * Checks if a violation is dismissed for the given transaction, optionally by a particular email address + * Checks if a violation is dismissed for the given transaction. */ -function isViolationDismissed(transaction: OnyxEntry, violation: TransactionViolation | undefined, currentUserEmail?: string): boolean { +function isViolationDismissed( + transaction: OnyxEntry, + violation: TransactionViolation | undefined, + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, +): boolean { if (!transaction || !violation) { return false; } - // If we don't provide an email, we just check if the violation has been dismissed by anyone - if (!currentUserEmail) { - return !!transaction?.comment?.dismissedViolations?.[violation.name]; + const violationDismissals = transaction.comment?.dismissedViolations?.[violation.name]; + if (!violationDismissals) { + return false; + } + + const dismissedByEmails = Object.keys(violationDismissals); + + // Current user dismissed it themselves + if (dismissedByEmails.includes(currentUserEmail || deprecatedCurrentUserEmail)) { + return true; } - return !!transaction?.comment?.dismissedViolations?.[violation.name]?.[currentUserEmail]; + // RTER violations on instant submit reports only need to be dismissed by one person to be considered dismissed + if (violation.name === CONST.VIOLATIONS.RTER && policy && isInstantSubmitEnabled(policy)) { + return dismissedByEmails.length > 0; + } + + // If the admin is looking at an open report, we check for both, submitter and admin. + if (!iouReport) { + return false; + } + + const currentUserAccountID = deprecatedCurrentUserAccountID; + const isSubmitter = iouReport.ownerAccountID === currentUserAccountID; + const shouldViewAsSubmitter = !isSubmitter && isOpenExpenseReport(iouReport); + + if (shouldViewAsSubmitter && iouReport.ownerAccountID) { + const reportOwnerEmail = getLoginsByAccountIDs([iouReport.ownerAccountID]).at(0); + if (reportOwnerEmail && dismissedByEmails.includes(reportOwnerEmail)) { + return true; + } + } + + return false; } /** @@ -1386,7 +1445,14 @@ function doesTransactionSupportViolations(transaction: Transaction | undefined): /** * Checks if any violations for the provided transaction are of type 'violation' */ -function hasViolation(transaction: Transaction | undefined, transactionViolations: TransactionViolation[] | OnyxCollection, showInReview?: boolean): boolean { +function hasViolation( + transaction: Transaction | undefined, + transactionViolations: TransactionViolation[] | OnyxCollection, + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, + showInReview?: boolean, +): boolean { if (!doesTransactionSupportViolations(transaction)) { return false; } @@ -1396,7 +1462,7 @@ function hasViolation(transaction: Transaction | undefined, transactionViolation (violation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION && (showInReview === undefined || showInReview === (violation.showInReview ?? false)) && - !isViolationDismissed(transaction, violation), + !isViolationDismissed(transaction, violation, currentUserEmail, iouReport, policy), ); } @@ -1404,7 +1470,7 @@ function hasDuplicateTransactions(currentUserEmail: string, iouReportID?: string const transactionsByIouReportID = getReportTransactions(iouReportID); const reportTransactions = allReportTransactions ?? transactionsByIouReportID; - return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail)); + return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail, undefined, undefined)); } /** @@ -1413,6 +1479,9 @@ function hasDuplicateTransactions(currentUserEmail: string, iouReportID?: string function hasNoticeTypeViolation( transaction: OnyxEntry, transactionViolations: TransactionViolation[] | OnyxCollection, + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, showInReview?: boolean, ): boolean { if (!doesTransactionSupportViolations(transaction)) { @@ -1424,7 +1493,7 @@ function hasNoticeTypeViolation( (violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.NOTICE && (showInReview === undefined || showInReview === (violation.showInReview ?? false)) && - !isViolationDismissed(transaction, violation), + !isViolationDismissed(transaction, violation, currentUserEmail, iouReport, policy), ); } @@ -1434,18 +1503,22 @@ function hasNoticeTypeViolation( function hasWarningTypeViolation( transaction: OnyxEntry, transactionViolations: TransactionViolation[] | OnyxCollection, + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, showInReview?: boolean, ): boolean { if (!doesTransactionSupportViolations(transaction)) { return false; } const violations = Array.isArray(transactionViolations) ? transactionViolations : transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction?.transactionID}`]; + const warningTypeViolations = violations?.filter( (violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.WARNING && (showInReview === undefined || showInReview === (violation.showInReview ?? false)) && - !isViolationDismissed(transaction, violation), + !isViolationDismissed(transaction, violation, currentUserEmail, iouReport, policy), ) ?? []; return warningTypeViolations.length > 0; @@ -1978,8 +2051,8 @@ function getAllSortedTransactions(iouReportID?: string): Array, policy: OnyxEntry) { + return transactions?.length === 1 && hasPendingUI(transactions?.at(0), getTransactionViolations(transactions?.at(0), allTransactionViolations, currentUserEmail, report, policy)); } function isExpenseSplit(transaction: OnyxEntry, originalTransaction: OnyxEntry): boolean { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 80543cd2c9da..19a07be7ef2f 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -258,7 +258,7 @@ import {buildAddMembersToWorkspaceOnyxData, buildUpdateWorkspaceMembersRoleOnyxD import {buildOptimisticRecentlyUsedCurrencies, buildPolicyData, generatePolicyID} from './Policy/Policy'; import {buildOptimisticPolicyRecentlyUsedTags, getPolicyTagsData} from './Policy/Tag'; import type {GuidedSetupData} from './Report'; -import {buildInviteToRoomOnyxData, completeOnboarding, getCurrentUserAccountID, notifyNewAction, optimisticReportLastData} from './Report'; +import {buildInviteToRoomOnyxData, completeOnboarding, getCurrentUserAccountID, getCurrentUserEmail, notifyNewAction, optimisticReportLastData} from './Report'; import {clearAllRelatedReportActionErrors} from './ReportActions'; import {sanitizeRecentWaypoints} from './Transaction'; import {removeDraftSplitTransaction, removeDraftTransaction, removeDraftTransactions} from './TransactionEdit'; @@ -10196,10 +10196,11 @@ function canSubmitReport( isReportArchived: boolean, ) { const currentUserAccountID = getCurrentUserAccountID(); + const currentUserEmailValue = getCurrentUserEmail() ?? ''; const isOpenExpenseReport = isOpenExpenseReportReportUtils(report); const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN; - const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, allViolations); - const hasTransactionWithoutRTERViolation = hasAnyTransactionWithoutRTERViolation(transactions, allViolations); + const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, allViolations, currentUserEmail, report, policy); + const hasTransactionWithoutRTERViolation = hasAnyTransactionWithoutRTERViolation(transactions, allViolations, currentUserEmailValue, report, policy); const hasOnlyPendingCardOrScanFailTransactions = transactions.length > 0 && transactions.every((t) => isPendingCardOrScanningTransaction(t)); return ( @@ -10424,7 +10425,7 @@ function approveMoneyRequest( // Remove duplicates violations if we approve the report if (hasDuplicates) { - const transactions = getReportTransactions(expenseReport.reportID).filter((transaction) => isDuplicate(transaction, currentUserEmailParam)); + const transactions = getReportTransactions(expenseReport.reportID).filter((transaction) => isDuplicate(transaction, currentUserEmailParam, expenseReport, policy)); if (!full) { transactions.filter((transaction) => !isOnHold(transaction)); } diff --git a/tests/unit/IOUUtilsTest.ts b/tests/unit/IOUUtilsTest.ts index 0830c9ca0d53..de4a5b7dd3b0 100644 --- a/tests/unit/IOUUtilsTest.ts +++ b/tests/unit/IOUUtilsTest.ts @@ -248,7 +248,7 @@ describe('hasRTERWithoutViolation', () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDWithViolation}`, transactionWithViolation); await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDWithoutViolation}`, transactionWithoutViolation); - expect(hasAnyTransactionWithoutRTERViolation([transactionWithoutViolation, transactionWithViolation], violations)).toBe(true); + expect(hasAnyTransactionWithoutRTERViolation([transactionWithoutViolation, transactionWithViolation], violations, '', undefined, undefined)).toBe(true); }); test('Return false if there is no rter without violation in all transactionViolations with given transactionIDs.', async () => { @@ -277,7 +277,7 @@ describe('hasRTERWithoutViolation', () => { }; await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDWithViolation}`, transactionWithViolation); - expect(hasAnyTransactionWithoutRTERViolation([transactionWithViolation], violations)).toBe(false); + expect(hasAnyTransactionWithoutRTERViolation([transactionWithViolation], violations, '', undefined, undefined)).toBe(false); }); }); diff --git a/tests/unit/ReportPrimaryActionUtilsTest.ts b/tests/unit/ReportPrimaryActionUtilsTest.ts index 8f9daca8b6e3..bfb399f1dcf5 100644 --- a/tests/unit/ReportPrimaryActionUtilsTest.ts +++ b/tests/unit/ReportPrimaryActionUtilsTest.ts @@ -753,7 +753,7 @@ describe('isReviewDuplicatesAction', () => { } as TransactionViolation, ]); - expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL)).toBe(true); + expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL, undefined)).toBe(true); }); it('should return false when report approver has no duplicated transactions', async () => { @@ -773,7 +773,7 @@ describe('isReviewDuplicatesAction', () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction); - expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL)).toBe(false); + expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL, undefined)).toBe(false); }); it('should return false when current user is neither the report submitter nor approver', async () => { @@ -798,7 +798,7 @@ describe('isReviewDuplicatesAction', () => { } as TransactionViolation, ]); - expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL)).toBe(false); + expect(isReviewDuplicatesAction(report, [transaction], CURRENT_USER_EMAIL, undefined)).toBe(false); }); }); @@ -1191,7 +1191,7 @@ describe('getTransactionThreadPrimaryAction', () => { } as unknown as Transaction, ]; - const result = isPrimaryMarkAsResolvedAction(report, reportTransactions, violations, policy); + const result = isPrimaryMarkAsResolvedAction(CURRENT_USER_EMAIL, report, reportTransactions, violations, policy); expect(result).toBe(true); }); @@ -1224,7 +1224,7 @@ describe('getTransactionThreadPrimaryAction', () => { } as unknown as Transaction, ]; - const result = isPrimaryMarkAsResolvedAction(report, reportTransactions, violations, policy); + const result = isPrimaryMarkAsResolvedAction(CURRENT_USER_EMAIL, report, reportTransactions, violations, policy); expect(result).toBe(false); }); @@ -1254,7 +1254,7 @@ describe('getTransactionThreadPrimaryAction', () => { } as unknown as Transaction, ]; - const result = isPrimaryMarkAsResolvedAction(report, reportTransactions, violations, policy); + const result = isPrimaryMarkAsResolvedAction(CURRENT_USER_EMAIL, report, reportTransactions, violations, policy); expect(result).toBe(false); }); @@ -1284,7 +1284,7 @@ describe('getTransactionThreadPrimaryAction', () => { } as unknown as Transaction, ]; - const result = isPrimaryMarkAsResolvedAction(report, reportTransactions, violations, policy); + const result = isPrimaryMarkAsResolvedAction(CURRENT_USER_EMAIL, report, reportTransactions, violations, policy); expect(result).toBe(false); }); }); diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index e3285dca8e8d..76a989652e4b 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -407,7 +407,7 @@ describe('TransactionUtils', () => { state: CONST.IOU.RECEIPT_STATE.SCAN_READY, }, }); - expect(TransactionUtils.shouldShowRTERViolationMessage([transaction])).toBe(true); + expect(TransactionUtils.shouldShowRTERViolationMessage([transaction], '', undefined, undefined)).toBe(true); }); }); @@ -718,73 +718,309 @@ describe('TransactionUtils', () => { }); describe('isViolationDismissed', () => { - it('should return true when violation is dismissed for current user', () => { - const transaction = generateTransaction({ - comment: { - dismissedViolations: { - [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { - [CURRENT_USER_EMAIL]: DateUtils.getDBTime(), + beforeAll(() => { + Onyx.init({ + keys: ONYXKEYS, + initialKeyStates: { + [ONYXKEYS.SESSION]: { + email: CURRENT_USER_EMAIL, + accountID: CURRENT_USER_ID, + }, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [CURRENT_USER_ID]: { + accountID: CURRENT_USER_ID, + login: CURRENT_USER_EMAIL, + displayName: 'Current User', + }, + [SECOND_USER_ID]: { + accountID: SECOND_USER_ID, + login: OTHER_USER_EMAIL, + displayName: 'Second User', }, }, }, }); - const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; - const result = TransactionUtils.isViolationDismissed(transaction, violation); - expect(result).toBe(true); }); - it('should return true when violation is dismissed by another user and no email is provided', () => { - // Given a transaction with a violation dismissed by another user - const transaction = generateTransaction({ - comment: { - dismissedViolations: { - [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { - [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + afterEach(() => Onyx.clear()); + + describe('Current user dismissed it themselves', () => { + it('should return true when current user dismissed the violation', () => { + // Given a transaction with a violation dismissed by current user + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [CURRENT_USER_EMAIL]: DateUtils.getDBTime(), + }, }, }, - }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When checking if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, undefined); + + // Then it should return true + expect(result).toBe(true); }); - const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; - // When checking if the violation is dismissed without providing a user email - const result = TransactionUtils.isViolationDismissed(transaction, violation, undefined); + it('should return false when violation is not dismissed at all', () => { + // Given a transaction with no dismissed violations + const transaction = generateTransaction({ + comment: {}, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; - // Then it should return true since the violation has been dismissed by someone - expect(result).toBe(true); + // When checking if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, undefined); + + // Then it should return false + expect(result).toBe(false); + }); + + it('should return false when violation was dismissed by someone else only', () => { + // Given a transaction with a violation dismissed by another user + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When checking if violation is dismissed for current user + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, undefined); + + // Then it should return false since current user hasn't dismissed it + expect(result).toBe(false); + }); }); - it('should return false when violation is not dismissed and no email is provided', () => { - // Given a transaction with no dismissed violations - const transaction = generateTransaction({ - comment: {}, + describe('Admin viewing OPEN report AND report owner dismissed it', () => { + it('should return true when admin views open report and owner dismissed violation', () => { + // Given an OPEN report owned by user 2 + const iouReport: Report = { + ...openReport, + ownerAccountID: SECOND_USER_ID, + }; + + // And a transaction where owner (user 2) dismissed a violation + const transaction = generateTransaction({ + reportID: iouReport.reportID, + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When current user (admin, not the owner) checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, iouReport, undefined); + + // Then it should return true because admin sees owner's perspective on open reports + expect(result).toBe(true); }); - const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; - // When checking if the violation is dismissed without providing a user email - const result = TransactionUtils.isViolationDismissed(transaction, violation, undefined); + it('should return false when admin views PROCESSING report and only owner dismissed violation', () => { + // Given a PROCESSING report (not OPEN) owned by user 2 + const iouReport: Report = { + ...processingReport, + ownerAccountID: SECOND_USER_ID, + }; - // Then it should return false since the violation has not been dismissed - expect(result).toBe(false); + // And a transaction where owner dismissed a violation + const transaction = generateTransaction({ + reportID: iouReport.reportID, + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When current user (admin, not the owner) checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, iouReport, undefined); + + // Then it should return false because on processing reports, admin must dismiss separately + expect(result).toBe(false); + }); + + it('should return false when submitter views their own open report (not condition 2)', () => { + // Given an OPEN report owned by current user + const iouReport: Report = { + ...openReport, + ownerAccountID: CURRENT_USER_ID, + }; + + // And a transaction where someone else dismissed a violation + const transaction = generateTransaction({ + reportID: iouReport.reportID, + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When current user (the submitter) checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, iouReport, undefined); + + // Then it should return false (condition 2 doesn't apply to submitters) + expect(result).toBe(false); + }); }); - it('should return false when violation is dismissed by different user and email is provided', () => { - // Given a transaction with a violation dismissed by a different user - const transaction = generateTransaction({ - comment: { - dismissedViolations: { - [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { - [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + describe('RTER violation on instant submit policy - dismissed by anyone', () => { + it('should return true when RTER violation dismissed by anyone on instant submit policy', () => { + // Given an instant submit policy + const policy: Policy = { + ...createRandomPolicy(0, CONST.POLICY.TYPE.TEAM), + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + }; + + // And a transaction with RTER violation dismissed by someone else + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.RTER]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, }, }, - }, + }); + const violation = {type: CONST.VIOLATION_TYPES.WARNING, name: CONST.VIOLATIONS.RTER}; + + // When current user checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, policy); + + // Then it should return true because on instant submit, anyone's dismissal counts + expect(result).toBe(true); }); - const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; - // When checking if the violation is dismissed for the current user - const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL); + it('should return false when RTER violation dismissed by anyone on NON-instant submit policy', () => { + // Given a non-instant submit policy + const policy: Policy = { + ...createRandomPolicy(0, CONST.POLICY.TYPE.TEAM), + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.WEEKLY, + }; - // Then it should return false since the current user has not dismissed it - expect(result).toBe(false); + // And a transaction with RTER violation dismissed by someone else + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.RTER]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.WARNING, name: CONST.VIOLATIONS.RTER}; + + // When current user checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, policy); + + // Then it should return false because on non-instant submit, each person must dismiss separately + expect(result).toBe(false); + }); + + it('should return false when non-RTER violation dismissed by anyone on instant submit policy', () => { + // Given an instant submit policy + const policy: Policy = { + ...createRandomPolicy(0, CONST.POLICY.TYPE.TEAM), + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + }; + + // And a transaction with non-RTER violation dismissed by someone else + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.DUPLICATED_TRANSACTION]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.WARNING, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + + // When current user checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, policy); + + // Then it should return false because condition 3 only applies to RTER violations + expect(result).toBe(false); + }); + + it('should return true when RTER violation dismissed by multiple people on instant submit policy', () => { + // Given an instant submit policy + const policy: Policy = { + ...createRandomPolicy(0, CONST.POLICY.TYPE.TEAM), + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + }; + + // And a transaction with RTER violation dismissed by multiple people + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.RTER]: { + [OTHER_USER_EMAIL]: DateUtils.getDBTime(), + [CURRENT_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.WARNING, name: CONST.VIOLATIONS.RTER}; + + // When current user checks if violation is dismissed + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, policy); + + // Then it should return true + expect(result).toBe(true); + }); + }); + + describe('Edge cases and data validation', () => { + it('should return false when transaction is null', () => { + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + const result = TransactionUtils.isViolationDismissed(undefined, violation, CURRENT_USER_EMAIL, undefined, undefined); + expect(result).toBe(false); + }); + + it('should return false when violation is null', () => { + const transaction = generateTransaction({}); + const result = TransactionUtils.isViolationDismissed(transaction, undefined, CURRENT_USER_EMAIL, undefined, undefined); + expect(result).toBe(false); + }); + + it('should return false when violation name does not exist in dismissedViolations', () => { + const transaction = generateTransaction({ + comment: { + dismissedViolations: { + [CONST.VIOLATIONS.SMARTSCAN_FAILED]: { + [CURRENT_USER_EMAIL]: DateUtils.getDBTime(), + }, + }, + }, + }); + const violation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION}; + const result = TransactionUtils.isViolationDismissed(transaction, violation, CURRENT_USER_EMAIL, undefined, undefined); + expect(result).toBe(false); + }); }); }); diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 02d605138b94..29318e50aefa 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -602,8 +602,8 @@ describe('getViolations', () => { await Onyx.multiSet({...transactionCollectionDataSet}); - const isSmartScanDismissed = isViolationDismissed(transaction, smartScanFailedViolation); - const isDuplicateViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation); + const isSmartScanDismissed = isViolationDismissed(transaction, smartScanFailedViolation, CARLOS_EMAIL, undefined, undefined); + const isDuplicateViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation, CARLOS_EMAIL, undefined, undefined); expect(isSmartScanDismissed).toBeTruthy(); expect(isDuplicateViolationDismissed).toBeFalsy(); @@ -625,7 +625,7 @@ describe('getViolations', () => { await Onyx.multiSet({...transactionCollectionDataSet}); // Should filter out the smartScanFailedViolation - const filteredViolations = getTransactionViolations(transaction, transactionViolationsCollection); + const filteredViolations = getTransactionViolations(transaction, transactionViolationsCollection, CARLOS_EMAIL, undefined, undefined); expect(filteredViolations).toEqual([duplicatedTransactionViolation, tagOutOfPolicyViolation]); }); @@ -643,7 +643,7 @@ describe('getViolations', () => { }; await Onyx.multiSet({...transactionCollectionDataSet}); - const hasWarningTypeViolationRes = hasWarningTypeViolation(transaction, transactionViolationsCollection); + const hasWarningTypeViolationRes = hasWarningTypeViolation(transaction, transactionViolationsCollection, '', undefined, undefined); expect(hasWarningTypeViolationRes).toBeTruthy(); }); }); From 9b6a154dea578e064d609b9c19ad7df5a667628e Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Mon, 17 Nov 2025 21:01:29 +0100 Subject: [PATCH 14/25] add comment --- src/components/MoneyReportHeader.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index e8ac972b59f4..2e09d9924636 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -552,6 +552,8 @@ function MoneyReportHeader({ return {icon: getStatusIcon(Expensicons.Flag), description: translate('iou.duplicateTransaction', {isSubmitted: isProcessingReport(moneyRequestReport)})}; } + // Show the broken connection violation message only if it's part of transactionViolations (i.e., visible to the user). + // This prevents displaying an empty message. if (!!transaction?.transactionID && !!transactionViolations.length && shouldShowBrokenConnectionViolation) { return { icon: getStatusIcon(Expensicons.Hourglass), From 1f79c59d0a7006c53189f97b1fca84d5550af3da Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 18 Nov 2025 14:19:01 +0100 Subject: [PATCH 15/25] clarify comments --- src/libs/TransactionUtils/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 2b7d527af738..56cb7f405f82 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1241,10 +1241,12 @@ function allHavePendingRTERViolation( } const transactionsWithRTERViolations = transactions.map((transaction) => { + // Get violations not dismissed by current user const filteredTransactionViolations = getTransactionViolations(transaction, transactionViolations, currentUserEmail, report, policy)?.filter((violation) => - // We should only consider RTER violations when they are visible to the current user + // Further filter to only violations visible to the current user shouldShowViolation(report, policy, violation.name, currentUserEmail), ); + // Check if there is pending rter violation in the filtered violations return hasPendingRTERViolation(filteredTransactionViolations); }); return transactionsWithRTERViolations.length > 0 && transactionsWithRTERViolations.every((value) => value === true); From 271273f31b93cf957778a6d7f11f70da6588f626 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 18 Nov 2025 14:28:30 +0100 Subject: [PATCH 16/25] fix hasDuplicateTransactions references --- src/components/MoneyReportHeader.tsx | 2 +- src/libs/TransactionUtils/index.ts | 11 ++++++++--- src/libs/actions/IOU.ts | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index 2e09d9924636..eb6ae0b88850 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -392,7 +392,7 @@ function MoneyReportHeader({ const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE; - const hasDuplicates = hasDuplicateTransactions(email ?? '', moneyRequestReport?.reportID); + const hasDuplicates = hasDuplicateTransactions(email ?? '', moneyRequestReport, policy); const shouldShowMarkAsResolved = isMarkAsResolvedAction(moneyRequestReport, transactionViolations); const shouldShowStatusBar = hasAllPendingRTERViolations || diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 56cb7f405f82..1c7e7b54cf11 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1468,11 +1468,16 @@ function hasViolation( ); } -function hasDuplicateTransactions(currentUserEmail: string, iouReportID?: string, allReportTransactions?: SearchTransaction[]): boolean { - const transactionsByIouReportID = getReportTransactions(iouReportID); +function hasDuplicateTransactions( + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, + allReportTransactions?: SearchTransaction[], +): boolean { + const transactionsByIouReportID = getReportTransactions(iouReport?.reportID); const reportTransactions = allReportTransactions ?? transactionsByIouReportID; - return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail, undefined, undefined)); + return reportTransactions.length > 0 && reportTransactions.some((transaction) => isDuplicate(transaction, currentUserEmail, iouReport, policy)); } /** diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 19a07be7ef2f..7af2ab3c08b3 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10251,7 +10251,7 @@ function approveMoneyRequest( const currentNextStepDeprecated = allNextSteps[`${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`] ?? null; let total = expenseReport.total ?? 0; const hasHeldExpenses = hasHeldExpensesReportUtils(expenseReport.reportID); - const hasDuplicates = hasDuplicateTransactions(currentUserEmailParam, expenseReport.reportID); + const hasDuplicates = hasDuplicateTransactions(currentUserEmailParam, expenseReport, policy); if (hasHeldExpenses && !full && !!expenseReport.unheldTotal) { total = expenseReport.unheldTotal; } From 6ebfa135fdb2354300b27c3c2a36aa11b7594e47 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 18 Nov 2025 14:47:17 +0100 Subject: [PATCH 17/25] prettier --- src/libs/TransactionUtils/index.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 1c7e7b54cf11..581a075b9ead 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1468,12 +1468,7 @@ function hasViolation( ); } -function hasDuplicateTransactions( - currentUserEmail: string, - iouReport: OnyxEntry, - policy: OnyxEntry, - allReportTransactions?: SearchTransaction[], -): boolean { +function hasDuplicateTransactions(currentUserEmail: string, iouReport: OnyxEntry, policy: OnyxEntry, allReportTransactions?: SearchTransaction[]): boolean { const transactionsByIouReportID = getReportTransactions(iouReport?.reportID); const reportTransactions = allReportTransactions ?? transactionsByIouReportID; From e515eb6f1b9d8e39ee61576582080661fb1c3c97 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 18 Nov 2025 16:04:11 +0100 Subject: [PATCH 18/25] Fix "Mark as cash" button displayed for invisible violation --- src/libs/TransactionUtils/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 3389a1d5543e..66976ccbbb51 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1180,7 +1180,7 @@ function shouldShowBrokenConnectionViolationForMultipleTransactions( ): boolean { const violations = transactionIDs.flatMap((id) => transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []); - const brokenConnectionViolations = violations.filter((violation) => isBrokenConnectionViolation(violation)); + const brokenConnectionViolations = violations.filter((violation) => isBrokenConnectionViolation(violation) && shouldShowViolation(report, policy, violation.name, deprecatedCurrentUserEmail)); return shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations, report, policy); } From 8c4fbf04f663a7fce086aa70c42c7b807b36d84e Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Tue, 18 Nov 2025 16:11:46 +0100 Subject: [PATCH 19/25] prettier --- src/libs/TransactionUtils/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 66976ccbbb51..a4426dd3d4bc 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1180,7 +1180,9 @@ function shouldShowBrokenConnectionViolationForMultipleTransactions( ): boolean { const violations = transactionIDs.flatMap((id) => transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []); - const brokenConnectionViolations = violations.filter((violation) => isBrokenConnectionViolation(violation) && shouldShowViolation(report, policy, violation.name, deprecatedCurrentUserEmail)); + const brokenConnectionViolations = violations.filter( + (violation) => isBrokenConnectionViolation(violation) && shouldShowViolation(report, policy, violation.name, deprecatedCurrentUserEmail), + ); return shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations, report, policy); } From 0f5570703d8ad8d4042aaa781c81d34c38b40d80 Mon Sep 17 00:00:00 2001 From: rayane-d <77965000+rayane-d@users.noreply.github.com> Date: Tue, 18 Nov 2025 22:24:27 +0100 Subject: [PATCH 20/25] Apply suggestions from code review Co-authored-by: Rocio Perez-Cano --- src/libs/TransactionPreviewUtils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/TransactionPreviewUtils.ts b/src/libs/TransactionPreviewUtils.ts index 9e36143e99cf..d3708c73ac38 100644 --- a/src/libs/TransactionPreviewUtils.ts +++ b/src/libs/TransactionPreviewUtils.ts @@ -213,6 +213,7 @@ function getTransactionPreviewTextAndTranslationPaths({ const hasFieldErrors = hasMissingSmartscanFields(transaction); const isPaidGroupPolicy = isPaidGroupPolicyUtil(iouReport); const currentUserEmail = getCurrentUserEmail(); + // 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(iouReport?.policyID); @@ -359,6 +360,7 @@ function createTransactionPreviewConditionals({ const isSettlementOrApprovalPartial = !!iouReport?.pendingFields?.partial; const currentUserEmail = getCurrentUserEmail(); + // 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(iouReport?.policyID); From 79a7c949b97789d7ccf5b332a7207846185e79bb Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Wed, 19 Nov 2025 00:40:47 +0100 Subject: [PATCH 21/25] add more tests --- tests/unit/ViolationUtilsTest.ts | 120 +++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 29318e50aefa..f2feee097f70 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -609,6 +609,45 @@ describe('getViolations', () => { expect(isDuplicateViolationDismissed).toBeFalsy(); }); + it('should check if violation is dismissed or not (with report and policy params)', async () => { + const policy: Policy = { + id: 'test-policy-id', + name: 'Test Policy', + type: CONST.POLICY.TYPE.TEAM, + role: CONST.POLICY.ROLE.ADMIN, + owner: CARLOS_EMAIL, + isPolicyExpenseChatEnabled: false, + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.WEEKLY, + outputCurrency: CONST.CURRENCY.USD, + }; + + const report: Report = { + reportID: 'test-report-id', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: CARLOS_ACCOUNT_ID, + policyID: policy.id, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + }; + + const transaction = getFakeTransaction('123', { + dismissedViolations: {smartscanFailed: {[CARLOS_EMAIL]: CARLOS_ACCOUNT_ID.toString()}}, + }); + + const transactionCollectionDataSet: TransactionCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction, + }; + + await Onyx.multiSet({...transactionCollectionDataSet}); + + const isSmartScanDismissed = isViolationDismissed(transaction, smartScanFailedViolation, CARLOS_EMAIL, report, policy); + const isDuplicateViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation, CARLOS_EMAIL, report, policy); + + expect(isSmartScanDismissed).toBeTruthy(); + expect(isDuplicateViolationDismissed).toBeFalsy(); + }); + it('should return filtered out dismissed violations', async () => { const transaction = getFakeTransaction('123', { dismissedViolations: {smartscanFailed: {[CARLOS_EMAIL]: CARLOS_ACCOUNT_ID.toString()}}, @@ -629,6 +668,47 @@ describe('getViolations', () => { expect(filteredViolations).toEqual([duplicatedTransactionViolation, tagOutOfPolicyViolation]); }); + it('should return filtered out dismissed violations (with report and policy params)', async () => { + const policy: Policy = { + id: 'test-policy-id', + name: 'Test Policy', + type: CONST.POLICY.TYPE.TEAM, + role: CONST.POLICY.ROLE.ADMIN, + owner: CARLOS_EMAIL, + isPolicyExpenseChatEnabled: false, + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + outputCurrency: CONST.CURRENCY.USD, + }; + + const report: Report = { + reportID: 'test-report-id', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: CARLOS_ACCOUNT_ID, + policyID: policy.id, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED, + }; + + const transaction = getFakeTransaction('123', { + dismissedViolations: {smartscanFailed: {[CARLOS_EMAIL]: CARLOS_ACCOUNT_ID.toString()}}, + }); + + const transactionCollectionDataSet: TransactionCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction, + }; + + const transactionViolationsCollection = { + [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`]: [duplicatedTransactionViolation, smartScanFailedViolation, tagOutOfPolicyViolation], + }; + + await Onyx.multiSet({...transactionCollectionDataSet}); + + // Should filter out the smartScanFailedViolation + const filteredViolations = getTransactionViolations(transaction, transactionViolationsCollection, CARLOS_EMAIL, report, policy); + expect(filteredViolations).toEqual([duplicatedTransactionViolation, tagOutOfPolicyViolation]); + }); + it('checks if transaction has warning type violation after filtering dismissed violations', async () => { const transaction = getFakeTransaction('123', { dismissedViolations: {smartscanFailed: {[CARLOS_EMAIL]: CARLOS_ACCOUNT_ID.toString()}}, @@ -646,6 +726,46 @@ describe('getViolations', () => { const hasWarningTypeViolationRes = hasWarningTypeViolation(transaction, transactionViolationsCollection, '', undefined, undefined); expect(hasWarningTypeViolationRes).toBeTruthy(); }); + + it('checks if transaction has warning type violation after filtering dismissed violations (with report and policy params)', async () => { + const policy: Policy = { + id: 'test-policy-id', + name: 'Test Policy', + type: CONST.POLICY.TYPE.TEAM, + role: CONST.POLICY.ROLE.ADMIN, + owner: CARLOS_EMAIL, + isPolicyExpenseChatEnabled: false, + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MONTHLY, + outputCurrency: CONST.CURRENCY.USD, + pendingAction: undefined, + }; + + const report: Report = { + reportID: 'test-report-id', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: CARLOS_ACCOUNT_ID, + policyID: policy.id, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + }; + + const transaction = getFakeTransaction('123', { + dismissedViolations: {smartscanFailed: {[CARLOS_EMAIL]: CARLOS_ACCOUNT_ID.toString()}}, + }); + + const transactionCollectionDataSet: TransactionCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]: transaction, + }; + + const transactionViolationsCollection = { + [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`]: [duplicatedTransactionViolation, smartScanFailedViolation, tagOutOfPolicyViolation], + }; + + await Onyx.multiSet({...transactionCollectionDataSet}); + const hasWarningTypeViolationRes = hasWarningTypeViolation(transaction, transactionViolationsCollection, CARLOS_EMAIL, report, policy); + expect(hasWarningTypeViolationRes).toBeTruthy(); + }); }); const brokenCardConnectionViolation: TransactionViolation = { From 716050e609bf748ff42e16df28575c9058aef71c Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Wed, 19 Nov 2025 15:28:04 +0100 Subject: [PATCH 22/25] - pass reportTransactions into hasVisibleViolationsForUser - make the transactions param in hasVisibleViolationsForUser required - make sure we pass the params everywhere --- src/components/MoneyReportHeader.tsx | 2 +- src/libs/ReportPreviewActionUtils.ts | 37 +++++++++++-------- src/libs/ReportPrimaryActionUtils.ts | 3 +- src/libs/ReportSecondaryActionUtils.ts | 4 +- src/libs/TransactionUtils/index.ts | 25 ++++++++++--- src/libs/Violations/ViolationsUtils.ts | 11 +++--- tests/actions/ReportPreviewActionUtilsTest.ts | 3 +- tests/unit/TransactionUtilsTest.ts | 9 ++++- tests/unit/ViolationUtilsTest.ts | 4 +- 9 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/components/MoneyReportHeader.tsx b/src/components/MoneyReportHeader.tsx index eb6ae0b88850..660cab74f70e 100644 --- a/src/components/MoneyReportHeader.tsx +++ b/src/components/MoneyReportHeader.tsx @@ -303,7 +303,7 @@ function MoneyReportHeader({ [transactions, violations, email, moneyRequestReport, policy], ); // Check if user should see broken connection violation warning. - const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, moneyRequestReport, policy, violations); + const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactions, moneyRequestReport, policy, violations, email ?? ''); const hasOnlyHeldExpenses = hasOnlyHeldExpensesReportUtils(moneyRequestReport?.reportID); const isPayAtEndExpense = isPayAtEndExpenseTransactionUtils(transaction); const isArchivedReport = useReportIsArchived(moneyRequestReport?.reportID); diff --git a/src/libs/ReportPreviewActionUtils.ts b/src/libs/ReportPreviewActionUtils.ts index 4addb80fb0e4..2b37a1936a0c 100644 --- a/src/libs/ReportPreviewActionUtils.ts +++ b/src/libs/ReportPreviewActionUtils.ts @@ -61,27 +61,28 @@ function canApprove( report: Report, violations: OnyxCollection, currentUserEmail: string, - policy?: Policy, - transactions?: Transaction[], + policy: Policy | undefined, + transactions: Transaction[], shouldConsiderViolations = true, ) { const currentUserID = getCurrentUserAccountID(); const isExpense = isExpenseReport(report); const isProcessing = isProcessingReport(report); - const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false; - const managerID = report?.managerID ?? CONST.DEFAULT_NUMBER_ID; + const isApprovalEnabled = policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL; + const managerID = report.managerID ?? CONST.DEFAULT_NUMBER_ID; const isCurrentUserManager = managerID === currentUserID; // We should consider only visible violations for the approver, invisible violations should not block approval - const hasAnyVisibleViolations = hasMissingSmartscanFields(report.reportID, transactions) || ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy); - const reportTransactions = transactions ?? getReportTransactions(report?.reportID); - const isAnyReceiptBeingScanned = transactions?.some((transaction) => isScanning(transaction)); + const reportTransactions = transactions.length ? transactions : getReportTransactions(report?.reportID); + const hasAnyVisibleViolations = + hasMissingSmartscanFields(report.reportID, reportTransactions) || ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, reportTransactions); + const isAnyReceiptBeingScanned = reportTransactions?.some((transaction) => isScanning(transaction)); if (isAnyReceiptBeingScanned) { return false; } - if (!!transactions && transactions?.length > 0 && transactions.every((transaction) => isPending(transaction))) { + if (reportTransactions.length > 0 && reportTransactions.every((transaction) => isPending(transaction))) { return false; } @@ -180,7 +181,14 @@ function canExport(report: Report, violations: OnyxCollection, isReportArchived: boolean, currentUserEmail: string, policy?: Policy, transactions?: Transaction[]) { +function canReview( + report: Report, + violations: OnyxCollection, + isReportArchived: boolean, + currentUserEmail: string, + policy: Policy | undefined, + transactions: Transaction[], +) { const hasAnyViolations = hasMissingSmartscanFields(report.reportID, transactions) || hasAnyViolationsUtil(report.reportID, violations); const hasVisibleViolations = hasAnyViolations && ViolationsUtils.hasVisibleViolationsForUser(report, violations, currentUserEmail, policy, transactions); const isSubmitter = isCurrentUserSubmitter(report); @@ -199,9 +207,8 @@ function canReview(report: Report, violations: OnyxCollection transaction.transactionID) ?? []; const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactions, violations, currentUserEmail, report, policy); - const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, report, policy, violations); + const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactions, report, policy, violations, currentUserEmail); if (hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!isAdmin || isSubmitter) && !isReportApproved({report}) && !isReportManuallyReimbursed(report))) { return true; @@ -219,9 +226,9 @@ function getReportPreviewAction( violations: OnyxCollection, isReportArchived: boolean, currentUserEmail: string, - report?: Report, - policy?: Policy, - transactions?: Transaction[], + report: Report | undefined, + policy: Policy | undefined, + transactions: Transaction[], invoiceReceiverPolicy?: Policy, isPaidAnimationRunning?: boolean, isApprovedAnimationRunning?: boolean, @@ -241,7 +248,7 @@ function getReportPreviewAction( if (isSubmittingAnimationRunning) { return CONST.REPORT.REPORT_PREVIEW_ACTIONS.SUBMIT; } - if (isAddExpenseAction(report, transactions ?? [], isReportArchived)) { + if (isAddExpenseAction(report, transactions, isReportArchived)) { return CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE; } diff --git a/src/libs/ReportPrimaryActionUtils.ts b/src/libs/ReportPrimaryActionUtils.ts index 69dd230d109b..709c6227c967 100644 --- a/src/libs/ReportPrimaryActionUtils.ts +++ b/src/libs/ReportPrimaryActionUtils.ts @@ -275,7 +275,6 @@ function isMarkAsCashAction(currentUserEmail: string, report: Report, reportTran return false; } - const transactionIDs = reportTransactions.map((t) => t.transactionID); const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations, currentUserEmail, report, policy); if (hasAllPendingRTERViolations) { @@ -286,7 +285,7 @@ function isMarkAsCashAction(currentUserEmail: string, report: Report, reportTran const isReportApprover = isApproverUtils(policy, currentUserEmail); const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN; - const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, report, policy, violations); + const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(reportTransactions, report, policy, violations, currentUserEmail); const userControlsReport = isReportSubmitter || isReportApprover || isAdmin; return userControlsReport && shouldShowBrokenConnectionViolation; } diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index a97a3d379316..a7461cd875b0 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -235,8 +235,6 @@ function isApproveAction(currentUserLogin: string, report: Report, reportTransac return false; } - const transactionIDs = reportTransactions.map((t) => t.transactionID); - const hasAllPendingRTERViolations = allHavePendingRTERViolation(reportTransactions, violations, currentUserLogin, report, policy); if (hasAllPendingRTERViolations) { @@ -245,7 +243,7 @@ function isApproveAction(currentUserLogin: string, report: Report, reportTransac const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN; - const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, report, policy, violations); + const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(reportTransactions, report, policy, violations, currentUserLogin); const isReportApprover = isApproverUtils(policy, currentUserLogin); const userControlsReport = isReportApprover || isAdmin; return userControlsReport && shouldShowBrokenConnectionViolation; diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index a4426dd3d4bc..da2888ba6367 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1172,17 +1172,32 @@ function shouldShowBrokenConnectionViolation(report: OnyxEntry | SearchR * Check if user should see broken connection violation warning based on selected transactions. */ function shouldShowBrokenConnectionViolationForMultipleTransactions( - transactionIDs: string[], + transactions: Transaction[], // eslint-disable-next-line @typescript-eslint/no-deprecated report: OnyxEntry | SearchReport, policy: OnyxEntry, transactionViolations: OnyxCollection, + currentUserEmail: string, ): boolean { - const violations = transactionIDs.flatMap((id) => transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []); + const brokenConnectionViolations = transactions.flatMap((transaction) => { + const violations = transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`] ?? []; - const brokenConnectionViolations = violations.filter( - (violation) => isBrokenConnectionViolation(violation) && shouldShowViolation(report, policy, violation.name, deprecatedCurrentUserEmail), - ); + if (!transaction) { + return []; + } + + return violations.filter((violation) => { + if (!isBrokenConnectionViolation(violation)) { + return false; + } + + if (isViolationDismissed(transaction, violation, currentUserEmail, report, policy)) { + return false; + } + + return shouldShowViolation(report, policy, violation.name, currentUserEmail); + }); + }); return shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations, report, policy); } diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index f33883a6782f..17fdfa4282c7 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -11,7 +11,7 @@ import {isReceiptError} from '@libs/ErrorUtils'; import Parser from '@libs/Parser'; import {getDistanceRateCustomUnitRate, getPerDiemRateCustomUnitRate, getSortedTagKeys, isTaxTrackingEnabled} from '@libs/PolicyUtils'; import * as TransactionUtils from '@libs/TransactionUtils'; -import {hasValidModifiedAmount, shouldShowViolation} from '@libs/TransactionUtils'; +import {hasValidModifiedAmount, isViolationDismissed, shouldShowViolation} from '@libs/TransactionUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Policy, PolicyCategories, PolicyTagLists, Report, ReportAction, Transaction, TransactionViolation, ViolationName} from '@src/types/onyx'; @@ -619,13 +619,14 @@ const ViolationsUtils = { /** * Checks if any transactions in the report have violations that should be visible to the current user. * Filters violations based on user role (submitter, admin, policy member) and report state. + * Also filters out dismissed violations. */ hasVisibleViolationsForUser( report: OnyxEntry, violations: OnyxCollection, currentUserEmail: string, - policy?: OnyxEntry, - transactions?: Transaction[], + policy: OnyxEntry, + transactions: Transaction[], ): boolean { if (!report || !violations || !transactions) { return false; @@ -638,9 +639,9 @@ const ViolationsUtils = { return false; } - // Check if any violation should be shown based on user role and violation type + // Check if any violation is not dismissed and should be shown based on user role and violation type return transactionViolations.some((violation: TransactionViolation) => { - return shouldShowViolation(report, policy, violation.name, currentUserEmail); + return !isViolationDismissed(transaction, violation, currentUserEmail, report, policy) && shouldShowViolation(report, policy, violation.name, currentUserEmail); }); }); }, diff --git a/tests/actions/ReportPreviewActionUtilsTest.ts b/tests/actions/ReportPreviewActionUtilsTest.ts index 1dcab6828c18..215fd56c2887 100644 --- a/tests/actions/ReportPreviewActionUtilsTest.ts +++ b/tests/actions/ReportPreviewActionUtilsTest.ts @@ -73,7 +73,8 @@ describe('getReportPreviewAction', () => { } as unknown as Report; await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report); - expect(getReportPreviewAction(VIOLATIONS, false, CURRENT_USER_EMAIL, report, undefined, [])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE); + const policy = createRandomPolicy(0, CONST.POLICY.TYPE.PERSONAL); + expect(getReportPreviewAction(VIOLATIONS, false, CURRENT_USER_EMAIL, report, policy, [])).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.ADD_EXPENSE); }); it('canSubmit should return true for expense preview report with manual submit', async () => { diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index 543b473ffaf7..c0f988ab5989 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -459,7 +459,6 @@ describe('TransactionUtils', () => { const policy = {role: CONST.POLICY.ROLE.USER} as Policy; const transaction1 = generateTransaction(); const transaction2 = generateTransaction(); - const transactionIDs = [transaction1.transactionID, transaction2.transactionID]; const transactionViolations = { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction1.transactionID}`]: [ { @@ -469,7 +468,13 @@ describe('TransactionUtils', () => { }, ], }; - const showBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions(transactionIDs, undefined, policy, transactionViolations); + const showBrokenConnectionViolation = shouldShowBrokenConnectionViolationForMultipleTransactions( + [transaction1, transaction2], + undefined, + policy, + transactionViolations, + CURRENT_USER_EMAIL, + ); expect(showBrokenConnectionViolation).toBe(true); }); diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index f2feee097f70..9762b504d450 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -896,12 +896,12 @@ describe('hasVisibleViolationsForUser', () => { expect(result).toBe(false); }); - it('should return false when transactions is null', () => { + it('should return false when transactions is empty', () => { const violations = { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${testTransactionID}`]: [missingCategoryViolation], }; - const result = ViolationsUtils.hasVisibleViolationsForUser(mockReport, violations, '', mockPolicy); + const result = ViolationsUtils.hasVisibleViolationsForUser(mockReport, violations, '', mockPolicy, []); expect(result).toBe(false); }); From b61de331fc86ed0ec59cd26649130fed8dca7256 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Wed, 19 Nov 2025 15:36:21 +0100 Subject: [PATCH 23/25] fix failing test --- tests/unit/TransactionUtilsTest.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index c0f988ab5989..01842391bdaf 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -456,7 +456,11 @@ describe('TransactionUtils', () => { }); it('should return true when a broken connection violation exists for any of the provided transactions and the user is the policy member', () => { - const policy = {role: CONST.POLICY.ROLE.USER} as Policy; + const policy = { + role: CONST.POLICY.ROLE.USER, + autoReporting: true, + autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT, + } as Policy; const transaction1 = generateTransaction(); const transaction2 = generateTransaction(); const transactionViolations = { From 9370cb196b9d5f821c474067d47a04c49aef7df8 Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Wed, 19 Nov 2025 15:51:40 +0100 Subject: [PATCH 24/25] fix merge conflicts --- src/libs/Violations/ViolationsUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index ec7c202eb727..7b23792f046c 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -11,7 +11,7 @@ import {isReceiptError} from '@libs/ErrorUtils'; import Parser from '@libs/Parser'; import {getDistanceRateCustomUnitRate, getPerDiemRateCustomUnitRate, getSortedTagKeys, isTaxTrackingEnabled} from '@libs/PolicyUtils'; import * as TransactionUtils from '@libs/TransactionUtils'; -import {hasValidModifiedAmount, isViolationDismissed, shouldShowViolation} from '@libs/TransactionUtils'; +import {isViolationDismissed, shouldShowViolation} from '@libs/TransactionUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Policy, PolicyCategories, PolicyTagLists, Report, ReportAction, Transaction, TransactionViolation, ViolationName} from '@src/types/onyx'; From 2719db58f49023f80b179d4091014bfe1c88db7f Mon Sep 17 00:00:00 2001 From: Rayane <77965000+rayane-d@users.noreply.github.com> Date: Fri, 21 Nov 2025 15:28:06 +0100 Subject: [PATCH 25/25] fix lint-changed errors --- src/libs/TransactionUtils/index.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 9b2e2ea6d5a7..da2cd6b5f4ca 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -68,6 +68,7 @@ import type { import type {Attendee, Participant, SplitExpense} from '@src/types/onyx/IOU'; import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon'; import type {OnyxData} from '@src/types/onyx/Request'; +// eslint-disable-next-line @typescript-eslint/no-deprecated import type {SearchReport, SearchTransaction} from '@src/types/onyx/SearchResults'; import type { Comment, @@ -1095,6 +1096,7 @@ function hasMissingSmartscanFields(transaction: OnyxInputOrEntry, r * Get all transaction violations of the transaction with given transactionID. */ function getTransactionViolations( + // eslint-disable-next-line @typescript-eslint/no-deprecated transaction: OnyxEntry, transactionViolations: OnyxCollection, currentUserEmail: string, @@ -1131,6 +1133,7 @@ function hasPendingRTERViolation(transactionViolations?: TransactionViolations | * Check if there is broken connection violation. */ function hasBrokenConnectionViolation( + // eslint-disable-next-line @typescript-eslint/no-deprecated transaction: Transaction | SearchTransaction, transactionViolations: OnyxCollection | undefined, currentUserEmail: string, @@ -1269,6 +1272,7 @@ function shouldShowViolation( * Check if there is pending rter violation in all transactionViolations with given transactionIDs. */ function allHavePendingRTERViolation( + // eslint-disable-next-line @typescript-eslint/no-deprecated transactions: OnyxEntry, transactionViolations: OnyxCollection | undefined, currentUserEmail: string, @@ -1302,6 +1306,7 @@ function checkIfShouldShowMarkAsCashButton(hasRTERPendingViolation: boolean, sho * Check if there is any transaction without RTER violation within the given transactionIDs. */ function hasAnyTransactionWithoutRTERViolation( + // eslint-disable-next-line @typescript-eslint/no-deprecated transactions: Transaction[] | SearchTransaction[], transactionViolations: OnyxCollection | undefined, currentUserEmail: string, @@ -1509,7 +1514,13 @@ function hasViolation( ); } -function hasDuplicateTransactions(currentUserEmail: string, iouReport: OnyxEntry, policy: OnyxEntry, allReportTransactions?: SearchTransaction[]): boolean { +function hasDuplicateTransactions( + currentUserEmail: string, + iouReport: OnyxEntry, + policy: OnyxEntry, + // eslint-disable-next-line @typescript-eslint/no-deprecated + allReportTransactions?: SearchTransaction[], +): boolean { const transactionsByIouReportID = getReportTransactions(iouReport?.reportID); const reportTransactions = allReportTransactions ?? transactionsByIouReportID;