diff --git a/.rubocop.yml b/.rubocop.yml index 26a1f499a..5bb235a61 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -217,6 +217,9 @@ Performance/EndWith: Performance/StartWith: Enabled: true +Rails/ActiveRecordCallbacksOrder: + Enabled: true + Rails/CreateTableWithTimestamps: Enabled: true Exclude: @@ -270,6 +273,9 @@ Rails/OutputSafety: Exclude: - app/helpers/text_with_links_helper.rb +Rails/PluckId: + Enabled: true + Rails/PluralizationGrammar: Enabled: true @@ -295,7 +301,7 @@ Rails/SaveBang: Rails/SkipsModelValidations: Enabled: true - Blacklist: + ForbiddenMethods: - update_attribute Exclude: - lib/acts_as_paranoid_aliases.rb @@ -322,6 +328,12 @@ Rails/UnknownEnv: Rails/Validation: Enabled: true +Rails/WhereEquals: + Enabled: true + +Rails/WhereNot: + Enabled: true + RSpec/AroundBlock: Enabled: true diff --git a/Gemfile b/Gemfile index a79574002..21ac0174c 100644 --- a/Gemfile +++ b/Gemfile @@ -109,7 +109,7 @@ group :development do gem "pronto-scss", "~> 0.11.0", require: false gem "rubocop", "~> 0.93.1", require: false gem "rubocop-performance", "~> 1.7.1", require: false - gem "rubocop-rails", "~> 2.6.0", require: false + gem "rubocop-rails", "~> 2.9.1", require: false gem "rubocop-rspec", "~> 1.44.1", require: false gem "rvm1-capistrano3", "~> 1.4.0", require: false gem "scss_lint", "~> 0.59.0", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 76368723a..c3b27accc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -555,10 +555,10 @@ GEM parser (>= 3.0.1.1) rubocop-performance (1.7.1) rubocop (>= 0.82.0) - rubocop-rails (2.6.0) + rubocop-rails (2.9.1) activesupport (>= 4.2.0) rack (>= 1.1) - rubocop (>= 0.82.0) + rubocop (>= 0.90.0, < 2.0) rubocop-rspec (1.44.1) rubocop (~> 0.87) rubocop-ast (>= 0.7.1) @@ -772,7 +772,7 @@ DEPENDENCIES rspec-rails (~> 4.0) rubocop (~> 0.93.1) rubocop-performance (~> 1.7.1) - rubocop-rails (~> 2.6.0) + rubocop-rails (~> 2.9.1) rubocop-rspec (~> 1.44.1) rvm1-capistrano3 (~> 1.4.0) sassc-rails (~> 2.1.2) diff --git a/app/controllers/admin/stats_controller.rb b/app/controllers/admin/stats_controller.rb index 577d90398..5f91ce61f 100644 --- a/app/controllers/admin/stats_controller.rb +++ b/app/controllers/admin/stats_controller.rb @@ -24,7 +24,7 @@ class Admin::StatsController < Admin::BaseController .count(:voter_id) @user_ids_who_didnt_vote_proposals = @verified_users - @user_ids_who_voted_proposals - budgets_ids = Budget.where.not(phase: "finished").pluck(:id) + budgets_ids = Budget.where.not(phase: "finished").ids @budgets = budgets_ids.size @investments = Budget::Investment.where(budget_id: budgets_ids).count end diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index a06e68d1e..7c4101dad 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -44,7 +44,7 @@ module Budgets def index @investments = investments.page(params[:page]).per(PER_PAGE).for_render - @investment_ids = @investments.pluck(:id) + @investment_ids = @investments.ids @investments_map_coordinates = MapLocation.where(investment: investments).map(&:json_data) @tag_cloud = tag_cloud diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 4ed198efa..85e4359ff 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -43,7 +43,7 @@ class DashboardController < Dashboard::BaseController end def set_done_and_pending_actions - @done_actions = proposed_actions.joins(:proposals).where("proposals.id = ?", proposal.id) + @done_actions = proposed_actions.joins(:proposals).where(proposals: { id: proposal.id }) @pending_actions = proposed_actions - @done_actions end diff --git a/app/controllers/officing/ballot_sheets_controller.rb b/app/controllers/officing/ballot_sheets_controller.rb index afc885f61..55b6fcc77 100644 --- a/app/controllers/officing/ballot_sheets_controller.rb +++ b/app/controllers/officing/ballot_sheets_controller.rb @@ -47,7 +47,7 @@ class Officing::BallotSheetsController < Officing::BaseController joins(:booth_assignment). final. where(id: current_user.poll_officer.officer_assignment_ids). - where("poll_booth_assignments.poll_id = ?", @poll.id). + where(poll_booth_assignments: { poll_id: @poll.id }). where(date: Date.current) end diff --git a/app/controllers/officing/results_controller.rb b/app/controllers/officing/results_controller.rb index b0baf6b7d..6dbfe96ab 100644 --- a/app/controllers/officing/results_controller.rb +++ b/app/controllers/officing/results_controller.rb @@ -99,7 +99,7 @@ class Officing::ResultsController < Officing::BaseController joins(:booth_assignment). final. where(id: current_user.poll_officer.officer_assignment_ids). - where("poll_booth_assignments.poll_id = ?", @poll.id). + where(poll_booth_assignments: { poll_id: @poll.id }). where(date: Date.current) end diff --git a/app/helpers/proposals_dashboard_helper.rb b/app/helpers/proposals_dashboard_helper.rb index e96b2a3dd..a73b6d0ff 100644 --- a/app/helpers/proposals_dashboard_helper.rb +++ b/app/helpers/proposals_dashboard_helper.rb @@ -117,7 +117,7 @@ module ProposalsDashboardHelper def new_resources_since_last_login?(resources, new_actions_since_last_login) if resources.present? - resources.pluck(:id).any? { |id| new_actions_since_last_login.include?(id) } + resources.ids.any? { |id| new_actions_since_last_login.include?(id) } end end diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 79f0e6cde..d490a3bf1 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -104,19 +104,19 @@ class Budget scope :for_render, -> { includes(:heading) } def self.by_valuator(valuator_id) - where("budget_valuator_assignments.valuator_id = ?", valuator_id).joins(:valuator_assignments) + where(budget_valuator_assignments: { valuator_id: valuator_id }).joins(:valuator_assignments) end def self.by_valuator_group(valuator_group_id) joins(:valuator_group_assignments). - where("budget_valuator_group_assignments.valuator_group_id = ?", valuator_group_id) + where(budget_valuator_group_assignments: { valuator_group_id: valuator_group_id }) end - before_create :set_original_heading_id - before_save :calculate_confidence_score - after_save :recalculate_heading_winners before_validation :set_responsible_name before_validation :set_denormalized_ids + before_save :calculate_confidence_score + before_create :set_original_heading_id + after_save :recalculate_heading_winners def comments_count comments.count @@ -164,10 +164,10 @@ class Budget results = results.winners if params[:advanced_filters].include?("winners") ids = [] - ids += results.valuation_finished_feasible.pluck(:id) if params[:advanced_filters].include?("feasible") - ids += results.where(selected: true).pluck(:id) if params[:advanced_filters].include?("selected") - ids += results.undecided.pluck(:id) if params[:advanced_filters].include?("undecided") - ids += results.unfeasible.pluck(:id) if params[:advanced_filters].include?("unfeasible") + ids += results.valuation_finished_feasible.ids if params[:advanced_filters].include?("feasible") + ids += results.where(selected: true).ids if params[:advanced_filters].include?("selected") + ids += results.undecided.ids if params[:advanced_filters].include?("undecided") + ids += results.unfeasible.ids if params[:advanced_filters].include?("unfeasible") results = results.where(id: ids) if ids.any? results end @@ -191,8 +191,8 @@ class Budget return results if max_per_heading <= 0 ids = [] - budget.headings.pluck(:id).each do |hid| - ids += Investment.where(heading_id: hid).order(confidence_score: :desc).limit(max_per_heading).pluck(:id) + budget.headings.ids.each do |hid| + ids += Investment.where(heading_id: hid).order(confidence_score: :desc).limit(max_per_heading).ids end results.where(id: ids) diff --git a/app/models/comment.rb b/app/models/comment.rb index f6607a849..89e6c88f1 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -32,10 +32,8 @@ class Comment < ApplicationRecord before_save :calculate_confidence_score scope :for_render, -> { with_hidden.includes(user: :organization) } - scope :with_visible_author, -> { joins(:user).where("users.hidden_at IS NULL") } - scope :not_as_admin_or_moderator, -> do - where("administrator_id IS NULL").where("moderator_id IS NULL") - end + scope :with_visible_author, -> { joins(:user).where(users: { hidden_at: nil }) } + scope :not_as_admin_or_moderator, -> { where(administrator_id: nil).where(moderator_id: nil) } scope :sort_by_flags, -> { order(flags_count: :desc, updated_at: :desc) } scope :public_for_api, -> do not_valuations diff --git a/app/models/concerns/followable.rb b/app/models/concerns/followable.rb index 8a08d6809..7d6210369 100644 --- a/app/models/concerns/followable.rb +++ b/app/models/concerns/followable.rb @@ -6,7 +6,7 @@ module Followable has_many :followers, through: :follows, source: :user scope :followed_by_user, ->(user) { - joins(:follows).where("follows.user_id = ?", user.id) + joins(:follows).where(follows: { user_id: user.id }) } end diff --git a/app/models/concerns/randomizable.rb b/app/models/concerns/randomizable.rb index 98a86b426..6b8299368 100644 --- a/app/models/concerns/randomizable.rb +++ b/app/models/concerns/randomizable.rb @@ -3,7 +3,7 @@ module Randomizable class_methods do def sort_by_random(seed = rand(10_000_000)) - ids = order(:id).pluck(:id).shuffle(random: Random.new(seed)) + ids = order(:id).ids.shuffle(random: Random.new(seed)) return all if ids.empty? diff --git a/app/models/dashboard/action.rb b/app/models/dashboard/action.rb index 72340f0b8..f3734c0d0 100644 --- a/app/models/dashboard/action.rb +++ b/app/models/dashboard/action.rb @@ -86,10 +86,7 @@ class Dashboard::Action < ApplicationRecord actions_for_today = get_actions_for_today(proposal) actions_for_date = get_actions_for_date(proposal, date) - actions_for_today_ids = actions_for_today.pluck(:id) - actions_for_date_ids = actions_for_date.pluck(:id) - - actions_for_today_ids - actions_for_date_ids + actions_for_today.ids - actions_for_date.ids end private diff --git a/app/models/debate.rb b/app/models/debate.rb index 47a78f360..7366249c6 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -45,7 +45,7 @@ class Debate < ApplicationRecord scope :sort_by_flags, -> { order(flags_count: :desc, updated_at: :desc) } scope :sort_by_recommendations, -> { order(cached_votes_total: :desc) } scope :last_week, -> { where("created_at >= ?", 7.days.ago) } - scope :featured, -> { where("featured_at is not null") } + scope :featured, -> { where.not(featured_at: nil) } scope :public_for_api, -> { all } # Ahoy setup @@ -58,8 +58,7 @@ class Debate < ApplicationRecord end def self.recommendations(user) - tagged_with(user.interests, any: true) - .where("author_id != ?", user.id) + tagged_with(user.interests, any: true).where.not(author_id: user.id) end def searchable_translations_definitions diff --git a/app/models/legislation/question.rb b/app/models/legislation/question.rb index 73f1cc946..89515d943 100644 --- a/app/models/legislation/question.rb +++ b/app/models/legislation/question.rb @@ -22,11 +22,11 @@ class Legislation::Question < ApplicationRecord scope :sorted, -> { order("id ASC") } def next_question_id - @next_question_id ||= process.questions.where("id > ?", id).sorted.limit(1).pluck(:id).first + @next_question_id ||= process.questions.where("id > ?", id).sorted.limit(1).ids.first end def first_question_id - @first_question_id ||= process.questions.sorted.limit(1).pluck(:id).first + @first_question_id ||= process.questions.sorted.limit(1).ids.first end def answer_for_user(user) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 8401a3a86..0498d24c4 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -15,7 +15,7 @@ class Milestone < ApplicationRecord scope :order_by_publication_date, -> { order(publication_date: :asc, created_at: :asc) } scope :published, -> { where("publication_date <= ?", Date.current.end_of_day) } - scope :with_status, -> { where("status_id IS NOT NULL") } + scope :with_status, -> { where.not(status_id: nil) } def self.title_max_length 80 diff --git a/app/models/poll.rb b/app/models/poll.rb index 08e11f854..ac9fd07fc 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -45,7 +45,7 @@ class Poll < ApplicationRecord scope :current, -> { where("starts_at <= ? and ? <= ends_at", Date.current.beginning_of_day, Date.current.beginning_of_day) } scope :expired, -> { where("ends_at < ?", Date.current.beginning_of_day) } scope :recounting, -> { where(ends_at: (Date.current.beginning_of_day - RECOUNT_DURATION)..Date.current.beginning_of_day) } - scope :published, -> { where("published = ?", true) } + scope :published, -> { where(published: true) } scope :by_geozone_id, ->(geozone_id) { where(geozones: { id: geozone_id }.joins(:geozones)) } scope :public_for_api, -> { all } scope :not_budget, -> { where(budget_id: nil) } @@ -122,7 +122,7 @@ class Poll < ApplicationRecord end def self.not_voted_by(user) - where("polls.id not in (?)", poll_ids_voted_by(user)) + where.not(id: poll_ids_voted_by(user)) end def self.poll_ids_voted_by(user) diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 9fe469915..643c51de5 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -106,7 +106,7 @@ class Proposal < ApplicationRecord def self.recommendations(user) tagged_with(user.interests, any: true) - .where("author_id != ?", user.id) + .where.not(author_id: user.id) .unsuccessful .not_followed_by_user(user) .not_archived @@ -114,7 +114,7 @@ class Proposal < ApplicationRecord end def self.not_followed_by_user(user) - where.not(id: followed_by_user(user).pluck(:id)) + where.not(id: followed_by_user(user).ids) end def to_param diff --git a/app/models/user.rb b/app/models/user.rb index 75da22c3b..918667840 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -243,7 +243,7 @@ class User < ApplicationRecord Comment.hide_all comment_ids Proposal.hide_all proposal_ids Budget::Investment.hide_all budget_investment_ids - ProposalNotification.hide_all ProposalNotification.where(author_id: id).pluck(:id) + ProposalNotification.hide_all ProposalNotification.where(author_id: id).ids end def full_restore diff --git a/app/models/verification/management/managed_user.rb b/app/models/verification/management/managed_user.rb index 45ed1bbab..babf18f58 100644 --- a/app/models/verification/management/managed_user.rb +++ b/app/models/verification/management/managed_user.rb @@ -2,7 +2,7 @@ class Verification::Management::ManagedUser include ActiveModel::Model def self.find(document_type, document_number) - User.where("document_number is not null"). + User.where.not(document_number: nil). find_or_initialize_by(document_type: document_type, document_number: document_number) end diff --git a/db/dev_seeds/flags.rb b/db/dev_seeds/flags.rb index ebddb901a..a31c0e550 100644 --- a/db/dev_seeds/flags.rb +++ b/db/dev_seeds/flags.rb @@ -1,19 +1,19 @@ section "Flagging Debates & Comments" do 40.times do debate = Debate.all.sample - flagger = User.where(["users.id <> ?", debate.author_id]).all.sample + flagger = User.where.not(id: debate.author_id).all.sample Flag.flag(flagger, debate) end 40.times do comment = Comment.all.sample - flagger = User.where(["users.id <> ?", comment.user_id]).all.sample + flagger = User.where.not(id: comment.user_id).all.sample Flag.flag(flagger, comment) end 40.times do proposal = Proposal.all.sample - flagger = User.where(["users.id <> ?", proposal.author_id]).all.sample + flagger = User.where.not(id: proposal.author_id).all.sample Flag.flag(flagger, proposal) end end diff --git a/db/migrate/20171002103314_add_poll_shift_task_index.rb b/db/migrate/20171002103314_add_poll_shift_task_index.rb index 3e28def56..8d283c7e1 100644 --- a/db/migrate/20171002103314_add_poll_shift_task_index.rb +++ b/db/migrate/20171002103314_add_poll_shift_task_index.rb @@ -1,6 +1,6 @@ class AddPollShiftTaskIndex < ActiveRecord::Migration[4.2] def change - remove_index "poll_shifts", name: "index_poll_shifts_on_booth_id_and_officer_id" + remove_index "poll_shifts", column: [:booth_id, :officer_id] add_index :poll_shifts, :task add_index :poll_shifts, [:booth_id, :officer_id, :task], unique: true end diff --git a/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb b/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb index 62e4d12cf..16772972b 100644 --- a/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb +++ b/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb @@ -1,5 +1,5 @@ class RemoveOfficerAssigmentComposedIndex < ActiveRecord::Migration[4.2] def change - remove_index "poll_officer_assignments", name: "index_poll_officer_assignments_on_officer_id_and_date" + remove_index "poll_officer_assignments", column: [:officer_id, :date] end end diff --git a/db/migrate/20171003212958_add_date_to_poll_shift_composed_index.rb b/db/migrate/20171003212958_add_date_to_poll_shift_composed_index.rb index 28c7e51ac..2b309b4bd 100644 --- a/db/migrate/20171003212958_add_date_to_poll_shift_composed_index.rb +++ b/db/migrate/20171003212958_add_date_to_poll_shift_composed_index.rb @@ -1,7 +1,7 @@ class AddDateToPollShiftComposedIndex < ActiveRecord::Migration[4.2] def change - remove_index "poll_shifts", name: "index_poll_shifts_on_booth_id_and_officer_id_and_task" - remove_index "poll_shifts", name: "index_poll_shifts_on_task" + remove_index "poll_shifts", column: [:booth_id, :officer_id, :task] + remove_index "poll_shifts", :task add_index :poll_shifts, [:booth_id, :officer_id, :date, :task], unique: true end end diff --git a/db/migrate/20190321144328_add_config_to_download_settings.rb b/db/migrate/20190321144328_add_config_to_download_settings.rb index 74c650011..5a5404026 100644 --- a/db/migrate/20190321144328_add_config_to_download_settings.rb +++ b/db/migrate/20190321144328_add_config_to_download_settings.rb @@ -2,7 +2,7 @@ class AddConfigToDownloadSettings < ActiveRecord::Migration[4.2] def change add_column :download_settings, :config, :integer, default: 0, null: false - remove_index :download_settings, name: "index_download_settings_on_name_model_and_name_field" + remove_index :download_settings, column: [:name_model, :name_field] add_index :download_settings, [:name_model, :name_field, :config], unique: true end end