diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index e99289e76..cf759cbd9 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -3,24 +3,33 @@ module PaperTrail class RecordTrail def initialize(record) @record = record + @in_after_callback = false end # Utility method for reifying. Anything executed inside the block will # appear like a new record. - def appear_as_new_record + def appear_as_unpersisted @record.instance_eval { alias :old_new_record? :new_record? alias :new_record? :present? + alias :old_persisted? :persisted? + alias :persisted? :nil? } yield - @record.instance_eval { alias :new_record? :old_new_record? } + @record.instance_eval { + alias :new_record? :old_new_record? + alias :persisted? :old_persisted? + } end def attributes_before_change - changed = @record.changed_attributes.select { |k, _v| - @record.class.column_names.include?(k) - } - @record.attributes.merge(changed) + Hash[@record.attributes.map do |k, v| + if @record.class.column_names.include?(k) + [k, attribute_in_previous_version(k)] + else + [k, v] + end + end] end def changed_and_not_ignored @@ -34,7 +43,7 @@ def changed_and_not_ignored } end skip = @record.paper_trail_options[:skip] - @record.changed - ignore - skip + changed_in_latest_version - ignore - skip end # Invoked after rollbacks to ensure versions records are not created for @@ -65,7 +74,7 @@ def changed_notably? # @api private def changes - notable_changes = @record.changes.delete_if { |k, _v| + notable_changes = changes_in_latest_version.delete_if { |k, _v| !notably_changed.include?(k) } AttributeSerializers::ObjectChangesAttribute. @@ -87,7 +96,7 @@ def enabled_for_model? # changed. def ignored_attr_has_changed? ignored = @record.paper_trail_options[:ignore] + @record.paper_trail_options[:skip] - ignored.any? && (@record.changed & ignored).any? + ignored.any? && (changed_in_latest_version & ignored).any? end # Returns true if this instance is the current, live one; @@ -107,9 +116,9 @@ def merge_metadata(data) # If it is an attribute that is changing in an existing object, # be sure to grab the current version. if @record.has_attribute?(v) && - @record.send("#{v}_changed?".to_sym) && + attribute_changed_in_latest_version?(v) && data[:event] != "create" - @record.send("#{v}_was".to_sym) + attribute_in_previous_version(v) else @record.send(v) end @@ -164,11 +173,14 @@ def previous_version end def record_create + @in_after_callback = true return unless enabled? versions_assoc = @record.send(@record.class.versions_association_name) version = versions_assoc.create! data_for_create update_transaction_id(version) save_associations(version) + ensure + @in_after_callback = false end # Returns data for record create @@ -225,6 +237,7 @@ def record_object_changes? end def record_update(force) + @in_after_callback = true if enabled? && (force || changed_notably?) versions_assoc = @record.send(@record.class.versions_association_name) version = versions_assoc.create(data_for_update) @@ -235,6 +248,8 @@ def record_update(force) save_associations(version) end end + ensure + @in_after_callback = false end # Returns data for record update @@ -474,5 +489,41 @@ def version def versions @record.public_send(@record.class.versions_association_name) end + + def attribute_in_previous_version(attr_name) + if @in_after_callback && rails_51? + @record.attribute_before_last_save(attr_name.to_s) + else + @record.attribute_was(attr_name.to_s) + end + end + + def changed_in_latest_version + if @in_after_callback && rails_51? + @record.saved_changes.keys + else + @record.changed + end + end + + def changes_in_latest_version + if @in_after_callback && rails_51? + @record.saved_changes + else + @record.changes + end + end + + def attribute_changed_in_latest_version?(attr_name) + if @in_after_callback && rails_51? + @record.saved_change_to_attribute?(attr_name.to_s) + else + @record.attribute_changed?(attr_name.to_s) + end + end + + def rails_51? + ActiveRecord::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR >= 1 + end end end diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index 9511c3dac..2d8cde7d4 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -264,7 +264,7 @@ def reify_has_ones(transaction_id, model, options = {}) if options[:mark_for_destruction] model.send(assoc.name).mark_for_destruction if model.send(assoc.name, true) else - model.paper_trail.appear_as_new_record do + model.paper_trail.appear_as_unpersisted do model.send "#{assoc.name}=", nil end end @@ -277,7 +277,7 @@ def reify_has_ones(transaction_id, model, options = {}) has_and_belongs_to_many: false ) ) - model.paper_trail.appear_as_new_record do + model.paper_trail.appear_as_unpersisted do without_persisting(child) do model.send "#{assoc.name}=", child end diff --git a/test/dummy/app/models/song.rb b/test/dummy/app/models/song.rb index 6e02dada0..1a118ef4f 100644 --- a/test/dummy/app/models/song.rb +++ b/test/dummy/app/models/song.rb @@ -1,7 +1,6 @@ # Example from 'Overwriting default accessors' in ActiveRecord::Base. class Song < ActiveRecord::Base has_paper_trail - attr_accessor :name # Uses an integer of seconds to hold the length of the song def length=(minutes) @@ -12,30 +11,36 @@ def length read_attribute(:length) / 60 end - # override attributes hashes like some libraries do - def attributes_with_name - if name - attributes_without_name.merge(name: name) - else - attributes_without_name + if ActiveRecord::VERSION::MAJOR >= 5 + attribute :name, :string + else + attr_accessor :name + + # override attributes hashes like some libraries do + def attributes_with_name + if name + attributes_without_name.merge(name: name) + else + attributes_without_name + end end - end - # `alias_method_chain` is deprecated in rails 5, but we cannot use the - # suggested replacement, `Module#prepend`, because we still support ruby 1.9. - alias attributes_without_name attributes - alias attributes attributes_with_name + # `alias_method_chain` is deprecated in rails 5, but we cannot use the + # suggested replacement, `Module#prepend`, because we still support ruby 1.9. + alias attributes_without_name attributes + alias attributes attributes_with_name - def changed_attributes_with_name - if name - changed_attributes_without_name.merge(name: name) - else - changed_attributes_without_name + def changed_attributes_with_name + if name + changed_attributes_without_name.merge(name: name) + else + changed_attributes_without_name + end end - end - # `alias_method_chain` is deprecated in rails 5, but we cannot use the - # suggested replacement, `Module#prepend`, because we still support ruby 1.9. - alias changed_attributes_without_name changed_attributes - alias changed_attributes changed_attributes_with_name + # `alias_method_chain` is deprecated in rails 5, but we cannot use the + # suggested replacement, `Module#prepend`, because we still support ruby 1.9. + alias changed_attributes_without_name changed_attributes + alias changed_attributes changed_attributes_with_name + end end