search

mollie / spree-mollie-gateway

Mollie payments for Spree Commerce.
https://www.mollie.com
16 stars 23 forks source link

Don't use localized money format when submitting amounts #22

Closed rvanlieshout closed 5 years ago

rvanlieshout commented 5 years ago

We shouldn't commit localized strings like 100,00 to the API

I'm not sure if hardcoding separator and mark is the best way to go

vernondegoede commented 5 years ago

Thanks @rvanlieshout!

vernondegoede commented 5 years ago

@rvanlieshout Thanks again for your contribution. After giving this a second thought, I'm not sure in which cases the previous changes would fail. Although your changes work (in all supported currencies), I'm not able to reproduce the issue in the description of your PR.

Since Spree uses RubyMoney - Money internally, I don't see why .to_s wouldn't work.

Can you point me in the right direction by supplying steps to reproduce?

After switching my currency to EUR, the correct amounts get sent to the Mollie API:

image

image

rvanlieshout commented 5 years ago

@vernondegoede we had spree-i18n installed and the amount pushed was localized to e.g. 123,30

vernondegoede commented 5 years ago

Interesting. Do you know where spree-i18n changes the amounts? I thought spree-i18n was only responsible for translations, not transforming monetary amounts.

Amounts are still the same on my machine after installing spree-i18n. Do you have any additional gems installed that could have caused this issue?

willemstuursma commented 5 years ago

maybe the C locale was set to a locale that uses , as a decimal separator, e.g. nl_NL. It's pretty common in PHP.

rvanlieshout commented 5 years ago

I did also change i18n default locale. The money gem was responsible for that translation

I’ll try to check the exact dependency soon

Op 21 sep. 2018 om 16:34 heeft Willem Stuursma-Ruwen notifications@github.com het volgende geschreven:

maybe the C locale was set to a locale that uses , as a decimal separator, e.g. nl_NL. It's pretty common in PHP.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

vernondegoede commented 5 years ago

I've been able to reproduce this and can confirm that this does correctly format Rubymoney (using different locales). I'll add some tests for a setup using spree-i18n in the near future, since this gem is pretty popular in the Spree community.

This fix, as well as some other bugfixes, have been released in gem 'spree_mollie_gateway', '~> 2.1', '>= 2.1.1'.

Thanks again 👍