diff --git a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx index 8503ba9afa0..d768fe8e5d9 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx @@ -86,7 +86,7 @@ function MoneyRequestPreviewContent({ const requestMerchant = truncate(merchant, {length: CONST.REQUEST_PREVIEW.MAX_LENGTH}); const hasReceipt = TransactionUtils.hasReceipt(transaction); const isScanning = hasReceipt && TransactionUtils.isReceiptBeingScanned(transaction); - const hasViolations = TransactionUtils.hasViolation(transaction, transactionViolations); + const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '', transactionViolations); const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction); const shouldShowRBR = hasViolations || hasFieldErrors; const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); @@ -152,13 +152,13 @@ function MoneyRequestPreviewContent({ let message = translate('iou.cash'); if (hasViolations && transaction) { - const violations = TransactionUtils.getTransactionViolations(transaction, transactionViolations); + const violations = TransactionUtils.getTransactionViolations(transaction.transactionID, transactionViolations); if (violations?.[0]) { const violationMessage = ViolationsUtils.getViolationTranslation(violations[0], translate); const isTooLong = violations.filter((v) => v.type === 'violation').length > 1 || violationMessage.length > 15; message += ` • ${isTooLong ? translate('violations.reviewRequired') : violationMessage}`; } - } else if (ReportUtils.isPaidGroupPolicyExpenseReport(iouReport) && ReportUtils.isReportApproved(iouReport) && !ReportUtils.isSettled(iouReport)) { + } else if (ReportUtils.isPaidGroupPolicyExpenseReport(iouReport) && ReportUtils.isReportApproved(iouReport) && !ReportUtils.isSettled(iouReport?.reportID)) { message += ` • ${translate('iou.approved')}`; } else if (iouReport?.isWaitingOnBankAccount) { message += ` • ${translate('iou.pending')}`; diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index db4d4bd2e95..5e869ac15e1 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -145,10 +145,7 @@ function MoneyRequestView({ const shouldShowBillable = isPolicyExpenseChat && (!!transactionBillable || !(policy?.disabledFields?.defaultBillable ?? true)); const {getViolationsForField} = useViolations(transactionViolations ?? []); - const hasViolations = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']): boolean => !!canUseViolations && getViolationsForField(field, data).length > 0, - [canUseViolations, getViolationsForField], - ); + const hasViolations = useCallback((field: ViolationField): boolean => !!canUseViolations && getViolationsForField(field).length > 0, [canUseViolations, getViolationsForField]); let amountDescription = `${translate('iou.amount')}`; @@ -202,7 +199,7 @@ function MoneyRequestView({ const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction; const getErrorForField = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']) => { + (field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations = true) => { // 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> = { @@ -228,8 +225,9 @@ function MoneyRequestView({ } // Return violations if there are any - if (canUseViolations && hasViolations(field, data)) { - const violations = getViolationsForField(field, data); + // At the moment, we only return violations for tags for workspaces with single-level tags + if (canUseViolations && shouldShowViolations && hasViolations(field)) { + const violations = getViolationsForField(field); return ViolationsUtils.getViolationTranslation(violations[0], translate); } @@ -400,16 +398,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', { - tagName: name, - }) - ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR - : undefined - } - error={getErrorForField('tag', { - tagName: name, - })} + brickRoadIndicator={getErrorForField('tag', {}, policyTagLists.length === 1) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + error={getErrorForField('tag', {}, policyTagLists.length === 1)} /> ))} diff --git a/src/hooks/useViolations.ts b/src/hooks/useViolations.ts index ea825b45bc0..29b2dcb8671 100644 --- a/src/hooks/useViolations.ts +++ b/src/hooks/useViolations.ts @@ -58,19 +58,7 @@ function useViolations(violations: TransactionViolation[]) { } return violationGroups ?? new Map(); }, [violations]); - - const getViolationsForField = useCallback( - (field: ViolationField, data?: TransactionViolation['data']) => { - const currentViolations = violationsByField.get(field) ?? []; - - if (data?.tagName) { - return currentViolations.filter((violation) => violation.data?.tagName === data.tagName); - } - - return currentViolations; - }, - [violationsByField], - ); + const getViolationsForField = useCallback((field: ViolationField) => violationsByField.get(field) ?? [], [violationsByField]); return { getViolationsForField, diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index e0c474e3d8f..c28d9461ce7 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -722,12 +722,10 @@ function hasParticipantInArray(report: Report, policyMemberAccountIDs: number[]) /** * Whether the Money Request report is settled */ -function isSettled(reportOrID: Report | OnyxEntry | string | undefined): boolean { - if (!allReports || !reportOrID) { +function isSettled(reportID: string | undefined): boolean { + if (!allReports || !reportID) { return false; } - const reportID = typeof reportOrID === 'string' ? reportOrID : reportOrID?.reportID; - const report: Report | EmptyObject = allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? {}; if (isEmptyObject(report) || report.isWaitingOnBankAccount) { return false; diff --git a/src/libs/TransactionUtils.ts b/src/libs/TransactionUtils.ts index 8be6d1c4142..ee0ec6d9755 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -587,20 +587,11 @@ function isOnHold(transaction: OnyxEntry): boolean { /** * Checks if any violations for the provided transaction are of type 'violation' */ -function hasViolation(transactionOrID: Transaction | OnyxEntry | string, transactionViolations: OnyxCollection): boolean { - if (!transactionOrID) { - return false; - } - const transactionID = typeof transactionOrID === 'string' ? transactionOrID : transactionOrID.transactionID; +function hasViolation(transactionID: string, transactionViolations: OnyxCollection): boolean { return Boolean(transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === 'violation')); } -function getTransactionViolations(transactionOrID: OnyxEntry | string, transactionViolations: OnyxCollection): TransactionViolation[] | null { - if (!transactionOrID) { - return null; - } - const transactionID = typeof transactionOrID === 'string' ? transactionOrID : transactionOrID.transactionID; - +function getTransactionViolations(transactionID: string, transactionViolations: OnyxCollection): TransactionViolation[] | null { return transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID] ?? null; } diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 16b42854915..6153ea62cd0 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -2,7 +2,6 @@ import reject from 'lodash/reject'; import Onyx from 'react-native-onyx'; import type {OnyxUpdate} from 'react-native-onyx'; import type {Phrase, PhraseParameters} from '@libs/Localize'; -import * as TransactionUtils from '@libs/TransactionUtils'; import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -31,7 +30,7 @@ const ViolationsUtils = { // Add 'categoryOutOfPolicy' violation if category is not in policy if (!hasCategoryOutOfPolicyViolation && categoryKey && !isCategoryInPolicy) { - newTransactionViolations.push({name: 'categoryOutOfPolicy', type: 'violation', userMessage: ''}); + newTransactionViolations.push({name: 'categoryOutOfPolicy', type: 'violation'}); } // Remove 'categoryOutOfPolicy' violation if category is in policy @@ -46,72 +45,40 @@ const ViolationsUtils = { // Add 'missingCategory' violation if category is required and not set if (!hasMissingCategoryViolation && policyRequiresCategories && !categoryKey) { - newTransactionViolations.push({name: 'missingCategory', type: 'violation', userMessage: ''}); + newTransactionViolations.push({name: 'missingCategory', type: 'violation'}); } } if (policyRequiresTags) { - const selectedTags = TransactionUtils.getTagArrayFromName(updatedTransaction.tag ?? '') ?? []; const policyTagKeys = Object.keys(policyTagList); - if (policyTagKeys.length === 0) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - type: 'violation', - userMessage: '', - }); - } - - policyTagKeys.forEach((key, index) => { - const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY && violation.data?.tagName === key); - const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG && violation.data?.tagName === key); - const selectedTag = selectedTags[index]; - const isTagInPolicy = Boolean(policyTagList[key]?.tags[selectedTag]?.enabled); + // 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 && selectedTag && !isTagInPolicy) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - type: 'violation', - userMessage: '', - data: { - tagName: key, - }, - }); + 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 && selectedTag && isTagInPolicy) { - newTransactionViolations = reject(newTransactionViolations, { - name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - data: { - tagName: key, - }, - }); + 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, - data: { - tagName: key, - }, - }); + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_TAG}); } - // Add 'missingTag violation' if tag is required and not set - if (!hasMissingTagViolation && !selectedTag && policyRequiresTags) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.MISSING_TAG, - type: 'violation', - userMessage: '', - data: { - tagName: key, - }, - }); + if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) { + newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation'}); } - }); + } } return { diff --git a/src/types/onyx/TransactionViolation.ts b/src/types/onyx/TransactionViolation.ts index b1764b4aeb8..9133eca63c6 100644 --- a/src/types/onyx/TransactionViolation.ts +++ b/src/types/onyx/TransactionViolation.ts @@ -9,7 +9,6 @@ type ViolationName = (typeof CONST.VIOLATIONS)[keyof typeof CONST.VIOLATIONS]; type TransactionViolation = { type: string; name: ViolationName; - userMessage: string; data?: { rejectedBy?: string; rejectReason?: string; diff --git a/tests/unit/ViolationUtilsTest.js b/tests/unit/ViolationUtilsTest.js index 4f03fe0a42f..ff86b5fc675 100644 --- a/tests/unit/ViolationUtilsTest.js +++ b/tests/unit/ViolationUtilsTest.js @@ -6,25 +6,21 @@ import ONYXKEYS from '@src/ONYXKEYS'; const categoryOutOfPolicyViolation = { name: 'categoryOutOfPolicy', type: 'violation', - userMessage: '', }; const missingCategoryViolation = { name: 'missingCategory', type: 'violation', - userMessage: '', }; const tagOutOfPolicyViolation = { name: 'tagOutOfPolicy', type: 'violation', - userMessage: '', }; const missingTagViolation = { name: 'missingTag', type: 'violation', - userMessage: '', }; describe('getViolationsOnyxData', () => { @@ -56,8 +52,8 @@ describe('getViolationsOnyxData', () => { it('should handle multiple violations', () => { transactionViolations = [ - {name: 'duplicatedTransaction', type: 'violation', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); expect(result.value).toEqual(expect.arrayContaining(transactionViolations)); @@ -90,8 +86,8 @@ describe('getViolationsOnyxData', () => { it('should add categoryOutOfPolicy violation to existing violations if they exist', () => { transaction.category = 'Bananas'; transactionViolations = [ - {name: 'duplicatedTransaction', type: 'violation', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); @@ -102,8 +98,8 @@ describe('getViolationsOnyxData', () => { it('should add missingCategory violation to existing violations if they exist', () => { transaction.category = undefined; transactionViolations = [ - {name: 'duplicatedTransaction', type: 'violation', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); @@ -157,7 +153,7 @@ describe('getViolationsOnyxData', () => { const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); - expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation, data: {tagName: 'Meals'}}])); + expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation}])); }); it('should add a tagOutOfPolicy violation when policy requires tags and tag is not in the policy', () => { @@ -165,31 +161,31 @@ describe('getViolationsOnyxData', () => { const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); - expect(result.value).toEqual(expect.arrayContaining([tagOutOfPolicyViolation])); + expect(result.value).toEqual([]); }); 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', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); - expect(result.value).toEqual(expect.arrayContaining([{...tagOutOfPolicyViolation, data: {tagName: 'Meals'}}, ...transactionViolations])); + expect(result.value).toEqual(expect.arrayContaining([{...tagOutOfPolicyViolation}, ...transactionViolations])); }); it('should add missingTag violation to existing violations if transaction does not have a tag', () => { transaction.tag = undefined; transactionViolations = [ - {name: 'duplicatedTransaction', type: 'violation', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); - expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation, data: {tagName: 'Meals'}}, ...transactionViolations])); + expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation}, ...transactionViolations])); }); });