IOU - Allow only 2 decimal points in amount selector

[isagoico]

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:

1. Navigate to a conversation
2. Request money
3. Enter an amount with 3 decimals

Expected Result:

Only 2 decimals should be allowed in amount field

Actual Result:

3 decimals are allowed in the amount field.

Workaround:

None needed.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.14-0

Reproducible in staging?: Yes
Reproducible in production?: Yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1636688148151100

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @danieldoglas (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[rushatgabhane]

Proposal

In IOUAmountPage

Change regex to /^\d+(,\d+)*(\.\d{0, 2})?$/

And toFixed(3) -> toFixed(2)

App/src/pages/iou/steps/IOUAmountPage.js

Lines 121 to 122 in 6701446

	const decimalNumberRegex = new RegExp(/^\d+(,\d+)*(\.\d{0,3})?$/, 'i');
	return amount === '' || (decimalNumberRegex.test(amount) && (parseFloat((amount * 100).toFixed(3)).toString().length <= CONST.IOU.AMOUNT_MAX_LENGTH));

[MelvinBot]

@danieldoglas Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[danieldoglas]

Will be working on this today

[danieldoglas]

I don't think we can just change this to two decimal cases.

Jamaican and Kuwaiti Dinar, for example, as 3 decimal cases. @rushatgabhane @isagoico, do you guys think we need to treat them by each currency?

[rushatgabhane]

Isn't the 3rd decimal ignored in the backend?

do you guys think we need to treat them by each currency

Yes, I agree.

[danieldoglas]

Ok.

So looking through the code, I got this method called Mobile_GetConstants, sending the parameter currencyList, will return all the accepted currencies from Expensify.

That will have a JSON array with the name, symbol and some other properties. Some of them has a decimal: 0 property.

I guess we could add the decimals for all of the accepted currencies, with the amount of decimals accepted, and treat that on the frontend to deal with the information received from the backend. That way, if we add another currency with 5 decimal cases, we don't need to change anything in the app.

@rushatgabhane are you OK with that solution?

cc: @johnmlee101

[rushatgabhane]

Yeah the array of currencies is known and kinda constant. So adding decimal property to all is fine. Looks great 👍

[danieldoglas]

Working on it.

[danieldoglas]

So, I'm working at the App for this. The logic implementation is almost done, but I just saw that we convert the value to an integer before sending it to the backend.

That means that the backend probably doesn't know where the decimal point is, and that could possibly break old.E.

I'll finish implementing it on new.E and check what happens in old.E.

[danieldoglas]

Ok, now I'm seeing that we actually use * 100 or / 100 in multiple places when dealing with amount.

I think that the best case scenario for us would be to deal correctly with the decimal places for all currencies, but that would be a change in several places in our app code today, including venmo and paypal integrations. The development is not so extensive, but I think this would take a while for testing.

Do you think the ROI is worth it @rushatgabhane?

cc: @johnmlee101

[danieldoglas]

Yep. Looks like this would need changes in our integrations servers, and probably more places that I don't know the context yet.

Thinking about the ROI, since most of our customers are using currencies with only 2 decimal cases, I guess we could keep it with only 2 decimals for now and do a plan to change that in the future based on our expansion to other countries with that necessity.

[danieldoglas]

I'll follow my gut here and keep it simple for now. I'll cancel my PR to libs for now too.