Unable to add specific amount of credits

ProtonMail / proton-mail-settings

React web application to manage ProtonMail settings

53 stars 13 forks source link

Unable to add specific amount of credits #256

Closed EmilMN closed 4 years ago

EmilMN commented 4 years ago

Summary:

If you try to add the value 35.77 as credits or donate and select PayPal as a payment method it does not work. Probably the same will occur with a credit card, QA will try.

To reproduce:

Go to Settings -> Dashboard (you can also go to Settings -> Donate
Click on the "add credits" button
Into the other field, enter 35.77 as payment details
Select PayPal as a payment method

You will see an error "Ammoun to pay must be an integer"

Screenshot:

CS Report: https://proton.atlassian.net/browse/CLIENT-1916

Swiip commented 4 years ago

It is due to weird JavaScript numbers handling. It could be fixed by a very simple Math.round but it raises two questions:

@emnproton

Do we want to raise an error if the user type 35.777? If yes, the round solution will not be enough.
Do we want to handle localizations? A french person could enter 35,77. It doesn't work right now.
- Subquestion: Simple solution, just replace eventual , by a .. Complicated solution: use a full localization library to handles all kinds of number representations.

EmilMN commented 4 years ago

I'm okay with not raising an error if the user enters 35.777 as an amount, as long as the payment can go through and the BE can accept the value. My reasoning is that it would be good if we can solve the original issue by using the Math.round function.
On v3, the credits input field doesn't accept , as a valid decimal separator, so this is not a regression in the current behavior.

@matstaz or others can chime in if they have a different opinion.

Swiip commented 4 years ago

In fact, it was a bit more complex.

Paypal is not accepting amounts with more than two digits, to be precise, PayPal want an integer amount in cents. The error message was from the PayPal API, not from us.

The simplest solution with a Math.round was not perfect because the user end up paying a amount different than what's on screen (even if it's about fractions of cents).

So I added an override of the amount in the input field on blur (not during the typing, it feels strange).