Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [P2P Distance] - Distance unit in the system message changes from miles to mi after changing rate. #47605

Merged
merged 8 commits into from
Aug 27, 2024
24 changes: 19 additions & 5 deletions src/libs/DistanceRequestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ 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(
Expand All @@ -137,6 +138,7 @@ function getRateForDisplay(
translate: LocaleContextProps['translate'],
toLocaleDigit: LocaleContextProps['toLocaleDigit'],
isOffline?: boolean,
useShortFormUnit?: boolean,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add it to JsDoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

): string {
if (isOffline && !rate) {
return translate('iou.defaultRate');
Expand All @@ -146,11 +148,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}`;
}

/**
Expand All @@ -159,14 +161,26 @@ 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(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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add it to JsDoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

): string {
if (!hasRoute || !rate || !unit || !distanceInMeters) {
return translate('iou.fieldPending');
}

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;
Expand Down Expand Up @@ -197,8 +211,8 @@ function getDistanceMerchant(
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, true);
const ratePerUnit = getRateForDisplay(unit, rate, currency, translate, toLocaleDigit, undefined, true);

return `${distanceInUnits} @ ${ratePerUnit}`;
}
Expand Down
14 changes: 11 additions & 3 deletions src/libs/PolicyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,23 @@ function getCustomUnitRate(policy: OnyxEntry<Policy>, 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 (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);
}

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);
}

/**
Expand Down Expand Up @@ -993,6 +1000,7 @@ export {
getTagLists,
getTaxByID,
getUnitRateValue,
getRateDisplayValue,
goBackFromInvalidPolicy,
hasAccountingConnections,
hasSyncError,
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/PolicyUtilsTest.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
});
Loading