https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292020-06-04T16:03:21ZRedmineRedmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=980622020-06-04T16:03:21ZGo MAEDA
<ul><li><strong>Category</strong> set to <i>Issues list</i></li><li><strong>Status</strong> changed from <i>New</i> to <i>Confirmed</i></li><li><strong>Target version</strong> set to <i>Candidate for next minor release</i></li></ul> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=980662020-06-05T02:40:53ZGo MAEDA
<ul></ul><p>Thank you for the fix. I have confirmed the issue.</p>
<p>Here is a test to catch the issue.</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index e4da5e1aa..c068de162 100644
</span><span class="gd">--- a/test/unit/query_test.rb
</span><span class="gi">+++ b/test/unit/query_test.rb
</span><span class="p">@@ -1629,6 +1629,8 @@</span> class QueryTest < ActiveSupport::TestCase
field.update_attribute :multiple, true
q = IssueQuery.new
<span class="gi">+ column = q.available_columns.detect {|c| c.name.to_s == 'cf_1'}
+ assert !column.sortable?
</span> assert !q.sortable_columns['cf_1']
end
</code></pre> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=980672020-06-05T04:24:38ZMischa The Evil
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/98067/diff?detail_id=79546">diff</a>)</li></ul><p>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?</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=980692020-06-05T06:53:13ZGo MAEDA
<ul></ul><p>Mischa The Evil wrote:</p>
<blockquote>
<p>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?</p>
</blockquote>
<p>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.</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=980882020-06-07T06:30:26ZMischa The Evil
<ul><li><strong>Assignee</strong> set to <i>Mischa The Evil</i></li></ul><p>I'll do some research to see if I can dig up some (historical) context for this issue.</p>
<p>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.</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1011192021-02-27T12:41:27ZMarius BĂLTEANU
<ul><li><strong>Target version</strong> changed from <i>Candidate for next minor release</i> to <i>4.0.8</i></li></ul><p>I'm assigning this to <a class="version" href="https://www.redmine.org/versions/161">4.0.8</a>, we should fix it.</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1011202021-02-27T12:52:21ZMarius BĂLTEANU
<ul></ul><p>I took a look in the code, but I didn't find what changed the behaviour in <a class="version" href="https://www.redmine.org/versions/110">3.3.0</a>.</p>
<p>Anyway, looking to the implementation, this line (<a class="source" href="https://www.redmine.org/projects/redmine/repository/svn/entry/trunk/app/models/query.rb#L123">source:/trunk/app/models/query.rb#L123</a>) never returns <code>nil</code> because of the <code>|| false</code>. To be more specific:<br /><pre><code class="ruby syntaxhl"><span class="nb">self</span><span class="p">.</span><span class="nf">sortable</span> <span class="o">=</span> <span class="n">custom_field</span><span class="p">.</span><span class="nf">order_statement</span> <span class="o">||</span> <span class="kp">false</span>
</code></pre></p>
<p>If <code>custom_field.order_statement</code> returns <code>nil</code>, then the expression returns <code>false</code>.</p>
<p>From my point of view, the patch proposed by Vincent is safe, but I've added it to the CI to check the test results.</p>
<p>Another fix that should work is the following<br /><pre><code class="ruby syntaxhl"> <span class="c1"># Returns true if the column is sortable, otherwise false</span>
<span class="k">def</span> <span class="nf">sortable?</span>
<span class="o">-</span> <span class="o">!</span><span class="vi">@sortable</span><span class="p">.</span><span class="nf">blank?</span>
<span class="o">+</span> <span class="vi">@sortable</span>
<span class="k">end</span>
</code></pre></p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1011212021-02-27T13:15:52ZMarius BĂLTEANU
<ul></ul><p>The tests pass with both fixes:</p>
<p>1. <a class="external" href="https://gitlab.com/redmine-org/redmine/-/pipelines/262768044">https://gitlab.com/redmine-org/redmine/-/pipelines/262768044</a><br />2. <a class="external" href="https://gitlab.com/redmine-org/redmine/-/pipelines/262768066">https://gitlab.com/redmine-org/redmine/-/pipelines/262768066</a></p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1012302021-03-08T09:10:08ZMarkus Boremski
<ul></ul><p>Should we change the Target-Version?</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1013122021-03-13T14:14:55ZMarius BĂLTEANU
<ul><li><strong>File</strong> <a href="/attachments/26802">0003-Add-test-for-33548.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/26802/0003-Add-test-for-33548.patch">0003-Add-test-for-33548.patch</a> added</li><li><strong>Assignee</strong> changed from <i>Mischa The Evil</i> to <i>Go MAEDA</i></li></ul><p>Mischa The Evil wrote:</p>
<blockquote>
<p>I'll do some research to see if I can dig up some (historical) context for this issue.</p>
<p>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.</p>
</blockquote>
<p>Mischa, please let us know when you find something.</p>
<p>Until then, I'm assigning this to Go Maeda in order to include the fix in <a class="version" href="https://www.redmine.org/versions/161">4.0.8</a>. The patch proposed by Vincent is safe from my point of view, I'm attaching a test case. We can refactor then in a major version.</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1013132021-03-14T03:18:29ZGo MAEDA
<ul><li><strong>Subject</strong> changed from <i>Fix column header when column is not sortable</i> to <i>Column header is clickable even when the column is not actually sortable</i></li><li><strong>Status</strong> changed from <i>Confirmed</i> to <i>Resolved</i></li><li><strong>Resolution</strong> set to <i>Fixed</i></li></ul><p>Committed the patch. Thank you.</p> Redmine - Defect #33548: Column header is clickable even when the column is not actually sortablehttps://www.redmine.org/issues/33548?journal_id=1013142021-03-14T06:59:34ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Closed</i></li></ul>