cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix some new rubocop warnings

Browse source
This commit is contained in:
Tom Hughes 2020-08-06 21:56:18 +01:00
parent c03ba5f6b8
commit ea59d95f4a
23 changed files with 99 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

30
.rubocop_todo.yml
View file

@ -1,10 +1,11 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2020-08-06 17:20:38 UTC using RuboCop version 0.89.0.
# on 2020-08-06 20:53:30 UTC using RuboCop version 0.89.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
# versions of RuboCop, may require this file to be generated again.
# versions of RuboCop, may require this file to be generated again.
# Work around erblint issues.
# https://github.com/openstreetmap/openstreetmap-website/issues/2472
@ -87,6 +88,25 @@ Metrics/ParameterLists:
Metrics/PerceivedComplexity:
Max: 26
# Offense count: 540
Minitest/MultipleAssertions:
Max: 81
# Offense count: 26
# Cop supports --auto-correct.
Minitest/TestMethodName:
Exclude:
- 'test/abilities/api_capability_test.rb'
- 'test/controllers/api/nodes_controller_test.rb'
- 'test/controllers/api/old_nodes_controller_test.rb'
- 'test/controllers/api/relations_controller_test.rb'
- 'test/controllers/api/ways_controller_test.rb'
- 'test/helpers/browse_helper_test.rb'
- 'test/integration/client_applications_test.rb'
- 'test/integration/short_links_test.rb'
- 'test/integration/user_blocks_test.rb'
- 'test/integration/user_creation_test.rb'
# Offense count: 6
Naming/AccessorMethodName:
Exclude:
@ -137,6 +157,14 @@ Rails/HelperInstanceVariable:
Exclude:
- 'app/helpers/title_helper.rb'
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: Include.
# Include: app/mailers/**/*.rb
Rails/MailerName:
Exclude:
- 'app/mailers/notifier.rb'
# Offense count: 5
# Configuration parameters: Include.
# Include: db/migrate/*.rb

2
app/controllers/user_blocks_controller.rb
View file

@ -128,7 +128,7 @@ class UserBlocksController < ApplicationController
@block_period = params[:user_block_period].to_i
@valid_params = false
if !UserBlock::PERIODS.include?(@block_period)
if UserBlock::PERIODS.exclude?(@block_period)
flash[:error] = t("user_blocks.filter.block_period")
elsif @user_block && !@user_block.active?

6
app/models/acl.rb
View file

@ -30,14 +30,14 @@ class Acl < ApplicationRecord
end
def self.no_account_creation(address, options = {})
match(address, options).where(:k => "no_account_creation").exists?
match(address, options).exists?(:k => "no_account_creation")
end
def self.no_note_comment(address, domain = nil)
match(address, :domain => domain).where(:k => "no_note_comment").exists?
match(address, :domain => domain).exists?(:k => "no_note_comment")
end
def self.no_trace_download(address, domain = nil)
match(address, :domain => domain).where(:k => "no_trace_download").exists?
match(address, :domain => domain).exists?(:k => "no_trace_download")
end
end

2
app/models/user.rb
View file

@ -218,7 +218,7 @@ class User < ApplicationRecord
end
def is_friends_with?(new_friend)
friendships.where(:befriendee => new_friend).exists?
friendships.exists?(:befriendee => new_friend)
end
##

2
lib/bounding_box.rb
View file

@ -89,7 +89,7 @@ class BoundingBox
end
def complete?
!to_a.include?(nil)
to_a.exclude?(nil)
end
def centre_lon

2
lib/classic_pagination/pagination.rb
View file

@ -158,7 +158,7 @@ module ActionController
def create_paginators_and_retrieve_collections #:nodoc:
Pagination::OPTIONS[self.class].each do |collection_id, options|
next if options[:actions] && !options[:actions].include?(action_name)
next if options[:actions]&.exclude?(action_name)
paginator, collection =
paginator_and_collection_for(collection_id, options)

2
lib/locale.rb
View file

@ -17,7 +17,7 @@ class Locale < I18n::Locale::Tag::Rfc4646
def expand
List.new(reverse.each_with_object([]) do |locale, expanded|
locale.candidates.uniq.reverse_each do |candidate|
expanded << candidate if candidate == locale || !expanded.include?(candidate)
expanded << candidate if candidate == locale || expanded.exclude?(candidate)
end
end.reverse.uniq)
end

2
script/update-spam-blocks
View file

@ -13,7 +13,7 @@ addresses = User.count(
addresses.each do |address, count|
next unless count > 1
next if Acl.where(:address => address).exists?
next if Acl.exists?(:address => address)
Acl.create({
:address => address,

37
test/controllers/api/amf_controller_test.rb
View file

@ -291,7 +291,7 @@ module Api
assert_response :success
amf_parse_response
rel = amf_result("/1")
assert_equal rel[0], 0
assert_equal(0, rel[0])
assert_equal rel[2], id
end
@ -301,8 +301,8 @@ module Api
assert_response :success
amf_parse_response
rel = amf_result("/1")
assert_equal rel[0], -4
assert_equal rel[1], "relation"
assert_equal(-4, rel[0])
assert_equal("relation", rel[1])
assert_equal rel[2], id
assert(rel[3].nil?) && rel[4].nil?
end
@ -313,8 +313,8 @@ module Api
assert_response :success
amf_parse_response
rel = amf_result("/1")
assert_equal rel[0], -4
assert_equal rel[1], "relation"
assert_equal(-4, rel[0])
assert_equal("relation", rel[1])
assert_equal rel[2], id
assert(rel[3].nil?) && rel[4].nil?
end
@ -426,8 +426,8 @@ module Api
history = amf_result("/1")
# ['way',wayid,history]
assert_equal history[0], "way"
assert_equal history[1], 0
assert_equal("way", history[0])
assert_equal(0, history[1])
assert_empty history[2]
end
@ -445,8 +445,7 @@ module Api
# ['node',nodeid,history]
# note that (as per getway_history) we actually round up
# to the next second
assert_equal history[0], "node",
'first element should be "node"'
assert_equal("node", history[0], 'first element should be "node"')
assert_equal history[1], node.id,
"second element should be the input node ID"
assert_equal history[2].first[0],
@ -464,8 +463,8 @@ module Api
history = amf_result("/1")
# ['node',nodeid,history]
assert_equal history[0], "node"
assert_equal history[1], 0
assert_equal("node", history[0])
assert_equal(0, history[1])
assert_empty history[2]
end
@ -979,15 +978,15 @@ module Api
new_node = Node.find(new_node_id)
assert_equal 1, new_node.version
assert new_node.visible
assert_equal 4.56, new_node.lon
assert_equal 12.34, new_node.lat
assert_in_delta(4.56, new_node.lon)
assert_in_delta(12.34, new_node.lat)
assert_equal({ "test" => "new" }, new_node.tags)
changed_node = Node.find(d)
assert_equal 2, changed_node.version
assert changed_node.visible
assert_equal 12.34, changed_node.lon
assert_equal 4.56, changed_node.lat
assert_in_delta(12.34, changed_node.lon)
assert_in_delta(4.56, changed_node.lat)
assert_equal({ "test" => "ok" }, changed_node.tags)
# node is not deleted because our other ways are using it
@ -1074,15 +1073,15 @@ module Api
new_node = Node.find(new_node_id)
assert_equal 1, new_node.version
assert new_node.visible
assert_equal 4.56, new_node.lon
assert_equal 12.34, new_node.lat
assert_in_delta(4.56, new_node.lon)
assert_in_delta(12.34, new_node.lat)
assert_equal({ "test" => "new" }, new_node.tags)
changed_node = Node.find(b)
assert_equal 2, changed_node.version
assert changed_node.visible
assert_equal 12.34, changed_node.lon
assert_equal 4.56, changed_node.lat
assert_in_delta(12.34, changed_node.lon)
assert_in_delta(4.56, changed_node.lat)
assert_equal({ "test" => "ok" }, changed_node.tags)
deleted_node = Node.find(d)

6
test/controllers/api/changes_controller_test.rb
View file

@ -60,7 +60,7 @@ module Api
zoom_to_test.each do |zoom|
get changes_path(:zoom => zoom)
assert_response :bad_request
assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
assert_equal("Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", @response.body)
end
end
@ -81,7 +81,7 @@ module Api
invalid.each do |hour|
get changes_path(:hours => hour)
assert_response :bad_request, "Problem with the hour: #{hour}"
assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", "Problem with the hour: #{hour}."
assert_equal("Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", @response.body, "Problem with the hour: #{hour}.")
end
end
@ -95,7 +95,7 @@ module Api
def test_changes_start_end_invalid
get changes_path(:start => "2010-04-03 10:55:00", :end => "2010-04-03 09:55:00")
assert_response :bad_request
assert_equal @response.body, "Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours"
assert_equal("Requested zoom is invalid, or the supplied start is after the end time, or the start duration is more than 24 hours", @response.body)
end
def test_changes_start_end_valid

2
test/controllers/api/changesets_controller_test.rb
View file

@ -908,7 +908,7 @@ module Api
CHANGESET
post changeset_upload_path(changeset), :params => diff, :headers => auth_header
assert_response :bad_request, "Shouldn't be able to upload a diff with the action ping"
assert_equal @response.body, "Unknown action ping, choices are create, modify, delete"
assert_equal("Unknown action ping, choices are create, modify, delete", @response.body)
end
##

2
test/controllers/api/map_controller_test.rb
View file

@ -181,7 +181,7 @@ module Api
.select { |a| a["version"] == node.version }
.select { |a| a["changeset"] == node.changeset_id }
.select { |a| a["timestamp"] == node.timestamp.xmlschema }
assert_equal result_nodes.count, 1
assert_equal(1, result_nodes.count)
result_node = result_nodes.first
assert_equal result_node["tags"], tag.k => tag.v

24
test/controllers/api/relations_controller_test.rb
View file

@ -296,10 +296,8 @@ module Api
assert_not_nil checkrelation,
"uploaded relation not found in data base after upload"
# compare values
assert_equal checkrelation.members.length, 0,
"saved relation contains members but should not"
assert_equal checkrelation.tags.length, 1,
"saved relation does not contain exactly one tag"
assert_equal(0, checkrelation.members.length, "saved relation contains members but should not")
assert_equal(1, checkrelation.tags.length, "saved relation does not contain exactly one tag")
assert_equal changeset.id, checkrelation.changeset.id,
"saved relation does not belong in the changeset it was assigned to"
assert_equal user.id, checkrelation.changeset.user_id,
@ -326,10 +324,8 @@ module Api
assert_not_nil checkrelation,
"uploaded relation not found in data base after upload"
# compare values
assert_equal checkrelation.members.length, 1,
"saved relation does not contain exactly one member"
assert_equal checkrelation.tags.length, 1,
"saved relation does not contain exactly one tag"
assert_equal(1, checkrelation.members.length, "saved relation does not contain exactly one member")
assert_equal(1, checkrelation.tags.length, "saved relation does not contain exactly one tag")
assert_equal changeset.id, checkrelation.changeset.id,
"saved relation does not belong in the changeset it was assigned to"
assert_equal user.id, checkrelation.changeset.user_id,
@ -356,10 +352,8 @@ module Api
assert_not_nil checkrelation,
"uploaded relation not found in data base after upload"
# compare values
assert_equal checkrelation.members.length, 1,
"saved relation does not contain exactly one member"
assert_equal checkrelation.tags.length, 1,
"saved relation does not contain exactly one tag"
assert_equal(1, checkrelation.members.length, "saved relation does not contain exactly one member")
assert_equal(1, checkrelation.tags.length, "saved relation does not contain exactly one tag")
assert_equal changeset.id, checkrelation.changeset.id,
"saved relation does not belong in the changeset it was assigned to"
assert_equal user.id, checkrelation.changeset.user_id,
@ -387,10 +381,8 @@ module Api
assert_not_nil checkrelation,
"uploaded relation not found in data base after upload"
# compare values
assert_equal checkrelation.members.length, 2,
"saved relation does not have exactly two members"
assert_equal checkrelation.tags.length, 1,
"saved relation does not contain exactly one tag"
assert_equal(2, checkrelation.members.length, "saved relation does not have exactly two members")
assert_equal(1, checkrelation.tags.length, "saved relation does not contain exactly one tag")
assert_equal changeset.id, checkrelation.changeset.id,
"saved relation does not belong in the changeset it was assigned to"
assert_equal user.id, checkrelation.changeset.user_id,

2
test/controllers/api/traces_controller_test.rb
View file

@ -307,7 +307,7 @@ module Api
updated = Trace.find(trace.id)
# Ensure there's only one tag in the database after updating
assert_equal Tracetag.count, 1
assert_equal(1, Tracetag.count)
# The new tag object might have a different id, so check the string representation
assert_equal trace.tagstring, updated.tagstring
end

3
test/controllers/api/ways_controller_test.rb
View file

@ -180,8 +180,7 @@ module Api
assert_not_nil checkway,
"uploaded way not found in data base after upload"
# compare values
assert_equal checkway.nds.length, 2,
"saved way does not contain exactly one node"
assert_equal(2, checkway.nds.length, "saved way does not contain exactly one node")
assert_equal checkway.nds[0], node1.id,
"saved way does not contain the right node on pos 0"
assert_equal checkway.nds[1], node2.id,

16
test/controllers/site_controller_test.rb
View file

@ -275,8 +275,8 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
assert_response :success
assert_template "edit"
assert_equal 1.0, assigns(:lat)
assert_equal 1.0, assigns(:lon)
assert_in_delta(1.0, assigns(:lat))
assert_in_delta(1.0, assigns(:lon))
assert_equal 18, assigns(:zoom)
end
@ -312,8 +312,8 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
get edit_path(:way => way.id)
assert_response :success
assert_template "edit"
assert_equal 3.0, assigns(:lat)
assert_equal 3.0, assigns(:lon)
assert_in_delta(3.0, assigns(:lat))
assert_in_delta(3.0, assigns(:lon))
assert_equal 17, assigns(:zoom)
end
@ -349,8 +349,8 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
get edit_path(:note => note.id)
assert_response :success
assert_template "edit"
assert_equal 1.0, assigns(:lat)
assert_equal 1.0, assigns(:lon)
assert_in_delta(1.0, assigns(:lat))
assert_in_delta(1.0, assigns(:lon))
assert_equal 17, assigns(:zoom)
end
@ -386,8 +386,8 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
get edit_path(:gpx => gpx.id)
assert_response :success
assert_template "edit"
assert_equal 1.0, assigns(:lat)
assert_equal 1.0, assigns(:lon)
assert_in_delta(1.0, assigns(:lat))
assert_in_delta(1.0, assigns(:lon))
assert_equal 16, assigns(:zoom)
end

4
test/helpers/user_helper_test.rb
View file

@ -73,10 +73,10 @@ class UserHelperTest < ActionView::TestCase
def test_auth_button
button = auth_button("google", "google")
assert_equal button, "<a class=\"auth_button\" title=\"Login with Google\" href=\"/auth/google\"><img alt=\"Login with a Google OpenID\" src=\"/images/google.png\" /></a>"
assert_equal("<a class=\"auth_button\" title=\"Login with Google\" href=\"/auth/google\"><img alt=\"Login with a Google OpenID\" src=\"/images/google.png\" /></a>", button)
button = auth_button("yahoo", "openid", :openid_url => "yahoo.com")
assert_equal button, "<a class=\"auth_button\" title=\"Login with Yahoo\" href=\"/auth/openid?openid_url=yahoo\.com\"><img alt=\"Login with a Yahoo OpenID\" src=\"/images/yahoo.png\" /></a>"
assert_equal("<a class=\"auth_button\" title=\"Login with Yahoo\" href=\"/auth/openid?openid_url=yahoo\.com\"><img alt=\"Login with a Yahoo OpenID\" src=\"/images/yahoo.png\" /></a>", button)
end
private

16
test/lib/country_test.rb
View file

@ -5,20 +5,20 @@ class CountryTest < ActiveSupport::TestCase
gb = Country.find("GB")
assert_not_nil gb
assert_equal "GB", gb.code
assert_equal(-8.623555, gb.min_lon)
assert_equal 59.360249, gb.max_lat
assert_equal 1.759, gb.max_lon
assert_equal 49.906193, gb.min_lat
assert_in_delta(-8.623555, gb.min_lon)
assert_in_delta(59.360249, gb.max_lat)
assert_in_delta(1.759, gb.max_lon)
assert_in_delta(49.906193, gb.min_lat)
end
def test_au
au = Country.find("AU")
assert_not_nil au
assert_equal "AU", au.code
assert_equal 112.911057, au.min_lon
assert_equal(-10.062805, au.max_lat)
assert_equal 153.639252, au.max_lon
assert_equal(-43.64397, au.min_lat)
assert_in_delta(112.911057, au.min_lon)
assert_in_delta(-10.062805, au.max_lat)
assert_in_delta(153.639252, au.max_lon)
assert_in_delta(-43.64397, au.min_lat)
end
def test_xx

2
test/models/message_test.rb
View file

@ -64,7 +64,7 @@ class MessageTest < ActiveSupport::TestCase
db_msg = msg.class.find(msg.id)
assert_equal char, db_msg.title, "Database silently truncated message title"
rescue ArgumentError => e
assert_equal e.to_s, "invalid byte sequence in UTF-8"
assert_equal("invalid byte sequence in UTF-8", e.to_s)
end
end

10
test/models/node_test.rb
View file

@ -88,7 +88,7 @@ class NodeTest < ActiveSupport::TestCase
assert_equal node_template.visible, node.visible
assert_equal node_template.timestamp.to_i, node.timestamp.to_i
assert_equal OldNode.where(:node_id => node_template.id).count, 1
assert_equal(1, OldNode.where(:node_id => node_template.id).count)
old_node = OldNode.where(:node_id => node_template.id).first
assert_not_nil old_node
assert_equal node_template.latitude, old_node.latitude
@ -105,7 +105,7 @@ class NodeTest < ActiveSupport::TestCase
node_template = Node.find(node.id)
assert_not_nil node_template
assert_equal OldNode.where(:node_id => node_template.id).count, 1
assert_equal(1, OldNode.where(:node_id => node_template.id).count)
assert_not_nil node
node_template.lat = 12.3456
@ -121,7 +121,7 @@ class NodeTest < ActiveSupport::TestCase
assert_equal node_template.visible, node.visible
# assert_equal node_template.tags, node.tags
assert_equal OldNode.where(:node_id => node_template.id).count, 2
assert_equal(2, OldNode.where(:node_id => node_template.id).count)
old_node = OldNode.where(:node_id => node_template.id, :version => 2).first
assert_not_nil old_node
assert_equal node_template.latitude, old_node.latitude
@ -137,7 +137,7 @@ class NodeTest < ActiveSupport::TestCase
node_template = Node.find(node.id)
assert_not_nil node_template
assert_equal OldNode.where(:node_id => node_template.id).count, 1
assert_equal(1, OldNode.where(:node_id => node_template.id).count)
assert_not_nil node
assert node.delete_with_history!(node_template, node.changeset.user)
@ -150,7 +150,7 @@ class NodeTest < ActiveSupport::TestCase
assert_not node.visible
# assert_equal node_template.tags, node.tags
assert_equal OldNode.where(:node_id => node_template.id).count, 2
assert_equal(2, OldNode.where(:node_id => node_template.id).count)
old_node = OldNode.where(:node_id => node_template.id, :version => 2).first
assert_not_nil old_node
assert_equal node_template.latitude, old_node.latitude

4
test/models/trace_test.rb
View file

@ -222,7 +222,7 @@ class TraceTest < ActiveSupport::TestCase
trace.import
assert File.exist?(icon_path)
assert_path_exists(icon_path)
end
def test_import_creates_large_picture
@ -233,7 +233,7 @@ class TraceTest < ActiveSupport::TestCase
trace.import
assert File.exist?(large_picture_path)
assert_path_exists(large_picture_path)
end
def test_import_handles_bz2

2
test/system/issues_test.rb
View file

@ -83,7 +83,7 @@ class IssuesTest < ApplicationSystemTestCase
assert page.has_content?("test comment")
issue.reload
assert_equal issue.comments.first.body, "test comment"
assert_equal("test comment", issue.comments.first.body)
end
def test_reassign_issue

2
test/test_helper.rb
View file

@ -175,7 +175,7 @@ module ActiveSupport
# when the owner of the changset has their data not marked as public
def assert_require_public_data(msg = "Shouldn't be able to use API when the user's data is not public")
assert_response :forbidden, msg
assert_equal @response.headers["Error"], "You must make your edits public to upload new data", "Wrong error message"
assert_equal("You must make your edits public to upload new data", @response.headers["Error"], "Wrong error message")
end
##