Fix unintuitive behavior with multiple order and group clauses
[#4545 state:committed] Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
This commit is contained in:
parent
6d7f2790cd
commit
902861a43a
@ -195,7 +195,7 @@ def execute_grouped_calculation(operation, column_name) #:nodoc:
|
||||
|
||||
select_statement << ", #{group_field} AS #{group_alias}"
|
||||
|
||||
relation = select(select_statement).group(group)
|
||||
relation = except(:group).select(select_statement).group(group)
|
||||
|
||||
calculated_data = @klass.connection.select_all(relation.to_sql)
|
||||
|
||||
|
@ -162,13 +162,9 @@ def build_arel
|
||||
arel = arel.take(@limit_value) if @limit_value.present?
|
||||
arel = arel.skip(@offset_value) if @offset_value.present?
|
||||
|
||||
@group_values.uniq.each do |g|
|
||||
arel = arel.group(g) if g.present?
|
||||
end
|
||||
arel = arel.group(*@group_values.uniq.select{|g| g.present?})
|
||||
|
||||
@order_values.uniq.each do |o|
|
||||
arel = arel.order(Arel::SqlLiteral.new(o.to_s)) if o.present?
|
||||
end
|
||||
arel = arel.order(*@order_values.uniq.select{|o| o.present?}.map(&:to_s))
|
||||
|
||||
selects = @select_values.uniq
|
||||
|
||||
|
@ -80,10 +80,15 @@ def apply_finder_options(options)
|
||||
|
||||
options.assert_valid_keys(VALID_FIND_OPTIONS)
|
||||
|
||||
[:joins, :select, :group, :having, :order, :limit, :offset, :from, :lock, :readonly].each do |finder|
|
||||
[:joins, :select, :group, :having, :limit, :offset, :from, :lock, :readonly].each do |finder|
|
||||
relation = relation.send(finder, options[finder]) if options.has_key?(finder)
|
||||
end
|
||||
|
||||
# Give precedence to newly-applied orders and groups to play nicely with with_scope
|
||||
[:group, :order].each do |finder|
|
||||
relation.send("#{finder}_values=", Array.wrap(options[finder]) + relation.send("#{finder}_values")) if options.has_key?(finder)
|
||||
end
|
||||
|
||||
relation = relation.where(options[:conditions]) if options.has_key?(:conditions)
|
||||
relation = relation.includes(options[:include]) if options.has_key?(:include)
|
||||
relation = relation.extending(options[:extend]) if options.has_key?(:extend)
|
||||
|
@ -1994,6 +1994,16 @@ def test_find_multiple_ordered_last
|
||||
assert_equal last, Developer.find(:all, :order => 'developers.name, developers.salary DESC').last
|
||||
end
|
||||
|
||||
def test_find_keeps_multiple_order_values
|
||||
combined = Developer.find(:all, :order => 'developers.name, developers.salary')
|
||||
assert_equal combined, Developer.find(:all, :order => ['developers.name', 'developers.salary'])
|
||||
end
|
||||
|
||||
def test_find_keeps_multiple_group_values
|
||||
combined = Developer.find(:all, :group => 'developers.name, developers.salary, developers.id, developers.created_at, developers.updated_at')
|
||||
assert_equal combined, Developer.find(:all, :group => ['developers.name', 'developers.salary', 'developers.id', 'developers.created_at', 'developers.updated_at'])
|
||||
end
|
||||
|
||||
def test_find_symbol_ordered_last
|
||||
last = Developer.find :last, :order => :salary
|
||||
assert_equal last, Developer.find(:all, :order => :salary).last
|
||||
|
Loading…
Reference in New Issue
Block a user