From 4e2234aa3f1b5f1f4629c421dcecb2b7d791a449 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Sun, 18 Aug 2024 02:45:37 +0530 Subject: [PATCH 1/7] fix: [P2P Distance] - Distance unit in the system message changes from miles to mi after changing rate. Signed-off-by: krishna2323 --- src/libs/DistanceRequestUtils.ts | 21 +++++++++++++++------ src/libs/PolicyUtils.ts | 11 ++++++++--- src/libs/TransactionUtils/index.ts | 11 +++++++++-- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/libs/DistanceRequestUtils.ts b/src/libs/DistanceRequestUtils.ts index fa98cf32ee39..01e310b82ed6 100644 --- a/src/libs/DistanceRequestUtils.ts +++ b/src/libs/DistanceRequestUtils.ts @@ -137,6 +137,7 @@ function getRateForDisplay( translate: LocaleContextProps['translate'], toLocaleDigit: LocaleContextProps['toLocaleDigit'], isOffline?: boolean, + useShortFormUnit?: boolean, ): string { if (isOffline && !rate) { return translate('iou.defaultRate'); @@ -146,11 +147,11 @@ function getRateForDisplay( } const singularDistanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.mile') : translate('common.kilometer'); - const formattedRate = PolicyUtils.getUnitRateValue(toLocaleDigit, {rate}); + const formattedRate = PolicyUtils.getUnitRateValue(toLocaleDigit, {rate}, useShortFormUnit); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const currencySymbol = CurrencyUtils.getCurrencySymbol(currency) || `${currency} `; - return `${currencySymbol}${formattedRate} / ${singularDistanceUnit}`; + return `${currencySymbol}${formattedRate} / ${useShortFormUnit ? unit : singularDistanceUnit}`; } /** @@ -161,7 +162,14 @@ function getRateForDisplay( * @param translate Translate function * @returns A string that describes the distance traveled */ -function getDistanceForDisplay(hasRoute: boolean, distanceInMeters: number, unit: Unit | undefined, rate: number | undefined, translate: LocaleContextProps['translate']): string { +function getDistanceForDisplay( + hasRoute: boolean, + distanceInMeters: number, + unit: Unit | undefined, + rate: number | undefined, + translate: LocaleContextProps['translate'], + useShortFormUnit?: boolean, +): string { if (!hasRoute || !rate || !unit || !distanceInMeters) { return translate('iou.fieldPending'); } @@ -171,7 +179,7 @@ function getDistanceForDisplay(hasRoute: boolean, distanceInMeters: number, unit const singularDistanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.mile') : translate('common.kilometer'); const unitString = distanceInUnits === '1' ? singularDistanceUnit : distanceUnit; - return `${distanceInUnits} ${unitString}`; + return `${distanceInUnits} ${useShortFormUnit ? unit : unitString}`; } /** @@ -192,13 +200,14 @@ function getDistanceMerchant( currency: string, translate: LocaleContextProps['translate'], toLocaleDigit: LocaleContextProps['toLocaleDigit'], + useShortFormUnit?: boolean, ): string { if (!hasRoute || !rate) { return translate('iou.fieldPending'); } - const distanceInUnits = getDistanceForDisplay(hasRoute, distanceInMeters, unit, rate, translate); - const ratePerUnit = getRateForDisplay(unit, rate, currency, translate, toLocaleDigit); + const distanceInUnits = getDistanceForDisplay(hasRoute, distanceInMeters, unit, rate, translate, useShortFormUnit); + const ratePerUnit = getRateForDisplay(unit, rate, currency, translate, toLocaleDigit, undefined, useShortFormUnit); return `${distanceInUnits} @ ${ratePerUnit}`; } diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 3f3a2a96a1e1..c5629f3751c5 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -137,16 +137,21 @@ function getCustomUnitRate(policy: OnyxEntry, customUnitRateID: string): return distanceUnit?.rates[customUnitRateID]; } -function getRateDisplayValue(value: number, toLocaleDigit: (arg: string) => string): string { +function getRateDisplayValue(value: number, toLocaleDigit: (arg: string) => string, withDecimals?: boolean): string { const numValue = getNumericValue(value, toLocaleDigit); if (Number.isNaN(numValue)) { return ''; } + + if (!Number.isNaN(Number(numValue)) && withDecimals) { + return Number(numValue).toFixed(2).toString().replace('.', toLocaleDigit('.')); + } + return numValue.toString().replace('.', toLocaleDigit('.')).substring(0, value.toString().length); } -function getUnitRateValue(toLocaleDigit: (arg: string) => string, customUnitRate?: Rate) { - return getRateDisplayValue((customUnitRate?.rate ?? 0) / CONST.POLICY.CUSTOM_UNIT_RATE_BASE_OFFSET, toLocaleDigit); +function getUnitRateValue(toLocaleDigit: (arg: string) => string, customUnitRate?: Rate, withDecimals?: boolean) { + return getRateDisplayValue((customUnitRate?.rate ?? 0) / CONST.POLICY.CUSTOM_UNIT_RATE_BASE_OFFSET, toLocaleDigit, withDecimals); } /** diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 4fc850c83631..7b3b025b6ead 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -775,8 +775,15 @@ function calculateAmountForUpdatedWaypointOrRate( const amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, rate ?? 0); const updatedAmount = isFromExpenseReport ? -amount : amount; const updatedCurrency = currency ?? CONST.CURRENCY.USD; - const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, updatedCurrency, Localize.translateLocal, (digit) => - toLocaleDigit(preferredLocale, digit), + const updatedMerchant = DistanceRequestUtils.getDistanceMerchant( + true, + distanceInMeters, + unit, + rate, + updatedCurrency, + Localize.translateLocal, + (digit) => toLocaleDigit(preferredLocale, digit), + true, ); return { From b8e14c97dd8116911583c42ce6d8ed4fbc622f31 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Tue, 20 Aug 2024 04:59:39 +0530 Subject: [PATCH 2/7] fix: use shortform for unit in money request preview. Signed-off-by: krishna2323 --- src/components/MoneyRequestConfirmationList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index ce6495e1385e..4ed9ff124444 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -656,7 +656,7 @@ function MoneyRequestConfirmationList({ */ IOU.setMoneyRequestPendingFields(transactionID, {waypoints: isDistanceRequestWithPendingRoute ? CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD : null}); - const distanceMerchant = DistanceRequestUtils.getDistanceMerchant(hasRoute, distance, unit, rate ?? 0, currency ?? CONST.CURRENCY.USD, translate, toLocaleDigit); + const distanceMerchant = DistanceRequestUtils.getDistanceMerchant(hasRoute, distance, unit, rate ?? 0, currency ?? CONST.CURRENCY.USD, translate, toLocaleDigit, true); IOU.setMoneyRequestMerchant(transactionID, distanceMerchant, true); }, [ isDistanceRequestWithPendingRoute, From 2151608a643056c85cc2c78c9a80a036f539e936 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 21 Aug 2024 03:57:16 +0530 Subject: [PATCH 3/7] add suggested changes. Signed-off-by: krishna2323 --- src/components/MoneyRequestConfirmationList.tsx | 2 +- src/libs/DistanceRequestUtils.ts | 12 +++++++++--- src/libs/TransactionUtils/index.ts | 11 ++--------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index 4ed9ff124444..ce6495e1385e 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -656,7 +656,7 @@ function MoneyRequestConfirmationList({ */ IOU.setMoneyRequestPendingFields(transactionID, {waypoints: isDistanceRequestWithPendingRoute ? CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD : null}); - const distanceMerchant = DistanceRequestUtils.getDistanceMerchant(hasRoute, distance, unit, rate ?? 0, currency ?? CONST.CURRENCY.USD, translate, toLocaleDigit, true); + const distanceMerchant = DistanceRequestUtils.getDistanceMerchant(hasRoute, distance, unit, rate ?? 0, currency ?? CONST.CURRENCY.USD, translate, toLocaleDigit); IOU.setMoneyRequestMerchant(transactionID, distanceMerchant, true); }, [ isDistanceRequestWithPendingRoute, diff --git a/src/libs/DistanceRequestUtils.ts b/src/libs/DistanceRequestUtils.ts index 01e310b82ed6..9694ca33b54f 100644 --- a/src/libs/DistanceRequestUtils.ts +++ b/src/libs/DistanceRequestUtils.ts @@ -128,8 +128,10 @@ function getRoundedDistanceInUnits(distanceInMeters: number, unit: Unit): string * @param currency The currency associated with the rate * @param translate Translate function * @param toLocaleDigit Function to convert to localized digit + * @param useShortFormUnit If true, the unit will be returned in short form (e.g., "mi", "km"). * @returns A string that displays the rate used for expense calculation */ + function getRateForDisplay( unit: Unit | undefined, rate: number | undefined, @@ -160,6 +162,7 @@ function getRateForDisplay( * @param unit Unit that should be used to display the distance * @param rate Expensable amount allowed per unit * @param translate Translate function + * @param useShortFormUnit If true, the unit will be returned in short form (e.g., "mi", "km"). * @returns A string that describes the distance traveled */ function getDistanceForDisplay( @@ -175,6 +178,10 @@ function getDistanceForDisplay( } const distanceInUnits = getRoundedDistanceInUnits(distanceInMeters, unit); + if (useShortFormUnit) { + return `${distanceInUnits} ${unit}`; + } + const distanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.miles') : translate('common.kilometers'); const singularDistanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.mile') : translate('common.kilometer'); const unitString = distanceInUnits === '1' ? singularDistanceUnit : distanceUnit; @@ -200,14 +207,13 @@ function getDistanceMerchant( currency: string, translate: LocaleContextProps['translate'], toLocaleDigit: LocaleContextProps['toLocaleDigit'], - useShortFormUnit?: boolean, ): string { if (!hasRoute || !rate) { return translate('iou.fieldPending'); } - const distanceInUnits = getDistanceForDisplay(hasRoute, distanceInMeters, unit, rate, translate, useShortFormUnit); - const ratePerUnit = getRateForDisplay(unit, rate, currency, translate, toLocaleDigit, undefined, useShortFormUnit); + const distanceInUnits = getDistanceForDisplay(hasRoute, distanceInMeters, unit, rate, translate, true); + const ratePerUnit = getRateForDisplay(unit, rate, currency, translate, toLocaleDigit, undefined, true); return `${distanceInUnits} @ ${ratePerUnit}`; } diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 2686261871b5..3aaf4476eb0b 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -775,15 +775,8 @@ function calculateAmountForUpdatedWaypointOrRate( const amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, rate ?? 0); const updatedAmount = isFromExpenseReport ? -amount : amount; const updatedCurrency = currency ?? CONST.CURRENCY.USD; - const updatedMerchant = DistanceRequestUtils.getDistanceMerchant( - true, - distanceInMeters, - unit, - rate, - updatedCurrency, - Localize.translateLocal, - (digit) => toLocaleDigit(preferredLocale, digit), - true, + const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, updatedCurrency, Localize.translateLocal, (digit) => + toLocaleDigit(preferredLocale, digit), ); return { From 3962b7df578869c26aac3ca94e22fb003556921f Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 21 Aug 2024 03:59:27 +0530 Subject: [PATCH 4/7] minor spacing fix. Signed-off-by: krishna2323 --- src/libs/DistanceRequestUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/DistanceRequestUtils.ts b/src/libs/DistanceRequestUtils.ts index 9694ca33b54f..02e17c98fb0c 100644 --- a/src/libs/DistanceRequestUtils.ts +++ b/src/libs/DistanceRequestUtils.ts @@ -131,7 +131,6 @@ function getRoundedDistanceInUnits(distanceInMeters: number, unit: Unit): string * @param useShortFormUnit If true, the unit will be returned in short form (e.g., "mi", "km"). * @returns A string that displays the rate used for expense calculation */ - function getRateForDisplay( unit: Unit | undefined, rate: number | undefined, From 4daf8df25c6cfeef83896befc1dcf15ed0cdaf91 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Wed, 21 Aug 2024 04:05:15 +0530 Subject: [PATCH 5/7] minor fix. Signed-off-by: krishna2323 --- src/libs/DistanceRequestUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/DistanceRequestUtils.ts b/src/libs/DistanceRequestUtils.ts index 02e17c98fb0c..d9b2c7137e43 100644 --- a/src/libs/DistanceRequestUtils.ts +++ b/src/libs/DistanceRequestUtils.ts @@ -185,7 +185,7 @@ function getDistanceForDisplay( const singularDistanceUnit = unit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? translate('common.mile') : translate('common.kilometer'); const unitString = distanceInUnits === '1' ? singularDistanceUnit : distanceUnit; - return `${distanceInUnits} ${useShortFormUnit ? unit : unitString}`; + return `${distanceInUnits} ${unitString}`; } /** From f2da0ae10d2e87662b3ffcde23b6e3a596bc4053 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Thu, 22 Aug 2024 06:30:07 +0530 Subject: [PATCH 6/7] update getRateDisplayValue to return correct decimal points. Signed-off-by: krishna2323 --- src/libs/PolicyUtils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index c5629f3751c5..20215b0e2384 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -143,8 +143,10 @@ function getRateDisplayValue(value: number, toLocaleDigit: (arg: string) => stri return ''; } - if (!Number.isNaN(Number(numValue)) && withDecimals) { - return Number(numValue).toFixed(2).toString().replace('.', toLocaleDigit('.')); + if (withDecimals) { + const decimalPart = numValue.toString().split('.')[1]; + const fixedDecimalPoints = decimalPart.length > 2 && !decimalPart.endsWith('0') ? 3 : 2; + return Number(numValue).toFixed(fixedDecimalPoints).toString().replace('.', toLocaleDigit('.')); } return numValue.toString().replace('.', toLocaleDigit('.')).substring(0, value.toString().length); From fdd5fc7317cc9d333de87eb218a0f01061379016 Mon Sep 17 00:00:00 2001 From: krishna2323 Date: Fri, 23 Aug 2024 16:52:04 +0530 Subject: [PATCH 7/7] add test for PolicyUtils. Signed-off-by: krishna2323 --- src/libs/PolicyUtils.ts | 1 + tests/unit/PolicyUtilsTest.ts | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tests/unit/PolicyUtilsTest.ts diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 20215b0e2384..d1a35587a501 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -1000,6 +1000,7 @@ export { getTagLists, getTaxByID, getUnitRateValue, + getRateDisplayValue, goBackFromInvalidPolicy, hasAccountingConnections, hasSyncError, diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts new file mode 100644 index 000000000000..8178bb99e877 --- /dev/null +++ b/tests/unit/PolicyUtilsTest.ts @@ -0,0 +1,48 @@ +import * as PolicyUtils from '@libs/PolicyUtils'; + +function toLocaleDigitMock(dot: string): string { + return dot; +} + +describe('PolicyUtils', () => { + describe('getRateDisplayValue', () => { + it('should return an empty string for NaN', () => { + const rate = PolicyUtils.getRateDisplayValue('invalid' as unknown as number, toLocaleDigitMock); + expect(rate).toEqual(''); + }); + + describe('withDecimals = false', () => { + it('should return integer value as is', () => { + const rate = PolicyUtils.getRateDisplayValue(100, toLocaleDigitMock); + expect(rate).toEqual('100'); + }); + + it('should return non-integer value as is', () => { + const rate = PolicyUtils.getRateDisplayValue(10.5, toLocaleDigitMock); + expect(rate).toEqual('10.5'); + }); + }); + + describe('withDecimals = true', () => { + it('should return integer value with 2 trailing zeros', () => { + const rate = PolicyUtils.getRateDisplayValue(10, toLocaleDigitMock, true); + expect(rate).toEqual('10.00'); + }); + + it('should return non-integer value with up to 2 trailing zeros', () => { + const rate = PolicyUtils.getRateDisplayValue(10.5, toLocaleDigitMock, true); + expect(rate).toEqual('10.50'); + }); + + it('should return non-integer value with 3 decimals as is', () => { + const rate = PolicyUtils.getRateDisplayValue(10.531, toLocaleDigitMock, true); + expect(rate).toEqual('10.531'); + }); + + it('should return non-integer value with 3+ decimals cut to 3', () => { + const rate = PolicyUtils.getRateDisplayValue(10.531345, toLocaleDigitMock, true); + expect(rate).toEqual('10.531'); + }); + }); + }); +});