switched Metrics/LineLength to Layout/LineLength. Updated a few ruboc…

…op rules. Added controller specific rules
DMPRoadmap · Jul 10, 2020 · 4a36402 · 4a36402
1 parent 2c0368f
commit 4a36402
Show file tree

Hide file tree

Showing 24 changed files with 95 additions and 84 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -67,10 +67,10 @@ Lint/AmbiguousBlockAssociation:
 # Restrict the length of each line of code to 90 characters. Enforcing this is important
 # as many developers are working on smaller screens, or split screens. Having to scroll
 # to read a full line of code makes code harder to read and more frustrating to work with.
-Metrics/LineLength:
+Layout/LineLength:
   # I've found that 90 is a suitable limit. Many developers balk at the 80 character
   # default.
-  Max: 90
+  Max: 100
   Exclude:
     # Excludes the spec helper, since this boilerplate file has comments that are much
     # longer.
@@ -83,6 +83,17 @@ Metrics/BlockLength:
   Exclude:
     - 'spec/**/*'
 
+# Leave the string formatting style as `"some text %{value}" % { value: "text" }`
+# since we're uncertain what effect `format` and `sprintf` may have on the Fastgetext
+# markup `_("text")`
+Style/FormatString:
+  EnforcedStyle: percent
+
+# Prefer the use of `"some %{token} text"` instead of `some %<token> text` or
+# `some %token text` since it would invalidate many of our translation strings
+Style/FormatStringToken:
+  EnforcedStyle: template
+
 # This cop enforces the use of boolean and/or "&&" and "||" over "and" "or".
 # Sometimes using "and"/"or" is preferrable, when these are used as control flow.
 #

diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb
@@ -18,24 +18,24 @@ def create_or_update
     begin
       p = Plan.find(p_params[:plan_id])
       if !p.question_exists?(p_params[:question_id])
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         render(status: :not_found, json: {
           msg: _("There is no question with id %{question_id} associated to plan id %{plan_id} for which to create or update an answer") % {
             question_id: p_params[:question_id],
             plan_id: p_params[:plan_id]
           }
         })
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
         return
       end
     rescue ActiveRecord::RecordNotFound
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       render(status: :not_found, json: {
         msg: _("There is no plan with id %{id} for which to create or update an answer") % {
           id: p_params[:plan_id]
         }
       })
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
       return
     end
     q = Question.find(p_params[:question_id])
@@ -124,7 +124,7 @@ def create_or_update
       end
 
       send_webhooks(current_user, @answer)
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       render json: {
         "qn_data": qn_data,
         "section_data": section_data,
@@ -158,7 +158,7 @@ def create_or_update
           }, formats: [:html])
         }
       }.to_json
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     end
   end
 

diff --git a/app/controllers/guidance_groups_controller.rb b/app/controllers/guidance_groups_controller.rb
@@ -58,9 +58,9 @@ def admin_update_publish
     authorize @guidance_group
 
     if @guidance_group.update(published: true)
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       flash[:notice] = _("Your guidance group has been published and is now available to users.")
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     else
       flash[:alert] = failure_message(@guidance_group, _("publish"))
     end
@@ -73,9 +73,9 @@ def admin_update_unpublish
     authorize @guidance_group
 
     if @guidance_group.update(published: false)
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       flash[:notice] = _("Your guidance group is no longer published and will not be available to users.")
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     else
       flash[:alert] = failure_message(@guidance_group, _("unpublish"))
     end

diff --git a/app/controllers/guidances_controller.rb b/app/controllers/guidances_controller.rb
@@ -102,9 +102,9 @@ def admin_publish
       if !guidance_group.published? || guidance_group.published.nil?
         guidance_group.update(published: true)
       end
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       flash[:notice] = _("Your guidance has been published and is now available to users.")
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     else
       flash[:alert] = failure_message(@guidance, _("publish"))
     end
@@ -120,9 +120,9 @@ def admin_unpublish
       unless guidance_group.guidances.where(published: true).exists?
         guidance_group.update(published: false)
       end
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       flash[:notice] = _("Your guidance is no longer published and will not be available to users.")
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     else
       flash[:alert] = failure_message(@guidance, _("unpublish"))
     end

diff --git a/app/controllers/org_admin/phases_controller.rb b/app/controllers/org_admin/phases_controller.rb
@@ -13,9 +13,9 @@ def show
       phase = Phase.includes(:template, :sections).order(:number).find(params[:id])
       authorize phase
       if !phase.template.latest?
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         flash[:notice] = _("You are viewing a historical version of this template. You will not be able to make changes.")
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
       end
       if phase.template.customization_of? && phase.template.latest?
         # The user is working with the latest version so only use the modifiable sections

diff --git a/app/controllers/org_admin/plans_controller.rb b/app/controllers/org_admin/plans_controller.rb
@@ -33,13 +33,13 @@ def feedback_complete
     end
 
     if plan.complete_feedback(current_user)
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       redirect_to(org_admin_plans_path,
         notice: _("%{plan_owner} has been notified that you have finished providing feedback") % {
           plan_owner: plan.owner.name(false)
         }
       )
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     else
       redirect_to org_admin_plans_path,
         alert: _("Unable to notify user that you have finished providing feedback.")

diff --git a/app/controllers/org_admin/templates_controller.rb b/app/controllers/org_admin/templates_controller.rb
@@ -105,9 +105,9 @@ def show
                        .select("phases.title", "phases.description", "sections.title",
                                "questions.text", "question_options.text")
       if !template.latest?
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         flash[:notice] = _("You are viewing a historical version of this template. You will not be able to make changes.")
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
       end
       render "container", locals: {
         partial_path: "show",
@@ -227,9 +227,9 @@ def destroy
           end
         end
       else
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         flash[:alert] = _("You cannot delete a #{template_type(template)} that has been used to create plans.")
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
       end
       if request.referrer.present?
         redirect_to request.referrer
@@ -260,7 +260,7 @@ def history
     def publish
       template = Template.find(params[:id])
       authorize template
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       publishable, errors = template.publishability
       if publishable
         if template.publish!
@@ -271,7 +271,7 @@ def publish
       else
         flash[:alert] = errors
       end
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
       redirect_to request.referrer.present? ? request.referrer : org_admin_templates_path
     end
 
@@ -321,7 +321,7 @@ def template_export
           end
 
           format.pdf do
-            # rubocop:disable Metrics/LineLength
+            # rubocop:disable Layout/LineLength
             render pdf: file_name,
               template: "template_exports/template_export",
               margin: @formatting[:margin],
@@ -335,7 +335,7 @@ def template_export
               right: "[page] of [topage]",
               encoding: "utf8"
             }
-            # rubocop:enable Metrics/LineLength
+            # rubocop:enable Layout/LineLength
           end
         end
       rescue ActiveRecord::RecordInvalid => e

diff --git a/app/controllers/paginable/templates_controller.rb b/app/controllers/paginable/templates_controller.rb
@@ -6,7 +6,7 @@ class Paginable::TemplatesController < ApplicationController
   include Paginable
 
   # TODO: Clean up this code for Rubocop
-  # rubocop:disable Metrics/LineLength
+  # rubocop:disable Layout/LineLength
 
   # GET /paginable/templates/:page  (AJAX)
   # -----------------------------------------------------
@@ -78,7 +78,7 @@ def customisable
     )
   end
 
-  # rubocop:enable Metrics/LineLength
+  # rubocop:enable Layout/LineLength
 
   # GET /paginable/templates/publicly_visible/:page  (AJAX)
   # -----------------------------------------------------

diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb
@@ -123,14 +123,14 @@ def create
 
         elsif !@plan.template.customization_of.nil?
           # We used a customized version of the the funder template
-          # rubocop:disable Metrics/LineLength
+          # rubocop:disable Layout/LineLength
           msg += " #{_('This plan is based on the')} #{@plan.funder&.name}: '#{@plan.template.title}' #{_('template with customisations by the')} #{plan_params[:org_name]}"
-          # rubocop:enable Metrics/LineLength
+          # rubocop:enable Layout/LineLength
         else
           # We used the specified org's or funder's template
-          # rubocop:disable Metrics/LineLength
+          # rubocop:disable Layout/LineLength
           msg += " #{_('This plan is based on the')} #{@plan.template.org.name}: '#{@plan.template.title}' template."
-          # rubocop:enable Metrics/LineLength
+          # rubocop:enable Layout/LineLength
         end
 
         @plan.add_user!(current_user.id, :creator)
@@ -385,13 +385,13 @@ def visibility
                  json: { msg: failure_message(plan, _("update")) }
         end
       else
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         render status: :forbidden, json: {
           msg: _("Unable to change the plan's status since it is needed at least %{percentage} percentage responded") % {
               percentage: Rails.configuration.x.plans.default_percentage_answered
           }
         }
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
       end
     else
       render status: :not_found,
@@ -407,7 +407,7 @@ def set_test
     plan = Plan.find(params[:id])
     authorize plan
     plan.visibility = (params[:is_test] === "1" ? :is_test : :privately_visible)
-    # rubocop:disable Metrics/LineLength
+    # rubocop:disable Layout/LineLength
     if plan.save
       render json: {
         code: 1,
@@ -418,7 +418,7 @@ def set_test
         code: 0, msg: _("Unable to change the plan's test status")
       }
     end
-    # rubocop:enable Metrics/LineLength
+    # rubocop:enable Layout/LineLength
   end
 
   # GET /plans/:id/overview

diff --git a/app/controllers/public_pages_controller.rb b/app/controllers/public_pages_controller.rb
@@ -47,7 +47,7 @@ def template_export
         end
 
         format.pdf do
-          # rubocop:disable Metrics/LineLength
+          # rubocop:disable Layout/LineLength
           render pdf: file_name,
             template: "template_exports/template_export",
             margin: @formatting[:margin],
@@ -61,7 +61,7 @@ def template_export
             right: "[page] of [topage]",
             encoding: "utf8"
           }
-          # rubocop:enable Metrics/LineLength
+          # rubocop:enable Layout/LineLength
         end
       end
     rescue ActiveRecord::RecordInvalid => e

diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
@@ -33,7 +33,7 @@ def new
       # The OAuth provider could not be determined or there was no unique UID!
       if !oauth["provider"].nil? && !oauth["uid"].nil?
         # Connect the new user with the identifier sent back by the OAuth provider
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         flash[:notice] = _("Please make a choice below. After linking your details to a %{application_name} account, you will be able to sign in directly with your institutional credentials.") % {
           application_name: ApplicationService.application_name
         }
@@ -99,9 +99,9 @@ def create
                                   value: oauth["uid"],
                                   attrs: oauth,
                                   identifiable: resource)
-                # rubocop:disable Metrics/LineLength
+                # rubocop:disable Layout/LineLength
                 flash[:notice] = _("Welcome! You have signed up successfully with your institutional credentials. You will now be able to access your account with them.")
-                # rubocop:enable Metrics/LineLength
+                # rubocop:enable Layout/LineLength
               end
             end
           end
@@ -114,10 +114,10 @@ def create
         end
       else
         clean_up_passwords resource
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         redirect_to after_sign_up_error_path_for(resource),
                     alert: _("Unable to create your account.#{errors_for_display(resource)}")
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
       end
     end
   end
@@ -168,9 +168,9 @@ def do_update(require_password = true, confirm = false)
       mandatory_params &&= false
     end
     if update_params[:org_id].blank?
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       message += _("Please select an organisation from the list, or enter your organisation's name.")
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
       mandatory_params &&= false
     end
     # has the user entered all the details

diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb
@@ -18,11 +18,11 @@ def create
     message = ""
     if role_params[:user].present? && plan.present?
       if @role.plan.owner.present? && @role.plan.owner.email == role_params[:user][:email]
-        # rubocop:disable Metrics/LineLength
+        # rubocop:disable Layout/LineLength
         flash[:notice] = _("Cannot share plan with %{email} since that email matches with the owner of the plan.") % {
           email: role_params[:user][:email]
         }
-        # rubocop:enable Metrics/LineLength
+        # rubocop:enable Layout/LineLength
       else
         user = User.where_case_insensitive("email", role_params[:user][:email]).first
         if Role.find_by(plan: @role.plan, user: user) # role already exists
@@ -56,9 +56,9 @@ def create
             end
             flash[:notice] = message
           else
-            # rubocop:disable Metrics/LineLength
+            # rubocop:disable Layout/LineLength
             flash[:alert] = _("You must provide a valid email address and select a permission level.")
-            # rubocop:enable Metrics/LineLength
+            # rubocop:enable Layout/LineLength
           end
         end
       end
@@ -77,12 +77,12 @@ def update
       deliver_if(recipients: @role.user, key: "users.added_as_coowner") do |r|
         UserMailer.permissions_change_notification(@role, current_user).deliver_now
       end
-      # rubocop:disable Metrics/LineLength
+      # rubocop:disable Layout/LineLength
       render json: {
         code: 1,
         msg: _("Successfully changed the permissions for %{email}. They have been notified via email.") % { email: @role.user.email }
       }
-      # rubocop:enable Metrics/LineLength
+      # rubocop:enable Layout/LineLength
     else
       render json: { code: 0, msg: flash[:alert] }
     end