From acbd7ab22e5d9179487bd98110234c54535036c4 Mon Sep 17 00:00:00 2001 From: Marcelo Casiraghi Date: Thu, 23 May 2013 00:17:15 -0300 Subject: [PATCH] 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. --- activerecord/CHANGELOG.md | 10 +++++ .../active_record/relation/query_methods.rb | 9 +++-- activerecord/test/cases/relations_test.rb | 38 ++++++++++++++++++- 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