begin refactoring delete_records method

Refactor by creating two methods delete_all_records and delete_records
to be called by delete_all and delete (or destroy) respectively.
This reduces the number of conditionals required to handle _how_
records get deleted.

The new delete_count method handles how scope is applied to which
delete action.

A delete_all_records method also has to be called in has_many_through
association because of how the methods are chained. This will be
refactored later on.

Author: eileencodes

activerecord/lib/active_record/associations/collection_association.rb

@@ -194,7 +194,7 @@ def delete_all(dependent = nil)
                       options[:dependent]
                     end
 
-        delete_records(:all, dependent).tap do
+        delete_all_records(dependent).tap do
           reset
           loaded!
         end

activerecord/lib/active_record/associations/has_many_association.rb

@@ -105,23 +105,36 @@ def inverse_updates_counter_cache?(reflection = reflection())
           }
         end
 
+        def delete_count(records, method, scope)
+          case method
+          when :destroy
+            records.length
+          when :delete_all
+            scope.delete_all
+          else
+            scope.update_all(reflection.foreign_key => nil)
+          end
+        end
+
+        def delete_all_records(method)
+          scope = self.scope
+
+          count = delete_count(:all, method, scope)
+
+          update_counter(-count)
+        end
+
         # Deletes the records according to the <tt>:dependent</tt> option.
         def delete_records(records, method)
+          scope = self.scope.where(reflection.klass.primary_key => records)
+
+          count = delete_count(records, method, scope)
+
           if method == :destroy
             records.each(&:destroy!)
-            update_counter(-records.length) unless inverse_updates_counter_cache?
+            update_counter(-count) unless inverse_updates_counter_cache?
           else
-            if records == :all || !reflection.klass.primary_key
-              scope = self.scope
-            else
-              scope = self.scope.where(reflection.klass.primary_key => records)
-            end
-
-            if method == :delete_all
-              update_counter(-scope.delete_all)
-            else
-              update_counter(-scope.update_all(reflection.foreign_key => nil))
-            end
+            update_counter(-count)
           end
         end
 

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -130,6 +130,10 @@ def update_through_counter?(method)
           end
         end
 
+        def delete_all_records(method)
+          delete_records(:all, method)
+        end
+
         def delete_records(records, method)
           ensure_not_nested