From e7bad9f05d70db666f992b55e024d77ead6db030 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 27 Nov 2020 18:12:49 +0000 Subject: [PATCH] FIX: Ensure directory items appear in a consistent order (#11370) User directory items are sorted by some activity metric. If those metrics have the same value, postgres does not guarantee the order in which they will be returned. This can cause issues in pagination - some users may appear twice, and some may be missed. To illustrate ``` pry(main)> query = DirectoryItem.where(period_type: DirectoryItem.period_types[:weekly]).order(:likes_received).limit(50); pry(main)> page1 = query.offset(0).pluck(:id); pry(main)> page2 = query.offset(50).pluck(:id); pry(main)> (page1 & page2).count # users on both pages => 29 ``` If we use the primary key to tie-break matching metrics, things are much more reliable ``` pry(main)> query = DirectoryItem.where(period_type: DirectoryItem.period_types[:weekly]).order(:likes_received, :id).limit(50); pry(main)> page1 = query.offset(0).pluck(:id); pry(main)> page2 = query.offset(50).pluck(:id); pry(main)> (page1 & page2).count # users on both pages => 0 ``` This most commonly effects new sites where all the directory metrics are zero. The fact that the ordering is indeterminate makes it difficult to write a reliable test case for this. --- app/controllers/directory_items_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/directory_items_controller.rb b/app/controllers/directory_items_controller.rb index 3f03ded0231..fd6286fab10 100644 --- a/app/controllers/directory_items_controller.rb +++ b/app/controllers/directory_items_controller.rb @@ -29,9 +29,9 @@ class DirectoryItemsController < ApplicationController order = params[:order] || DirectoryItem.headings.first dir = params[:asc] ? 'ASC' : 'DESC' if DirectoryItem.headings.include?(order.to_sym) - result = result.order("directory_items.#{order} #{dir}") + result = result.order("directory_items.#{order} #{dir}, directory_items.id") elsif params[:order] === 'username' - result = result.order("users.#{order} #{dir}") + result = result.order("users.#{order} #{dir}, directory_items.id") end if period_type == DirectoryItem.period_types[:all]