Merge pull request #3321 from consul/backport-fix_random_order

Fix random proposals order in the same session

app/controllers/budgets/investments_controller.rb

@@ -4,6 +4,7 @@ module Budgets
     include FeatureFlags
     include CommentableActions
     include FlagActions
+    include RandomSeed
     include ImageAttributes
 
     PER_PAGE = 10
@@ -119,16 +120,6 @@ module Budgets
         @investment_votes = current_user ? current_user.budget_investment_votes(investments) : {}
       end
 
-      def set_random_seed
-        if params[:order] == 'random' || params[:order].blank?
-          seed = params[:random_seed] || session[:random_seed] || rand
-          params[:random_seed] = seed
-          session[:random_seed] = params[:random_seed]
-        else
-          params[:random_seed] = nil
-        end
-      end
-
       def investment_params
         params.require(:budget_investment)
               .permit(:title, :description, :heading_id, :tag_list,
@@ -170,7 +161,7 @@ module Budgets
       def investments
         if @current_order == 'random'
           @budget.investments.apply_filters_and_search(@budget, params, @current_filter)
-                             .send("sort_by_#{@current_order}", params[:random_seed])
+                             .sort_by_random(session[:random_seed])
         else
           @budget.investments.apply_filters_and_search(@budget, params, @current_filter)
                              .send("sort_by_#{@current_order}")

app/controllers/concerns/random_seed.rb

@@ -0,0 +1,10 @@
+module RandomSeed
+  extend ActiveSupport::Concern
+
+  def set_random_seed
+    seed = (params[:random_seed] || session[:random_seed] || rand(10_000_000)).to_i
+
+    session[:random_seed] = seed
+    params[:random_seed] = seed
+  end
+end

app/controllers/legislation/processes_controller.rb

@@ -1,4 +1,6 @@
 class Legislation::ProcessesController < Legislation::BaseController
+  include RandomSeed
+
   has_filters %w[open past], only: :index
   has_filters %w[random winners], only: :proposals
 
@@ -103,7 +105,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?
+
+    if @current_filter == "random"
+      @proposals = @proposals.sort_by_random(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)
@@ -123,14 +130,4 @@ class Legislation::ProcessesController < Legislation::BaseController
       return if member_method?
       @process = ::Legislation::Process.find(params[:process_id])
     end
-
-    def set_random_seed
-      seed = (params[:random_seed] || session[:random_seed] || rand).to_f
-      seed = (-1..1).cover?(seed) ? seed : 1
-
-      session[:random_seed] = seed
-      params[:random_seed] = seed
-
-      ::Legislation::Proposal.connection.execute "select setseed(#{seed})"
-    end
 end

app/models/budget/investment.rb

@@ -25,6 +25,7 @@ class Budget
     include Filterable
     include Flaggable
     include Milestoneable
+    include Randomizable
 
     belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id'
     belongs_to :heading
@@ -55,7 +56,6 @@ class Budget
     scope :sort_by_confidence_score, -> { reorder(confidence_score: :desc, id: :desc) }
     scope :sort_by_ballots,          -> { reorder(ballot_lines_count: :desc, id: :desc) }
     scope :sort_by_price,            -> { reorder(price: :desc, confidence_score: :desc, id: :desc) }
-    scope :sort_by_random,           ->(seed) { reorder("budget_investments.id % #{seed.to_f.nonzero? ? seed.to_f : 1}, budget_investments.id") }
 
     scope :sort_by_id, -> { order("id DESC") }
     scope :sort_by_title, -> { order("title ASC") }

app/models/concerns/randomizable.rb

@@ -0,0 +1,16 @@
+module Randomizable
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def sort_by_random(seed = rand(10_000_000))
+      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
+  end
+end

app/models/debate.rb

@@ -12,6 +12,7 @@ class Debate < ActiveRecord::Base
   include Graphqlable
   include Relationable
   include Notifiable
+  include Randomizable
 
   acts_as_votable
   acts_as_paranoid column: :hidden_at
@@ -37,7 +38,6 @@ class Debate < ActiveRecord::Base
   scope :sort_by_confidence_score, -> { reorder(confidence_score: :desc) }
   scope :sort_by_created_at,       -> { reorder(created_at: :desc) }
   scope :sort_by_most_commented,   -> { reorder(comments_count: :desc) }
-  scope :sort_by_random,           -> { reorder("RANDOM()") }
   scope :sort_by_relevance,        -> { all }
   scope :sort_by_flags,            -> { order(flags_count: :desc, updated_at: :desc) }
   scope :sort_by_recommendations,  -> { order(cached_votes_total: :desc) }

app/models/legislation/proposal.rb

@@ -12,6 +12,7 @@ class Legislation::Proposal < ActiveRecord::Base
   include Documentable
   include Notifiable
   include Imageable
+  include Randomizable
 
   documentable max_documents_allowed: 3,
                max_file_size: 3.megabytes,
@@ -48,11 +49,9 @@ 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 :winners,                  -> { selected.sort_by_confidence_score }
 
   def to_param

app/models/proposal.rb

@@ -21,6 +21,7 @@ class Proposal < ActiveRecord::Base
   include EmbedVideosHelper
   include Relationable
   include Milestoneable
+  include Randomizable
 
   acts_as_votable
   acts_as_paranoid column: :hidden_at
@@ -58,7 +59,6 @@ class Proposal < ActiveRecord::Base
   scope :sort_by_confidence_score, -> { reorder(confidence_score: :desc) }
   scope :sort_by_created_at,       -> { reorder(created_at: :desc) }
   scope :sort_by_most_commented,   -> { reorder(comments_count: :desc) }
-  scope :sort_by_random,           -> { reorder("RANDOM()") }
   scope :sort_by_relevance,        -> { all }
   scope :sort_by_flags,            -> { order(flags_count: :desc, updated_at: :desc) }
   scope :sort_by_archival_date,    -> { archived.sort_by_confidence_score }

spec/features/budgets/investments_spec.rb

@@ -685,16 +685,16 @@ feature 'Budget Investments' do
       expect(current_url).to include('page=1')
     end
 
-    scenario 'Each user has a different and consistent random budget investment order when random_seed is disctint' do
+    scenario "Each user has a different and consistent random budget investment order" do
       (Kaminari.config.default_per_page * 1.3).to_i.times { create(:budget_investment, heading: heading) }
 
       in_browser(:one) do
-        visit budget_investments_path(budget, heading: heading, random_seed: rand)
+        visit budget_investments_path(budget, heading: heading)
         @first_user_investments_order = investments_order
       end
 
       in_browser(:two) do
-        visit budget_investments_path(budget, heading: heading, random_seed: rand)
+        visit budget_investments_path(budget, heading: heading)
         @second_user_investments_order = investments_order
       end