RubyMoney / monetize

A library for converting various objects into `Money` objects.
MIT License
430 stars 107 forks source link

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

Open thai-truong opened 5 months ago

thai-truong commented 5 months ago

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:

[8] pry(main)> "RM100".to_money => #

[14] pry(main)> Monetize.parse("100 RM") => #



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.
jaredbeck commented 3 months ago

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.

thai-truong commented 3 months ago

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!