RubyMoney/money-rails

monetized field subunit value incorrect when currency exponent changes during construction

Closed this issue · 8 comments

class Foo < ActiveRecord::Base
  # Use USD as model level currency because it has exponent 2
  register_currency :usd

  monetize :bar_amount, as: :bar, with_model_currency: :bar_currency_code, disable_validation: true
end

f = Foo.new({
  bar: 1,
  bar_currency_code: 'JPY' # exponent 1
})
f.bar_amount # this should be 1, but instead is 100

Reading bar before saving will fix the bar_amount, and putting bar_currency_code before bar in the initialization hash also prevents the bug.

Demonstrated by this commit with some hacked together tests:
asavageiv@1871392

I think the solution to this is to modify the subunit amount when the currency changes for the field or perhaps adding an after_initialize hook that reads the field to fix up the value. Happy to submit a PR if you can guide me on the right approach.

We've encountered this in our project when no validations are run the monetized field

Looked into this a little more. The after_initialize hook doesn't work because the monetized field may be a method that composes other monetized fields that have not yet been set during initialization. E.g. in the tests Transaction#total_cents combines amount_cents + tax_cents which aren't necessarily set when the after_initialize hook is called.

This is actually easy to reproduce any time the currency exponent changes.

class Product < ActiveRecord::Base
  monetize :price_subunit, as: :price, with_model_currency: :currency
end
p = Product.new(price: 1, currency: 'USD')
p.price_subunit
=> 100
p.currency = 'JPY'
p.price_subunit
=> 100
p.price
=> #<Money fractional:1 currency:JPY>
p.price_subunit
=> 1

I think the solution is to override the currency setter to update the monetized subunit when the currency changes.

Overriding the currency setter is not quite trivial because multiple attributes can use the same currency model, so all the subunits need to be updated at the same time. I'm trying a different approach by checking if the currency changed when reading the subunit

I found #500, #493, and #421, so I see this has been an issue for some time.

@antstorm I'd appreciate any input you have. I've tried 2 approaches so far:

  1. I naively overrode the currency assignment method to update the subunit value, but I didn't account for the case where the model currency was shared between multiple subunit values. I might be able to get this approach to work if I store a map of instance_currency_name to all monetized attributes it is for.
  2. I tried to do define_method "#{subunit_name}" but it was complicated by quite a bit by methods that weren't yet defined and I never did quite get it working.

Any suggestions?

@semmons99 Is this fixed or a "will not fix"?

@asavageiv simply a nudge of "if this is still relevant complain it was closed and submit a PR" 😎

would love to see this reopened with a PR

PR opened.