From ad5976ebc0ad3aecad19dc0a1967191df8078b17 Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Wed, 30 Nov 2016 12:10:59 -0500
Subject: [PATCH 1/3] Add support for Rails 5.1

This updates the code to work with Rails 5.1.0.alpha, as of commit
/~https://github.com/rails/rails/commit/1bdc395d956f789b6612796ac6f130cde90d3066.

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
/~https://github.com/rails/rails/commit/c546a2b045600b6d700140adf2a4df666d6e08ce
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 #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.
---
 lib/paper_trail/record_trail.rb | 73 ++++++++++++++++++++++++++++-----
 lib/paper_trail/reifier.rb      |  4 +-
 test/dummy/app/models/song.rb   | 49 ++++++++++++----------
 3 files changed, 91 insertions(+), 35 deletions(-)

diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb
index e99289e76..ceb0bec33 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)
+      @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
@@ -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..6ba1ef24e 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
+  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

From 81d84027ddd676757b70894fd730b686d90ec041 Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Wed, 30 Nov 2016 13:29:09 -0500
Subject: [PATCH 2/3] Oh right, this has to run against Rails 5.0

I had removed the second argument since I made it optional upstream, but
in Rails 5 it's still mandatory.
---
 test/dummy/app/models/song.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/dummy/app/models/song.rb b/test/dummy/app/models/song.rb
index 6ba1ef24e..1a118ef4f 100644
--- a/test/dummy/app/models/song.rb
+++ b/test/dummy/app/models/song.rb
@@ -12,7 +12,7 @@ def length
   end
 
   if ActiveRecord::VERSION::MAJOR >= 5
-    attribute :name
+    attribute :name, :string
   else
     attr_accessor :name
 

From 3c773084e6088daf60bf82844208c57f42def2e6 Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Wed, 30 Nov 2016 13:48:09 -0500
Subject: [PATCH 3/3] Ruby 1.9 support

EOL software woo!
---
 lib/paper_trail/record_trail.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb
index ceb0bec33..cf759cbd9 100644
--- a/lib/paper_trail/record_trail.rb
+++ b/lib/paper_trail/record_trail.rb
@@ -23,13 +23,13 @@ def appear_as_unpersisted
     end
 
     def attributes_before_change
-      @record.attributes.map do |k, v|
+      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.to_h
+      end]
     end
 
     def changed_and_not_ignored