Skip to content

Commit

Permalink
Fix Reported issue involving reoder and last
Browse files Browse the repository at this point in the history
This should Resolve activerecord-hackery#418

Since the 4.1 version of `reverse_order!` is used, an additional check has been
 added to actually reorder the `ORDER BY` clause if there are non-nil elements
 of it.

Additionally, a spec has been added to ensure proper ordering takes place.
  • Loading branch information
Jake Yesbeck committed Apr 15, 2016
1 parent 5542266 commit c17fada
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/squeel/adapters/active_record/4.1/relation_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ def build_from
def build_order(arel)
orders = order_visit(dehashified_order_values)
orders = orders.uniq.reject(&:blank?)
orders = reverse_sql_order(orders) if reverse_order_value && !reordering_value
if reverse_order_value && (!reordering_value || Array(order_values).any?(&:present?))
orders = reverse_sql_order(orders)
end

arel.order(*orders) unless orders.empty?
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,11 @@ module ActiveRecord
sql.should_not match /ORDER BY/
end

it 'reverses the order if used with last' do
expected = Person.all.sort_by { |p| p.id }.last
result = Person.reorder(:id).last
result.should eq(expected)
end
end

describe '#from' do
Expand Down

0 comments on commit c17fada

Please sign in to comment.