Skip to content

Commit

Permalink
Merge pull request #39298 from Krishna2323/krishna2323/improvement/39227
Browse files Browse the repository at this point in the history
improvement: Use Const for 'violation' everywhere we check for violation type.
  • Loading branch information
yuwenmemon authored Apr 2, 2024
2 parents 16525e6 + 0887c09 commit a4d01a8
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 25 deletions.
11 changes: 10 additions & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3335,7 +3335,16 @@ const CONST = {
},

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

/**
* 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 @@ -28,7 +28,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 @@ -43,7 +43,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 @@ -77,7 +77,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 @@ -91,7 +91,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 @@ -142,7 +142,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 @@ -157,7 +157,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
30 changes: 15 additions & 15 deletions tests/unit/ViolationUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,22 @@ import type {PolicyCategories, PolicyTagList, Transaction, TransactionViolation}

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 @@ -54,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 @@ -94,8 +94,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 @@ -106,8 +106,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 @@ -177,8 +177,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 @@ -189,8 +189,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 @@ -253,7 +253,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 a4d01a8

Please sign in to comment.