Project

General

Profile

Actions

Defect #33912

open

Sorting of multiple columns including Date column does not work

Added by Yuichi HARADA over 3 years ago. Updated over 1 year ago.

Status:
Confirmed
Priority:
Normal
Assignee:
-
Category:
Time tracking
Start date:
Due date:
% Done:

0%

Estimated time:
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

Files

customquery.png (115 KB) customquery.png Yuichi HARADA, 2020-08-27 07:17
spenttime-with-customquery.png (155 KB) spenttime-with-customquery.png Yuichi HARADA, 2020-08-27 07:21
fixed-33912.patch (3.17 KB) fixed-33912.patch Yuichi HARADA, 2020-09-02 07:30
Actions #1

Updated by Yuichi HARADA over 3 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(',')))
Actions #2

Updated by Go MAEDA over 3 years ago

  • Status changed from New to Confirmed
Actions #3

Updated by Felix Schäfer over 1 year 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?

Actions #4

Updated by Go MAEDA over 1 year 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.

Actions #5

Updated by Go MAEDA over 1 year ago

  • Target version set to Candidate for next major release
Actions

Also available in: Atom PDF