From 83d6cc4815debe2086073387ecbf38c0f47cfec5 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Apr 2017 10:46:07 -0700
Subject: [PATCH] Move around AR::Dirty and fix _attribute method

We already have a _read_attribute method that can get the value we need
from the model.  Lets define that method in AM::Dirty and use the
existing one from AR::Dirty rather than introducing a new method.
---
 .../lib/active_model/attribute_methods.rb     |  4 ++++
 activemodel/lib/active_model/dirty.rb         | 12 ++++------
 .../active_record/attribute_methods/dirty.rb  |  4 ----
 activerecord/test/cases/dirty_test.rb         | 22 +++++++++++++++++++
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb
index 166c6ac21f..b5c0b43b61 100644
--- a/activemodel/lib/active_model/attribute_methods.rb
+++ b/activemodel/lib/active_model/attribute_methods.rb
@@ -472,5 +472,9 @@ module ActiveModel
       def missing_attribute(attr_name, stack)
         raise ActiveModel::MissingAttributeError, "missing attribute: #{attr_name}", stack
       end
+
+      def _read_attribute(attr)
+        __send__(attr)
+      end
   end
 end
diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb
index 37b51ad354..dc81f74779 100644
--- a/activemodel/lib/active_model/dirty.rb
+++ b/activemodel/lib/active_model/dirty.rb
@@ -179,13 +179,13 @@ module ActiveModel
     # Handles <tt>*_changed?</tt> for +method_missing+.
     def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc:
       !!changes_include?(attr) &&
-        (to == OPTION_NOT_GIVEN || to == _attributes(attr)) &&
+        (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) &&
         (from == OPTION_NOT_GIVEN || from == changed_attributes[attr])
     end
 
     # Handles <tt>*_was</tt> for +method_missing+.
     def attribute_was(attr) # :nodoc:
-      attribute_changed?(attr) ? changed_attributes[attr] : _attributes(attr)
+      attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr)
     end
 
     # Handles <tt>*_previously_changed?</tt> for +method_missing+.
@@ -226,7 +226,7 @@ module ActiveModel
 
       # Handles <tt>*_change</tt> for +method_missing+.
       def attribute_change(attr)
-        [changed_attributes[attr], _attributes(attr)] if attribute_changed?(attr)
+        [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr)
       end
 
       # Handles <tt>*_previous_change</tt> for +method_missing+.
@@ -239,7 +239,7 @@ module ActiveModel
         return if attribute_changed?(attr)
 
         begin
-          value = _attributes(attr)
+          value = _read_attribute(attr)
           value = value.duplicable? ? value.clone : value
         rescue TypeError, NoMethodError
         end
@@ -268,9 +268,5 @@ module ActiveModel
       def clear_attribute_changes(attributes) # :doc:
         attributes_changed_by_setter.except!(*attributes)
       end
-
-      def _attributes(attr)
-        __send__(attr)
-      end
   end
 end
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index 63978cfb3e..bd5003d63a 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -328,10 +328,6 @@ module ActiveRecord
         def clear_changed_attributes_cache
           remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes)
         end
-
-        def _attributes(attr)
-          _read_attribute(attr)
-        end
     end
   end
 end
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 15d7a32e21..721861975a 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -671,6 +671,28 @@ class DirtyTest < ActiveRecord::TestCase
     assert binary.changed?
   end
 
+  test "changes is correct for subclass" do
+    foo = Class.new(Pirate) do
+      def catchphrase
+        super.upcase
+      end
+    end
+
+    pirate = foo.create!(catchphrase: "arrrr")
+
+    new_catchphrase = "arrrr matey!"
+
+    pirate.catchphrase = new_catchphrase
+    assert pirate.catchphrase_changed?
+
+    expected_changes = {
+      "catchphrase" => ["arrrr", new_catchphrase]
+    }
+
+    assert_equal new_catchphrase.upcase, pirate.catchphrase
+    assert_equal expected_changes, pirate.changes
+  end
+
   test "changes is correct if override attribute reader" do
     pirate = Pirate.create!(catchphrase: "arrrr")
     def pirate.catchphrase