Issues in paginated views may be lost because sorting criteria are not unique (#29581).

Patch by Mizuki ISHIKAWA.

git-svn-id: http://svn.redmine.org/redmine/trunk@18263 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb
index 07eac0aadcca8f9fc0b3d6a06246ef382e7f908f..589089bada2bdc4db2a17ce3a71b6709bbbdd556 100644 (file)
--- a/app/models/issue_query.rb
+++ b/app/models/issue_query.rb
@@ -297,6 +297,10 @@ class IssueQuery < Query
   # Valid options are :order, :offset, :limit, :include, :conditions
   def issues(options={})
     order_option = [group_by_sort_order, (options[:order] || sort_clause)].flatten.reject(&:blank?)
+    # The default order of IssueQuery is issues.id DESC(by IssueQuery#default_sort_criteria)
+    unless ["#{Issue.table_name}.id ASC", "#{Issue.table_name}.id DESC"].any?{|i| order_option.include?(i)}
+      order_option << "#{Issue.table_name}.id DESC"
+    end
 
     scope = Issue.visible.
       joins(:status, :project).
@@ -339,6 +343,10 @@ class IssueQuery < Query
   # Returns the issues ids
   def issue_ids(options={})
     order_option = [group_by_sort_order, (options[:order] || sort_clause)].flatten.reject(&:blank?)
+    # The default order of IssueQuery is issues.id DESC(by IssueQuery#default_sort_criteria)
+    unless ["#{Issue.table_name}.id ASC", "#{Issue.table_name}.id DESC"].any?{|i| order_option.include?(i)}
+      order_option << "#{Issue.table_name}.id DESC"
+    end
 
     Issue.visible.
       joins(:status, :project).

diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb
index 9c767266bba4dabef701667115378211f1f86b31..5ca95ea9167142ff34215f93c03d5193cd9edc2f 100644 (file)
--- a/app/models/time_entry_query.rb
+++ b/app/models/time_entry_query.rb
@@ -148,6 +148,7 @@ 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"
     base_scope.
       order(order_option).
       joins(joins_for_order_statement(order_option.join(',')))

diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index f0a67ee6f7c52c29cb5c3617c01fe7fd58705787..4e590b7592ef439eea342650ac47c9d8674c4826 100644 (file)
--- a/test/unit/query_test.rb
+++ b/test/unit/query_test.rb
@@ -2329,4 +2329,18 @@ class QueryTest < ActiveSupport::TestCase
     query.filters = {'spent_time' => {:operator => '><', :values => ['1', '2']}}
     assert_equal [3], query.issues.pluck(:id)
   end
+
+  def test_issues_should_be_in_the_same_order_when_paginating
+    q = IssueQuery.new
+    q.sort_criteria = {'0' => ['priority', 'desc']}
+    issue_ids = q.issues.pluck(:id)
+    paginated_issue_ids = []
+    # Test with a maximum of 2 records per page.
+    ((q.issue_count / 2) + 1).times do |i|
+      paginated_issue_ids += q.issues(:offset => (i * 2), :limit => 2).pluck(:id)
+    end
+
+    # Non-paginated issue ids and paginated issue ids should be in the same order.
+    assert_equal issue_ids, paginated_issue_ids
+  end
 end

diff --git a/test/unit/time_entry_query_test.rb b/test/unit/time_entry_query_test.rb
index 0d824bc3dba8915c8cf42a2ed4d8ad0b30af5916..1f8301f8f17ca8a7dcf46acfc828b9e58a7e0d4f 100644 (file)
--- a/test/unit/time_entry_query_test.rb
+++ b/test/unit/time_entry_query_test.rb
@@ -129,4 +129,19 @@ class TimeEntryQueryTest < ActiveSupport::TestCase
     query = TimeEntryQuery.new(:project => Project.find(2), :name => '_')
     assert !query.available_filters.has_key?('project.status')
   end
+
+  def test_results_scope_should_be_in_the_same_order_when_paginating
+    4.times { TimeEntry.generate! }
+    q = TimeEntryQuery.new
+    q.sort_criteria = {'0' => ['user', 'asc']}
+    time_entry_ids = q.results_scope.pluck(:id)
+    paginated_time_entry_ids = []
+    # Test with a maximum of 2 records per page.
+    ((q.results_scope.count / 2) + 1).times do |i|
+      paginated_time_entry_ids += q.results_scope.offset((i * 2)).limit(2).pluck(:id)
+    end
+
+    # Non-paginated time entry ids and paginated time entry ids should be in the same order.
+    assert_equal time_entry_ids, paginated_time_entry_ids
+  end
 end