From d6c511ba68f0d7ad53b7d53d5c0de8c3b150cde9 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 24 Jun 2025 17:59:51 +0100 Subject: [PATCH] Fix new rubocop warnings --- Dangerfile | 2 +- app/controllers/api/notes_controller.rb | 2 +- .../concerns/pagination_methods.rb | 4 +- app/controllers/passwords_controller.rb | 2 +- app/controllers/traces/data_controller.rb | 2 +- app/controllers/users_controller.rb | 22 ++++----- app/models/acl.rb | 8 ++-- app/models/user.rb | 2 +- db/migrate/021_move_to_innodb.rb | 2 +- .../api/messages/inboxes_controller_test.rb | 4 +- test/models/acl_test.rb | 48 +++++++++---------- 11 files changed, 48 insertions(+), 50 deletions(-) diff --git a/Dangerfile b/Dangerfile index 6e2aeced8..79e2f61da 100644 --- a/Dangerfile +++ b/Dangerfile @@ -24,7 +24,7 @@ else end # Report if there are merge-commits in PR -if git.commits.any? { |c| c.parents.count > 1 } +if git.commits.any? { |c| c.parents.many? } warn("Merge commits are found in PR. Please rebase to get rid of the merge commits in this PR, see CONTRIBUTING.md.") auto_label.set(pr_number, "merge-commits", "D93F0B") else diff --git a/app/controllers/api/notes_controller.rb b/app/controllers/api/notes_controller.rb index af0c5e039..3b0b66757 100644 --- a/app/controllers/api/notes_controller.rb +++ b/app/controllers/api/notes_controller.rb @@ -75,7 +75,7 @@ module Api # Create a new note def create # Check the ACLs - raise OSM::APIAccessDenied if current_user.nil? && Acl.no_note_comment(request.remote_ip) + raise OSM::APIAccessDenied if current_user.nil? && Acl.no_note_comment?(request.remote_ip) # Check the arguments are sane raise OSM::APIBadUserInput, "No lat was given" unless params[:lat] diff --git a/app/controllers/concerns/pagination_methods.rb b/app/controllers/concerns/pagination_methods.rb index 682086a27..23eb8c724 100644 --- a/app/controllers/concerns/pagination_methods.rb +++ b/app/controllers/concerns/pagination_methods.rb @@ -27,8 +27,8 @@ module PaginationMethods [ page_items, - (newer_items_cursor if page_items.count.positive? && items.exists?(["#{qualified_cursor_column} > ?", newer_items_cursor])), - (older_items_cursor if page_items.count.positive? && items.exists?(["#{qualified_cursor_column} < ?", older_items_cursor])) + (newer_items_cursor if page_items.any? && items.exists?(["#{qualified_cursor_column} > ?", newer_items_cursor])), + (older_items_cursor if page_items.any? && items.exists?(["#{qualified_cursor_column} < ?", older_items_cursor])) ] end end diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 26b21b6d9..1b1a60aa1 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -36,7 +36,7 @@ class PasswordsController < ApplicationController if user.nil? users = User.visible.where("LOWER(email) = LOWER(?)", params[:email]) - user = users.first if users.count == 1 + user = users.first if users.one? end if user diff --git a/app/controllers/traces/data_controller.rb b/app/controllers/traces/data_controller.rb index 76ed051f5..84119f2da 100644 --- a/app/controllers/traces/data_controller.rb +++ b/app/controllers/traces/data_controller.rb @@ -14,7 +14,7 @@ module Traces trace = Trace.visible.find(params[:trace_id]) if trace.public? || (current_user && current_user == trace.user) - if Acl.no_trace_download(request.remote_ip) + if Acl.no_trace_download?(request.remote_ip) head :forbidden elsif request.format == Mime[:xml] send_data(trace.xml_file.read, :filename => "#{trace.id}.xml", :type => request.format.to_s, :disposition => "attachment") diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9ccf6fe51..c38a79987 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -52,17 +52,17 @@ class UsersController < ApplicationController else flash.now[:warning] = t ".duplicate_social_email" end - else - check_signup_allowed - + elsif check_signup_allowed? self.current_user = User.new + else + render :action => "blocked" end end def create self.current_user = User.new(user_params) - if check_signup_allowed(current_user.email) + if check_signup_allowed?(current_user.email) if current_user.auth_uid.present? # We are creating an account with external authentication and # no password was specified so create a random one @@ -94,6 +94,8 @@ class UsersController < ApplicationController render :action => "new", :referer => params[:referer] end end + else + render :action => "blocked" end end @@ -228,7 +230,7 @@ class UsersController < ApplicationController ## # check signup acls - def check_signup_allowed(email = nil) + def check_signup_allowed?(email = nil) domain = if email.nil? nil else @@ -241,19 +243,15 @@ class UsersController < ApplicationController domain_mx_servers(domain) end - return true if Acl.allow_account_creation(request.remote_ip, :domain => domain, :mx => mx_servers) + return true if Acl.allow_account_creation?(request.remote_ip, :domain => domain, :mx => mx_servers) - blocked = Acl.no_account_creation(request.remote_ip, :domain => domain, :mx => mx_servers) + blocked = Acl.no_account_creation?(request.remote_ip, :domain => domain, :mx => mx_servers) blocked ||= SIGNUP_IP_LIMITER && !SIGNUP_IP_LIMITER.allow?(request.remote_ip) blocked ||= email && SIGNUP_EMAIL_LIMITER && !SIGNUP_EMAIL_LIMITER.allow?(canonical_email(email)) - if blocked - logger.info "Blocked signup from #{request.remote_ip} for #{email}" - - render :action => "blocked" - end + logger.info "Blocked signup from #{request.remote_ip} for #{email}" if blocked !blocked end diff --git a/app/models/acl.rb b/app/models/acl.rb index 81e720159..e3695ce1d 100644 --- a/app/models/acl.rb +++ b/app/models/acl.rb @@ -37,19 +37,19 @@ class Acl < ApplicationRecord acls end - def self.no_account_creation(address, options = {}) + def self.no_account_creation?(address, options = {}) match(address, options).exists?(:k => "no_account_creation") end - def self.allow_account_creation(address, options = {}) + def self.allow_account_creation?(address, options = {}) match(address, options).exists?(:k => "allow_account_creation") end - def self.no_note_comment(address, domain = nil) + def self.no_note_comment?(address, domain = nil) match(address, :domain => domain).exists?(:k => "no_note_comment") end - def self.no_trace_download(address, domain = nil) + def self.no_trace_download?(address, domain = nil) match(address, :domain => domain).exists?(:k => "no_trace_download") end end diff --git a/app/models/user.rb b/app/models/user.rb index 400ce864c..d19d1c10a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -161,7 +161,7 @@ class User < ApplicationRecord if user.nil? users = where("LOWER(email) = LOWER(?) OR LOWER(NORMALIZE(display_name, NFKC)) = LOWER(NORMALIZE(?, NFKC))", options[:username].strip, options[:username]) - user = users.first if users.count == 1 + user = users.first if users.one? end if user && PasswordHash.check(user.pass_crypt, user.pass_salt, options[:password]) diff --git a/db/migrate/021_move_to_innodb.rb b/db/migrate/021_move_to_innodb.rb index 4dac8410c..1e09384db 100644 --- a/db/migrate/021_move_to_innodb.rb +++ b/db/migrate/021_move_to_innodb.rb @@ -14,7 +14,7 @@ class MoveToInnodb < ActiveRecord::Migration[4.2] # current version to something less so that we can update the version in # batches of 10000 tbl.classify.constantize.update_all(:version => -1) - tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 }) while tbl.classify.constantize.where(:version => -1).count.positive? + tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 }) while tbl.classify.constantize.where(:version => -1).any? # execute "UPDATE current_#{tbl} SET version = " + # "(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)" # The above update causes a MySQL error: diff --git a/test/controllers/api/messages/inboxes_controller_test.rb b/test/controllers/api/messages/inboxes_controller_test.rb index 65c61f94d..1f6c51878 100644 --- a/test/controllers/api/messages/inboxes_controller_test.rb +++ b/test/controllers/api/messages/inboxes_controller_test.rb @@ -105,7 +105,7 @@ module Api jsm = js["messages"] assert_operator jsm.count, :<=, 20 - break if jsm.nil? || jsm.count.zero? + break if jsm.nil? || jsm.none? assert_operator(jsm[0]["id"], :>=, params[:from_id]) unless params[:from_id].nil? # ensure ascending order @@ -139,7 +139,7 @@ module Api jsm = js["messages"] assert_operator jsm.count, :<=, 20 - break if jsm.nil? || jsm.count.zero? + break if jsm.nil? || jsm.none? if params[:from_id].nil? real_max_id = jsm[0]["id"] diff --git a/test/models/acl_test.rb b/test/models/acl_test.rb index 6bd87729c..bf7c81214 100644 --- a/test/models/acl_test.rb +++ b/test/models/acl_test.rb @@ -9,58 +9,58 @@ class AclTest < ActiveSupport::TestCase end def test_no_account_creation_by_subnet - assert_not Acl.no_account_creation("192.168.1.1") + assert_not Acl.no_account_creation?("192.168.1.1") create(:acl, :address => "192.168.0.0/16", :k => "no_account_creation") - assert Acl.no_account_creation("192.168.1.1") + assert Acl.no_account_creation?("192.168.1.1") end def test_no_account_creation_by_domain - assert_not Acl.no_account_creation("192.168.1.1", :domain => "example.com") - assert_not Acl.no_account_creation("192.168.1.1", :domain => "test.example.com") + assert_not Acl.no_account_creation?("192.168.1.1", :domain => "example.com") + assert_not Acl.no_account_creation?("192.168.1.1", :domain => "test.example.com") create(:acl, :domain => "example.com", :k => "no_account_creation") - assert Acl.no_account_creation("192.168.1.1", :domain => "example.com") - assert Acl.no_account_creation("192.168.1.1", :domain => "test.example.com") + assert Acl.no_account_creation?("192.168.1.1", :domain => "example.com") + assert Acl.no_account_creation?("192.168.1.1", :domain => "test.example.com") end def test_no_account_creation_by_mx - assert_not Acl.no_account_creation("192.168.1.1", :mx => "mail.example.com") + assert_not Acl.no_account_creation?("192.168.1.1", :mx => "mail.example.com") create(:acl, :mx => "mail.example.com", :k => "no_account_creation") - assert Acl.no_account_creation("192.168.1.1", :mx => "mail.example.com") + assert Acl.no_account_creation?("192.168.1.1", :mx => "mail.example.com") end def test_allow_account_creation_by_subnet - assert_not Acl.allow_account_creation("192.168.1.1") + assert_not Acl.allow_account_creation?("192.168.1.1") create(:acl, :address => "192.168.0.0/16", :k => "allow_account_creation") - assert Acl.allow_account_creation("192.168.1.1") + assert Acl.allow_account_creation?("192.168.1.1") end def test_allow_account_creation_by_domain - assert_not Acl.allow_account_creation("192.168.1.1", :domain => "example.com") - assert_not Acl.allow_account_creation("192.168.1.1", :domain => "test.example.com") + assert_not Acl.allow_account_creation?("192.168.1.1", :domain => "example.com") + assert_not Acl.allow_account_creation?("192.168.1.1", :domain => "test.example.com") create(:acl, :domain => "example.com", :k => "allow_account_creation") - assert Acl.allow_account_creation("192.168.1.1", :domain => "example.com") - assert Acl.allow_account_creation("192.168.1.1", :domain => "test.example.com") + assert Acl.allow_account_creation?("192.168.1.1", :domain => "example.com") + assert Acl.allow_account_creation?("192.168.1.1", :domain => "test.example.com") end def test_allow_account_creation_by_mx - assert_not Acl.allow_account_creation("192.168.1.1", :mx => "mail.example.com") + assert_not Acl.allow_account_creation?("192.168.1.1", :mx => "mail.example.com") create(:acl, :mx => "mail.example.com", :k => "allow_account_creation") - assert Acl.allow_account_creation("192.168.1.1", :mx => "mail.example.com") + assert Acl.allow_account_creation?("192.168.1.1", :mx => "mail.example.com") end def test_no_note_comment_by_domain - assert_not Acl.no_note_comment("192.168.1.1", "example.com") - assert_not Acl.no_note_comment("192.168.1.1", "test.example.com") + assert_not Acl.no_note_comment?("192.168.1.1", "example.com") + assert_not Acl.no_note_comment?("192.168.1.1", "test.example.com") create(:acl, :domain => "example.com", :k => "no_note_comment") - assert Acl.no_note_comment("192.168.1.1", "example.com") - assert Acl.no_note_comment("192.168.1.1", "test.example.com") + assert Acl.no_note_comment?("192.168.1.1", "example.com") + assert Acl.no_note_comment?("192.168.1.1", "test.example.com") end def test_no_trace_download_by_domain - assert_not Acl.no_trace_download("192.168.1.1", "example.com") - assert_not Acl.no_trace_download("192.168.1.1", "test.example.com") + assert_not Acl.no_trace_download?("192.168.1.1", "example.com") + assert_not Acl.no_trace_download?("192.168.1.1", "test.example.com") create(:acl, :domain => "example.com", :k => "no_trace_download") - assert Acl.no_trace_download("192.168.1.1", "example.com") - assert Acl.no_trace_download("192.168.1.1", "test.example.com") + assert Acl.no_trace_download?("192.168.1.1", "example.com") + assert Acl.no_trace_download?("192.168.1.1", "test.example.com") end end -- 2.39.5