Fix wrong behavior where associations with dependent: :destroy options

was using nullify strategy

This caused a regression in applications trying to upgrade.

Also if the user set the dependent option as destroy he expects to get
the records removed from the database.

3 files changed, 7 insertions, 10 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 53dc1374b4..7e6ef27964 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -545,11 +545,11 @@
     Method `delete_all` should not be invoking callbacks and this
     feature was deprecated in Rails 4.0. This is being removed.
     `delete_all` will continue to honor the `:dependent` option. However
-    if `:dependent` value is `:destroy` then the default deletion
+    if `:dependent` value is `:destroy` then the `:delete_all` deletion
     strategy for that collection will be applied.
 
     User can also force a deletion strategy by passing parameter to
-    `delete_all`. For example you can do `@post.comments.delete_all(:nullify)` .
+    `delete_all`. For example you can do `@post.comments.delete_all(:nullify)`.
 
     *Neeraj Singh*
 
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 6b06a5f7fd..62f23f54f9 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -151,7 +151,7 @@ module ActiveRecord
 
       # Removes all records from the association without calling callbacks
       # on the associated records. It honors the `:dependent` option. However
-      # if the `:dependent` value is `:destroy` then in that case the default
+      # if the `:dependent` value is `:destroy` then in that case the `:delete_all`
       # deletion strategy for the association is applied.
       #
       # You can force a particular deletion strategy by passing a parameter.
@@ -170,9 +170,7 @@ module ActiveRecord
         dependent = if dependent.present?
                       dependent
                     elsif options[:dependent] == :destroy
-                      # since delete_all should not invoke callbacks so use the default deletion strategy
-                      # for :destroy
-                      reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) ? :delete_all : :nullify
+                      :delete_all
                     else
                       options[:dependent]
                     end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index b824a0ee7c..dfc8a68e8c 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -101,11 +101,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_do_not_call_callbacks_for_delete_all
-    bulb_count = Bulb.count
     car = Car.create(:name => 'honda')
     car.funky_bulbs.create!
     assert_nothing_raised { car.reload.funky_bulbs.delete_all }
-    assert_equal bulb_count + 1, Bulb.count, "bulbs should have been deleted using :nullify strategey"
+    assert_equal 0, Bulb.count, "bulbs should have been deleted using :delete_all strategey"
   end
 
   def test_building_the_associated_object_with_implicit_sti_base_class
@@ -864,7 +863,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 1, firm.dependent_clients_of_firm.size
     assert_equal 1, Client.find_by_id(client_id).client_of
 
-    # :nullify is called on each client
+    # :delete_all is called on each client since the dependent options is :destroy
     firm.dependent_clients_of_firm.clear
 
     assert_equal 0, firm.dependent_clients_of_firm.size
@@ -872,7 +871,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [], Client.destroyed_client_ids[firm.id]
 
     # Should be destroyed since the association is dependent.
-    assert_nil Client.find_by_id(client_id).client_of
+    assert_nil Client.find_by_id(client_id)
   end
 
   def test_delete_all_with_option_delete_all