Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/MoneyRequestConfirmationList.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/IOUPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ function IOUPreview(props) {
{props.isBillSplit && !_.isEmpty(participantAccountIDs) && (
<Text style={[styles.textLabel, styles.colorMuted, styles.ml1]}>
{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),
})}
</Text>
)}
Expand Down
45 changes: 20 additions & 25 deletions src/libs/CurrencyUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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,

@aldo-expensify aldo-expensify Aug 5, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The library used in the backend and the library used in the frontend (Intl) format the RSD currency with 0 decimals. A client needed decimals, so we updated the backend to display RSD with two decimals.
Now, we have to force the frontend to also do the same so we don't get bug reports because of inconsistent behaviour.

I ONLY enabled this for RSD because otherwise the formatting will be inconsistent between backend and frontend for currencies with more than 2 decimals.

});
}

export {getCurrencyDecimals, getCurrencyUnit, getLocalizedCurrencySymbol, isCurrencySymbolLTR, convertToSmallestUnit, convertToWholeUnit, convertToDisplayString};
export {getCurrencyDecimals, getCurrencyUnit, getLocalizedCurrencySymbol, isCurrencySymbolLTR, convertToBackendAmount, convertToFrontendAmount, convertToDisplayString};
18 changes: 12 additions & 6 deletions src/libs/IOUUtils.js
Original file line number Diff line number Diff line change
@@ -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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Due to this rounding up, the amount mismatched when doing split. See #52310 for more context.

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

/**
Expand Down
15 changes: 1 addition & 14 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 3 additions & 4 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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}`;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/pages/EditRequestPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/pages/iou/steps/MoneyRequestAmountForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/steps/NewRequestAmountPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
61 changes: 33 additions & 28 deletions tests/unit/CurrencyUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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);
});
});

Expand All @@ -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);
});
Expand Down
Loading