From 0d4bf17745eb0589431d8ec35eff1dc4eeacf899 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Wed, 23 Apr 2014 22:31:08 -0400 Subject: write a new method to be accessed from delete_all The delete method is very coupled with delete all even though only a portion of the conditionals apply. Decoupling this will make the code easier to understand and manipulate. --- .../lib/active_record/associations/collection_association.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 3a0a5165b6..cb64b7887e 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -194,7 +194,7 @@ module ActiveRecord options[:dependent] end - delete(:all, dependent: dependent).tap do + delete_all_with_dependency(dependent).tap do reset loaded! end @@ -259,6 +259,14 @@ module ActiveRecord end end + def delete_all_with_dependency(dependent) + if (loaded? || dependent == :destroy) && dependent != :delete_all + delete_or_destroy(load_target, dependent) + else + delete_records(:all, dependent) + end + end + # Deletes the +records+ and removes them from this association calling # +before_remove+ , +after_remove+ , +before_destroy+ and +after_destroy+ callbacks. # -- cgit v1.2.3 From 7ad476be6f5b843cded4d1d84d2d22f3f4b43dfc Mon Sep 17 00:00:00 2001 From: eileencodes Date: Wed, 23 Apr 2014 22:33:24 -0400 Subject: remove unnecessary code from delete method Now that we have a new method delete_all_with_dependency this coupled conditional is no longer needed. --- .../lib/active_record/associations/collection_association.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index cb64b7887e..a0eba60101 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -247,16 +247,8 @@ module ActiveRecord _options = records.extract_options! dependent = _options[:dependent] || options[:dependent] - if records.first == :all - if (loaded? || dependent == :destroy) && dependent != :delete_all - delete_or_destroy(load_target, dependent) - else - delete_records(:all, dependent) - end - else - records = find(records) if records.any? { |record| record.kind_of?(Fixnum) || record.kind_of?(String) } - delete_or_destroy(records, dependent) - end + records = find(records) if records.any? { |record| record.kind_of?(Fixnum) || record.kind_of?(String) } + delete_or_destroy(records, dependent) end def delete_all_with_dependency(dependent) -- cgit v1.2.3 From e0e586094f968b1f8fa410aa84d105bc8e44e537 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Wed, 23 Apr 2014 22:35:23 -0400 Subject: simplify the delete all w/ dependency method After reviewing this code I realized the conditional that was there previously was basically saying if the dependency is not delete all. This is a better, cleaner, and clearer way to write this method. --- .../lib/active_record/associations/collection_association.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index a0eba60101..9bf253d976 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -252,10 +252,10 @@ module ActiveRecord end def delete_all_with_dependency(dependent) - if (loaded? || dependent == :destroy) && dependent != :delete_all - delete_or_destroy(load_target, dependent) - else + if dependent == :delete_all delete_records(:all, dependent) + else + delete_or_destroy(load_target, dependent) end end -- cgit v1.2.3