From 660c59016b5bc53c6f62287daf9227a0bdaee8b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 19 Dec 2018 16:43:19 +0100 Subject: [PATCH] Fix random proposals order in the same session Using `setseed` and ordering by `RAND()` doesn't always return the same results because, although the generated random numbers will always be the same, PostgreSQL doesn't guarantee the order of the rows it will apply those random numbers to, similar to the way it doesn't guarantee an order when the `ORDER BY` clause isn't specified. Using something like `reorder("legislation_proposals.id % #{seed}")`, like we do in budget investments, is certainly more elegant but it makes the test checking two users get different results fail sometimes, so that approach might need some adjustments in order to make the results more random. --- .../legislation/processes_controller.rb | 12 +++++++----- app/models/legislation/proposal.rb | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/controllers/legislation/processes_controller.rb b/app/controllers/legislation/processes_controller.rb index 0e5d030be..de4213191 100644 --- a/app/controllers/legislation/processes_controller.rb +++ b/app/controllers/legislation/processes_controller.rb @@ -103,7 +103,12 @@ class Legislation::ProcessesController < Legislation::BaseController @proposals = @proposals.search(params[:search]) if params[:search].present? @current_filter = "winners" if params[:filter].blank? && @proposals.winners.any? - @proposals = @proposals.send(@current_filter).page(params[:page]) + + if @current_filter == "random" + @proposals = @proposals.send(@current_filter, session[:random_seed]).page(params[:page]) + else + @proposals = @proposals.send(@current_filter).page(params[:page]) + end if @process.proposals_phase.started? || (current_user && current_user.administrator?) legislation_proposal_votes(@proposals) @@ -125,12 +130,9 @@ class Legislation::ProcessesController < Legislation::BaseController end def set_random_seed - seed = (params[:random_seed] || session[:random_seed] || rand).to_f - seed = (-1..1).cover?(seed) ? seed : 1 + seed = (params[:random_seed] || session[:random_seed] || rand(10_000_000)).to_i session[:random_seed] = seed params[:random_seed] = seed - - ::Legislation::Proposal.connection.execute "select setseed(#{seed})" end end diff --git a/app/models/legislation/proposal.rb b/app/models/legislation/proposal.rb index 15ed53428..531a46387 100644 --- a/app/models/legislation/proposal.rb +++ b/app/models/legislation/proposal.rb @@ -48,13 +48,23 @@ class Legislation::Proposal < ActiveRecord::Base scope :sort_by_title, -> { reorder(title: :asc) } scope :sort_by_id, -> { reorder(id: :asc) } scope :sort_by_supports, -> { reorder(cached_votes_score: :desc) } - scope :sort_by_random, -> { reorder("RANDOM()") } scope :sort_by_flags, -> { order(flags_count: :desc, updated_at: :desc) } scope :last_week, -> { where("proposals.created_at >= ?", 7.days.ago)} scope :selected, -> { where(selected: true) } - scope :random, -> { sort_by_random } + scope :random, -> (seed) { sort_by_random(seed) } scope :winners, -> { selected.sort_by_confidence_score } + def self.sort_by_random(seed) + ids = pluck(:id).shuffle(random: Random.new(seed)) + + return all if ids.empty? + + ids_with_order = ids.map.with_index { |id, order| "(#{id}, #{order})" }.join(", ") + + joins("LEFT JOIN (VALUES #{ids_with_order}) AS ids(id, ordering) ON #{table_name}.id = ids.id") + .order("ids.ordering") + end + def to_param "#{id}-#{title}".parameterize end