monetize icon indicating copy to clipboard operation
monetize copied to clipboard

Fix bug introduced by PR#167 not covering all available currencies

Open thai-truong opened this issue 1 year ago • 2 comments

Currently, there is an existing bug in monetize v1.13.0 introduced by https://github.com/RubyMoney/monetize/pull/167 that attempted to implement additional logic for currency parsing discussed in https://github.com/RubyMoney/monetize/issues/153.

Example of the bug:

  • Expected:
"RM100".to_money    # #<Money fractional:10000 currency:MYR>
Monetize.parse("RM100")   # #<Money fractional:10000 currency:MYR>
Monetize.parse("100 RM")   # #<Money fractional:10000 currency:MYR>
  • Reality:
[1] pry(main)> Monetize.parse("RM100")
=> #<Money fractional:10000 currency:USD>

[8] pry(main)> "RM100".to_money
=> #<Money fractional:10000 currency:USD>

[14] pry(main)> Monetize.parse("100 RM")
=> #<Money fractional:10000 currency:USD>

This PR aims to:

  • Fix that bug.
  • Implement the additional currency parsing logic in https://github.com/RubyMoney/monetize/issues/153.
  • Improve parsing logic in general by validating parsed currency input against Money::Currency.all.map(&:iso_code).
  • Update spec/monetize_spec.rb to accommodate for the aforementioned changes.

thai-truong avatar Jun 06 '24 07:06 thai-truong

Currently, there is an existing bug in monetize v1.13.0 ..

Does this bug only affect the Malaysian ringgit (MYR) or are other currencies affected? Thanks.

jaredbeck avatar Jul 29 '24 19:07 jaredbeck

Currently, there is an existing bug in monetize v1.13.0 ..

Does this bug only affect the Malaysian ringgit (MYR) or are other currencies affected? Thanks.

This affects any currency that is not in Monetize::Parser::CURRENCY_SYMBOLS.values, according to this line in the code.

I've personally run into the aforementioned issue with Malaysian Ringgit (as "RM", not "MYR") and Australian Dollar (AUD) since neither of them are in Monetize::Parser::CURRENCY_SYMBOLS.values.

Screenshot from 2024-07-30 13-49-35

Monetize::Parser::CURRENCY_SYMBOLS's main purpose seems to be to map non-ISO currency symbols to their respective ISO-4217 currency code. That's why I think it is unsuitable to be used in the manner implemented in https://github.com/RubyMoney/monetize/pull/167

What do you think? And thanks for taking a look!

thai-truong avatar Jul 30 '24 06:07 thai-truong