Permalink
Browse files

Merge pull request #14757 from estsauver/14752

Fix behavior of select! to be consistent with select #14752

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/relation/delegation.rb
	activerecord/lib/active_record/relation/query_methods.rb
	activerecord/test/cases/relation/mutation_test.rb
  • Loading branch information...
rafaelfranca committed Apr 22, 2014
1 parent d63c39f commit 0ce2e905baead3467924064069431d34674f09d2
@@ -1,3 +1,9 @@
* Fix name collision with `Array#select!` with `Relation#select!`.
Fixes #14752.
*Earl St Sauver*
* When a destroyed record is duped, the dup is not `destroyed?`.
*Kuldeep Aggarwal*
@@ -106,7 +106,7 @@ def build_scope
scope.where_values = Array(values[:where]) + Array(preload_values[:where])
scope.references_values = Array(values[:references]) + Array(preload_values[:references])
scope.select! preload_values[:select] || values[:select] || table[Arel.star]
scope._select! preload_values[:select] || values[:select] || table[Arel.star]
scope.includes! preload_values[:includes] || values[:includes]
if options[:as]
@@ -30,6 +30,8 @@ def other
else
other.joins!(*v)
end
elsif k == :select
other._select!(v)
else
other.send("#{k}!", v)
end
@@ -66,7 +68,13 @@ def merge
# expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that
# `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values
# don't fall through the cracks.
relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value)
unless value.nil? || (value.blank? && false != value)
if name == :select
relation._select!(*value)
else
relation.send("#{name}!", *value)
end
end
end
merge_multi_values
@@ -216,11 +216,11 @@ def select(*fields)
to_a.select { |*block_args| yield(*block_args) }
else
raise ArgumentError, 'Call this with at least one field' if fields.empty?
spawn.select!(*fields)
spawn._select!(*fields)
end
end
def select!(*fields) # :nodoc:
def _select!(*fields) # :nodoc:
self.select_values += fields.flatten
self
end
@@ -231,13 +231,18 @@ def relation
@relation ||= Relation.new FakeKlass.new('posts'), :b
end
(Relation::MULTI_VALUE_METHODS - [:references, :extending, :order]).each do |method|
(Relation::MULTI_VALUE_METHODS - [:references, :extending, :order, :select]).each do |method|
test "##{method}!" do
assert relation.public_send("#{method}!", :foo).equal?(relation)
assert_equal [:foo], relation.public_send("#{method}_values")
end
end
test "#_select!" do
assert relation.public_send("_select!", :foo).equal?(relation)
assert_equal [:foo], relation.public_send("select_values")
end
test "#order!" do
assert relation.order!('name ASC').equal?(relation)
assert_equal ['name ASC'], relation.order_values

0 comments on commit 0ce2e90

Please sign in to comment.