From 5866437eefa19a38010c6045b9015b9508dd8a0d Mon Sep 17 00:00:00 2001 From: Leandro Facchinetti Date: Mon, 12 May 2014 12:06:35 -0300 Subject: Fix `Relation#delete_all` inconsistency When relation scopes include one of `uniq`, `group`, `having` or `offset`, the generated query ignores them and that causes unintended records to be deleted. This solves the issue by restricting the deletion when those scopes are present. rails/rails#11985 --- activerecord/lib/active_record/relation.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 24b33ab0a8..d92ff781ee 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -12,6 +12,7 @@ module ActiveRecord SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :from, :reordering, :reverse_order, :distinct, :create_with, :uniq] + INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having] VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS @@ -430,12 +431,21 @@ module ActiveRecord # If you need to destroy dependent associations or call your before_* or # +after_destroy+ callbacks, use the +destroy_all+ method instead. # - # If a limit scope is supplied, +delete_all+ raises an ActiveRecord error: + # If an invalid method is supplied, +delete_all+ raises an ActiveRecord error: # # Post.limit(100).delete_all - # # => ActiveRecord::ActiveRecordError: delete_all doesn't support limit scope + # # => ActiveRecord::ActiveRecordError: delete_all doesn't support limit def delete_all(conditions = nil) - raise ActiveRecordError.new("delete_all doesn't support limit scope") if self.limit_value + invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select { |method| + if MULTI_VALUE_METHODS.include?(method) + send("#{method}_values").any? + else + send("#{method}_value") + end + } + if invalid_methods.any? + raise ActiveRecordError.new("delete_all doesn't support #{invalid_methods.join(', ')}") + end if conditions where(conditions).delete_all -- cgit v1.2.3