Pass the order option as an array to satisfy sqlserver adapter (#12713).

Unlike other adapters, the sqlserver adapter processes the order option and wipes it when using functions.
Here we can see a "ASC" inserted in the COALESCE call:

irb(main):001:0> Issue.order("coalesce(estimated_hours, 0), id").to_sql
=> "SELECT [issues].* FROM [issues] ORDER BY coalesce(estimated_hours ASC, 0) ASC, id ASC"

This does not happen when passing the order SQL fragments separately.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11115 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/helpers/sort_helper.rb

@@ -80,12 +80,13 @@ module SortHelper
       @criteria.collect {|k,o| k + (o ? '' : ':desc')}.join(',')
     end
 
+    # Returns an array of SQL fragments used to sort the list
     def to_sql
       sql = @criteria.collect do |k,o|
         if s = @available_criteria[k]
-          (o ? s.to_a : s.to_a.collect {|c| append_desc(c)}).join(', ')
+          (o ? s.to_a : s.to_a.collect {|c| append_desc(c)})
         end
-      end.compact.join(', ')
+      end.flatten.compact
       sql.blank? ? nil : sql
     end
 

app/models/issue_query.rb

@@ -270,14 +270,13 @@ class IssueQuery < Query
   # Returns the issues
   # Valid options are :order, :offset, :limit, :include, :conditions
   def issues(options={})
-    order_option = [group_by_sort_order, options[:order]].reject {|s| s.blank?}.join(',')
+    order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?)
-    order_option = nil if order_option.blank?
 
     issues = Issue.visible.where(options[:conditions]).all(
       :include => ([:status, :project] + (options[:include] || [])).uniq,
       :conditions => statement,
       :order => order_option,
-      :joins => joins_for_order_statement(order_option),
+      :joins => joins_for_order_statement(order_option.join(',')),
       :limit  => options[:limit],
       :offset => options[:offset]
     )
@@ -295,13 +294,12 @@ class IssueQuery < Query
 
   # Returns the issues ids
   def issue_ids(options={})
-    order_option = [group_by_sort_order, options[:order]].reject {|s| s.blank?}.join(',')
+    order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?)
-    order_option = nil if order_option.blank?
 
     Issue.visible.scoped(:conditions => options[:conditions]).scoped(:include => ([:status, :project] + (options[:include] || [])).uniq,
                      :conditions => statement,
                      :order => order_option,
-                     :joins => joins_for_order_statement(order_option),
+                     :joins => joins_for_order_statement(order_option.join(',')),
                      :limit  => options[:limit],
                      :offset => options[:offset]).find_ids
   rescue ::ActiveRecord::StatementInvalid => e

test/unit/helpers/sort_helper_test.rb

@@ -30,21 +30,21 @@ class SortHelperTest < ActionView::TestCase
     sort_init 'attr1', 'desc'
     sort_update(['attr1', 'attr2'])
 
-    assert_equal 'attr1 DESC', sort_clause
+    assert_equal ['attr1 DESC'], sort_clause
   end
 
   def test_default_sort_clause_with_hash
     sort_init 'attr1', 'desc'
     sort_update({'attr1' => 'table1.attr1', 'attr2' => 'table2.attr2'})
 
-    assert_equal 'table1.attr1 DESC', sort_clause
+    assert_equal ['table1.attr1 DESC'], sort_clause
   end
 
   def test_default_sort_clause_with_multiple_columns
     sort_init 'attr1', 'desc'
     sort_update({'attr1' => ['table1.attr1', 'table1.attr2'], 'attr2' => 'table2.attr2'})
 
-    assert_equal 'table1.attr1 DESC, table1.attr2 DESC', sort_clause
+    assert_equal ['table1.attr1 DESC', 'table1.attr2 DESC'], sort_clause
   end
 
   def test_params_sort
@@ -53,7 +53,7 @@ class SortHelperTest < ActionView::TestCase
     sort_init 'attr1', 'desc'
     sort_update({'attr1' => 'table1.attr1', 'attr2' => 'table2.attr2'})
 
-    assert_equal 'table1.attr1, table2.attr2 DESC', sort_clause
+    assert_equal ['table1.attr1', 'table2.attr2 DESC'], sort_clause
     assert_equal 'attr1,attr2:desc', @session['foo_bar_sort']
   end
 
@@ -63,7 +63,7 @@ class SortHelperTest < ActionView::TestCase
     sort_init 'attr1', 'desc'
     sort_update({'attr1' => 'table1.attr1', 'attr2' => 'table2.attr2'})
 
-    assert_equal 'table1.attr1 DESC', sort_clause
+    assert_equal ['table1.attr1 DESC'], sort_clause
     assert_equal 'attr1:desc', @session['foo_bar_sort']
   end
 
@@ -73,7 +73,7 @@ class SortHelperTest < ActionView::TestCase
     sort_init 'attr1', 'desc'
     sort_update({'attr1' => 'table1.attr1', 'attr2' => 'table2.attr2'})
 
-    assert_equal 'table1.attr1, table2.attr2', sort_clause
+    assert_equal ['table1.attr1', 'table2.attr2'], sort_clause
     assert_equal 'attr1,attr2', @session['foo_bar_sort']
   end