diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ed16d68253..cd3ca2ab34 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Use strings to represent non-string `order_values`. + + *Yves Senn* + * Checks to see if the record contains the foreign key to set the inverse automatically. *Edo Balvers* diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 62c555f6d6..b6a7c25b4b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -995,12 +995,6 @@ module ActiveRecord 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 @@ -1016,17 +1010,6 @@ module ActiveRecord orders.reject!(&:blank?) orders = reverse_sql_order(orders) if reverse_order_value - orders = orders.flat_map do |order| - case order - when Symbol - table[order].asc - when Hash - order.map { |field, dir| table[field].send(dir) } - else - order - end - end - arel.order(*orders) unless orders.empty? end @@ -1048,8 +1031,17 @@ module ActiveRecord # if a symbol is given we prepend the quoted table name order_args.map! do |arg| - arg.is_a?(Symbol) ? Arel::Nodes::Ascending.new(klass.arel_table[arg]) : arg - end + case arg + when Symbol + table[arg].asc + when Hash + arg.map { |field, dir| + table[field].send(dir) + } + else + arg + end + end.flatten! end # Checks to make sure that the arguments are not blank. Note that if some diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 9f9244f9b2..23500bf5d8 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -146,5 +146,17 @@ class MergingDifferentRelationsTest < ActiveRecord::TestCase merge(Author.order(:name)).pluck("authors.name") assert_equal ["Bob", "Bob", "David"], posts_by_author_name + + posts_by_author_name = Post.limit(3).joins(:author). + merge(Author.order("name")).pluck("authors.name") + + assert_equal ["Bob", "Bob", "David"], posts_by_author_name + end + + test "merging order relations (using a hash argument)" do + posts_by_author_name = Post.limit(4).joins(:author). + merge(Author.order(name: :desc)).pluck("authors.name") + + assert_equal ["Mary", "Mary", "Mary", "David"], posts_by_author_name end end diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index 020fb24afa..a70f979442 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -7,10 +7,6 @@ module ActiveRecord extend ActiveRecord::Delegation::DelegateCache inherited self - def arel_table - Post.arel_table - end - def connection Post.connection end @@ -21,7 +17,7 @@ module ActiveRecord end def relation - @relation ||= Relation.new FakeKlass.new('posts'), :b + @relation ||= Relation.new FakeKlass.new('posts'), Post.arel_table end (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order]).each do |method|