From f587cfd2bbcc0a873c1a2e33332440a1d1939d49 Mon Sep 17 00:00:00 2001 From: FitseTLT Date: Thu, 15 May 2025 00:26:01 +0300 Subject: [PATCH 1/4] added missing violation onyx data --- src/libs/Violations/ViolationsUtils.ts | 84 +++++++++++++++++++++----- src/libs/actions/IOU.ts | 8 ++- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index b6a7572c0f2b..83594f72ac7b 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -228,23 +228,50 @@ const ViolationsUtils = { const isControlPolicy = policy.type === CONST.POLICY.TYPE.CORPORATE; const inputDate = new Date(updatedTransaction.modifiedCreated ?? updatedTransaction.created); const shouldDisplayFutureDateViolation = !isInvoiceTransaction && DateUtils.isFutureDay(inputDate) && isControlPolicy; - const hasReceiptRequiredViolation = transactionViolations.some((violation) => violation.name === 'receiptRequired'); - const hasOverLimitViolation = transactionViolations.some((violation) => violation.name === 'overLimit'); + const hasReceiptRequiredViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.RECEIPT_REQUIRED && violation.data); + const hasCategoryReceiptRequiredViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.RECEIPT_REQUIRED && !violation.data); + const hasOverLimitViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.OVER_LIMIT); + const hasCategoryOverLimitViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.OVER_CATEGORY_LIMIT); + const hasMissingCommentViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_COMMENT); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const amount = updatedTransaction.modifiedAmount || updatedTransaction.amount; // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const currency = updatedTransaction.modifiedCurrency || updatedTransaction.currency; const canCalculateAmountViolations = policy.outputCurrency === currency; + const categoryName = updatedTransaction.category; + const categoryMaxAmountNoReceipt = policyCategories[categoryName ?? '']?.maxAmountNoReceipt; + const maxAmountNoReceipt = policy.maxExpenseAmountNoReceipt; + + // The category maxExpenseAmountNoReceipt and maxExpenseAmount settings override the respective policy settings. const shouldShowReceiptRequiredViolation = canCalculateAmountViolations && !isInvoiceTransaction && - policy.maxExpenseAmountNoReceipt && - Math.abs(amount) > policy.maxExpenseAmountNoReceipt && + typeof categoryMaxAmountNoReceipt !== 'number' && + typeof maxAmountNoReceipt === 'number' && + Math.abs(amount) > maxAmountNoReceipt && + !TransactionUtils.hasReceipt(updatedTransaction) && + isControlPolicy; + const shouldShowCategoryReceiptRequiredViolation = + canCalculateAmountViolations && + !isInvoiceTransaction && + typeof categoryMaxAmountNoReceipt === 'number' && + Math.abs(amount) > categoryMaxAmountNoReceipt && !TransactionUtils.hasReceipt(updatedTransaction) && isControlPolicy; + + const overLimitAmount = policy.maxExpenseAmount; + const categoryOverLimit = policyCategories[categoryName ?? '']?.maxExpenseAmount; const shouldShowOverLimitViolation = - canCalculateAmountViolations && !isInvoiceTransaction && policy.maxExpenseAmount && Math.abs(amount) > policy.maxExpenseAmount && isControlPolicy; + canCalculateAmountViolations && + !isInvoiceTransaction && + typeof categoryOverLimit !== 'number' && + typeof overLimitAmount === 'number' && + Math.abs(amount) > overLimitAmount && + isControlPolicy; + const shouldCategoryShowOverLimitViolation = + canCalculateAmountViolations && !isInvoiceTransaction && typeof categoryOverLimit === 'number' && Math.abs(amount) > categoryOverLimit && isControlPolicy; + const shouldShowMissingComment = !isInvoiceTransaction && policyCategories?.[categoryName ?? '']?.areCommentsRequired && !updatedTransaction.comment?.comment && isControlPolicy; const hasFutureDateViolation = transactionViolations.some((violation) => violation.name === 'futureDate'); // Add 'futureDate' violation if transaction date is in the future and policy type is corporate if (!hasFutureDateViolation && shouldDisplayFutureDateViolation) { @@ -256,34 +283,59 @@ const ViolationsUtils = { newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.FUTURE_DATE}); } - if (canCalculateAmountViolations && !hasReceiptRequiredViolation && shouldShowReceiptRequiredViolation) { + if ( + canCalculateAmountViolations && + ((hasReceiptRequiredViolation && !shouldShowReceiptRequiredViolation) || (hasCategoryReceiptRequiredViolation && !shouldShowCategoryReceiptRequiredViolation)) + ) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.RECEIPT_REQUIRED}); + } + + if ( + canCalculateAmountViolations && + ((!hasReceiptRequiredViolation && !!shouldShowReceiptRequiredViolation) || (!hasCategoryReceiptRequiredViolation && shouldShowCategoryReceiptRequiredViolation)) + ) { newTransactionViolations.push({ name: CONST.VIOLATIONS.RECEIPT_REQUIRED, - data: { - formattedLimit: CurrencyUtils.convertAmountToDisplayString(policy.maxExpenseAmountNoReceipt, policy.outputCurrency), - }, + data: + shouldShowCategoryReceiptRequiredViolation || !policy.maxExpenseAmountNoReceipt + ? undefined + : { + formattedLimit: CurrencyUtils.convertAmountToDisplayString(policy.maxExpenseAmountNoReceipt, policy.outputCurrency), + }, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true, }); } - if (canCalculateAmountViolations && hasReceiptRequiredViolation && !shouldShowReceiptRequiredViolation) { - newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.RECEIPT_REQUIRED}); + if (canCalculateAmountViolations && hasOverLimitViolation && !shouldShowOverLimitViolation) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.OVER_LIMIT}); } - if (canCalculateAmountViolations && !hasOverLimitViolation && shouldShowOverLimitViolation) { + if (canCalculateAmountViolations && hasCategoryOverLimitViolation && !shouldCategoryShowOverLimitViolation) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.OVER_CATEGORY_LIMIT}); + } + + if (canCalculateAmountViolations && ((!hasOverLimitViolation && !!shouldShowOverLimitViolation) || (!hasCategoryOverLimitViolation && shouldCategoryShowOverLimitViolation))) { newTransactionViolations.push({ - name: CONST.VIOLATIONS.OVER_LIMIT, + name: shouldCategoryShowOverLimitViolation ? CONST.VIOLATIONS.OVER_CATEGORY_LIMIT : CONST.VIOLATIONS.OVER_LIMIT, data: { - formattedLimit: CurrencyUtils.convertAmountToDisplayString(policy.maxExpenseAmount, policy.outputCurrency), + formattedLimit: CurrencyUtils.convertAmountToDisplayString(shouldCategoryShowOverLimitViolation ? categoryOverLimit : policy.maxExpenseAmount, policy.outputCurrency), }, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true, }); } - if (canCalculateAmountViolations && hasOverLimitViolation && !shouldShowOverLimitViolation) { - newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.OVER_LIMIT}); + if (!hasMissingCommentViolation && shouldShowMissingComment) { + newTransactionViolations.push({ + name: CONST.VIOLATIONS.MISSING_COMMENT, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + }); + } + + if (hasMissingCommentViolation && !shouldShowMissingComment) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_COMMENT}); } return { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index f438b9d47488..be8fc9157d3d 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -4103,8 +4103,14 @@ function getUpdateMoneyRequestParams( value: {...iouReport, ...(isTotalIndeterministic && {pendingFields: {total: null}})}, }); } + const hasModifiedComment = 'comment' in transactionChanges; - if (policy && isPaidGroupPolicy(policy) && updatedTransaction && (hasModifiedTag || hasModifiedCategory || hasModifiedDistanceRate || hasModifiedAmount || hasModifiedCreated)) { + if ( + policy && + isPaidGroupPolicy(policy) && + updatedTransaction && + (hasModifiedTag || hasModifiedCategory || hasModifiedComment || hasModifiedDistanceRate || hasModifiedAmount || hasModifiedCreated) + ) { const currentTransactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] ?? []; const violationsOnyxData = ViolationsUtils.getViolationsOnyxData( updatedTransaction, From 0d7303db5a057634d6443a94a76d62cf235adfcc Mon Sep 17 00:00:00 2001 From: FitseTLT Date: Thu, 15 May 2025 22:51:14 +0300 Subject: [PATCH 2/4] added test --- tests/unit/ViolationUtilsTest.ts | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 607499d2a398..7c52c217ddc6 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -34,6 +34,13 @@ const receiptRequiredViolation = { }, }; +const categoryReceiptRequiredViolation = { + name: CONST.VIOLATIONS.RECEIPT_REQUIRED, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + data: undefined, +}; + const overLimitViolation = { name: CONST.VIOLATIONS.OVER_LIMIT, type: CONST.VIOLATION_TYPES.VIOLATION, @@ -43,6 +50,21 @@ const overLimitViolation = { }, }; +const categoryOverLimitViolation = { + name: CONST.VIOLATIONS.OVER_CATEGORY_LIMIT, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + data: { + formattedLimit: convertAmountToDisplayString(CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT), + }, +}; + +const categoryMissingCommentViolation = { + name: CONST.VIOLATIONS.MISSING_COMMENT, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, +}; + const customUnitOutOfPolicyViolation = { name: CONST.VIOLATIONS.CUSTOM_UNIT_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION, @@ -210,6 +232,30 @@ describe('getViolationsOnyxData', () => { }); }); + describe('policyCategoryRules', () => { + beforeEach(() => { + policy.type = CONST.POLICY.TYPE.CORPORATE; + policy.outputCurrency = CONST.CURRENCY.USD; + policyCategories = { + Food: { + name: 'Food', + enabled: true, + areCommentsRequired: true, + maxAmountNoReceipt: 0, + maxExpenseAmount: CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT, + }, + }; + transaction.category = 'Food'; + transaction.amount = CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT + 1; + transaction.comment = {comment: ''}; + }); + + it.only('should add category specific violations', () => { + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false, false); + expect(result.value).toEqual(expect.arrayContaining([categoryOverLimitViolation, categoryReceiptRequiredViolation, categoryMissingCommentViolation, ...transactionViolations])); + }); + }); + describe('policyRequiresCategories', () => { beforeEach(() => { policy.requiresCategory = true; From 2517f12200db6f037c4b140500abf267311cfac4 Mon Sep 17 00:00:00 2001 From: FitseTLT Date: Mon, 19 May 2025 14:51:58 +0300 Subject: [PATCH 3/4] calculate violation data on receipt change --- src/libs/actions/IOU.ts | 59 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index d31c9c0bbea2..23577b986f54 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -212,7 +212,7 @@ import {buildOptimisticPolicyRecentlyUsedCategories} from './Policy/Category'; import {buildAddMembersToWorkspaceOnyxData, buildUpdateWorkspaceMembersRoleOnyxData} from './Policy/Member'; import {buildOptimisticPolicyRecentlyUsedDestinations} from './Policy/PerDiem'; import {buildOptimisticRecentlyUsedCurrencies, buildPolicyData, generatePolicyID} from './Policy/Policy'; -import {buildOptimisticPolicyRecentlyUsedTags} from './Policy/Tag'; +import {buildOptimisticPolicyRecentlyUsedTags, getPolicyTagsData} from './Policy/Tag'; import {buildInviteToRoomOnyxData, completeOnboarding, getCurrentUserAccountID, notifyNewAction} from './Report'; import {clearAllRelatedReportActionErrors} from './ReportActions'; import {getRecentWaypoints, sanitizeRecentWaypoints} from './Transaction'; @@ -677,6 +677,13 @@ Onyx.connect({ }, }); +let allPolicyCategories: OnyxCollection = {}; +Onyx.connect({ + key: ONYXKEYS.COLLECTION.POLICY_CATEGORIES, + waitForCollectionCallback: true, + callback: (val) => (allPolicyCategories = val), +}); + const allPolicies: OnyxCollection = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.POLICY, @@ -9860,6 +9867,8 @@ function detachReceipt(transactionID: string | undefined) { return; } const transaction = allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; + const expenseReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`] ?? null; + const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${expenseReport?.policyID}`]; const newTransaction = transaction ? { ...transaction, @@ -9907,7 +9916,28 @@ function detachReceipt(transactionID: string | undefined) { }, }, ]; - const expenseReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`] ?? null; + + if (policy && isPaidGroupPolicy(policy) && newTransaction) { + const policyCategories = allPolicyCategories?.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policy.id}`]; + const policyTagList = getPolicyTagsData(policy.id); + const currentTransactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] ?? []; + const violationsOnyxData = ViolationsUtils.getViolationsOnyxData( + newTransaction, + currentTransactionViolations, + policy, + policyTagList ?? {}, + policyCategories ?? {}, + hasDependentTags(policy, policyTagList ?? {}), + isInvoiceReportReportUtils(expenseReport), + ); + optimisticData.push(violationsOnyxData); + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`, + value: currentTransactionViolations, + }); + } + const updatedReportAction = buildOptimisticDetachReceipt(expenseReport?.reportID, transactionID, transaction?.merchant); optimisticData.push({ @@ -9962,12 +9992,14 @@ function replaceReceipt({transactionID, file, source}: ReplaceReceipt) { } const transaction = allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; + const expenseReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`] ?? null; + const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${expenseReport?.policyID}`]; const oldReceipt = transaction?.receipt ?? {}; const receiptOptimistic = { source, state: CONST.IOU.RECEIPT_STATE.OPEN, }; - + const newTransaction = transaction && {...transaction, receipt: receiptOptimistic, filename: file.name}; const retryParams = {transactionID, file: undefined, source}; const optimisticData: OnyxUpdate[] = [ @@ -10012,6 +10044,27 @@ function replaceReceipt({transactionID, file, source}: ReplaceReceipt) { }, ]; + if (policy && isPaidGroupPolicy(policy) && newTransaction) { + const policyCategories = allPolicyCategories?.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policy.id}`]; + const policyTagList = getPolicyTagsData(policy.id); + const currentTransactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] ?? []; + const violationsOnyxData = ViolationsUtils.getViolationsOnyxData( + newTransaction, + currentTransactionViolations, + policy, + policyTagList ?? {}, + policyCategories ?? {}, + hasDependentTags(policy, policyTagList ?? {}), + isInvoiceReportReportUtils(expenseReport), + ); + optimisticData.push(violationsOnyxData); + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`, + value: currentTransactionViolations, + }); + } + const parameters: ReplaceReceiptParams = { transactionID, receipt: file, From deef8dba89206b1aa684e7ab627dd145fa675817 Mon Sep 17 00:00:00 2001 From: FitseTLT Date: Tue, 27 May 2025 18:45:27 +0300 Subject: [PATCH 4/4] reset transaction receipt on detach receipt --- src/libs/actions/IOU.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 7cdfd0ad878d..e07e8e1acd66 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -10028,9 +10028,7 @@ function detachReceipt(transactionID: string | undefined) { ? { ...transaction, filename: '', - receipt: { - source: '', - }, + receipt: {}, } : null;