In our system the prices are defined with three digits subunit. and the charges on Stripe are 10 times higher.
Solidus Version:
2.10.0
To Reproduce
Our Money configuration has the following:
Spree.config do |config|
config.currency = 'CHF'
end
Money::Currency.table[:chf].merge!(subunit_to_unit: 1000, smallest_denomination: 50)
to override the default behaviour.
Given an amount of 60.20 CHF, when a Payment is captured or authorized (see https://github.com/solidusio/solidus/blob/66af3b67dfe39535ed539aa36c907b014ff3f8e5/core/app/models/spree/payment/processing.rb#L51 and https://github.com/solidusio/solidus/blob/66af3b67dfe39535ed539aa36c907b014ff3f8e5/core/app/models/spree/payment/processing.rb#L174) the method cents is called which returns 60200 instead of the expected 6020, necessary for the Stripe charge.
Additional context
Our current patch consists of:
def two_digits_cents
((cents / currency.subunit_to_unit.to_f) * 100).to_i
end
money.two_digits_cents instead of money.money.cents.I'd love to open a PR with a fix, but I am not sure if this approach is correct. If you confirm me that it is, I'll proceed with a PR.
@coorasse I'm not very familiar with this aspect of Solidus so I may be wrong, but I suspect the problem stems from the subunit_to_unit configuration, which represents the proportion between the subunit (cents) and the unit (franc). When you set it to 1000, you tell Money that a franc is made of 1000 cents, which means 60.20 CHF will be (incorrectly) multiplied by 1000 and converted into 60200 cents.
Can you clarify why you set that value to 1000? Perhaps there's a better way to achieve the same goal.
We configured it like that because the shop administrator wants to set prices using 3 digits.
@coorasse do you know the business requirement behind that choice? As far as I can see, for a product to cost 60.203 CHF doesn't mean anything to a customer if the smallest unit is 0.01 CHF.
In fact, look at what happens with your fix:
cents = 60203 # 60.203 CHF
subunit_to_unit = 1000
((cents / subunit_to_unit.to_f) * 100).to_i # => 6020 (60.20 CHF)
This means that, even though the price of the product is 60.203 CHF, you're charging the customer 60.20 CHF (which, off the top of my head, may also cause accounting problems in the long run).
I think that's the root of the problem here. Increasing subunit_to_unit to 1000 allows you to specify such granular prices, but it's a hack that I suspect may create weird problems such as this.
If you could share a bit behind why you need such precision in the prices, we can probably find another solution.
The client needs to set them with three digits. I cannot share too much information, but since the amounts bought are high, it really makes a difference for them to also be able to change the third digit. I can think about the gas price for example.
Next to that, in this business case, we have also the option smallest_denomination: 50, which rounds the total always to 0.05 cents since you cannot invoice with a higher detail (as you also correctly pointed out).
So the example you reported, is absolutely true in general, but it does not affect us because you cannot have a total of 60.203 CHF: only single item prices. If you buy an item for 60.203, the cart total will display 60.20 anyway.
That was also my doubt: can we change it in the Payment model? Is there a case where the payment needs to be with cents precision? (so 60203?)
In the case of Stripe, specifically, you can only submit two digits, so 6020 => 60.20CHF would be any way better than 60203 => 602.30 CHF ๐
Still, at the moment, if you configure Solidus with any currency which has more than two digits for cents, Stripe charges are wrong. https://github.com/RubyMoney/money/blob/master/config/currency_iso.json#L223
So maybe we can ignore our specific case and generalise the issue?
Got it, the use case makes sense!
With that said, I don't know if this is a general issue: I think if you create a Stripe charge in a currency that has more than two digits for cents, Stripe will compute the final amount correctly. For instance, I expect that a charge with { "amount": 12345, "currency": "BHD" } would be converted into BHD 12.345. The reason it's not working in your case is that Swiss francs use two digits, but you're sending three.
In your specific situation, one problem I see is that Solidus may not mark the order as paid if the order total is smaller than the payment total because of the rounding that takes place in your custom #two_digit_cents method.
Have you tried the Money.infinite_precision option? I wasn't familiar with it, but it looks like it will allow you to specify as many digits as you want after the decimal separator without impacting the conversion between unit and subunit. I would try that or some other solution that has to do with the rounding configuration rather than the actual currency configuration.
Keep me posted on how it goes!
Still, at the moment, if you configure Solidus with any currency which has more than two digits for cents, Stripe charges are wrong. https://github.com/RubyMoney/money/blob/master/config/currency_iso.json#L223
@coorasse with Payment Intents at the moment the number if digits is hardcoded, see https://github.com/solidusio/solidus_stripe/blob/master/app/controllers/solidus_stripe/intents_controller.rb#L54. I guess this may not a problem for your specific scenario, as you said that the order will be charged up to 2 decimals anyways, but it may need to be addressed for other situations.
@spaghetticode yeah, I think we need to fix that to use current_order.currency.subunit_to_unit, otherwise it won't work for currencies where the unit is made up by more or fewer than 100 sub-units.
Thanks for the hint @aldesantis! Here are my findings so far:
Money.infinite_precision = true seems a better option than playing around with subunit_to_unit and smallest_denomination options.precision: 2 by default https://github.com/solidusio/solidus/blob/master/backend/app/views/spree/admin/shared/_number_with_currency.html.erb#L10 (that was the same also before. I just noticed that now,maybe it should be a different issue)money.money.cents I mentioned above returns now a BigDecimal instead of an Integer, therefore I had to explicitly do money.cents.to_i to avoid errors when calling Stripe. (by the way: the call money.money.cents should be replaced with money.cents anyway.So, to conclude, I think:
Money.infinite_precision = true is the configuration I was looking for. Thanks!cents.to_i (PR needed?)Thanks for the help so far!
@coorasse thanks for the tests, I think the issues you found should be addressed โ I'm pretty sure you're not the only store needing greater-than-usual precision for product prices.
Would you have some time to submit your patches as PRs to the core? If not, I can also try to find some time this week!
If it is not urgent, yes, I'd be glad to open a PR. Not this week, though. ๐
Most helpful comment
If it is not urgent, yes, I'd be glad to open a PR. Not this week, though. ๐