From 16c16e3cdfec21997d6a92b8ca34d6b28fb9bf7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 17 May 2020 21:10:09 +0200 Subject: [PATCH] Mark safe SQL with `Arel.sql` Rails 5.2 is raising a warning in some places: DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s). 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(). IMHO this warning is simply wrong, since we're using known PostgreSQL functions like LOWER() or RANDOM(). AFAIK this code works without warnings in Rails 6.0 [1][2] However, since the warning is annoying, we need to take measures so our logs are clean. [1] https://github.com/rails/rails/commit/6c82b6c99d [2] https://github.com/rails/rails/commit/64d8c54e16 --- app/controllers/admin/geozones_controller.rb | 2 +- app/controllers/legislation/annotations_controller.rb | 2 +- app/models/application_record.rb | 4 ++-- app/models/comment.rb | 2 +- app/models/poll/question.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/geozones_controller.rb b/app/controllers/admin/geozones_controller.rb index 6e3b0843a..1b924d444 100644 --- a/app/controllers/admin/geozones_controller.rb +++ b/app/controllers/admin/geozones_controller.rb @@ -4,7 +4,7 @@ class Admin::GeozonesController < Admin::BaseController load_and_authorize_resource def index - @geozones = Geozone.all.order("LOWER(name)") + @geozones = Geozone.all.order(Arel.sql("LOWER(name)")) end def new diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index a238e064e..1d26211c2 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -61,7 +61,7 @@ class Legislation::AnnotationsController < Legislation::BaseController end def search - @annotations = @draft_version.annotations.order("LENGTH(quote) DESC") + @annotations = @draft_version.annotations.order(Arel.sql("LENGTH(quote) DESC")) annotations_hash = { total: @annotations.size, rows: @annotations } render json: annotations_hash.to_json(methods: :weight) end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 4c7aa15dc..50235a9ca 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -3,9 +3,9 @@ class ApplicationRecord < ActiveRecord::Base def self.sample(count = 1) if count == 1 - reorder("RANDOM()").first + reorder(Arel.sql("RANDOM()")).first else - reorder("RANDOM()").limit(count) + reorder(Arel.sql("RANDOM()")).limit(count) end end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 7af862a73..f6607a849 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -49,7 +49,7 @@ class Comment < ApplicationRecord scope :sort_by_most_voted, -> { order(confidence_score: :desc, created_at: :desc) } scope :sort_descendants_by_most_voted, -> { order(confidence_score: :desc, created_at: :asc) } - scope :sort_by_supports, -> { order("cached_votes_up - cached_votes_down DESC") } + scope :sort_by_supports, -> { order(Arel.sql("cached_votes_up - cached_votes_down DESC")) } scope :sort_by_newest, -> { order(created_at: :desc) } scope :sort_descendants_by_newest, -> { order(created_at: :desc) } diff --git a/app/models/poll/question.rb b/app/models/poll/question.rb index 3e95997df..6f489c8d8 100644 --- a/app/models/poll/question.rb +++ b/app/models/poll/question.rb @@ -28,7 +28,7 @@ class Poll::Question < ApplicationRecord scope :by_poll_id, ->(poll_id) { where(poll_id: poll_id) } - scope :sort_for_list, -> { order("poll_questions.proposal_id IS NULL", :created_at) } + scope :sort_for_list, -> { order(Arel.sql("poll_questions.proposal_id IS NULL"), :created_at) } scope :for_render, -> { includes(:author, :proposal) } def self.search(params)