order should always be concatenated.

order that is declared first has highest priority in all cases.

Here are some examples.

Car.order('name desc').find(:first, :order => 'id').name

Car.named_scope_with_order.named_scope_with_another_order

Car.order('id DESC').scoping do
  Car.find(:first, :order => 'id asc')
end

No special treatment to with_scope or scoping.

Also note that if default_scope declares an order then the order
declared in default_scope has the highest priority unless
with_exclusive_scope is used.

Signed-off-by: Santiago Pastorino <santiago@wyeworks.com>

3 files changed, 24 insertions, 6 deletions

diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index d58e302cb2..07b06e15a3 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1139,18 +1139,18 @@ class BasicsTest < ActiveRecord::TestCase
     scoped_developers = Developer.send(:with_scope, :find => { :limit => 1 }) do
       Developer.find(:all, :order => 'salary DESC')
     end
-    # Test scope order + find order, find has priority
+    # Test scope order + find order, order has priority
     scoped_developers = Developer.send(:with_scope, :find => { :limit => 3, :order => 'id DESC' }) do
       Developer.find(:all, :order => 'salary ASC')
     end
     assert scoped_developers.include?(developers(:poor_jamis))
-    assert scoped_developers.include?(developers(:david))
+    assert ! scoped_developers.include?(developers(:david))
     assert ! scoped_developers.include?(developers(:jamis))
     assert_equal 3, scoped_developers.size
 
     # Test without scoped find conditions to ensure we get the right thing
-    developers = Developer.find(:all, :order => 'id', :limit => 1)
-    assert scoped_developers.include?(developers(:david))
+    assert ! scoped_developers.include?(Developer.find(1))
+    assert scoped_developers.include?(Developer.find(11))
   end
 
   def test_scoped_find_limit_offset_including_has_many_association
diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb
index cdfd62a675..e27bb65144 100644
--- a/activerecord/test/cases/relation_scoping_test.rb
+++ b/activerecord/test/cases/relation_scoping_test.rb
@@ -398,8 +398,8 @@ class DefaultScopingTest < ActiveRecord::TestCase
     assert_equal expected, received
   end
 
-  def test_overwriting_default_scope
-    expected = Developer.find(:all, :order => 'salary').collect { |dev| dev.salary }
+  def test_order_in_default_scope_should_prevail
+    expected = Developer.find(:all, :order => 'salary desc').collect { |dev| dev.salary }
     received = DeveloperOrderedBySalary.find(:all, :order => 'salary').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 be038bfa74..e21e1cb635 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -667,4 +667,22 @@ class RelationTest < ActiveRecord::TestCase
   def test_relations_limit_with_conditions_or_limit
     assert_equal Post.limit(2).size, Post.limit(2).all.size
   end
+
+  def test_order_with_find_with_order
+    assert_equal 'zyke', Car.order('name desc').find(:first, :order => 'id').name
+  end
+
+  def test_default_scope_order_with_named_scope_order
+    assert_equal 'zyke', Car.order_using_new_style.limit(1).first.name
+    assert_equal 'zyke', Car.order_using_old_style.limit(1).first.name
+  end
+
+  def test_order_using_scoping
+    car = Car.order('id DESC').scoping do
+      Car.find(:first, :order => 'id asc')
+    end
+    assert_equal 'zyke', car.name
+  end
+
+
 end