From 085c60992b28ade18c666294df7a557746b5af8e Mon Sep 17 00:00:00 2001 From: rhymes Date: Mon, 10 Aug 2020 19:35:55 +0200 Subject: [PATCH] Rubocop: Enable and fix Style/OptionalBooleanParameter cop (#9711) * Enable and fix Style/OptionalBooleanParameter cop * Keep regular parameters for workers * Keep regular parameters for workers * Fix spec * Trigger Travis correctly --- .rubocop.yml | 2 +- app/controllers/follows_controller.rb | 2 +- app/helpers/comments_helper.rb | 2 +- app/models/notification.rb | 4 ++-- app/services/github/oauth_client.rb | 2 +- app/services/notifications/new_follower/send.rb | 2 +- app/services/podcasts/feed.rb | 10 +++++----- app/services/rss_reader.rb | 6 +++--- app/services/search/client.rb | 2 +- app/services/twitter_client/client.rb | 2 +- app/views/comments/_comment_proper.html.erb | 3 ++- app/workers/notifications/new_follower_worker.rb | 4 ++-- app/workers/users/delete_worker.rb | 2 +- lib/tasks/fetch.rake | 3 ++- spec/requests/user/user_settings_spec.rb | 6 +++--- spec/services/notifications/new_follower/send_spec.rb | 2 +- spec/system/notifications/notifications_page_spec.rb | 2 +- 17 files changed, 29 insertions(+), 27 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ef08051c9..25adaef25 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -433,7 +433,7 @@ Style/ClassAndModuleChildren: Style/OptionalBooleanParameter: Description: 'Use keyword arguments when defining method with boolean argument.' StyleGuide: '#boolean-keyword-arguments' - Enabled: pending + Enabled: true Style/OptionHash: Description: "Don't use option hashes when you can use keyword arguments." diff --git a/app/controllers/follows_controller.rb b/app/controllers/follows_controller.rb index 94725d3bc..961e1ba37 100644 --- a/app/controllers/follows_controller.rb +++ b/app/controllers/follows_controller.rb @@ -102,7 +102,7 @@ class FollowsController < ApplicationController def unfollow(followable, need_notification: false) user_follow = current_user.stop_following(followable) - Notification.send_new_follower_notification_without_delay(user_follow, true) if need_notification + Notification.send_new_follower_notification_without_delay(user_follow, is_read: true) if need_notification "unfollowed" end diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 1e629c4c1..b5095de60 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -1,5 +1,5 @@ module CommentsHelper - def comment_class(comment, is_view_root = false) + def comment_class(comment, is_view_root: false) if comment.root? || is_view_root "root" else diff --git a/app/models/notification.rb b/app/models/notification.rb index 08ccbaf49..976b2bc82 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -24,7 +24,7 @@ class Notification < ApplicationRecord scope :unread, -> { where(read: false) } class << self - def send_new_follower_notification(follow, is_read = false) + def send_new_follower_notification(follow, is_read: false) return unless follow && Follow.need_new_follower_notification_for?(follow.followable_type) return if follow.followable_type == "User" && UserBlock.blocking?(follow.followable_id, follow.follower_id) @@ -32,7 +32,7 @@ class Notification < ApplicationRecord Notifications::NewFollowerWorker.perform_async(follow_data, is_read) end - def send_new_follower_notification_without_delay(follow, is_read = false) + def send_new_follower_notification_without_delay(follow, is_read: false) return unless follow && Follow.need_new_follower_notification_for?(follow.followable_type) return if follow.followable_type == "User" && UserBlock.blocking?(follow.followable_id, follow.follower_id) diff --git a/app/services/github/oauth_client.rb b/app/services/github/oauth_client.rb index a7652837c..3ed4d6fc3 100644 --- a/app/services/github/oauth_client.rb +++ b/app/services/github/oauth_client.rb @@ -42,7 +42,7 @@ module Github end # adapted from https://api.rubyonrails.org/classes/Module.html#method-i-delegate_missing_to - def respond_to_missing?(method, _include_all = false) + def respond_to_missing?(method, _include_all = false) # rubocop:disable Style/OptionalBooleanParameter target.respond_to?(method, false) || super end diff --git a/app/services/notifications/new_follower/send.rb b/app/services/notifications/new_follower/send.rb index 2eecc91a5..a8da554a9 100644 --- a/app/services/notifications/new_follower/send.rb +++ b/app/services/notifications/new_follower/send.rb @@ -6,7 +6,7 @@ module Notifications # * :followable_id [Integer] # * :followable_type [String] - "User" or "Organization" # * :follower_id [Integer] - user id - def initialize(follow_data, is_read = false) + def initialize(follow_data, is_read: false) # we explicitly symbolize_keys because FollowData.new will fail otherwise with an error of # ":followable_id is missing in Hash input". FollowData expects a symbol, not a string. follow_data.symbolize_keys! diff --git a/app/services/podcasts/feed.rb b/app/services/podcasts/feed.rb index b968ec379..4b2ab0f9a 100644 --- a/app/services/podcasts/feed.rb +++ b/app/services/podcasts/feed.rb @@ -12,7 +12,7 @@ module Podcasts rss = HTTParty.get(podcast.feed_url, limit: 7).body feed = RSS::Parser.parse(rss, false) - set_unreachable(:unparsable, force_update) && return unless feed + set_unreachable(status: :unparsable, force_update: force_update) && return unless feed get_episode = Podcasts::GetEpisode.new(podcast) feed.items.first(limit).each do |item| @@ -21,18 +21,18 @@ module Podcasts podcast.update_columns(reachable: true, status_notice: "") feed.items.size rescue Net::OpenTimeout, Errno::ECONNREFUSED, SocketError, HTTParty::RedirectionTooDeep - set_unreachable(:unreachable, force_update) + set_unreachable(status: :unreachable, force_update: force_update) rescue OpenSSL::SSL::SSLError - set_unreachable(:ssl_failed, force_update) + set_unreachable(status: :ssl_failed, force_update: force_update) rescue RSS::NotWellFormedError - set_unreachable(:unparsable, force_update) + set_unreachable(status: :unparsable, force_update: force_update) end private attr_reader :podcast - def set_unreachable(status = :unreachable, force_update = false) + def set_unreachable(status: :unreachable, force_update: false) # don't recheck if the podcast was already unreachable or force update is required need_refetching = podcast.reachable || force_update podcast.update_columns(reachable: false, status_notice: I18n.t(status, scope: "podcasts.statuses")) diff --git a/app/services/rss_reader.rb b/app/services/rss_reader.rb index 3918ff079..f7c2a406e 100644 --- a/app/services/rss_reader.rb +++ b/app/services/rss_reader.rb @@ -1,9 +1,9 @@ class RssReader - def self.get_all_articles(force = true) - new.get_all_articles(force) + def self.get_all_articles(force: true) + new.get_all_articles(force: force) end - def get_all_articles(force = true) + def get_all_articles(force: true) articles = [] User.where.not(feed_url: [nil, ""]).find_each do |user| diff --git a/app/services/search/client.rb b/app/services/search/client.rb index a73852b7d..1b0369e50 100644 --- a/app/services/search/client.rb +++ b/app/services/search/client.rb @@ -21,7 +21,7 @@ module Search end # adapted from https://api.rubyonrails.org/classes/Module.html#method-i-delegate_missing_to - def respond_to_missing?(method, _include_all = false) + def respond_to_missing?(method, _include_all = false) # rubocop:disable Style/OptionalBooleanParameter target.respond_to?(method, false) || super end diff --git a/app/services/twitter_client/client.rb b/app/services/twitter_client/client.rb index 06e4cebb7..590814b3d 100644 --- a/app/services/twitter_client/client.rb +++ b/app/services/twitter_client/client.rb @@ -12,7 +12,7 @@ module TwitterClient end # adapted from https://api.rubyonrails.org/classes/Module.html#method-i-delegate_missing_to - def respond_to_missing?(method, _include_all = false) + def respond_to_missing?(method, _include_all = false) # rubocop:disable Style/OptionalBooleanParameter target.respond_to?(method, false) || super end diff --git a/app/views/comments/_comment_proper.html.erb b/app/views/comments/_comment_proper.html.erb index 8c2f8e7a5..f0148a866 100644 --- a/app/views/comments/_comment_proper.html.erb +++ b/app/views/comments/_comment_proper.html.erb @@ -1,5 +1,6 @@
-
3 %> comment-deep-<%= comment.depth %>" data-comment-id="<%= comment.id %>" data-comment-author-id="<%= comment_user_id_unless_deleted comment %>" data-content-user-id="<%= comment_user_id_unless_deleted comment %>"> +
3 %> comment-deep-<%= comment.depth %>" data-comment-id="<%= comment.id %>" data-comment-author-id="<%= comment_user_id_unless_deleted comment %>" data-content-user-id="<%= comment_user_id_unless_deleted comment %>"> <% if comment.deleted %>
diff --git a/app/workers/notifications/new_follower_worker.rb b/app/workers/notifications/new_follower_worker.rb index 02dbbb388..67c58ec5c 100644 --- a/app/workers/notifications/new_follower_worker.rb +++ b/app/workers/notifications/new_follower_worker.rb @@ -4,8 +4,8 @@ module Notifications sidekiq_options queue: :medium_priority, retry: 10 - def perform(follow_data, is_read = false) - Notifications::NewFollower::Send.call(follow_data, is_read) + def perform(follow_data, is_read = false) # rubocop:disable Style/OptionalBooleanParameter + Notifications::NewFollower::Send.call(follow_data, is_read: is_read) end end end diff --git a/app/workers/users/delete_worker.rb b/app/workers/users/delete_worker.rb index 39f4ac569..6d0c30e87 100644 --- a/app/workers/users/delete_worker.rb +++ b/app/workers/users/delete_worker.rb @@ -4,7 +4,7 @@ module Users sidekiq_options queue: :high_priority, retry: 10 - def perform(user_id, admin_delete = false) + def perform(user_id, admin_delete = false) # rubocop:disable Style/OptionalBooleanParameter user = User.find_by(id: user_id) return unless user diff --git a/lib/tasks/fetch.rake b/lib/tasks/fetch.rake index 09d757636..4ae7ee188 100644 --- a/lib/tasks/fetch.rake +++ b/lib/tasks/fetch.rake @@ -8,7 +8,8 @@ end task fetch_all_rss: :environment do Rails.application.eager_load! - RssReader.get_all_articles(false) # False means don't force fetch. Fetch "random" subset instead of all of them. + + RssReader.get_all_articles(force: false) # don't force fetch. Fetch "random" subset instead of all of them. end task resave_supported_tags: :environment do diff --git a/spec/requests/user/user_settings_spec.rb b/spec/requests/user/user_settings_spec.rb index 1933f43ac..fa3b149ea 100644 --- a/spec/requests/user/user_settings_spec.rb +++ b/spec/requests/user/user_settings_spec.rb @@ -244,9 +244,9 @@ RSpec.describe "UserSettings", type: :request do end context "when requesting an export of the articles" do - def send_request(flag = true) + def send_request(export_requested: true) put "/users/#{user.id}", params: { - user: { tab: "misc", export_requested: flag } + user: { tab: "misc", export_requested: export_requested } } end @@ -282,7 +282,7 @@ RSpec.describe "UserSettings", type: :request do it "does not send an email if there was no request" do sidekiq_perform_enqueued_jobs do - expect { send_request(false) }.not_to(change { ActionMailer::Base.deliveries.count }) + expect { send_request(export_requested: false) }.not_to(change { ActionMailer::Base.deliveries.count }) end end end diff --git a/spec/services/notifications/new_follower/send_spec.rb b/spec/services/notifications/new_follower/send_spec.rb index 1502e23b6..4398af434 100644 --- a/spec/services/notifications/new_follower/send_spec.rb +++ b/spec/services/notifications/new_follower/send_spec.rb @@ -52,7 +52,7 @@ RSpec.describe Notifications::NewFollower::Send, type: :service do end it "creates a read notification" do - notification = described_class.call(follow_data(follow), true) + notification = described_class.call(follow_data(follow), is_read: true) expect(notification.read).to be_truthy end diff --git a/spec/system/notifications/notifications_page_spec.rb b/spec/system/notifications/notifications_page_spec.rb index 5277496d6..4d97cbcfc 100644 --- a/spec/system/notifications/notifications_page_spec.rb +++ b/spec/system/notifications/notifications_page_spec.rb @@ -47,7 +47,7 @@ RSpec.describe "Notifications page", type: :system, js: true do it "allows user to follow other users back" do follow = leslie.follow(alex) - Notification.send_new_follower_notification_without_delay(follow, "Published") + Notification.send_new_follower_notification_without_delay(follow, is_read: true) visit "/notifications" expect(page).to have_css("div.single-notification") click_button("Follow back")