diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 9410d809e55f..70c65d1d66ce 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -144,7 +144,10 @@ function MoneyRequestView({ const shouldShowBillable = isPolicyExpenseChat && (!!transactionBillable || !(policy?.disabledFields?.defaultBillable ?? true)); const {getViolationsForField} = useViolations(transactionViolations ?? []); - const hasViolations = useCallback((field: ViolationField): boolean => !!canUseViolations && getViolationsForField(field).length > 0, [canUseViolations, getViolationsForField]); + const hasViolations = useCallback( + (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']): boolean => !!canUseViolations && getViolationsForField(field, data).length > 0, + [canUseViolations, getViolationsForField], + ); let amountDescription = `${translate('iou.amount')}`; @@ -197,7 +200,7 @@ function MoneyRequestView({ const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction; const getErrorForField = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations = true) => { + (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']) => { // Checks applied when creating a new money request // NOTE: receipt field can return multiple violations, so we need to handle it separately const fieldChecks: Partial> = { @@ -223,9 +226,8 @@ function MoneyRequestView({ } // Return violations if there are any - // At the moment, we only return violations for tags for workspaces with single-level tags - if (canUseViolations && shouldShowViolations && hasViolations(field)) { - const violations = getViolationsForField(field); + if (canUseViolations && hasViolations(field, data)) { + const violations = getViolationsForField(field, data); return ViolationsUtils.getViolationTranslation(violations[0], translate); } @@ -395,8 +397,8 @@ function MoneyRequestView({ ROUTES.MONEY_REQUEST_STEP_TAG.getRoute(CONST.IOU.ACTION.EDIT, CONST.IOU.TYPE.REQUEST, index, transaction?.transactionID ?? '', report.reportID), ) } - brickRoadIndicator={getErrorForField('tag', {}, policyTagLists.length === 1) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} - error={getErrorForField('tag', {}, policyTagLists.length === 1)} + brickRoadIndicator={getErrorForField('tag', {tagListIndex: index, tagListName: name}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + error={getErrorForField('tag', {tagListIndex: index, tagListName: name})} /> ))} diff --git a/src/hooks/useViolations.ts b/src/hooks/useViolations.ts index 29b2dcb86718..3df457f1205a 100644 --- a/src/hooks/useViolations.ts +++ b/src/hooks/useViolations.ts @@ -58,7 +58,34 @@ function useViolations(violations: TransactionViolation[]) { } return violationGroups ?? new Map(); }, [violations]); - const getViolationsForField = useCallback((field: ViolationField) => violationsByField.get(field) ?? [], [violationsByField]); + + const getViolationsForField = useCallback( + (field: ViolationField, data?: TransactionViolation['data']) => { + const currentViolations = violationsByField.get(field) ?? []; + + // someTagLevelsRequired has special logic becase data.errorIndexes is a bit unique in how it denotes the tag list that has the violation + // tagListIndex can be 0 so we compare with undefined + if (currentViolations[0]?.name === 'someTagLevelsRequired' && data?.tagListIndex !== undefined && Array.isArray(currentViolations[0]?.data?.errorIndexes)) { + return currentViolations + .filter((violation) => violation.data?.errorIndexes?.includes(data?.tagListIndex ?? -1)) + .map((violation) => ({ + ...violation, + data: { + ...violation.data, + tagName: data?.tagListName, + }, + })); + } + + // tagOutOfPolicy has special logic because we have to account for multi-level tags and use tagName to find the right tag to put the violation on + if (currentViolations[0]?.name === 'tagOutOfPolicy' && data?.tagListName !== undefined && currentViolations[0]?.data?.tagName) { + return currentViolations.filter((violation) => violation.data?.tagName === data?.tagListName); + } + + return currentViolations; + }, + [violationsByField], + ); return { getViolationsForField, diff --git a/src/languages/en.ts b/src/languages/en.ts index 3575854ee7e2..07cad2eaa4e9 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -2412,7 +2412,7 @@ export default { return ''; }, smartscanFailed: 'Receipt scanning failed. Enter details manually.', - someTagLevelsRequired: 'Missing tag', + someTagLevelsRequired: ({tagName}: ViolationsTagOutOfPolicyParams) => `Missing ${tagName ?? 'Tag'}`, tagOutOfPolicy: ({tagName}: ViolationsTagOutOfPolicyParams) => `${tagName ?? 'Tag'} no longer valid`, taxAmountChanged: 'Tax amount was modified', taxOutOfPolicy: ({taxName}: ViolationsTaxOutOfPolicyParams) => `${taxName ?? 'Tax'} no longer valid`, diff --git a/src/languages/es.ts b/src/languages/es.ts index 51a83e55fee2..a2d18880f518 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -2904,7 +2904,7 @@ export default { return ''; }, smartscanFailed: 'No se pudo escanear el recibo. Introduce los datos manualmente', - someTagLevelsRequired: 'Falta etiqueta', + someTagLevelsRequired: ({tagName}: ViolationsTagOutOfPolicyParams) => `Falta ${tagName ?? 'Tag'}`, tagOutOfPolicy: ({tagName}: ViolationsTagOutOfPolicyParams) => `La etiqueta ${tagName ? `${tagName} ` : ''}ya no es válida`, taxAmountChanged: 'El importe del impuesto fue modificado', taxOutOfPolicy: ({taxName}: ViolationsTaxOutOfPolicyParams) => `${taxName ?? 'El impuesto'} ya no es válido`, diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 6153ea62cd0d..fe2e5af537a7 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -7,6 +7,106 @@ import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PolicyCategories, PolicyTagList, Transaction, TransactionViolation} from '@src/types/onyx'; +/** + * Calculates tag out of policy and missing tag violations for the given transaction + */ +function getTagViolationsForSingleLevelTags( + updatedTransaction: Transaction, + transactionViolations: TransactionViolation[], + policyRequiresTags: boolean, + policyTagList: PolicyTagList, +): TransactionViolation[] { + const policyTagKeys = Object.keys(policyTagList); + const policyTagListName = policyTagKeys[0]; + const policyTags = policyTagList[policyTagListName]?.tags; + const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY); + const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG); + const isTagInPolicy = policyTags ? !!policyTags[updatedTransaction.tag ?? '']?.enabled : false; + let newTransactionViolations = [...transactionViolations]; + + // 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'}); + } + + // Remove 'tagOutOfPolicy' violation if tag is in policy + if (hasTagOutOfPolicyViolation && updatedTransaction.tag && isTagInPolicy) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY}); + } + + // Remove 'missingTag' violation if tag is valid according to policy + if (hasMissingTagViolation && isTagInPolicy) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_TAG}); + } + + // Add 'missingTag violation' if tag is required and not set + if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) { + newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation'}); + } + return newTransactionViolations; +} + +/** + * Calculates some tag levels required and missing tag violations for the given transaction + */ +function getTagViolationsForMultiLevelTags( + updatedTransaction: Transaction, + transactionViolations: TransactionViolation[], + policyRequiresTags: boolean, + policyTagList: PolicyTagList, +): TransactionViolation[] { + const policyTagKeys = Object.keys(policyTagList); + const selectedTags = updatedTransaction.tag?.split(CONST.COLON) ?? []; + let newTransactionViolations = [...transactionViolations]; + newTransactionViolations = newTransactionViolations.filter( + (violation) => violation.name !== CONST.VIOLATIONS.SOME_TAG_LEVELS_REQUIRED && violation.name !== CONST.VIOLATIONS.TAG_OUT_OF_POLICY, + ); + + // We first get the errorIndexes for someTagLevelsRequired. If it's not empty, we puth SOME_TAG_LEVELS_REQUIRED in Onyx. + // Otherwise, we put TAG_OUT_OF_POLICY in Onyx (when applicable) + const errorIndexes = []; + for (let i = 0; i < policyTagKeys.length; i++) { + const isTagRequired = policyTagList[policyTagKeys[i]].required ?? true; + const isTagSelected = Boolean(selectedTags[i]); + if (isTagRequired && (!isTagSelected || (selectedTags.length === 1 && selectedTags[0] === ''))) { + errorIndexes.push(i); + } + } + if (errorIndexes.length !== 0) { + newTransactionViolations.push({ + name: CONST.VIOLATIONS.SOME_TAG_LEVELS_REQUIRED, + type: 'violation', + data: { + errorIndexes, + }, + }); + } else { + let hasInvalidTag = false; + for (let i = 0; i < policyTagKeys.length; i++) { + const selectedTag = selectedTags[i]; + const tags = policyTagList[policyTagKeys[i]].tags; + const isTagInPolicy = Object.values(tags).some((tag) => tag.name === selectedTag && Boolean(tag.enabled)); + if (!isTagInPolicy) { + newTransactionViolations.push({ + name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, + type: 'violation', + data: { + tagName: policyTagKeys[i], + }, + }); + hasInvalidTag = true; + break; + } + } + if (!hasInvalidTag) { + newTransactionViolations = reject(newTransactionViolations, { + name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, + }); + } + } + return newTransactionViolations; +} + const ViolationsUtils = { /** * Checks a transaction for policy violations and returns an object with Onyx method, key and updated transaction @@ -22,6 +122,7 @@ const ViolationsUtils = { ): OnyxUpdate { let newTransactionViolations = [...transactionViolations]; + // Calculate client-side category violations if (policyRequiresCategories) { const hasCategoryOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === 'categoryOutOfPolicy'); const hasMissingCategoryViolation = transactionViolations.some((violation) => violation.name === 'missingCategory'); @@ -49,36 +150,12 @@ const ViolationsUtils = { } } + // Calculate client-side tag violations if (policyRequiresTags) { - const policyTagKeys = Object.keys(policyTagList); - - // At the moment, we only return violations for tags for workspaces with single-level tags - if (policyTagKeys.length === 1) { - const policyTagListName = policyTagKeys[0]; - const policyTags = policyTagList[policyTagListName]?.tags; - const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY); - const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG); - const isTagInPolicy = policyTags ? !!policyTags[updatedTransaction.tag ?? '']?.enabled : false; - - // 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'}); - } - - // Remove 'tagOutOfPolicy' violation if tag is in policy - if (hasTagOutOfPolicyViolation && updatedTransaction.tag && isTagInPolicy) { - newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY}); - } - - // Remove 'missingTag' violation if tag is valid according to policy - if (hasMissingTagViolation && isTagInPolicy) { - newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_TAG}); - } - // 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 = + Object.keys(policyTagList).length === 1 + ? getTagViolationsForSingleLevelTags(updatedTransaction, newTransactionViolations, policyRequiresTags, policyTagList) + : getTagViolationsForMultiLevelTags(updatedTransaction, newTransactionViolations, policyRequiresTags, policyTagList); } return { @@ -181,7 +258,7 @@ const ViolationsUtils = { case 'smartscanFailed': return translate('violations.smartscanFailed'); case 'someTagLevelsRequired': - return translate('violations.someTagLevelsRequired'); + return translate('violations.someTagLevelsRequired', {tagName}); case 'tagOutOfPolicy': return translate('violations.tagOutOfPolicy', {tagName}); case 'taxAmountChanged': diff --git a/src/types/onyx/TransactionViolation.ts b/src/types/onyx/TransactionViolation.ts index 9133eca63c65..28de4582bd5e 100644 --- a/src/types/onyx/TransactionViolation.ts +++ b/src/types/onyx/TransactionViolation.ts @@ -26,6 +26,9 @@ type TransactionViolation = { isTransactionOlderThan7Days?: boolean; member?: string; taxName?: string; + tagListIndex?: number; + tagListName?: string; + errorIndexes?: number[]; }; }; diff --git a/tests/unit/ViolationUtilsTest.js b/tests/unit/ViolationUtilsTest.js index ff86b5fc6753..15a3a4f7de07 100644 --- a/tests/unit/ViolationUtilsTest.js +++ b/tests/unit/ViolationUtilsTest.js @@ -132,11 +132,6 @@ describe('getViolationsOnyxData', () => { Lunch: {name: 'Lunch', enabled: true}, Dinner: {name: 'Dinner', enabled: true}, }, - Tag: { - name: 'Tag', - required: true, - tags: {Lunch: {enabled: true}, Dinner: {enabled: true}}, - }, }, }; transaction.tag = 'Lunch'; @@ -201,4 +196,77 @@ describe('getViolationsOnyxData', () => { expect(result.value).not.toContainEqual([missingTagViolation]); }); }); + describe('policy has multi level tags', () => { + beforeEach(() => { + policyRequiresTags = true; + policyTags = { + Department: { + name: 'Department', + tags: { + Accounting: { + name: 'Accounting', + enabled: true, + }, + }, + required: true, + }, + Region: { + name: 'Region', + tags: { + Africa: { + name: 'Africa', + enabled: true, + }, + }, + }, + Project: { + name: 'Project', + tags: { + Project1: { + name: 'Project1', + enabled: true, + }, + }, + required: true, + }, + }; + }); + it('should return someTagLevelsRequired when a required tag is missing', () => { + const someTagLevelsRequiredViolation = { + name: 'someTagLevelsRequired', + type: 'violation', + data: { + errorIndexes: [0, 1, 2], + }, + }; + + // Test case where transaction has no tags + let result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); + expect(result.value).toEqual([someTagLevelsRequiredViolation]); + + // Test case where transaction has 1 tag + transaction.tag = 'Accounting'; + someTagLevelsRequiredViolation.data = {errorIndexes: [1, 2]}; + result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); + expect(result.value).toEqual([someTagLevelsRequiredViolation]); + + // Test case where transaction has 2 tags + transaction.tag = 'Accounting::Project1'; + someTagLevelsRequiredViolation.data = {errorIndexes: [1]}; + result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); + expect(result.value).toEqual([someTagLevelsRequiredViolation]); + + // Test case where transaction has all tags + transaction.tag = 'Accounting:Africa:Project1'; + result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); + expect(result.value).toEqual([]); + }); + it('should return tagOutOfPolicy when a tag is not enabled in the policy but is set in the transaction', () => { + policyTags.Department.tags.Accounting.enabled = false; + transaction.tag = 'Accounting:Africa:Project1'; + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); + const violation = {...tagOutOfPolicyViolation, data: {tagName: 'Department'}}; + expect(result.value).toEqual([violation]); + }); + }); });