Defect #33912

Sorting of multiple columns including Date column does not work

Added by Yuichi HARADA over 2 years ago. Updated 6 months ago.

Status:ConfirmedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Time tracking
Target version:Candidate for next major release
Resolution: Affected version:

Description

When specifying the sorting condition of multiple columns including Date column in the custom query, sorting of columns after Date column does not work.

  • Defining a custom query
  • List of spent time with custom query

customquery.png (115 KB) Yuichi HARADA, 2020-08-27 07:17

spenttime-with-customquery.png (155 KB) Yuichi HARADA, 2020-08-27 07:21

fixed-33912.patch Magnifier (3.17 KB) Yuichi HARADA, 2020-09-02 07:30

History

#1 Updated by Yuichi HARADA over 2 years ago

I think the following patch will fix the problem.

diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb
index c8f8eeaeb..59a4f25ca 100644
--- a/app/models/time_entry_query.rb
+++ b/app/models/time_entry_query.rb
@@ -23,7 +23,7 @@ class TimeEntryQuery < Query

   self.available_columns = [
     QueryColumn.new(:project, :sortable => "#{Project.table_name}.name", :groupable => true),
-    QueryColumn.new(:spent_on, :sortable => ["#{TimeEntry.table_name}.spent_on", "#{TimeEntry.table_name}.created_on"], :default_order => 'desc', :groupable => true),
+    QueryColumn.new(:spent_on, :sortable => "#{TimeEntry.table_name}.spent_on", :default_order => 'desc', :groupable => true),
     TimestampQueryColumn.new(:created_on, :sortable => "#{TimeEntry.table_name}.created_on", :default_order => 'desc', :groupable => true),
     QueryColumn.new(:tweek, :sortable => ["#{TimeEntry.table_name}.spent_on", "#{TimeEntry.table_name}.created_on"], :caption => :label_week),
     QueryColumn.new(:author, :sortable => lambda {User.fields_for_order_statement}),
@@ -152,7 +152,12 @@ class TimeEntryQuery < Query
   def results_scope(options={})
     order_option = [group_by_sort_order, (options[:order] || sort_clause)].flatten.reject(&:blank?)

-    order_option << "#{TimeEntry.table_name}.id ASC" 
+    order_option <<
+      if order_option.reverse.find{|order| order.match?(/#{TimeEntry.table_name}\./)}&.match?(/\sDESC$/i)
+        "#{TimeEntry.table_name}.id DESC" 
+      else
+        "#{TimeEntry.table_name}.id ASC" 
+      end
     base_scope.
       order(order_option).
       joins(joins_for_order_statement(order_option.join(',')))

#2 Updated by Go MAEDA about 2 years ago

  • Status changed from New to Confirmed

#3 Updated by Felix Schäfer 7 months ago

We (Plan.io) can confirm this issue and concur with the first part of the proposed patch. In addition the QueryColumn should be changed to a TimestampQueryColumn.

I am not sure about the second part of the patch though. Currently the IssueQuery defaults to id DESC if no other id sorting is defined regardless of other sortings. If this is changed in TimeEntryQuery it should also be consistently change in the IssueQuery.

Go Maeda, what would be needed to commit this patch? Do you think a test would be necessary?

#4 Updated by Go MAEDA 7 months ago

Felix Schäfer wrote:

I am not sure about the second part of the patch though. Currently the IssueQuery defaults to id DESC if no other id sorting is defined regardless of other sortings. If this is changed in TimeEntryQuery it should also be consistently change in the IssueQuery.

Without the second part, the sorting order is reversed from the current and an existing test TimelogControllerTest#test_index_should_sort_by_spent_on_and_created_on will fail.

Go Maeda, what would be needed to commit this patch? Do you think a test would be necessary?

I can commit the patch if how handle the second part of the patch is decided.

#5 Updated by Go MAEDA 6 months ago

  • Target version set to Candidate for next major release

Also available in: Atom PDF