Skip to content
Browse files

Refactor configure_dependency_for_has_many to use AssociationCollecti…

…on#delete_all. It was necessary to change test_before_destroy in lifecycle_test.rb so that it checks topic.replies.size *before* doing the destroy, as afterwards it will now (correctly) be 0.
  • Loading branch information...
1 parent 27ea048 commit 38fbfa6390c449dfc9a3db2975e65bd0fd665b18 @jonleighton jonleighton committed
Showing with 24 additions and 41 deletions.
  1. +20 −38 activerecord/lib/active_record/associations.rb
  2. +4 −3 activerecord/test/cases/lifecycle_test.rb
View
58 activerecord/lib/active_record/associations.rb
@@ -1617,11 +1617,13 @@ def add_touch_callbacks(reflection, touch_attribute)
# Active Record codebase, is meant to allow plugins to define extra
# finder conditions.
def configure_dependency_for_has_many(reflection, extra_conditions = nil)
- if reflection.options.include?(:dependent)
+ if reflection.options[:dependent]
+ method_name = "has_many_dependent_for_#{reflection.name}"
+
case reflection.options[:dependent]
- when :destroy
- method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym
- define_method(method_name) do
+ when :destroy, :delete_all, :nullify
+ define_method(method_name) do
+ if reflection.options[:dependent] == :destroy
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
@@ -1633,35 +1635,23 @@ class << o
o.destroy
end
end
- before_destroy method_name
- when :delete_all
- before_destroy do |record|
- self.class.send(:delete_all_has_many_dependencies,
- record,
- reflection.name,
- reflection.klass,
- reflection.dependent_conditions(record, self.class, extra_conditions))
- end
- when :nullify
- before_destroy do |record|
- self.class.send(:nullify_has_many_dependencies,
- record,
- reflection.name,
- reflection.klass,
- reflection.primary_key_name,
- reflection.dependent_conditions(record, self.class, extra_conditions))
+
+ reflection.klass.send(:with_scope, :find => { :conditions => extra_conditions }) do
+ # AssociationProxy#delete_all looks at the :dependent option and acts accordingly
+ send(reflection.name).delete_all
end
- when :restrict
- method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym
- define_method(method_name) do
- unless send(reflection.name).empty?
- raise DeleteRestrictionError.new(reflection)
- end
+ end
+ when :restrict
+ define_method(method_name) do
+ unless send(reflection.name).empty?
+ raise DeleteRestrictionError.new(reflection)
end
- before_destroy method_name
- else
- raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})"
+ end
+ else
+ raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})"
end
+
+ before_destroy method_name
end
end
@@ -1724,14 +1714,6 @@ def #{method_name}
end
end
- def delete_all_has_many_dependencies(record, reflection_name, association_class, dependent_conditions)
- association_class.delete_all(dependent_conditions)
- end
-
- def nullify_has_many_dependencies(record, reflection_name, association_class, primary_key_name, dependent_conditions)
- association_class.update_all("#{primary_key_name} = NULL", dependent_conditions)
- end
-
mattr_accessor :valid_keys_for_has_many_association
@@valid_keys_for_has_many_association = [
:class_name, :table_name, :foreign_key, :primary_key,
View
7 activerecord/test/cases/lifecycle_test.rb
@@ -101,9 +101,10 @@ class LifecycleTest < ActiveRecord::TestCase
fixtures :topics, :developers, :minimalistics
def test_before_destroy
- original_count = Topic.count
- (topic_to_be_destroyed = Topic.find(1)).destroy
- assert_equal original_count - (1 + topic_to_be_destroyed.replies.size), Topic.count
+ topic = Topic.find(1)
+ assert_difference 'Topic.count', -(1 + topic.replies.size) do
+ topic.destroy
+ end
end
def test_auto_observer

0 comments on commit 38fbfa6

Please sign in to comment.
Something went wrong with that request. Please try again.