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

Fix AM/AR Dirty plugin issues with AR 5.2 #116

Merged
merged 3 commits into from
Nov 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion lib/mobility/plugins/active_model/dirty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ module Dirty
def write(locale, value, options = {})
locale_accessor = Mobility.normalize_locale_accessor(attribute, locale)
if model.changed_attributes.has_key?(locale_accessor) && model.changed_attributes[locale_accessor] == value
model.attributes_changed_by_setter.except!(locale_accessor)
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
27 changes: 25 additions & 2 deletions lib/mobility/plugins/active_record/dirty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ 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.
# 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
Expand All @@ -49,6 +51,7 @@ def included(model_class)
end
end
model_class.extend has_attribute
model_class.include ReadAttribute if ::ActiveRecord::VERSION::STRING >= '5.2'
end

private
Expand Down Expand Up @@ -94,6 +97,26 @@ def define_attribute_methods
alias_method :"#{name}_in_database", :"#{name}_was"
end
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