From adc5906253631c5354d9da5635b2eafb3ae1c13b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 21:56:57 +0200 Subject: [PATCH 01/13] Apply `Rails/PluralizationGrammar` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ spec/features/admin/budget_phases_spec.rb | 4 ++-- spec/features/admin/legislation/processes_spec.rb | 2 +- spec/features/legislation/resume_spec.rb | 14 +++++++------- spec/models/debate_spec.rb | 2 +- spec/models/legislation/people_proposals_spec.rb | 2 +- spec/models/legislation/proposal_spec.rb | 4 ++-- spec/models/proposal_spec.rb | 2 +- 9 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index cf9fb516e..7b50034aa 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -141,9 +141,6 @@ Rails/Output: Rails/OutputSafety: Enabled: true -Rails/PluralizationGrammar: - Enabled: true - Rails/Presence: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 62496d567..081bc2be0 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -129,6 +129,9 @@ Rails/Date: Rails/HttpPositionalArguments: Enabled: true +Rails/PluralizationGrammar: + Enabled: true + Rails/RelativeDateConstant: Enabled: true diff --git a/spec/features/admin/budget_phases_spec.rb b/spec/features/admin/budget_phases_spec.rb index 142bc7f02..fa1a4031d 100644 --- a/spec/features/admin/budget_phases_spec.rb +++ b/spec/features/admin/budget_phases_spec.rb @@ -19,7 +19,7 @@ describe "Admin budget phases" do scenario "Update phase", :js do visit edit_admin_budget_budget_phase_path(budget, budget.current_phase) - fill_in "start_date", with: Date.current + 1.days + fill_in "start_date", with: Date.current + 1.day fill_in "end_date", with: Date.current + 12.days fill_in_translatable_ckeditor "summary", :en, with: "New summary of the phase." fill_in_translatable_ckeditor "description", :en, with: "New description of the phase." @@ -29,7 +29,7 @@ describe "Admin budget phases" do expect(page).to have_current_path(edit_admin_budget_path(budget)) expect(page).to have_content "Changes saved" - expect(budget.current_phase.starts_at.to_date).to eq((Date.current + 1.days).to_date) + expect(budget.current_phase.starts_at.to_date).to eq((Date.current + 1.day).to_date) expect(budget.current_phase.ends_at.to_date).to eq((Date.current + 12.days).to_date) expect(budget.current_phase.summary).to include("New summary of the phase.") expect(budget.current_phase.description).to include("New description of the phase.") diff --git a/spec/features/admin/legislation/processes_spec.rb b/spec/features/admin/legislation/processes_spec.rb index 056202584..138f989c4 100644 --- a/spec/features/admin/legislation/processes_spec.rb +++ b/spec/features/admin/legislation/processes_spec.rb @@ -97,7 +97,7 @@ describe "Admin collaborative legislation" do fill_in "legislation_process[draft_start_date]", with: (base_date - 3.days).strftime("%d/%m/%Y") fill_in "legislation_process[draft_end_date]", - with: (base_date - 1.days).strftime("%d/%m/%Y") + with: (base_date - 1.day).strftime("%d/%m/%Y") fill_in "legislation_process[draft_publication_date]", with: (base_date + 3.days).strftime("%d/%m/%Y") fill_in "legislation_process[allegations_start_date]", diff --git a/spec/features/legislation/resume_spec.rb b/spec/features/legislation/resume_spec.rb index 749826980..643fe17c7 100644 --- a/spec/features/legislation/resume_spec.rb +++ b/spec/features/legislation/resume_spec.rb @@ -18,7 +18,7 @@ describe "Legislation" do context "process empty" do before do - @process = create(:legislation_process, :empty, end_date: Date.current - 1.days) + @process = create(:legislation_process, :empty, end_date: Date.current - 1.day) end scenario "warning empty" do @@ -29,7 +29,7 @@ describe "Legislation" do context "phases empty" do before do - @process = create(:legislation_process, end_date: Date.current - 1.days) + @process = create(:legislation_process, end_date: Date.current - 1.day) end scenario "debates empty" do @@ -56,7 +56,7 @@ describe "Legislation" do context "process empty" do before do - @process = create(:legislation_process, :empty, end_date: Date.current - 1.days) + @process = create(:legislation_process, :empty, end_date: Date.current - 1.day) end scenario "warning empty" do @@ -68,7 +68,7 @@ describe "Legislation" do context "only debates exist" do before do user = create(:user, :level_two) - @process = create(:legislation_process, end_date: Date.current - 1.days) + @process = create(:legislation_process, end_date: Date.current - 1.day) @debate = create(:legislation_question, process: @process, title: "Question 1") create(:debate_comment, user: user, commentable_id: @debate.id, body: "Answer 1") create(:debate_comment, user: user, commentable_id: @debate.id, body: "Answer 2") @@ -123,7 +123,7 @@ describe "Legislation" do context "only proposals exist" do before do - @process = create(:legislation_process, end_date: Date.current - 1.days) + @process = create(:legislation_process, end_date: Date.current - 1.day) create(:legislation_proposal, legislation_process_id: @process.id, title: "Legislation proposal 1", selected: true) create(:legislation_proposal, legislation_process_id: @process.id, @@ -171,7 +171,7 @@ describe "Legislation" do context "only text comments exist" do before do user = create(:user, :level_two) - @process = create(:legislation_process, end_date: Date.current - 1.days) + @process = create(:legislation_process, end_date: Date.current - 1.day) draft_version_1 = create(:legislation_draft_version, process: @process, title: "Version 1", body: "Body of the first version", status: "published") @@ -231,7 +231,7 @@ describe "Legislation" do describe Legislation::ProcessesController, type: :controller do before do user = create(:user, :level_two) - @process = create(:legislation_process, end_date: Date.current - 1.days) + @process = create(:legislation_process, end_date: Date.current - 1.day) @debate = create(:legislation_question, process: @process, title: "Question 1") create(:debate_comment, user: user, commentable_id: @debate.id, body: "Answer 1") create(:debate_comment, user: user, commentable_id: @debate.id, body: "Answer 2") diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index 2afd2f3c9..ecba4d46f 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -240,7 +240,7 @@ describe Debate do it "remains the same for not voted debates" do new = create(:debate, created_at: now) old = create(:debate, created_at: 1.day.ago) - older = create(:debate, created_at: 2.month.ago) + older = create(:debate, created_at: 2.months.ago) expect(new.hot_score).to be 0 expect(old.hot_score).to be 0 expect(older.hot_score).to be 0 diff --git a/spec/models/legislation/people_proposals_spec.rb b/spec/models/legislation/people_proposals_spec.rb index c1aef7571..0dd0a577a 100644 --- a/spec/models/legislation/people_proposals_spec.rb +++ b/spec/models/legislation/people_proposals_spec.rb @@ -47,7 +47,7 @@ describe Legislation::PeopleProposal do it "remains the same for not voted people_proposals" do new = create(:legislation_people_proposal, created_at: now) old = create(:legislation_people_proposal, created_at: 1.day.ago) - older = create(:legislation_people_proposal, created_at: 2.month.ago) + older = create(:legislation_people_proposal, created_at: 2.months.ago) expect(new.hot_score).to be 0 expect(old.hot_score).to be 0 expect(older.hot_score).to be 0 diff --git a/spec/models/legislation/proposal_spec.rb b/spec/models/legislation/proposal_spec.rb index f2f72b055..2959a7a5f 100644 --- a/spec/models/legislation/proposal_spec.rb +++ b/spec/models/legislation/proposal_spec.rb @@ -47,7 +47,7 @@ describe Legislation::Proposal do it "remains the same for not voted proposals" do new = create(:legislation_proposal, created_at: now) old = create(:legislation_proposal, created_at: 1.day.ago) - older = create(:legislation_proposal, created_at: 2.month.ago) + older = create(:legislation_proposal, created_at: 2.months.ago) expect(new.hot_score).to be 0 expect(old.hot_score).to be 0 expect(older.hot_score).to be 0 @@ -67,7 +67,7 @@ describe Legislation::Proposal do newer_proposal = create(:legislation_proposal, created_at: now) 5.times { newer_proposal.vote_by(voter: create(:user), vote: "yes") } - older_proposal = create(:legislation_proposal, created_at: 2.day.ago) + older_proposal = create(:legislation_proposal, created_at: 2.days.ago) 5.times { older_proposal.vote_by(voter: create(:user), vote: "yes") } expect(newer_proposal.hot_score).to be > older_proposal.hot_score diff --git a/spec/models/proposal_spec.rb b/spec/models/proposal_spec.rb index 17fe65682..ddaee48dc 100644 --- a/spec/models/proposal_spec.rb +++ b/spec/models/proposal_spec.rb @@ -285,7 +285,7 @@ describe Proposal do it "remains the same for not voted proposals" do new = create(:proposal, created_at: now) old = create(:proposal, created_at: 1.day.ago) - older = create(:proposal, created_at: 2.month.ago) + older = create(:proposal, created_at: 2.months.ago) expect(new.hot_score).to be 0 expect(old.hot_score).to be 0 expect(older.hot_score).to be 0 From a5ba13b599206bb26d1948fb0fe43d8b221ba3ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 22:03:46 +0200 Subject: [PATCH 02/13] Apply `Rails/Presence` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ app/controllers/polls/answers_controller.rb | 2 +- app/controllers/polls/questions_controller.rb | 2 +- app/helpers/banners_helper.rb | 4 ++-- app/helpers/legislation_helper.rb | 4 ++-- app/models/legislation/people_proposal.rb | 2 +- app/models/legislation/proposal.rb | 2 +- app/models/proposal.rb | 2 +- app/models/tracker.rb | 4 ++-- app/models/valuator.rb | 4 ++-- config/initializers/i18n_translation.rb | 8 ++------ lib/remote_translations/microsoft/sentences_parser.rb | 2 +- 13 files changed, 19 insertions(+), 23 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7b50034aa..218cdc615 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -141,9 +141,6 @@ Rails/Output: Rails/OutputSafety: Enabled: true -Rails/Presence: - Enabled: true - Rails/Present: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 081bc2be0..6ea9cd9f3 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -132,6 +132,9 @@ Rails/HttpPositionalArguments: Rails/PluralizationGrammar: Enabled: true +Rails/Presence: + Enabled: true + Rails/RelativeDateConstant: Enabled: true diff --git a/app/controllers/polls/answers_controller.rb b/app/controllers/polls/answers_controller.rb index 61b3b33d3..c8b0081f2 100644 --- a/app/controllers/polls/answers_controller.rb +++ b/app/controllers/polls/answers_controller.rb @@ -51,7 +51,7 @@ class Polls::AnswersController < ApplicationController end def load_for_answers - @page = params[:page].present? ? params[:page] : 1 + @page = params[:page].presence || 1 question_answers @answers_by_question_id = { @question.id => @question.answers .by_author(current_user) diff --git a/app/controllers/polls/questions_controller.rb b/app/controllers/polls/questions_controller.rb index d1ad187da..675c6ffa5 100644 --- a/app/controllers/polls/questions_controller.rb +++ b/app/controllers/polls/questions_controller.rb @@ -37,7 +37,7 @@ class Polls::QuestionsController < ApplicationController private def load_for_answers - @page = params[:page].present? ? params[:page] : 1 + @page = params[:page].presence || 1 question_answers @answers_by_question_id = { @question.id => @question.answers .by_author(current_user) diff --git a/app/helpers/banners_helper.rb b/app/helpers/banners_helper.rb index 20f7ab93d..59cd07c1a 100644 --- a/app/helpers/banners_helper.rb +++ b/app/helpers/banners_helper.rb @@ -13,11 +13,11 @@ module BannersHelper end def banner_bg_color_or_default - @banner.background_color.present? ? @banner.background_color : banner_default_bg_color + @banner.background_color.presence || banner_default_bg_color end def banner_font_color_or_default - @banner.font_color.present? ? @banner.font_color : banner_default_font_color + @banner.font_color.presence || banner_default_font_color end end diff --git a/app/helpers/legislation_helper.rb b/app/helpers/legislation_helper.rb index 943f1b35d..5dd916571 100644 --- a/app/helpers/legislation_helper.rb +++ b/app/helpers/legislation_helper.rb @@ -35,11 +35,11 @@ module LegislationHelper end def bg_color_or_default - @process.background_color.present? ? @process.background_color : default_bg_color + @process.background_color.presence || default_bg_color end def font_color_or_default - @process.font_color.present? ? @process.font_color : default_font_color + @process.font_color.presence || default_font_color end def css_for_process_header diff --git a/app/models/legislation/people_proposal.rb b/app/models/legislation/people_proposal.rb index 69696bb05..b85b197d7 100644 --- a/app/models/legislation/people_proposal.rb +++ b/app/models/legislation/people_proposal.rb @@ -65,7 +65,7 @@ class Legislation::PeopleProposal < ApplicationRecord def self.search(terms) by_code = search_by_code(terms.strip) - by_code.present? ? by_code : pg_search(terms) + by_code.presence || pg_search(terms) end def self.search_by_code(terms) diff --git a/app/models/legislation/proposal.rb b/app/models/legislation/proposal.rb index aa8a686b7..44becffc8 100644 --- a/app/models/legislation/proposal.rb +++ b/app/models/legislation/proposal.rb @@ -66,7 +66,7 @@ class Legislation::Proposal < ApplicationRecord def self.search(terms) by_code = search_by_code(terms.strip) - by_code.present? ? by_code : pg_search(terms) + by_code.presence || pg_search(terms) end def self.search_by_code(terms) diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 4ef4b2638..a4d9d10eb 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -141,7 +141,7 @@ class Proposal < ApplicationRecord def self.search(terms) by_code = search_by_code(terms.strip) - by_code.present? ? by_code : pg_search(terms) + by_code.presence || pg_search(terms) end def self.search_by_code(terms) diff --git a/app/models/tracker.rb b/app/models/tracker.rb index d5f5c4fa2..81f3d8021 100644 --- a/app/models/tracker.rb +++ b/app/models/tracker.rb @@ -9,11 +9,11 @@ class Tracker < ApplicationRecord validates :user_id, presence: true, uniqueness: true def description_or_email - description.present? ? description : email + description.presence || email end def description_or_name - description.present? ? description : name + description.presence || name end def assigned_investment_ids diff --git a/app/models/valuator.rb b/app/models/valuator.rb index 985984a78..372dacc01 100644 --- a/app/models/valuator.rb +++ b/app/models/valuator.rb @@ -10,11 +10,11 @@ class Valuator < ApplicationRecord validates :user_id, presence: true, uniqueness: true def description_or_email - description.present? ? description : email + description.presence || email end def description_or_name - description.present? ? description : name + description.presence || name end def assigned_investment_ids diff --git a/config/initializers/i18n_translation.rb b/config/initializers/i18n_translation.rb index 702c2f5e3..fb23270cd 100644 --- a/config/initializers/i18n_translation.rb +++ b/config/initializers/i18n_translation.rb @@ -7,16 +7,12 @@ module ActionView include TagHelper def t(key, options = {}) - current_locale = options[:locale].present? ? options[:locale] : I18n.locale + current_locale = options[:locale].presence || I18n.locale i18_content = I18nContent.by_key(key).first translation = I18nContentTranslation.where(i18n_content_id: i18_content&.id, locale: current_locale).first&.value - if translation.present? - translation - else - translate(key, options) - end + translation.presence || translate(key, options) end end end diff --git a/lib/remote_translations/microsoft/sentences_parser.rb b/lib/remote_translations/microsoft/sentences_parser.rb index d6f026288..bdf0e446d 100644 --- a/lib/remote_translations/microsoft/sentences_parser.rb +++ b/lib/remote_translations/microsoft/sentences_parser.rb @@ -12,7 +12,7 @@ module RemoteTranslations::Microsoft::SentencesParser def get_split_position(valid_point, valid_whitespace, minimum_valid_index) split_position = minimum_valid_index if valid_point.present? || valid_whitespace.present? - valid_position = valid_point.present? ? valid_point : valid_whitespace + valid_position = valid_point.presence || valid_whitespace split_position = split_position + valid_position end split_position From daa86ca3fc5ce8212a153874ce06d4db43e4720f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 22:35:58 +0200 Subject: [PATCH 03/13] Apply `Rails/RequestReferer` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ app/controllers/admin/legislation/homepages_controller.rb | 2 +- app/controllers/admin/legislation/processes_controller.rb | 2 +- .../admin/poll/questions/answers/videos_controller.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 218cdc615..a23458f4d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -150,9 +150,6 @@ Rails/ReadWriteAttribute: Rails/RedundantReceiverInWithOptions: Enabled: true -Rails/RequestReferer: - Enabled: true - Rails/ReversibleMigration: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 6ea9cd9f3..db624cd91 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -138,6 +138,9 @@ Rails/Presence: Rails/RelativeDateConstant: Enabled: true +Rails/RequestReferer: + Enabled: true + Rails/TimeZone: Enabled: true diff --git a/app/controllers/admin/legislation/homepages_controller.rb b/app/controllers/admin/legislation/homepages_controller.rb index 72ff890e9..6cc2062a8 100644 --- a/app/controllers/admin/legislation/homepages_controller.rb +++ b/app/controllers/admin/legislation/homepages_controller.rb @@ -9,7 +9,7 @@ class Admin::Legislation::HomepagesController < Admin::Legislation::BaseControll def update if @process.update(process_params) link = legislation_process_path(@process).html_safe - redirect_back(fallback_location: (request.referrer || root_path), + redirect_back(fallback_location: (request.referer || root_path), notice: t("admin.legislation.processes.update.notice", link: link)) else flash.now[:error] = t("admin.legislation.processes.update.error") diff --git a/app/controllers/admin/legislation/processes_controller.rb b/app/controllers/admin/legislation/processes_controller.rb index ae1cea8a0..4c3b0f6e0 100644 --- a/app/controllers/admin/legislation/processes_controller.rb +++ b/app/controllers/admin/legislation/processes_controller.rb @@ -35,7 +35,7 @@ class Admin::Legislation::ProcessesController < Admin::Legislation::BaseControll set_tag_list link = legislation_process_path(@process).html_safe - redirect_back(fallback_location: (request.referrer || root_path), + redirect_back(fallback_location: (request.referer || root_path), notice: t("admin.legislation.processes.update.notice", link: link)) else flash.now[:error] = t("admin.legislation.processes.update.error") diff --git a/app/controllers/admin/poll/questions/answers/videos_controller.rb b/app/controllers/admin/poll/questions/answers/videos_controller.rb index 3e76531d8..c5c6f14be 100644 --- a/app/controllers/admin/poll/questions/answers/videos_controller.rb +++ b/app/controllers/admin/poll/questions/answers/videos_controller.rb @@ -38,7 +38,7 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseContr else t("flash.actions.destroy.error") end - redirect_back(fallback_location: (request.referrer || root_path), notice: notice) + redirect_back(fallback_location: (request.referer || root_path), notice: notice) end private From 9fe8c475284f9a604710aa1238f60f1d8c33aeda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 23:00:33 +0200 Subject: [PATCH 04/13] Apply `Rails/SafeNavigation` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 4 ++++ app/controllers/admin/base_controller.rb | 2 +- app/controllers/admin/poll/questions_controller.rb | 2 +- app/controllers/application_controller.rb | 4 ++-- app/controllers/budgets/investments_controller.rb | 2 +- app/controllers/comments_controller.rb | 2 +- app/controllers/management/sessions_controller.rb | 4 ++-- app/controllers/moderation/base_controller.rb | 2 +- app/controllers/officing/base_controller.rb | 2 +- app/controllers/polls_controller.rb | 2 +- app/controllers/tracking/base_controller.rb | 2 +- app/controllers/tracking/budget_investments_controller.rb | 3 +-- app/controllers/valuation/base_controller.rb | 2 +- .../valuation/budget_investments_controller.rb | 3 +-- app/helpers/budget_headings_helper.rb | 2 +- app/helpers/site_customization_helper.rb | 2 +- app/models/abilities/common.rb | 2 +- app/models/admin_notification.rb | 2 +- app/models/budget.rb | 2 +- app/models/budget/ballot/line.rb | 6 +++--- app/models/budget/investment.rb | 6 +++--- app/models/concerns/globalizable.rb | 2 +- app/models/concerns/mappable.rb | 2 +- app/models/concerns/sanitizable.rb | 2 +- app/models/debate.rb | 2 +- app/models/flag.rb | 2 +- app/models/legislation/proposal.rb | 2 +- app/models/poll/question.rb | 8 ++++---- app/models/poll/question/answer.rb | 2 +- app/models/poll/voter.rb | 4 ++-- app/models/proposal.rb | 2 +- app/models/proposal_notification.rb | 2 +- app/models/site_customization/content_block.rb | 2 +- app/models/site_customization/image.rb | 4 ++-- app/models/valuator.rb | 2 +- app/models/verification/email.rb | 2 +- app/models/verification/letter.rb | 2 +- spec/support/common_actions/users.rb | 2 +- 39 files changed, 52 insertions(+), 53 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a23458f4d..6a374e962 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -153,9 +153,6 @@ Rails/RedundantReceiverInWithOptions: Rails/ReversibleMigration: Enabled: true -Rails/SafeNavigation: - Enabled: true - Rails/SaveBang: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index db624cd91..2af5aff4a 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -141,6 +141,10 @@ Rails/RelativeDateConstant: Rails/RequestReferer: Enabled: true +Rails/SafeNavigation: + Enabled: true + ConvertTry: true + Rails/TimeZone: Enabled: true diff --git a/app/controllers/admin/base_controller.rb b/app/controllers/admin/base_controller.rb index a12540871..569dfbe0c 100644 --- a/app/controllers/admin/base_controller.rb +++ b/app/controllers/admin/base_controller.rb @@ -8,7 +8,7 @@ class Admin::BaseController < ApplicationController private def verify_administrator - raise CanCan::AccessDenied unless current_user.try(:administrator?) + raise CanCan::AccessDenied unless current_user&.administrator? end end diff --git a/app/controllers/admin/poll/questions_controller.rb b/app/controllers/admin/poll/questions_controller.rb index 279f9de6f..9b07b56b4 100644 --- a/app/controllers/admin/poll/questions_controller.rb +++ b/app/controllers/admin/poll/questions_controller.rb @@ -21,7 +21,7 @@ class Admin::Poll::QuestionsController < Admin::Poll::BaseController end def create - @question.author = @question.proposal.try(:author) || current_user + @question.author = @question.proposal&.author || current_user @question.votation_type = VotationType.build_by_type(@question, params[:votation_type]) if @question.save diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5451451ff..56d9dc046 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -110,9 +110,9 @@ class ApplicationController < ActionController::Base end def set_default_budget_filter - if @budget.try(:balloting?) || @budget.try(:publishing_prices?) + if @budget&.balloting? || @budget&.publishing_prices? params[:filter] ||= "selected" - elsif @budget.try(:finished?) + elsif @budget&.finished? params[:filter] ||= "winners" end end diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index 74414293f..6b1c07832 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -141,7 +141,7 @@ module Budgets def load_heading if params[:heading_id].present? @heading = @budget.headings.find_by_slug_or_id! params[:heading_id] - @assigned_heading = @ballot.try(:heading_for_group, @heading.try(:group)) + @assigned_heading = @ballot&.heading_for_group(@heading&.group) load_map end end diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 18ee03e09..6504c3f27 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -85,7 +85,7 @@ class CommentsController < ApplicationController def add_notification(comment) notifiable = comment.reply? ? comment.parent : comment.commentable - notifiable_author_id = notifiable.try(:author_id) + notifiable_author_id = notifiable&.author_id if notifiable_author_id.present? && notifiable_author_id != comment.author_id Notification.add(notifiable.author, notifiable) end diff --git a/app/controllers/management/sessions_controller.rb b/app/controllers/management/sessions_controller.rb index 49784c53e..5617c22fb 100644 --- a/app/controllers/management/sessions_controller.rb +++ b/app/controllers/management/sessions_controller.rb @@ -27,13 +27,13 @@ class Management::SessionsController < ActionController::Base end def admin? - if current_user.try(:administrator?) + if current_user&.administrator? session[:manager] = { login: "admin_user_#{current_user.id}" } end end def manager? - if current_user.try(:manager?) + if current_user&.manager? session[:manager] = { login: "manager_user_#{current_user.id}" } end end diff --git a/app/controllers/moderation/base_controller.rb b/app/controllers/moderation/base_controller.rb index c175cb1bd..39d5b19ab 100644 --- a/app/controllers/moderation/base_controller.rb +++ b/app/controllers/moderation/base_controller.rb @@ -9,7 +9,7 @@ class Moderation::BaseController < ApplicationController private def verify_moderator - raise CanCan::AccessDenied unless current_user.try(:moderator?) || current_user.try(:administrator?) + raise CanCan::AccessDenied unless current_user&.moderator? || current_user&.administrator? end end diff --git a/app/controllers/officing/base_controller.rb b/app/controllers/officing/base_controller.rb index 42b85860b..c1b80539d 100644 --- a/app/controllers/officing/base_controller.rb +++ b/app/controllers/officing/base_controller.rb @@ -10,7 +10,7 @@ class Officing::BaseController < ApplicationController private def verify_officer - raise CanCan::AccessDenied unless current_user.try(:poll_officer?) + raise CanCan::AccessDenied unless current_user&.poll_officer? end def check_officer_assignment diff --git a/app/controllers/polls_controller.rb b/app/controllers/polls_controller.rb index b95303384..ba2e848dd 100644 --- a/app/controllers/polls_controller.rb +++ b/app/controllers/polls_controller.rb @@ -25,7 +25,7 @@ class PollsController < ApplicationController .where.not(description: "").order(:given_order) @answers_by_question_id = {} - poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user.try(:id)) + poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user&.id) @last_pair_question_answers = {} @questions.each do |question| diff --git a/app/controllers/tracking/base_controller.rb b/app/controllers/tracking/base_controller.rb index f924bc088..f42ac9f5c 100644 --- a/app/controllers/tracking/base_controller.rb +++ b/app/controllers/tracking/base_controller.rb @@ -9,7 +9,7 @@ class Tracking::BaseController < ApplicationController private def verify_tracker - raise CanCan::AccessDenied unless current_user.try(:tracker?) || current_user.try(:administrator?) + raise CanCan::AccessDenied unless current_user&.tracker? || current_user&.administrator? end end diff --git a/app/controllers/tracking/budget_investments_controller.rb b/app/controllers/tracking/budget_investments_controller.rb index d588964ec..4586b7308 100644 --- a/app/controllers/tracking/budget_investments_controller.rb +++ b/app/controllers/tracking/budget_investments_controller.rb @@ -47,8 +47,7 @@ class Tracking::BudgetInvestmentsController < Tracking::BaseController end def heading_filters - investments = @budget.investments.by_tracker(current_user.tracker.try(:id)) - .distinct + investments = @budget.investments.by_tracker(current_user.tracker&.id).distinct investment_headings = Budget::Heading.where(id: investments.pluck(:heading_id).uniq) .order(name: :asc) all_headings_filter = [ diff --git a/app/controllers/valuation/base_controller.rb b/app/controllers/valuation/base_controller.rb index 64001e6d7..bfb865cbe 100644 --- a/app/controllers/valuation/base_controller.rb +++ b/app/controllers/valuation/base_controller.rb @@ -9,7 +9,7 @@ class Valuation::BaseController < ApplicationController private def verify_valuator - raise CanCan::AccessDenied unless current_user.try(:valuator?) || current_user.try(:administrator?) + raise CanCan::AccessDenied unless current_user&.valuator? || current_user&.administrator? end end diff --git a/app/controllers/valuation/budget_investments_controller.rb b/app/controllers/valuation/budget_investments_controller.rb index 73324a0a4..24b61950e 100644 --- a/app/controllers/valuation/budget_investments_controller.rb +++ b/app/controllers/valuation/budget_investments_controller.rb @@ -73,8 +73,7 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController end def heading_filters - investments = @budget.investments.by_valuator(current_user.valuator.try(:id)) - .visible_to_valuators.distinct + investments = @budget.investments.by_valuator(current_user.valuator&.id).visible_to_valuators.distinct investment_headings = Budget::Heading.where(id: investments.pluck(:heading_id)).sort_by(&:name) all_headings_filter = [ diff --git a/app/helpers/budget_headings_helper.rb b/app/helpers/budget_headings_helper.rb index 80c21e9e8..fc014131b 100644 --- a/app/helpers/budget_headings_helper.rb +++ b/app/helpers/budget_headings_helper.rb @@ -8,7 +8,7 @@ module BudgetHeadingsHelper def heading_link(assigned_heading = nil, budget = nil) return nil unless assigned_heading && budget - heading_path = budget_investments_path(budget, heading_id: assigned_heading.try(:id)) + heading_path = budget_investments_path(budget, heading_id: assigned_heading&.id) link_to(assigned_heading.name, heading_path) end diff --git a/app/helpers/site_customization_helper.rb b/app/helpers/site_customization_helper.rb index 4a3e631b6..5046c071a 100644 --- a/app/helpers/site_customization_helper.rb +++ b/app/helpers/site_customization_helper.rb @@ -12,7 +12,7 @@ module SiteCustomizationHelper I18nContentTranslation.where( i18n_content_id: content.id, locale: locale - ).first.try(:value) + ).first&.value else false end diff --git a/app/models/abilities/common.rb b/app/models/abilities/common.rb index d26a38680..8087e4dba 100644 --- a/app/models/abilities/common.rb +++ b/app/models/abilities/common.rb @@ -72,7 +72,7 @@ module Abilities can [:create, :destroy], Follow can [:destroy], Document do |document| - document.documentable.try(:author_id) == user.id + document.documentable&.author_id == user.id end can [:destroy], Image, imageable: { author_id: user.id } diff --git a/app/models/admin_notification.rb b/app/models/admin_notification.rb index 3fdec9c16..7971bc9f3 100644 --- a/app/models/admin_notification.rb +++ b/app/models/admin_notification.rb @@ -25,7 +25,7 @@ class AdminNotification < ApplicationRecord end def list_of_recipients_count - list_of_recipients.try(:count) || 0 + list_of_recipients&.count || 0 end def deliver diff --git a/app/models/budget.rb b/app/models/budget.rb index 8b8b7c6f6..3ca9d2147 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -79,7 +79,7 @@ class Budget < ApplicationRecord if phases.exists? && phases.send(phase).description.present? phases.send(phase).description else - send("description_#{phase}").try(:html_safe) + send("description_#{phase}")&.html_safe end end diff --git a/app/models/budget/ballot/line.rb b/app/models/budget/ballot/line.rb index c64ed39c0..b52f9e562 100644 --- a/app/models/budget/ballot/line.rb +++ b/app/models/budget/ballot/line.rb @@ -34,9 +34,9 @@ class Budget private def set_denormalized_ids - self.heading_id ||= investment.try(:heading_id) - self.group_id ||= investment.try(:group_id) - self.budget_id ||= investment.try(:budget_id) + self.heading_id ||= investment&.heading_id + self.group_id ||= investment&.group_id + self.budget_id ||= investment&.budget_id end def store_user_heading diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 935d33a17..c80b060fa 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -319,7 +319,7 @@ class Budget end def set_responsible_name - self.responsible_name = author.try(:document_number) if author.try(:document_number).present? + self.responsible_name = author&.document_number if author&.document_number.present? end def should_show_aside? @@ -400,8 +400,8 @@ class Budget private def set_denormalized_ids - self.group_id = heading.try(:group_id) if heading_id_changed? - self.budget_id ||= heading.try(:group).try(:budget_id) + self.group_id = heading&.group_id if heading_id_changed? + self.budget_id ||= heading&.group&.budget_id end def change_log diff --git a/app/models/concerns/globalizable.rb b/app/models/concerns/globalizable.rb index 3b683c912..9051188b5 100644 --- a/app/models/concerns/globalizable.rb +++ b/app/models/concerns/globalizable.rb @@ -10,7 +10,7 @@ module Globalizable after_validation :copy_error_to_current_translation, on: :update def description - self.read_attribute(:description).try :html_safe + self.read_attribute(:description)&.html_safe end def locales_not_marked_for_destruction diff --git a/app/models/concerns/mappable.rb b/app/models/concerns/mappable.rb index fe2b847b9..4762da578 100644 --- a/app/models/concerns/mappable.rb +++ b/app/models/concerns/mappable.rb @@ -12,7 +12,7 @@ module Mappable def map_must_be_valid return true if skip_map? - unless map_location.try(:available?) + unless map_location&.available? skip_map_error = I18n.t("activerecord.errors.models.map_location.attributes.map.invalid") errors.add(:skip_map, skip_map_error) end diff --git a/app/models/concerns/sanitizable.rb b/app/models/concerns/sanitizable.rb index 8602257ef..387d089ff 100644 --- a/app/models/concerns/sanitizable.rb +++ b/app/models/concerns/sanitizable.rb @@ -7,7 +7,7 @@ module Sanitizable unless included_modules.include? Globalizable def description - super.try :html_safe + super&.html_safe end end end diff --git a/app/models/debate.rb b/app/models/debate.rb index 375f79c5f..d5d2f4dc3 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -74,7 +74,7 @@ class Debate < ApplicationRecord { author.username => "B", tag_list.join(" ") => "B", - geozone.try(:name) => "B", + geozone&.name => "B", }.merge!(searchable_globalized_values) end diff --git a/app/models/flag.rb b/app/models/flag.rb index e69393d49..b01d7b9a9 100644 --- a/app/models/flag.rb +++ b/app/models/flag.rb @@ -23,7 +23,7 @@ class Flag < ApplicationRecord def self.flagged?(user, flaggable) return false unless user - !!by_user_and_flaggable(user, flaggable).try(:first) + !!by_user_and_flaggable(user, flaggable)&.first end end diff --git a/app/models/legislation/proposal.rb b/app/models/legislation/proposal.rb index 44becffc8..311c38de2 100644 --- a/app/models/legislation/proposal.rb +++ b/app/models/legislation/proposal.rb @@ -59,7 +59,7 @@ class Legislation::Proposal < ApplicationRecord { title => "A", author.username => "B", tag_list.join(" ") => "B", - geozone.try(:name) => "B", + geozone&.name => "B", summary => "C", description => "D" } end diff --git a/app/models/poll/question.rb b/app/models/poll/question.rb index 0016e0c02..bd8d8e79b 100644 --- a/app/models/poll/question.rb +++ b/app/models/poll/question.rb @@ -44,10 +44,10 @@ class Poll::Question < ApplicationRecord end def searchable_values - { title => "A", - proposal.try(:title) => "A", - author.username => "C", - author_visible_name => "C" } + { title => "A", + proposal&.title => "A", + author.username => "C", + author_visible_name => "C" } end def copy_attributes_from_proposal(proposal) diff --git a/app/models/poll/question/answer.rb b/app/models/poll/question/answer.rb index 48968e51b..5ac37c10a 100644 --- a/app/models/poll/question/answer.rb +++ b/app/models/poll/question/answer.rb @@ -20,7 +20,7 @@ class Poll::Question::Answer < ApplicationRecord scope :visibles, -> { where(hidden: false) } def description - self[:description].try :html_safe + self[:description]&.html_safe end def self.order_answers(ordered_array) diff --git a/app/models/poll/voter.rb b/app/models/poll/voter.rb index c899cabc9..d079ea991 100644 --- a/app/models/poll/voter.rb +++ b/app/models/poll/voter.rb @@ -42,7 +42,7 @@ class Poll private def set_denormalized_booth_assignment_id - self.booth_assignment_id ||= officer_assignment.try(:booth_assignment_id) + self.booth_assignment_id ||= officer_assignment&.booth_assignment_id end def in_census? @@ -56,7 +56,7 @@ class Poll def fill_stats_fields if in_census? self.gender = census_api_response.gender - self.geozone_id = Geozone.select(:id).where(census_code: census_api_response.district_code).first.try(:id) + self.geozone_id = Geozone.select(:id).where(census_code: census_api_response.district_code).first&.id self.age = voter_age(census_api_response.date_of_birth) end end diff --git a/app/models/proposal.rb b/app/models/proposal.rb index a4d9d10eb..9733a542f 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -135,7 +135,7 @@ class Proposal < ApplicationRecord { author.username => "B", tag_list.join(" ") => "B", - geozone.try(:name) => "B" + geozone&.name => "B" }.merge!(searchable_globalized_values) end diff --git a/app/models/proposal_notification.rb b/app/models/proposal_notification.rb index 3ea3ea906..e3ccb67a9 100644 --- a/app/models/proposal_notification.rb +++ b/app/models/proposal_notification.rb @@ -25,7 +25,7 @@ class ProposalNotification < ApplicationRecord after_create :set_author def minimum_interval - return true if proposal.try(:notifications).blank? + return true if proposal&.notifications.blank? interval = Setting[:proposal_notification_minimum_interval_in_days] minimum_interval = (Time.current - interval.to_i.days).to_datetime if proposal.notifications.last.created_at > minimum_interval diff --git a/app/models/site_customization/content_block.rb b/app/models/site_customization/content_block.rb index 2219ef5d4..a96bc8b47 100644 --- a/app/models/site_customization/content_block.rb +++ b/app/models/site_customization/content_block.rb @@ -6,6 +6,6 @@ class SiteCustomization::ContentBlock < ApplicationRecord def self.block_for(name, locale) locale ||= I18n.default_locale - find_by(name: name, locale: locale).try(:body) + find_by(name: name, locale: locale)&.body end end diff --git a/app/models/site_customization/image.rb b/app/models/site_customization/image.rb index c800e21b1..bcfbdd376 100644 --- a/app/models/site_customization/image.rb +++ b/app/models/site_customization/image.rb @@ -29,11 +29,11 @@ class SiteCustomization::Image < ApplicationRecord end def required_width - VALID_IMAGES[name].try(:first) + VALID_IMAGES[name]&.first end def required_height - VALID_IMAGES[name].try(:second) + VALID_IMAGES[name]&.second end private diff --git a/app/models/valuator.rb b/app/models/valuator.rb index 372dacc01..625395f0c 100644 --- a/app/models/valuator.rb +++ b/app/models/valuator.rb @@ -18,7 +18,7 @@ class Valuator < ApplicationRecord end def assigned_investment_ids - investment_ids + [valuator_group.try(:investment_ids)].flatten + investment_ids + [valuator_group&.investment_ids].flatten end end diff --git a/app/models/verification/email.rb b/app/models/verification/email.rb index 95faecad3..823f165cf 100644 --- a/app/models/verification/email.rb +++ b/app/models/verification/email.rb @@ -8,7 +8,7 @@ class Verification::Email def initialize(verified_user) @verified_user = verified_user - @recipient = @verified_user.try(:email) + @recipient = @verified_user&.email end def save diff --git a/app/models/verification/letter.rb b/app/models/verification/letter.rb index e70da2b5f..599a628f1 100644 --- a/app/models/verification/letter.rb +++ b/app/models/verification/letter.rb @@ -28,7 +28,7 @@ class Verification::Letter def validate_correct_code return if errors.include?(:verification_code) - if user.try(:letter_verification_code).to_i != verification_code.to_i + if user&.letter_verification_code.to_i != verification_code.to_i errors.add(:verification_code, I18n.t("verification.letter.errors.incorrect_code")) end end diff --git a/spec/support/common_actions/users.rb b/spec/support/common_actions/users.rb index c5271273f..27b2986f1 100644 --- a/spec/support/common_actions/users.rb +++ b/spec/support/common_actions/users.rb @@ -54,7 +54,7 @@ module Users end def confirm_email - body = ActionMailer::Base.deliveries.last.try(:body) + body = ActionMailer::Base.deliveries.last&.body expect(body).to be_present sent_token = /.*confirmation_token=(.*)".*/.match(body.to_s)[1] From 27c73c22ea9e7d28cca03423fd06ad0c4f04e264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 00:37:31 +0200 Subject: [PATCH 05/13] Configure `Rails/UnknownEnv` rubocop rule We use staging and preproduction environments, which are not valid by default. This rule is useful because misspelling the name of an environment might otherwise go unnoticed. --- .rubocop.yml | 3 --- .rubocop_basic.yml | 9 +++++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6a374e962..5e563232c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -165,9 +165,6 @@ Rails/SkipsModelValidations: Rails/UniqBeforePluck: Enabled: true -Rails/UnknownEnv: - Enabled: true - Rails/Validation: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 2af5aff4a..865a6c76f 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -148,6 +148,15 @@ Rails/SafeNavigation: Rails/TimeZone: Enabled: true +Rails/UnknownEnv: + Enabled: true + Environments: + - development + - test + - production + - preproduction + - staging + RSpec/NotToNot: Enabled: true From 0788925c1bb756802ad225b2c1ea386329db7afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 00:46:28 +0200 Subject: [PATCH 06/13] Apply `Rails/Validation` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ app/models/budget/content_block.rb | 3 +-- app/models/newsletter.rb | 4 +--- app/models/votation_type.rb | 9 +++------ 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 5e563232c..a64b6f7d0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -165,9 +165,6 @@ Rails/SkipsModelValidations: Rails/UniqBeforePluck: Enabled: true -Rails/Validation: - Enabled: true - RSpec/AlignLeftLetBrace: Enabled: false diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 865a6c76f..127a6dca8 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -157,6 +157,9 @@ Rails/UnknownEnv: - preproduction - staging +Rails/Validation: + Enabled: true + RSpec/NotToNot: Enabled: true diff --git a/app/models/budget/content_block.rb b/app/models/budget/content_block.rb index ae3bf44e9..06de2bd19 100644 --- a/app/models/budget/content_block.rb +++ b/app/models/budget/content_block.rb @@ -1,8 +1,7 @@ class Budget class ContentBlock < ApplicationRecord validates :locale, presence: true, inclusion: { in: I18n.available_locales.map(&:to_s) } - validates :heading, presence: true - validates_uniqueness_of :heading, scope: :locale + validates :heading, presence: true, uniqueness: { scope: :locale } belongs_to :heading end diff --git a/app/models/newsletter.rb b/app/models/newsletter.rb index df088ce8e..e6fcf7720 100644 --- a/app/models/newsletter.rb +++ b/app/models/newsletter.rb @@ -3,12 +3,10 @@ class Newsletter < ApplicationRecord validates :subject, presence: true validates :segment_recipient, presence: true - validates :from, presence: true + validates :from, presence: true, format: { with: /@/ } validates :body, presence: true validate :validate_segment_recipient - validates_format_of :from, :with => /@/ - acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases diff --git a/app/models/votation_type.rb b/app/models/votation_type.rb index c518408f7..891ba250e 100644 --- a/app/models/votation_type.rb +++ b/app/models/votation_type.rb @@ -30,12 +30,9 @@ class VotationType < ApplicationRecord validates :questionable, presence: true validates :questionable_type, inclusion: { in: QUESTIONABLE_TYPES } - validates_presence_of :max_votes, allow_blank: false, - if: :max_votes_required? - validates_presence_of :max_groups_answers, allow_blank: false, - if: :max_groups_answers_required? - validates_presence_of :prioritization_type, allow_blank: false, - if: :prioritization_type_required? + validates :max_votes, presence: { allow_blank: false, if: :max_votes_required? } + validates :max_groups_answers, presence: { allow_blank: false, if: :max_groups_answers_required? } + validates :prioritization_type, presence: { allow_blank: false, if: :prioritization_type_required? } after_create :add_skip_question_answer, if: :display_skip_question? From 58ba517717b54ff58c585a2260616e57129b05d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 01:07:25 +0200 Subject: [PATCH 07/13] Apply `RSpec/ExampleWording` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ spec/models/budget_spec.rb | 2 +- spec/models/notification_spec.rb | 4 ++-- spec/models/poll/voter_spec.rb | 6 +++--- spec/models/setting_spec.rb | 2 +- spec/models/valuator_group_spec.rb | 6 +++--- spec/shared/models/acts_as_paranoid.rb | 4 ++-- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a64b6f7d0..37c46bef1 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -210,9 +210,6 @@ RSpec/EmptyLineAfterSubject: RSpec/ExampleLength: Enabled: false -RSpec/ExampleWording: - Enabled: true - RSpec/ExpectActual: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 127a6dca8..269326ad2 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -160,6 +160,9 @@ Rails/UnknownEnv: Rails/Validation: Enabled: true +RSpec/ExampleWording: + Enabled: true + RSpec/NotToNot: Enabled: true diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index 851d6076a..1ea1d534a 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -223,7 +223,7 @@ describe Budget do end describe "#has_winning_investments?" do - it "should return true if there is a winner investment" do + it "returns true if there is a winner investment" do budget.investments << build(:budget_investment, :winner, price: 3, ballot_lines_count: 2) expect(budget.has_winning_investments?).to eq true diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 54d680267..155764883 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -6,11 +6,11 @@ describe Notification do context "validations" do - it "should be valid" do + it "is valid" do expect(notification).to be_valid end - it "should not be valid without a user" do + it "is not valid without a user" do notification.user = nil expect(notification).not_to be_valid end diff --git a/spec/models/poll/voter_spec.rb b/spec/models/poll/voter_spec.rb index aab54eb42..4c4ec9a5e 100644 --- a/spec/models/poll/voter_spec.rb +++ b/spec/models/poll/voter_spec.rb @@ -109,19 +109,19 @@ describe Poll::Voter do end context "assignments" do - it "should not be valid without a booth_assignment_id when origin is booth" do + it "is not valid without a booth_assignment_id when origin is booth" do voter.origin = "booth" voter.booth_assignment_id = nil expect(voter).not_to be_valid end - it "should not be valid without an officer_assignment_id when origin is booth" do + it "is not valid without an officer_assignment_id when origin is booth" do voter.origin = "booth" voter.officer_assignment_id = nil expect(voter).not_to be_valid end - it "should be valid without assignments when origin is web" do + it "is valid without assignments when origin is web" do voter.origin = "web" voter.booth_assignment_id = nil voter.officer_assignment_id = nil diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 282dc6d47..a82c4e33a 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -9,7 +9,7 @@ describe Setting do expect(described_class["official_level_1_name"]).to eq("Stormtrooper") end - it "shoulds return nil" do + it "returns nil" do expect(described_class["undefined_key"]).to eq(nil) end diff --git a/spec/models/valuator_group_spec.rb b/spec/models/valuator_group_spec.rb index 910fd7ec7..2aadfb31b 100644 --- a/spec/models/valuator_group_spec.rb +++ b/spec/models/valuator_group_spec.rb @@ -3,15 +3,15 @@ require "rails_helper" describe ValuatorGroup do describe "Validations" do - it "should be valid" do + it "is valid" do expect(build(:valuator_group)).to be_valid end - it "should not be valid without a name" do + it "is not valid without a name" do expect(build(:valuator_group, name: nil)).not_to be_valid end - it "should not be valid with the same name as an existing one" do + it "is not valid with the same name as an existing one" do create(:valuator_group, name: "The Valuators") expect(build(:valuator_group, name: "The Valuators")).not_to be_valid diff --git a/spec/shared/models/acts_as_paranoid.rb b/spec/shared/models/acts_as_paranoid.rb index 63c392915..dbe3d337e 100644 --- a/spec/shared/models/acts_as_paranoid.rb +++ b/spec/shared/models/acts_as_paranoid.rb @@ -14,14 +14,14 @@ shared_examples "acts as paranoid" do |factory_name| describe "#{described_class} translations" do - it "should be hidden after parent resource destroy" do + it "is hidden after parent resource destroy" do resource.destroy resource.reload expect(resource.translations.with_deleted.first.hidden_at).not_to be_blank end - it "should be destroyed after parent resource really_destroy" do + it "is destroyed after parent resource really_destroy" do expect { resource.really_destroy! }.to change { resource.translations.with_deleted.count }.from(1).to(0) end From 044eabd7efaf566f182854239b7e900cdb3f97af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 01:34:22 +0200 Subject: [PATCH 08/13] Apply `Rspec/LetSetup` rubocop rule --- .rubocop.yml | 3 -- .rubocop_basic.yml | 3 ++ spec/features/notifications_spec.rb | 6 ++-- spec/features/polls/votation_type_spec.rb | 40 +++++++++++++---------- spec/features/polls/voter_spec.rb | 2 +- spec/mailers/dashboard/mailer_spec.rb | 7 ++-- spec/models/newsletter_spec.rb | 2 +- spec/models/poll/ballot_spec.rb | 2 +- 8 files changed, 34 insertions(+), 31 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 37c46bef1..8f52dad0d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -253,9 +253,6 @@ RSpec/LeadingSubject: RSpec/LetBeforeExamples: Enabled: true -RSpec/LetSetup: - Enabled: true - RSpec/MessageChain: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 269326ad2..2301d4072 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -163,6 +163,9 @@ Rails/Validation: RSpec/ExampleWording: Enabled: true +RSpec/LetSetup: + Enabled: true + RSpec/NotToNot: Enabled: true diff --git a/spec/features/notifications_spec.rb b/spec/features/notifications_spec.rb index c998ac62e..8bf3acb5d 100644 --- a/spec/features/notifications_spec.rb +++ b/spec/features/notifications_spec.rb @@ -186,11 +186,11 @@ describe "Notifications" do let!(:user2) { create(:user) } let!(:user3) { create(:user) } let!(:proposal_notification) { create(:proposal_notification) } - let!(:notification1) { create(:notification, notifiable: proposal_notification, user: user1) } - let!(:notification2) { create(:notification, notifiable: proposal_notification, user: user2) } - let!(:notification3) { create(:notification, notifiable: proposal_notification, user: user3) } before do + create(:notification, notifiable: proposal_notification, user: user1) + create(:notification, notifiable: proposal_notification, user: user2) + create(:notification, notifiable: proposal_notification, user: user3) reset_mailer Delayed::Worker.delay_jobs = true end diff --git a/spec/features/polls/votation_type_spec.rb b/spec/features/polls/votation_type_spec.rb index 1a42d7b40..ffc477c9f 100644 --- a/spec/features/polls/votation_type_spec.rb +++ b/spec/features/polls/votation_type_spec.rb @@ -63,12 +63,13 @@ describe "Poll Votation Type" do let(:poll_current) { create(:poll, :current) } let(:question) { create(:poll_question_multiple, poll: poll_current) } let!(:answer1) { create(:poll_question_answer, question: question, title: "answer_1") } - let!(:answer2) { create(:poll_question_answer, question: question, title: "answer_2") } - let!(:answer3) { create(:poll_question_answer, question: question, title: "answer_3") } - let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") } - let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") } before do + create(:poll_question_answer, question: question, title: "answer_2") + create(:poll_question_answer, question: question, title: "answer_3") + create(:poll_question_answer, question: question, title: "answer_4") + create(:poll_question_answer, question: question, title: "answer_5") + login_as(user) end @@ -152,13 +153,14 @@ describe "Poll Votation Type" do let(:user) { create(:user, :verified) } let(:poll_current) { create(:poll, :current) } let(:question) { create(:poll_question_prioritized, poll: poll_current) } - let!(:answer1) { create(:poll_question_answer, question: question, title: "answer_1") } - let!(:answer2) { create(:poll_question_answer, question: question, title: "answer_2") } - let!(:answer3) { create(:poll_question_answer, question: question, title: "answer_3") } - let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") } - let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") } before do + create(:poll_question_answer, question: question, title: "answer_1") + create(:poll_question_answer, question: question, title: "answer_2") + create(:poll_question_answer, question: question, title: "answer_3") + create(:poll_question_answer, question: question, title: "answer_4") + create(:poll_question_answer, question: question, title: "answer_5") + login_as(user) end @@ -218,12 +220,13 @@ describe "Poll Votation Type" do let(:poll_current) { create(:poll, :current) } let(:question) { create(:poll_question_positive_open, poll: poll_current) } let!(:answer1) { create(:poll_question_answer, question: question, title: "answer_1") } - let!(:answer2) { create(:poll_question_answer, question: question, title: "answer_2") } - let!(:answer3) { create(:poll_question_answer, question: question, title: "answer_3") } - let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") } - let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") } before do + create(:poll_question_answer, question: question, title: "answer_2") + create(:poll_question_answer, question: question, title: "answer_3") + create(:poll_question_answer, question: question, title: "answer_4") + create(:poll_question_answer, question: question, title: "answer_5") + login_as(user) end @@ -324,13 +327,14 @@ describe "Poll Votation Type" do let(:user) { create(:user, :verified) } let(:poll_current) { create(:poll, :current) } let(:question) { create(:poll_question_answer_set_open, poll: poll_current) } - let!(:answer1) { create(:poll_question_answer, question: question, title: "answer_1") } - let!(:answer2) { create(:poll_question_answer, question: question, title: "answer_2") } - let!(:answer3) { create(:poll_question_answer, question: question, title: "answer_3") } - let!(:answer4) { create(:poll_question_answer, question: question, title: "answer_4") } - let!(:answer5) { create(:poll_question_answer, question: question, title: "answer_5") } before do + create(:poll_question_answer, question: question, title: "answer_1") + create(:poll_question_answer, question: question, title: "answer_2") + create(:poll_question_answer, question: question, title: "answer_3") + create(:poll_question_answer, question: question, title: "answer_4") + create(:poll_question_answer, question: question, title: "answer_5") + login_as(user) end diff --git a/spec/features/polls/voter_spec.rb b/spec/features/polls/voter_spec.rb index 760957254..f444df31a 100644 --- a/spec/features/polls/voter_spec.rb +++ b/spec/features/polls/voter_spec.rb @@ -83,7 +83,7 @@ describe "Voter" do end context "The person has decided not to vote at this time" do - let!(:user) { create(:user, :in_census) } + before { create(:user, :in_census) } scenario "Show not to vote at this time button" do login_through_form_as_officer(officer.user) diff --git a/spec/mailers/dashboard/mailer_spec.rb b/spec/mailers/dashboard/mailer_spec.rb index 4751a51eb..fff03da78 100644 --- a/spec/mailers/dashboard/mailer_spec.rb +++ b/spec/mailers/dashboard/mailer_spec.rb @@ -221,13 +221,12 @@ describe Dashboard::Mailer do before do ActionMailer::Base.deliveries.clear + + create(:dashboard_action, :resource, :active, day_offset: 0, published_proposal: true) + create(:dashboard_action, :proposed_action, :active, day_offset: 0, published_proposal: true) end let!(:proposal) { build(:proposal, :draft) } - let!(:resource) { create(:dashboard_action, :resource, :active, day_offset: 0, - published_proposal: true) } - let!(:proposed_action) { create(:dashboard_action, :proposed_action, :active, day_offset: 0, - published_proposal: true) } it "Disables email delivery using setting" do Setting["dashboard.emails"] = nil diff --git a/spec/models/newsletter_spec.rb b/spec/models/newsletter_spec.rb index cdde03e7f..364250255 100644 --- a/spec/models/newsletter_spec.rb +++ b/spec/models/newsletter_spec.rb @@ -66,12 +66,12 @@ describe Newsletter do describe "#deliver" do let!(:proposals) { Array.new(3) { create(:proposal) } } - let!(:debate) { create(:debate) } let!(:recipients) { proposals.map(&:author).map(&:email) } let!(:newsletter) { create(:newsletter, segment_recipient: "proposal_authors") } before do + create(:debate) reset_mailer Delayed::Worker.delay_jobs = true end diff --git a/spec/models/poll/ballot_spec.rb b/spec/models/poll/ballot_spec.rb index 8e449a533..f48a5dc6a 100644 --- a/spec/models/poll/ballot_spec.rb +++ b/spec/models/poll/ballot_spec.rb @@ -9,7 +9,7 @@ describe Poll::Ballot do let(:poll) { create(:poll, budget: budget) } let(:poll_ballot_sheet) { create(:poll_ballot_sheet, poll: poll) } let(:poll_ballot) { create(:poll_ballot, ballot_sheet: poll_ballot_sheet, external_id: 1, data: investment.id) } - let!(:ballot) { create(:budget_ballot, budget: budget, physical: true, poll_ballot: poll_ballot) } + before { create(:budget_ballot, budget: budget, physical: true, poll_ballot: poll_ballot) } describe "#verify" do From 969a4e21c91a06bfa0338dafaba2145f0be5e9d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 02:04:58 +0200 Subject: [PATCH 09/13] Apply `RSpec/RepeatedExample` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ spec/features/budgets/investments_spec.rb | 18 ------------------ spec/models/abilities/valuator_spec.rb | 6 ------ 4 files changed, 3 insertions(+), 27 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8f52dad0d..37ef049be 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -291,9 +291,6 @@ Rails/HttpStatus: RSpec/RepeatedDescription: Enabled: true -RSpec/RepeatedExample: - Enabled: true - RSpec/ReturnFromStub: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 2301d4072..452eef9f7 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -169,6 +169,9 @@ RSpec/LetSetup: RSpec/NotToNot: Enabled: true +RSpec/RepeatedExample: + Enabled: true + Style/PercentLiteralDelimiters: Enabled: true diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb index 0894d9314..4e5c70250 100644 --- a/spec/features/budgets/investments_spec.rb +++ b/spec/features/budgets/investments_spec.rb @@ -1292,24 +1292,6 @@ describe "Budget Investments" do expect(page).not_to have_content("Local government is not competent in this matter") end - scenario "Show (unfeasible budget investment with valuation not finished)" do - user = create(:user) - login_as(user) - - investment = create(:budget_investment, - :unfeasible, - valuation_finished: false, - budget: budget, - group: group, - heading: heading, - unfeasibility_explanation: "Local government is not competent in this matter") - - visit budget_investment_path(budget, id: investment.id) - - expect(page).not_to have_content("Unfeasibility explanation") - expect(page).not_to have_content("Local government is not competent in this matter") - end - it_behaves_like "followable", "budget_investment", "budget_investment_path", { "budget_id": "budget_id", "id": "id" } it_behaves_like "imageable", "budget_investment", "budget_investment_path", { "budget_id": "budget_id", "id": "id" } diff --git a/spec/models/abilities/valuator_spec.rb b/spec/models/abilities/valuator_spec.rb index 324292ca5..c50f9adfd 100644 --- a/spec/models/abilities/valuator_spec.rb +++ b/spec/models/abilities/valuator_spec.rb @@ -27,12 +27,6 @@ describe Abilities::Valuator do should_not be_able_to(:valuate, assigned_investment) end - it "cannot valuate an assigned investment with a finished valuation" do - assigned_investment.update(valuation_finished: true) - - should_not be_able_to(:valuate, assigned_investment) - end - it { should_not be_able_to(:update, non_assigned_investment) } it { should_not be_able_to(:valuate, non_assigned_investment) } From c05b9c2aac394bfbb1a9180d6b78a1190755d4a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 02:13:38 +0200 Subject: [PATCH 10/13] Apply Capybara/CurrentPathExpectation rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ spec/features/admin/activity_spec.rb | 3 ++- spec/features/admin/users_spec.rb | 2 +- spec/features/budget_polls/budgets_spec.rb | 4 ++-- spec/features/proposals_spec.rb | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 37ef049be..0caae6c68 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -183,9 +183,6 @@ RSpec/BeEql: RSpec/BeforeAfterAll: Enabled: true -Capybara/CurrentPathExpectation: - Enabled: true - Capybara/FeatureMethods: Enabled: false diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 452eef9f7..3767e18ba 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -11,6 +11,9 @@ AllCops: # to ignore them, so only the ones explicitly set in this file are enabled. DisabledByDefault: true +Capybara/CurrentPathExpectation: + Enabled: true + Capybara/FeatureMethods: Enabled: true EnabledMethods: diff --git a/spec/features/admin/activity_spec.rb b/spec/features/admin/activity_spec.rb index 891e59947..726df1c6f 100644 --- a/spec/features/admin/activity_spec.rb +++ b/spec/features/admin/activity_spec.rb @@ -205,7 +205,8 @@ describe "Admin activity" do within("#proposal_#{proposal.id}") do click_link "Hide author" - expect(current_path).to eq(debates_path) + + expect(page).to have_current_path(debates_path) end visit admin_activity_path diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index e3a0d9950..cdc5590be 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -18,7 +18,7 @@ describe "Admin users" do scenario "The username links to their public profile" do click_link @user.name - expect(current_path).to eq(user_path(@user)) + expect(page).to have_current_path(user_path(@user)) end scenario "Search" do diff --git a/spec/features/budget_polls/budgets_spec.rb b/spec/features/budget_polls/budgets_spec.rb index 6362e008a..545bebae2 100644 --- a/spec/features/budget_polls/budgets_spec.rb +++ b/spec/features/budget_polls/budgets_spec.rb @@ -18,7 +18,7 @@ describe "Admin Budgets" do balloting_phase = budget.phases.where(kind: "balloting").first - expect(current_path).to match(/admin\/polls\/\d+/) + expect(page).to have_current_path(/admin\/polls\/\d+/) expect(page).to have_content(budget.name) expect(page).to have_content(balloting_phase.starts_at.to_date) expect(page).to have_content(balloting_phase.ends_at.to_date) @@ -37,7 +37,7 @@ describe "Admin Budgets" do click_link "Bulletins de l’admin" - expect(current_path).to match(/admin\/polls\/\d+/) + expect(page).to have_current_path(/admin\/polls\/\d+/) expect(page).to have_content("Budget pour le changement climatique") expect(Poll.count).to eq(1) diff --git a/spec/features/proposals_spec.rb b/spec/features/proposals_spec.rb index 19aa724d6..42b764a31 100644 --- a/spec/features/proposals_spec.rb +++ b/spec/features/proposals_spec.rb @@ -1948,7 +1948,7 @@ describe "Successful proposals" do click_link "Create a proposal" end - expect(current_path).to eq(new_proposal_path) + expect(page).to have_current_path(new_proposal_path) fill_in "Proposal title", with: "Help refugees" fill_in "Proposal summary", with: "In summary what we want is..." From 9541ce892c3c93dd6bcd658ca24a3c0f858efbfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 02:43:43 +0200 Subject: [PATCH 11/13] Apply Bundler rubocop rules We're not using the InsecureProtocolSource rule because I don't feel it's necessary. --- .rubocop.yml | 6 ------ .rubocop_basic.yml | 6 ++++++ Gemfile | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0caae6c68..a726ce2b2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,14 +1,8 @@ inherit_from: .rubocop_basic.yml -Bundler/DuplicatedGem: - Enabled: true - Bundler/InsecureProtocolSource: Enabled: true -Bundler/OrderedGems: - Enabled: true - Gemspec/DuplicatedAssignment: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 3767e18ba..52e7324ce 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -11,6 +11,12 @@ AllCops: # to ignore them, so only the ones explicitly set in this file are enabled. DisabledByDefault: true +Bundler/DuplicatedGem: + Enabled: true + +Bundler/OrderedGems: + Enabled: true + Capybara/CurrentPathExpectation: Enabled: true diff --git a/Gemfile b/Gemfile index b3c510745..81a57dd66 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,8 @@ gem "acts_as_votable", "~> 0.11.1" gem "ahoy_matey", "~> 1.6.0" gem "ancestry", "~> 3.0.2" gem "autoprefixer-rails", "~> 8.2.0" +gem "axlsx", "~> 3.0.0.pre" +gem "axlsx_rails", "~> 0.5.2" gem "browser", "~> 2.5.3" gem "cancancan", "~> 2.3.0" gem "ckeditor", "~> 4.2.3" @@ -40,6 +42,7 @@ gem "paperclip", "~> 5.2.1" gem "paranoia", "~> 2.4.1" gem "pg", "~> 0.21.0" gem "pg_search", "~> 2.0.1" +gem "recipient_interceptor", "~> 0.2.0" gem "redcarpet", "~> 3.4.0" gem "responders", "~> 2.4.0" gem "rinku", "~> 2.0.2", require: "rails_rinku" @@ -55,11 +58,8 @@ gem "turnout", "~> 2.4.0" gem "uglifier", "~> 4.1.2" gem "unicorn", "~> 5.4.1" gem "whenever", "~> 0.10.0", require: false -gem "recipient_interceptor", "~> 0.2.0" -gem "wkhtmltopdf-binary", "~> 0.12.4" gem "wicked_pdf", "~> 1.1.0" -gem "axlsx", "~> 3.0.0.pre" -gem "axlsx_rails", "~> 0.5.2" +gem "wkhtmltopdf-binary", "~> 0.12.4" source "https://rails-assets.org" do gem "rails-assets-leaflet" From 59e107e56597b0a9098480beaffa066a8feea9e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Jun 2019 15:35:32 +0200 Subject: [PATCH 12/13] Apply `RSpec/HookArgument` rubocop rule --- .rubocop.yml | 3 --- .rubocop_basic.yml | 3 +++ spec/shared/request_spec_helper.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a726ce2b2..342173e6e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -216,9 +216,6 @@ RSpec/FilePath: RSpec/Focus: Enabled: true -RSpec/HookArgument: - Enabled: true - RSpec/ImplicitExpect: Enabled: true EnforcedStyle: should diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 52e7324ce..2adca39ed 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -172,6 +172,9 @@ Rails/Validation: RSpec/ExampleWording: Enabled: true +RSpec/HookArgument: + Enabled: true + RSpec/LetSetup: Enabled: true diff --git a/spec/shared/request_spec_helper.rb b/spec/shared/request_spec_helper.rb index f3f8de26b..30959ced6 100644 --- a/spec/shared/request_spec_helper.rb +++ b/spec/shared/request_spec_helper.rb @@ -2,8 +2,8 @@ module RequestSpecHelper include Warden::Test::Helpers def self.included(base) - base.before(:each) { Warden.test_mode! } - base.after(:each) { Warden.test_reset! } + base.before { Warden.test_mode! } + base.after { Warden.test_reset! } end def sign_in(resource) From 2243809d2e1f193456eb09fcc2422b81f0a92bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Jun 2019 15:22:35 +0200 Subject: [PATCH 13/13] Move basic RSpec rubocop rules to basic cops These are rules we were already applying. We've excluded the `factories` folder for some rules because there's a factory defining a `context` attribute, which rubocop thought was the `context` RSpec keyword. --- .rubocop.yml | 30 ------------------------------ .rubocop_basic.yml | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 342173e6e..d2733f69c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -168,15 +168,9 @@ RSpec/AlignRightLetBrace: RSpec/AnyInstance: Enabled: false -RSpec/AroundBlock: - Enabled: true - RSpec/BeEql: Enabled: true -RSpec/BeforeAfterAll: - Enabled: true - Capybara/FeatureMethods: Enabled: false @@ -192,12 +186,6 @@ RSpec/DescribeMethod: RSpec/DescribeSymbol: Enabled: true -RSpec/EmptyExampleGroup: - Enabled: true - -RSpec/EmptyLineAfterSubject: - Enabled: true - RSpec/ExampleLength: Enabled: false @@ -213,9 +201,6 @@ RSpec/ExpectOutput: RSpec/FilePath: Enabled: true -RSpec/Focus: - Enabled: true - RSpec/ImplicitExpect: Enabled: true EnforcedStyle: should @@ -238,9 +223,6 @@ RSpec/IteratedExpectation: RSpec/LeadingSubject: Enabled: true -RSpec/LetBeforeExamples: - Enabled: true - RSpec/MessageChain: Enabled: true @@ -267,9 +249,6 @@ RSpec/NestedGroups: Enabled: true Max: 4 -RSpec/OverwritingSetup: - Enabled: true - RSpec/PredicateMatcher: Enabled: true @@ -282,12 +261,6 @@ RSpec/RepeatedDescription: RSpec/ReturnFromStub: Enabled: true -RSpec/ScatteredLet: - Enabled: true - -RSpec/ScatteredSetup: - Enabled: true - RSpec/SharedContext: Enabled: true @@ -300,9 +273,6 @@ RSpec/SubjectStub: RSpec/VerifiedDoubles: Enabled: true -RSpec/VoidExpect: - Enabled: true - Security/Eval: Enabled: true diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 2adca39ed..88fdf0f9c 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -169,21 +169,55 @@ Rails/UnknownEnv: Rails/Validation: Enabled: true +RSpec/AroundBlock: + Enabled: true + +RSpec/BeforeAfterAll: + Enabled: true + +RSpec/EmptyExampleGroup: + Enabled: true + Exclude: + - spec/factories/**/* + +RSpec/EmptyLineAfterExampleGroup: + Enabled: true + Exclude: + - spec/factories/**/* + RSpec/ExampleWording: Enabled: true +RSpec/Focus: + Enabled: true + RSpec/HookArgument: Enabled: true +RSpec/LetBeforeExamples: + Enabled: true + RSpec/LetSetup: Enabled: true RSpec/NotToNot: Enabled: true +RSpec/OverwritingSetup: + Enabled: true + RSpec/RepeatedExample: Enabled: true +RSpec/ScatteredLet: + Enabled: true + +RSpec/ScatteredSetup: + Enabled: true + +RSpec/VoidExpect: + Enabled: true + Style/PercentLiteralDelimiters: Enabled: true