From 8b069d33fcb9025e4ce6925a0a11a7119a174393 Mon Sep 17 00:00:00 2001 From: Laust Rud Jacobsen Date: Sun, 15 Nov 2015 18:40:14 +0100 Subject: [PATCH] Rubocop: return early instead of wrapping in guard clause --- .rubocop.yml | 3 +++ .rubocop_todo.yml | 5 ----- app/controllers/application_controller.rb | 8 ++++---- app/controllers/apps_controller.rb | 65 +++++++++++++++++++++++++++++++++++++---------------------------- app/models/app.rb | 21 +++++++++++---------- lib/overrides/hoptoad_notifier/hoptoad_notifier.rb | 20 ++++++++++---------- 6 files changed, 65 insertions(+), 57 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 50f23fe..b8a0a72 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -23,6 +23,9 @@ Style/IndentationConsistency: # modifiers. EnforcedStyle: rails +Style/GuardClause: + MinBodyLength: 10 + Style/AccessModifierIndentation: EnforcedStyle: outdent diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5181c35..b964f9a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -75,11 +75,6 @@ Style/EachWithObject: - 'app/models/notice_fingerprinter.rb' - 'lib/recurse.rb' -# Offense count: 27 -# Configuration parameters: MinBodyLength. -Style/GuardClause: - Enabled: false - # Offense count: 873 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0f5af89..0417a01 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -23,10 +23,10 @@ protected # Check if the current_user is admin or not and redirect to root url if not # def require_admin! - unless user_signed_in? && current_user.admin? - flash[:error] = "Sorry, you don't have permission to do that" - redirect_to_root - end + return if user_signed_in? && current_user.admin? + + flash[:error] = "Sorry, you don't have permission to do that" + redirect_to_root end def redirect_to_root diff --git a/app/controllers/apps_controller.rb b/app/controllers/apps_controller.rb index bf93fd9..0013ea5 100644 --- a/app/controllers/apps_controller.rb +++ b/app/controllers/apps_controller.rb @@ -93,13 +93,16 @@ class AppsController < ApplicationController protected def initialize_subclassed_notification_service + notification_type = params[:app]. + fetch(:notification_service_attributes, {}). + fetch(:type, nil) + return if notification_type.blank? + # set the app's notification service - if params[:app][:notification_service_attributes] && (notification_type = params[:app][:notification_service_attributes][:type]) - available_notification_classes = [NotificationService] + NotificationService.subclasses - notification_class = available_notification_classes.detect { |c| c.name == notification_type } - if notification_class.present? - app.notification_service = notification_class.new(params[:app][:notification_service_attributes]) - end + available_notification_classes = [NotificationService] + NotificationService.subclasses + notification_class = available_notification_classes.detect { |c| c.name == notification_type } + if notification_class.present? + app.notification_service = notification_class.new(params[:app][:notification_service_attributes]) end end @@ -112,32 +115,38 @@ protected # email_at_notices is edited as a string, and stored as an array. def parse_email_at_notices_or_set_default - if params[:app] && (val = params[:app][:email_at_notices]) - # Sanitize negative values, split on comma, - # strip, parse as integer, remove all '0's. - # If empty, set as default and show an error message. - email_at_notices = val.gsub(/-\d+/, "").split(",").map { |v| v.strip.to_i }.reject { |v| v == 0 } - if email_at_notices.any? - params[:app][:email_at_notices] = email_at_notices - else - default_array = params[:app][:email_at_notices] = Errbit::Config.email_at_notices - flash[:error] = "Couldn't parse your notification frequency. Value was reset to default (#{default_array.join(', ')})." - end + return if params[:app].blank? + + val = params[:app][:email_at_notices] + return if val.blank? + + # Sanitize negative values, split on comma, + # strip, parse as integer, remove all '0's. + # If empty, set as default and show an error message. + email_at_notices = val.gsub(/-\d+/, "").split(",").map { |v| v.strip.to_i }.reject { |v| v == 0 } + if email_at_notices.any? + params[:app][:email_at_notices] = email_at_notices + else + default_array = params[:app][:email_at_notices] = Errbit::Config.email_at_notices + flash[:error] = "Couldn't parse your notification frequency. Value was reset to default (#{default_array.join(', ')})." end end def parse_notice_at_notices_or_set_default - if params[:app][:notification_service_attributes] && (val = params[:app][:notification_service_attributes][:notify_at_notices]) - # Sanitize negative values, split on comma, - # strip, parse as integer, remove all '0's. - # If empty, set as default and show an error message. - notify_at_notices = val.gsub(/-\d+/, "").split(",").map { |v| v.strip.to_i } - if notify_at_notices.any? - params[:app][:notification_service_attributes][:notify_at_notices] = notify_at_notices - else - default_array = params[:app][:notification_service_attributes][:notify_at_notices] = Errbit::Config.notify_at_notices - flash[:error] = "Couldn't parse your notification frequency. Value was reset to default (#{default_array.join(', ')})." - end + return if params[:app][:notification_service_attributes].blank? + + val = params[:app][:notification_service_attributes][:notify_at_notices] + return if val.blank? + + # Sanitize negative values, split on comma, + # strip, parse as integer, remove all '0's. + # If empty, set as default and show an error message. + notify_at_notices = val.gsub(/-\d+/, "").split(",").map { |v| v.strip.to_i } + if notify_at_notices.any? + params[:app][:notification_service_attributes][:notify_at_notices] = notify_at_notices + else + default_array = params[:app][:notification_service_attributes][:notify_at_notices] = Errbit::Config.notify_at_notices + flash[:error] = "Couldn't parse your notification frequency. Value was reset to default (#{default_array.join(', ')})." end end diff --git a/app/models/app.rb b/app/models/app.rb index 96f62bc..ad1198a 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -152,16 +152,17 @@ class App # Copy app attributes from another app. def copy_attributes_from(app_id) - if (copy_app = App.where(_id: app_id).first) - # Copy fields - (copy_app.fields.keys - %w(_id name created_at updated_at)).each do |k| - send("#{k}=", copy_app.send(k)) - end - # Clone the embedded objects that can be changed via apps/edit (ignore errs & deploys, etc.) - %w(watchers issue_tracker notification_service).each do |relation| - if (obj = copy_app.send(relation)) - send("#{relation}=", obj.is_a?(Array) ? obj.map(&:clone) : obj.clone) - end + copy_app = App.where(_id: app_id).first + return if copy_app.blank? + + # Copy fields + (copy_app.fields.keys - %w(_id name created_at updated_at)).each do |k| + send("#{k}=", copy_app.send(k)) + end + # Clone the embedded objects that can be changed via apps/edit (ignore errs & deploys, etc.) + %w(watchers issue_tracker notification_service).each do |relation| + if (obj = copy_app.send(relation)) + send("#{relation}=", obj.is_a?(Array) ? obj.map(&:clone) : obj.clone) end end end diff --git a/lib/overrides/hoptoad_notifier/hoptoad_notifier.rb b/lib/overrides/hoptoad_notifier/hoptoad_notifier.rb index 594db63..839d767 100644 --- a/lib/overrides/hoptoad_notifier/hoptoad_notifier.rb +++ b/lib/overrides/hoptoad_notifier/hoptoad_notifier.rb @@ -6,18 +6,18 @@ HoptoadNotifier.module_eval do class << self private def send_notice(notice) # Log the error internally if we are not in a development environment. - if configuration.public? - app = App.find_or_initialize_by(name: "Self.Errbit") - app.github_repo = "errbit/errbit" - app.save! - notice.send("api_key=", app.api_key) + return unless configuration.public? - # Create notice internally. - report = ErrorReport.new(notice.to_xml) - report.generate_notice! + app = App.find_or_initialize_by(name: "Self.Errbit") + app.github_repo = "errbit/errbit" + app.save! + notice.send("api_key=", app.api_key) - logger.info "Internal error was logged to 'Self.Errbit' app." - end + # Create notice internally. + report = ErrorReport.new(notice.to_xml) + report.generate_notice! + + logger.info "Internal error was logged to 'Self.Errbit' app." end end end -- libgit2 0.21.2