Skip to content

Commit

Permalink
Add support for Rails 5.1
Browse files Browse the repository at this point in the history
This updates the code to work with Rails 5.1.0.alpha, as of commit
rails/rails@1bdc395.

The biggest change was to call the new methods provided by
`ActiveRecord::Dirty` when needed. In the next version of Rails after
5.1, dirty will be "cleared" before after_save callbacks are run. This
means that code in an after_save callback will behave the same as if it
was run after calling `.save`.

Since the concept of "changed" is fairly nebulous, two new method sets
were introduced with names that specify whether they're operating on
changes from what we think is in the database to what's in memory, or
the changes that were just persisted. Paper trail will now use the
latter set of methods if available and called from an after_ callback.

There were a few other unrelated changes required to get this working on
master. The first was the change from `appear_as_new_record` to
`appear_as_unpersisted`. This was due to a single test that broke as a
result of
rails/rails@c546a2b
where reifying a version with a nil has_one association was persisting the
change to the foreign key. I am actually unsure how that commit caused
the problem (or more specifically, I'm unsure how it was passing
before). However, as best as I can tell, the purpose of
`appear_as_new_record` was to attempt to prevent the callbacks in
`AutosaveAssociation` (which is the module responsible for persisting
foreign key changes earlier than most people want most of the time
because backwards compatibility or the maintainer hates himself or
something) from running. By also stubbing out `persisted?` we can
actually prevent those. A more stable option might be to use `suppress`
instead, similar to the other branch in `reify_has_one`.

The remaining breakage was due to the `Song` model relying on internals
that have changed from underneath it. From Rails 5 onwards there is a
public API available to do what it wants, so we can just use that
instead.

This commit is not expected to pass on Rails 3, as paper-trail-gem#898 makes it sound
like support might be dropped. If this needs to be ammended to support
Rails 3, I will do so, but I will also grumble very loudly about it.
  • Loading branch information
sgrif committed Nov 30, 2016
1 parent 9936e64 commit d716b6a
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 48 deletions.
104 changes: 80 additions & 24 deletions lib/paper_trail/record_trail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
@record.attributes.map do |k, v|
if @record.class.column_names.include?(k)
[k, attribute_in_previous_version(k)]
else
[k, v]
end
end.to_h
end

def changed_and_not_ignored
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -164,11 +173,13 @@ def previous_version
end

def record_create
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)
in_after_callback do
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)
end
end

# Returns data for record create
Expand Down Expand Up @@ -225,14 +236,16 @@ def record_object_changes?
end

def record_update(force)
if enabled? && (force || changed_notably?)
versions_assoc = @record.send(@record.class.versions_association_name)
version = versions_assoc.create(data_for_update)
if version.errors.any?
log_version_errors(version, :update)
else
update_transaction_id(version)
save_associations(version)
in_after_callback do
if enabled? && (force || changed_notably?)
versions_assoc = @record.send(@record.class.versions_association_name)
version = versions_assoc.create(data_for_update)
if version.errors.any?
log_version_errors(version, :update)
else
update_transaction_id(version)
save_associations(version)
end
end
end
end
Expand Down Expand Up @@ -474,5 +487,48 @@ 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 in_after_callback
@in_after_callback = true
yield
ensure
@in_after_callback = false
end

def rails_51?
ActiveRecord::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR >= 1
end
end
end
4 changes: 2 additions & 2 deletions lib/paper_trail/reifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
49 changes: 27 additions & 22 deletions test/dummy/app/models/song.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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
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
1 change: 1 addition & 0 deletions test/unit/model_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase
should "return persist the changes on the live instance properly" do
assert_equal "Yellow Submarine", @song.name
end

should 'return "overwritten" virtual attribute on the reified instance' do
assert_equal "Good Vibrations", @song.versions.last.reify.name
end
Expand Down

0 comments on commit d716b6a

Please sign in to comment.