From 17f442c7237878e46c708f1ee80ddfef112b7d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 21 May 2020 03:04:51 +0200 Subject: [PATCH] Extract method to get a few random records In Ruby 5.2, we get a warning when using the "RANDOM()" function: DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "RANDOM()". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). This warning doesn't make much sense, though, since RANDOM() is a common function which is not dangerous at all. However, since the warning is annoying, we'll probably have to find a way to deal with it. So I'm extracting all our RANDOM() usages into a method. This way we'll only have to change one method to avoid this warning. I've chosen `sample` because it's similar to Ruby's Array#sample, and because `order_by_random` would be confusing if we consider we already have a method called `sort_by_random`. --- app/models/application_record.rb | 8 ++++++++ db/dev_seeds/budgets.rb | 2 +- db/dev_seeds/flags.rb | 6 +++--- db/dev_seeds/hiddings.rb | 12 ++++++------ db/dev_seeds/notifications.rb | 4 ++-- db/dev_seeds/polls.rb | 2 +- 6 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 10a4cba84..4c7aa15dc 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,3 +1,11 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + def self.sample(count = 1) + if count == 1 + reorder("RANDOM()").first + else + reorder("RANDOM()").limit(count) + end + end end diff --git a/db/dev_seeds/budgets.rb b/db/dev_seeds/budgets.rb index a1e4738f2..e8a1269bd 100644 --- a/db/dev_seeds/budgets.rb +++ b/db/dev_seeds/budgets.rb @@ -147,7 +147,7 @@ end section "Marking investments as visible to valuators" do (1..50).to_a.sample.times do - Budget::Investment.reorder("RANDOM()").first.update(visible_to_valuators: true) + Budget::Investment.sample.update(visible_to_valuators: true) end end diff --git a/db/dev_seeds/flags.rb b/db/dev_seeds/flags.rb index c57cac524..ebddb901a 100644 --- a/db/dev_seeds/flags.rb +++ b/db/dev_seeds/flags.rb @@ -19,7 +19,7 @@ section "Flagging Debates & Comments" do end section "Ignoring flags in Debates, comments & proposals" do - Debate.flagged.reorder("RANDOM()").limit(10).each(&:ignore_flag) - Comment.flagged.reorder("RANDOM()").limit(30).each(&:ignore_flag) - Proposal.flagged.reorder("RANDOM()").limit(10).each(&:ignore_flag) + Debate.flagged.sample(10).each(&:ignore_flag) + Comment.flagged.sample(30).each(&:ignore_flag) + Proposal.flagged.sample(10).each(&:ignore_flag) end diff --git a/db/dev_seeds/hiddings.rb b/db/dev_seeds/hiddings.rb index ed1d938d2..d91f5a886 100644 --- a/db/dev_seeds/hiddings.rb +++ b/db/dev_seeds/hiddings.rb @@ -1,11 +1,11 @@ section "Hiding debates, comments & proposals" do - Comment.with_hidden.flagged.reorder("RANDOM()").limit(30).each(&:hide) - Debate.with_hidden.flagged.reorder("RANDOM()").limit(5).each(&:hide) - Proposal.with_hidden.flagged.reorder("RANDOM()").limit(10).each(&:hide) + Comment.with_hidden.flagged.sample(30).each(&:hide) + Debate.with_hidden.flagged.sample(5).each(&:hide) + Proposal.with_hidden.flagged.sample(10).each(&:hide) end section "Confirming hiding in debates, comments & proposals" do - Comment.only_hidden.flagged.reorder("RANDOM()").limit(10).each(&:confirm_hide) - Debate.only_hidden.flagged.reorder("RANDOM()").limit(5).each(&:confirm_hide) - Proposal.only_hidden.flagged.reorder("RANDOM()").limit(5).each(&:confirm_hide) + Comment.only_hidden.flagged.sample(10).each(&:confirm_hide) + Debate.only_hidden.flagged.sample(5).each(&:confirm_hide) + Proposal.only_hidden.flagged.sample(5).each(&:confirm_hide) end diff --git a/db/dev_seeds/notifications.rb b/db/dev_seeds/notifications.rb index 5452c7f79..237d42215 100644 --- a/db/dev_seeds/notifications.rb +++ b/db/dev_seeds/notifications.rb @@ -4,10 +4,10 @@ section "Creating comment notifications" do title: Faker::Lorem.sentence(3).truncate(60), description: "

#{Faker::Lorem.paragraphs.join("

")}

", tag_list: Tag.all.sample(3).join(","), - geozone: Geozone.reorder("RANDOM()").first, + geozone: Geozone.sample, terms_of_service: "1") - comment = Comment.create!(user: User.reorder("RANDOM()").first, + comment = Comment.create!(user: User.sample, body: Faker::Lorem.sentence, commentable: debate) diff --git a/db/dev_seeds/polls.rb b/db/dev_seeds/polls.rb index e26c99d7d..9813865dc 100644 --- a/db/dev_seeds/polls.rb +++ b/db/dev_seeds/polls.rb @@ -13,7 +13,7 @@ section "Creating polls" do starts_at: 5.days.ago, ends_at: 5.days.from_now, geozone_restricted: true, - geozones: Geozone.reorder("RANDOM()").limit(3)) + geozones: Geozone.sample(3)) Poll.create!(name: I18n.t("seeds.polls.recounting_poll"), slug: I18n.t("seeds.polls.recounting_poll").parameterize,