From 6b4cb7a05b275d699fbc4cdee65f2e2e2262b987 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Fri, 9 Aug 2024 08:57:27 +0200 Subject: [PATCH 01/19] Handle customUnitOutOfPolicy violation --- .../ReportActionItem/MoneyRequestView.tsx | 5 +- src/hooks/useViolations.ts | 4 +- src/languages/en.ts | 2 +- src/languages/es.ts | 2 +- src/libs/Violations/ViolationsUtils.ts | 13 +- src/libs/actions/IOU.ts | 23 ++-- tests/unit/ViolationUtilsTest.ts | 121 +++++++++++------- 7 files changed, 105 insertions(+), 65 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 00f31c7c38f..75f9f8ef9a0 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -230,7 +230,8 @@ function MoneyRequestView({ const currency = policy ? policy.outputCurrency : PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD; - const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? DistanceRequestUtils.getRateForP2P(currency) : distanceRates[rateID] ?? {}; + const isCustomUnitRateIDForP2P = TransactionUtils.isCustomUnitRateIDForP2P(transaction); + const mileageRate = isCustomUnitRateIDForP2P ? DistanceRequestUtils.getRateForP2P(currency) : distanceRates[rateID] ?? {}; const {unit} = mileageRate; const rate = transaction?.comment?.customUnit?.defaultP2PRate ?? mileageRate.rate; @@ -355,6 +356,8 @@ function MoneyRequestView({ onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DISTANCE_RATE.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '-1', report?.reportID ?? '-1')) } + brickRoadIndicator={getErrorForField('customUnitRateID') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + errorText={getErrorForField('customUnitRateID')} /> diff --git a/src/hooks/useViolations.ts b/src/hooks/useViolations.ts index 44b8e982139..f645b03a99c 100644 --- a/src/hooks/useViolations.ts +++ b/src/hooks/useViolations.ts @@ -5,7 +5,7 @@ import type {TransactionViolation, ViolationName} from '@src/types/onyx'; /** * Names of Fields where violations can occur. */ -type ViolationField = 'amount' | 'billable' | 'category' | 'comment' | 'date' | 'merchant' | 'receipt' | 'tag' | 'tax' | 'none'; +type ViolationField = 'amount' | 'billable' | 'category' | 'comment' | 'date' | 'merchant' | 'receipt' | 'tag' | 'tax' | 'customUnitRateID' | 'none'; /** * Map from Violation Names to the field where that violation can occur. @@ -17,7 +17,7 @@ const violationFields: Record = { cashExpenseWithNoReceipt: 'receipt', categoryOutOfPolicy: 'category', conversionSurcharge: 'amount', - customUnitOutOfPolicy: 'merchant', + customUnitOutOfPolicy: 'customUnitRateID', duplicatedTransaction: 'merchant', fieldRequired: 'merchant', futureDate: 'date', diff --git a/src/languages/en.ts b/src/languages/en.ts index 72df3a16d2a..731bc0680d2 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -4017,7 +4017,7 @@ export default { cashExpenseWithNoReceipt: ({formattedLimit}: ViolationsCashExpenseWithNoReceiptParams) => `Receipt required${formattedLimit ? ` over ${formattedLimit}` : ''}`, categoryOutOfPolicy: 'Category no longer valid', conversionSurcharge: ({surcharge}: ViolationsConversionSurchargeParams) => `Applied ${surcharge}% conversion surcharge`, - customUnitOutOfPolicy: 'Unit no longer valid', + customUnitOutOfPolicy: 'Rate not valid for this workspace', duplicatedTransaction: 'Duplicate', fieldRequired: 'Report fields are required', futureDate: 'Future date not allowed', diff --git a/src/languages/es.ts b/src/languages/es.ts index 68ab4276d06..b4396140589 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -4533,7 +4533,7 @@ export default { cashExpenseWithNoReceipt: ({formattedLimit}: ViolationsCashExpenseWithNoReceiptParams) => `Recibo obligatorio para cantidades mayores de ${formattedLimit}`, categoryOutOfPolicy: 'La categoría ya no es válida', conversionSurcharge: ({surcharge}: ViolationsConversionSurchargeParams = {}) => `${surcharge}% de recargo aplicado`, - customUnitOutOfPolicy: 'La unidad ya no es válida', + customUnitOutOfPolicy: 'Tasa inválida para este espacio de trabajo', duplicatedTransaction: 'Duplicado', fieldRequired: 'Los campos del informe son obligatorios', futureDate: 'Fecha futura no permitida', diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 5d64bc6c3c6..1f58b42ff8b 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -2,12 +2,12 @@ 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 {getSortedTagKeys} from '@libs/PolicyUtils'; +import {getSortedTagKeys, getCustomUnitRate} from '@libs/PolicyUtils'; import * as TransactionUtils from '@libs/TransactionUtils'; import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {PolicyCategories, PolicyTagList, Transaction, TransactionViolation, ViolationName} from '@src/types/onyx'; +import type {Policy, PolicyCategories, PolicyTagList, Transaction, TransactionViolation, ViolationName} from '@src/types/onyx'; /** * Calculates tag out of policy and missing tag violations for the given transaction @@ -165,9 +165,8 @@ const ViolationsUtils = { getViolationsOnyxData( updatedTransaction: Transaction, transactionViolations: TransactionViolation[], - policyRequiresTags: boolean, + policy: Policy, policyTagList: PolicyTagList, - policyRequiresCategories: boolean, policyCategories: PolicyCategories, hasDependentTags: boolean, ): OnyxUpdate { @@ -183,6 +182,7 @@ const ViolationsUtils = { let newTransactionViolations = [...transactionViolations]; // Calculate client-side category violations + const policyRequiresCategories = !!policy.requiresCategory; if (policyRequiresCategories) { const hasCategoryOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === 'categoryOutOfPolicy'); const hasMissingCategoryViolation = transactionViolations.some((violation) => violation.name === 'missingCategory'); @@ -211,6 +211,7 @@ const ViolationsUtils = { } // Calculate client-side tag violations + const policyRequiresTags = !!policy.requiresTag; if (policyRequiresTags) { newTransactionViolations = Object.keys(policyTagList).length === 1 @@ -218,6 +219,10 @@ const ViolationsUtils = { : getTagViolationsForMultiLevelTags(updatedTransaction, newTransactionViolations, policyTagList, hasDependentTags); } + if (updatedTransaction.modifiedCustomUnitRateID && !!getCustomUnitRate(policy, updatedTransaction.modifiedCustomUnitRateID)) { + newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.CUSTOM_UNIT_OUT_OF_POLICY}); + } + return { onyxMethod: Onyx.METHOD.SET, key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${updatedTransaction.transactionID}`, diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 9549bc23a9d..4ecc388e7ba 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -878,9 +878,8 @@ function buildOnyxDataForMoneyRequest( const violationsOnyxData = ViolationsUtils.getViolationsOnyxData( transaction, [], - !!policy.requiresTag, + policy, policyTagList ?? {}, - !!policy.requiresCategory, policyCategories ?? {}, PolicyUtils.hasDependentTags(policy, policyTagList ?? {}), ); @@ -1245,9 +1244,8 @@ function buildOnyxDataForInvoice( const violationsOnyxData = ViolationsUtils.getViolationsOnyxData( transaction, [], - !!policy.requiresTag, + policy, policyTagList ?? {}, - !!policy.requiresCategory, policyCategories ?? {}, PolicyUtils.hasDependentTags(policy, policyTagList ?? {}), ); @@ -1622,9 +1620,8 @@ function buildOnyxDataForTrackExpense( const violationsOnyxData = ViolationsUtils.getViolationsOnyxData( transaction, [], - !!policy.requiresTag, + policy, policyTagList ?? {}, - !!policy.requiresCategory, policyCategories ?? {}, PolicyUtils.hasDependentTags(policy, policyTagList ?? {}), ); @@ -2637,7 +2634,8 @@ function getUpdateMoneyRequestParams( } // Update recently used categories if the category is changed - if ('category' in transactionChanges) { + const hasModifiedCategory = 'category' in transactionChanges; + if (hasModifiedCategory) { const optimisticPolicyRecentlyUsedCategories = Category.buildOptimisticPolicyRecentlyUsedCategories(iouReport?.policyID, transactionChanges.category); if (optimisticPolicyRecentlyUsedCategories.length) { optimisticData.push({ @@ -2649,7 +2647,8 @@ function getUpdateMoneyRequestParams( } // Update recently used categories if the tag is changed - if ('tag' in transactionChanges) { + const hasModifiedTag = 'tag' in transactionChanges; + if (hasModifiedTag) { const optimisticPolicyRecentlyUsedTags = Tag.buildOptimisticPolicyRecentlyUsedTags(iouReport?.policyID, transactionChanges.tag); if (!isEmptyObject(optimisticPolicyRecentlyUsedTags)) { optimisticData.push({ @@ -2692,15 +2691,14 @@ function getUpdateMoneyRequestParams( }); } - if (policy && PolicyUtils.isPaidGroupPolicy(policy) && updatedTransaction && ('tag' in transactionChanges || 'category' in transactionChanges)) { + if (policy && PolicyUtils.isPaidGroupPolicy(policy) && updatedTransaction && (hasModifiedTag || hasModifiedCategory || hasModifiedDistanceRate)) { const currentTransactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] ?? []; optimisticData.push( ViolationsUtils.getViolationsOnyxData( updatedTransaction, currentTransactionViolations, - !!policy.requiresTag, + policy, policyTagList ?? {}, - !!policy.requiresCategory, policyCategories ?? {}, PolicyUtils.hasDependentTags(policy, policyTagList ?? {}), ), @@ -5357,9 +5355,8 @@ function editRegularMoneyRequest( const updatedViolationsOnyxData = ViolationsUtils.getViolationsOnyxData( updatedTransaction, currentTransactionViolations, - !!policy.requiresTag, + policy, policyTags, - !!policy.requiresCategory, policyCategories, PolicyUtils.hasDependentTags(policy, policyTags), ); diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 5e8f93a56db..16196fa40b2 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -3,47 +3,50 @@ import Onyx from 'react-native-onyx'; import ViolationsUtils from '@libs/Violations/ViolationsUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {PolicyCategories, PolicyTagList, Transaction, TransactionViolation} from '@src/types/onyx'; +import type {Policy, PolicyCategories, PolicyTagList, Transaction, TransactionViolation} from '@src/types/onyx'; const categoryOutOfPolicyViolation = { - name: 'categoryOutOfPolicy', + name: CONST.VIOLATIONS.CATEGORY_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION, }; const missingCategoryViolation = { - name: 'missingCategory', + name: CONST.VIOLATIONS.MISSING_CATEGORY, type: CONST.VIOLATION_TYPES.VIOLATION, }; -const tagOutOfPolicyViolation = { - name: 'tagOutOfPolicy', +const customUnitOutOfPolicyViolation = { + name: CONST.VIOLATIONS.CUSTOM_UNIT_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION, }; const missingTagViolation = { - name: 'missingTag', + name: CONST.VIOLATIONS.MISSING_TAG, + type: CONST.VIOLATION_TYPES.VIOLATION, +}; + +const tagOutOfPolicyViolation = { + name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION, }; describe('getViolationsOnyxData', () => { let transaction: Transaction; let transactionViolations: TransactionViolation[]; - let policyRequiresTags: boolean; + let policy: Policy; let policyTags: PolicyTagList; - let policyRequiresCategories: boolean; let policyCategories: PolicyCategories; beforeEach(() => { transaction = {transactionID: '123', reportID: '1234', amount: 100, comment: {}, created: '2023-07-24 13:46:20', merchant: 'United Airlines', currency: 'USD'}; transactionViolations = []; - policyRequiresTags = false; + policy = {requiresTag: false, requiresCategory: false} as Policy; policyTags = {}; - policyRequiresCategories = false; policyCategories = {}; }); it('should return an object with correct shape and with empty transactionViolations array', () => { - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result).toEqual({ onyxMethod: Onyx.METHOD.SET, @@ -57,31 +60,64 @@ describe('getViolationsOnyxData', () => { {name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION}, {name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION}, ]; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining(transactionViolations)); }); + describe('distance rate was modified', () => { + beforeEach(() => { + transactionViolations = [customUnitOutOfPolicyViolation,]; + + const customUnitRateID = 'rate_id'; + transaction.modifiedCustomUnitRateID = customUnitRateID; + policy.customUnits = { + unit_id: { + attributes: {unit: 'mi'}, + customUnitID: 'unit_id', + defaultCategory: 'Car', + enabled: true, + name: 'Distance', + rates: { + [customUnitRateID]: { + currency: 'USD', + customUnitRateID, + enabled: true, + name: 'Default Rate', + rate: 65.5 + } + } + } + }; + }); + + it('should remove the customUnitOutOfPolicy violation if the modified one belongs to the policy', () => { + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); + + expect(result.value).not.toContainEqual(customUnitOutOfPolicyViolation); + }); + }); + describe('policyRequiresCategories', () => { beforeEach(() => { - policyRequiresCategories = true; + policy.requiresCategory = true; policyCategories = {Food: {name: 'Food', unencodedName: '', enabled: true, areCommentsRequired: false, externalID: '1234', origin: '12345'}}; transaction.category = 'Food'; }); it('should add missingCategory violation if no category is included', () => { transaction.category = undefined; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([missingCategoryViolation, ...transactionViolations])); }); it('should add categoryOutOfPolicy violation when category is not in policy', () => { transaction.category = 'Bananas'; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([categoryOutOfPolicyViolation, ...transactionViolations])); }); it('should not include a categoryOutOfPolicy violation when category is in policy', () => { - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).not.toContainEqual(categoryOutOfPolicyViolation); }); @@ -90,9 +126,8 @@ describe('getViolationsOnyxData', () => { const result = ViolationsUtils.getViolationsOnyxData( partialTransaction, transactionViolations, - policyRequiresTags, + policy, policyTags, - policyRequiresCategories, policyCategories, false, ); @@ -106,7 +141,7 @@ describe('getViolationsOnyxData', () => { {name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION}, ]; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([categoryOutOfPolicyViolation, ...transactionViolations])); }); @@ -118,7 +153,7 @@ describe('getViolationsOnyxData', () => { {name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION}, ]; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([missingCategoryViolation, ...transactionViolations])); }); @@ -126,20 +161,20 @@ describe('getViolationsOnyxData', () => { describe('policy does not require Categories', () => { beforeEach(() => { - policyRequiresCategories = false; + policy.requiresCategory = false; }); it('should not add any violations when categories are not required', () => { - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); - expect(result.value).not.toContainEqual([categoryOutOfPolicyViolation]); - expect(result.value).not.toContainEqual([missingCategoryViolation]); + expect(result.value).not.toContainEqual(categoryOutOfPolicyViolation); + expect(result.value).not.toContainEqual(missingCategoryViolation); }); }); describe('policyRequiresTags', () => { beforeEach(() => { - policyRequiresTags = true; + policy.requiresTag = true; policyTags = { Meals: { name: 'Meals', @@ -155,7 +190,7 @@ describe('getViolationsOnyxData', () => { }); it("shouldn't update the transactionViolations if the policy requires tags and the transaction has a tag from the policy", () => { - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(transactionViolations); }); @@ -163,7 +198,7 @@ describe('getViolationsOnyxData', () => { it('should add a missingTag violation if none is provided and policy requires tags', () => { transaction.tag = undefined; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation}])); }); @@ -171,7 +206,7 @@ describe('getViolationsOnyxData', () => { it('should add a tagOutOfPolicy violation when policy requires tags and tag is not in the policy', () => { policyTags = {}; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual([]); }); @@ -181,9 +216,8 @@ describe('getViolationsOnyxData', () => { const result = ViolationsUtils.getViolationsOnyxData( partialTransaction, transactionViolations, - policyRequiresTags, + policy, policyTags, - policyRequiresCategories, policyCategories, false, ); @@ -197,7 +231,7 @@ describe('getViolationsOnyxData', () => { {name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION}, ]; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([{...tagOutOfPolicyViolation}, ...transactionViolations])); }); @@ -209,7 +243,7 @@ describe('getViolationsOnyxData', () => { {name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION}, ]; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation}, ...transactionViolations])); }); @@ -217,19 +251,20 @@ describe('getViolationsOnyxData', () => { describe('policy does not require Tags', () => { beforeEach(() => { - policyRequiresTags = false; + policy.requiresTag = false; }); it('should not add any violations when tags are not required', () => { - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); - expect(result.value).not.toContainEqual([tagOutOfPolicyViolation]); - expect(result.value).not.toContainEqual([missingTagViolation]); + expect(result.value).not.toContainEqual(tagOutOfPolicyViolation); + expect(result.value).not.toContainEqual(missingTagViolation); }); }); + describe('policy has multi level tags', () => { beforeEach(() => { - policyRequiresTags = true; + policy.requiresTag = true; policyTags = { Department: { name: 'Department', @@ -276,30 +311,30 @@ describe('getViolationsOnyxData', () => { }; // Test case where transaction has no tags - let result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + let result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual([someTagLevelsRequiredViolation]); // Test case where transaction has 1 tag transaction.tag = 'Africa'; someTagLevelsRequiredViolation.data = {errorIndexes: [1, 2]}; - result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual([someTagLevelsRequiredViolation]); // Test case where transaction has 2 tags transaction.tag = 'Africa::Project1'; someTagLevelsRequiredViolation.data = {errorIndexes: [1]}; - result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).toEqual([someTagLevelsRequiredViolation]); // Test case where transaction has all tags transaction.tag = 'Africa:Accounting:Project1'; - result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); 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 = 'Africa:Accounting:Project1'; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, false); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false); const violation = {...tagOutOfPolicyViolation, data: {tagName: 'Department'}}; expect(result.value).toEqual([violation]); }); @@ -308,7 +343,7 @@ describe('getViolationsOnyxData', () => { const missingRegionTag = {...missingTagViolation, data: {tagName: 'Region'}}; const missingProjectTag = {...missingTagViolation, data: {tagName: 'Project'}}; transaction.tag = undefined; - const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories, true); + const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, true); expect(result.value).toEqual(expect.arrayContaining([missingDepartmentTag, missingRegionTag, missingProjectTag])); }); }); From 927879bd85b31f0e78d0e927f78879718b3f5970 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Sat, 10 Aug 2024 17:45:26 +0200 Subject: [PATCH 02/19] Lint --- src/libs/Violations/ViolationsUtils.ts | 2 +- tests/unit/ViolationUtilsTest.ts | 28 +++++++------------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 1f58b42ff8b..03c9d97692a 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -2,7 +2,7 @@ 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 {getSortedTagKeys, getCustomUnitRate} from '@libs/PolicyUtils'; +import {getCustomUnitRate, getSortedTagKeys} from '@libs/PolicyUtils'; import * as TransactionUtils from '@libs/TransactionUtils'; import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 16196fa40b2..add6e60ac44 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -66,7 +66,7 @@ describe('getViolationsOnyxData', () => { describe('distance rate was modified', () => { beforeEach(() => { - transactionViolations = [customUnitOutOfPolicyViolation,]; + transactionViolations = [customUnitOutOfPolicyViolation]; const customUnitRateID = 'rate_id'; transaction.modifiedCustomUnitRateID = customUnitRateID; @@ -83,10 +83,10 @@ describe('getViolationsOnyxData', () => { customUnitRateID, enabled: true, name: 'Default Rate', - rate: 65.5 - } - } - } + rate: 65.5, + }, + }, + }, }; }); @@ -123,14 +123,7 @@ describe('getViolationsOnyxData', () => { it('should not add a category violation when the transaction is partial', () => { const partialTransaction = {...transaction, amount: 0, merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, category: undefined}; - const result = ViolationsUtils.getViolationsOnyxData( - partialTransaction, - transactionViolations, - policy, - policyTags, - policyCategories, - false, - ); + const result = ViolationsUtils.getViolationsOnyxData(partialTransaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).not.toContainEqual(missingCategoryViolation); }); @@ -213,14 +206,7 @@ describe('getViolationsOnyxData', () => { it('should not add a tag violation when the transaction is partial', () => { const partialTransaction = {...transaction, amount: 0, merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT, tag: undefined}; - const result = ViolationsUtils.getViolationsOnyxData( - partialTransaction, - transactionViolations, - policy, - policyTags, - policyCategories, - false, - ); + const result = ViolationsUtils.getViolationsOnyxData(partialTransaction, transactionViolations, policy, policyTags, policyCategories, false); expect(result.value).not.toContainEqual(missingTagViolation); }); From cc472a2839d9124eb2edf076f14e5898c05f8e0e Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Sat, 10 Aug 2024 18:00:47 +0200 Subject: [PATCH 03/19] Lint --- tests/unit/ViolationUtilsTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index add6e60ac44..87d3fecdb17 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -71,9 +71,9 @@ describe('getViolationsOnyxData', () => { const customUnitRateID = 'rate_id'; transaction.modifiedCustomUnitRateID = customUnitRateID; policy.customUnits = { - unit_id: { + unitId: { attributes: {unit: 'mi'}, - customUnitID: 'unit_id', + customUnitID: 'unitId', defaultCategory: 'Car', enabled: true, name: 'Distance', From 742d1a37c84db7f01363bebcf00bc375bb628853 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Sun, 11 Aug 2024 21:04:28 +0200 Subject: [PATCH 04/19] Revert accidental change --- src/components/ReportActionItem/MoneyRequestView.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 4dedfc95d30..a6cf1242528 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -239,8 +239,7 @@ function MoneyRequestView({ const currency = policy ? policy.outputCurrency : PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD; - const isCustomUnitRateIDForP2P = TransactionUtils.isCustomUnitRateIDForP2P(transaction); - const mileageRate = isCustomUnitRateIDForP2P ? DistanceRequestUtils.getRateForP2P(currency) : distanceRates[rateID] ?? {}; + const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? DistanceRequestUtils.getRateForP2P(currency) : distanceRates[rateID] ?? {}; const {unit} = mileageRate; const rate = transaction?.comment?.customUnit?.defaultP2PRate ?? mileageRate.rate; From ce53514ab2a46fc7b0d211fec755a744de6cc452 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Fri, 6 Sep 2024 16:56:29 -0700 Subject: [PATCH 05/19] WIP fix optimistic rate update --- src/libs/TransactionUtils/index.ts | 10 ++++++++++ src/types/onyx/Transaction.ts | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 339f57c8e04..132d367523e 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -257,6 +257,16 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra if (Object.hasOwn(transactionChanges, 'customUnitRateID')) { updatedTransaction.modifiedCustomUnitRateID = transactionChanges.customUnitRateID; + if (!updatedTransaction.comment) { + updatedTransaction.comment = {}; + } + updatedTransaction.comment = { + ...updatedTransaction?.comment, + customUnit: { + ...updatedTransaction?.comment?.customUnit, + defaultP2PRate: null, + }, + }; shouldStopSmartscan = true; } diff --git a/src/types/onyx/Transaction.ts b/src/types/onyx/Transaction.ts index 2677e8a74fd..6543e04931b 100644 --- a/src/types/onyx/Transaction.ts +++ b/src/types/onyx/Transaction.ts @@ -99,7 +99,7 @@ type TransactionCustomUnit = { name?: ValueOf; /** Default rate for custom unit */ - defaultP2PRate?: number; + defaultP2PRate: number | null; }; /** Types of geometry */ From cc9752844bccf43643563b4115ca0c5ad710a958 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Sun, 8 Sep 2024 11:29:35 -0700 Subject: [PATCH 06/19] Frontend only modifiedCustomUnitRateID is unnecessary --- src/libs/TransactionUtils/index.ts | 10 ++++------ src/libs/Violations/ViolationsUtils.ts | 2 +- src/types/onyx/Transaction.ts | 5 +---- tests/unit/ViolationUtilsTest.ts | 8 +++++++- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 132d367523e..f80c14c80c7 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -256,14 +256,11 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra } if (Object.hasOwn(transactionChanges, 'customUnitRateID')) { - updatedTransaction.modifiedCustomUnitRateID = transactionChanges.customUnitRateID; - if (!updatedTransaction.comment) { - updatedTransaction.comment = {}; - } updatedTransaction.comment = { - ...updatedTransaction?.comment, + ...(updatedTransaction?.comment ?? {}), customUnit: { ...updatedTransaction?.comment?.customUnit, + customUnitRateID: transactionChanges.customUnitRateID, defaultP2PRate: null, }, }; @@ -774,6 +771,7 @@ function calculateAmountForUpdatedWaypointOrRate( ) { const hasModifiedRouteWithPendingWaypoints = !isEmptyObject(transactionChanges.waypoints) && isEmptyObject(transactionChanges?.routes?.route0?.geometry); const hasModifiedRateWithPendingWaypoints = !!transactionChanges?.customUnitRateID && isFetchingWaypointsFromServer(transaction); + console.log('Ndebug hasModifiedRateWithPendingWaypoints', hasModifiedRateWithPendingWaypoints); if (hasModifiedRouteWithPendingWaypoints || hasModifiedRateWithPendingWaypoints) { return { amount: CONST.IOU.DEFAULT_AMOUNT, @@ -835,7 +833,7 @@ function isPayAtEndExpense(transaction: Transaction | undefined | null): boolean * Get custom unit rate (distance rate) ID from the transaction object */ function getRateID(transaction: OnyxInputOrEntry): string | undefined { - return transaction?.modifiedCustomUnitRateID ?? transaction?.comment?.customUnit?.customUnitRateID?.toString(); + return transaction?.comment?.customUnit?.customUnitRateID ?? CONST.CUSTOM_UNITS.FAKE_P2P_ID; } /** diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index e9a54b58e65..adbc0546022 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -219,7 +219,7 @@ const ViolationsUtils = { : getTagViolationsForMultiLevelTags(updatedTransaction, newTransactionViolations, policyTagList, hasDependentTags); } - if (updatedTransaction.modifiedCustomUnitRateID && !!getCustomUnitRate(policy, updatedTransaction.modifiedCustomUnitRateID)) { + if (updatedTransaction?.comment?.customUnit?.customUnitRateID && !!getCustomUnitRate(policy, updatedTransaction?.comment?.customUnit?.customUnitRateID)) { newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.CUSTOM_UNIT_OUT_OF_POLICY}); } diff --git a/src/types/onyx/Transaction.ts b/src/types/onyx/Transaction.ts index 6543e04931b..bc74921c62b 100644 --- a/src/types/onyx/Transaction.ts +++ b/src/types/onyx/Transaction.ts @@ -99,7 +99,7 @@ type TransactionCustomUnit = { name?: ValueOf; /** Default rate for custom unit */ - defaultP2PRate: number | null; + defaultP2PRate?: number | null; }; /** Types of geometry */ @@ -357,9 +357,6 @@ type Transaction = OnyxCommon.OnyxValueWithOfflineFeedback< /** The edited waypoints for the distance expense */ modifiedWaypoints?: WaypointCollection; - /** The edited distance rate for the distance request */ - modifiedCustomUnitRateID?: string; - /** * Used during the creation flow before the transaction is saved to the server and helps dictate where * the user is navigated to when pressing the back button on the confirmation step diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 51c4af1f37f..e5acf41da17 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -69,7 +69,13 @@ describe('getViolationsOnyxData', () => { transactionViolations = [customUnitOutOfPolicyViolation]; const customUnitRateID = 'rate_id'; - transaction.modifiedCustomUnitRateID = customUnitRateID; + transaction.comment = { + ...transaction.comment, + customUnit: { + ...(transaction?.comment?.customUnit ?? {}), + customUnitRateID, + }, + }; policy.customUnits = { unitId: { attributes: {unit: 'mi'}, From 6445f4d838f3fd96db2748ca0b2b0ab8e57ff5aa Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Sun, 8 Sep 2024 11:29:57 -0700 Subject: [PATCH 07/19] Fix optimistic amount update when rate changes --- 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 2be121428a4..e0704228cc6 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -2482,7 +2482,7 @@ function getUpdateMoneyRequestParams( if (transaction && updatedTransaction && (hasPendingWaypoints || hasModifiedDistanceRate)) { updatedTransaction = { ...updatedTransaction, - ...TransactionUtils.calculateAmountForUpdatedWaypointOrRate(transaction, transactionChanges, policy, ReportUtils.isExpenseReport(iouReport)), + ...TransactionUtils.calculateAmountForUpdatedWaypointOrRate(updatedTransaction, transactionChanges, policy, ReportUtils.isExpenseReport(iouReport)), }; // Delete the draft transaction when editing waypoints when the server responds successfully and there are no errors From 02e5e69322618a5a752b7d62e940fd1202d29f59 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 09:03:44 -0700 Subject: [PATCH 08/19] WIP fix optimistic rate modified expense message --- src/libs/ReportUtils.ts | 8 +++++--- src/libs/actions/IOU.ts | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 5a30ba66161..0512864fbc9 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3415,6 +3415,7 @@ function getModifiedExpenseOriginalMessage( transactionChanges: TransactionChanges, isFromExpenseReport: boolean, policy: OnyxInputOrEntry, + updatedTransaction?: OnyxInputOrEntry, ): OriginalMessageModifiedExpense { const originalMessage: OriginalMessageModifiedExpense = {}; // Remark: Comment field is the only one which has new/old prefixes for the keys (newComment/ oldComment), @@ -3474,12 +3475,12 @@ function getModifiedExpenseOriginalMessage( originalMessage.billable = transactionChanges?.billable ? Localize.translateLocal('common.billable').toLowerCase() : Localize.translateLocal('common.nonBillable').toLowerCase(); } - if ('customUnitRateID' in transactionChanges) { + if ('customUnitRateID' in transactionChanges && updatedTransaction && 'customUnitRateID' in updatedTransaction) { originalMessage.oldAmount = TransactionUtils.getAmount(oldTransaction, isFromExpenseReport); originalMessage.oldCurrency = TransactionUtils.getCurrency(oldTransaction); originalMessage.oldMerchant = TransactionUtils.getMerchant(oldTransaction); - const modifiedDistanceFields = TransactionUtils.calculateAmountForUpdatedWaypointOrRate(oldTransaction, transactionChanges, policy, isFromExpenseReport); + const modifiedDistanceFields = TransactionUtils.calculateAmountForUpdatedWaypointOrRate(updatedTransaction, transactionChanges, policy, isFromExpenseReport); // For the originalMessage, we should use the non-negative amount, similar to what TransactionUtils.getAmount does for oldAmount originalMessage.amount = Math.abs(modifiedDistanceFields.modifiedAmount); @@ -4844,8 +4845,9 @@ function buildOptimisticModifiedExpenseReportAction( transactionChanges: TransactionChanges, isFromExpenseReport: boolean, policy: OnyxInputOrEntry, + updatedTransaction?: OnyxInputOrEntry, ): OptimisticModifiedExpenseReportAction { - const originalMessage = getModifiedExpenseOriginalMessage(oldTransaction, transactionChanges, isFromExpenseReport, policy); + const originalMessage = getModifiedExpenseOriginalMessage(oldTransaction, transactionChanges, isFromExpenseReport, policy, updatedTransaction); return { actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE, actorAccountID: currentUserAccountID, diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index e0704228cc6..161bfb7d9c9 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -2459,7 +2459,7 @@ function getUpdateMoneyRequestParams( const iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionThread?.parentReportID}`] ?? null; const isFromExpenseReport = ReportUtils.isExpenseReport(iouReport); const isScanning = TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction); - let updatedTransaction = transaction ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) : null; + let updatedTransaction: OnyxTypes.OnyxInputOrEntry = transaction ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) : null; const transactionDetails = ReportUtils.getTransactionDetails(updatedTransaction); if (transactionDetails?.waypoints) { @@ -2511,7 +2511,7 @@ function getUpdateMoneyRequestParams( // - we're updating the distance rate while the waypoints are still pending // In these cases, there isn't a valid optimistic mileage data we can use, // and the report action is created on the server with the distance-related response from the MapBox API - const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, transactionChanges, isFromExpenseReport, policy); + const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, transactionChanges, isFromExpenseReport, policy, updatedTransaction); if (!hasPendingWaypoints && !(hasModifiedDistanceRate && TransactionUtils.isFetchingWaypointsFromServer(transaction))) { params.reportActionID = updatedReportAction.reportActionID; From 517d0ba044afcfe8d03343d1c3fc9389b460150f Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 12:23:29 -0700 Subject: [PATCH 09/19] Fix optimistic modified rate in modified expense message --- 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 0512864fbc9..4af55256d87 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3475,7 +3475,7 @@ function getModifiedExpenseOriginalMessage( originalMessage.billable = transactionChanges?.billable ? Localize.translateLocal('common.billable').toLowerCase() : Localize.translateLocal('common.nonBillable').toLowerCase(); } - if ('customUnitRateID' in transactionChanges && updatedTransaction && 'customUnitRateID' in updatedTransaction) { + if ('customUnitRateID' in transactionChanges && (updatedTransaction?.comment?.customUnit?.customUnitRateID ?? '')) { originalMessage.oldAmount = TransactionUtils.getAmount(oldTransaction, isFromExpenseReport); originalMessage.oldCurrency = TransactionUtils.getCurrency(oldTransaction); originalMessage.oldMerchant = TransactionUtils.getMerchant(oldTransaction); From af551f3753c30c6cd6a2436ebfecef995d9e73b0 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 14:31:37 -0700 Subject: [PATCH 10/19] WIP modified distance or rate message matching backend --- src/languages/en.ts | 2 +- src/languages/es.ts | 4 +-- src/languages/types.ts | 2 +- src/libs/ModifiedExpenseMessage.ts | 56 +++++++++++++++++++++++++----- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/languages/en.ts b/src/languages/en.ts index 44435cdbf97..0af5edaf58c 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -840,7 +840,7 @@ export default { pendingConversionMessage: "Total will update when you're back online", changedTheExpense: 'changed the expense', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `the ${valueName} to ${newValueToDisplay}`, - setTheDistance: ({newDistanceToDisplay, newAmountToDisplay}: SetTheDistanceParams) => `set the distance to ${newDistanceToDisplay}, which set the amount to ${newAmountToDisplay}`, + setTheDistance: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceParams) => `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `the ${valueName} (previously ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `the ${valueName} to ${newValueToDisplay} (previously ${oldValueToDisplay})`, updatedTheDistance: ({newDistanceToDisplay, oldDistanceToDisplay, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => diff --git a/src/languages/es.ts b/src/languages/es.ts index 0b1313c316c..ca107ebeac7 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -833,8 +833,8 @@ export default { pendingConversionMessage: 'El total se actualizará cuando estés online', changedTheExpense: 'cambió el gasto', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay}`, - setTheDistance: ({newDistanceToDisplay, newAmountToDisplay}: SetTheDistanceParams) => - `estableció la distancia a ${newDistanceToDisplay}, lo que estableció el importe a ${newAmountToDisplay}`, + setTheDistance: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceParams) => + `estableció la ${changedField} a ${newMerchant}, lo que estableció el importe a ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} (previamente ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `${valueName === 'comerciante' || valueName === 'importe' || valueName === 'gasto' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay} (previamente ${oldValueToDisplay})`, diff --git a/src/languages/types.ts b/src/languages/types.ts index b79eb033213..b714bd3011b 100644 --- a/src/languages/types.ts +++ b/src/languages/types.ts @@ -203,7 +203,7 @@ type ParentNavigationSummaryParams = {reportName?: string; workspaceName?: strin type SetTheRequestParams = {valueName: string; newValueToDisplay: string}; -type SetTheDistanceParams = {newDistanceToDisplay: string; newAmountToDisplay: string}; +type SetTheDistanceParams = {changedField: string; newMerchant: string; newAmountToDisplay: string}; type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string}; diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index 798bc74869e..3d883ef8856 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -10,6 +10,7 @@ import * as PolicyUtils from './PolicyUtils'; import * as ReportActionsUtils from './ReportActionsUtils'; import * as ReportConnection from './ReportConnection'; import * as TransactionUtils from './TransactionUtils'; +import Log from './Log'; let allPolicyTags: OnyxCollection = {}; Onyx.connect({ @@ -94,16 +95,53 @@ function getMessageLine(prefix: string, messageFragments: string[]): string { }, prefix); } -function getForDistanceRequest(newDistance: string, oldDistance: string, newAmount: string, oldAmount: string): string { - if (!oldDistance) { - return Localize.translateLocal('iou.setTheDistance', {newDistanceToDisplay: newDistance, newAmountToDisplay: newAmount}); +// function getForDistanceRequest(newDistance: string, oldDistance: string, newAmount: string, oldAmount: string): string { +// // If it doesn't match the regex for distance, we should just return the default message +// if (!oldDistance) { +// return Localize.translateLocal('iou.setTheDistance', {newDistanceToDisplay: newDistance, newAmountToDisplay: newAmount}); +// } + +// // The new translate function should take in changedField, newMerchant, oldMerchant, newAmount, oldAmount + +// // return "set the $changedField to $newMerchant, which set the amount to $newAmount"; +// // } +// // return "changed the $changedField to $newMerchant (previously $oldMerchant), which updated the amount to $newAmount (previously $oldAmount)"; + + +// return Localize.translateLocal('iou.updatedTheDistance', { +// newDistanceToDisplay: newDistance, +// oldDistanceToDisplay: oldDistance, +// newAmountToDisplay: newAmount, +// oldAmountToDisplay: oldAmount, +// }); +// } + + +function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmount: string, oldAmount: string): string { + const distanceMerchantRegex = /^[0-9.]+ \w+ @ (-|-\()?(\p{Sc}|\w){1,3} ?[0-9.]+\)? \/ \w+$/; + let changedField: 'distance' | 'rate' = 'distance'; + + if (distanceMerchantRegex.test(newMerchant) && distanceMerchantRegex.test(oldMerchant)) { + const oldValues = oldMerchant.split('@'); + const oldDistance = oldValues[0]?.trim() || ''; + const oldRate = oldValues[1]?.trim() || ''; + const newValues = newMerchant.split('@'); + const newDistance = newValues[0]?.trim() || ''; + const newRate = newValues[1]?.trim() || ''; + + if (oldDistance === newDistance && oldRate !== newRate) { + changedField = 'rate'; + } + } else { + Log.hmmm("Distance request merchant doesn't match NewDot format. Defaulting to showing as distance changed.", {newMerchant, oldMerchant}); + } + const translatedChangedField = Localize.translateLocal(`common.${changedField}`); + + if (!oldMerchant.length) { + // return `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmount}`; + return Localize.translateLocal('iou.setTheDistance', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); } - return Localize.translateLocal('iou.updatedTheDistance', { - newDistanceToDisplay: newDistance, - oldDistanceToDisplay: oldDistance, - newAmountToDisplay: newAmount, - oldAmountToDisplay: oldAmount, - }); + return `changed the ${changedField} to ${newMerchant} (previously ${oldMerchant}), which updated the amount to ${newAmount} (previously ${oldAmount})`; } /** From 32c73a95daceb333ad0f3ea30f898c192f54ae80 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 14:38:26 -0700 Subject: [PATCH 11/19] Implement updated the distance translations --- src/languages/en.ts | 4 ++-- src/languages/es.ts | 4 ++-- src/languages/types.ts | 2 +- src/libs/ModifiedExpenseMessage.ts | 25 +------------------------ 4 files changed, 6 insertions(+), 29 deletions(-) diff --git a/src/languages/en.ts b/src/languages/en.ts index 0af5edaf58c..a9c69d49d38 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -843,8 +843,8 @@ export default { setTheDistance: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceParams) => `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `the ${valueName} (previously ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `the ${valueName} to ${newValueToDisplay} (previously ${oldValueToDisplay})`, - updatedTheDistance: ({newDistanceToDisplay, oldDistanceToDisplay, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => - `changed the distance to ${newDistanceToDisplay} (previously ${oldDistanceToDisplay}), which updated the amount to ${newAmountToDisplay} (previously ${oldAmountToDisplay})`, + updatedTheDistance: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => + `changed the ${changedField} to ${newMerchant} (previously ${oldMerchant}), which updated the amount to ${newAmountToDisplay} (previously ${oldAmountToDisplay})`, threadExpenseReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `${formattedAmount} ${comment ? `for ${comment}` : 'expense'}`, threadTrackReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `Tracking ${formattedAmount} ${comment ? `for ${comment}` : ''}`, threadPaySomeoneReportName: ({formattedAmount, comment}: ThreadSentMoneyReportNameParams) => `${formattedAmount} sent${comment ? ` for ${comment}` : ''}`, diff --git a/src/languages/es.ts b/src/languages/es.ts index ca107ebeac7..70fab9c589e 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -838,8 +838,8 @@ export default { removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} (previamente ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `${valueName === 'comerciante' || valueName === 'importe' || valueName === 'gasto' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay} (previamente ${oldValueToDisplay})`, - updatedTheDistance: ({newDistanceToDisplay, oldDistanceToDisplay, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => - `cambió la distancia a ${newDistanceToDisplay} (previamente ${oldDistanceToDisplay}), lo que cambió el importe a ${newAmountToDisplay} (previamente ${oldAmountToDisplay})`, + updatedTheDistance: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => + `cambió la ${changedField} a ${newMerchant} (previamente ${oldMerchant}), lo que cambió el importe a ${newAmountToDisplay} (previamente ${oldAmountToDisplay})`, threadExpenseReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `${comment ? `${formattedAmount} para ${comment}` : `Gasto de ${formattedAmount}`}`, threadTrackReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `Seguimiento ${formattedAmount} ${comment ? `para ${comment}` : ''}`, threadPaySomeoneReportName: ({formattedAmount, comment}: ThreadSentMoneyReportNameParams) => `${formattedAmount} enviado${comment ? ` para ${comment}` : ''}`, diff --git a/src/languages/types.ts b/src/languages/types.ts index b714bd3011b..f92dd7aa309 100644 --- a/src/languages/types.ts +++ b/src/languages/types.ts @@ -209,7 +209,7 @@ type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string}; type UpdatedTheRequestParams = {valueName: string; newValueToDisplay: string; oldValueToDisplay: string}; -type UpdatedTheDistanceParams = {newDistanceToDisplay: string; oldDistanceToDisplay: string; newAmountToDisplay: string; oldAmountToDisplay: string}; +type UpdatedTheDistanceParams = {changedField: string; newMerchant: string; oldMerchant: string; newAmountToDisplay: string; oldAmountToDisplay: string}; type FormattedMaxLengthParams = {formattedMaxLength: string}; diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index 3d883ef8856..e3c8276eb34 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -95,28 +95,6 @@ function getMessageLine(prefix: string, messageFragments: string[]): string { }, prefix); } -// function getForDistanceRequest(newDistance: string, oldDistance: string, newAmount: string, oldAmount: string): string { -// // If it doesn't match the regex for distance, we should just return the default message -// if (!oldDistance) { -// return Localize.translateLocal('iou.setTheDistance', {newDistanceToDisplay: newDistance, newAmountToDisplay: newAmount}); -// } - -// // The new translate function should take in changedField, newMerchant, oldMerchant, newAmount, oldAmount - -// // return "set the $changedField to $newMerchant, which set the amount to $newAmount"; -// // } -// // return "changed the $changedField to $newMerchant (previously $oldMerchant), which updated the amount to $newAmount (previously $oldAmount)"; - - -// return Localize.translateLocal('iou.updatedTheDistance', { -// newDistanceToDisplay: newDistance, -// oldDistanceToDisplay: oldDistance, -// newAmountToDisplay: newAmount, -// oldAmountToDisplay: oldAmount, -// }); -// } - - function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmount: string, oldAmount: string): string { const distanceMerchantRegex = /^[0-9.]+ \w+ @ (-|-\()?(\p{Sc}|\w){1,3} ?[0-9.]+\)? \/ \w+$/; let changedField: 'distance' | 'rate' = 'distance'; @@ -138,10 +116,9 @@ function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmou const translatedChangedField = Localize.translateLocal(`common.${changedField}`); if (!oldMerchant.length) { - // return `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmount}`; return Localize.translateLocal('iou.setTheDistance', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); } - return `changed the ${changedField} to ${newMerchant} (previously ${oldMerchant}), which updated the amount to ${newAmount} (previously ${oldAmount})`; + return Localize.translateLocal('iou.updatedTheDistance', {changedField: translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay: newAmount, oldAmountToDisplay: oldAmount}); } /** From f994254974364cbd2c9a2ea607faf355c1f675a9 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 14:42:33 -0700 Subject: [PATCH 12/19] The field is the merchant specific to distance expenses --- src/languages/en.ts | 8 ++++---- src/languages/es.ts | 8 ++++---- src/languages/types.ts | 8 ++++---- src/libs/ModifiedExpenseMessage.ts | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/languages/en.ts b/src/languages/en.ts index a9c69d49d38..81c5d508fdd 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -77,7 +77,7 @@ import type { ResolutionConstraintsParams, RoomNameReservedErrorParams, RoomRenamedToParams, - SetTheDistanceParams, + SetTheDistanceMerchantParams, SetTheRequestParams, SettledAfterAddedBankAccountParams, SettleExpensifyCardParams, @@ -97,7 +97,7 @@ import type { UnapprovedParams, UnshareParams, UntilTimeParams, - UpdatedTheDistanceParams, + UpdatedTheDistanceMerchantParams, UpdatedTheRequestParams, UsePlusButtonParams, UserIsAlreadyMemberParams, @@ -840,10 +840,10 @@ export default { pendingConversionMessage: "Total will update when you're back online", changedTheExpense: 'changed the expense', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `the ${valueName} to ${newValueToDisplay}`, - setTheDistance: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceParams) => `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, + setTheDistanceMerchant: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `the ${valueName} (previously ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `the ${valueName} to ${newValueToDisplay} (previously ${oldValueToDisplay})`, - updatedTheDistance: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => + updatedTheDistanceMerchant: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => `changed the ${changedField} to ${newMerchant} (previously ${oldMerchant}), which updated the amount to ${newAmountToDisplay} (previously ${oldAmountToDisplay})`, threadExpenseReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `${formattedAmount} ${comment ? `for ${comment}` : 'expense'}`, threadTrackReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `Tracking ${formattedAmount} ${comment ? `for ${comment}` : ''}`, diff --git a/src/languages/es.ts b/src/languages/es.ts index 70fab9c589e..74aae166b9f 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -76,7 +76,7 @@ import type { ResolutionConstraintsParams, RoomNameReservedErrorParams, RoomRenamedToParams, - SetTheDistanceParams, + SetTheDistanceMerchantParams, SetTheRequestParams, SettledAfterAddedBankAccountParams, SettleExpensifyCardParams, @@ -95,7 +95,7 @@ import type { UnapprovedParams, UnshareParams, UntilTimeParams, - UpdatedTheDistanceParams, + UpdatedTheDistanceMerchantParams, UpdatedTheRequestParams, UsePlusButtonParams, UserIsAlreadyMemberParams, @@ -833,12 +833,12 @@ export default { pendingConversionMessage: 'El total se actualizará cuando estés online', changedTheExpense: 'cambió el gasto', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay}`, - setTheDistance: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceParams) => + setTheDistanceMerchant: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => `estableció la ${changedField} a ${newMerchant}, lo que estableció el importe a ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} (previamente ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `${valueName === 'comerciante' || valueName === 'importe' || valueName === 'gasto' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay} (previamente ${oldValueToDisplay})`, - updatedTheDistance: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceParams) => + updatedTheDistanceMerchant: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => `cambió la ${changedField} a ${newMerchant} (previamente ${oldMerchant}), lo que cambió el importe a ${newAmountToDisplay} (previamente ${oldAmountToDisplay})`, threadExpenseReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `${comment ? `${formattedAmount} para ${comment}` : `Gasto de ${formattedAmount}`}`, threadTrackReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `Seguimiento ${formattedAmount} ${comment ? `para ${comment}` : ''}`, diff --git a/src/languages/types.ts b/src/languages/types.ts index f92dd7aa309..d5d256d359f 100644 --- a/src/languages/types.ts +++ b/src/languages/types.ts @@ -203,13 +203,13 @@ type ParentNavigationSummaryParams = {reportName?: string; workspaceName?: strin type SetTheRequestParams = {valueName: string; newValueToDisplay: string}; -type SetTheDistanceParams = {changedField: string; newMerchant: string; newAmountToDisplay: string}; +type SetTheDistanceMerchantParams = {changedField: string; newMerchant: string; newAmountToDisplay: string}; type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string}; type UpdatedTheRequestParams = {valueName: string; newValueToDisplay: string; oldValueToDisplay: string}; -type UpdatedTheDistanceParams = {changedField: string; newMerchant: string; oldMerchant: string; newAmountToDisplay: string; oldAmountToDisplay: string}; +type UpdatedTheDistanceMerchantParams = {changedField: string; newMerchant: string; oldMerchant: string; newAmountToDisplay: string; oldAmountToDisplay: string}; type FormattedMaxLengthParams = {formattedMaxLength: string}; @@ -430,7 +430,7 @@ export type { ResolutionConstraintsParams, RoomNameReservedErrorParams, RoomRenamedToParams, - SetTheDistanceParams, + SetTheDistanceMerchantParams, SetTheRequestParams, SettleExpensifyCardParams, SettledAfterAddedBankAccountParams, @@ -447,7 +447,7 @@ export type { TranslationFlatObject, TranslationPaths, UntilTimeParams, - UpdatedTheDistanceParams, + UpdatedTheDistanceMerchantParams, UpdatedTheRequestParams, UsePlusButtonParams, UserIsAlreadyMemberParams, diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index e3c8276eb34..142df4fdd75 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -116,9 +116,9 @@ function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmou const translatedChangedField = Localize.translateLocal(`common.${changedField}`); if (!oldMerchant.length) { - return Localize.translateLocal('iou.setTheDistance', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); + return Localize.translateLocal('iou.setTheDistanceMerchant', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); } - return Localize.translateLocal('iou.updatedTheDistance', {changedField: translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay: newAmount, oldAmountToDisplay: oldAmount}); + return Localize.translateLocal('iou.updatedTheDistanceMerchant', {changedField: translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay: newAmount, oldAmountToDisplay: oldAmount}); } /** From 967460b715692d2ac6a97a5a67fc484bbcd15516 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 15:16:55 -0700 Subject: [PATCH 13/19] Make sure translatedChangedField is lowercase --- src/libs/ModifiedExpenseMessage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index 142df4fdd75..eb7f694ee33 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -113,7 +113,7 @@ function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmou } else { Log.hmmm("Distance request merchant doesn't match NewDot format. Defaulting to showing as distance changed.", {newMerchant, oldMerchant}); } - const translatedChangedField = Localize.translateLocal(`common.${changedField}`); + const translatedChangedField = Localize.translateLocal(`common.${changedField}`).toLowerCase(); if (!oldMerchant.length) { return Localize.translateLocal('iou.setTheDistanceMerchant', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); From 929e6872dc421102413c8358a4610ba9444eb1e5 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 16:04:19 -0700 Subject: [PATCH 14/19] Fix matching currency symbols in distance merchant --- src/libs/ModifiedExpenseMessage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index eb7f694ee33..c658bb0976a 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -96,7 +96,7 @@ function getMessageLine(prefix: string, messageFragments: string[]): string { } function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmount: string, oldAmount: string): string { - const distanceMerchantRegex = /^[0-9.]+ \w+ @ (-|-\()?(\p{Sc}|\w){1,3} ?[0-9.]+\)? \/ \w+$/; + const distanceMerchantRegex = /^[0-9.]+ \w+ @ (-|-\()?[^0-9.\s]{1,3} ?[0-9.]+\)? \/ \w+$/; let changedField: 'distance' | 'rate' = 'distance'; if (distanceMerchantRegex.test(newMerchant) && distanceMerchantRegex.test(oldMerchant)) { From 3b944bf81dbffd4b34e81afcd1969a4faf8cecfb Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 20:36:05 -0700 Subject: [PATCH 15/19] Fix style and clean up --- src/languages/en.ts | 3 ++- src/libs/ModifiedExpenseMessage.ts | 10 ++++++++-- src/libs/TransactionUtils/index.ts | 1 - src/libs/actions/IOU.ts | 4 +++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/languages/en.ts b/src/languages/en.ts index 81c5d508fdd..b220f2aa7d4 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -840,7 +840,8 @@ export default { pendingConversionMessage: "Total will update when you're back online", changedTheExpense: 'changed the expense', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `the ${valueName} to ${newValueToDisplay}`, - setTheDistanceMerchant: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, + setTheDistanceMerchant: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => + `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `the ${valueName} (previously ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `the ${valueName} to ${newValueToDisplay} (previously ${oldValueToDisplay})`, updatedTheDistanceMerchant: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index c658bb0976a..6bef126cb91 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -6,11 +6,11 @@ import type {PolicyTagLists, ReportAction} from '@src/types/onyx'; import * as CurrencyUtils from './CurrencyUtils'; import DateUtils from './DateUtils'; import * as Localize from './Localize'; +import Log from './Log'; import * as PolicyUtils from './PolicyUtils'; import * as ReportActionsUtils from './ReportActionsUtils'; import * as ReportConnection from './ReportConnection'; import * as TransactionUtils from './TransactionUtils'; -import Log from './Log'; let allPolicyTags: OnyxCollection = {}; Onyx.connect({ @@ -118,7 +118,13 @@ function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmou if (!oldMerchant.length) { return Localize.translateLocal('iou.setTheDistanceMerchant', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); } - return Localize.translateLocal('iou.updatedTheDistanceMerchant', {changedField: translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay: newAmount, oldAmountToDisplay: oldAmount}); + return Localize.translateLocal('iou.updatedTheDistanceMerchant', { + changedField: translatedChangedField, + newMerchant, + oldMerchant, + newAmountToDisplay: newAmount, + oldAmountToDisplay: oldAmount, + }); } /** diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index f80c14c80c7..f9d2e34f850 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -771,7 +771,6 @@ function calculateAmountForUpdatedWaypointOrRate( ) { const hasModifiedRouteWithPendingWaypoints = !isEmptyObject(transactionChanges.waypoints) && isEmptyObject(transactionChanges?.routes?.route0?.geometry); const hasModifiedRateWithPendingWaypoints = !!transactionChanges?.customUnitRateID && isFetchingWaypointsFromServer(transaction); - console.log('Ndebug hasModifiedRateWithPendingWaypoints', hasModifiedRateWithPendingWaypoints); if (hasModifiedRouteWithPendingWaypoints || hasModifiedRateWithPendingWaypoints) { return { amount: CONST.IOU.DEFAULT_AMOUNT, diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 161bfb7d9c9..fea41a7f9ae 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -2459,7 +2459,9 @@ function getUpdateMoneyRequestParams( const iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionThread?.parentReportID}`] ?? null; const isFromExpenseReport = ReportUtils.isExpenseReport(iouReport); const isScanning = TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction); - let updatedTransaction: OnyxTypes.OnyxInputOrEntry = transaction ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) : null; + let updatedTransaction: OnyxTypes.OnyxInputOrEntry = transaction + ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) + : null; const transactionDetails = ReportUtils.getTransactionDetails(updatedTransaction); if (transactionDetails?.waypoints) { From 1f4c3487e9fa39e8b1f6f036e627d37c3958ad59 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 20:45:11 -0700 Subject: [PATCH 16/19] Clarify that changedField must be translated --- src/languages/en.ts | 8 ++++---- src/languages/es.ts | 8 ++++---- src/languages/types.ts | 4 ++-- src/libs/ModifiedExpenseMessage.ts | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/languages/en.ts b/src/languages/en.ts index b220f2aa7d4..1c75cb7307a 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -840,12 +840,12 @@ export default { pendingConversionMessage: "Total will update when you're back online", changedTheExpense: 'changed the expense', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `the ${valueName} to ${newValueToDisplay}`, - setTheDistanceMerchant: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => - `set the ${changedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, + setTheDistanceMerchant: ({translatedChangedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => + `set the ${translatedChangedField} to ${newMerchant}, which set the amount to ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `the ${valueName} (previously ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `the ${valueName} to ${newValueToDisplay} (previously ${oldValueToDisplay})`, - updatedTheDistanceMerchant: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => - `changed the ${changedField} to ${newMerchant} (previously ${oldMerchant}), which updated the amount to ${newAmountToDisplay} (previously ${oldAmountToDisplay})`, + updatedTheDistanceMerchant: ({translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => + `changed the ${translatedChangedField} to ${newMerchant} (previously ${oldMerchant}), which updated the amount to ${newAmountToDisplay} (previously ${oldAmountToDisplay})`, threadExpenseReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `${formattedAmount} ${comment ? `for ${comment}` : 'expense'}`, threadTrackReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `Tracking ${formattedAmount} ${comment ? `for ${comment}` : ''}`, threadPaySomeoneReportName: ({formattedAmount, comment}: ThreadSentMoneyReportNameParams) => `${formattedAmount} sent${comment ? ` for ${comment}` : ''}`, diff --git a/src/languages/es.ts b/src/languages/es.ts index 74aae166b9f..4363ac2de18 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -833,13 +833,13 @@ export default { pendingConversionMessage: 'El total se actualizará cuando estés online', changedTheExpense: 'cambió el gasto', setTheRequest: ({valueName, newValueToDisplay}: SetTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay}`, - setTheDistanceMerchant: ({changedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => - `estableció la ${changedField} a ${newMerchant}, lo que estableció el importe a ${newAmountToDisplay}`, + setTheDistanceMerchant: ({translatedChangedField, newMerchant, newAmountToDisplay}: SetTheDistanceMerchantParams) => + `estableció la ${translatedChangedField} a ${newMerchant}, lo que estableció el importe a ${newAmountToDisplay}`, removedTheRequest: ({valueName, oldValueToDisplay}: RemovedTheRequestParams) => `${valueName === 'comerciante' ? 'el' : 'la'} ${valueName} (previamente ${oldValueToDisplay})`, updatedTheRequest: ({valueName, newValueToDisplay, oldValueToDisplay}: UpdatedTheRequestParams) => `${valueName === 'comerciante' || valueName === 'importe' || valueName === 'gasto' ? 'el' : 'la'} ${valueName} a ${newValueToDisplay} (previamente ${oldValueToDisplay})`, - updatedTheDistanceMerchant: ({changedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => - `cambió la ${changedField} a ${newMerchant} (previamente ${oldMerchant}), lo que cambió el importe a ${newAmountToDisplay} (previamente ${oldAmountToDisplay})`, + updatedTheDistanceMerchant: ({translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay, oldAmountToDisplay}: UpdatedTheDistanceMerchantParams) => + `cambió la ${translatedChangedField} a ${newMerchant} (previamente ${oldMerchant}), lo que cambió el importe a ${newAmountToDisplay} (previamente ${oldAmountToDisplay})`, threadExpenseReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `${comment ? `${formattedAmount} para ${comment}` : `Gasto de ${formattedAmount}`}`, threadTrackReportName: ({formattedAmount, comment}: ThreadRequestReportNameParams) => `Seguimiento ${formattedAmount} ${comment ? `para ${comment}` : ''}`, threadPaySomeoneReportName: ({formattedAmount, comment}: ThreadSentMoneyReportNameParams) => `${formattedAmount} enviado${comment ? ` para ${comment}` : ''}`, diff --git a/src/languages/types.ts b/src/languages/types.ts index d5d256d359f..993b1dff92c 100644 --- a/src/languages/types.ts +++ b/src/languages/types.ts @@ -203,13 +203,13 @@ type ParentNavigationSummaryParams = {reportName?: string; workspaceName?: strin type SetTheRequestParams = {valueName: string; newValueToDisplay: string}; -type SetTheDistanceMerchantParams = {changedField: string; newMerchant: string; newAmountToDisplay: string}; +type SetTheDistanceMerchantParams = {translatedChangedField: string; newMerchant: string; newAmountToDisplay: string}; type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string}; type UpdatedTheRequestParams = {valueName: string; newValueToDisplay: string; oldValueToDisplay: string}; -type UpdatedTheDistanceMerchantParams = {changedField: string; newMerchant: string; oldMerchant: string; newAmountToDisplay: string; oldAmountToDisplay: string}; +type UpdatedTheDistanceMerchantParams = {translatedChangedField: string; newMerchant: string; oldMerchant: string; newAmountToDisplay: string; oldAmountToDisplay: string}; type FormattedMaxLengthParams = {formattedMaxLength: string}; diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index 6bef126cb91..e07e4e79297 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -113,13 +113,13 @@ function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmou } else { Log.hmmm("Distance request merchant doesn't match NewDot format. Defaulting to showing as distance changed.", {newMerchant, oldMerchant}); } - const translatedChangedField = Localize.translateLocal(`common.${changedField}`).toLowerCase(); + const translatedChangedField = Localize.translateLocal(`common.${changedField}`).toLowerCase(); if (!oldMerchant.length) { - return Localize.translateLocal('iou.setTheDistanceMerchant', {changedField: translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); + return Localize.translateLocal('iou.setTheDistanceMerchant', {translatedChangedField, newMerchant, newAmountToDisplay: newAmount}); } return Localize.translateLocal('iou.updatedTheDistanceMerchant', { - changedField: translatedChangedField, + translatedChangedField, newMerchant, oldMerchant, newAmountToDisplay: newAmount, From dd14a19bfa69685f0c05941e1834e9524d43033f Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 20:54:53 -0700 Subject: [PATCH 17/19] Move distance merchant regex to CONST --- src/CONST.ts | 1 + src/libs/ModifiedExpenseMessage.ts | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 42fe8062866..dce0c8e2300 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -2467,6 +2467,7 @@ const CONST = { SHORT_MENTION: new RegExp(`@[\\w\\-\\+\\'#@]+(?:\\.[\\w\\-\\'\\+]+)*(?![^\`]*\`)`, 'gim'), REPORT_ID_FROM_PATH: /\/r\/(\d+)/, + DISTANCE_MERCHANT: /^[0-9.]+ \w+ @ (-|-\()?[^0-9.\s]{1,3} ?[0-9.]+\)? \/ \w+$/, }, PRONOUNS: { diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts index e07e4e79297..99f483cf6ef 100644 --- a/src/libs/ModifiedExpenseMessage.ts +++ b/src/libs/ModifiedExpenseMessage.ts @@ -96,10 +96,9 @@ function getMessageLine(prefix: string, messageFragments: string[]): string { } function getForDistanceRequest(newMerchant: string, oldMerchant: string, newAmount: string, oldAmount: string): string { - const distanceMerchantRegex = /^[0-9.]+ \w+ @ (-|-\()?[^0-9.\s]{1,3} ?[0-9.]+\)? \/ \w+$/; let changedField: 'distance' | 'rate' = 'distance'; - if (distanceMerchantRegex.test(newMerchant) && distanceMerchantRegex.test(oldMerchant)) { + if (CONST.REGEX.DISTANCE_MERCHANT.test(newMerchant) && CONST.REGEX.DISTANCE_MERCHANT.test(oldMerchant)) { const oldValues = oldMerchant.split('@'); const oldDistance = oldValues[0]?.trim() || ''; const oldRate = oldValues[1]?.trim() || ''; From d454c2f496479778d8e165a7ec68fdbc1569bf24 Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 21:11:05 -0700 Subject: [PATCH 18/19] Small cleanup and test distance rate message --- src/libs/ReportUtils.ts | 2 +- tests/unit/ModifiedExpenseMessageTest.ts | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 4af55256d87..4488b56641f 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3475,7 +3475,7 @@ function getModifiedExpenseOriginalMessage( originalMessage.billable = transactionChanges?.billable ? Localize.translateLocal('common.billable').toLowerCase() : Localize.translateLocal('common.nonBillable').toLowerCase(); } - if ('customUnitRateID' in transactionChanges && (updatedTransaction?.comment?.customUnit?.customUnitRateID ?? '')) { + if ('customUnitRateID' in transactionChanges && updatedTransaction?.comment?.customUnit?.customUnitRateID) { originalMessage.oldAmount = TransactionUtils.getAmount(oldTransaction, isFromExpenseReport); originalMessage.oldCurrency = TransactionUtils.getCurrency(oldTransaction); originalMessage.oldMerchant = TransactionUtils.getMerchant(oldTransaction); diff --git a/tests/unit/ModifiedExpenseMessageTest.ts b/tests/unit/ModifiedExpenseMessageTest.ts index d7fe10bc050..d93e2421af4 100644 --- a/tests/unit/ModifiedExpenseMessageTest.ts +++ b/tests/unit/ModifiedExpenseMessageTest.ts @@ -354,5 +354,26 @@ describe('ModifiedExpenseMessage', () => { expect(result).toEqual(expectedResult); }); }); + + describe('when the distance rate is changed', () => { + const reportAction = { + ...createRandomReportAction(1), + actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE, + originalMessage: { + oldMerchant: '56.36 mi @ $0.67 / mi', + merchant: '56.36 mi @ $0.99 / mi', + oldAmount: 3776, + amount: 5580, + oldCurrency: CONST.CURRENCY.USD, + currency: CONST.CURRENCY.USD, + }, + }; + + it('then the message says the rate is changed and shows the new and old merchant and amount', () => { + const expectedResult = `changed the rate to 56.36 mi @ $0.99 / mi (previously 56.36 mi @ $0.67 / mi), which updated the amount to $55.80 (previously $37.76)`; + const result = ModifiedExpenseMessage.getForReportAction(report.reportID, reportAction); + expect(result).toEqual(expectedResult); + }); + }); }); }); From 0461782acafc609aaad31b51c1db25abf1a9b4bb Mon Sep 17 00:00:00 2001 From: neil-marcellini Date: Mon, 9 Sep 2024 21:16:46 -0700 Subject: [PATCH 19/19] Test modified expense for distance changed --- tests/unit/ModifiedExpenseMessageTest.ts | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/unit/ModifiedExpenseMessageTest.ts b/tests/unit/ModifiedExpenseMessageTest.ts index d93e2421af4..63263d67a9b 100644 --- a/tests/unit/ModifiedExpenseMessageTest.ts +++ b/tests/unit/ModifiedExpenseMessageTest.ts @@ -355,6 +355,27 @@ describe('ModifiedExpenseMessage', () => { }); }); + describe('when the distance is changed', () => { + const reportAction = { + ...createRandomReportAction(1), + actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE, + originalMessage: { + oldMerchant: '1.00 mi @ $0.67 / mi', + merchant: '10.00 mi @ $0.67 / mi', + oldAmount: 67, + amount: 670, + oldCurrency: CONST.CURRENCY.USD, + currency: CONST.CURRENCY.USD, + }, + }; + + it('then the message says the distance is changed and shows the new and old merchant and amount', () => { + const expectedResult = `changed the distance to ${reportAction.originalMessage.merchant} (previously ${reportAction.originalMessage.oldMerchant}), which updated the amount to $6.70 (previously $0.67)`; + const result = ModifiedExpenseMessage.getForReportAction(report.reportID, reportAction); + expect(result).toEqual(expectedResult); + }); + }); + describe('when the distance rate is changed', () => { const reportAction = { ...createRandomReportAction(1), @@ -370,7 +391,7 @@ describe('ModifiedExpenseMessage', () => { }; it('then the message says the rate is changed and shows the new and old merchant and amount', () => { - const expectedResult = `changed the rate to 56.36 mi @ $0.99 / mi (previously 56.36 mi @ $0.67 / mi), which updated the amount to $55.80 (previously $37.76)`; + const expectedResult = `changed the rate to ${reportAction.originalMessage.merchant} (previously ${reportAction.originalMessage.oldMerchant}), which updated the amount to $55.80 (previously $37.76)`; const result = ModifiedExpenseMessage.getForReportAction(report.reportID, reportAction); expect(result).toEqual(expectedResult); });