Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monetizable: Skip subunit write for non-attributes in getter #706

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

edithemmings
Copy link

@edithemmings edithemmings commented Oct 11, 2024

Discovered an issue where making an update to instance currencies causes a MissingAttributeError for converted methods

Example model:

class Transaction < ActiveRecord::Base
  monetize :amount_cents, with_model_currency: :currency
  monetize :tax_cents, with_model_currency: :currency
  monetize :total_cents, with_model_currency: :currency
  monetize :optional_amount_cents, with_model_currency: :currency, allow_nil: true

  def total_cents(foo = 0, bar: 0)
    amount_cents + tax_cents + foo + bar
  end
end

Triggering the error:

transaction = Transaction.create(currency: 'USD')
transaction.update(currency: 'CAD')
ActiveModel::MissingAttributeError: can't write unknown attribute `total_cents`

This occurs because of a condition in read_monetized that tries to update the attribute if the currency changes. This case is needed for maintaining persisted values if the currency changes in a way that the subunit to unit ratio also changes. (This logic is also referred to as being controversial in the TODO comment on the test coverage)

Regardless, the attempt to update the attribute fails for monetized methods because there is no attribute to update. So I added a condition to check for the attribute's presence.

@edithemmings edithemmings force-pushed the main branch 2 times, most recently from 2ca99c0 to fbc4f21 Compare October 12, 2024 15:52
@edithemmings edithemmings changed the title Monetizable: Ignore converted methods when writing attributes Monetizable: Skip subunit write for non-attributes in getter Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant