Rubocop: routine fixes (#9345)

* rubocop -A

* Fix Style/HashLikeCase

* regenerated todo file

.rubocop.yml

@@ -142,6 +142,10 @@ Lint/DeprecatedOpenSSLConstant:
   Description: "Don't use algorithm constants for `OpenSSL::Cipher` and `OpenSSL::Digest`."
   Enabled: true
 
+Lint/DuplicateElsifCondition:
+  Description: 'Do not repeat conditions used in if `elsif`.'
+  Enabled: true
+
 # Lint/HeredocMethodCallPosition:
 #   Description: >-
 #                  Checks for the ordering of a method call where
@@ -240,6 +244,13 @@ Style/AccessorGrouping:
   Description: 'Checks for grouping of accessors in `class` and `module` bodies.'
   Enabled: true
 
+Style/ArrayCoercion:
+  Description: >-
+                  Use Array() instead of explicit Array check or [*var], when dealing
+                  with a variable you want to treat as an Array, but you're not certain it's an array.
+  StyleGuide: '#array-coercion'
+  Enabled: true
+
 # Style/AutoResourceCleanup:
 #   Description: 'Suggests the usage of an auto resource cleanup version of a method (if available).'
 #   Enabled: false
@@ -250,6 +261,11 @@ Style/BisectedAttrAccessor:
                 for the same method can be combined into single `attr_accessor`.
   Enabled: true
 
+Style/CaseLikeIf:
+  Description: 'This cop identifies places where `if-elsif` constructions can be replaced with `case-when`.'
+  StyleGuide: '#case-vs-if-else'
+  Enabled: true
+
 Style/CollectionMethods:
   Description: 'Preferred collection methods.'
   StyleGuide: '#map-find-select-reduce-include-size'
@@ -289,6 +305,25 @@ Style/FrozenStringLiteralComment:
   Enabled: true
   EnforcedStyle: never
 
+Style/HashAsLastArrayItem:
+  Description: >-
+                 Checks for presence or absence of braces around hash literal as a last
+                 array item depending on configuration.
+  StyleGuide: '#hash-literal-as-last-array-item'
+  Enabled: true
+  EnforcedStyle: braces
+
+Style/HashEachMethods:
+  Description: 'Use Hash#each_key and Hash#each_value.'
+  StyleGuide: '#hash-each'
+  Enabled: true
+
+Style/HashLikeCase:
+  Description: >-
+                  Checks for places where `case-when` represents a simple 1:1
+                  mapping and can be replaced with a hash lookup.
+  Enabled: true
+
 Style/HashSyntax:
   Description: >-
                  Prefer Ruby 1.9 hash syntax { a: 1, b: 2 } over 1.8 syntax
@@ -297,11 +332,6 @@ Style/HashSyntax:
   Enabled: true
   EnforcedStyle: ruby19_no_mixed_keys
 
-Style/HashEachMethods:
-  Description: 'Use Hash#each_key and Hash#each_value.'
-  StyleGuide: '#hash-each'
-  Enabled: true
-
 Style/HashTransformKeys:
   Description: 'Prefer `transform_keys` over `each_with_object` and `map`.'
   StyleGuide: '#hash-transform-methods'
@@ -369,6 +399,13 @@ Style/RedundantFetchBlock:
   Reference: 'https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code'
   Enabled: true
 
+Style/RedundantFileExtensionInRequire:
+  Description: >-
+                  Checks for the presence of superfluous `.rb` extension in
+                  the filename provided to `require` and `require_relative`.
+  StyleGuide: '#no-explicit-rb-to-require'
+  Enabled: true
+
 Style/RedundantRegexpCharacterClass:
   Description: 'Checks for unnecessary single-element Regexp character classes.'
   Enabled: true

.rubocop_todo.yml

@@ -6,7 +6,7 @@ require:
 
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2020-07-14 10:49:48 UTC using RuboCop version 0.87.1.
+# on 2020-07-16 09:06:37 UTC using RuboCop version 0.88.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new

app/controllers/api/v0/articles_controller.rb

@@ -109,7 +109,7 @@ module Api
       def article_params
         allowed_params = [
           :title, :body_markdown, :published, :series,
-          :main_image, :canonical_url, :description, tags: []
+          :main_image, :canonical_url, :description, { tags: [] }
         ]
         allowed_params << :organization_id if params.dig("article", "organization_id") && allowed_to_change_org_id?
         params.require(:article).permit(allowed_params)

app/controllers/chat_channels_controller.rb

@@ -7,16 +7,17 @@ class ChatChannelsController < ApplicationController
   private_constant :CHANNEL_ATTRIBUTES_FOR_SERIALIZATION
 
   def index
-    if params[:state] == "unopened"
+    case params[:state]
+    when "unopened"
       authorize ChatChannel
       render_unopened_json_response
-    elsif params[:state] == "unopened_ids"
+    when "unopened_ids"
       authorize ChatChannel
       render_unopened_ids_response
-    elsif params[:state] == "pending"
+    when "pending"
       authorize ChatChannel
       render_pending_json_response
-    elsif params[:state] == "joining_request"
+    when "joining_request"
       authorize ChatChannel
       render_joining_request_json_response
     else

app/controllers/follows_controller.rb

@@ -53,11 +53,12 @@ class FollowsController < ApplicationController
   def create
     authorize Follow
 
-    followable = if params[:followable_type] == "Organization"
+    followable = case params[:followable_type]
+                 when "Organization"
                    Organization.find(params[:followable_id])
-                 elsif params[:followable_type] == "Tag"
+                 when "Tag"
                    Tag.find(params[:followable_id])
-                 elsif params[:followable_type] == "Podcast"
+                 when "Podcast"
                    Podcast.find(params[:followable_id])
                  else
                    User.find(params[:followable_id])

app/controllers/internal/tags_controller.rb

@@ -7,9 +7,10 @@ module Internal
     end
 
     def index
-      @tags = if params[:state] == "supported"
+      @tags = case params[:state]
+              when "supported"
                 Tag.where(supported: true).order("taggings_count DESC").page(params[:page]).per(50)
-              elsif params[:state] == "unsupported"
+              when "unsupported"
                 Tag.where(supported: false).order("taggings_count DESC").page(params[:page]).per(50)
               else
                 Tag.order("taggings_count DESC").page(params[:page]).per(50)

app/controllers/users_controller.rb

@@ -13,9 +13,10 @@ class UsersController < ApplicationController
 
   def index
     @users =
-      if params[:state] == "follow_suggestions"
+      case params[:state]
+      when "follow_suggestions"
         determine_follow_suggestions(current_user)
-      elsif params[:state] == "sidebar_suggestions"
+      when "sidebar_suggestions"
         Suggester::Users::Sidebar.new(current_user, params[:tag]).suggest.sample(3)
       else
         User.none

app/labor/follow_checker.rb

@@ -12,11 +12,12 @@ class FollowChecker
 
     cache_key = "user-#{follower.id}-#{follower.updated_at.rfc3339}/is_following_#{followable_type}_#{followable_id}"
     Rails.cache.fetch(cache_key, expires_in: 20.hours) do
-      followable = if followable_type == "Tag"
+      followable = case followable_type
+                   when "Tag"
                      Tag.find(followable_id)
-                   elsif followable_type == "Organization"
+                   when "Organization"
                      Organization.find(followable_id)
-                   elsif followable_type == "Podcast"
+                   when "Podcast"
                      Podcast.find(followable_id)
                    else
                      User.find(followable_id)

app/labor/loggers/log_worker_queue_stats.rb

@@ -15,7 +15,7 @@ module Loggers
       end
 
       def record_queue_stats(queues)
-        queue_hash = queues.map { |queue| [queue.name, size: queue.size, latency: queue.latency] }.to_h
+        queue_hash = queues.map { |queue| [queue.name, { size: queue.size, latency: queue.latency }] }.to_h
         queue_hash.each do |queue_name, queue_values|
           latency = queue_values.fetch(:latency, 0)
           size = queue_values.fetch(:size, 0)

app/liquid_tags/github_tag.rb

@@ -14,9 +14,10 @@ class GithubTag < LiquidTagBase
   end
 
   def pre_render
-    if issue_or_readme == "issue"
+    case issue_or_readme
+    when "issue"
       GithubTag::GithubIssueTag.new(@link).render
-    elsif issue_or_readme == "readme"
+    when "readme"
       gt = GithubTag::GithubReadmeTag.new(@link)
       gt.render
     end

app/liquid_tags/glitch_tag.rb

@@ -4,6 +4,13 @@ class GlitchTag < LiquidTagBase
   PARTIAL = "liquids/glitch".freeze
   ID_REGEXP = /\A[a-zA-Z0-9\-]{1,110}\z/.freeze
   OPTION_REGEXP = /(app|code|no-files|preview-first|no-attribution|file=\w(\.\w)?)/.freeze
+  OPTIONS_TO_QUERY_PAIR = {
+    "app" => %w[previewSize 100],
+    "code" => %w[previewSize 0],
+    "no-files" => %w[sidebarCollapsed true],
+    "preview-first" => %w[previewFirst true],
+    "no-attribution" => %w[attributionHidden true]
+  }.freeze
 
   def initialize(_tag_name, id, _parse_context)
     super
@@ -38,24 +45,9 @@ class GlitchTag < LiquidTagBase
     option.match(OPTION_REGEXP)
   end
 
-  def option_to_query_pair(option)
-    case option
-    when "app"
-      %w[previewSize 100]
-    when "code"
-      %w[previewSize 0]
-    when "no-files"
-      %w[sidebarCollapsed true]
-    when "preview-first"
-      %w[previewFirst true]
-    when "no-attribution"
-      %w[attributionHidden true]
-    end
-  end
-
   def build_options(options)
     # Convert options to query param pairs
-    params = options.map { |option| option_to_query_pair(option) }.compact
+    params = options.map { |option| OPTIONS_TO_QUERY_PAIR[option] }.compact
 
     # Deal with the file option if present or use default
     file_option = options.detect { |option| option.start_with?("file=") }

app/models/follow.rb

@@ -2,6 +2,18 @@ class Follow < ApplicationRecord
   extend ActsAsFollower::FollowerLib
   extend ActsAsFollower::FollowScopes
 
+  COUNTER_CULTURE_COLUMN_NAME_BY_TYPE = {
+    "User" => "following_users_count",
+    "Organization" => "following_orgs_count",
+    "ActsAsTaggableOn::Tag" => "following_tags_count"
+  }.freeze
+
+  COUNTER_CULTURE_COLUMNS_NAMES = {
+    ["follows.followable_type = ?", "User"] => "following_users_count",
+    ["follows.followable_type = ?", "Organization"] => "following_orgs_count",
+    ["follows.followable_type = ?", "ActsAsTaggableOn::Tag"] => "following_tags_count"
+  }.freeze
+
   # Follows belong to the "followable" interface, and also to followers
   belongs_to :followable, polymorphic: true
   belongs_to :follower,   polymorphic: true
@@ -14,21 +26,8 @@ class Follow < ApplicationRecord
   scope :follower_podcast, ->(id) { where(follower_id: id, followable_type: "Podcast") }
   scope :follower_tag, ->(id) { where(follower_id: id, followable_type: "ActsAsTaggableOn::Tag") }
 
-  counter_culture :follower, column_name: proc { |follow|
-    case follow.followable_type
-    when "User"
-      "following_users_count"
-    when "Organization"
-      "following_orgs_count"
-    when "ActsAsTaggableOn::Tag"
-      "following_tags_count"
-      # add more whens if we add more follow types
-    end
-  }, column_names: {
-    ["follows.followable_type = ?", "User"] => "following_users_count",
-    ["follows.followable_type = ?", "Organization"] => "following_orgs_count",
-    ["follows.followable_type = ?", "ActsAsTaggableOn::Tag"] => "following_tags_count"
-  }
+  counter_culture :follower, column_name: proc { |follow| COUNTER_CULTURE_COLUMN_NAME_BY_TYPE[follow.followable_type] },
+                             column_names: COUNTER_CULTURE_COLUMNS_NAMES
   after_save :touch_follower
   after_create :send_email_notification
   after_create_commit :create_chat_channel

app/models/message.rb

@@ -151,7 +151,8 @@ class Message < ApplicationRecord
 
   # rubocop:disable Rails/OutputSafety
   def handle_slash_command(html)
-    response = if html.to_s.strip == "<p>/call</p>"
+    response = case html.to_s.strip
+               when "<p>/call</p>"
                  "<a href='/video_chats/#{chat_channel_id}'
                     class='chatchannels__richlink chatchannels__richlink--base'
                     target='_blank' rel='noopener' data-content='sidecar-video'>
@@ -159,7 +160,7 @@ class Message < ApplicationRecord
                       Let's video chat 😄
                     </h1>
                     </a>".html_safe
-               elsif html.to_s.strip == "<p>/play codenames</p>" # proof of concept
+               when "<p>/play codenames</p>" # proof of concept
                  "<a href='https://www.horsepaste.com/connect-channel-#{rand(1_000_000_000)}'
                     class='chatchannels__richlink chatchannels__richlink--base'
                     target='_blank' rel='noopener' data-content='sidecar-content-plus-video'>

app/models/tag.rb

@@ -1,4 +1,4 @@
-require_relative "../lib/acts_as_taggable_on/tag.rb"
+require_relative "../lib/acts_as_taggable_on/tag"
 
 class Tag < ActsAsTaggableOn::Tag
   attr_accessor :points, :tag_moderator_id, :remove_moderator_id

app/services/article_api_index_service.rb

@@ -81,11 +81,12 @@ class ArticleApiIndexService
   def state_articles(state)
     articles = Article.published.includes(:user, :organization)
 
-    articles = if state == "fresh"
+    articles = case state
+               when "fresh"
                  articles.where(
                    "public_reactions_count < ? AND featured_number > ? AND score > ?", 2, 7.hours.ago.to_i, -2
                  )
-               elsif state == "rising"
+               when "rising"
                  articles.where(
                    "public_reactions_count > ? AND public_reactions_count < ? AND featured_number > ?",
                    19, 33, 3.days.ago.to_i

app/services/notifications/milestone/send.rb

@@ -55,9 +55,10 @@ module Notifications
           action: "Milestone::#{type}::#{@next_milestone}",
         )
 
-        if type == "View"
+        case type
+        when "View"
           last_milestone_notification.blank? && article.page_views_count > @next_milestone
-        elsif type == "Reaction"
+        when "Reaction"
           last_milestone_notification.blank? && article.public_reactions_count > @next_milestone
         end
       end

app/services/notifications/new_follower/send.rb

@@ -28,9 +28,10 @@ module Notifications
           .where("created_at > ?", 24.hours.ago).order("created_at DESC")
 
         notification_params = { action: "Follow" }
-        if followable_type == "User"
+        case followable_type
+        when "User"
           notification_params[:user_id] = followable_id
-        elsif followable_type == "Organization"
+        when "Organization"
           notification_params[:organization_id] = followable_id
         end
 

app/services/notifications/reactions/send.rb

@@ -37,9 +37,10 @@ module Notifications
           notifiable_id: reaction.reactable_id,
           action: "Reaction"
         }
-        if receiver.is_a?(User)
+        case receiver
+        when User
           notification_params[:user_id] = receiver.id
-        elsif receiver.is_a?(Organization)
+        when Organization
           notification_params[:organization_id] = receiver.id
         end
 

app/workers/reactions/bust_reactable_cache_worker.rb

@@ -9,9 +9,10 @@ module Reactions
       return unless reaction&.reactable
 
       CacheBuster.bust(reaction.user.path)
-      if reaction.reactable_type == "Article"
+      case reaction.reactable_type
+      when "Article"
         CacheBuster.bust("/reactions?article_id=#{reaction.reactable_id}")
-      elsif reaction.reactable_type == "Comment"
+      when "Comment"
         path = "/reactions?commentable_id=#{reaction.reactable.commentable_id}&" \
           "commentable_type=#{reaction.reactable.commentable_type}"
         CacheBuster.bust(path)

app/workers/users/record_field_test_event_worker.rb

@@ -9,9 +9,10 @@ module Users
       user = User.find_by(id: user_id)
       return unless user
 
-      if goal == "user_views_article_four_days_in_week"
+      case goal
+      when "user_views_article_four_days_in_week"
         determine_weekly_pageview_goal(user, experiment)
-      elsif goal == "user_views_article_four_hours_in_day"
+      when "user_views_article_four_hours_in_day"
         determine_daily_pageview_goal(user, experiment)
       else
         field_test_converted(experiment, participant: user, goal: goal)

spec/support/omniauth_helpers.rb

@@ -67,7 +67,7 @@ module OmniauthHelpers
     class_name = error.present? ? error.class.name : ""
 
     [
-      tags: [
+      { tags: [
         "class:#{class_name}",
         "message:#{error&.message}",
         "reason:#{error.try(:error_reason)}",
@@ -76,7 +76,7 @@ module OmniauthHelpers
         "provider:#{provider}",
         "origin:",
         "params:#{params}",
-      ],
+      ] },
     ]
   end