Ensure AutosaveAssociation runs remove callbacks [#2146 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

2 files changed, 33 insertions, 20 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index f024f99a34..ad375be184 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -186,7 +186,6 @@ module ActiveRecord
         end
       end
 
-
       # Removes +records+ from this association calling +before_remove+ and
       # +after_remove+ callbacks.
       #
@@ -195,22 +194,25 @@ module ActiveRecord
       # are actually removed from the database, that depends precisely on
       # +delete_records+. They are in any case removed from the collection.
       def delete(*records)
-        records = flatten_deeper(records)
-        records.each { |record| raise_on_type_mismatch(record) }
-        
-        transaction do
-          records.each { |record| callback(:before_remove, record) }
-          
-          old_records = records.reject {|r| r.new_record? }
+        remove_records(records) do |records, old_records|
           delete_records(old_records) if old_records.any?
-          
-          records.each do |record|
-            @target.delete(record)
-            callback(:after_remove, record)
-          end
+          records.each { |record| @target.delete(record) }
         end
       end
 
+      # Destroy +records+ and remove from this association calling +before_remove+
+      # and +after_remove+ callbacks.
+      #
+      # Note this method will always remove records from database ignoring the
+      # +:dependent+ option.
+      def destroy(*records)
+        remove_records(records) do |records, old_records|
+          old_records.each { |record| record.destroy }
+        end
+
+        load_target
+      end
+
       # Removes all records from this association.  Returns +self+ so method calls may be chained.
       def clear
         return self if length.zero? # forces load_target if it hasn't happened already
@@ -223,15 +225,14 @@ module ActiveRecord
 
         self
       end
-      
-      def destroy_all
-        transaction do
-          each { |record| record.destroy }
-        end
 
+      # Destory all the records from this association
+      def destroy_all
+        load_target
+        destroy(@target)
         reset_target!
       end
-      
+
       def create(attrs = {})
         if attrs.is_a?(Array)
           attrs.collect { |attr| create(attr) }
@@ -431,6 +432,18 @@ module ActiveRecord
           record
         end
 
+        def remove_records(*records)
+          records = flatten_deeper(records)
+          records.each { |record| raise_on_type_mismatch(record) }
+
+          transaction do
+            records.each { |record| callback(:before_remove, record) }
+            old_records = records.reject { |r| r.new_record? }
+            yield(records, old_records)
+            records.each { |record| callback(:after_remove, record) }
+          end
+        end
+
         def callback(method, record)
           callbacks_for(method).each do |callback|
             ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record)
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 6dcc5005d1..741aa2acbe 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -283,7 +283,7 @@ module ActiveRecord
         if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
           records.each do |record|
             if autosave && record.marked_for_destruction?
-              record.destroy
+              association.destroy(record)
             elsif @new_record_before_save || record.new_record?
               if autosave
                 association.send(:insert_record, record, false, false)