Fix new rubocop warnings
authorTom Hughes <tom@compton.nu>
Mon, 22 Jan 2018 14:36:16 +0000 (14:36 +0000)
committerTom Hughes <tom@compton.nu>
Mon, 22 Jan 2018 18:55:45 +0000 (18:55 +0000)
37 files changed:
.rubocop.yml
.rubocop_todo.yml
app/controllers/amf_controller.rb
app/controllers/api_controller.rb
app/controllers/application_controller.rb
app/controllers/changeset_controller.rb
app/controllers/diary_entry_controller.rb
app/controllers/geocoder_controller.rb
app/controllers/node_controller.rb
app/controllers/notes_controller.rb
app/controllers/oauth_controller.rb
app/controllers/relation_controller.rb
app/controllers/site_controller.rb
app/controllers/trace_controller.rb
app/controllers/user_controller.rb
app/helpers/application_helper.rb
app/helpers/browse_helper.rb
app/models/relation.rb
app/models/user.rb
app/models/way.rb
app/views/user/api_read.builder
config/application.rb
config/environments/development.rb
config/environments/production.rb
config/initializers/cors.rb
config/initializers/omniauth.rb
db/migrate/021_move_to_innodb.rb
lib/bounding_box.rb
lib/diff_reader.rb
lib/session_persistence.rb
test/controllers/amf_controller_test.rb
test/controllers/changeset_controller_test.rb
test/controllers/node_controller_test.rb
test/integration/user_blocks_test.rb
test/integration/user_terms_seen_test.rb
test/lib/i18n_test.rb
test/test_helper.rb

index 55be8141cf31c53115cecdcbe08ffd8d266b9089..5e7be9797e8ebdef5de86cb4a3ff65acdf264cee 100644 (file)
@@ -41,12 +41,18 @@ Naming/FileName:
 Rails/ApplicationRecord:
   Enabled: false
 
+Rails/CreateTableWithTimestamps:
+  Enabled: false
+
 Rails/HasManyOrHasOneDependent:
   Enabled: false
 
 Rails/HttpPositionalArguments:
   Enabled: false
 
+Rails/InverseOf:
+  Enabled: false
+
 Rails/SkipsModelValidations:
   Exclude:
     - 'db/migrate/*.rb'
index b4104079cb75734dd1f5802d9a4401482fe7e598..3ebd4d35f686bb0f4252f2689524b342ea3406ba 100644 (file)
@@ -55,11 +55,6 @@ Lint/InterpolationCheck:
   Exclude:
     - 'test/controllers/node_controller_test.rb'
 
-# Offense count: 2
-Lint/RescueWithoutErrorClass:
-  Exclude:
-    - 'app/helpers/browse_helper.rb'
-
 # Offense count: 2
 Lint/ShadowingOuterLocalVariable:
   Exclude:
@@ -227,3 +222,8 @@ Style/NumericLiterals:
 # SupportedStyles: compact, exploded
 Style/RaiseArgs:
   Enabled: false
+
+# Offense count: 2
+Style/RescueStandardError:
+  Exclude:
+    - 'app/helpers/browse_helper.rb'
index 4c24a1cec79ecf16e9a2580aec40ac8c4fbfe35d..9f909ea1089940364388a4a95fb858321d5a4fc4 100644 (file)
@@ -508,14 +508,10 @@ class AmfController < ApplicationController
     rels = []
     if searchterm.to_i > 0
       rel = Relation.where(:id => searchterm.to_i).first
-      if rel && rel.visible
-        rels.push([rel.id, rel.tags, rel.members, rel.version])
-      end
+      rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel && rel.visible
     else
       RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t|
-        if t.relation.visible
-          rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version])
-        end
+        rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) if t.relation.visible
       end
     end
     rels
@@ -558,9 +554,7 @@ class AmfController < ApplicationController
             mid = renumberednodes[mid] if m[0] == "Node"
             mid = renumberedways[mid] if m[0] == "Way"
           end
-          if mid
-            typedmembers << [m[0], mid, m[2].delete("\000-\037\ufffe\uffff", "^\011\012\015")]
-          end
+          typedmembers << [m[0], mid, m[2].delete("\000-\037\ufffe\uffff", "^\011\012\015")] if mid
         end
 
         # assign new contents
@@ -748,9 +742,7 @@ class AmfController < ApplicationController
             return [-4, "node", id]
           end
 
-          unless visible || node.ways.empty?
-            return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version
-          end
+          return -1, "Point #{id} has since become part of a way, so you cannot save it as a POI.", id, id, version unless visible || node.ways.empty?
         end
         # We always need a new node, based on the data that has been sent to us
         new_node = Node.new
@@ -793,9 +785,7 @@ class AmfController < ApplicationController
       n = Node.where(:id => id).first
       if n
         v = n.version
-        unless timestamp == ""
-          n = OldNode.where("node_id = ? AND timestamp <= ?", id, timestamp).unredacted.order("timestamp DESC").first
-        end
+        n = OldNode.where("node_id = ? AND timestamp <= ?", id, timestamp).unredacted.order("timestamp DESC").first unless timestamp == ""
       end
 
       if n
index c6cc3ba5f46cbaf71412b7e414e3f48a0363e2b8..f9b48cb1c5281800d49d5facae8ea0100d94525d 100644 (file)
@@ -157,9 +157,7 @@ class ApiController < ApplicationController
     # - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this
     nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0]
 
-    unless nodes_to_fetch.empty?
-      nodes += Node.includes(:node_tags).find(nodes_to_fetch)
-    end
+    nodes += Node.includes(:node_tags).find(nodes_to_fetch) unless nodes_to_fetch.empty?
 
     visible_nodes = {}
     changeset_cache = {}
index a51dad8755f7ad8494cf919e9e86b7b94a18f595..a24df48e08b0dcc8da6b2d54fb7c078ff3da4172 100644 (file)
@@ -29,9 +29,7 @@ class ApplicationController < ActionController::Base
         end
       end
     elsif session[:token]
-      if self.current_user = User.authenticate(:token => session[:token])
-        session[:user] = current_user.id
-      end
+      session[:user] = current_user.id if self.current_user = User.authenticate(:token => session[:token])
     end
   rescue StandardError => ex
     logger.info("Exception authorizing user: #{ex}")
@@ -381,9 +379,7 @@ class ApplicationController < ActionController::Base
   ##
   # ensure that there is a "this_user" instance variable
   def lookup_this_user
-    unless @this_user = User.active.find_by(:display_name => params[:display_name])
-      render_unknown_user params[:display_name]
-    end
+    render_unknown_user params[:display_name] unless @this_user = User.active.find_by(:display_name => params[:display_name])
   end
 
   ##
@@ -469,9 +465,7 @@ class ApplicationController < ActionController::Base
       authdata = request.env["HTTP_AUTHORIZATION"].to_s.split
     end
     # only basic authentication supported
-    if authdata && authdata[0] == "Basic"
-      user, pass = Base64.decode64(authdata[1]).split(":", 2)
-    end
+    user, pass = Base64.decode64(authdata[1]).split(":", 2) if authdata && authdata[0] == "Basic"
     [user, pass]
   end
 
index 8fbbe1362605992668f143909d89d88a83f83550..f294d23d59026c31b931c04c9f9b97d8bdfb849f 100644 (file)
@@ -7,12 +7,12 @@ class ChangesetController < ApplicationController
   skip_before_action :verify_authenticity_token, :except => [:list]
   before_action :authorize_web, :only => [:list, :feed, :comments_feed]
   before_action :set_locale, :only => [:list, :feed, :comments_feed]
-  before_action :authorize, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_action :authorize, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
   before_action :require_moderator, :only => [:hide_comment, :unhide_comment]
-  before_action :require_allow_write_api, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_action :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe]
-  before_action :check_api_writable, :only => [:create, :update, :delete, :upload, :include, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_action :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed]
+  before_action :require_allow_write_api, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_action :require_public_data, :only => [:create, :update, :upload, :close, :comment, :subscribe, :unsubscribe]
+  before_action :check_api_writable, :only => [:create, :update, :upload, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed]
   before_action(:only => [:list, :feed, :comments_feed]) { |c| c.check_database_readable(true) }
   around_action :api_call_handle_error, :except => [:list, :feed, :comments_feed]
   around_action :api_call_timeout, :except => [:list, :feed, :comments_feed, :upload]
@@ -296,9 +296,7 @@ class ChangesetController < ApplicationController
         changesets = changesets.where(:user_id => current_user.nearby)
       end
 
-      if @params[:max_id]
-        changesets = changesets.where("changesets.id <= ?", @params[:max_id])
-      end
+      changesets = changesets.where("changesets.id <= ?", @params[:max_id]) if @params[:max_id]
 
       @edits = changesets.order("changesets.id DESC").limit(20).preload(:user, :changeset_tags, :comments)
 
@@ -334,9 +332,7 @@ class ChangesetController < ApplicationController
 
     # Notify current subscribers of the new comment
     changeset.subscribers.visible.each do |user|
-      if current_user != user
-        Notifier.changeset_comment_notification(comment, user).deliver_now
-      end
+      Notifier.changeset_comment_notification(comment, user).deliver_now if current_user != user
     end
 
     # Add the commenter to the subscribers if necessary
index 9e0fd4991bd4f62b6ec60e295efc44cf69aa6b49..6eb662bd843ec44370700a264b8c8c54eb229ec7 100644 (file)
@@ -65,9 +65,7 @@ class DiaryEntryController < ApplicationController
 
       # Notify current subscribers of the new comment
       @entry.subscribers.visible.each do |user|
-        if current_user != user
-          Notifier.diary_comment_notification(@diary_comment, user).deliver_now
-        end
+        Notifier.diary_comment_notification(@diary_comment, user).deliver_now if current_user != user
       end
 
       # Add the commenter to the subscribers if necessary
index b9bde31abc4df3ecb43938bba5fd728f5d4c2d07..e9fa7f26af847ca7e61b27710ff4e787f83ea359 100644 (file)
@@ -112,9 +112,7 @@ class GeocoderController < ApplicationController
     maxlat = params[:maxlat]
 
     # get view box
-    if minlon && minlat && maxlon && maxlat
-      viewbox = "&viewbox=#{minlon},#{maxlat},#{maxlon},#{minlat}"
-    end
+    viewbox = "&viewbox=#{minlon},#{maxlat},#{maxlon},#{minlat}" if minlon && minlat && maxlon && maxlat
 
     # get objects to excude
     exclude = "&exclude_place_ids=#{params[:exclude]}" if params[:exclude]
@@ -153,9 +151,7 @@ class GeocoderController < ApplicationController
         rank = (place.attributes["place_rank"].to_i + 1) / 2
         prefix_name = t "geocoder.search_osm_nominatim.admin_levels.level#{rank}", :default => prefix_name
         place.elements["extratags"].elements.each("tag") do |extratag|
-          if extratag.attributes["key"] == "place"
-            prefix_name = t "geocoder.search_osm_nominatim.prefix.place.#{extratag.attributes['value']}", :default => prefix_name
-          end
+          prefix_name = t "geocoder.search_osm_nominatim.prefix.place.#{extratag.attributes['value']}", :default => prefix_name if extratag.attributes["key"] == "place"
         end
       end
       prefix = t "geocoder.search_osm_nominatim.prefix_format", :name => prefix_name
index 29651bceb5dc5d13e97260904ccdef3f9f8b31be..20baf6bb4d0285575e8faa14d92ce08fc23e4bd8 100644 (file)
@@ -40,9 +40,7 @@ class NodeController < ApplicationController
     node = Node.find(params[:id])
     new_node = Node.from_xml(request.raw_post)
 
-    unless new_node && new_node.id == node.id
-      raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})"
-    end
+    raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id
 
     node.update_from(new_node, current_user)
     render :plain => node.version.to_s
@@ -55,24 +53,18 @@ class NodeController < ApplicationController
     node = Node.find(params[:id])
     new_node = Node.from_xml(request.raw_post)
 
-    unless new_node && new_node.id == node.id
-      raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})"
-    end
+    raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id
     node.delete_with_history!(new_node, current_user)
     render :plain => node.version.to_s
   end
 
   # Dump the details on many nodes whose ids are given in the "nodes" parameter.
   def nodes
-    unless params["nodes"]
-      raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]"
-    end
+    raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" unless params["nodes"]
 
     ids = params["nodes"].split(",").collect(&:to_i)
 
-    if ids.empty?
-      raise OSM::APIBadUserInput, "No nodes were given to search for"
-    end
+    raise OSM::APIBadUserInput, "No nodes were given to search for" if ids.empty?
     doc = OSM::API.new.get_xml_doc
 
     Node.find(ids).each do |node|
index 92f63e304f76c539b009cdbbfff4d593e8236447..a51d70f9030300056b73f5e532ed73523524211f 100644 (file)
@@ -350,9 +350,7 @@ class NotesController < ApplicationController
     comment = note.comments.create!(attributes)
 
     note.comments.map(&:author).uniq.each do |user|
-      if notify && user && user != current_user && user.visible?
-        Notifier.note_comment_notification(comment, user).deliver_now
-      end
+      Notifier.note_comment_notification(comment, user).deliver_now if notify && user && user != current_user && user.visible?
     end
   end
 end
index 734783a35eef61f4df429abfee6b1aa2fd9facf8..84bbcf1850e5c95cf6d6d925c0c009b3f7e83315 100644 (file)
@@ -63,9 +63,7 @@ class OauthController < ApplicationController
                                     "&oauth_token=#{@token.token}"
                                 end
 
-          unless @token.oauth10?
-            @redirect_url.query += "&oauth_verifier=#{@token.verifier}"
-          end
+          @redirect_url.query += "&oauth_verifier=#{@token.verifier}" unless @token.oauth10?
 
           redirect_to @redirect_url.to_s
         end
index 25532a95c72c0dc965f0ddeb1757099a7755773f..059fb8d7e5bc32a7dca26128a705b800c4b0c16d 100644 (file)
@@ -35,9 +35,7 @@ class RelationController < ApplicationController
     relation = Relation.find(params[:id])
     new_relation = Relation.from_xml(request.raw_post)
 
-    unless new_relation && new_relation.id == relation.id
-      raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})"
-    end
+    raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})" unless new_relation && new_relation.id == relation.id
 
     relation.update_from new_relation, current_user
     render :plain => relation.version.to_s
@@ -123,15 +121,11 @@ class RelationController < ApplicationController
   end
 
   def relations
-    unless params["relations"]
-      raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]"
-    end
+    raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"]
 
     ids = params["relations"].split(",").collect(&:to_i)
 
-    if ids.empty?
-      raise OSM::APIBadUserInput, "No relations were given to search for"
-    end
+    raise OSM::APIBadUserInput, "No relations were given to search for" if ids.empty?
 
     doc = OSM::API.new.get_xml_doc
 
index 74dbc41f553c2a652661b762f68190dc1a92bc7c..fa8ec5a1ee0478036750be78dec10150fd391d0e 100644 (file)
@@ -11,9 +11,7 @@ class SiteController < ApplicationController
   before_action :update_totp, :only => [:index]
 
   def index
-    unless STATUS == :database_readonly || STATUS == :database_offline
-      session[:location] ||= OSM.ip_location(request.env["REMOTE_ADDR"])
-    end
+    session[:location] ||= OSM.ip_location(request.env["REMOTE_ADDR"]) unless STATUS == :database_readonly || STATUS == :database_offline
   end
 
   def permalink
@@ -147,9 +145,7 @@ class SiteController < ApplicationController
   def redirect_map_params
     anchor = []
 
-    if params[:lat] && params[:lon]
-      anchor << "map=#{params.delete(:zoom) || 5}/#{params.delete(:lat)}/#{params.delete(:lon)}"
-    end
+    anchor << "map=#{params.delete(:zoom) || 5}/#{params.delete(:lat)}/#{params.delete(:lon)}" if params[:lat] && params[:lon]
 
     if params[:layers]
       anchor << "layers=#{params.delete(:layers)}"
@@ -157,8 +153,6 @@ class SiteController < ApplicationController
       anchor << "layers=N"
     end
 
-    if anchor.present?
-      redirect_to params.to_unsafe_h.merge(:anchor => anchor.join("&"))
-    end
+    redirect_to params.to_unsafe_h.merge(:anchor => anchor.join("&")) if anchor.present?
   end
 end
index 105405ccfdad7e0fd11b525443a4786ed496f307..a720c5fff438dc1ed4e8d9152e85dacf44ed4cc1 100644 (file)
@@ -119,9 +119,7 @@ class TraceController < ApplicationController
         if @trace.id
           flash[:notice] = t "trace.create.trace_uploaded"
 
-          if current_user.traces.where(:inserted => false).count > 4
-            flash[:warning] = t "trace.trace_header.traces_waiting", :count => current_user.traces.where(:inserted => false).count
-          end
+          flash[:warning] = t "trace.trace_header.traces_waiting", :count => current_user.traces.where(:inserted => false).count if current_user.traces.where(:inserted => false).count > 4
 
           redirect_to :action => :list, :display_name => current_user.display_name
         end
@@ -176,9 +174,7 @@ class TraceController < ApplicationController
         @trace.description = params[:trace][:description]
         @trace.tagstring = params[:trace][:tagstring]
         @trace.visibility = params[:trace][:visibility]
-        if @trace.save
-          redirect_to :action => "view", :display_name => current_user.display_name
-        end
+        redirect_to :action => "view", :display_name => current_user.display_name if @trace.save
       end
     end
   rescue ActiveRecord::RecordNotFound
@@ -205,9 +201,7 @@ class TraceController < ApplicationController
   def georss
     @traces = Trace.visible_to_all.visible
 
-    if params[:display_name]
-      @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] })
-    end
+    @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] }) if params[:display_name]
 
     @traces = @traces.tagged(params[:tag]) if params[:tag]
     @traces = @traces.order("timestamp DESC")
index 5c41a79dc84da7f336d7ed0152094571c0e0fe8a..0c3ad0b05e31444fee19e1aa82318525da740b34 100644 (file)
@@ -45,9 +45,7 @@ class UserController < ApplicationController
       if current_user
         current_user.terms_seen = true
 
-        if current_user.save
-          flash[:notice] = t("user.new.terms declined", :url => t("user.new.terms declined url")).html_safe
-        end
+        flash[:notice] = t("user.new.terms declined", :url => t("user.new.terms declined url")).html_safe if current_user.save
 
         if params[:referer]
           redirect_to params[:referer]
@@ -533,9 +531,7 @@ class UserController < ApplicationController
       session[:new_user].auth_provider = provider
       session[:new_user].auth_uid = uid
 
-      if email_verified && email == session[:new_user].email
-        session[:new_user].status = "active"
-      end
+      session[:new_user].status = "active" if email_verified && email == session[:new_user].email
 
       redirect_to :action => "terms"
     else
index a330eb5f97d47a19d98d874d3681384219e82d54..adcf5c6c011ec6fdcc31821b2c0af3b74065c083 100644 (file)
@@ -40,9 +40,7 @@ module ApplicationHelper
   end
 
   def if_user(user, tag = :div, &block)
-    if user
-      content_tag(tag, capture(&block), :class => "hidden show_if_user_#{user.id}")
-    end
+    content_tag(tag, capture(&block), :class => "hidden show_if_user_#{user.id}") if user
   end
 
   def unless_user(user, tag = :div, &block)
@@ -110,9 +108,7 @@ module ApplicationHelper
     if current_user
       data[:user] = current_user.id.to_json
 
-      unless current_user.home_lon.nil? || current_user.home_lat.nil?
-        data[:user_home] = { :lat => current_user.home_lat, :lon => current_user.home_lon }
-      end
+      data[:user_home] = { :lat => current_user.home_lat, :lon => current_user.home_lon } unless current_user.home_lon.nil? || current_user.home_lat.nil?
     end
 
     data[:location] = session[:location] if session[:location]
index c2e974e9365f406c9ce136679af744ea6f936e1f..b90e27f8545b6995fdb0631e7329946b40f6bd27 100644 (file)
@@ -8,18 +8,14 @@ module BrowseHelper
            object.id
          end
     name = t "printable_name.with_id", :id => id.to_s
-    if version
-      name = t "printable_name.with_version", :id => name, :version => object.version.to_s
-    end
+    name = t "printable_name.with_version", :id => name, :version => object.version.to_s if version
 
     # don't look at object tags if redacted, so as to avoid giving
     # away redacted version tag information.
     unless object.redacted?
       locale = I18n.locale.to_s
 
-      while locale =~ /-[^-]+/ && !object.tags.include?("name:#{I18n.locale}")
-        locale = locale.sub(/-[^-]+/, "")
-      end
+      locale = locale.sub(/-[^-]+/, "") while locale =~ /-[^-]+/ && !object.tags.include?("name:#{I18n.locale}")
 
       if object.tags.include? "name:#{locale}"
         name = t "printable_name.with_name_html", :name => content_tag(:bdi, object.tags["name:#{locale}"].to_s), :id => content_tag(:bdi, name)
index 157794cd69cee7424771a248b0331b569f6d32d8..2495830eefaa36972f8921bf1fd8e88cac427879 100644 (file)
@@ -182,9 +182,7 @@ class Relation < ActiveRecord::Base
   end
 
   def delete_with_history!(new_relation, user)
-    unless visible
-      raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id)
-    end
+    raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id) unless visible
 
     # need to start the transaction here, so that the database can
     # provide repeatable reads for the used-by checks. this means it
@@ -208,9 +206,7 @@ class Relation < ActiveRecord::Base
     Relation.transaction do
       lock!
       check_consistency(self, new_relation, user)
-      unless new_relation.preconditions_ok?(members)
-        raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid."
-      end
+      raise OSM::APIPreconditionFailedError, "Cannot update relation #{id}: data or member data is invalid." unless new_relation.preconditions_ok?(members)
       self.changeset_id = new_relation.changeset_id
       self.changeset = new_relation.changeset
       self.tags = new_relation.tags
@@ -222,9 +218,7 @@ class Relation < ActiveRecord::Base
 
   def create_with_history(user)
     check_create_consistency(self, user)
-    unless preconditions_ok?
-      raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid."
-    end
+    raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." unless preconditions_ok?
     self.version = 0
     self.visible = true
     save_with_history!
@@ -259,9 +253,7 @@ class Relation < ActiveRecord::Base
       element = model.lock("for share").find_by(:id => m[1])
 
       # and check that it is OK to use.
-      unless element && element.visible? && element.preconditions_ok?
-        raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}"
-      end
+      raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" unless element && element.visible? && element.preconditions_ok?
       hash[m[1]] = true
     end
 
index 7a8414ec073c177940ad9601511467d7d89c96e2..9eeb98290360d58cda7c759ec7b8dc37e6fb754e 100644 (file)
@@ -277,9 +277,7 @@ class User < ActiveRecord::Base
   ##
   # perform a spam check on a user
   def spam_check
-    if status == "active" && spam_score > SPAM_THRESHOLD
-      update(:status => "suspended")
-    end
+    update(:status => "suspended") if status == "active" && spam_score > SPAM_THRESHOLD
   end
 
   ##
index 1954f744ca9332d1c45c7bd133e27f9b7b24bed2..e5b73ceaaab009db0ef5855ec38c4f108d6dbce4 100644 (file)
@@ -120,14 +120,10 @@ class Way < ActiveRecord::Base
     way_nodes.each do |nd|
       if visible_nodes
         # if there is a list of visible nodes then use that to weed out deleted nodes
-        if visible_nodes[nd.node_id]
-          ordered_nodes[nd.sequence_id] = nd.node_id.to_s
-        end
+        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if visible_nodes[nd.node_id]
       else
         # otherwise, manually go to the db to check things
-        if nd.node && nd.node.visible?
-          ordered_nodes[nd.sequence_id] = nd.node_id.to_s
-        end
+        ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node && nd.node.visible?
       end
     end
 
@@ -184,9 +180,7 @@ class Way < ActiveRecord::Base
     Way.transaction do
       lock!
       check_consistency(self, new_way, user)
-      unless new_way.preconditions_ok?(nds)
-        raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid."
-      end
+      raise OSM::APIPreconditionFailedError, "Cannot update way #{id}: data is invalid." unless new_way.preconditions_ok?(nds)
 
       self.changeset_id = new_way.changeset_id
       self.changeset = new_way.changeset
@@ -199,9 +193,7 @@ class Way < ActiveRecord::Base
 
   def create_with_history(user)
     check_create_consistency(self, user)
-    unless preconditions_ok?
-      raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid."
-    end
+    raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." unless preconditions_ok?
     self.version = 0
     self.visible = true
     save_with_history!
@@ -209,9 +201,7 @@ class Way < ActiveRecord::Base
 
   def preconditions_ok?(old_nodes = [])
     return false if nds.empty?
-    if nds.length > MAX_NUMBER_OF_WAY_NODES
-      raise OSM::APITooManyWayNodesError.new(id, nds.length, MAX_NUMBER_OF_WAY_NODES)
-    end
+    raise OSM::APITooManyWayNodesError.new(id, nds.length, MAX_NUMBER_OF_WAY_NODES) if nds.length > MAX_NUMBER_OF_WAY_NODES
 
     # check only the new nodes, for efficiency - old nodes having been checked last time and can't
     # be deleted when they're in-use.
index 0e8bfa5eff34f1e5358fb75ceab8c6e36cf16558..fe5af4bcf6cfa547c108fec36eb9270a4cdfe0a4 100644 (file)
@@ -10,9 +10,7 @@ xml.osm("version" => API_VERSION, "generator" => GENERATOR) do
     else
       xml.tag! "contributor-terms", :agreed => @this_user.terms_agreed.present?
     end
-    if @this_user.image.file? || @this_user.image_use_gravatar
-      xml.tag! "img", :href => user_image_url(@this_user, :size => 256)
-    end
+    xml.tag! "img", :href => user_image_url(@this_user, :size => 256) if @this_user.image.file? || @this_user.image_use_gravatar
     xml.tag! "roles" do
       @this_user.roles.each do |role|
         xml.tag! role.role
index 02dd1d2b296867520fcdc7c0f521d2c1e025ab1a..9ba122dafb8b1518af4930f92ceec945befff7ec 100644 (file)
@@ -34,9 +34,7 @@ module OpenStreetMap
     config.paths["app/models"].skip_eager_load! if STATUS == :database_offline
 
     # Use memcached for caching if required
-    if defined?(MEMCACHE_SERVERS)
-      config.cache_store = :mem_cache_store, MEMCACHE_SERVERS, { :namespace => "rails:cache" }
-    end
+    config.cache_store = :mem_cache_store, MEMCACHE_SERVERS, { :namespace => "rails:cache" } if defined?(MEMCACHE_SERVERS)
 
     # Use logstash for logging if required
     if defined?(LOGSTASH_PATH)
index 8e7213a9d35dd080ffe4d2798db9d0c7a879afce..97226480c6dfdf1bb688ea94ce59ac1a7b6f127c 100644 (file)
@@ -35,9 +35,7 @@ Rails.application.configure do
   config.active_support.deprecation = :log
 
   # Raise an error on page load if there are pending migrations.
-  unless STATUS == :database_offline
-    config.active_record.migration_error = :page_load
-  end
+  config.active_record.migration_error = :page_load unless STATUS == :database_offline
 
   # Debug mode disables concatenation and preprocessing of assets.
   # This option may cause significant delays in view rendering with a large
index c4c98ad48fc3de6a72bbbf830fac696e998cc5a8..56bd2288109d56ce6bf6fcebf2cfa7f1152b266c 100644 (file)
@@ -93,9 +93,7 @@ Rails.application.configure do
   end
 
   # Do not dump schema after migrations.
-  unless STATUS == :database_offline
-    config.active_record.dump_schema_after_migration = false
-  end
+  config.active_record.dump_schema_after_migration = false unless STATUS == :database_offline
 
   # Enable autoloading of dependencies.
   config.enable_dependency_loading = true
index ee0b0d98fe236f74f9ce1e1a6fdba5fc528a31e6..2bd558d2f3f24ec4f4f9884cb9b1cf4c815505dc 100644 (file)
@@ -7,9 +7,7 @@ module OpenStreetMap
   class Cors < Rack::Cors
     def call(env)
       status, headers, body = super env
-      if headers["Access-Control-Allow-Origin"]
-        headers["Cache-Control"] = "no-cache"
-      end
+      headers["Cache-Control"] = "no-cache" if headers["Access-Control-Allow-Origin"]
       [status, headers, body]
     end
   end
index 180469bfce74309b2f94b2aebeb8a0e41fac947e..7e499c0e6960cc7aa2545d0ada0eb73bfe9274d7 100644 (file)
@@ -26,9 +26,7 @@ windowslive_options = { :name => "windowslive", :scope => "wl.signin,wl.emails"
 github_options = { :name => "github", :scope => "user:email" }
 wikipedia_options = { :name => "wikipedia", :client_options => { :site => "https://meta.wikimedia.org" } }
 
-if defined?(GOOGLE_OPENID_REALM)
-  google_options[:openid_realm] = GOOGLE_OPENID_REALM
-end
+google_options[:openid_realm] = GOOGLE_OPENID_REALM if defined?(GOOGLE_OPENID_REALM)
 
 Rails.application.config.middleware.use OmniAuth::Builder do
   provider :openid, openid_options
index 5be81b69533d6854549cc185416190567e5c7630..3232e27411fe4181c198d425e743042c86bb76b1 100644 (file)
@@ -19,9 +19,7 @@ class MoveToInnodb < ActiveRecord::Migration[5.0]
       # current version to something less so that we can update the version in
       # batches of 10000
       tbl.classify.constantize.update_all(:version => -1)
-      while tbl.classify.constantize.where(:version => -1).count > 0
-        tbl.classify.constantize.update_all("version=(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)", { :version => -1 }, { :limit => 10000 })
-      end
+      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 > 0
       # execute "UPDATE current_#{tbl} SET version = " +
       #  "(SELECT max(version) FROM #{tbl} WHERE #{tbl}.id = current_#{tbl}.id)"
       # The above update causes a MySQL error:
index f12683d3c4831cd99db1ecb51ebb29245989e48a..1fc5dc69698a238821e125d2ae2f26d1dff485bf 100644 (file)
@@ -18,23 +18,17 @@ class BoundingBox
   end
 
   def self.from_bbox_params(params)
-    if params[:bbox] && params[:bbox].count(",") == 3
-      bbox_array = params[:bbox].split(",")
-    end
+    bbox_array = params[:bbox].split(",") if params[:bbox] && params[:bbox].count(",") == 3
     from_bbox_array(bbox_array)
   end
 
   def self.from_lon_lat_params(params)
-    if params[:minlon] && params[:minlat] && params[:maxlon] && params[:maxlat]
-      bbox_array = [params[:minlon], params[:minlat], params[:maxlon], params[:maxlat]]
-    end
+    bbox_array = [params[:minlon], params[:minlat], params[:maxlon], params[:maxlat]] if params[:minlon] && params[:minlat] && params[:maxlon] && params[:maxlat]
     from_bbox_array(bbox_array)
   end
 
   def self.from_lrbt_params(params)
-    if params[:l] && params[:b] && params[:t] && params[:t]
-      bbox_array = [params[:l], params[:b], params[:r], params[:t]]
-    end
+    bbox_array = [params[:l], params[:b], params[:r], params[:t]] if params[:l] && params[:b] && params[:t] && params[:t]
     from_bbox_array(bbox_array)
   end
 
@@ -65,12 +59,8 @@ class BoundingBox
 
   def check_boundaries
     # check the bbox is sane
-    if min_lon > max_lon
-      raise OSM::APIBadBoundingBox, "The minimum longitude must be less than the maximum longitude, but it wasn't"
-    end
-    if min_lat > max_lat
-      raise OSM::APIBadBoundingBox, "The minimum latitude must be less than the maximum latitude, but it wasn't"
-    end
+    raise OSM::APIBadBoundingBox, "The minimum longitude must be less than the maximum longitude, but it wasn't" if min_lon > max_lon
+    raise OSM::APIBadBoundingBox, "The minimum latitude must be less than the maximum latitude, but it wasn't" if min_lat > max_lat
     if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT
       raise OSM::APIBadBoundingBox, "The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," \
                                        " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}"
@@ -166,9 +156,7 @@ class BoundingBox
     private
 
     def from_bbox_array(bbox_array)
-      unless bbox_array
-        raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat"
-      end
+      raise OSM::APIBadUserInput, "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array
       # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and
       # max_lat within their respective boundaries.
       min_lon = [[bbox_array[0].to_f, -LON_LIMIT].max, +LON_LIMIT].min
index 94c41a6d57751be96c3497fb99ffefb7d85e4229..cb553bfbcad71a095c0940f61b3fb5341d1d4712 100644 (file)
@@ -60,9 +60,7 @@ class DiffReader
           attributes = {}
 
           if @reader.has_attributes?
-            while @reader.move_to_next_attribute == 1
-              attributes[@reader.name] = @reader.value
-            end
+            attributes[@reader.name] = @reader.value while @reader.move_to_next_attribute == 1
 
             @reader.move_to_element
           end
@@ -112,9 +110,7 @@ class DiffReader
   # such as save_ and delete_with_history.
   def check(model, xml, new)
     raise OSM::APIBadXMLError.new(model, xml) if new.nil?
-    unless new.changeset_id == @changeset.id
-      raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id)
-    end
+    raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id) unless new.changeset_id == @changeset.id
   end
 
   ##
index 5e933f48a39151360ad45616866d13e8cbecb3c1..6bd05f5ce7fb58a74ad013c95f23f9d8d3e822c7 100644 (file)
@@ -52,9 +52,7 @@ module SessionPersistence
 
   # Filter callback
   def persist_session
-    if session[session_persistence_key]
-      request.session_options[:expire_after] = session[session_persistence_key]
-    end
+    request.session_options[:expire_after] = session[session_persistence_key] if session[session_persistence_key]
   rescue StandardError
     reset_session
   end
index 83ef03bbf95cc4fb1d31dc1d2169eb5c6153d1d7..bd7a518843ef8237fa6140773e199e2fa0f92818 100644 (file)
@@ -1,8 +1,9 @@
 require "test_helper"
 require "stringio"
-include Potlatch
 
 class AmfControllerTest < ActionController::TestCase
+  include Potlatch
+
   ##
   # test all routes which lead to this controller
   def test_routes
@@ -730,8 +731,8 @@ class AmfControllerTest < ActionController::TestCase
     # This node has no tags
 
     # create a node with random lat/lon
-    lat = rand(100) - 50 + rand
-    lon = rand(100) - 50 + rand
+    lat = rand(-50..49) + rand
+    lon = rand(-50..49) + rand
 
     changeset = create(:changeset)
     user = changeset.user
@@ -770,8 +771,8 @@ class AmfControllerTest < ActionController::TestCase
     # This node has some tags
 
     # create a node with random lat/lon
-    lat = rand(100) - 50 + rand
-    lon = rand(100) - 50 + rand
+    lat = rand(-50..49) + rand
+    lon = rand(-50..49) + rand
 
     amf_content "putpoi", "/2", ["#{user.email}:test", changeset.id, nil, nil, lon, lat, { "key" => "value", "ping" => "pong" }, nil]
     post :amf_write
@@ -811,8 +812,8 @@ class AmfControllerTest < ActionController::TestCase
     # This node has no tags
 
     # create a node with random lat/lon
-    lat = rand(100) - 50 + rand
-    lon = rand(100) - 50 + rand
+    lat = rand(-50..49) + rand
+    lon = rand(-50..49) + rand
 
     changeset = create(:changeset)
     user = changeset.user
@@ -847,8 +848,8 @@ class AmfControllerTest < ActionController::TestCase
     # This node has no tags
 
     # create a node with random lat/lon
-    lat = rand(100) - 50 + rand
-    lon = rand(100) - 50 + rand
+    lat = rand(-50..49) + rand
+    lon = rand(-50..49) + rand
 
     changeset = create(:changeset)
     user = changeset.user
index a0f7960c31b7197702ac2927734853e7996c13b3..5205714df9171ca72232ce460438b4fe72a0c3db 100644 (file)
@@ -812,9 +812,7 @@ CHANGESET
     assert_equal 2, Node.find(new_node_id).tags.size, "new node should have two tags"
     assert_equal [new_node_id, node.id], Way.find(way.id).nds, "way nodes should match"
     Relation.find(relation.id).members.each do |type, id, _role|
-      if type == "node"
-        assert_equal new_node_id, id, "relation should contain new node"
-      end
+      assert_equal new_node_id, id, "relation should contain new node" if type == "node"
     end
   end
 
index 42b63bdf64910678dfa0c103aeff4ffe3f784ce4..5f737f798a57c26a0a102739bfe62a013baa2ed7 100644 (file)
@@ -33,8 +33,8 @@ class NodeControllerTest < ActionController::TestCase
     changeset = create(:changeset, :user => user)
 
     # create a node with random lat/lon
-    lat = rand(100) - 50 + rand
-    lon = rand(100) - 50 + rand
+    lat = rand(-50..50) + rand
+    lon = rand(-50..50) + rand
 
     ## First try with no auth
     # create a minimal xml file
index 4048d53034863c614a273becad8d97e98db43c08..9381365920a8ab116efbb65ee7cedf2f70bc12ac 100644 (file)
@@ -2,7 +2,7 @@ require "test_helper"
 
 class UserBlocksTest < ActionDispatch::IntegrationTest
   def auth_header(user, pass)
-    { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) }
+    { "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) }
   end
 
   def test_api_blocked
index dcc7fa7e995e8395081544e6861c3842e0260097..7ec7fa7307248e038f47bfa118c5dcd27cfd4297 100644 (file)
@@ -74,7 +74,7 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest
   private
 
   def auth_header(user, pass)
-    { "HTTP_AUTHORIZATION" => format("Basic %s", Base64.encode64("#{user}:#{pass}")) }
+    { "HTTP_AUTHORIZATION" => format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) }
   end
 
   def with_terms_seen(value)
index 689f65097530a481749237099d14f9679a3e5c6a..3aa528ceb819db6244b9e54b7b971d005657cb23 100644 (file)
@@ -24,9 +24,7 @@ class I18nTest < ActiveSupport::TestCase
           end
         end
 
-        if key =~ /^(active(model|record)\.)?errors\./
-          variables.push("attribute")
-        end
+        variables.push("attribute") if key =~ /^(active(model|record)\.)?errors\./
 
         value = I18n.t(key, :locale => locale, :fallback => true)
 
index 70f69a3ae94a590140eebd1a97fa15f46437775c..c9ec46dcf41cf1b78c9cab334b8027dde167aa0e 100644 (file)
@@ -82,7 +82,7 @@ module ActiveSupport
     ##
     # set request headers for HTTP basic authentication
     def basic_authorization(user, pass)
-      @request.env["HTTP_AUTHORIZATION"] = format("Basic %s", Base64.encode64("#{user}:#{pass}"))
+      @request.env["HTTP_AUTHORIZATION"] = format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}"))
     end
 
     ##