Get rid of sort_helper when using queries.

git-svn-id: http://svn.redmine.org/redmine/trunk@16390 e93f8b46-1217-0410-a6f0-8f06a7374b81

17 files changed, 203 insertions, 163 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index a04c1a6b0..2b9512fc7 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -37,15 +37,10 @@ class IssuesController < ApplicationController
   helper :queries
   include QueriesHelper
   helper :repositories
-  helper :sort
-  include SortHelper
   helper :timelog
 
   def index
     retrieve_query
-    sort_init(@query.sort_criteria.empty? ? [['id', 'desc']] : @query.sort_criteria)
-    sort_update(@query.sortable_columns)
-    @query.sort_criteria = sort_criteria.to_a
 
     if @query.valid?
       case params[:format]
@@ -66,9 +61,7 @@ class IssuesController < ApplicationController
       @issue_count = @query.issue_count
       @issue_pages = Paginator.new @issue_count, @limit, params['page']
       @offset ||= @issue_pages.offset
-      @issues = @query.issues(:order => sort_clause,
-                              :offset => @offset,
-                              :limit => @limit)
+      @issues = @query.issues(:offset => @offset, :limit => @limit)
 
       respond_to do |format|
         format.html { render :template => 'issues/index', :layout => !request.xhr? }
@@ -421,10 +414,8 @@ class IssuesController < ApplicationController
     else
       retrieve_query_from_session
       if @query
-        sort_init(@query.sort_criteria.empty? ? [['id', 'desc']] : @query.sort_criteria)
-        sort_update(@query.sortable_columns, 'issues_index_sort')
         limit = 500
-        issue_ids = @query.issue_ids(:order => sort_clause, :limit => (limit + 1))
+        issue_ids = @query.issue_ids(:limit => (limit + 1))
         if (idx = issue_ids.index(@issue.id)) && idx < limit
           if issue_ids.size < 500
             @issue_position = idx + 1
diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb
index 106c0692d..cce9c1e69 100644
--- a/app/controllers/journals_controller.rb
+++ b/app/controllers/journals_controller.rb
@@ -27,13 +27,9 @@ class JournalsController < ApplicationController
   helper :custom_fields
   helper :queries
   include QueriesHelper
-  helper :sort
-  include SortHelper
 
   def index
     retrieve_query
-    sort_init 'id', 'desc'
-    sort_update(@query.sortable_columns)
     if @query.valid?
       @journals = @query.journals(:order => "#{Journal.table_name}.created_on DESC",
                                   :limit => 25)
diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb
index a88ede4a5..9cfe16312 100644
--- a/app/controllers/timelog_controller.rb
+++ b/app/controllers/timelog_controller.rb
@@ -32,8 +32,6 @@ class TimelogController < ApplicationController
 
   rescue_from Query::StatementInvalid, :with => :query_statement_invalid
 
-  helper :sort
-  include SortHelper
   helper :issues
   include TimelogHelper
   helper :custom_fields
@@ -43,9 +41,7 @@ class TimelogController < ApplicationController
 
   def index
     retrieve_time_entry_query
-    sort_init(@query.sort_criteria.empty? ? [['spent_on', 'desc']] : @query.sort_criteria)
-    sort_update(@query.sortable_columns)
-    scope = time_entry_scope(:order => sort_clause).
+    scope = time_entry_scope.
       preload(:issue => [:project, :tracker, :status, :assigned_to, :priority]).
       preload(:project, :user)
 
diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb
index 197bbb998..1d0db2877 100644
--- a/app/helpers/queries_helper.rb
+++ b/app/helpers/queries_helper.rb
@@ -161,10 +161,28 @@ module QueriesHelper
     content_tag('span', label + " " + value, :class => "total-for-#{column.name.to_s.dasherize}")
   end
 
-  def column_header(column)
-    column.sortable ? sort_header_tag(column.name.to_s, :caption => column.caption,
-                                                        :default_order => column.default_order) :
-                      content_tag('th', h(column.caption))
+  def column_header(query, column)
+    if column.sortable?
+      css, order = nil, column.default_order
+      if column.name.to_s == query.sort_criteria.first_key
+        if query.sort_criteria.first_asc?
+          css = 'sort asc'
+          order = 'desc'
+        else
+          css = 'sort desc'
+          order = 'asc'
+        end
+      end
+      sort_param = { :sort => query.sort_criteria.add(column.name, order).to_param }
+      content = link_to(column.caption,
+          {:params => request.query_parameters.merge(sort_param)},
+          :title => l(:label_sort_by, "\"#{column.caption}\""),
+          :class => css
+        )
+    else
+      content = column.caption
+    end
+    content_tag('th', content)
   end
 
   def column_content(column, item)
@@ -257,19 +275,26 @@ module QueriesHelper
       raise ::Unauthorized unless @query.visible?
       @query.project = @project
       session[session_key] = {:id => @query.id, :project_id => @query.project_id} if use_session
-      sort_clear
     elsif api_request? || params[:set_filter] || !use_session || session[session_key].nil? || session[session_key][:project_id] != (@project ? @project.id : nil)
       # Give it a name, required to be valid
       @query = klass.new(:name => "_", :project => @project)
       @query.build_from_params(params)
-      session[session_key] = {:project_id => @query.project_id, :filters => @query.filters, :group_by => @query.group_by, :column_names => @query.column_names, :totalable_names => @query.totalable_names} if use_session
+      session[session_key] = {:project_id => @query.project_id, :filters => @query.filters, :group_by => @query.group_by, :column_names => @query.column_names, :totalable_names => @query.totalable_names, :sort => @query.sort_criteria.to_a} if use_session
     else
       # retrieve from session
       @query = nil
       @query = klass.find_by_id(session[session_key][:id]) if session[session_key][:id]
-      @query ||= klass.new(:name => "_", :filters => session[session_key][:filters], :group_by => session[session_key][:group_by], :column_names => session[session_key][:column_names], :totalable_names => session[session_key][:totalable_names])
+      @query ||= klass.new(:name => "_", :filters => session[session_key][:filters], :group_by => session[session_key][:group_by], :column_names => session[session_key][:column_names], :totalable_names => session[session_key][:totalable_names], :sort_criteria => session[session_key][:sort])
       @query.project = @project
     end
+    if params[:sort].present?
+      @query.sort_criteria = params[:sort]
+      if use_session
+        session[session_key] ||= {}
+        session[session_key][:sort] = @query.sort_criteria.to_a
+      end
+    end
+    @query
   end
 
   def retrieve_query_from_session(klass=IssueQuery)
@@ -281,7 +306,7 @@ module QueriesHelper
         @query = IssueQuery.find_by_id(session_data[:id])
         return unless @query
       else
-        @query = IssueQuery.new(:name => "_", :filters => session_data[:filters], :group_by => session_data[:group_by], :column_names => session_data[:column_names], :totalable_names => session_data[:totalable_names])
+        @query = IssueQuery.new(:name => "_", :filters => session_data[:filters], :group_by => session_data[:group_by], :column_names => session_data[:column_names], :totalable_names => session_data[:totalable_names], :sort_criteria => session[session_key][:sort])
       end
       if session_data.has_key?(:project_id)
         @query.project_id = session_data[:project_id]
@@ -318,9 +343,16 @@ module QueriesHelper
     if query.group_by.present?
       tags << hidden_field_tag("group_by", query.group_by, :id => nil)
     end
+    if query.sort_criteria.present?
+      tags << hidden_field_tag("sort", query.sort_criteria.to_param, :id => nil)
+    end
 
     tags
   end
+ 
+  def query_hidden_sort_tag(query)
+    hidden_field_tag("sort", query.sort_criteria.to_param, :id => nil)
+  end
 
   # Returns the queries that are rendered in the sidebar
   def sidebar_queries(klass, project)
diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb
index 8fb13818f..c6bf5383e 100644
--- a/app/helpers/sort_helper.rb
+++ b/app/helpers/sort_helper.rb
@@ -53,97 +53,6 @@
 #
 
 module SortHelper
-  class SortCriteria
-
-    def initialize
-      @criteria = []
-    end
-
-    def available_criteria=(criteria)
-      unless criteria.is_a?(Hash)
-        criteria = criteria.inject({}) {|h,k| h[k] = k; h}
-      end
-      @available_criteria = criteria
-    end
-
-    def from_param(param)
-      @criteria = param.to_s.split(',').collect {|s| s.split(':')[0..1]}
-      normalize!
-    end
-
-    def criteria=(arg)
-      @criteria = arg
-      normalize!
-    end
-
-    def to_param
-      @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]
-          s = [s] unless s.is_a?(Array)
-          s.collect {|c| append_order(c, o ? "ASC" : "DESC")}
-        end
-      end.flatten.compact
-      sql.blank? ? nil : sql
-    end
-
-    def to_a
-      @criteria.dup
-    end
-
-    def add!(key, asc)
-      @criteria.delete_if {|k,o| k == key}
-      @criteria = [[key, asc]] + @criteria
-      normalize!
-    end
-
-    def add(*args)
-      r = self.class.new.from_param(to_param)
-      r.add!(*args)
-      r
-    end
-
-    def first_key
-      @criteria.first && @criteria.first.first
-    end
-
-    def first_asc?
-      @criteria.first && @criteria.first.last
-    end
-
-    def empty?
-      @criteria.empty?
-    end
-
-    private
-
-    def normalize!
-      @criteria ||= []
-      @criteria = @criteria.collect {|s| s = Array(s); [s.first, (s.last == false || s.last == 'desc') ? false : true]}
-      @criteria = @criteria.select {|k,o| @available_criteria.has_key?(k)} if @available_criteria
-      @criteria.slice!(3)
-      self
-    end
-
-    # Appends ASC/DESC to the sort criterion unless it has a fixed order
-    def append_order(criterion, order)
-      if criterion =~ / (asc|desc)$/i
-        criterion
-      else
-        "#{criterion} #{order}"
-      end
-    end
-
-    # Appends DESC to the sort criterion unless it has a fixed order
-    def append_desc(criterion)
-      append_order(criterion, "DESC")
-    end
-  end
-
   def sort_name
     controller_name + '_' + action_name + '_sort'
   end
@@ -173,10 +82,8 @@ module SortHelper
   #
   def sort_update(criteria, sort_name=nil)
     sort_name ||= self.sort_name
-    @sort_criteria = SortCriteria.new
-    @sort_criteria.available_criteria = criteria
-    @sort_criteria.from_param(params[:sort] || session[sort_name])
-    @sort_criteria.criteria = @sort_default if @sort_criteria.empty?
+    @sort_criteria = Redmine::SortCriteria.new(params[:sort] || session[sort_name] || @sort_default)
+    @sortable_columns = criteria
     session[sort_name] = @sort_criteria.to_param
   end
 
@@ -190,7 +97,7 @@ module SortHelper
   # Use this to sort the controller's table items collection.
   #
   def sort_clause()
-    @sort_criteria.to_sql
+    @sort_criteria.sort_clause(@sortable_columns)
   end
 
   def sort_criteria
diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb
index 71fe8d288..9f3bf290d 100644
--- a/app/models/issue_query.rb
+++ b/app/models/issue_query.rb
@@ -234,6 +234,10 @@ class IssueQuery < Query
     Setting.issue_list_default_totals.map(&:to_sym)
   end
 
+  def default_sort_criteria
+    [['id', 'desc']]
+  end
+
   def base_scope
     Issue.visible.joins(:status, :project).where(statement)
   end
@@ -267,7 +271,7 @@ 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]].flatten.reject(&:blank?)
+    order_option = [group_by_sort_order, (options[:order] || sort_clause)].flatten.reject(&:blank?)
 
     scope = Issue.visible.
       joins(:status, :project).
@@ -309,7 +313,7 @@ class IssueQuery < Query
 
   # Returns the issues ids
   def issue_ids(options={})
-    order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?)
+    order_option = [group_by_sort_order, (options[:order] || sort_clause)].flatten.reject(&:blank?)
 
     Issue.visible.
       joins(:status, :project).
diff --git a/app/models/query.rb b/app/models/query.rb
index 7a43ca6fc..192502024 100644
--- a/app/models/query.rb
+++ b/app/models/query.rb
@@ -377,6 +377,7 @@ class Query < ActiveRecord::Base
     self.group_by = params[:group_by] || (params[:query] && params[:query][:group_by])
     self.column_names = params[:c] || (params[:query] && params[:query][:column_names])
     self.totalable_names = params[:t] || (params[:query] && params[:query][:totalable_names])
+    self.sort_criteria = params[:sort] || (params[:query] && params[:query][:sort_criteria])
     self
   end
 
@@ -710,37 +711,40 @@ class Query < ActiveRecord::Base
     options[:totalable_names] || default_totalable_names || []
   end
 
+  def default_sort_criteria
+    []
+  end
+
   def sort_criteria=(arg)
-    c = []
-    if arg.is_a?(Hash)
-      arg = arg.keys.sort.collect {|k| arg[k]}
-    end
-    if arg
-      c = arg.select {|k,o| !k.to_s.blank?}.slice(0,3).collect {|k,o| [k.to_s, (o == 'desc' || o == false) ? 'desc' : 'asc']}
-    end
-    write_attribute(:sort_criteria, c)
+    c = Redmine::SortCriteria.new(arg)
+    write_attribute(:sort_criteria, c.to_a)
+    c
   end
 
   def sort_criteria
-    read_attribute(:sort_criteria) || []
+    c = read_attribute(:sort_criteria)
+    if c.blank?
+      c = default_sort_criteria
+    end
+    Redmine::SortCriteria.new(c)
   end
 
-  def sort_criteria_key(arg)
-    sort_criteria && sort_criteria[arg] && sort_criteria[arg].first
+  def sort_criteria_key(index)
+    sort_criteria[index].try(:first)
   end
 
-  def sort_criteria_order(arg)
-    sort_criteria && sort_criteria[arg] && sort_criteria[arg].last
+  def sort_criteria_order(index)
+    sort_criteria[index].try(:last)
   end
 
-  def sort_criteria_order_for(key)
-    sort_criteria.detect {|k, order| key.to_s == k}.try(:last)
+  def sort_clause
+    sort_criteria.sort_clause(sortable_columns)
   end
 
   # Returns the SQL sort order that should be prepended for grouping
   def group_by_sort_order
     if column = group_by_column
-      order = (sort_criteria_order_for(column.name) || column.default_order || 'asc').try(:upcase)
+      order = (sort_criteria.order_for(column.name) || column.default_order || 'asc').try(:upcase)
       Array(column.sortable).map {|s| "#{s} #{order}"}
     end
   end
@@ -878,6 +882,14 @@ class Query < ActiveRecord::Base
     totals
   end
 
+  def css_classes
+    s = sort_criteria.first
+    if s.present?
+      key, asc = s
+      "sort-by-#{key.to_s.dasherize} sort-#{asc}"
+    end
+  end
+
   private
 
   def grouped_query(&block)
diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb
index 5b39bd34e..3d37e94da 100644
--- a/app/models/time_entry_query.rb
+++ b/app/models/time_entry_query.rb
@@ -105,6 +105,10 @@ class TimeEntryQuery < Query
   def default_totalable_names
     [:hours]
   end
+ 
+  def default_sort_criteria
+    [['spent_on', 'desc']]
+  end
 
   def base_scope
     TimeEntry.visible.
@@ -114,7 +118,7 @@ class TimeEntryQuery < Query
   end
 
   def results_scope(options={})
-    order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?)
+    order_option = [group_by_sort_order, (options[:order] || sort_clause)].flatten.reject(&:blank?)
 
     base_scope.
       order(order_option).
diff --git a/app/views/issues/_list.html.erb b/app/views/issues/_list.html.erb
index 1e7d01ef7..f9172143b 100644
--- a/app/views/issues/_list.html.erb
+++ b/app/views/issues/_list.html.erb
@@ -1,7 +1,7 @@
 <%= form_tag({}, :data => {:cm_url => issues_context_menu_path}) do -%>
 <%= hidden_field_tag 'back_url', url_for(:params => request.query_parameters), :id => nil %>
 <div class="autoscroll">
-<table class="list issues odd-even <%= sort_css_classes %>">
+<table class="list issues odd-even <%= query.css_classes %>">
   <thead>
     <tr>
       <th class="checkbox hide-when-print">
@@ -9,7 +9,7 @@
               :title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}" %>
       </th>
       <% query.inline_columns.each do |column| %>
-        <%= column_header(column) %>
+        <%= column_header(query, column) %>
       <% end %>
     </tr>
   </thead>
diff --git a/app/views/queries/_query_form.html.erb b/app/views/queries/_query_form.html.erb
index 7cc01ed2f..9ca432b62 100644
--- a/app/views/queries/_query_form.html.erb
+++ b/app/views/queries/_query_form.html.erb
@@ -1,5 +1,6 @@
 <%= hidden_field_tag 'set_filter', '1' %>
 <%= hidden_field_tag 'type', @query.type, :disabled => true, :id => 'query_type' %>
+<%= query_hidden_sort_tag(@query) %>
 
 <div id="query_form_with_buttons" class="hide-when-print">
 <div id="query_form_content">
diff --git a/app/views/timelog/_list.html.erb b/app/views/timelog/_list.html.erb
index d4836056a..21115dfde 100644
--- a/app/views/timelog/_list.html.erb
+++ b/app/views/timelog/_list.html.erb
@@ -9,7 +9,7 @@
         :title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}" %>
     </th>
     <% @query.inline_columns.each do |column| %>
-      <%= column_header(column) %>
+      <%= column_header(@query, column) %>
     <% end %>
     <th></th>
   </tr>
diff --git a/lib/redmine/helpers/time_report.rb b/lib/redmine/helpers/time_report.rb
index e06499ea8..9e3448ba7 100644
--- a/lib/redmine/helpers/time_report.rb
+++ b/lib/redmine/helpers/time_report.rb
@@ -46,6 +46,7 @@ module Redmine
           time_columns = %w(tyear tmonth tweek spent_on)
           @hours = []
           @scope.includes(:activity).
+              reorder(nil).
               group(@criteria.collect{|criteria| @available_criteria[criteria][:sql]} + time_columns).
               joins(@criteria.collect{|criteria| @available_criteria[criteria][:joins]}.compact).
               sum(:hours).each do |hash, hours|
diff --git a/lib/redmine/sort_criteria.rb b/lib/redmine/sort_criteria.rb
new file mode 100644
index 000000000..5fd2e978d
--- /dev/null
+++ b/lib/redmine/sort_criteria.rb
@@ -0,0 +1,104 @@
+# Redmine - project management software
+# Copyright (C) 2006-2017  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+module Redmine
+  class SortCriteria < Array
+    def initialize(arg=nil)
+      super()
+      if arg.is_a?(Array)
+        replace arg
+      elsif arg.is_a?(String)
+        replace arg.split(',').collect {|s| s.split(':')[0..1]}
+      elsif arg.is_a?(Hash)
+        replace arg.values
+      elsif arg
+        raise ArgumentError.new("SortCriteria#new takes an Array, String or Hash, not a #{arg.class.name}.")
+      end
+      normalize!
+    end
+
+    def to_param
+      self.collect {|k,o| k + (o == 'desc' ? ':desc' : '')}.join(',')
+    end
+
+    def to_a
+      Array.new(self)
+    end
+
+    def add!(key, asc)
+      key = key.to_s
+      delete_if {|k,o| k == key}
+      prepend([key, asc])
+      normalize!
+    end
+
+    def add(*args)
+      self.class.new(self).add!(*args)
+    end
+
+    def first_key
+      first.try(:first)
+    end
+
+    def first_asc?
+      first.try(:last) == 'asc'
+    end
+
+    def key_at(arg)
+      self[arg].try(:first)
+    end
+
+    def order_at(arg)
+      self[arg].try(:last)
+    end
+
+    def order_for(key)
+      detect {|k, order| key.to_s == k}.try(:last)
+    end
+
+    def sort_clause(sortable_columns)
+      if sortable_columns.is_a?(Array)
+        sortable_columns = sortable_columns.inject({}) {|h,k| h[k]=k; h}
+      end
+
+      sql = self.collect do |k,o|
+        if s = sortable_columns[k]
+          s = [s] unless s.is_a?(Array)
+          s.collect {|c| append_order(c, o)}
+        end
+      end.flatten.compact
+      sql.blank? ? nil : sql
+    end
+
+    private
+
+    def normalize!
+      self.collect! {|s| s = Array(s); [s.first, (s.last == false || s.last.to_s == 'desc') ? 'desc' : 'asc']}
+      self.slice!(3)
+      self
+    end
+
+    # Appends ASC/DESC to the sort criterion unless it has a fixed order
+    def append_order(criterion, order)
+      if criterion =~ / (asc|desc)$/i
+        criterion
+      else
+        "#{criterion} #{order.to_s.upcase}"
+      end
+    end
+  end
+end
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 24ec49a70..914d7c386 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -682,10 +682,6 @@ class IssuesControllerTest < Redmine::ControllerTest
     get :index, :sort => 'tracker,id:desc'
     assert_response :success
 
-    sort_params = @request.session['issues_index_sort']
-    assert sort_params.is_a?(String)
-    assert_equal 'tracker,id:desc', sort_params
-
     assert_equal issues_in_list.sort_by {|issue| [issue.tracker.position, -issue.id]}, issues_in_list
     assert_select 'table.issues.sort-by-tracker.sort-asc'
   end
@@ -1402,8 +1398,7 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_show_should_display_prev_next_links_with_query_in_session
-    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => nil}
-    @request.session['issues_index_sort'] = 'id'
+    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => nil, :sort => [['id', 'asc']]}
 
     with_settings :display_subprojects_issues => '0' do
       get :show, :id => 3
@@ -1440,7 +1435,7 @@ class IssuesControllerTest < Redmine::ControllerTest
     @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => nil}
 
     %w(project tracker status priority author assigned_to category fixed_version).each do |assoc_sort|
-      @request.session['issues_index_sort'] = assoc_sort
+      @request.session[:issue_query][:sort] = [[assoc_sort, 'asc']]
 
       get :show, :id => 3
       assert_response :success, "Wrong response status for #{assoc_sort} sort"
@@ -1452,8 +1447,7 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_show_should_display_prev_next_links_with_project_query_in_session
-    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => 1}
-    @request.session['issues_index_sort'] = 'id'
+    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => 1, :sort => [['id','asc']]}
 
     with_settings :display_subprojects_issues => '0' do
       get :show, :id => 3
@@ -1468,8 +1462,7 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_show_should_not_display_prev_link_for_first_issue
-    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => 1}
-    @request.session['issues_index_sort'] = 'id'
+    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'o'}}, :project_id => 1, :sort => [['id', 'asc']]}
 
     with_settings :display_subprojects_issues => '0' do
       get :show, :id => 1
@@ -1483,8 +1476,7 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_show_should_not_display_prev_next_links_for_issue_not_in_query_results
-    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'c'}}, :project_id => 1}
-    @request.session['issues_index_sort'] = 'id'
+    @request.session[:issue_query] = {:filters => {'status_id' => {:values => [''], :operator => 'c'}}, :project_id => 1, :sort => [['id', 'asc']]}
 
     get :show, :id => 1
     assert_response :success
diff --git a/test/functional/issues_custom_fields_visibility_test.rb b/test/functional/issues_custom_fields_visibility_test.rb
index 6ebe0c48f..cd395d88c 100644
--- a/test/functional/issues_custom_fields_visibility_test.rb
+++ b/test/functional/issues_custom_fields_visibility_test.rb
@@ -215,7 +215,7 @@ class IssuesCustomFieldsVisibilityTest < Redmine::ControllerTest
     # ValueB is not visible to user and ignored while sorting
     assert_equal %w(ValueB ValueA ValueC), issues_in_list.map{|i| i.custom_field_value(@field2)}
 
-    get :index, :set_filter => '1', "cf_#{@field2.id}" => '*'
+    get :index, :set_filter => '1', "cf_#{@field2.id}" => '*', :sort => "cf_#{@field2.id}"
     assert_equal %w(ValueA ValueC), issues_in_list.map{|i| i.custom_field_value(@field2)}
 
     CustomField.update_all(:field_format => 'list')
diff --git a/test/unit/helpers/sort_helper_test.rb b/test/unit/helpers/sort_helper_test.rb
index 33657dced..069474377 100644
--- a/test/unit/helpers/sort_helper_test.rb
+++ b/test/unit/helpers/sort_helper_test.rb
@@ -64,8 +64,8 @@ class SortHelperTest < Redmine::HelperTest
     sort_init 'attr1', 'desc'
     sort_update({'attr1' => 'table1.attr1', 'attr2' => 'table2.attr2'})
 
-    assert_equal ['table1.attr1 DESC'], sort_clause
-    assert_equal 'attr1:desc', @session['foo_bar_sort']
+    assert_nil sort_clause
+    assert_equal 'invalid_key', @session['foo_bar_sort']
   end
 
   def test_invalid_order_params_sort
diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index d691f66be..885c30caa 100644
--- a/test/unit/query_test.rb
+++ b/test/unit/query_test.rb
@@ -1441,7 +1441,7 @@ class QueryTest < ActiveSupport::TestCase
 
   def test_default_sort
     q = IssueQuery.new
-    assert_equal [], q.sort_criteria
+    assert_equal [['id', 'desc']], q.sort_criteria
   end
 
   def test_set_sort_criteria_with_hash