Add and apply EmptyLineAfterGuardClause rule

We were inconsistent on this one. I consider it particularly useful when
a method starts with a `return` statement.

In other cases, we probably shouldn't have a guard rule in the middle of
a method in any case, but that's a different refactoring.
This commit is contained in:
Javi Martín
2019-10-24 16:19:27 +02:00
parent db97f9d08c
commit d0d681a44b
69 changed files with 102 additions and 1 deletions

View File

@@ -31,6 +31,9 @@ Capybara/FeatureMethods:
FactoryBot/AttributeDefinedStatically:
Enabled: true
Layout/EmptyLineAfterGuardClause:
Enabled: true
Layout/EmptyLineBetweenDefs:
Enabled: true

View File

@@ -30,6 +30,7 @@ class Admin::BudgetsController < Admin::BaseController
def calculate_winners
return unless @budget.balloting_process?
@budget.headings.each { |heading| Budget::Result.new(@budget, heading).delay.calculate_winners }
redirect_to admin_budget_budget_investments_path(
budget_id: @budget.id,

View File

@@ -55,6 +55,7 @@ class Admin::SettingsController < Admin::BaseController
def request_referer
return request.referer + params[:setting][:tab] if params[:setting][:tab]
request.referer
end
end

View File

@@ -8,6 +8,7 @@ class CommunitiesController < ApplicationController
def show
raise ActionController::RoutingError, "Not Found" unless communitable_exists?
redirect_to root_path if Setting["feature.community"].blank?
end

View File

@@ -5,11 +5,13 @@ module Dashboard::ExpectsDateRange
def start_date(fallback_date = proposal.created_at.to_date)
return Date.parse(params[:start_date]) unless params[:start_date].blank?
fallback_date
end
def end_date
return Date.parse(params[:end_date]) unless params[:end_date].blank?
Date.current
end
end

View File

@@ -47,6 +47,7 @@ module Dashboard::GroupSupports
def interval
return 1.week if params[:group_by] == "week"
return 1.month if params[:group_by] == "month"
1.day
end
end

View File

@@ -18,6 +18,7 @@ module Search
def parse_search_date
return unless search_by_date?
params[:advanced_search][:date_range] = search_date_range
end

View File

@@ -12,6 +12,7 @@ class GraphqlController < ApplicationController
def query
begin
if query_string.nil? then raise GraphqlController::QueryStringError end
response = consul_schema.execute query_string, variables: query_variables
render json: response, status: :ok
rescue GraphqlController::QueryStringError

View File

@@ -159,6 +159,7 @@ class Legislation::ProcessesController < Legislation::BaseController
def set_process
return if member_method?
@process = ::Legislation::Process.find(params[:process_id])
end
end

View File

@@ -45,6 +45,7 @@ class Management::DocumentVerificationsController < Management::BaseController
def clean_document_number
return if params[:document_verification][:document_number].blank?
params[:document_verification][:document_number] = params[:document_verification][:document_number].gsub(/[^a-z0-9]+/i, "").upcase
end
end

View File

@@ -31,6 +31,7 @@ class Officing::BaseController < ApplicationController
def verify_booth
return unless current_booth.blank?
booths = current_user.poll_officer.todays_booths
case booths.count
when 0

View File

@@ -42,6 +42,7 @@ class Officing::ResultsController < Officing::BaseController
results.each_pair do |answer_index, count|
next if count.blank?
answer = question.question_answers.where(given_order: answer_index.to_i + 1).first.title
go_back_to_new if question.blank?

View File

@@ -44,6 +44,7 @@ class Polls::AnswersController < ApplicationController
exist = false
@question.question_answers.each do |question_answer|
break if exist
exist = true if question_answer.title == params[:answer]
end
exist

View File

@@ -151,6 +151,7 @@ class ProposalsController < ApplicationController
def load_featured
return unless !@advanced_search_terms && @search_terms.blank? && @tag_filter.blank? && params[:retired].blank? && @current_order != "recommendations"
if Setting["feature.featured_proposals"]
@featured_proposals = Proposal.not_archived.unsuccessful
.sort_by_confidence_score.limit(Setting["featured_proposals_number"])

View File

@@ -11,6 +11,7 @@ class Tracking::BudgetInvestmentProgressBarsController < Tracking::ProgressBarsC
return if current_user.administrator? ||
Budget::TrackerAssignment.exists?(investment_id: params[:budget_investment_id],
tracker_id: current_user.tracker.id)
raise ActionController::RoutingError.new("Not Found")
end
end

View File

@@ -71,6 +71,7 @@ class Tracking::BudgetInvestmentsController < Tracking::BaseController
return if current_user.administrator? ||
Budget::TrackerAssignment.exists?(investment_id: params[:id],
tracker_id: current_user.tracker.id)
raise ActionController::RoutingError.new("Not Found")
end

View File

@@ -15,6 +15,7 @@ class Users::SessionsController < Devise::SessionsController
def verifying_via_email?
return false if resource.blank?
stored_path = session[stored_location_key_for(resource)] || ""
stored_path[0..5] == "/email"
end

View File

@@ -114,6 +114,7 @@ class Valuation::BudgetInvestmentsController < Valuation::BaseController
return if current_user.administrator? ||
Budget::ValuatorAssignment.exists?(investment_id: params[:id],
valuator_id: current_user.valuator.id)
raise ActionController::RoutingError.new("Not Found")
end

View File

@@ -57,6 +57,7 @@ class Verification::SmsController < ApplicationController
def verified_user
return false unless params[:verified_user]
@verified_user = VerifiedUser.by_user(current_user).where(id: params[:verified_user][:id]).first
end

View File

@@ -1,6 +1,7 @@
module Admin::ProposalDashboardActionsHelper
def active_human_readable(active)
return t("admin.dashboard.actions.index.active") if active
t("admin.dashboard.actions.index.inactive")
end

View File

@@ -1,6 +1,7 @@
module ApplicationHelper
def home_page?
return false if user_signed_in?
# Using path because fullpath yields false negatives since it contains
# parameters too
request.path == "/"
@@ -41,6 +42,7 @@ module ApplicationHelper
def author_of?(authorable, user)
return false if authorable.blank? || user.blank?
authorable.author_id == user.id
end

View File

@@ -7,6 +7,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&.id)
link_to(assigned_heading.name, heading_path)
end

View File

@@ -45,6 +45,7 @@ module BudgetsHelper
def css_for_ballot_heading(heading)
return "" if current_ballot.blank? || @current_filter == "unfeasible"
current_ballot.has_lines_in_heading?(heading) ? "is-active" : ""
end
@@ -74,6 +75,7 @@ module BudgetsHelper
def current_budget_map_locations
return unless current_budget.present?
if current_budget.publishing_prices_or_later? && current_budget.investments.selected.any?
investments = current_budget.investments.selected
else

View File

@@ -34,6 +34,7 @@ module EmbedVideosHelper
return if video_url.blank?
return if video_url.match(VIMEO_REGEX)
return if video_url.match(YOUTUBE_REGEX)
errors.add(:video_url, :invalid)
end
end

View File

@@ -1,6 +1,7 @@
module ImagesHelper
def image_absolute_url(image, version)
return "" unless image
if Paperclip::Attachment.default_options[:storage] == :filesystem
URI(request.url) + image.attachment.url(version)
else

View File

@@ -12,6 +12,7 @@ module OfficingHelper
return nil if params.blank?
return nil if params[:questions].blank?
return nil if params[:questions][question_id.to_s].blank?
params[:questions][question_id.to_s][answer_index.to_s]
end
end

View File

@@ -70,22 +70,26 @@ module ProposalsDashboardHelper
def daily_selected_class
return nil if params[:group_by].blank?
"hollow"
end
def weekly_selected_class
return nil if params[:group_by] == "week"
"hollow"
end
def monthly_selected_class
return nil if params[:group_by] == "month"
"hollow"
end
def resource_card_class(resource, proposal)
return "alert" unless resource.active_for?(proposal)
return "success" if resource.executed_for?(proposal)
"primary"
end
@@ -93,6 +97,7 @@ module ProposalsDashboardHelper
return t("dashboard.resource.resource_locked") unless resource.active_for?(proposal)
return t("dashboard.resource.view_resource") if resource.executed_for?(proposal)
return t("dashboard.resource.resource_requested") if resource.requested_for?(proposal)
t("dashboard.resource.request_resource")
end

View File

@@ -1,11 +1,13 @@
module ShiftsHelper
def shift_vote_collection_dates(booth, polls)
return [] if polls.blank?
date_options((start_date(polls)..end_date(polls)), Poll::Shift.tasks[:vote_collection], booth)
end
def shift_recount_scrutiny_dates(booth, polls)
return [] if polls.blank?
dates = polls.map(&:ends_at).map(&:to_date).sort.inject([]) do |total, date|
initial_date = date < Date.current ? Date.current : date
total << (initial_date..date + Poll::RECOUNT_DURATION).to_a

View File

@@ -1,6 +1,7 @@
module TextWithLinksHelper
def sanitize_and_auto_link(text)
return unless text
sanitized = sanitize(text, tags: [], attributes: [])
auto_link_already_sanitized_html(sanitized)
end

View File

@@ -5,6 +5,7 @@ module VotesHelper
def votes_percentage(vote, debate)
return "0%" if debate.total_votes == 0
if vote == "likes"
debate_percentage_of_likes(debate).to_s + "%"
elsif vote == "dislikes"

View File

@@ -41,6 +41,7 @@ class AdminNotification < ApplicationRecord
def complete_link_url
return unless link.present?
unless self.link[/\Ahttp:\/\//] || self.link[/\Ahttps:\/\//]
self.link = "http://#{self.link}"
end

View File

@@ -68,6 +68,7 @@ class Budget
def heading_for_group(group)
return nil unless has_lines_in_group?(group)
investments.find_by(group: group).heading
end

View File

@@ -24,6 +24,7 @@ class Budget
def check_valid_heading
return if ballot.valid_heading?(heading)
errors.add(:heading, "This heading's budget is invalid, or a heading on the same group was already selected")
end

View File

@@ -279,6 +279,7 @@ class Budget
return :not_logged_in unless user
return :organization if user.organization?
return :not_verified unless user.can?(:vote, Budget::Investment)
nil
end

View File

@@ -166,6 +166,7 @@ class Budget::Stats
def population_percent(population, participants)
return "N/A" unless population.to_f.positive?
calculate_percentage(participants, population)
end

View File

@@ -28,6 +28,7 @@ class CommentNotifier
def email_on_comment_reply?
return false unless @comment.reply?
parent_author = @comment.parent.author
parent_author != @author && parent_author.email_on_comment_reply?
end

View File

@@ -3,6 +3,7 @@ module Conflictable
def conflictive?
return false unless flags_count > 0 && cached_votes_up > 0
cached_votes_up / flags_count.to_f < 5
end
end

View File

@@ -19,6 +19,7 @@ module Filterable
def allowed_filter?(filter, value)
return if value.blank?
["official_level", "date_range"].include?(filter)
end
end

View File

@@ -16,36 +16,43 @@ module Verification
def verification_email_sent?
return true if skip_verification?
email_verification_token.present?
end
def verification_sms_sent?
return true if skip_verification?
unconfirmed_phone.present? && sms_confirmation_code.present?
end
def verification_letter_sent?
return true if skip_verification?
letter_requested_at.present? && letter_verification_code.present?
end
def residence_verified?
return true if skip_verification?
residence_verified_at.present?
end
def sms_verified?
return true if skip_verification?
confirmed_phone.present?
end
def level_two_verified?
return true if skip_verification?
level_two_verified_at.present? || (residence_verified? && sms_verified?)
end
def level_three_verified?
return true if skip_verification?
verified_at.present?
end

View File

@@ -123,6 +123,7 @@ class Debate < ApplicationRecord
def votable_by?(user)
return false unless user
total_votes <= 100 ||
!user.unverified? ||
Setting["max_ratio_anon_votes_on_debates"].to_i == 100 ||
@@ -132,6 +133,7 @@ class Debate < ApplicationRecord
def anonymous_votes_ratio
return 0 if cached_votes_total == 0
(cached_anonymous_votes_total.to_f / cached_votes_total) * 100
end

View File

@@ -12,6 +12,7 @@ class DirectMessage < ApplicationRecord
def max_per_day
return if errors.any?
max = Setting[:direct_message_max_per_day]
return unless max

View File

@@ -12,17 +12,20 @@ class Flag < ApplicationRecord
def self.flag(user, flaggable)
return false if flagged?(user, flaggable)
create!(user: user, flaggable: flaggable)
end
def self.unflag(user, flaggable)
flags = by_user_and_flaggable(user, flaggable)
return false if flags.empty?
flags.destroy_all
end
def self.flagged?(user, flaggable)
return false unless user
!!by_user_and_flaggable(user, flaggable)&.first
end
end

View File

@@ -38,6 +38,7 @@ class I18nContent < ApplicationRecord
def self.flat_hash(input, path = nil, output = {})
return output.update({ path => input }) unless input.is_a? Hash
input.map { |key, value| flat_hash(value, [path, key].compact.join("."), output) }
return output
end

View File

@@ -17,6 +17,7 @@ class Lock < ApplicationRecord
def too_many_tries?
return false unless tries > 0
tries % Lock.max_tries == 0
end

View File

@@ -125,6 +125,7 @@ class Officing::Residence
def valid_year_of_birth?
return true if Setting.force_presence_date_of_birth?
@census_api_response.date_of_birth.year.to_s == year_of_birth.to_s
end

View File

@@ -98,6 +98,7 @@ class Poll < ApplicationRecord
def self.answerable_by(user)
return none if user.nil? || user.unverified?
current.joins('LEFT JOIN "geozones_polls" ON "geozones_polls"."poll_id" = "polls"."id"')
.where("geozone_restricted = ? OR geozones_polls.geozone_id = ?", false, user.geozone_id)
end
@@ -109,6 +110,7 @@ class Poll < ApplicationRecord
def votable_by?(user)
return false if user_has_an_online_ballot?(user)
answerable_by?(user) &&
not_voted_by?(user)
end
@@ -157,6 +159,7 @@ class Poll < ApplicationRecord
return unless starts_at.present?
return unless ends_at.present?
return unless Poll.overlaping_with(self).any?
errors.add(:starts_at, I18n.t("activerecord.errors.messages.another_poll_active"))
end

View File

@@ -63,6 +63,7 @@ class Poll::Question < ApplicationRecord
def self.answerable_by(user)
return none if user.nil? || user.unverified?
where(poll_id: Poll.answerable_by(user).pluck(:id))
end

View File

@@ -11,6 +11,7 @@ class Poll::Question::Answer::Video < ApplicationRecord
return if url.blank?
return if url.match(VIMEO_REGEX)
return if url.match(YOUTUBE_REGEX)
errors.add(:url, :invalid)
end
end

View File

@@ -21,6 +21,7 @@ class Poll::Recount < ApplicationRecord
[:white, :null, :total].each do |amount|
next unless send("#{amount}_amount_changed?") && send("#{amount}_amount_was").present?
self["#{amount}_amount_log"] += ":#{send("#{amount}_amount_was")}"
amounts_changed = true
end

View File

@@ -26,6 +26,7 @@ class ProposalNotification < ApplicationRecord
def minimum_interval
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

View File

@@ -62,6 +62,7 @@ class Signature < ApplicationRecord
def clean_document_number
return if document_number.blank?
self.document_number = document_number.gsub(/[^a-z0-9]+/i, "").upcase
end

View File

@@ -46,6 +46,7 @@ class SignatureSheet < ApplicationRecord
def parse_date_of_birth(required_fields_to_verify)
return required_fields_to_verify[1] if Setting.force_presence_date_of_birth?
nil
end

View File

@@ -180,6 +180,7 @@ class User < ApplicationRecord
def add_official_position!(position, level)
return if position.blank? || level.blank?
update! official_position: position, official_level: level.to_i
end
@@ -194,6 +195,7 @@ class User < ApplicationRecord
def display_official_position_badge?
return true if official_level > 1
official_position_badge? && official_level == 1
end
@@ -246,6 +248,7 @@ class User < ApplicationRecord
def take_votes_from(other_user)
return if other_user.blank?
Poll::Voter.where(user_id: other_user.id).update_all(user_id: id)
Budget::Ballot.where(user_id: other_user.id).update_all(user_id: id)
Vote.where("voter_id = ? AND voter_type = ?", other_user.id, "User").update_all(voter_id: id)
@@ -276,6 +279,7 @@ class User < ApplicationRecord
def password_required?
return false if skip_password_validation
super
end
@@ -373,6 +377,7 @@ class User < ApplicationRecord
def clean_document_number
return unless document_number.present?
self.document_number = document_number.gsub(/[^a-z0-9]+/i, "").upcase
end

View File

@@ -32,6 +32,7 @@ class Verification::Letter
def validate_correct_code
return if errors.include?(:verification_code)
if user&.letter_verification_code.to_i != verification_code.to_i
errors.add(:verification_code, I18n.t("verification.letter.errors.incorrect_code"))
end

View File

@@ -43,6 +43,7 @@ class Verification::Management::Email
def validate_user
return if errors.count > 0
if !user?
errors.add(:email, I18n.t("errors.messages.user_not_found"))
elsif user.level_three_verified?
@@ -52,6 +53,7 @@ class Verification::Management::Email
def validate_document_number
return if errors.count > 0
if document_number_mismatch?
errors.add(:email,
I18n.t("management.email_verifications.document_mismatch",

View File

@@ -43,6 +43,7 @@ class Verification::Residence
def allowed_age
return if errors[:date_of_birth].any? || Age.in_years(date_of_birth) >= User.minimum_required_age
errors.add(:date_of_birth, I18n.t("verification.residence.new.error_not_allowed_age"))
end

View File

@@ -13,6 +13,7 @@ class Verification::Sms
def save
return false unless valid?
update_user_phone_information
send_sms
Lock.increase_tries(user)

View File

@@ -10,6 +10,7 @@ module I18n
key = pluralization_key(entry, count)
return "#{count}" unless entry.has_key?(key)
entry[key]
end
end

View File

@@ -145,6 +145,7 @@ section "Creating Poll Voters" do
def randomly_answer_questions(poll, user)
poll.questions.each do |question|
next unless [true, false].sample
Poll::Answer.create!(question_id: question.id,
author: user,
answer: question.question_answers.sample.title)

View File

@@ -5,6 +5,7 @@ module ActiveModel::Dates
attrs["#{field}(3i)"]
return nil unless day.present? && month.present? && year.present?
Date.new(year.to_i, month.to_i, day.to_i)
end

View File

@@ -4,6 +4,7 @@ module ActsAsParanoidAliases
class_eval do
def hide
return false if hidden?
update_attribute(:hidden_at, Time.current)
after_hide
end
@@ -25,6 +26,7 @@ module ActsAsParanoidAliases
def restore(opts = {})
return false unless hidden?
super(opts)
update_attribute(:confirmed_hide_at, nil) if self.class.column_names.include? "confirmed_hide_at"
after_restore
@@ -54,11 +56,13 @@ module ActsAsParanoidAliases
def hide_all(ids)
return if ids.blank?
where(id: ids).each(&:hide)
end
def restore_all(ids)
return if ids.blank?
only_hidden.where(id: ids).each(&:restore)
end
end

View File

@@ -1,6 +1,7 @@
module Age
def self.in_years(dob, now = Date.current)
return nil if dob.blank?
# reference: http://stackoverflow.com/questions/819263/get-persons-age-in-ruby#comment21200772_819263
now.year - dob.year - (now.month > dob.month || (now.month == dob.month && now.day >= dob.day) ? 0 : 1)
end

View File

@@ -22,6 +22,7 @@ class CensusApi
str = data[:datos_habitante][:item][:fecha_nacimiento_string]
day, month, year = str.match(/(\d\d?)\D(\d\d?)\D(\d\d\d?\d?)/)[1..3]
return nil unless day.present? && month.present? && year.present?
Time.zone.local(year.to_i, month.to_i, day.to_i).to_date
end

View File

@@ -25,6 +25,7 @@ class EvaluationCommentEmail
def related_users
return [] if comment.commentable.nil?
comment.commentable
.admin_and_valuator_users_associated
.reject { |associated_user| associated_user.user == comment.author }

View File

@@ -6,6 +6,7 @@ class ManagerAuthenticator
def auth
return false unless [@manager[:login], @manager[:user_key], @manager[:date]].all? { |manager| manager.present? }
return @manager if manager_exists? && application_authorized?
false
end

View File

@@ -17,6 +17,7 @@ class RemoteCensusApi
def extract_value(path_value)
path = parse_response_path(path_value)
return nil unless path.present?
@body.dig(*path)
end
@@ -29,8 +30,10 @@ class RemoteCensusApi
path_value = Setting["remote_census.response.date_of_birth"]
str = extract_value(path_value)
return nil unless str.present?
day, month, year = str.match(/(\d\d?)\D(\d\d?)\D(\d\d\d?\d?)/)[1..3]
return nil unless day.present? && month.present? && year.present?
Time.zone.local(year.to_i, month.to_i, day.to_i).to_date
end
@@ -111,6 +114,7 @@ class RemoteCensusApi
to_set = path.empty? ? structure : structure.dig(*path)
return unless to_set
to_set[final_key] = value
end

View File

@@ -8,6 +8,7 @@ class SMSApi
def url
return "" unless end_point_available?
open(Rails.application.secrets.sms_end_point).base_uri.to_s
end

View File

@@ -184,8 +184,9 @@ describe "Admin budgets" do
within "#budget-phases-table" do
Budget::Phase::PHASE_KINDS.each do |phase_kind|
phase_index = Budget::Phase::PHASE_KINDS.index(phase_kind)
break if phase_kind == Budget::Phase::PHASE_KINDS.last
phase_index = Budget::Phase::PHASE_KINDS.index(phase_kind)
next_phase_kind = Budget::Phase::PHASE_KINDS[phase_index + 1]
next_phase_name = translated_phase_name(phase_kind: next_phase_kind)
expect(translated_phase_name(phase_kind: phase_kind)).to appear_before(next_phase_name)

View File

@@ -38,6 +38,7 @@ RSpec.configure do |config|
uncommitted transaction data setup over the spec's database connection.
MSG
end
DatabaseCleaner.clean_with(:truncation)
end