Skip to content

Commit

Permalink
Corrected Currency Display: Enforce Two Decimal Places in Amounts
Browse files Browse the repository at this point in the history
  • Loading branch information
abzokhattab committed Feb 5, 2024
1 parent 42deba4 commit 2169530
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
16 changes: 13 additions & 3 deletions src/libs/CurrencyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,19 @@ function convertToBackendAmount(amountAsFloat: number): number {
*
* @note we do not support any currencies with more than two decimal places.
*/
function convertToFrontendAmount(amountAsInt: number): number {
function convertToFrontendAmountAsInteger(amountAsInt: number): number {
return Math.trunc(amountAsInt) / 100.0;
}

/**
* Takes an amount in "cents" as an integer and converts it to a string amount used in the frontend.
*
* @note we do not support any currencies with more than two decimal places.
*/
function convertToFrontendAmountAsString(amountAsInt: number): string {
return convertToFrontendAmountAsInteger(amountAsInt).toFixed(2);
}

/**
* Given an amount in the "cents", convert it to a string for display in the UI.
* The backend always handle things in "cents" (subunit equal to 1/100)
Expand All @@ -105,7 +114,7 @@ function convertToDisplayString(amountInCents = 0, currency: string = CONST.CURR
return Localize.translateLocal('common.tbd');
}

const convertedAmount = convertToFrontendAmount(amountInCents);
const convertedAmount = convertToFrontendAmountAsInteger(amountInCents);
return NumberFormatUtils.format(BaseLocaleListener.getPreferredLocale(), convertedAmount, {
style: 'currency',
currency,
Expand All @@ -131,7 +140,8 @@ export {
getCurrencySymbol,
isCurrencySymbolLTR,
convertToBackendAmount,
convertToFrontendAmount,
convertToFrontendAmountAsInteger,
convertToFrontendAmountAsString,
convertToDisplayString,
isValidCurrencyCode,
};
6 changes: 3 additions & 3 deletions src/pages/iou/steps/MoneyRequestAmountForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const getNewSelection = (oldSelection, prevLength, newLength) => {
};

const isAmountInvalid = (amount) => !amount.length || parseFloat(amount) < 0.01;
const isTaxAmountInvalid = (currentAmount, taxAmount, isTaxAmountForm) => isTaxAmountForm && currentAmount > CurrencyUtils.convertToFrontendAmount(taxAmount);
const isTaxAmountInvalid = (currentAmount, taxAmount, isTaxAmountForm) => isTaxAmountForm && currentAmount > CurrencyUtils.convertToFrontendAmountAsInteger(taxAmount);

const AMOUNT_VIEW_ID = 'amountView';
const NUM_PAD_CONTAINER_VIEW_ID = 'numPadContainerView';
Expand All @@ -83,7 +83,7 @@ function MoneyRequestAmountForm({amount, taxAmount, currency, isEditing, forward
const isTaxAmountForm = Navigation.getActiveRoute().includes('taxAmount');

const decimals = CurrencyUtils.getCurrencyDecimals(currency);
const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : '';
const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmountAsString(amount) : '';

const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString);
const [formError, setFormError] = useState('');
Expand Down Expand Up @@ -119,7 +119,7 @@ function MoneyRequestAmountForm({amount, taxAmount, currency, isEditing, forward
};

const initializeAmount = useCallback((newAmount) => {
const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmount(newAmount).toString() : '';
const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmountAsString(newAmount) : '';
setCurrentAmount(frontendAmount);
setSelection({
start: frontendAmount.length,
Expand Down
17 changes: 14 additions & 3 deletions tests/unit/CurrencyUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,29 @@ describe('CurrencyUtils', () => {
});
});

describe('convertToFrontendAmount', () => {
describe('convertToFrontendAmountAsInteger', () => {
test.each([
[2500, 25],
[2550, 25.5],
[25, 0.25],
[2500, 25],
[2500.5, 25], // The backend should never send a decimal .5 value
])('Correctly converts %s to amount in units handled in frontend', (amount, expectedResult) => {
expect(CurrencyUtils.convertToFrontendAmount(amount)).toBe(expectedResult);
])('Correctly converts %s to amount in units handled in frontend as an integer', (amount, expectedResult) => {
expect(CurrencyUtils.convertToFrontendAmountAsInteger(amount)).toBe(expectedResult);
});
});

describe('convertToFrontendAmountAsString', () => {
test.each([
[2500, '25.00'],
[2550, '25.50'],
[25, '0.25'],
[2500, '25.00'],
[2500.5, '25.00'],
])('Correctly converts %s to amount in units handled in frontend as a string', (amount, expectedResult) => {
expect(CurrencyUtils.convertToFrontendAmountAsString(amount)).toBe(expectedResult);
});
});
describe('convertToDisplayString', () => {
test.each([
[CONST.CURRENCY.USD, 25, '$0.25'],
Expand Down

0 comments on commit 2169530

Please sign in to comment.