Support the :dependent option on has_many :through associations. For historical and practical reasons, :delete_all is the default deletion strategy employed by association.delete(*records), despite the fact that the default strategy is :nullify for regular has_many. Also, this only works at all if the source reflection is a belongs_to. For other situations, you should directly modify the through association.

activerecord/CHANGELOG

@@ -1,5 +1,13 @@
 *Rails 3.1.0 (unreleased)*
 
+* Support the :dependent option on has_many :through associations. For historical and practical
+  reasons, :delete_all is the default deletion strategy employed by association.delete(*records),
+  despite the fact that the default strategy is :nullify for regular has_many. Also, this only
+  works at all if the source reflection is a belongs_to. For other situations, you should directly
+  modify the through association.
+
+  [Jon Leighton]
+
 * Changed the behaviour of association.destroy for has_and_belongs_to_many and has_many :through.
   From now on, 'destroy' or 'delete' on an association will be taken to mean 'get rid of the link',
   not (necessarily) 'get rid of the associated records'.

activerecord/lib/active_record/associations.rb

@@ -1606,12 +1606,11 @@ module ActiveRecord
                   send(reflection.name).each do |o|
                     # No point in executing the counter update since we're going to destroy the parent anyway
                     counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym
-                    if(o.respond_to? counter_method) then
+                    if o.respond_to?(counter_method)
                       class << o
                         self
                       end.send(:define_method, counter_method, Proc.new {})
                     end
-                    o.destroy
                   end
                 end
 

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -43,19 +43,18 @@ module ActiveRecord
           end
         end
 
-        # TODO - add dependent option support
-        def delete_records(records, method = @reflection.options[:dependent])
-          through_association = @owner.send(@reflection.through_reflection.name)
+        def deletion_scope(records)
+          @owner.send(@reflection.through_reflection.name).where(construct_join_attributes(*records))
+        end
 
+        def delete_records(records, method = @reflection.options[:dependent])
           case method
           when :destroy
-            records.each do |record|
-              through_association.where(construct_join_attributes(record)).destroy_all
-            end
+            deletion_scope(records).destroy_all
+          when :nullify
+            deletion_scope(records).update_all(@reflection.source_reflection.foreign_key => nil)
           else
-            records.each do |record|
-              through_association.where(construct_join_attributes(record)).delete_all
-            end
+            deletion_scope(records).delete_all
           end
         end
 

activerecord/lib/active_record/associations/through_association.rb

@@ -74,22 +74,41 @@ module ActiveRecord
             right.create_on(right.create_and(conditions)))
         end
 
-        # Construct attributes for :through pointing to owner and associate.
-        def construct_join_attributes(associate)
-          # TODO: revisit this to allow it for deletion, supposing dependent option is supported
-          raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro)
+        # Construct attributes for :through pointing to owner and associate. This is used by the
+        # methods which create and delete records on the association.
+        #
+        # We only support indirectly modifying through associations which has a belongs_to source.
+        # This is the "has_many :tags, :through => :taggings" situation, where the join model
+        # typically has a belongs_to on both side. In other words, associations which could also
+        # be represented as has_and_belongs_to_many associations.
+        #
+        # We do not support creating/deleting records on the association where the source has
+        # some other type, because this opens up a whole can of worms, and in basically any
+        # situation it is more natural for the user to just create or modify their join records
+        # directly as required.
+        def construct_join_attributes(*records)
+          if @reflection.source_reflection.macro != :belongs_to
+            raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection)
+          end
 
           join_attributes = {
             @reflection.source_reflection.foreign_key =>
-              associate.send(@reflection.source_reflection.association_primary_key)
+              records.map { |record|
+                record.send(@reflection.source_reflection.association_primary_key)
+              }
           }
 
           if @reflection.options[:source_type]
-            join_attributes.merge!(@reflection.source_reflection.foreign_type => associate.class.base_class.name)
+            join_attributes[@reflection.source_reflection.foreign_type] =
+              records.map { |record| record.class.base_class.name }
           end
 
+          if records.count == 1
+            Hash[join_attributes.map { |k, v| [k, v.first] }]
+          else
             join_attributes
           end
+        end
 
         # The reason that we are operating directly on the scope here (rather than passing
         # back some arel conditions to be added to the scope) is because scope.where([x, y])

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -155,6 +155,106 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_delete_through_belongs_to_with_dependent_nullify
+    Reference.make_comments = true
+
+    person    = people(:michael)
+    job       = jobs(:magician)
+    reference = Reference.where(:job_id => job.id, :person_id => person.id).first
+
+    assert_no_difference ['Job.count', 'Reference.count'] do
+      assert_difference 'person.jobs.count', -1 do
+        person.jobs_with_dependent_nullify.delete(job)
+      end
+    end
+
+    assert_equal nil, reference.reload.job_id
+  ensure
+    Reference.make_comments = false
+  end
+
+  def test_delete_through_belongs_to_with_dependent_delete_all
+    Reference.make_comments = true
+
+    person = people(:michael)
+    job    = jobs(:magician)
+
+    # Make sure we're not deleting everything
+    assert person.jobs.count >= 2
+
+    assert_no_difference 'Job.count' do
+      assert_difference ['person.jobs.count', 'Reference.count'], -1 do
+        person.jobs_with_dependent_delete_all.delete(job)
+      end
+    end
+
+    # Check that the destroy callback on Reference did not run
+    assert_equal nil, person.reload.comments
+  ensure
+    Reference.make_comments = false
+  end
+
+  def test_delete_through_belongs_to_with_dependent_destroy
+    Reference.make_comments = true
+
+    person = people(:michael)
+    job    = jobs(:magician)
+
+    # Make sure we're not deleting everything
+    assert person.jobs.count >= 2
+
+    assert_no_difference 'Job.count' do
+      assert_difference ['person.jobs.count', 'Reference.count'], -1 do
+        person.jobs_with_dependent_destroy.delete(job)
+      end
+    end
+
+    # Check that the destroy callback on Reference ran
+    assert_equal "Reference destroyed", person.reload.comments
+  ensure
+    Reference.make_comments = false
+  end
+
+  def test_belongs_to_with_dependent_destroy
+    person = PersonWithDependentDestroyJobs.find(1)
+
+    # Create a reference which is not linked to a job. This should not be destroyed.
+    person.references.create!
+
+    assert_no_difference 'Job.count' do
+      assert_difference 'Reference.count', -person.jobs.count do
+        person.destroy
+      end
+    end
+  end
+
+  def test_belongs_to_with_dependent_delete_all
+    person = PersonWithDependentDeleteAllJobs.find(1)
+
+    # Create a reference which is not linked to a job. This should not be destroyed.
+    person.references.create!
+
+    assert_no_difference 'Job.count' do
+      assert_difference 'Reference.count', -person.jobs.count do
+        person.destroy
+      end
+    end
+  end
+
+  def test_belongs_to_with_dependent_nullify
+    person = PersonWithDependentNullifyJobs.find(1)
+
+    references = person.references.to_a
+
+    assert_no_difference ['Reference.count', 'Job.count'] do
+      person.destroy
+    end
+
+    references.each do |reference|
+      assert_equal nil, reference.reload.job_id
+    end
+  end
+
   def test_replace_association
     assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)}
 

activerecord/test/models/person.rb

@@ -6,10 +6,14 @@ class Person < ActiveRecord::Base
   has_many :references
   has_many :bad_references
   has_many :fixed_bad_references, :conditions => { :favourite => true }, :class_name => 'BadReference'
-  has_many :jobs, :through => :references
   has_one :favourite_reference, :class_name => 'Reference', :conditions => ['favourite=?', true]
   has_many :posts_with_comments_sorted_by_comment_id, :through => :readers, :source => :post, :include => :comments, :order => 'comments.id'
 
+  has_many :jobs, :through => :references
+  has_many :jobs_with_dependent_destroy, :source => :job, :through => :references, :dependent => :destroy
+  has_many :jobs_with_dependent_delete_all, :source => :job, :through => :references, :dependent => :delete_all
+  has_many :jobs_with_dependent_nullify, :source => :job, :through => :references, :dependent => :nullify
+
   belongs_to :primary_contact, :class_name => 'Person'
   has_many :agents, :class_name => 'Person', :foreign_key => 'primary_contact_id'
   has_many :agents_of_agents, :through => :agents, :source => :agents
@@ -18,3 +22,24 @@ class Person < ActiveRecord::Base
   scope :males, :conditions => { :gender => 'M' }
   scope :females, :conditions => { :gender => 'F' }
 end
+
+class PersonWithDependentDestroyJobs < ActiveRecord::Base
+  self.table_name = 'people'
+
+  has_many :references, :foreign_key => :person_id
+  has_many :jobs, :source => :job, :through => :references, :dependent => :destroy
+end
+
+class PersonWithDependentDeleteAllJobs < ActiveRecord::Base
+  self.table_name = 'people'
+
+  has_many :references, :foreign_key => :person_id
+  has_many :jobs, :source => :job, :through => :references, :dependent => :delete_all
+end
+
+class PersonWithDependentNullifyJobs < ActiveRecord::Base
+  self.table_name = 'people'
+
+  has_many :references, :foreign_key => :person_id
+  has_many :jobs, :source => :job, :through => :references, :dependent => :nullify
+end

activerecord/test/models/reference.rb

@@ -1,6 +1,18 @@
 class Reference < ActiveRecord::Base
   belongs_to :person
   belongs_to :job
+
+  class << self
+    attr_accessor :make_comments
+  end
+
+  before_destroy :make_comments
+
+  def make_comments
+    if self.class.make_comments
+      person.update_attributes :comments => "Reference destroyed"
+    end
+  end
 end
 
 class BadReference < ActiveRecord::Base

activerecord/test/schema/schema.rb

@@ -425,6 +425,7 @@ ActiveRecord::Schema.define do
     t.string     :gender, :limit => 1
     t.references :number1_fan
     t.integer    :lock_version, :null => false, :default => 0
+    t.string     :comments
   end
 
   create_table :pets, :primary_key => :pet_id ,:force => true do |t|