Add support for :order option to with_scope. Closes #3887. [eric.daspet@survol.net]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4274 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 36 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 0f7804a896..b85ae69707 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Add support for :order option to with_scope. Closes #3887. [eric.daspet@survol.net]
+
 * Prettify output of schema_dumper by making things line up. Closes #4241 [Caio  Chassot <caio@v2studio.com>]
 
 * Make build_postgresql_databases task make databases owned by the postgres user. Closes #4790. [mlaster@metavillage.com]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index d7cbd010da..d4345807a5 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -837,7 +837,7 @@ module ActiveRecord #:nodoc:
         method_scoping.assert_valid_keys([ :find, :create ])
 
         if f = method_scoping[:find]
-          f.assert_valid_keys([ :conditions, :joins, :select, :include, :from, :offset, :limit, :readonly ])
+          f.assert_valid_keys([ :conditions, :joins, :select, :include, :from, :offset, :limit, :order, :readonly ])
           f[:readonly] = true if !f[:joins].blank? && !f.has_key?(:readonly)
         end
 
@@ -1012,8 +1012,8 @@ module ActiveRecord #:nodoc:
           add_conditions!(sql, options[:conditions], scope)
 
           sql << " GROUP BY #{options[:group]} " if options[:group]
-          sql << " ORDER BY #{options[:order]} " if options[:order]
 
+          add_order!(sql, options[:order])
           add_limit!(sql, options, scope)
 
           sql
@@ -1036,6 +1036,15 @@ module ActiveRecord #:nodoc:
           end
         end
 
+        def add_order!(sql, order)
+          if order
+            sql << " ORDER BY #{order}"
+            sql << ", #{scope(:find, :order)}" if scoped?(:find, :order)
+          else
+            sql << " ORDER BY #{scope(:find, :order)}" if scoped?(:find, :order)
+          end
+        end
+
         # The optional scope argument is for the current :find scope.
         def add_limit!(sql, options, scope = :auto)
           scope = scope(:find) if :auto == scope
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 63bc3ea6a2..c4c0742b7b 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -1164,6 +1164,29 @@ class BasicsTest < Test::Unit::TestCase
     assert_equal Developer.count, developers.size
   end
 
+  def test_scoped_find_order   
+    # Test order in scope   
+    scoped_developers = Developer.with_scope(:find => { :limit => 1, :order => 'salary DESC' }) do
+      Developer.find(:all)
+    end   
+    assert_equal 'Jamis', scoped_developers.first.name
+    assert scoped_developers.include?(developers(:jamis))
+    # Test scope without order and order in find
+    scoped_developers = Developer.with_scope(:find => { :limit => 1 }) do
+      Developer.find(:all, :order => 'salary DESC')
+    end   
+    # Test scope order + find order, find has priority
+    scoped_developers = Developer.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(:dev_10))
+    # 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))
+  end
+
   def test_base_class
     assert LoosePerson.abstract_class?
     assert !LooseDescendant.abstract_class?