From 57751f121c5cea270c5d543799f5b599f24f7841 Mon Sep 17 00:00:00 2001 From: Krishna Gupta Date: Sat, 30 Mar 2024 03:08:22 +0530 Subject: [PATCH 1/2] improvement: Use Const for 'violation' everywhere we check for violation type. Signed-off-by: Krishna Gupta --- src/CONST.ts | 10 +++++- .../MoneyRequestPreviewContent.tsx | 2 +- src/hooks/useViolations.ts | 3 +- src/libs/TransactionUtils.ts | 4 ++- src/libs/Violations/ViolationsUtils.ts | 12 +++---- tests/unit/ViolationUtilsTest.ts | 31 ++++++++++--------- 6 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 0fa1c64be44d..1db67a818b84 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -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. */ diff --git a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx index 2e9f9a553b71..2c6f14cec4c2 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx @@ -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; diff --git a/src/hooks/useViolations.ts b/src/hooks/useViolations.ts index 3df457f1205a..7aadb4ce4ca3 100644 --- a/src/hooks/useViolations.ts +++ b/src/hooks/useViolations.ts @@ -1,4 +1,5 @@ import {useCallback, useMemo} from 'react'; +import CONST from '@src/CONST'; import type {TransactionViolation, ViolationName} from '@src/types/onyx'; /** @@ -49,7 +50,7 @@ type ViolationsMap = Map; 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(); for (const violation of filteredViolations) { const field = violationFields[violation.name]; diff --git a/src/libs/TransactionUtils.ts b/src/libs/TransactionUtils.ts index 907edc208570..cdd23aac1596 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -562,7 +562,9 @@ function isOnHold(transaction: OnyxEntry): boolean { * Checks if any violations for the provided transaction are of type 'violation' */ function hasViolation(transactionID: string, transactionViolations: OnyxCollection): 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[] | null { diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index fe2e5af537a7..f2aa53e55443 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -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 @@ -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; } @@ -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, }, @@ -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], }, @@ -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 @@ -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}); } } diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 354a90802077..6c45fea0058a 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -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', () => { @@ -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)); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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], }, From 0887c096514b6b13e36ec9ed5e45b71382f199ff Mon Sep 17 00:00:00 2001 From: Krishna Gupta Date: Sat, 30 Mar 2024 04:20:54 +0530 Subject: [PATCH 2/2] add warning violation type. Signed-off-by: Krishna Gupta --- src/CONST.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CONST.ts b/src/CONST.ts index 1db67a818b84..290a9df75621 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -3337,6 +3337,7 @@ const CONST = { VIOLATION_TYPES: { VIOLATION: 'violation', NOTICE: 'notice', + WARNING: 'warning', }, /**