to_money doesn't handle 3 decimal places all the time

[mark-shan]

In shopify/billing, we were running a task to buld money objects from a csv. Mode was returning money objects with 3 decimal places which we assumed was harmless.

The PR where it was detected https://github.com/Shopify/billing/pull/15628
The line if code which converts csv value to money

We generated a csv of credits to issue using mode, and that returned values with 3 decimal places.

We expected $29 of credits to be created, but $29000 ended up being created.
https://github.com/Shopify/billing/blob/a89cebbecd837c13295fa73554e81a73bf3ee843/app/jobs/maintenance/partially_reverse_or_refund_payables_task.rb#L20

[kevinodotnet]

to_money is supposed to be banned, I thought, for this reason:

[1] pry(main)> "29.000".to_money("USD")
=> #<Money value:29000.00 currency:USD>
[2] pry(main)> Money.new("29.000", "USD")
=> #<Money value:29.00 currency:USD>

Do we know how this got through?

[mark-shan]

to_money is supposed to be banned, I thought, for this reason:

[1] pry(main)> "29.000".to_money("USD")
=> #<Money value:29000.00 currency:USD>
[2] pry(main)> Money.new("29.000", "USD")
=> #<Money value:29.00 currency:USD>

Do we know how this got through?

cautiontape bot was giving the opposite suggestion for the original PR for this maintenance task https://github.com/Shopify/billing/pull/11435#issuecomment-953130003

@AndreiUngur did recently suggest we remove that 😄

[kevinodotnet]

@mark-shan oh boy! :) Well, that's certainly confusing ... thanks, I'll tag the new enablement team on this one. Only a matter of time before this goes sideways on someone again.

[elfassy]

TheMoney::Parser::Fuzzy parser is deprecated but for legacy reasons is still being used by .to_money(...). Please replace all .to_money with Money.new(value, currency). If you really need fuzzy parsing (aka strings have thousands delimiters), use Money::Parser::LocaleAware instead... and pass along the locale to use to parse the string

[lavoiesl]

Adding a cop to catch the use of to_money: #264

[elfassy]

This seems serious enough that I'm good with introducing a breaking change. Scanning through the code, I don't see many people using .to_money with data coming directly from the user (which would maybe benefit from the fuzzy matching). I'll open a PR