diff --git a/src/components/MoneyRequestConfirmationList.js b/src/components/MoneyRequestConfirmationList.js index 4079d1240e61..2cc458d0e4ad 100755 --- a/src/components/MoneyRequestConfirmationList.js +++ b/src/components/MoneyRequestConfirmationList.js @@ -133,7 +133,7 @@ function MoneyRequestConfirmationList(props) { */ const getParticipantsWithAmount = useCallback( (participantsList) => { - const iouAmount = IOUUtils.calculateAmount(participantsList.length, props.iouAmount); + const iouAmount = IOUUtils.calculateAmount(participantsList.length, props.iouAmount, props.iouCurrencyCode); return OptionsListUtils.getIOUConfirmationOptionsFromParticipants(participantsList, CurrencyUtils.convertToDisplayString(iouAmount, props.iouCurrencyCode)); }, [props.iouAmount, props.iouCurrencyCode], @@ -176,7 +176,7 @@ function MoneyRequestConfirmationList(props) { })); } - const myIOUAmount = IOUUtils.calculateAmount(selectedParticipants.length, props.iouAmount, true); + const myIOUAmount = IOUUtils.calculateAmount(selectedParticipants.length, props.iouAmount, props.iouCurrencyCode, true); const formattedPayeeOption = OptionsListUtils.getIOUConfirmationOptionsFromPayeePersonalDetail( payeePersonalDetails, CurrencyUtils.convertToDisplayString(myIOUAmount, props.iouCurrencyCode), diff --git a/src/components/ReportActionItem/IOUPreview.js b/src/components/ReportActionItem/IOUPreview.js index 85a0b22ac327..0aa2aa16524e 100644 --- a/src/components/ReportActionItem/IOUPreview.js +++ b/src/components/ReportActionItem/IOUPreview.js @@ -237,7 +237,7 @@ function IOUPreview(props) { {props.isBillSplit && !_.isEmpty(participantAccountIDs) && ( {props.translate('iou.amountEach', { - amount: CurrencyUtils.convertToDisplayString(IOUUtils.calculateAmount(participantAccountIDs.length - 1, requestAmount), requestCurrency), + amount: CurrencyUtils.convertToDisplayString(IOUUtils.calculateAmount(participantAccountIDs.length - 1, requestAmount, requestCurrency), requestCurrency), })} )} diff --git a/src/libs/CurrencyUtils.js b/src/libs/CurrencyUtils.js index f8d56f7e60e9..271086ede36e 100644 --- a/src/libs/CurrencyUtils.js +++ b/src/libs/CurrencyUtils.js @@ -28,7 +28,7 @@ Onyx.connect({ */ function getCurrencyDecimals(currency = CONST.CURRENCY.USD) { const decimals = lodashGet(currencyList, [currency, 'decimals']); - return _.isUndefined(decimals) ? 2 : Math.min(decimals, 2); + return _.isUndefined(decimals) ? 2 : decimals; } /** @@ -73,54 +73,49 @@ function isCurrencySymbolLTR(currencyCode) { } /** - * Takes an amount as a floating point number and converts it to an integer amount. - * For example, given [25, USD], return 2500. - * Given [25.50, USD] return 2550. - * Given [2500, JPY], return 2500. + * Takes an amount as a floating point number and converts it to an integer equivalent to the amount in "cents". + * This is because the backend always stores amounts in "cents". The backend works in integer cents to avoid precision errors + * when doing math operations. * - * @note we do not currently support any currencies with more than two decimal places. Sorry Tunisia :( + * @note we do not currently support any currencies with more than two decimal places. Decimal past the second place will be rounded. Sorry Tunisia :( * - * @param {String} currency * @param {Number} amountAsFloat * @returns {Number} */ -function convertToSmallestUnit(currency, amountAsFloat) { - const currencyUnit = getCurrencyUnit(currency); - // We round off the number to resolve floating-point precision issues. - return Math.round(amountAsFloat * currencyUnit); +function convertToBackendAmount(amountAsFloat) { + return Math.round(amountAsFloat * 100); } /** - * Takes an amount as an integer and converts it to a floating point amount. - * For example, give [25, USD], return 0.25 - * Given [2550, USD], return 25.50 - * Given [2500, JPY], return 2500 + * Takes an amount in "cents" as an integer and converts it to a floating point amount used in the frontend. * * @note we do not support any currencies with more than two decimal places. * - * @param {String} currency * @param {Number} amountAsInt * @returns {Number} */ -function convertToWholeUnit(currency, amountAsInt) { - const currencyUnit = getCurrencyUnit(currency); - return Math.trunc(amountAsInt) / currencyUnit; +function convertToFrontendAmount(amountAsInt) { + return Math.trunc(amountAsInt) / 100.0; } /** - * Given an amount in the smallest units of a currency, convert it to a string for display in the UI. + * 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) * - * @param {Number} amountInSmallestUnit – should be an integer. Anything after a decimal place will be dropped. + * @param {Number} amountInCents – should be an integer. Anything after a decimal place will be dropped. * @param {String} currency * @returns {String} */ -function convertToDisplayString(amountInSmallestUnit, currency = CONST.CURRENCY.USD) { - const currencyUnit = getCurrencyUnit(currency); - const convertedAmount = Math.trunc(amountInSmallestUnit) / currencyUnit; +function convertToDisplayString(amountInCents, currency = CONST.CURRENCY.USD) { + const convertedAmount = convertToFrontendAmount(amountInCents); return NumberFormatUtils.format(BaseLocaleListener.getPreferredLocale(), convertedAmount, { style: 'currency', currency, + + // We are forcing the number of decimals because we override the default number of decimals in the backend for RSD + // See: https://github.com/Expensify/PHP-Libs/pull/834 + minimumFractionDigits: currency === 'RSD' ? getCurrencyDecimals(currency) : undefined, }); } -export {getCurrencyDecimals, getCurrencyUnit, getLocalizedCurrencySymbol, isCurrencySymbolLTR, convertToSmallestUnit, convertToWholeUnit, convertToDisplayString}; +export {getCurrencyDecimals, getCurrencyUnit, getLocalizedCurrencySymbol, isCurrencySymbolLTR, convertToBackendAmount, convertToFrontendAmount, convertToDisplayString}; diff --git a/src/libs/IOUUtils.js b/src/libs/IOUUtils.js index 241baf26c998..91e3b09a587c 100644 --- a/src/libs/IOUUtils.js +++ b/src/libs/IOUUtils.js @@ -1,25 +1,31 @@ import _ from 'underscore'; import CONST from '../CONST'; import * as ReportActionsUtils from './ReportActionsUtils'; +import * as CurrencyUtils from './CurrencyUtils'; /** * Calculates the amount per user given a list of participants * * @param {Number} numberOfParticipants - Number of participants in the chat. It should not include the current user. - * @param {Number} total - IOU total amount in the smallest units of the currency + * @param {Number} total - IOU total amount in backend format (cents, no matter the currency) + * @param {String} currency - This is used to know how many decimal places are valid to use when splitting the total * @param {Boolean} isDefaultUser - Whether we are calculating the amount for the current user * @returns {Number} */ -function calculateAmount(numberOfParticipants, total, isDefaultUser = false) { +function calculateAmount(numberOfParticipants, total, currency, isDefaultUser = false) { + // Since the backend can maximum store 2 decimal places, any currency with more than 2 decimals + // has to be capped to 2 decimal places + const currencyUnit = Math.min(100, CurrencyUtils.getCurrencyUnit(currency)); + const totalInCurrencySubunit = Math.round((total / 100) * currencyUnit); const totalParticipants = numberOfParticipants + 1; - const amountPerPerson = Math.round(total / totalParticipants); + const amountPerPerson = Math.round(totalInCurrencySubunit / totalParticipants); let finalAmount = amountPerPerson; if (isDefaultUser) { const sumAmount = amountPerPerson * totalParticipants; - const difference = total - sumAmount; - finalAmount = total !== sumAmount ? amountPerPerson + difference : amountPerPerson; + const difference = totalInCurrencySubunit - sumAmount; + finalAmount = totalInCurrencySubunit !== sumAmount ? amountPerPerson + difference : amountPerPerson; } - return finalAmount; + return Math.round((finalAmount * 100) / currencyUnit); } /** diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 0b5158e49f88..745d76ef2b9e 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -12,7 +12,6 @@ import * as Expensicons from '../components/Icon/Expensicons'; import Navigation from './Navigation/Navigation'; import ROUTES from '../ROUTES'; import * as NumberUtils from './NumberUtils'; -import * as NumberFormatUtils from './NumberFormatUtils'; import * as ReportActionsUtils from './ReportActionsUtils'; import * as TransactionUtils from './TransactionUtils'; import Permissions from './Permissions'; @@ -41,17 +40,6 @@ Onyx.connect({ }, }); -let preferredLocale = CONST.LOCALES.DEFAULT; -Onyx.connect({ - key: ONYXKEYS.NVP_PREFERRED_LOCALE, - callback: (val) => { - if (!val) { - return; - } - preferredLocale = val; - }, -}); - let allPersonalDetails; let currentUserPersonalDetails; Onyx.connect({ @@ -1811,8 +1799,7 @@ function buildOptimisticExpenseReport(chatReportID, policyID, payeeAccountID, to * @returns {Array} */ function getIOUReportActionMessage(type, total, comment, currency, paymentType = '', isSettlingUp = false) { - const currencyUnit = CurrencyUtils.getCurrencyUnit(currency); - const amount = NumberFormatUtils.format(preferredLocale, Math.abs(total) / currencyUnit, {style: 'currency', currency}); + const amount = CurrencyUtils.convertToDisplayString(total, currency); let paymentMethodMessage; switch (paymentType) { case CONST.IOU.PAYMENT_TYPE.ELSEWHERE: diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index b15a897bb4aa..18d282c7ca5d 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -590,8 +590,8 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco } // Loop through participants creating individual chats, iouReports and reportActionIDs as needed - const splitAmount = IOUUtils.calculateAmount(participants.length, amount, false); - const splits = [{email: currentUserEmail, accountID: currentUserAccountID, amount: IOUUtils.calculateAmount(participants.length, amount, true)}]; + const splitAmount = IOUUtils.calculateAmount(participants.length, amount, currency, false); + const splits = [{email: currentUserEmail, accountID: currentUserAccountID, amount: IOUUtils.calculateAmount(participants.length, amount, currency, true)}]; const hasMultipleParticipants = participants.length > 1; _.each(participants, (participant) => { @@ -1094,8 +1094,7 @@ function deleteMoneyRequest(transactionID, reportAction, isSingleTransactionView * @returns {String} */ function buildPayPalPaymentUrl(amount, submitterPayPalMeAddress, currency) { - const currencyUnit = CurrencyUtils.getCurrencyUnit(currency); - return `https://paypal.me/${submitterPayPalMeAddress}/${Math.abs(amount) / currencyUnit}${currency}`; + return `https://paypal.me/${submitterPayPalMeAddress}/${Math.abs(amount) / 100}${currency}`; } /** diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 971ad056ae7e..9be8aa2580c6 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -94,7 +94,7 @@ function EditRequestPage({report, route}) { defaultCurrency={transactionCurrency} reportID={report.reportID} onSubmit={(transactionChanges) => { - const amount = CurrencyUtils.convertToSmallestUnit(transactionCurrency, Number.parseFloat(transactionChanges)); + const amount = CurrencyUtils.convertToBackendAmount(transactionCurrency, Number.parseFloat(transactionChanges)); // In case the amount hasn't been changed, do not make the API request. if (amount === transactionAmount) { Navigation.dismissModal(); diff --git a/src/pages/iou/steps/MoneyRequestAmountForm.js b/src/pages/iou/steps/MoneyRequestAmountForm.js index 6ba85f100070..c5fe3e269fd8 100644 --- a/src/pages/iou/steps/MoneyRequestAmountForm.js +++ b/src/pages/iou/steps/MoneyRequestAmountForm.js @@ -66,7 +66,7 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu const textInput = useRef(null); - const selectedAmountAsString = amount ? CurrencyUtils.convertToWholeUnit(currency, amount).toString() : ''; + const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : ''; const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString); const [shouldUpdateSelection, setShouldUpdateSelection] = useState(true); @@ -100,7 +100,7 @@ function MoneyRequestAmountForm({amount, currency, isEditing, forwardedRef, onCu if (!currency || !amount) { return; } - const amountAsStringForState = CurrencyUtils.convertToWholeUnit(currency, amount).toString(); + const amountAsStringForState = CurrencyUtils.convertToFrontendAmount(amount).toString(); setCurrentAmount(amountAsStringForState); setSelection({ start: amountAsStringForState.length, diff --git a/src/pages/iou/steps/NewRequestAmountPage.js b/src/pages/iou/steps/NewRequestAmountPage.js index 29f5baa173b4..a10d15845c76 100644 --- a/src/pages/iou/steps/NewRequestAmountPage.js +++ b/src/pages/iou/steps/NewRequestAmountPage.js @@ -145,7 +145,7 @@ function NewRequestAmountPage({route, iou, report}) { }; const navigateToNextPage = (currentAmount) => { - const amountInSmallestCurrencyUnits = CurrencyUtils.convertToSmallestUnit(currency, Number.parseFloat(currentAmount)); + const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(currentAmount)); IOU.setMoneyRequestAmount(amountInSmallestCurrencyUnits); IOU.setMoneyRequestCurrency(currency); diff --git a/tests/unit/CurrencyUtilsTest.js b/tests/unit/CurrencyUtilsTest.js index 937bf95044cf..5058c56bfa00 100644 --- a/tests/unit/CurrencyUtilsTest.js +++ b/tests/unit/CurrencyUtilsTest.js @@ -63,6 +63,7 @@ describe('CurrencyUtils', () => { test('Currency decimals smaller than or equal 2', () => { expect(CurrencyUtils.getCurrencyDecimals('JPY')).toBe(0); expect(CurrencyUtils.getCurrencyDecimals('USD')).toBe(2); + expect(CurrencyUtils.getCurrencyDecimals('RSD')).toBe(2); }); test('Currency decimals larger than 2 should return 2', () => { @@ -85,36 +86,36 @@ describe('CurrencyUtils', () => { }); }); - describe('convertToSmallestUnit', () => { + describe('convertToBackendAmount', () => { test.each([ - [CONST.CURRENCY.USD, 25, 2500], - [CONST.CURRENCY.USD, 25.25, 2525], - [CONST.CURRENCY.USD, 25.5, 2550], - [CONST.CURRENCY.USD, 2500, 250000], - [CONST.CURRENCY.USD, 80.6, 8060], - [CONST.CURRENCY.USD, 80.9, 8090], - [CONST.CURRENCY.USD, 80.99, 8099], - ['JPY', 25, 25], - ['JPY', 25.25, 25], - ['JPY', 25.5, 26], - ['JPY', 2500, 2500], - ['JPY', 80.6, 81], - ['JPY', 80.9, 81], - ['JPY', 80.99, 81], - ])('Correctly converts %s to amount in smallest units', (currency, amount, expectedResult) => { - expect(CurrencyUtils.convertToSmallestUnit(currency, amount)).toBe(expectedResult); + [25, 2500], + [25.25, 2525], + [25.5, 2550], + [2500, 250000], + [80.6, 8060], + [80.9, 8090], + [80.99, 8099], + [25, 2500], + [25.25, 2525], + [25.5, 2550], + [2500, 250000], + [80.6, 8060], + [80.9, 8090], + [80.99, 8099], + ])('Correctly converts %s to amount in cents (subunit) handled in backend', (amount, expectedResult) => { + expect(CurrencyUtils.convertToBackendAmount(amount)).toBe(expectedResult); }); }); - describe('convertToWholeUnit', () => { + describe('convertToFrontendAmount', () => { test.each([ - [CONST.CURRENCY.USD, 2500, 25], - [CONST.CURRENCY.USD, 2550, 25.5], - ['JPY', 25, 25], - ['JPY', 2500, 2500], - ['JPY', 25.5, 25], - ])('Correctly converts %s to amount in whole units', (currency, amount, expectedResult) => { - expect(CurrencyUtils.convertToWholeUnit(currency, amount)).toBe(expectedResult); + [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); }); }); @@ -124,9 +125,13 @@ describe('CurrencyUtils', () => { [CONST.CURRENCY.USD, 2500, '$25.00'], [CONST.CURRENCY.USD, 150, '$1.50'], [CONST.CURRENCY.USD, 250000, '$2,500.00'], - ['JPY', 25, '¥25'], - ['JPY', 2500, '¥2,500'], - ['JPY', 25.5, '¥25'], + ['JPY', 2500, '¥25'], + ['JPY', 250000, '¥2,500'], + ['JPY', 2500.5, '¥25'], + ['RSD', 100, 'RSD\xa01.00'], + ['RSD', 145, 'RSD\xa01.45'], + ['BHD', 12345, 'BHD\xa0123.450'], + ['BHD', 1, 'BHD\xa00.010'], ])('Correctly displays %s', (currency, amount, expectedResult) => { expect(CurrencyUtils.convertToDisplayString(amount, currency)).toBe(expectedResult); }); diff --git a/tests/unit/IOUUtilsTest.js b/tests/unit/IOUUtilsTest.js index 1fa7974e762d..0d40ee87424c 100644 --- a/tests/unit/IOUUtilsTest.js +++ b/tests/unit/IOUUtilsTest.js @@ -138,20 +138,52 @@ describe('IOUUtils', () => { test('103 JPY split among 3 participants including the default user should be [35, 34, 34]', () => { const participantsAccountIDs = [100, 101]; - expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 103, true)).toBe(35); - expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 103)).toBe(34); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 10300, 'JPY', true)).toBe(3500); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 10300, 'JPY')).toBe(3400); + }); + + test('103 USD split among 3 participants including the default user should be [34.34, 34.33, 34.33]', () => { + const participantsAccountIDs = [100, 101]; + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 10300, 'USD', true)).toBe(3434); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 10300, 'USD')).toBe(3433); }); test('10 AFN split among 4 participants including the default user should be [1, 3, 3, 3]', () => { const participantsAccountIDs = [100, 101, 102]; - expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 10, true)).toBe(1); - expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 10)).toBe(3); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 1000, 'AFN', true)).toBe(100); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 1000, 'AFN')).toBe(300); + }); + + test('10.12 USD split among 4 participants including the default user should be [2.53, 2.53, 2.53, 2.53]', () => { + const participantsAccountIDs = [100, 101, 102]; + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 1012, 'USD', true)).toBe(253); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 1012, 'USD')).toBe(253); + }); + + test('10.12 USD split among 3 participants including the default user should be [3.38, 3.37, 3.37]', () => { + const participantsAccountIDs = [100, 102]; + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 1012, 'USD', true)).toBe(338); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 1012, 'USD')).toBe(337); }); - test('0.02 USD split among 4 participants including the default user should be [-1, 1, 1, 1]', () => { + test('0.02 USD split among 4 participants including the default user should be [-0.01, 0.01, 0.01, 0.01]', () => { const participantsAccountIDs = [100, 101, 102]; - expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 2, true)).toBe(-1); - expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 2)).toBe(1); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 2, 'USD', true)).toBe(-1); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 2, 'USD')).toBe(1); + }); + + test('1 RSD split among 3 participants including the default user should be [0.34, 0.33, 0.33]', () => { + // RSD is a special case that we forced to have 2 decimals + const participantsAccountIDs = [100, 101]; + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 100, 'RSD', true)).toBe(34); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 100, 'RSD')).toBe(33); + }); + + test('1 BHD split among 3 participants including the default user should be [0.34, 0.33, 0.33]', () => { + // BHD has 3 decimal places, but it still produces parts with only 2 decimal places because of a backend limitation + const participantsAccountIDs = [100, 101]; + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 100, 'BHD', true)).toBe(34); + expect(IOUUtils.calculateAmount(participantsAccountIDs.length, 100, 'BHD')).toBe(33); }); }); }); diff --git a/tests/unit/currencyList.json b/tests/unit/currencyList.json index 1242509c0813..d3659d9b1370 100644 --- a/tests/unit/currencyList.json +++ b/tests/unit/currencyList.json @@ -622,7 +622,6 @@ "RSD": { "symbol": "РСД", "name": "Serbian Dinar", - "decimals": 0, "ISO4217": "941" }, "RUB": {