Skip to content

Commit

Permalink
Override _read_attribute in Dirty plugin
Browse files Browse the repository at this point in the history
This is required due to a fix in AR 5.2:

rails/rails#28661

Previously, `__send__` was used to fetch the value of the attribute
before changes, which worked fine with locale accessors like
`title_en`. However, with the fix above, `@attributes` is now used,
which does not contain translated attributes (let alone locale
accessors). Since in general, locale accessors are an open-ended set, we
can't treat them the way normal attributes are treated, so we resort to
overriding `_read_attribute` to check if the attribute that has changed
is one that Mobility changed, and if so use `__send__`. This fixes the
issue, although overriding a private method is not ideal.
  • Loading branch information
shioyama committed Nov 30, 2017
1 parent 4b78cb6 commit 2c39333
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 17 deletions.
18 changes: 18 additions & 0 deletions lib/mobility/plugins/active_model/dirty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def write(locale, value, options = {})
if model.changed_attributes.has_key?(locale_accessor) && model.changed_attributes[locale_accessor] == value
model.send(:attributes_changed_by_setter).except!(locale_accessor)
elsif read(locale, options.merge(fallback: false)) != value
model.send(:mobility_changed_attributes) << locale_accessor
model.send(:attribute_will_change!, locale_accessor)
end
super
Expand Down Expand Up @@ -63,6 +64,10 @@ def initialize(*attribute_names)
private :restore_attribute!
end

def included(model_class)
model_class.include ChangedAttributes
end

private

# Get method suffixes. Creating an object just to get the list of
Expand All @@ -74,6 +79,19 @@ def method_suffixes
include ::ActiveModel::Dirty
end.attribute_method_matchers.map(&:suffix).select { |m| m =~ /\A_/ }
end

# Tracks which translated attributes have been changed, separate from
# the default tracking of changes in ActiveModel/ActiveRecord Dirty.
# This is required in order for the Mobility ActiveRecord Dirty
# plugin to correctly read the value of locale accessors like
# +title_en+ in dirty tracking.
module ChangedAttributes
private

def mobility_changed_attributes
@mobility_changed_attributes ||= Set.new
end
end
end
end
end
Expand Down
57 changes: 40 additions & 17 deletions lib/mobility/plugins/active_record/dirty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,6 @@ def initialize(*attribute_names)
define_attribute_methods if ::ActiveRecord::VERSION::STRING >= '5.1'
end

# Overrides +ActiveRecord::AttributeMethods::ClassMethods#has_attribute+ to treat fallthrough attribute methods
# just like "real" attribute methods.
#
# @note Patching +has_attribute?+ is necessary as of AR 5.1 due to this commit[https://github.com/rails/rails/commit/4fed08fa787a316fa51f14baca9eae11913f5050].
# (I have voiced my opposition to this change here[https://github.com/rails/rails/pull/27963#issuecomment-310092787]).
# @param [Attributes] attributes
def included(model_class)
names = @attribute_names
method_name_regex = /\A(#{names.join('|'.freeze)})_([a-z]{2}(_[a-z]{2})?)(=?|\??)\z/.freeze
has_attribute = Module.new do
define_method :has_attribute? do |attr_name|
super(attr_name) || !!method_name_regex.match(attr_name)
end
end
model_class.extend has_attribute
end

private

def define_method_overrides
Expand Down Expand Up @@ -94,6 +77,46 @@ def define_attribute_methods
alias_method :"#{name}_in_database", :"#{name}_was"
end
end

# Overrides +ActiveRecord::AttributeMethods::ClassMethods#has_attribute+ and
# +ActiveModel::AttributeMethods#_read_attribute+ to treat
# fallthrough attribute methods just like "real" attribute methods.
#
# @note Patching +has_attribute?+ is necessary as of AR 5.1 due to this commit[https://github.com/rails/rails/commit/4fed08fa787a316fa51f14baca9eae11913f5050].
# (I have voiced my opposition to this change here[https://github.com/rails/rails/pull/27963#issuecomment-310092787]).
# @param [Attributes] attributes
def included(model_class)
super
names = @attribute_names
method_name_regex = /\A(#{names.join('|'.freeze)})_([a-z]{2}(_[a-z]{2})?)(=?|\??)\z/.freeze
has_attribute = Module.new do
define_method :has_attribute? do |attr_name|
super(attr_name) || !!method_name_regex.match(attr_name)
end
end
model_class.extend has_attribute
model_class.include ReadAttribute
end

# Overrides _read_attribute to correctly dispatch reads on translated
# attributes to their respective setters, rather than to
# +@attributes+, which would otherwise return +nil+.
#
# For background on why this is necessary, see:
# https://github.com/shioyama/mobility/issues/115
module ReadAttribute
# @note We first check if attributes has the key +attr+ to avoid
# doing any extra work in case this is a "normal" (non-translated)
# attribute.
def _read_attribute(attr, *args)
if @attributes.key?(attr)
super
else
mobility_changed_attributes.include?(attr) ? __send__(attr) : super
end
end
private :_read_attribute
end
end
end
end
Expand Down

0 comments on commit 2c39333

Please sign in to comment.