From 6682121069f48f14a9838ec9e1f23b119239ddf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 19 Dec 2018 19:18:10 +0100 Subject: [PATCH] Reuse code to set and order by a random seed --- app/controllers/budgets/investments_controller.rb | 13 ++----------- app/controllers/legislation/processes_controller.rb | 2 +- app/models/budget/investment.rb | 2 +- app/models/concerns/randomizable.rb | 4 +--- app/models/debate.rb | 2 +- app/models/proposal.rb | 2 +- spec/features/budgets/investments_spec.rb | 6 +++--- 7 files changed, 10 insertions(+), 21 deletions(-) diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index a48fa3188..0c054381d 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/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}") diff --git a/app/controllers/legislation/processes_controller.rb b/app/controllers/legislation/processes_controller.rb index 5c796224f..eeef4fd78 100644 --- a/app/controllers/legislation/processes_controller.rb +++ b/app/controllers/legislation/processes_controller.rb @@ -107,7 +107,7 @@ class Legislation::ProcessesController < Legislation::BaseController @current_filter = "winners" if params[:filter].blank? && @proposals.winners.any? if @current_filter == "random" - @proposals = @proposals.send(@current_filter, session[:random_seed]).page(params[:page]) + @proposals = @proposals.sort_by_random(session[:random_seed]).page(params[:page]) else @proposals = @proposals.send(@current_filter).page(params[:page]) end diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 8da3a1bc1..a4d3be921 100644 --- a/app/models/budget/investment.rb +++ b/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") } diff --git a/app/models/concerns/randomizable.rb b/app/models/concerns/randomizable.rb index e632b06ce..c346f1f02 100644 --- a/app/models/concerns/randomizable.rb +++ b/app/models/concerns/randomizable.rb @@ -2,7 +2,7 @@ module Randomizable extend ActiveSupport::Concern class_methods do - def sort_by_random(seed) + def sort_by_random(seed = rand(10_000_000)) ids = pluck(:id).shuffle(random: Random.new(seed)) return all if ids.empty? @@ -12,7 +12,5 @@ module Randomizable joins("LEFT JOIN (VALUES #{ids_with_order}) AS ids(id, ordering) ON #{table_name}.id = ids.id") .order("ids.ordering") end - - alias_method :random, :sort_by_random end end diff --git a/app/models/debate.rb b/app/models/debate.rb index 2aa1bdce4..08743848b 100644 --- a/app/models/debate.rb +++ b/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) } diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 50c7abd09..3cf6811e4 100644 --- a/app/models/proposal.rb +++ b/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 } diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 00cc74239..52ce89fd2 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/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