Defect #33548

Fix column header when column is not sortable

Added by Vincent Robert about 1 month ago. Updated about 1 month ago.

Status:ConfirmedStart date:
Priority:NormalDue date:
Assignee:Mischa The Evil% Done:

0%

Category:Issues list
Target version:Candidate for next minor release
Resolution: Affected version:4.1.1

Description

Please find below a patch to fix the 'sortable?' method in QueryColumn class.

Currently, the method returns true if @sortable is False.
Therefore, some headers display a link when the column is actually not sortable (multi-value custom-fields columns in Projects page for instance).

Thank you for your work

diff --git a/app/models/query.rb b/app/models/query.rb
index 0dec7d211..c3311ffbf 100644
--- a/app/models/query.rb
+++ b/app/models/query.rb
@@ -51,7 +51,7 @@ class QueryColumn

   # Returns true if the column is sortable, otherwise false
   def sortable?
-    !@sortable.nil?
+    !@sortable.blank?
   end

   def sortable

patch.diff Magnifier (326 Bytes) Vincent Robert, 2020-06-03 17:14

History

#1 Updated by Go MAEDA about 1 month ago

  • Category set to Issues list
  • Status changed from New to Confirmed
  • Target version set to Candidate for next minor release

#2 Updated by Go MAEDA about 1 month ago

Thank you for the fix. I have confirmed the issue.

Here is a test to catch the issue.

diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index e4da5e1aa..c068de162 100644
--- a/test/unit/query_test.rb
+++ b/test/unit/query_test.rb
@@ -1629,6 +1629,8 @@ class QueryTest < ActiveSupport::TestCase
     field.update_attribute :multiple, true

     q = IssueQuery.new
+    column = q.available_columns.detect {|c| c.name.to_s == 'cf_1'}
+    assert !column.sortable?
     assert !q.sortable_columns['cf_1']
   end

#3 Updated by Mischa The Evil about 1 month ago

  • Description updated (diff)

I haven't looked deeply into this, but this makes me wonder: did this method ever worked correctly? And if so: what changed and was it intentional?

#4 Updated by Go MAEDA about 1 month ago

Mischa The Evil wrote:

I haven't looked deeply into this, but this makes me wonder: did this method ever worked correctly? And if so: what changed and was it intentional?

I still don't know the cause, but it appears that 3.3 or later are affected. In my test, 2.6 through 3.2 worked as expected.

#5 Updated by Mischa The Evil about 1 month ago

  • Assignee set to Mischa The Evil

I'll do some research to see if I can dig up some (historical) context for this issue.

Note: if it's already clear for someone what's exactly going on here and what the proper way to fix it would be, feel free to jump-in/take-over. I just want to make sure that we're fixing the cause of the issue at its root instead of quick-fixing it with a patch while it's not well-understood.

Also available in: Atom PDF