Move raising rollback to top methods to avoid raising so deep

carlosantoniodasilva · Nov 24, 2012 · 2eda87f · 2eda87f
1 parent c973f00
commit 2eda87f
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 4 deletions.
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
@@ -467,9 +467,11 @@ def delete_or_destroy(records, method)
         def remove_records(existing_records, records, method)
           records.each { |record| callback(:before_remove, record) }
 
-          delete_records(existing_records, method) if existing_records.any?
-          records.each { |record| target.delete(record) }
+          if existing_records.any?
+            delete_records(existing_records, method) or return false
+          end
 
+          records.each { |record| target.delete(record) }
           records.each { |record| callback(:after_remove, record) }
         end
 

diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -25,7 +25,9 @@ def handle_dependency
             load_target.each(&:mark_for_destruction)
           end
 
-          delete_all
+          deleted_records = delete_all
+          raise ActiveRecord::Rollback unless deleted_records && deleted_records.all?(&:destroyed?)
+          deleted_records
         end
       end
 
@@ -108,7 +110,7 @@ def inverse_updates_counter_cache?(reflection = reflection)
         # Deletes the records according to the <tt>:dependent</tt> option.
         def delete_records(records, method)
           if method == :destroy
-            records.each { |r| r.destroy }
+            return false unless records.all? { |r| r.destroy }
             update_counter(-records.length) unless inverse_updates_counter_cache?
           else
             if records == :all