From 92641efeecae0910c6292c0c6bf071197b24e63d Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Tue, 31 Jul 2012 10:24:18 +0300 Subject: AR::Relation#order: make new order prepend old one User.order("name asc").order("created_at desc") # SELECT * FROM users ORDER BY created_at desc, name asc This also affects order defined in `default_scope` or any kind of associations. --- activerecord/CHANGELOG.md | 9 +++++++++ activerecord/lib/active_record/relation/finder_methods.rb | 2 +- activerecord/lib/active_record/relation/query_methods.rb | 4 ++-- .../associations/has_and_belongs_to_many_associations_test.rb | 4 ++-- .../test/cases/associations/has_many_associations_test.rb | 4 ++-- activerecord/test/cases/relation_scoping_test.rb | 8 ++++---- activerecord/test/cases/relations_test.rb | 10 +++++----- guides/source/active_record_querying.textile | 9 ++++++++- guides/source/upgrading_ruby_on_rails.textile | 2 ++ 9 files changed, 35 insertions(+), 17 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 5f3b2eaf7c..00bc45e4cc 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,14 @@ ## Rails 4.0.0 (unreleased) ## +* AR::Relation#order: make new order prepend old one. + + User.order("name asc").order("created_at desc") + # SELECT * FROM users ORDER BY created_at desc, name asc + + This also affects order defined in `default_scope` or any kind of associations. + + *Bogdan Gusiev* + * `Model.all` now returns an `ActiveRecord::Relation`, rather than an array of records. Use `Model.to_a` or `Relation#to_a` if you really want an array. diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index c01aed2d8e..97c3db683f 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -311,7 +311,7 @@ module ActiveRecord @records.first else @first ||= - if order_values.empty? && primary_key + if with_default_scope.order_values.empty? && primary_key order(arel_table[primary_key].asc).limit(1).to_a.first else limit(1).to_a.first diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 94db2846f3..967b82aeb9 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -214,7 +214,7 @@ module ActiveRecord references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact! references!(references) if references.any? - self.order_values += args + self.order_values = args + self.order_values self end @@ -226,7 +226,7 @@ module ActiveRecord # # User.order('email DESC').reorder('id ASC').order('name ASC') # - # generates a query with 'ORDER BY id ASC, name ASC'. + # generates a query with 'ORDER BY name ASC, id ASC'. def reorder(*args) args.blank? ? self : spawn.reorder!(*args) end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index a9e18dd8fe..e48ac84592 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -513,9 +513,9 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal high_id_jamis, projects(:active_record).developers.find_by_name('Jamis') end - def test_find_should_append_to_association_order + def test_find_should_prepend_to_association_order ordered_developers = projects(:active_record).developers.order('projects.id') - assert_equal ['developers.name desc, developers.id desc', 'projects.id'], ordered_developers.order_values + assert_equal ['projects.id', 'developers.name desc, developers.id desc'], ordered_developers.order_values end def test_dynamic_find_all_should_respect_readonly_access diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 6122104335..6b675d3d54 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -231,9 +231,9 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, companies(:first_firm).limited_clients.limit(nil).to_a.size end - def test_find_should_append_to_association_order + def test_find_should_prepend_to_association_order ordered_clients = companies(:first_firm).clients_sorted_desc.order('companies.id') - assert_equal ['id DESC', 'companies.id'], ordered_clients.order_values + assert_equal ['companies.id', 'id DESC'], ordered_clients.order_values end def test_dynamic_find_should_respect_association_order diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index ba77bdcc8b..ef9905da2e 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -385,7 +385,7 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_scope_overwrites_default - expected = Developer.all.merge!(:order => 'salary DESC, name DESC').to_a.collect { |dev| dev.name } + expected = Developer.all.merge!(:order => ' name DESC, salary DESC').to_a.collect { |dev| dev.name } received = DeveloperOrderedBySalary.by_name.to_a.collect { |dev| dev.name } assert_equal expected, received end @@ -397,13 +397,13 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_order_after_reorder_combines_orders - expected = Developer.order('name DESC, id DESC').collect { |dev| [dev.name, dev.id] } + expected = Developer.order('id DESC, name DESC').collect { |dev| [dev.name, dev.id] } received = Developer.order('name ASC').reorder('name DESC').order('id DESC').collect { |dev| [dev.name, dev.id] } assert_equal expected, received end - def test_order_in_default_scope_should_prevail - expected = Developer.all.merge!(:order => 'salary desc').to_a.collect { |dev| dev.salary } + def test_order_in_default_scope_should_not_prevail + expected = Developer.all.merge!(:order => 'salary').to_a.collect { |dev| dev.salary } received = DeveloperOrderedBySalary.all.merge!(:order => 'salary').to_a.collect { |dev| dev.salary } assert_equal expected, received end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 9c64cb35e4..4aa1977cf9 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -165,7 +165,7 @@ class RelationTest < ActiveRecord::TestCase end def test_finding_with_order_concatenated - topics = Topic.order('author_name').order('title') + topics = Topic.order('title').order('author_name') assert_equal 4, topics.to_a.size assert_equal topics(:fourth).title, topics.first.title end @@ -1068,20 +1068,20 @@ class RelationTest < ActiveRecord::TestCase end def test_default_scope_order_with_scope_order - assert_equal 'zyke', CoolCar.order_using_new_style.limit(1).first.name - assert_equal 'zyke', FastCar.order_using_new_style.limit(1).first.name + assert_equal 'honda', CoolCar.order_using_new_style.limit(1).first.name + assert_equal 'honda', FastCar.order_using_new_style.limit(1).first.name end def test_order_using_scoping car1 = CoolCar.order('id DESC').scoping do CoolCar.all.merge!(:order => 'id asc').first end - assert_equal 'zyke', car1.name + assert_equal 'honda', car1.name car2 = FastCar.order('id DESC').scoping do FastCar.all.merge!(:order => 'id asc').first end - assert_equal 'zyke', car2.name + assert_equal 'honda', car2.name end def test_unscoped_block_style diff --git a/guides/source/active_record_querying.textile b/guides/source/active_record_querying.textile index b13932e8cb..dff829a4c1 100644 --- a/guides/source/active_record_querying.textile +++ b/guides/source/active_record_querying.textile @@ -492,7 +492,7 @@ This code will generate SQL like this: SELECT * FROM clients WHERE (clients.orders_count IN (1,3,5)) -h3. Ordering +h3(#ordering). Ordering To retrieve records from the database in a specific order, you can use the +order+ method. @@ -518,6 +518,13 @@ Client.order("orders_count ASC, created_at DESC") Client.order("orders_count ASC", "created_at DESC") +If you want to call +order+ multiple times e.g. in different context, new order will prepend previous one + + +Client.order("orders_count ASC").order("created_at DESC") +# SELECT * FROM clients ORDER BY created_at DESC, orders_count ASC + + h3. Selecting Specific Fields By default, Model.find selects all the fields from the result set using +select *+. diff --git a/guides/source/upgrading_ruby_on_rails.textile b/guides/source/upgrading_ruby_on_rails.textile index 4bf4751127..5024bc4c37 100644 --- a/guides/source/upgrading_ruby_on_rails.textile +++ b/guides/source/upgrading_ruby_on_rails.textile @@ -42,6 +42,8 @@ h4(#active_record4_0). Active Record The delete method in collection associations can now receive Fixnum or String arguments as record ids, besides records, pretty much like the destroy method does. Previously it raised ActiveRecord::AssociationTypeMismatch for such arguments. From Rails 4.0 on delete automatically tries to find the records matching the given ids before deleting them. +Rails 4.0 has changed how orders get stacked in +ActiveRecord::Relation+. In previous versions of rails new order was applied after previous defined order. But this is no long true. Check "ActiveRecord Query guide":active_record_querying.html#ordering for more information. + h4(#active_model4_0). Active Model Rails 4.0 has changed how errors attach with the ActiveModel::Validations::ConfirmationValidator. Now when confirmation validations fail the error will be attached to :#{attribute}_confirmation instead of attribute. -- cgit v1.2.3