Get rid of sort_helper when using queries.

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

11 files changed, 89 insertions, 146 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>