Skip to content

Commit 8d5627f

Browse files
inimagainimaga
authored
Merge pull request #89482 from Expensify/claude-preserveShowInReviewOnMergedProhibitedViolation
Preserve showInReview on merged prohibited expense violation
2 parents d8bbee9 + f814ccc commit 8d5627f

3 files changed

Lines changed: 99 additions & 1 deletion

File tree

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1669,6 +1669,7 @@ function mergeProhibitedViolations(transactionViolations: TransactionViolations)
16691669
prohibitedExpenseRule: prohibitedExpenses,
16701670
},
16711671
type: CONST.VIOLATION_TYPES.VIOLATION,
1672+
showInReview: prohibitedViolations.some((v) => v.showInReview),
16721673
};
16731674

16741675
return [...transactionViolations.filter((violation: TransactionViolation) => violation.name !== CONST.VIOLATIONS.PROHIBITED_EXPENSE), mergedProhibitedViolations];

‎tests/unit/TransactionUtilsTest.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
88
import type {Attendee} from '@src/types/onyx/IOU';
99
import * as TransactionUtils from '../../src/libs/TransactionUtils';
1010
import type {Card, Policy, Report, Transaction} from '../../src/types/onyx';
11+
import type {TransactionViolation} from '../../src/types/onyx/TransactionViolation';
1112
import createRandomPolicy, {createCategoryTaxExpenseRules} from '../utils/collections/policies';
1213
import {createRandomReport} from '../utils/collections/reports';
1314
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
@@ -3205,6 +3206,95 @@ describe('TransactionUtils', () => {
32053206
});
32063207
});
32073208

3209+
describe('mergeProhibitedViolations', () => {
3210+
it('should preserve showInReview as true when at least one source violation has showInReview: true', () => {
3211+
const violations: TransactionViolation[] = [
3212+
{
3213+
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
3214+
type: CONST.VIOLATION_TYPES.VIOLATION,
3215+
data: {prohibitedExpenseRule: 'alcohol'},
3216+
showInReview: true,
3217+
},
3218+
{
3219+
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
3220+
type: CONST.VIOLATION_TYPES.VIOLATION,
3221+
data: {prohibitedExpenseRule: 'gambling'},
3222+
showInReview: false,
3223+
},
3224+
{
3225+
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
3226+
type: CONST.VIOLATION_TYPES.VIOLATION,
3227+
data: {prohibitedExpenseRule: 'tobacco'},
3228+
},
3229+
];
3230+
3231+
const result = TransactionUtils.mergeProhibitedViolations(violations);
3232+
3233+
expect(result).toHaveLength(1);
3234+
expect(result.at(0)?.name).toBe(CONST.VIOLATIONS.PROHIBITED_EXPENSE);
3235+
expect(result.at(0)?.data?.prohibitedExpenseRule).toEqual(['alcohol', 'gambling', 'tobacco']);
3236+
expect(result.at(0)?.showInReview).toBe(true);
3237+
});
3238+
3239+
it('should set showInReview to false when no source violation has showInReview: true', () => {
3240+
const violations: TransactionViolation[] = [
3241+
{
3242+
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
3243+
type: CONST.VIOLATION_TYPES.VIOLATION,
3244+
data: {prohibitedExpenseRule: 'alcohol'},
3245+
showInReview: false,
3246+
},
3247+
{
3248+
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
3249+
type: CONST.VIOLATION_TYPES.VIOLATION,
3250+
data: {prohibitedExpenseRule: 'gambling'},
3251+
},
3252+
];
3253+
3254+
const result = TransactionUtils.mergeProhibitedViolations(violations);
3255+
3256+
expect(result).toHaveLength(1);
3257+
expect(result.at(0)?.showInReview).toBeFalsy();
3258+
});
3259+
3260+
it('should not modify non-prohibited violations', () => {
3261+
const violations: TransactionViolation[] = [
3262+
{
3263+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
3264+
type: CONST.VIOLATION_TYPES.VIOLATION,
3265+
},
3266+
{
3267+
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
3268+
type: CONST.VIOLATION_TYPES.VIOLATION,
3269+
data: {prohibitedExpenseRule: 'alcohol'},
3270+
showInReview: true,
3271+
},
3272+
];
3273+
3274+
const result = TransactionUtils.mergeProhibitedViolations(violations);
3275+
3276+
expect(result).toHaveLength(2);
3277+
const duplicateViolation = result.find((v) => v.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION);
3278+
const prohibitedViolation = result.find((v) => v.name === CONST.VIOLATIONS.PROHIBITED_EXPENSE);
3279+
expect(duplicateViolation).toBeDefined();
3280+
expect(prohibitedViolation?.showInReview).toBe(true);
3281+
expect(prohibitedViolation?.data?.prohibitedExpenseRule).toEqual(['alcohol']);
3282+
});
3283+
3284+
it('should return violations unchanged when there are no prohibited violations', () => {
3285+
const violations: TransactionViolation[] = [
3286+
{
3287+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
3288+
type: CONST.VIOLATION_TYPES.VIOLATION,
3289+
},
3290+
];
3291+
3292+
const result = TransactionUtils.mergeProhibitedViolations(violations);
3293+
3294+
expect(result).toEqual(violations);
3295+
});
3296+
});
3297+
32083298
describe('shouldClearConvertedAmount', () => {
32093299
it('returns false when destinationCurrency is undefined', () => {
32103300
const transaction = generateTransaction({currency: 'USD'});

‎tests/unit/useTransactionViolationsTest.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jest.mock('@libs/TransactionUtils', () => {
1414
return {
1515
isViolationDismissed: jest.fn(),
1616
shouldShowViolation: jest.fn(),
17-
mergeProhibitedViolations: (transactionViolations: Array<{name: string; type: string; data?: {prohibitedExpenseRule?: string | string[]}}>) => {
17+
mergeProhibitedViolations: (transactionViolations: Array<{name: string; type: string; showInReview?: boolean; data?: {prohibitedExpenseRule?: string | string[]}}>) => {
1818
const prohibitedViolations = transactionViolations.filter((violation) => violation.name === CONST_MOCK.VIOLATIONS.PROHIBITED_EXPENSE);
1919

2020
if (prohibitedViolations.length === 0) {
@@ -35,6 +35,7 @@ jest.mock('@libs/TransactionUtils', () => {
3535
prohibitedExpenseRule: prohibitedExpenses,
3636
},
3737
type: CONST_MOCK.VIOLATION_TYPES.VIOLATION,
38+
showInReview: prohibitedViolations.some((v) => v.showInReview),
3839
};
3940

4041
return [...transactionViolations.filter((violation) => violation.name !== CONST_MOCK.VIOLATIONS.PROHIBITED_EXPENSE), mergedProhibitedViolations];
@@ -103,6 +104,7 @@ describe('useTransactionViolations', () => {
103104
data: {
104105
prohibitedExpenseRule: 'alcohol',
105106
},
107+
showInReview: true,
106108
},
107109
];
108110

@@ -119,6 +121,7 @@ describe('useTransactionViolations', () => {
119121
expect(result.current.at(0)?.name).toBe(CONST.VIOLATIONS.PROHIBITED_EXPENSE);
120122
expect(result.current.at(0)?.data?.prohibitedExpenseRule).toEqual(['alcohol']);
121123
expect(result.current.at(0)?.type).toBe(CONST.VIOLATION_TYPES.VIOLATION);
124+
expect(result.current.at(0)?.showInReview).toBe(true);
122125
});
123126

124127
it('should merge multiple prohibited violations into one', async () => {
@@ -130,20 +133,23 @@ describe('useTransactionViolations', () => {
130133
data: {
131134
prohibitedExpenseRule: 'alcohol',
132135
},
136+
showInReview: true,
133137
},
134138
{
135139
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
136140
type: CONST.VIOLATION_TYPES.VIOLATION,
137141
data: {
138142
prohibitedExpenseRule: 'gambling',
139143
},
144+
showInReview: true,
140145
},
141146
{
142147
name: CONST.VIOLATIONS.PROHIBITED_EXPENSE,
143148
type: CONST.VIOLATION_TYPES.VIOLATION,
144149
data: {
145150
prohibitedExpenseRule: 'tobacco',
146151
},
152+
showInReview: true,
147153
},
148154
];
149155

@@ -160,6 +166,7 @@ describe('useTransactionViolations', () => {
160166
expect(result.current.at(0)?.name).toBe(CONST.VIOLATIONS.PROHIBITED_EXPENSE);
161167
expect(result.current.at(0)?.data?.prohibitedExpenseRule).toEqual(['alcohol', 'gambling', 'tobacco']);
162168
expect(result.current.at(0)?.type).toBe(CONST.VIOLATION_TYPES.VIOLATION);
169+
expect(result.current.at(0)?.showInReview).toBe(true);
163170
});
164171

165172
it('should handle empty prohibitedExpenseRule arrays', async () => {

0 commit comments

Comments
 (0)