mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Build the reverse_order on its proper method.
The reverse_order method was using a flag to control if the order should be reversed or not. Instead of using this variable just build the reverse order inside its proper method. This implementation was leading to an unexpected behavior when using reverse_order and then applying reorder(nil). Example: Before Post.order(:name).reverse_order.reorder(nil) # => SELECT "posts".* FROM "posts" ORDER BY "posts"."id" DESC After Post.order(:name).reverse_order.reorder(nil) # => SELECT "posts".* FROM "posts"
This commit is contained in:
parent
6bbbe0b651
commit
6c311e0b75
4 changed files with 26 additions and 7 deletions
|
@ -139,7 +139,6 @@ module ActiveRecord
|
||||||
def merge_single_values
|
def merge_single_values
|
||||||
relation.from_value = values[:from] unless relation.from_value
|
relation.from_value = values[:from] unless relation.from_value
|
||||||
relation.lock_value = values[:lock] unless relation.lock_value
|
relation.lock_value = values[:lock] unless relation.lock_value
|
||||||
relation.reverse_order_value = values[:reverse_order]
|
|
||||||
|
|
||||||
unless values[:create_with].blank?
|
unless values[:create_with].blank?
|
||||||
relation.create_with_value = (relation.create_with_value || {}).merge(values[:create_with])
|
relation.create_with_value = (relation.create_with_value || {}).merge(values[:create_with])
|
||||||
|
|
|
@ -825,7 +825,9 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def reverse_order! # :nodoc:
|
def reverse_order! # :nodoc:
|
||||||
self.reverse_order_value = !reverse_order_value
|
orders = order_values.uniq
|
||||||
|
orders.reject!(&:blank?)
|
||||||
|
self.order_values = reverse_sql_order(orders)
|
||||||
self
|
self
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -871,7 +873,6 @@ module ActiveRecord
|
||||||
|
|
||||||
case scope
|
case scope
|
||||||
when :order
|
when :order
|
||||||
self.reverse_order_value = false
|
|
||||||
result = []
|
result = []
|
||||||
else
|
else
|
||||||
result = [] unless single_val_method
|
result = [] unless single_val_method
|
||||||
|
@ -1031,7 +1032,6 @@ module ActiveRecord
|
||||||
def build_order(arel)
|
def build_order(arel)
|
||||||
orders = order_values.uniq
|
orders = order_values.uniq
|
||||||
orders.reject!(&:blank?)
|
orders.reject!(&:blank?)
|
||||||
orders = reverse_sql_order(orders) if reverse_order_value
|
|
||||||
|
|
||||||
arel.order(*orders) unless orders.empty?
|
arel.order(*orders) unless orders.empty?
|
||||||
end
|
end
|
||||||
|
|
|
@ -107,10 +107,18 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
test 'reverse_order!' do
|
test 'reverse_order!' do
|
||||||
assert relation.reverse_order!.equal?(relation)
|
relation = Post.order('title ASC, comments_count DESC')
|
||||||
assert relation.reverse_order_value
|
|
||||||
relation.reverse_order!
|
relation.reverse_order!
|
||||||
assert !relation.reverse_order_value
|
|
||||||
|
assert_equal 'title DESC', relation.order_values.first
|
||||||
|
assert_equal 'comments_count ASC', relation.order_values.last
|
||||||
|
|
||||||
|
|
||||||
|
relation.reverse_order!
|
||||||
|
|
||||||
|
assert_equal 'title ASC', relation.order_values.first
|
||||||
|
assert_equal 'comments_count DESC', relation.order_values.last
|
||||||
end
|
end
|
||||||
|
|
||||||
test 'create_with!' do
|
test 'create_with!' do
|
||||||
|
|
|
@ -1424,6 +1424,18 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
assert_equal [], scope.references_values
|
assert_equal [], scope.references_values
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_order_with_reorder_nil_removes_the_order
|
||||||
|
relation = Post.order(:title).reorder(nil)
|
||||||
|
|
||||||
|
assert_nil relation.order_values.first
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_reverse_order_with_reorder_nil_removes_the_order
|
||||||
|
relation = Post.order(:title).reverse_order.reorder(nil)
|
||||||
|
|
||||||
|
assert_nil relation.order_values.first
|
||||||
|
end
|
||||||
|
|
||||||
def test_presence
|
def test_presence
|
||||||
topics = Topic.all
|
topics = Topic.all
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue