Viincenttt / MollieApi

This project allows you to easily add the Mollie payment provider to your application.
MIT License
147 stars 85 forks source link

Error on get PaymentMethodList when working with currencies without decimal places (PaymentMethodClient) #293

Closed Flo301 closed 1 year ago

Flo301 commented 1 year ago

Hi there, want to report another finding.

How to reproduce?

Take a currency from the mollie list (https://docs.mollie.com/payments/multicurrency) which has no decimal places like Japanese yen (JPY) and create a new Amount("JPY", 249M) object. Now take the Amout as parameter for the GetPaymentMethodList method. It will return a error => Bad Request - The amount contains an invalid value

Example:

var methodsResponse = await _paymentMethodClient.GetPaymentMethodListAsync(amount: new Amount("JPY", 249M));

Conclusion:

Using the constructor of Amount with a decimal value will always set a string with two decimal places to Amount.Value 249M => "249.00". So this solution works not for currencies without decimals places. Screenshot from 2023-01-29 01-10-29

Workaround:

ILS and JPY are the only currencies without decimal places, so ...

var _amount = new Amount(currency, value);
if (_amount.Currency == "ILS" || _amount.Currency == "JPY")
{
   _amount.Value = value.ToString("0");
}

For sure there is a better way to handle this. Let me know what you are thinking

Viincenttt commented 1 year ago

Hi @Flo301 ,

You are right, this looks like a bug. At the moment, the Amount constructor always sets the amount value to 2 decimals, regardless of currency. I think the best way to solve this would be to add a extra optional constructor argument with a default value of 2, that holds the number of decimal places. For example:

public Amount(string currency, decimal value, int numberOfDecimalPlaces = 2) {
            this.Currency = currency;
            // Add code here to use numberOfDecimalPlaces 
            this.Value = value.ToString("0.00", CultureInfo.InvariantCulture);
        }

Since this library is a API wrapper, I don't want to maintain a list of currencies and the number of decimal places they have. I found this list on Wikipedia: https://en.wikipedia.org/wiki/ISO_4217 and there are quite some exceptions.

Viincenttt commented 1 year ago

This is now live in 2.2.0.4