Fixed that sorting time entries by custom field raises a SQL error (#14366).

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

diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb
index 64946a3ade28f6703b9f1a20a9078b0669f8681b..58f6070310ed5cf85a530995aab7c5ec2edfb1d5 100644 (file)
--- a/app/controllers/timelog_controller.rb
+++ b/app/controllers/timelog_controller.rb
@@ -43,10 +43,10 @@ class TimelogController < ApplicationController
 
   def index
     @query = TimeEntryQuery.build_from_params(params, :project => @project, :name => '_')
-    scope = time_entry_scope
 
     sort_init(@query.sort_criteria.empty? ? [['spent_on', 'desc']] : @query.sort_criteria)
     sort_update(@query.sortable_columns)
+    scope = time_entry_scope(:order => sort_clause)
 
     respond_to do |format|
       format.html {
@@ -55,7 +55,6 @@ class TimelogController < ApplicationController
         @entry_pages = Paginator.new @entry_count, per_page_option, params['page']
         @entries = scope.all(
           :include => [:project, :activity, :user, {:issue => :tracker}],
-          :order => sort_clause,
           :limit  =>  @entry_pages.per_page,
           :offset =>  @entry_pages.offset
         )
@@ -68,15 +67,13 @@ class TimelogController < ApplicationController
         @offset, @limit = api_offset_and_limit
         @entries = scope.all(
           :include => [:project, :activity, :user, {:issue => :tracker}],
-          :order => sort_clause,
           :limit  => @limit,
           :offset => @offset
         )
       }
       format.atom {
-        entries = scope.all(
+        entries = scope.reorder("#{TimeEntry.table_name}.created_on DESC").all(
           :include => [:project, :activity, :user, {:issue => :tracker}],
-          :order => "#{TimeEntry.table_name}.created_on DESC",
           :limit => Setting.feeds_limit.to_i
         )
         render_feed(entries, :title => l(:label_spent_time))
@@ -84,8 +81,7 @@ class TimelogController < ApplicationController
       format.csv {
         # Export all entries
         @entries = scope.all(
-          :include => [:project, :activity, :user, {:issue => [:tracker, :assigned_to, :priority]}],
-          :order => sort_clause
+          :include => [:project, :activity, :user, {:issue => [:tracker, :assigned_to, :priority]}]
         )
         send_data(query_to_csv(@entries, @query, params), :type => 'text/csv; header=present', :filename => 'timelog.csv')
       }
@@ -296,8 +292,8 @@ private
   end
 
   # Returns the TimeEntry scope for index and report actions
-  def time_entry_scope
-    scope = TimeEntry.visible.where(@query.statement)
+  def time_entry_scope(options={})
+    scope = @query.results_scope(options)
     if @issue
       scope = scope.on_issue(@issue)
     elsif @project

diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb
index 3a6ab64ef16bebed8780102cb10d9c9d1ebc8657..3325b55a8677c7fb06960140e04999f2d0b184ff 100644 (file)
--- a/app/models/time_entry_query.rb
+++ b/app/models/time_entry_query.rb
@@ -100,6 +100,15 @@ class TimeEntryQuery < Query
     @default_columns_names ||= [:project, :spent_on, :user, :activity, :issue, :comments, :hours]
   end
 
+  def results_scope(options={})
+    order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?)
+
+    TimeEntry.visible.
+      where(statement).
+      order(order_option).
+      joins(joins_for_order_statement(order_option.join(',')))
+  end
+
   # Accepts :from/:to params as shortcut filters
   def build_from_params(params)
     super

diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb
index 5735260ca7f8e065855975a8ae6d57545489fc23..9d07d3e21a8ceb17c5de38e8c9b4d94faaaf72db 100644 (file)
--- a/test/functional/timelog_controller_test.rb
+++ b/test/functional/timelog_controller_test.rb
@@ -559,6 +559,24 @@ class TimelogControllerTest < ActionController::TestCase
     assert_select "td.#{field_name}", :text => 'CF Value'
   end
 
+  def test_index_with_time_entry_custom_field_sorting
+    field = TimeEntryCustomField.generate!(:field_format => 'string', :name => 'String Field')
+    TimeEntry.generate!(:hours => 2.5, :custom_field_values => {field.id => 'CF Value 1'})
+    TimeEntry.generate!(:hours => 2.5, :custom_field_values => {field.id => 'CF Value 3'})
+    TimeEntry.generate!(:hours => 2.5, :custom_field_values => {field.id => 'CF Value 2'})
+    field_name = "cf_#{field.id}"
+
+    get :index, :c => ["hours", field_name], :sort => field_name
+    assert_response :success
+    assert_include field_name.to_sym, assigns(:query).column_names
+    assert_select "th a.sort", :text => 'String Field'
+
+    # Make sure that values are properly sorted
+    values = assigns(:entries).map {|e| e.custom_field_value(field)}.compact
+    assert_equal 3, values.size
+    assert_equal values.sort, values
+  end
+
   def test_index_atom_feed
     get :index, :project_id => 1, :format => 'atom'
     assert_response :success