summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2016-06-23 14:15:46 +0100
committerSean McGivern <sean@gitlab.com>2016-06-23 14:33:20 +0100
commitaa973f2eecce822f70f78f0086b0550bc35d05d9 (patch)
tree45d1dfd081ea017a1c3cd94784aae6c6bb481cb5
parent2de9d66fe44fd98e6ba1264058c642d05a33f2e8 (diff)
downloadgitlab-ce-18915-pagination-with-priority-sort-repeats-results.tar.gz
Fix pagination on sorts with lots of ties18915-pagination-with-priority-sort-repeats-results
Postgres and MySQL don't guarantee that pagination with `LIMIT` and `OFFSET` will work if the ordering isn't unique. From the Postgres docs: > When using `LIMIT`, it is important to use an `ORDER BY` clause that > constrains the result rows into a unique order. Otherwise you will get > an unpredictable subset of the query's rows Before: [1] pry(main)> issues = 1.upto(Issue.count).map { |i| Issue.sort('priority').page(i).per(1).map(&:id) }.flatten [2] pry(main)> issues.count => 81 [3] pry(main)> issues.uniq.count => 42 After: [1] pry(main)> issues = 1.upto(Issue.count).map { |i| Issue.sort('priority').page(i).per(1).map(&:id) }.flatten [2] pry(main)> issues.count => 81 [3] pry(main)> issues.uniq.count => 81
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/concerns/issuable.rb21
2 files changed, 13 insertions, 9 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 8a37274c023..3841dcbef7b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 8.10.0 (unreleased)
- Wrap code blocks on Activies and Todos page. !4783 (winniehell)
- Add Sidekiq queue duration to transaction metrics.
- Fix MR-auto-close text added to description. !4836
+ - Fix pagination when sorting by columns with lots of ties (like priority)
- Implement Subresource Integrity for CSS and JavaScript assets. This prevents malicious assets from loading in the case of a CDN compromise.
v 8.9.0
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 0ccd3474b81..38da0dafad2 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -112,15 +112,18 @@ module Issuable
end
def sort(method, excluded_labels: [])
- case method.to_s
- when 'milestone_due_asc' then order_milestone_due_asc
- when 'milestone_due_desc' then order_milestone_due_desc
- when 'downvotes_desc' then order_downvotes_desc
- when 'upvotes_desc' then order_upvotes_desc
- when 'priority' then order_labels_priority(excluded_labels: excluded_labels)
- else
- order_by(method)
- end
+ sorted = case method.to_s
+ when 'milestone_due_asc' then order_milestone_due_asc
+ when 'milestone_due_desc' then order_milestone_due_desc
+ when 'downvotes_desc' then order_downvotes_desc
+ when 'upvotes_desc' then order_upvotes_desc
+ when 'priority' then order_labels_priority(excluded_labels: excluded_labels)
+ else
+ order_by(method)
+ end
+
+ # Break ties with the ID column for pagination
+ sorted.order(id: :desc)
end
def order_labels_priority(excluded_labels: [])