aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarcelo Casiraghi <marcelocasiraghi@gmail.com>2013-05-23 00:17:15 -0300
committerRobin Dupret <robin.dupret@gmail.com>2014-03-04 18:20:00 +0100
commitacbd7ab22e5d9179487bd98110234c54535036c4 (patch)
treea84e1a5205de1e33dc635dffb0dc8bd48e8542e0
parent705915ab5cf24430892107764b0050c07e1df583 (diff)
downloadrails-acbd7ab22e5d9179487bd98110234c54535036c4.tar.gz
rails-acbd7ab22e5d9179487bd98110234c54535036c4.tar.bz2
rails-acbd7ab22e5d9179487bd98110234c54535036c4.zip
Allow string hash values on AR order method
This behavior has almost no performance impact: String not allowed 66.910000 0.030000 66.940000 ( 67.024976) String allowed 69.360000 0.030000 69.390000 ( 69.503096) Benchmarked with http://git.io/Y0YuRw.
-rw-r--r--activerecord/CHANGELOG.md10
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb9
-rw-r--r--activerecord/test/cases/relations_test.rb38
3 files changed, 52 insertions, 5 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 3e6758c222..4a522299ed 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,13 @@
+* Allow strings to specify the `#order` value.
+
+ Example:
+
+ Model.order(id: 'asc').to_sql == Model.order(id: :asc).to_sql
+
+ Fixes #10732.
+
+ *Marcelo Casiraghi*
+
* Dynamically register PostgreSQL enum OIDs. This prevents "unknown OID"
warnings on enum columns.
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index d88858611c..a22849f4a6 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1030,10 +1030,13 @@ module ActiveRecord
arel.order(*orders) unless orders.empty?
end
+ VALID_DIRECTIONS = [:asc, :desc, 'asc', 'desc'] # :nodoc:
+
def validate_order_args(args)
args.grep(Hash) do |h|
- unless (h.values - [:asc, :desc]).empty?
- raise ArgumentError, 'Direction should be :asc or :desc'
+ h.values.map(&:downcase).each do |value|
+ raise ArgumentError, "Direction '#{value}' is invalid. Valid " \
+ "directions are asc and desc." unless VALID_DIRECTIONS.include?(value)
end
end
end
@@ -1055,7 +1058,7 @@ module ActiveRecord
when Hash
arg.map { |field, dir|
field = klass.attribute_alias(field) if klass.attribute_alias?(field)
- table[field].send(dir)
+ table[field].send(dir.downcase)
}
else
arg
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 8718110c36..a08171cfcc 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -171,7 +171,6 @@ class RelationTest < ActiveRecord::TestCase
assert_equal topics(:first).title, topics.first.title
end
-
def test_finding_with_arel_order
topics = Topic.order(Topic.arel_table[:id].asc)
assert_equal 5, topics.to_a.size
@@ -194,8 +193,43 @@ class RelationTest < ActiveRecord::TestCase
assert_equal Topic.order(:id).to_sql, Topic.order(:id => :asc).to_sql
end
+ def test_finding_with_desc_order_with_string
+ topics = Topic.order(id: "desc")
+ assert_equal 5, topics.to_a.size
+ assert_equal [topics(:fifth), topics(:fourth), topics(:third), topics(:second), topics(:first)], topics.to_a
+ end
+
+ def test_finding_with_asc_order_with_string
+ topics = Topic.order(id: 'asc')
+ assert_equal 5, topics.to_a.size
+ assert_equal [topics(:first), topics(:second), topics(:third), topics(:fourth), topics(:fifth)], topics.to_a
+ end
+
+ def test_nothing_raises_on_upcase_desc_arg
+ Topic.order(id: "DESC")
+ end
+
+ def test_nothing_raises_on_downcase_desc_arg
+ Topic.order(id: "desc")
+ end
+
+ def test_nothing_raises_on_upcase_asc_arg
+ Topic.order(id: "ASC")
+ end
+
+ def test_nothing_raises_on_downcase_asc_arg
+ Topic.order(id: "asc")
+ end
+
+ def test_nothing_raises_on_case_insensitive_args
+ Topic.order(id: "DeSc")
+ Topic.order(id: :DeSc)
+ Topic.order(id: "aSc")
+ Topic.order(id: :aSc)
+ end
+
def test_raising_exception_on_invalid_hash_params
- assert_raise(ArgumentError) { Topic.order(:name, "id DESC", :id => :DeSc) }
+ assert_raise(ArgumentError) { Topic.order(:name, "id DESC", id: :asfsdf) }
end
def test_finding_last_with_arel_order