Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/pull/2955' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
tomhughes committed Nov 11, 2020
2 parents a2bb438 + 182d188 commit f614eae
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 54 deletions.
6 changes: 0 additions & 6 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -176,18 +176,12 @@ Style/NumericLiterals:
# Offense count: 20
Style/OptionalBooleanParameter:
Exclude:
- 'app/controllers/api/notes_controller.rb'
- 'app/controllers/application_controller.rb'
- 'app/helpers/browse_helper.rb'
- 'app/models/changeset.rb'
- 'app/models/node.rb'
- 'app/models/relation.rb'
- 'app/models/trace.rb'
- 'app/models/tracepoint.rb'
- 'app/models/way.rb'
- 'test/models/diary_entry_test.rb'
- 'test/models/trace_test.rb'
- 'test/models/tracetag_test.rb'

# Offense count: 28
# Cop supports --auto-correct.
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/notes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def destroy
@note.status = "hidden"
@note.save

add_comment(@note, comment, "hidden", false)
add_comment(@note, comment, "hidden", :notify => false)
end

# Return a copy of the updated note
Expand Down Expand Up @@ -369,7 +369,7 @@ def closed_condition(notes)

##
# Add a comment to a note
def add_comment(note, text, event, notify = true)
def add_comment(note, text, event, notify: true)
attributes = { :visible => true, :event => event, :body => text }

if current_user
Expand Down
10 changes: 5 additions & 5 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def require_cookies
end
end

def check_database_readable(need_api = false)
def check_database_readable(need_api: false)
if Settings.status == "database_offline" || (need_api && Settings.status == "api_offline")
if request.xhr?
report_error "Database offline for maintenance", :service_unavailable
Expand All @@ -94,7 +94,7 @@ def check_database_readable(need_api = false)
end
end

def check_database_writable(need_api = false)
def check_database_writable(need_api: false)
if Settings.status == "database_offline" || Settings.status == "database_readonly" ||
(need_api && (Settings.status == "api_offline" || Settings.status == "api_readonly"))
if request.xhr?
Expand Down Expand Up @@ -171,7 +171,7 @@ def report_error(message, status = :bad_request)
end
end

def preferred_languages(reset = false)
def preferred_languages(reset: false)
@preferred_languages = nil if reset
@preferred_languages ||= if params[:locale]
Locale.list(params[:locale])
Expand All @@ -184,13 +184,13 @@ def preferred_languages(reset = false)

helper_method :preferred_languages

def set_locale(reset = false)
def set_locale(reset: false)
if current_user&.languages&.empty? && !http_accept_language.user_preferred_languages.empty?
current_user.languages = http_accept_language.user_preferred_languages
current_user.save
end

I18n.locale = Locale.available.preferred(preferred_languages(reset))
I18n.locale = Locale.available.preferred(preferred_languages(:reset => reset))

response.headers["Vary"] = "Accept-Language"
response.headers["Content-Language"] = I18n.locale.to_s
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/browse_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class BrowseController < ApplicationController

before_action :authorize_web
before_action :set_locale
before_action -> { check_database_readable(true) }
before_action -> { check_database_readable(:need_api => true) }
before_action :require_oauth
around_action :web_timeout
authorize_resource :class => false
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/changeset_comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class ChangesetCommentsController < ApplicationController

authorize_resource

before_action -> { check_database_readable(true) }
before_action -> { check_database_readable(:need_api => true) }
around_action :web_timeout

##
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/changesets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class ChangesetsController < ApplicationController

before_action :authorize_web
before_action :set_locale
before_action -> { check_database_readable(true) }, :only => [:index, :feed]
before_action -> { check_database_readable(:need_api => true) }, :only => [:index, :feed]

authorize_resource

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ def update_user(user, params)
if user.save
session[:fingerprint] = user.fingerprint

set_locale(true)
set_locale(:reset => true)

if user.new_email.blank? || user.new_email == user.email
flash.now[:notice] = t "users.account.flash update success"
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/browse_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module BrowseHelper
def printable_name(object, version = false)
def printable_name(object, version: false)
id = if object.id.is_a?(Array)
object.id[0]
else
Expand Down
6 changes: 3 additions & 3 deletions app/views/browse/changeset.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
</h4>
<ul class="list-unstyled">
<% @ways.each do |way| %>
<li><%= link_to printable_name(way, true), { :action => "way", :id => way.way_id.to_s }, { :class => link_class("way", way), :title => link_title(way) } %></li>
<li><%= link_to printable_name(way, :version => true), { :action => "way", :id => way.way_id.to_s }, { :class => link_class("way", way), :title => link_title(way) } %></li>
<% end %>
</ul>
<% end %>
Expand All @@ -102,7 +102,7 @@
</h4>
<ul class="list-unstyled">
<% @relations.each do |relation| %>
<li><%= link_to printable_name(relation, true), { :action => "relation", :id => relation.relation_id.to_s }, { :class => link_class("relation", relation), :title => link_title(relation) } %></li>
<li><%= link_to printable_name(relation, :version => true), { :action => "relation", :id => relation.relation_id.to_s }, { :class => link_class("relation", relation), :title => link_title(relation) } %></li>
<% end %>
</ul>
<% end %>
Expand All @@ -114,7 +114,7 @@
</h4>
<ul class="list-unstyled">
<% @nodes.each do |node| %>
<li><%= link_to printable_name(node, true), { :action => "node", :id => node.node_id.to_s }, { :class => link_class("node", node), :title => link_title(node), :rel => link_follow(node) } %></li>
<li><%= link_to printable_name(node, :version => true), { :action => "node", :id => node.node_id.to_s }, { :class => link_class("node", node), :title => link_title(node), :rel => link_follow(node) } %></li>
<% end %>
</ul>
<% end %>
Expand Down
16 changes: 8 additions & 8 deletions test/helpers/browse_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ def test_printable_name
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
assert_dom_equal node.id.to_s, printable_name(node_v1)
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)

I18n.locale = "pt"
Expand All @@ -41,8 +41,8 @@ def test_printable_name
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
assert_dom_equal node.id.to_s, printable_name(node_v1)
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)

I18n.locale = "pt-BR"
Expand All @@ -51,8 +51,8 @@ def test_printable_name
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
assert_dom_equal node.id.to_s, printable_name(node_v1)
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)

I18n.locale = "de"
Expand All @@ -61,8 +61,8 @@ def test_printable_name
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
assert_dom_equal node.id.to_s, printable_name(node_v1)
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, true)
assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
end

Expand Down
16 changes: 8 additions & 8 deletions test/models/diary_entry_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@ def setup

def test_diary_entry_validations
diary_entry_valid({})
diary_entry_valid({ :title => "" }, false)
diary_entry_valid({ :title => "" }, :valid => false)
diary_entry_valid(:title => "a" * 255)
diary_entry_valid({ :title => "a" * 256 }, false)
diary_entry_valid({ :body => "" }, false)
diary_entry_valid({ :body => "" }, :valid => false)
diary_entry_valid(:latitude => 90)
diary_entry_valid({ :latitude => 90.00001 }, false)
diary_entry_valid({ :latitude => 90.00001 }, :valid => false)
diary_entry_valid(:latitude => -90)
diary_entry_valid({ :latitude => -90.00001 }, false)
diary_entry_valid({ :latitude => -90.00001 }, :valid => false)
diary_entry_valid(:longitude => 180)
diary_entry_valid({ :longitude => 180.00001 }, false)
diary_entry_valid({ :longitude => 180.00001 }, :valid => false)
diary_entry_valid(:longitude => -180)
diary_entry_valid({ :longitude => -180.00001 }, false)
diary_entry_valid({ :longitude => -180.00001 }, :valid => false)
end

def test_diary_entry_visible
Expand Down Expand Up @@ -48,8 +48,8 @@ def test_diary_entry_visible_comments

private

def diary_entry_valid(attrs, result = true)
def diary_entry_valid(attrs, valid: true)
entry = build(:diary_entry, attrs)
assert_equal result, entry.valid?, "Expected #{attrs.inspect} to be #{result}"
assert_equal valid, entry.valid?, "Expected #{attrs.inspect} to be #{valid}"
end
end
14 changes: 7 additions & 7 deletions test/models/trace_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ def test_tagged

def test_validations
trace_valid({})
trace_valid({ :user_id => nil }, false)
trace_valid({ :user_id => nil }, :valid => false)
trace_valid(:name => "a" * 255)
trace_valid({ :name => "a" * 256 }, false)
trace_valid({ :description => nil }, false)
trace_valid({ :name => "a" * 256 }, :valid => false)
trace_valid({ :description => nil }, :valid => false)
trace_valid(:description => "a" * 255)
trace_valid({ :description => "a" * 256 }, false)
trace_valid({ :description => "a" * 256 }, :valid => false)
trace_valid(:visibility => "private")
trace_valid(:visibility => "public")
trace_valid(:visibility => "trackable")
trace_valid(:visibility => "identifiable")
trace_valid({ :visibility => "foo" }, false)
trace_valid({ :visibility => "foo" }, :valid => false)
end

def test_tagstring
Expand Down Expand Up @@ -319,9 +319,9 @@ def check_xml_file(id, md5sum)
assert_equal md5sum, md5sum(create(:trace, :fixture => id).xml_file)
end

def trace_valid(attrs, result = true)
def trace_valid(attrs, valid: true)
entry = build(:trace, attrs)
assert_equal result, entry.valid?, "Expected #{attrs.inspect} to be #{result}"
assert_equal valid, entry.valid?, "Expected #{attrs.inspect} to be #{valid}"
end

def md5sum(io)
Expand Down
20 changes: 10 additions & 10 deletions test/models/tracetag_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,23 @@
class TracetagTest < ActiveSupport::TestCase
def test_validations
tracetag_valid({})
tracetag_valid({ :tag => nil }, false)
tracetag_valid({ :tag => "" }, false)
tracetag_valid({ :tag => nil }, :valid => false)
tracetag_valid({ :tag => "" }, :valid => false)
tracetag_valid(:tag => "a")
tracetag_valid(:tag => "a" * 255)
tracetag_valid({ :tag => "a" * 256 }, false)
tracetag_valid({ :tag => "a/b" }, false)
tracetag_valid({ :tag => "a;b" }, false)
tracetag_valid({ :tag => "a.b" }, false)
tracetag_valid({ :tag => "a,b" }, false)
tracetag_valid({ :tag => "a?b" }, false)
tracetag_valid({ :tag => "a" * 256 }, :valid => false)
tracetag_valid({ :tag => "a/b" }, :valid => false)
tracetag_valid({ :tag => "a;b" }, :valid => false)
tracetag_valid({ :tag => "a.b" }, :valid => false)
tracetag_valid({ :tag => "a,b" }, :valid => false)
tracetag_valid({ :tag => "a?b" }, :valid => false)
end

private

def tracetag_valid(attrs, result = true)
def tracetag_valid(attrs, valid: true)
entry = build(:tracetag)
entry.assign_attributes(attrs)
assert_equal result, entry.valid?, "Expected #{attrs.inspect} to be #{result}"
assert_equal valid, entry.valid?, "Expected #{attrs.inspect} to be #{valid}"
end
end

0 comments on commit f614eae

Please sign in to comment.