Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

currencies with different fractions than 2 #10

Open
Rixamos opened this issue Nov 1, 2020 · 11 comments
Open

currencies with different fractions than 2 #10

Rixamos opened this issue Nov 1, 2020 · 11 comments

Comments

@Rixamos
Copy link

Rixamos commented Nov 1, 2020

Hello, I was testing this lib adn I noticed that is converting any currency on a currency with 2 decimals, in this case if you try to pay in i.e. JPY the amount is multiplied by 100.
I am quite noob on github and I am not mainly a developer, I did some change to the code but I don t know how to send it to help in case it is useful to solve this issue :-) feel free to contact me to per mit me to learn how I can help (I tried to fork and push but I don t know if affects in some way your job or the licence so I removed my public fork atm)

@darkfrog26
Copy link
Contributor

If you created a fork you can submit a PR back that I can accept to merge back to master. Take a look at this page for some more information: https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request

@darkfrog26
Copy link
Contributor

Let me know if you have any problems and I'll do what I can to help.

@Rixamos Rixamos changed the title currencies with different fractins than 2 currencies with different fractions than 2 Nov 1, 2020
@Rixamos
Copy link
Author

Rixamos commented Nov 1, 2020

Thank you very much for yourr help and your library, I tried to do my best, I modified a bit the tests cause were failing (I supposed wa s something related to the acocunt that is in use witht he tests but I am also bad at testing :) eheh "forgive me father for my noobness"
I hope this change will work correctly in most cases and to help people that maybe had the same issue

@darkfrog26
Copy link
Contributor

I saw your PR, but wanted to take a shot at going through it myself. Could you take a look at #13 and let me know if it solves your needs?

@Rixamos
Copy link
Author

Rixamos commented Nov 3, 2020

Sure, I will take a look (probably tomorrow because it is a bit late today for me from italy :) ) .
Just to clarify my use case, I am trying to manage more currencies than just one per time, this by having people with a personal setting that is currency so the payment can be done in multiple currencies.
What I notices is the amount passed to Stripe was everytime multiplied and on Charges was everytime divided per 100 (I suspect there are more place affected, as i saw fastly you changed also the BankAccount to manage the defaultCurrency)
My simple tests have been USD and JPY. Tomorrow I will repeat my tests to have an idea about how is workign the #13
Thank you very much for your support.

@Rixamos
Copy link
Author

Rixamos commented Nov 3, 2020

I tried with
amount = Money(amount,Currency.getInstance(currency)), //amount in bigdecimal without scale and adding currency
amount = Money(amountScaled), // amount in bigdecimal with scale included
amount = Money(amountScaled,Currency.getInstance(currency)), // amount in bigdecimal with scale included and currency
but the line 166 in Implicits, the encoder, multiplies per the defaultCurrency the amount.
so in Stripe the amounts are multiplied per 100.
When the amount comes back, is considered X*100 but the decoder will divide it again to seems all ok on the app side, however in stripe account it isn't
In add on my personal opinion the "def apply(d: BigDecimal)" is a bit missleading in case someone pass the bigdecimal scaled yet.
I am not expert in scala and encoders and decorders, but is it possible to give to the encoder/decoder a parameter of the currenc currency/scale?
Thanks for your help on this issue.

@darkfrog26
Copy link
Contributor

Well, realistically this is a problem that should be handled by Stripe more. There are a few places where the currency is specified in the resulting value, and that should be used but it seems pretty hacky.

As far as coming from Stripe it should work already, but didn't you say it's no supported yet?

@Rixamos
Copy link
Author

Rixamos commented Nov 3, 2020

In the currencies that stripe supports there are some currencies without decimal fraction, i put an extract of how it works in Money.scala
Practically for whom has cents 100 = 1.00 and from whom doesn't 100=100 this is the trick.
Practically is working with the minimum possible granularity per currency
https://stripe.com/docs/currencies#zero-decimal here there is the explenation

@darkfrog26
Copy link
Contributor

That explanation states that JPY is a zero decimal currency...

@Rixamos
Copy link
Author

Rixamos commented Nov 4, 2020

Hello, yes, probably I explained myself wrongly, but this is the exact issue i noticed in JPY that for example has 0 decimals, there are also other currencies have 3 or 4 decimals if i remember correctly but at the moment aren t supported by stripe.
However as you can see from the link there are some with 0 decimals that are and fall in this issue.
In next days i will not be available but next week i ll return online so, sorry in advance for the silence in next days

@darkfrog26
Copy link
Contributor

No worries. I'll try to give this some more thought and see if there's a good way we can handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants