esh-g
commented
1 year ago Proposal
Please re-state the problem we are trying to solve
Some currencies are automatically rounded off and decimals are removed while requesting money.
What is the root cause of this problem?
Some currencies don't allow decimals places to be present in their denominations. We are already getting the decimals allowed by a currency in the ONYXKEYS.CURRENCY_LIST.
Example:
VND: {
ISO4217: "704",
decimals: 0,
name: "Vietnam Dong",
symbol: "₫"
}So, if we select this currency the decimals are removed as decimals : 0.
What changes should be made to fix this?
We should not allow the user to enter decimals places for currencies that don't allow it. This can be done by modifying the MoneyRequestUtils.validateAmount() method.
-
https://github.com/Expensify/App/blob/9701fbe5ec24bf5031da390075b2a9fb125b3ce3/src/pages/iou/steps/MoneyRequestAmountForm.js#L64-L72 In the
MoneyRequestAccountFormwe should get the decimals allowed for the currency in the body of the component like this:const decimals = CurrencyUtils.getCurrencyDecimals(currency); -
https://github.com/Expensify/App/blob/9701fbe5ec24bf5031da390075b2a9fb125b3ce3/src/libs/MoneyRequestUtils.js#L59-L62 Modify the
MoneyRequestUtils.validateAmount()to take two arguments instead of one:amountanddecimals. Then, use a dynamicregexto validate the amount according to the decimals like this:function validateAmount(amount, decimals) { const regexString = decimals === 0 ? `^(\\d+(,\\d+)?)?$` // don't allow decimal point if decimals === 0 : `^(\\d+(,\\d+)*(\\.\\d{0,${decimals}})?)?$`; // Allow the decimal point and the desired number of digits after the point const decimalNumberRegex = new RegExp(regexString, 'i'); return amount === '' || (decimalNumberRegex.test(amount) && calculateAmountLength(amount) <= CONST.IOU.AMOUNT_MAX_LENGTH); } -
https://github.com/Expensify/App/blob/9701fbe5ec24bf5031da390075b2a9fb125b3ce3/src/pages/iou/steps/MoneyRequestAmountForm.js#L126 Modify the use of the
validateAmount()function and pass thedecimalswe got in step 1 to the function like this:if (!MoneyRequestUtils.validateAmount(newAmountWithoutSpaces, decimals)) { -
We can also add a function to strip the decimals places if the
currentAmountif it doesn't match the decimals when the currency is changed. Like we have these functions here: https://github.com/Expensify/App/blob/1eafc7dff1cb609e72a97d6f1d1993a2bf22aaa6/src/libs/MoneyRequestUtils.js#L10-L24function stripDecimalsFromAmount(amount) { return amount.replace(/\.\d*/, ''); }Then add a
useEffecttoMoneyRequestAccountFormto use the function oncurrencychange:useEffect(() => { if (!MoneyRequestUtils.validateAmount(currentAmount, decimals)) { setCurrentAmount(MoneyRequestUtils.stripDecimalsFromAmount(currentAmount)); } }, [currency]);
melvin-bot[bot]
sakluger
parasharrajat
grgia
priyankrawat
JmillsExpensify
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
We should return an error stating that decimals are not supported in the selected currency.
Actual Result:
No error is returned and the amount is rounded.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.62.0 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/93399543/faf670c6-a706-4534-92ea-136ac30b1b6e
https://github.com/Expensify/App/assets/93399543/c0c8f7af-8350-4be0-84ea-62aa2101610e
Expensify/Expensify Issue URL: Issue reported by: @priyankrawat Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690213457772999
View all open jobs on GitHub
Upwork Automation - Do Not Edit