diff options
author | Leandro Facchinetti <git@leafac.com> | 2014-05-12 12:06:35 -0300 |
---|---|---|
committer | Leandro Facchinetti <git@leafac.com> | 2014-05-16 00:39:02 -0300 |
commit | 5866437eefa19a38010c6045b9015b9508dd8a0d (patch) | |
tree | 6d407fb8b452a91f77fef23f5bfe6784ae4491b5 /activerecord/lib | |
parent | f25f5336ee07cc42207dc036d1a962b500969d10 (diff) | |
download | rails-5866437eefa19a38010c6045b9015b9508dd8a0d.tar.gz rails-5866437eefa19a38010c6045b9015b9508dd8a0d.tar.bz2 rails-5866437eefa19a38010c6045b9015b9508dd8a0d.zip |
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
Diffstat (limited to 'activerecord/lib')
-rw-r--r-- | activerecord/lib/active_record/relation.rb | 16 |
1 files changed, 13 insertions, 3 deletions
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 <tt>before_*</tt> 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 |