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.

Author: rafaelfranca

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*
 

activerecord/lib/active_record/associations/collection_association.rb

@@ -151,7 +151,7 @@ def transaction(*args)
 
       # 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 @@ def delete_all(dependent = nil)
         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

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -101,11 +101,10 @@ def test_create_from_association_with_nil_values_should_work
   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,15 +863,15 @@ def test_clearing_a_dependent_association_collection
     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
     assert_equal 0, firm.dependent_clients_of_firm(true).size
     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