From 633ea6a826cb5e587165f78d15770271cb39f670 Mon Sep 17 00:00:00 2001
From: Tima Maslyuchenko <n3imo0o@gmail.com>
Date: Wed, 26 Sep 2012 22:16:17 +0300
Subject: learn ActiveRecord::QueryMethods#order work with hash arguments
---
activerecord/CHANGELOG.md | 10 +++++
.../lib/active_record/relation/query_methods.rb | 51 ++++++++++++++++++++--
activerecord/test/cases/relations_test.rb | 16 +++++++
3 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9abfe2e6fd..7b80598171 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,15 @@
## Rails 4.0.0 (unreleased) ##
+* Learn ActiveRecord::QueryMethods#order work with hash arguments
+
+ When symbol or hash passed we convert it to Arel::Nodes::Ordering.
+ If we pass invalid direction(like name: :DeSc) ActiveRecord::QueryMethods#order will raise an exception
+
+ User.order(:name, email: :desc)
+ # SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC
+
+ *Tima Maslyuchenko*
+
* Rename `ActiveRecord::Fixtures` class to `ActiveRecord::FixtureSet`.
Instances of this class normally hold a collection of fixtures (records)
loaded either from a single YAML file, or from a file and a folder
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 3c59bd8a68..2b12e66ed0 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -202,6 +202,15 @@ module ActiveRecord
#
# User.order('name DESC, email')
# => SELECT "users".* FROM "users" ORDER BY name DESC, email
+ #
+ # User.order(:name)
+ # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC
+ #
+ # User.order(email: :desc)
+ # => SELECT "users".* FROM "users" ORDER BY "users"."email" DESC
+ #
+ # User.order(:name, email: :desc)
+ # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC
def order(*args)
args.blank? ? self : spawn.order!(*args)
end
@@ -209,6 +218,8 @@ module ActiveRecord
# Like #order, but modifies relation in place.
def order!(*args)
args.flatten!
+
+ validate_order_args args
references = args.reject { |arg| Arel::Node === arg }
references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact!
@@ -234,6 +245,8 @@ module ActiveRecord
# Like #reorder, but modifies relation in place.
def reorder!(*args)
args.flatten!
+
+ validate_order_args args
self.reordering_value = true
self.order_values = args
@@ -658,9 +671,7 @@ module ActiveRecord
arel.group(*group_values.uniq.reject{|g| g.blank?}) unless group_values.empty?
- order = order_values
- order = reverse_sql_order(order) if reverse_order_value
- arel.order(*order.uniq.reject{|o| o.blank?}) unless order.empty?
+ build_order(arel)
build_select(arel, select_values.uniq)
@@ -786,11 +797,17 @@ module ActiveRecord
case o
when Arel::Nodes::Ordering
o.reverse
- when String, Symbol
+ when String
o.to_s.split(',').collect do |s|
s.strip!
s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC')
end
+ when Symbol
+ { o => :desc }
+ when Hash
+ o.each_with_object({}) do |(field, dir), memo|
+ memo[field] = (dir == :asc ? :desc : :asc )
+ end
else
o
end
@@ -800,6 +817,32 @@ module ActiveRecord
def array_of_strings?(o)
o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)}
end
+
+ def build_order(arel)
+ orders = order_values
+ orders = reverse_sql_order(orders) if reverse_order_value
+
+ orders = orders.uniq.reject(&:blank?).map do |order|
+ case order
+ when Symbol
+ table[order].asc
+ when Hash
+ order.map { |field, dir| table[field].send(dir) }
+ else
+ order
+ end
+ end.flatten
+
+ arel.order(*orders) unless orders.empty?
+ end
+
+ def validate_order_args(args)
+ args.select { |a| Hash === a }.each do |h|
+ unless (h.values - [:asc, :desc]).empty?
+ raise ArgumentError, 'Direction should be :asc or :desc'
+ end
+ end
+ end
end
end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index d801770118..fdbc0a3fdb 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -157,6 +157,22 @@ class RelationTest < ActiveRecord::TestCase
assert_equal 4, topics.to_a.size
assert_equal topics(:first).title, topics.first.title
end
+
+ def test_finding_with_assoc_order
+ topics = Topic.order(:id => :desc)
+ assert_equal 4, topics.to_a.size
+ assert_equal topics(:fourth).title, topics.first.title
+ end
+
+ def test_finding_with_reverted_assoc_order
+ topics = Topic.order(:id => :asc).reverse_order
+ assert_equal 4, topics.to_a.size
+ assert_equal topics(:fourth).title, topics.first.title
+ end
+
+ def test_raising_exception_on_invalid_hash_params
+ assert_raise(ArgumentError) { Topic.order(:name, "id DESC", :id => :DeSc) }
+ end
def test_finding_last_with_arel_order
topics = Topic.order(Topic.arel_table[:id].asc)
--
cgit v1.2.3