From fe1f3f0699bba272887b575e370657b56659596a Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Mon, 19 Feb 2024 14:40:36 -0800 Subject: [PATCH 01/23] Cleanup --- .../MoneyRequestPreviewContent.tsx | 6 +++--- src/libs/ReportUtils.ts | 6 ++---- src/libs/TransactionUtils.ts | 13 ++----------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx index e2f7314afd73..41726549b285 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx @@ -85,7 +85,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); @@ -148,13 +148,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/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index d0c3bf3e8c03..ea7009c25465 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -721,12 +721,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) { 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 d3eafc6554db..05bcbaa8f17e 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -550,20 +550,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; } From 45441a4e9884ca95cdbb310d6d530cb342852897 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Mon, 19 Feb 2024 16:26:39 -0800 Subject: [PATCH 02/23] Fix client side violations --- src/hooks/useViolations.ts | 15 +++------------ src/libs/Violations/ViolationsUtils.ts | 6 ------ 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/hooks/useViolations.ts b/src/hooks/useViolations.ts index ea825b45bc0b..e43820281e8a 100644 --- a/src/hooks/useViolations.ts +++ b/src/hooks/useViolations.ts @@ -59,18 +59,9 @@ 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, data?: TransactionViolation['data']) => { + return violationsByField.get(field) ?? []; + }, [violationsByField]); return { getViolationsForField, diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index a1cd001badee..620a9e7b4e84 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -83,9 +83,6 @@ const ViolationsUtils = { if (hasTagOutOfPolicyViolation && selectedTag && isTagInPolicy) { newTransactionViolations = reject(newTransactionViolations, { name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - data: { - tagName: key, - }, }); } @@ -93,9 +90,6 @@ const ViolationsUtils = { if (hasMissingTagViolation && isTagInPolicy) { newTransactionViolations = reject(newTransactionViolations, { name: CONST.VIOLATIONS.MISSING_TAG, - data: { - tagName: key, - }, }); } From 59987c2bfa56db042517594dc57d71b61a72aed5 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Mon, 19 Feb 2024 16:26:54 -0800 Subject: [PATCH 03/23] Fix currency issue --- src/libs/actions/IOU.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index cc85fb92dd66..0f450ac66cf1 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -1140,7 +1140,7 @@ function getUpdateMoneyRequestParams( ? IOUUtils.updateIOUOwnerAndTotal(iouReport, updatedReportAction.actorAccountID ?? -1, diff, TransactionUtils.getCurrency(transaction), false, true) : {}; } - updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.modifiedCurrency); + updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.currency); optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, From 52ac1d9c9f96e59d054752b47a2fcfa48628d2b9 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Mon, 19 Feb 2024 16:34:17 -0800 Subject: [PATCH 04/23] Remove unused userMessage --- src/libs/Violations/ViolationsUtils.ts | 7 ++----- src/types/onyx/TransactionViolation.ts | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 620a9e7b4e84..f4a407d6c8bd 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -30,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 @@ -45,7 +45,7 @@ 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'}); } } @@ -57,7 +57,6 @@ const ViolationsUtils = { newTransactionViolations.push({ name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: 'violation', - userMessage: '', }); } @@ -72,7 +71,6 @@ const ViolationsUtils = { newTransactionViolations.push({ name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: 'violation', - userMessage: '', data: { tagName: key, }, @@ -98,7 +96,6 @@ const ViolationsUtils = { newTransactionViolations.push({ name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation', - userMessage: '', data: { tagName: key, }, diff --git a/src/types/onyx/TransactionViolation.ts b/src/types/onyx/TransactionViolation.ts index b1764b4aeb80..9133eca63c65 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; From 5bd1c45238f609386f17d19eb56b763e407af2e9 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 08:31:42 -0800 Subject: [PATCH 05/23] Lint --- .../MoneyRequestPreview/MoneyRequestPreviewContent.tsx | 2 +- src/hooks/useViolations.ts | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx index 41726549b285..c1a54257df25 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx @@ -85,7 +85,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?.transactionID, transactionViolations); + const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '', transactionViolations); const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction); const shouldShowRBR = hasViolations || hasFieldErrors; const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); diff --git a/src/hooks/useViolations.ts b/src/hooks/useViolations.ts index e43820281e8a..29b2dcb86718 100644 --- a/src/hooks/useViolations.ts +++ b/src/hooks/useViolations.ts @@ -58,10 +58,7 @@ function useViolations(violations: TransactionViolation[]) { } return violationGroups ?? new Map(); }, [violations]); - - const getViolationsForField = useCallback((field: ViolationField, data?: TransactionViolation['data']) => { - return violationsByField.get(field) ?? []; - }, [violationsByField]); + const getViolationsForField = useCallback((field: ViolationField) => violationsByField.get(field) ?? [], [violationsByField]); return { getViolationsForField, From 947feb9678158e368fdee64a319ab0776f3d5d17 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 11:09:05 -0800 Subject: [PATCH 06/23] Lint 2 --- src/components/ReportActionItem/MoneyRequestView.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index d3c86698f910..9fb28384c5eb 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -143,7 +143,7 @@ function MoneyRequestView({ const {getViolationsForField} = useViolations(transactionViolations ?? []); const hasViolations = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']): boolean => !!canUseViolations && getViolationsForField(field, data).length > 0, + (field: ViolationField): boolean => !!canUseViolations && getViolationsForField(field).length > 0, [canUseViolations, getViolationsForField], ); @@ -226,7 +226,7 @@ function MoneyRequestView({ // Return violations if there are any if (canUseViolations && hasViolations(field, data)) { - const violations = getViolationsForField(field, data); + const violations = getViolationsForField(field); return ViolationsUtils.getViolationTranslation(violations[0], translate); } From ff80f9c758cc69d1747d8496b60f213644023ed8 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 11:30:18 -0800 Subject: [PATCH 07/23] Lint 3 --- src/components/ReportActionItem/MoneyRequestView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 9fb28384c5eb..8167ce127da7 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -225,7 +225,7 @@ function MoneyRequestView({ } // Return violations if there are any - if (canUseViolations && hasViolations(field, data)) { + if (canUseViolations && hasViolations(field)) { const violations = getViolationsForField(field); return ViolationsUtils.getViolationTranslation(violations[0], translate); } From f4b38fbe3f5f995cad6f5ef4f9fa4cc9043bbde4 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 11:43:52 -0800 Subject: [PATCH 08/23] Lint 4 --- src/components/ReportActionItem/MoneyRequestView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 8167ce127da7..b9a3fef8e1e9 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -199,7 +199,7 @@ function MoneyRequestView({ const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction; const getErrorForField = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data']) => { + (field: ViolationField) => { // 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> = { From e71da03fc1f0f61374d2947f8fa14dd2eaf7288e Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 11:47:33 -0800 Subject: [PATCH 09/23] Lint --- src/components/ReportActionItem/MoneyRequestView.tsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index b9a3fef8e1e9..9e12ae01bb98 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -389,15 +389,9 @@ function MoneyRequestView({ ) } brickRoadIndicator={ - getErrorForField('tag', { - tagName: name, - }) - ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR - : undefined + getErrorForField('tag') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined } - error={getErrorForField('tag', { - tagName: name, - })} + error={getErrorForField('tag')} /> ))} From dd95657d17cbbabc0f086f70816a6100958d5f1a Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 11:52:48 -0800 Subject: [PATCH 10/23] Prettier --- src/components/ReportActionItem/MoneyRequestView.tsx | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 9e12ae01bb98..dc1cbfeb1b8e 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -142,10 +142,7 @@ 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): boolean => !!canUseViolations && getViolationsForField(field).length > 0, [canUseViolations, getViolationsForField]); let amountDescription = `${translate('iou.amount')}`; @@ -388,9 +385,7 @@ function MoneyRequestView({ ROUTES.MONEY_REQUEST_STEP_TAG.getRoute(CONST.IOU.ACTION.EDIT, CONST.IOU.TYPE.REQUEST, index, transaction?.transactionID ?? '', report.reportID), ) } - brickRoadIndicator={ - getErrorForField('tag') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined - } + brickRoadIndicator={getErrorForField('tag') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} error={getErrorForField('tag')} /> From a99ff4cd44929c8b95e64c758bb72bad51c4573e Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Tue, 20 Feb 2024 11:59:02 -0800 Subject: [PATCH 11/23] Remove userMessage from tests --- tests/unit/ViolationUtilsTest.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/tests/unit/ViolationUtilsTest.js b/tests/unit/ViolationUtilsTest.js index 4f03fe0a42fa..bc9207fb8021 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); @@ -171,8 +167,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', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); @@ -183,8 +179,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', userMessage: ''}, - {name: 'receiptRequired', type: 'violation', userMessage: ''}, + {name: 'duplicatedTransaction', type: 'violation'}, + {name: 'receiptRequired', type: 'violation'}, ]; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories); From 5e95a19b5cad1eff7e09110557c613ee51af4e37 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Wed, 21 Feb 2024 13:28:08 -0800 Subject: [PATCH 12/23] Remove conditions added for multi tags --- src/libs/Violations/ViolationsUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index f4a407d6c8bd..38a30097adc0 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -61,8 +61,8 @@ const ViolationsUtils = { } 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 hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY); + const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG); const selectedTag = selectedTags[index]; const isTagInPolicy = Boolean(policyTagList[key]?.tags[selectedTag]?.enabled); From bc73ef19816feea22f38af3497b477a5081f8781 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Wed, 21 Feb 2024 17:12:41 -0800 Subject: [PATCH 13/23] Fix tag violations for single tags --- .../MoneyRequestPreviewContent.tsx | 2 +- .../ReportActionItem/MoneyRequestView.tsx | 21 +++- src/languages/en.ts | 2 +- src/libs/Violations/ViolationsUtils.ts | 107 +++++++++++------- 4 files changed, 85 insertions(+), 47 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx index c1a54257df25..41726549b285 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx @@ -85,7 +85,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?.transactionID ?? '', transactionViolations); + const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID, transactionViolations); const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction); const shouldShowRBR = hasViolations || hasFieldErrors; const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index dc1cbfeb1b8e..3fb993b6a9e8 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -196,7 +196,7 @@ function MoneyRequestView({ const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction; const getErrorForField = useCallback( - (field: ViolationField) => { + (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> = { @@ -222,9 +222,20 @@ function MoneyRequestView({ } // Return violations if there are any - if (canUseViolations && hasViolations(field)) { + if (canUseViolations && hasViolations(field, data)) { const violations = getViolationsForField(field); - return ViolationsUtils.getViolationTranslation(violations[0], translate); + for (let i = 0; i < violations.length; i++) { + let violation = violations[i]; + if (violation.name === 'someTagLevelsRequired' && violation.data?.errorIndexes?.includes(data.index)) { + violation['data']['tagName'] = data.tagName; + return ViolationsUtils.getViolationTranslation(violation, translate); + } else if (violation.name === 'tagOutOfPolicy' && violation.data?.tagName === data.tagName) { + violation['data']['tagName'] = data.tagName; + return ViolationsUtils.getViolationTranslation(violation, translate); + } else { + return ViolationsUtils.getViolationTranslation(violation, translate); + } + } } return ''; @@ -385,8 +396,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') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} - error={getErrorForField('tag')} + brickRoadIndicator={getErrorForField('tag', {tagName: name, index}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + error={getErrorForField('tag', {tagName: name, index})} /> ))} diff --git a/src/languages/en.ts b/src/languages/en.ts index 0553d6470ddc..f6c389520891 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -2328,7 +2328,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/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 38a30097adc0..cfd5c30d135a 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -50,58 +50,85 @@ const ViolationsUtils = { } if (policyRequiresTags) { - const selectedTags = updatedTransaction.tag?.split(CONST.COLON) ?? []; const policyTagKeys = Object.keys(policyTagList); - - if (policyTagKeys.length === 0) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - type: 'violation', - }); - } - - policyTagKeys.forEach((key, index) => { - 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 selectedTag = selectedTags[index]; - const isTagInPolicy = Boolean(policyTagList[key]?.tags[selectedTag]?.enabled); + if (policyTagKeys.length === 1) { + const policyTagListName = Object.keys(policyTagList)[0]; + const policyTags = policyTagList[policyTagListName]?.tags; + const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === 'tagOutOfPolicy'); + const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === 'missingTag'); + 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', - data: { - tagName: key, - }, - }); + if (!hasTagOutOfPolicyViolation && updatedTransaction.tag && !isTagInPolicy) { + newTransactionViolations.push({name: 'tagOutOfPolicy', type: 'violation', userMessage: ''}); } // Remove 'tagOutOfPolicy' violation if tag is in policy - if (hasTagOutOfPolicyViolation && selectedTag && isTagInPolicy) { - newTransactionViolations = reject(newTransactionViolations, { - name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - }); + if (hasTagOutOfPolicyViolation && updatedTransaction.tag && isTagInPolicy) { + newTransactionViolations = reject(newTransactionViolations, {name: 'tagOutOfPolicy'}); } // Remove 'missingTag' violation if tag is valid according to policy if (hasMissingTagViolation && isTagInPolicy) { - newTransactionViolations = reject(newTransactionViolations, { - name: CONST.VIOLATIONS.MISSING_TAG, - }); + newTransactionViolations = reject(newTransactionViolations, {name: 'missingTag'}); } - // Add 'missingTag violation' if tag is required and not set - if (!hasMissingTagViolation && !selectedTag && policyRequiresTags) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.MISSING_TAG, - type: 'violation', - data: { - tagName: key, - }, - }); + if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) { + newTransactionViolations.push({name: 'missingTag', type: 'violation', userMessage: ''}); } - }); + } else { + const selectedTags = updatedTransaction.tag?.split(CONST.COLON) ?? []; + + // if (policyTagKeys.length === 0) { + // newTransactionViolations.push({ + // name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, + // type: 'violation', + // }); + // } + + 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); + + // Add 'tagOutOfPolicy' violation if tag is not in policy + if (!hasTagOutOfPolicyViolation && selectedTag && !isTagInPolicy) { + newTransactionViolations.push({ + name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, + type: 'violation', + data: { + tagName: key, + }, + }); + } + + // Remove 'tagOutOfPolicy' violation if tag is in policy + if (hasTagOutOfPolicyViolation && selectedTag && 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 && !selectedTag && policyRequiresTags) { + newTransactionViolations.push({ + name: CONST.VIOLATIONS.MISSING_TAG, + type: 'violation', + data: { + tagName: key, + }, + }); + } + }); + } } return { @@ -204,7 +231,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': From 3f7a8df2dc66807ca5c6027fc2e04d6501a7d06d Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 09:13:18 -0800 Subject: [PATCH 14/23] Make single tags work --- .../ReportActionItem/MoneyRequestView.tsx | 20 ++----- src/libs/Violations/ViolationsUtils.ts | 52 ------------------- 2 files changed, 5 insertions(+), 67 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 3fb993b6a9e8..fcd44d6664ab 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -222,20 +222,10 @@ function MoneyRequestView({ } // Return violations if there are any - if (canUseViolations && hasViolations(field, data)) { + // At the moment, we only return violations for tags for workspaces with single-level tags + if (canUseViolations && (data?.tagListCount ?? 1) === 1 && hasViolations(field, data)) { const violations = getViolationsForField(field); - for (let i = 0; i < violations.length; i++) { - let violation = violations[i]; - if (violation.name === 'someTagLevelsRequired' && violation.data?.errorIndexes?.includes(data.index)) { - violation['data']['tagName'] = data.tagName; - return ViolationsUtils.getViolationTranslation(violation, translate); - } else if (violation.name === 'tagOutOfPolicy' && violation.data?.tagName === data.tagName) { - violation['data']['tagName'] = data.tagName; - return ViolationsUtils.getViolationTranslation(violation, translate); - } else { - return ViolationsUtils.getViolationTranslation(violation, translate); - } - } + return ViolationsUtils.getViolationTranslation(violations[0], translate); } return ''; @@ -396,8 +386,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, index}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} - error={getErrorForField('tag', {tagName: name, index})} + brickRoadIndicator={getErrorForField('tag', {tagListCount: policyTagLists.length}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + error={getErrorForField('tag', {tagListCount: policyTagLists.length})} /> ))} diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index cfd5c30d135a..164800024a5e 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -76,58 +76,6 @@ const ViolationsUtils = { if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) { newTransactionViolations.push({name: 'missingTag', type: 'violation', userMessage: ''}); } - } else { - const selectedTags = updatedTransaction.tag?.split(CONST.COLON) ?? []; - - // if (policyTagKeys.length === 0) { - // newTransactionViolations.push({ - // name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - // type: 'violation', - // }); - // } - - 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); - - // Add 'tagOutOfPolicy' violation if tag is not in policy - if (!hasTagOutOfPolicyViolation && selectedTag && !isTagInPolicy) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, - type: 'violation', - data: { - tagName: key, - }, - }); - } - - // Remove 'tagOutOfPolicy' violation if tag is in policy - if (hasTagOutOfPolicyViolation && selectedTag && 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 && !selectedTag && policyRequiresTags) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.MISSING_TAG, - type: 'violation', - data: { - tagName: key, - }, - }); - } - }); } } From 08d76093a22d6c3725fa42c273a786341f130e74 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 09:15:57 -0800 Subject: [PATCH 15/23] Update currency fix --- src/libs/actions/IOU.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 26a7c9376239..6e610bd2860f 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -1140,7 +1140,7 @@ function getUpdateMoneyRequestParams( ? IOUUtils.updateIOUOwnerAndTotal(iouReport, updatedReportAction.actorAccountID ?? -1, diff, TransactionUtils.getCurrency(transaction), false, true) : {}; } - updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, updatedTransaction?.currency); + updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, transactionDetails?.currency); optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.reportID}`, From 36c1eb42c23ec77625bbb651dfcd4dcf20d3b1d4 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 09:17:02 -0800 Subject: [PATCH 16/23] Comment --- src/libs/Violations/ViolationsUtils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 164800024a5e..f5baf2ae7637 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -51,6 +51,8 @@ const ViolationsUtils = { 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 = Object.keys(policyTagList)[0]; const policyTags = policyTagList[policyTagListName]?.tags; From a82d1524f826f7628ef068762a5044b602ccc72a Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 10:08:20 -0800 Subject: [PATCH 17/23] Lint + Types --- .../MoneyRequestPreview/MoneyRequestPreviewContent.tsx | 2 +- src/components/ReportActionItem/MoneyRequestView.tsx | 8 ++++---- src/languages/en.ts | 2 +- src/libs/Violations/ViolationsUtils.ts | 10 +++++----- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx index eacd6cd4285e..4656c0cb194f 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?.transactionID, transactionViolations); + const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '', transactionViolations); const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction); const shouldShowRBR = hasViolations || hasFieldErrors; const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index d8b82e14fd8f..81037a5f75a1 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -199,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: Boolean = 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> = { @@ -226,7 +226,7 @@ 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 && (data?.tagListCount ?? 1) === 1 && hasViolations(field, data)) { + if (canUseViolations && shouldShowViolations && hasViolations(field)) { const violations = getViolationsForField(field); return ViolationsUtils.getViolationTranslation(violations[0], translate); } @@ -394,8 +394,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', {tagListCount: policyTagLists.length}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} - error={getErrorForField('tag', {tagListCount: policyTagLists.length})} + brickRoadIndicator={getErrorForField('tag', {}, policyTagLists.length === 1) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + error={getErrorForField('tag', {}, policyTagLists.length === 1)} /> ))} diff --git a/src/languages/en.ts b/src/languages/en.ts index f6c389520891..0553d6470ddc 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -2328,7 +2328,7 @@ export default { return ''; }, smartscanFailed: 'Receipt scanning failed. Enter details manually.', - someTagLevelsRequired: ({tagName}: ViolationsTagOutOfPolicyParams) => `Missing ${tagName ?? 'tag'}`, + someTagLevelsRequired: 'Missing 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/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index f5baf2ae7637..b668b58a91f4 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -62,21 +62,21 @@ const ViolationsUtils = { // Add 'tagOutOfPolicy' violation if tag is not in policy if (!hasTagOutOfPolicyViolation && updatedTransaction.tag && !isTagInPolicy) { - newTransactionViolations.push({name: 'tagOutOfPolicy', type: 'violation', userMessage: ''}); + 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: 'tagOutOfPolicy'}); + 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: 'missingTag'}); + 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: 'missingTag', type: 'violation', userMessage: ''}); + newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation'}); } } } @@ -181,7 +181,7 @@ const ViolationsUtils = { case 'smartscanFailed': return translate('violations.smartscanFailed'); case 'someTagLevelsRequired': - return translate('violations.someTagLevelsRequired', {tagName}); + return translate('violations.someTagLevelsRequired'); case 'tagOutOfPolicy': return translate('violations.tagOutOfPolicy', {tagName}); case 'taxAmountChanged': From be7a804031e85d6419f848def6777a7a1bd057b5 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 10:21:39 -0800 Subject: [PATCH 18/23] Fix violation tests --- tests/unit/ViolationUtilsTest.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/ViolationUtilsTest.js b/tests/unit/ViolationUtilsTest.js index bc9207fb8021..ff86b5fc6753 100644 --- a/tests/unit/ViolationUtilsTest.js +++ b/tests/unit/ViolationUtilsTest.js @@ -153,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', () => { @@ -161,7 +161,7 @@ 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', () => { @@ -173,7 +173,7 @@ describe('getViolationsOnyxData', () => { 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', () => { @@ -185,7 +185,7 @@ describe('getViolationsOnyxData', () => { 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])); }); }); From f1562e7dc7f8c99fc77899ae8834c5694da3cebd Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 10:26:37 -0800 Subject: [PATCH 19/23] Update src/libs/ReportUtils.ts Co-authored-by: Abdelhafidh Belalia <16493223+s77rt@users.noreply.github.com> --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 8c18db1274b8..91c74bc1b37a 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -723,7 +723,7 @@ function hasParticipantInArray(report: Report, policyMemberAccountIDs: number[]) * Whether the Money Request report is settled */ function isSettled(reportID: string | undefined): boolean { - if (!allReports) { + if (!allReports || !reportID) { return false; } const report: Report | EmptyObject = allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? {}; From b3d140ca34ef7cd879ee653d12f18e25c12597aa Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 10:27:01 -0800 Subject: [PATCH 20/23] Lint --- src/components/ReportActionItem/MoneyRequestView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 81037a5f75a1..8851106ddd22 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -199,7 +199,7 @@ function MoneyRequestView({ const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction; const getErrorForField = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations: Boolean = true) => { + (field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations: boolean = 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> = { From 9a97869aa4110286f90c2a171609ef7d34cf31f1 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Thu, 22 Feb 2024 12:00:54 -0800 Subject: [PATCH 21/23] Lint --- src/components/ReportActionItem/MoneyRequestView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 8851106ddd22..a63299e43948 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -199,7 +199,7 @@ function MoneyRequestView({ const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction; const getErrorForField = useCallback( - (field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations: boolean = true) => { + (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> = { From a441ade26d235fedae2bb64d2b7b504896599f99 Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Fri, 23 Feb 2024 10:54:08 -0800 Subject: [PATCH 22/23] Lint --- src/libs/Violations/ViolationsUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 9834d7268444..b668b58a91f4 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'; From 1ee821d1ddab9657f9875b2b7be8428b5b40661b Mon Sep 17 00:00:00 2001 From: Carlos Alvarez Date: Fri, 23 Feb 2024 11:10:45 -0800 Subject: [PATCH 23/23] Apply suggestions from code review Co-authored-by: Abdelhafidh Belalia <16493223+s77rt@users.noreply.github.com> --- src/libs/Violations/ViolationsUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index b668b58a91f4..6153ea62cd0d 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -54,10 +54,10 @@ const ViolationsUtils = { // At the moment, we only return violations for tags for workspaces with single-level tags if (policyTagKeys.length === 1) { - const policyTagListName = Object.keys(policyTagList)[0]; + const policyTagListName = policyTagKeys[0]; const policyTags = policyTagList[policyTagListName]?.tags; - const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === 'tagOutOfPolicy'); - const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === 'missingTag'); + 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