Make record.association.destroy(*records) on habtm and hm:t only delete records in the join table. This is to make the destroy method more consistent across the different types of associations. For more details see the CHANGELOG entry.

4 files changed, 30 insertions, 23 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 2811f53424..9bd59132f5 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -192,7 +192,7 @@ module ActiveRecord
       def destroy(*records)
         records = find(records) if records.any? {|record| record.kind_of?(Fixnum) || record.kind_of?(String)}
         remove_records(records) do |_records, old_records|
-          old_records.each { |record| record.destroy }
+          delete_records(old_records, :destroy) if old_records.any?
         end
 
         load_target
@@ -462,6 +462,12 @@ module ActiveRecord
           end
         end
 
+        # Delete the given records from the association, using one of the methods :destroy,
+        # :delete_all or :nullify. The default method used is given by the :dependent option.
+        def delete_records(records, method = @reflection.options[:dependent])
+          raise NotImplementedError
+        end
+
         def callback(method, record)
           callbacks_for(method).each do |callback|
             case callback
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 3329a4af8e..d70f326e55 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -40,7 +40,7 @@ module ActiveRecord
           load_target.size
         end
 
-        def delete_records(records)
+        def delete_records(records, method = nil)
           if sql = @reflection.options[:delete_sql]
             records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) }
           else
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index caefd14ee3..aff955dd5f 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -54,20 +54,20 @@ module ActiveRecord
         end
 
         # Deletes the records according to the <tt>:dependent</tt> option.
-        def delete_records(records)
-          case @reflection.options[:dependent]
-            when :destroy
-              records.each { |r| r.destroy }
-            when :delete_all
-              @reflection.klass.delete(records.map { |r| r.id })
-            else
-              updates    = { @reflection.foreign_key => nil }
-              conditions = { @reflection.association_primary_key => records.map { |r| r.id } }
+        def delete_records(records, method = @reflection.options[:dependent])
+          case method
+          when :destroy
+            records.each { |r| r.destroy }
+          when :delete_all
+            @reflection.klass.delete(records.map { |r| r.id })
+          else
+            updates    = { @reflection.foreign_key => nil }
+            conditions = { @reflection.association_primary_key => records.map { |r| r.id } }
 
-              scoped.where(conditions).update_all(updates)
+            scoped.where(conditions).update_all(updates)
           end
 
-          if has_cached_counter? && @reflection.options[:dependent] != :destroy
+          if has_cached_counter? && method != :destroy
             @owner.class.update_counters(@owner.id, cached_counter_attribute_name => -records.size)
           end
         end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index d5b901beff..3174ea6373 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -8,13 +8,6 @@ module ActiveRecord
 
       alias_method :new, :build
 
-      def destroy(*records)
-        transaction do
-          delete_records(records.flatten)
-          super
-        end
-      end
-
       # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been
       # loaded and calling collection.size if it has. If it's more likely than not that the collection does
       # have a size larger than zero, and you need to fetch that collection afterwards, it'll take one fewer
@@ -51,10 +44,18 @@ module ActiveRecord
         end
 
         # TODO - add dependent option support
-        def delete_records(records)
+        def delete_records(records, method = @reflection.options[:dependent])
           through_association = @owner.send(@reflection.through_reflection.name)
-          records.each do |associate|
-            through_association.where(construct_join_attributes(associate)).delete_all
+
+          case method
+          when :destroy
+            records.each do |record|
+              through_association.where(construct_join_attributes(record)).destroy_all
+            end
+          else
+            records.each do |record|
+              through_association.where(construct_join_attributes(record)).delete_all
+            end
           end
         end