Skip to content

Commit

Permalink
improvement: Use Const for 'violation' everywhere we check for violat…
Browse files Browse the repository at this point in the history
…ion type.

Signed-off-by: Krishna Gupta <[email protected]>
  • Loading branch information
Krishna2323 committed Mar 29, 2024
1 parent 336b0bd commit 57751f1
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 25 deletions.
10 changes: 9 additions & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3332,7 +3332,15 @@ const CONST = {
},

/**
* Constants for types of violations.
* Constants for types of violation.
*/
VIOLATION_TYPES: {
VIOLATION: 'violation',
NOTICE: 'notice',
},

/**
* Constants for types of violation names.
* Defined here because they need to be referenced by the type system to generate the
* ViolationNames type.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ function MoneyRequestPreviewContent({
const violations = TransactionUtils.getTransactionViolations(transaction.transactionID, transactionViolations);
if (violations?.[0]) {
const violationMessage = ViolationsUtils.getViolationTranslation(violations[0], translate);
const violationsCount = violations.filter((v) => v.type === 'violation').length;
const violationsCount = violations.filter((v) => v.type === CONST.VIOLATION_TYPES.VIOLATION).length;
const isTooLong = violationsCount > 1 || violationMessage.length > 15;
const hasViolationsAndFieldErrors = violationsCount > 0 && hasFieldErrors;

Expand Down
3 changes: 2 additions & 1 deletion src/hooks/useViolations.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {useCallback, useMemo} from 'react';
import CONST from '@src/CONST';
import type {TransactionViolation, ViolationName} from '@src/types/onyx';

/**
Expand Down Expand Up @@ -49,7 +50,7 @@ type ViolationsMap = Map<ViolationField, TransactionViolation[]>;

function useViolations(violations: TransactionViolation[]) {
const violationsByField = useMemo((): ViolationsMap => {
const filteredViolations = violations.filter((violation) => violation.type === 'violation');
const filteredViolations = violations.filter((violation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION);
const violationGroups = new Map<ViolationField, TransactionViolation[]>();
for (const violation of filteredViolations) {
const field = violationFields[violation.name];
Expand Down
4 changes: 3 additions & 1 deletion src/libs/TransactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,9 @@ function isOnHold(transaction: OnyxEntry<Transaction>): boolean {
* Checks if any violations for the provided transaction are of type 'violation'
*/
function hasViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
return Boolean(transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === 'violation'));
return Boolean(
transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION),
);
}

function getTransactionViolations(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): TransactionViolation[] | null {
Expand Down
12 changes: 6 additions & 6 deletions src/libs/Violations/ViolationsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function getTagViolationsForSingleLevelTags(

// Add 'tagOutOfPolicy' violation if tag is not in policy
if (!hasTagOutOfPolicyViolation && updatedTransaction.tag && !isTagInPolicy) {
newTransactionViolations.push({name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: 'violation'});
newTransactionViolations.push({name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION});
}

// Remove 'tagOutOfPolicy' violation if tag is in policy
Expand All @@ -41,7 +41,7 @@ function getTagViolationsForSingleLevelTags(

// Add 'missingTag violation' if tag is required and not set
if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) {
newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation'});
newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: CONST.VIOLATION_TYPES.VIOLATION});
}
return newTransactionViolations;
}
Expand Down Expand Up @@ -75,7 +75,7 @@ function getTagViolationsForMultiLevelTags(
if (errorIndexes.length !== 0) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.SOME_TAG_LEVELS_REQUIRED,
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
data: {
errorIndexes,
},
Expand All @@ -89,7 +89,7 @@ function getTagViolationsForMultiLevelTags(
if (!isTagInPolicy) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
data: {
tagName: policyTagKeys[i],
},
Expand Down Expand Up @@ -131,7 +131,7 @@ const ViolationsUtils = {

// Add 'categoryOutOfPolicy' violation if category is not in policy
if (!hasCategoryOutOfPolicyViolation && categoryKey && !isCategoryInPolicy) {
newTransactionViolations.push({name: 'categoryOutOfPolicy', type: 'violation'});
newTransactionViolations.push({name: 'categoryOutOfPolicy', type: CONST.VIOLATION_TYPES.VIOLATION});
}

// Remove 'categoryOutOfPolicy' violation if category is in policy
Expand All @@ -146,7 +146,7 @@ const ViolationsUtils = {

// Add 'missingCategory' violation if category is required and not set
if (!hasMissingCategoryViolation && policyRequiresCategories && !categoryKey) {
newTransactionViolations.push({name: 'missingCategory', type: 'violation'});
newTransactionViolations.push({name: 'missingCategory', type: CONST.VIOLATION_TYPES.VIOLATION});
}
}

Expand Down
31 changes: 16 additions & 15 deletions tests/unit/ViolationUtilsTest.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
import {beforeEach} from '@jest/globals';
import Onyx from 'react-native-onyx';
import ViolationsUtils from '@libs/Violations/ViolationsUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PolicyCategories, PolicyTagList, Transaction, TransactionViolation} from '@src/types/onyx';

const categoryOutOfPolicyViolation = {
name: 'categoryOutOfPolicy',
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
};

const missingCategoryViolation = {
name: 'missingCategory',
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
};

const tagOutOfPolicyViolation = {
name: 'tagOutOfPolicy',
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
};

const missingTagViolation = {
name: 'missingTag',
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
};

describe('getViolationsOnyxData', () => {
Expand Down Expand Up @@ -53,8 +54,8 @@ describe('getViolationsOnyxData', () => {

it('should handle multiple violations', () => {
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
expect(result.value).toEqual(expect.arrayContaining(transactionViolations));
Expand Down Expand Up @@ -87,8 +88,8 @@ describe('getViolationsOnyxData', () => {
it('should add categoryOutOfPolicy violation to existing violations if they exist', () => {
transaction.category = 'Bananas';
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
Expand All @@ -99,8 +100,8 @@ describe('getViolationsOnyxData', () => {
it('should add missingCategory violation to existing violations if they exist', () => {
transaction.category = undefined;
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
Expand Down Expand Up @@ -164,8 +165,8 @@ describe('getViolationsOnyxData', () => {
it('should add tagOutOfPolicy violation to existing violations if transaction has tag that is not in the policy', () => {
transaction.tag = 'Bananas';
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
Expand All @@ -176,8 +177,8 @@ describe('getViolationsOnyxData', () => {
it('should add missingTag violation to existing violations if transaction does not have a tag', () => {
transaction.tag = undefined;
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
Expand Down Expand Up @@ -240,7 +241,7 @@ describe('getViolationsOnyxData', () => {
it('should return someTagLevelsRequired when a required tag is missing', () => {
const someTagLevelsRequiredViolation = {
name: 'someTagLevelsRequired',
type: 'violation',
type: CONST.VIOLATION_TYPES.VIOLATION,
data: {
errorIndexes: [0, 1, 2],
},
Expand Down

0 comments on commit 57751f1

Please sign in to comment.