RubyMoney / google_currency

Ruby Money::Bank interface for the Google Currency exchange data
http://rubymoney.github.com/google_currency
MIT License
182 stars 90 forks source link

Problem using GoogleCurrency with the Chilean Peso #38

Closed Wootenblatz closed 9 years ago

Wootenblatz commented 9 years ago

We had a customer from Chile order a product today and our system charged them upwards of $4700 USD for something that should have been $47.28. We get some of our pricing from a third party and they said that this item should have cost 29555.0 CLP.

Google's finance site converts that amount of CLP to 47.2880 USD so this doesn't seem to be something wrong on their end. https://www.google.com/finance/converter?a=29555.0&from=CLP&to=USD

Every time I get pricing from this third party I always put it into "cents" because that is how the Money gem operates. In this case I guess these would be Chilean Centavos (according to wikipedia anyway). When I run that number through the gem it converts to $4,728.80 USD instead of $47.28 USD.

Here is some rails console output showing this in action for CLP currency: 2.1.3 :047 > p.selling_price => 2955500 2.1.3 :048 > p.currency => "CLP" 2.1.3 :049 > p.selling_price / 100.0 => 29555.0 2.1.3 :050 > foreign_price = Money.new(p.selling_price, p.currency) => # 2.1.3 :051 > exchanged_price = foreign_price.exchange_to(:USD) => # 2.1.3 :052 > foreign_price.to_f => 2955500.0 2.1.3 :053 > exchanged_price.to_f => 4728.8

Here is the same logic but with Mexican Pesos instead: 2.1.3 :055 > p.selling_price => 74353 2.1.3 :056 > p.currency => "MXN" 2.1.3 :057 > p.selling_price / 100.0 => 743.53 2.1.3 :058 > foreign_price = Money.new(p.selling_price, p.currency) => # 2.1.3 :059 > exchanged_price = foreign_price.exchange_to(:USD) => # 2.1.3 :060 > foreign_price.to_f => 743.53 2.1.3 :061 > exchanged_price.to_f => 48.47

Am I doing something wrong or is this a bug? The results make it look like the original CLP price is already in centavos, but Google's Finance site indicates otherwise.

semmons99 commented 9 years ago

according to https://github.com/RubyMoney/money/blob/master/config/currency_iso.json#L447 the subunit to unit for CLP is 1, which is where your problem arises from. According to wikipedia the subunit coins are out of circulation, but it appears some financial institutions still treat it as if it exists. I'd suggest making a PR to the money gem to change the subunit to 100. That should solve your issue.

Wootenblatz commented 9 years ago

Hi Shane, I've got some code that should handle this patch, hopefully it isn't too horrible. I haven't done that much ruby involving overriding libraries like this. This was my first time trying to override a class method, I've only done instance methods before today. I do not have a test written yet because I wanted to get your thoughts on if my change is acceptable or not.

Originally I wanted to try to override the Money::Currency::Loader module but had no luck after hours of searching for some pointers on the right way to do it.

I settled on patching CLP's subunit_to_unit to 100 inside the table class method for Money::Currency. The important thing I wanted to accomplish was to leave the original method intact inside Money::Currency. I believe I've achieved that, so should that method change in some minor way, this code should still work.

I wanted this code to live outside google_currency.rb but haven't been able to figure out where it should go or what file name to use.

Here are my changes with existing tests passing: https://github.com/zkarpinski/google_currency/commit/ddf992fe5360e3f03543b9b64390f6eda3ea393e

Obviously GoogleCurrency has no existing tests for things in the Money::Currency class. This patch feels a little dirty to me, but maybe that is the nature of resolving the discrepancy between Google and the ISO standards for CLP. With all of that in mind, how do you think I should create a test for this change?

Thanks for your work on this Gem, it is greatly appreciated!

semmons99 commented 9 years ago

Easy way to do this in lib/money/bank/google_currency.rb

# after requiring 'money'
Money::Currency.table[:clp][:subunit_to_unit] = 100