Merge pull request #10816 from bogdan/less-dirty-dirty

Make AM::Dirty less dirty to plugin into AR or other library

7 files changed, 32 insertions, 26 deletions

diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index 3d3c61ed1c..d9719afee3 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Added new API methods `reset_changes` and `changed_applied` to AM::Dirty
+    that control changes state. Previsously you needed to update internal
+    instance variables, but now API methods are available.
+
+    *Bogdan Gusiev*
+
 *   Fix has_secure_password. `password_confirmation` validations are triggered
     even if no `password_confirmation` is set.
 
diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb
index ea5ddf71de..be2759b141 100644
--- a/activemodel/lib/active_model/dirty.rb
+++ b/activemodel/lib/active_model/dirty.rb
@@ -14,13 +14,7 @@ module ActiveModel
   #   track.
   # * Call <tt>attr_name_will_change!</tt> before each change to the tracked
   #   attribute.
-  #
-  # If you wish to also track previous changes on save or update, you need to
-  # add:
-  #
-  #   @previously_changed = changes
-  #
-  # inside of your save or update method.
+  # * Call <tt>changes_applied</tt> after the changes are persisted.
   #
   # A minimal implementation could be:
   #
@@ -39,8 +33,8 @@ module ActiveModel
   #     end
   #
   #     def save
-  #       @previously_changed = changes
-  #       @changed_attributes.clear
+  #       # do persistence work
+  #       changes_applied
   #     end
   #   end
   #
@@ -129,7 +123,7 @@ module ActiveModel
     #   person.save
     #   person.previous_changes # => {"name" => ["bob", "robert"]}
     def previous_changes
-      @previously_changed
+      @previously_changed ||= {}
     end
 
     # Returns a hash of the attributes with unsaved changes indicating their original
@@ -154,6 +148,18 @@ module ActiveModel
 
     private
 
+      # Removes current changes and makes them accessible through +previous_changes+.
+      def changes_applied
+        @previously_changed = changes
+        @changed_attributes = {}
+      end
+
+      # Removes all dirty data: current changes and previous changes
+      def reset_changes
+        @previously_changed = {}
+        @changed_attributes = {}
+      end
+
       # Handle <tt>*_change</tt> for +method_missing+.
       def attribute_change(attr)
         [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr)
diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb
index ba45089cca..ed34ca8a6e 100644
--- a/activemodel/test/cases/dirty_test.rb
+++ b/activemodel/test/cases/dirty_test.rb
@@ -29,8 +29,7 @@ class DirtyTest < ActiveModel::TestCase
     end
 
     def save
-      @previously_changed = changes
-      @changed_attributes.clear
+      changes_applied
     end
   end
 
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index dc2399643c..19e81abba5 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -19,8 +19,7 @@ module ActiveRecord
       # Attempts to +save+ the record and clears changed attributes if successful.
       def save(*)
         if status = super
-          @previously_changed = changes
-          @changed_attributes.clear
+          changes_applied
         end
         status
       end
@@ -28,16 +27,14 @@ module ActiveRecord
       # Attempts to <tt>save!</tt> the record and clears changed attributes if successful.
       def save!(*)
         super.tap do
-          @previously_changed = changes
-          @changed_attributes.clear
+          changes_applied
         end
       end
 
       # <tt>reload</tt> the record and clears changed attributes.
       def reload(*)
         super.tap do
-          @previously_changed.clear
-          @changed_attributes.clear
+          reset_changes
         end
       end
 
@@ -48,11 +45,11 @@ module ActiveRecord
 
         # The attribute already has an unsaved change.
         if attribute_changed?(attr)
-          old = @changed_attributes[attr]
-          @changed_attributes.delete(attr) unless _field_changed?(attr, old, value)
+          old = changed_attributes[attr]
+          changed_attributes.delete(attr) unless _field_changed?(attr, old, value)
         else
           old = clone_attribute_value(:read_attribute, attr)
-          @changed_attributes[attr] = old if _field_changed?(attr, old, value)
+          changed_attributes[attr] = old if _field_changed?(attr, old, value)
         end
 
         # Carry on.
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 7a1069fead..366ebde418 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -413,8 +413,6 @@ module ActiveRecord
       @aggregation_cache        = {}
       @association_cache        = {}
       @attributes_cache         = {}
-      @previously_changed       = {}
-      @changed_attributes       = {}
       @readonly                 = false
       @destroyed                = false
       @marked_for_destruction   = false
@@ -431,7 +429,7 @@ module ActiveRecord
       # optimistic locking) won't get written unless they get marked as changed
       self.class.columns.each do |c|
         attr, orig_value = c.name, c.default
-        @changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr])
+        changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr])
       end
     end
 
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index d630f31f5f..bdd00ee259 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -434,7 +434,7 @@ module ActiveRecord
 
         changes[self.class.locking_column] = increment_lock if locking_enabled?
 
-        @changed_attributes.except!(*changes.keys)
+        changed_attributes.except!(*changes.keys)
         primary_key = self.class.primary_key
         self.class.unscoped.where(primary_key => self[primary_key]).update_all(changes) == 1
       end
diff --git a/guides/source/active_model_basics.md b/guides/source/active_model_basics.md
index 1d87646e49..0019d08328 100644
--- a/guides/source/active_model_basics.md
+++ b/guides/source/active_model_basics.md
@@ -120,8 +120,8 @@ class Person
   end
 
   def save
-    @previously_changed = changes
     # do save work...
+    changes_applied
   end
 end
 ```