From 8fe18995964717ff6f88858291c6f464cc40397f Mon Sep 17 00:00:00 2001
From: Tom Hughes <tom@compton.nu>
Date: Tue, 19 Jan 2016 00:19:09 +0000
Subject: [PATCH] Fix rubocop warnings

---
 .rubocop.yml                                  |  16 ++-
 app/controllers/amf_controller.rb             |  25 ++--
 app/controllers/api_controller.rb             |   6 +-
 app/controllers/application_controller.rb     |  16 +--
 app/controllers/browse_controller.rb          |  10 +-
 app/controllers/changeset_controller.rb       |  61 +++++----
 app/controllers/geocoder_controller.rb        |  44 +++----
 app/controllers/notes_controller.rb           |  18 ++-
 app/controllers/oauth_controller.rb           |  52 ++++----
 app/controllers/trace_controller.rb           |  48 ++++---
 app/controllers/user_controller.rb            |  30 ++---
 app/helpers/browse_helper.rb                  |  28 ++--
 app/helpers/geocoder_helper.rb                |  14 +-
 app/models/changeset.rb                       |  14 +-
 app/models/client_application.rb              |   2 +-
 app/models/node.rb                            |   8 +-
 app/models/note.rb                            |   2 +-
 app/models/notifier.rb                        |  32 ++---
 app/models/old_node.rb                        |   2 +-
 app/models/relation.rb                        |  14 +-
 app/models/request_token.rb                   |   2 +-
 app/models/trace.rb                           |  84 ++++++------
 app/models/user.rb                            |   2 +-
 app/models/user_role.rb                       |   2 +-
 app/models/way.rb                             |   8 +-
 config/initializers/abstract_adapter.rb       |   2 +-
 config/initializers/omniauth.rb               |   2 +-
 config/initializers/streaming.rb              |   2 +-
 config/preinitializer.rb                      |  14 +-
 db/migrate/008_remove_segments.rb             |   2 +-
 db/migrate/023_add_changesets.rb              |   2 +-
 db/migrate/041_add_fine_o_auth_permissions.rb |   2 +-
 lib/auth.rb                                   |   9 +-
 lib/bounding_box.rb                           |  30 ++---
 lib/classic_pagination/pagination.rb          |  16 +--
 lib/classic_pagination/pagination_helper.rb   |  12 +-
 lib/diff_reader.rb                            |   2 +-
 lib/editors.rb                                |   4 +-
 lib/gpx.rb                                    |  18 ++-
 lib/password_hash.rb                          |   4 +-
 lib/potlatch.rb                               |  14 +-
 lib/potlatch2.rb                              |   2 +-
 lib/quova.rb                                  |   2 +-
 lib/redactable.rb                             |   4 +-
 lib/session_persistence.rb                    |  12 +-
 lib/short_link.rb                             | 124 +++++++++---------
 test/controllers/changeset_controller_test.rb |  48 +++----
 test/controllers/node_controller_test.rb      |   8 +-
 test/controllers/relation_controller_test.rb  |  34 ++---
 test/controllers/way_controller_test.rb       |  26 ++--
 test/integration/oauth_test.rb                |   1 +
 test/integration/user_creation_test.rb        |   2 +-
 test/models/message_test.rb                   |   2 +-
 test/models/note_comment_test.rb              |   4 +-
 test/models/request_token_test.rb             |  12 ++
 test/models/trace_test.rb                     |  28 ++--
 test/models/user_preference_test.rb           |   2 +-
 test/models/way_test.rb                       |   2 +-
 58 files changed, 500 insertions(+), 488 deletions(-)
 create mode 100644 test/models/request_token_test.rb

diff --git a/.rubocop.yml b/.rubocop.yml
index 213eb27aa..62685c8ed 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1,7 +1,7 @@
 inherit_from: .rubocop_todo.yml
 
-AllCops:
-   RunRailsCops: true
+Rails:
+  Enabled: true
 
 Style/BracesAroundHashParameters:
   EnforcedStyle: context_dependent
@@ -15,10 +15,16 @@ Style/FileName:
     - 'script/locale/reload-languages'
     - 'script/update-spam-blocks'
 
+Style/IfInsideElse:
+  Enabled: false
+
 Style/GlobalVars:
   Exclude:
     - 'lib/quad_tile/extconf.rb'
-    
+
+Style/GuardClause:
+  Enabled: false
+
 Style/HashSyntax:
   EnforcedStyle: hash_rockets
   Exclude:
@@ -27,3 +33,7 @@ Style/HashSyntax:
 
 Style/StringLiterals:
   EnforcedStyle: double_quotes
+
+Style/WordArray:
+  Exclude:
+    - 'test/models/message_test.rb'
diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index f2b592b49..f528919c3 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -189,11 +189,11 @@ class AmfController < ApplicationController
   def getpresets(usertoken, lang) #:doc:
     user = getuser(usertoken)
 
-    if user && !user.languages.empty?
-      langs = Locale.list(user.languages)
-    else
-      langs = Locale.list(http_accept_language.user_preferred_languages)
-    end
+    langs = if user && !user.languages.empty?
+              Locale.list(user.languages)
+            else
+              Locale.list(http_accept_language.user_preferred_languages)
+            end
 
     lang = getlocales.preferred(langs)
     (real_lang, localised) = getlocalized(lang.to_s)
@@ -471,11 +471,11 @@ class AmfController < ApplicationController
       return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists?
 
       query = Trace.visible_to(user)
-      if searchterm.to_i > 0
-        query = query.where(:id => searchterm.to_i)
-      else
-        query = query.where("MATCH(name) AGAINST (?)", searchterm).limit(21)
-      end
+      query = if searchterm.to_i > 0
+                query.where(:id => searchterm.to_i)
+              else
+                query.where("MATCH(name) AGAINST (?)", searchterm).limit(21)
+              end
       gpxs = query.collect do |gpx|
         [gpx.id, gpx.name, gpx.description]
       end
@@ -868,11 +868,10 @@ class AmfController < ApplicationController
 
   def getuser(token) #:doc:
     if token =~ /^(.+)\:(.+)$/
-      user = User.authenticate(:username => $1, :password => $2)
+      User.authenticate(:username => $1, :password => $2)
     else
-      user = User.authenticate(:token => token)
+      User.authenticate(:token => token)
     end
-    user
   end
 
   def getlocales
diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 379bb7e86..d158019f3 100644
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -136,7 +136,7 @@ class ApiController < ApplicationController
     doc = OSM::API.new.get_xml_doc
 
     # add bounds
-    doc.root << bbox.add_bounds_to(XML::Node.new "bounds")
+    doc.root << bbox.add_bounds_to(XML::Node.new("bounds"))
 
     # get ways
     # find which ways are needed
@@ -258,8 +258,8 @@ class ApiController < ApplicationController
 
     api = XML::Node.new "api"
     version = XML::Node.new "version"
-    version["minimum"] = "#{API_VERSION}"
-    version["maximum"] = "#{API_VERSION}"
+    version["minimum"] = API_VERSION.to_s
+    version["maximum"] = API_VERSION.to_s
     api << version
     area = XML::Node.new "area"
     area["maximum"] = MAX_REQUEST_AREA.to_s
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9954b775b..98eecd600 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -140,13 +140,13 @@ class ApplicationController < ActionController::Base
     unless Authenticator.new(self, [:token]).allow?
       username, passwd = get_auth_data # parse from headers
       # authenticate per-scheme
-      if username.nil?
-        @user = nil # no authentication provided - perhaps first connect (client should retry after 401)
-      elsif username == "token"
-        @user = User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth
-      else
-        @user = User.authenticate(:username => username, :password => passwd) # basic auth
-      end
+      @user = if username.nil?
+                nil # no authentication provided - perhaps first connect (client should retry after 401)
+              elsif username == "token"
+                User.authenticate(:token => passwd) # preferred - random token for user from db, passed in basic auth
+              else
+                User.authenticate(:username => username, :password => passwd) # basic auth
+              end
     end
 
     # have we identified the user?
@@ -276,7 +276,7 @@ class ApplicationController < ActionController::Base
     response.headers["Error"] = message
 
     if request.headers["X-Error-Format"] &&
-       request.headers["X-Error-Format"].downcase == "xml"
+       request.headers["X-Error-Format"].casecmp("xml").zero?
       result = OSM::API.new.get_xml_doc
       result.root.name = "osmError"
       result.root << (XML::Node.new("status") << "#{Rack::Utils.status_code(status)} #{Rack::Utils::HTTP_STATUS_CODES[status]}")
diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb
index 0ac45b926..7d2f80eb9 100644
--- a/app/controllers/browse_controller.rb
+++ b/app/controllers/browse_controller.rb
@@ -58,11 +58,11 @@ class BrowseController < ApplicationController
   def changeset
     @type = "changeset"
     @changeset = Changeset.find(params[:id])
-    if @user && @user.moderator?
-      @comments = @changeset.comments.unscope(:where => :visible).includes(:author)
-    else
-      @comments = @changeset.comments.includes(:author)
-    end
+    @comments = if @user && @user.moderator?
+                  @changeset.comments.unscope(:where => :visible).includes(:author)
+                else
+                  @changeset.comments.includes(:author)
+                end
     @node_pages, @nodes = paginate(:old_nodes, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "node_page")
     @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page")
     @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page")
diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index 733c20922..137991276 100644
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -186,16 +186,14 @@ class ChangesetController < ApplicationController
           # first version, so it must be newly-created.
           created = XML::Node.new "create"
           created << elt.to_xml_node(changeset_cache, user_display_name_cache)
+        elsif elt.visible
+          # must be a modify
+          modified = XML::Node.new "modify"
+          modified << elt.to_xml_node(changeset_cache, user_display_name_cache)
         else
-          if elt.visible
-            # must be a modify
-            modified = XML::Node.new "modify"
-            modified << elt.to_xml_node(changeset_cache, user_display_name_cache)
-          else
-            # if the element isn't visible then it must have been deleted
-            deleted = XML::Node.new "delete"
-            deleted << elt.to_xml_node(changeset_cache, user_display_name_cache)
-          end
+          # if the element isn't visible then it must have been deleted
+          deleted = XML::Node.new "delete"
+          deleted << elt.to_xml_node(changeset_cache, user_display_name_cache)
         end
     end
 
@@ -277,11 +275,11 @@ class ChangesetController < ApplicationController
       changesets = conditions_nonempty(Changeset.all)
 
       if params[:display_name]
-        if user.data_public? || user == @user
-          changesets = changesets.where(:user_id => user.id)
-        else
-          changesets = changesets.where("false")
-        end
+        changesets = if user.data_public? || user == @user
+                       changesets.where(:user_id => user.id)
+                     else
+                       changesets.where("false")
+                     end
       elsif params[:bbox]
         changesets = conditions_bbox(changesets, BoundingBox.from_bbox_params(params))
       elsif params[:friends] && @user
@@ -459,11 +457,12 @@ class ChangesetController < ApplicationController
     if bbox
       bbox.check_boundaries
       bbox = bbox.to_scaled
-      return changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?",
-                              bbox.max_lon.to_i, bbox.min_lon.to_i,
-                              bbox.max_lat.to_i, bbox.min_lat.to_i)
+
+      changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?",
+                       bbox.max_lon.to_i, bbox.min_lon.to_i,
+                       bbox.max_lat.to_i, bbox.min_lat.to_i)
     else
-      return changesets
+      changesets
     end
   end
 
@@ -471,7 +470,7 @@ class ChangesetController < ApplicationController
   # restrict changesets to those by a particular user
   def conditions_user(changesets, user, name)
     if user.nil? && name.nil?
-      return changesets
+      changesets
     else
       # shouldn't provide both name and UID
       fail OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name
@@ -497,7 +496,8 @@ class ChangesetController < ApplicationController
 
         fail OSM::APINotFoundError if @user.nil? || @user.id != u.id
       end
-      return changesets.where(:user_id => u.id)
+
+      changesets.where(:user_id => u.id)
     end
   end
 
@@ -506,20 +506,19 @@ class ChangesetController < ApplicationController
   def conditions_time(changesets, time)
     if time.nil?
       return changesets
-    else
+    elsif time.count(",") == 1
       # if there is a range, i.e: comma separated, then the first is
       # low, second is high - same as with bounding boxes.
-      if time.count(",") == 1
-        # check that we actually have 2 elements in the array
-        times = time.split(/,/)
-        fail OSM::APIBadUserInput.new("bad time range") if times.size != 2
 
-        from, to = times.collect { |t| DateTime.parse(t) }
-        return changesets.where("closed_at >= ? and created_at <= ?", from, to)
-      else
-        # if there is no comma, assume its a lower limit on time
-        return changesets.where("closed_at >= ?", DateTime.parse(time))
-      end
+      # check that we actually have 2 elements in the array
+      times = time.split(/,/)
+      fail OSM::APIBadUserInput.new("bad time range") if times.size != 2
+
+      from, to = times.collect { |t| DateTime.parse(t) }
+      return changesets.where("closed_at >= ? and created_at <= ?", from, to)
+    else
+      # if there is no comma, assume its a lower limit on time
+      return changesets.where("closed_at >= ?", DateTime.parse(time))
     end
     # stupid DateTime seems to throw both of these for bad parsing, so
     # we have to catch both and ensure the correct code path is taken.
diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb
index b6fb455ee..5f29f418d 100644
--- a/app/controllers/geocoder_controller.rb
+++ b/app/controllers/geocoder_controller.rb
@@ -19,13 +19,13 @@ class GeocoderController < ApplicationController
       @sources.push "osm_nominatim_reverse"
       @sources.push "geonames_reverse" if defined?(GEONAMES_USERNAME)
     elsif params[:query]
-      if params[:query].match(/^\d{5}(-\d{4})?$/)
+      if params[:query] =~ /^\d{5}(-\d{4})?$/
         @sources.push "us_postcode"
         @sources.push "osm_nominatim"
-      elsif params[:query].match(/^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i)
+      elsif params[:query] =~ /^(GIR 0AA|[A-PR-UWYZ]([0-9]{1,2}|([A-HK-Y][0-9]|[A-HK-Y][0-9]([0-9]|[ABEHMNPRV-Y]))|[0-9][A-HJKS-UW])\s*[0-9][ABD-HJLNP-UW-Z]{2})$/i
         @sources.push "uk_postcode"
         @sources.push "osm_nominatim"
-      elsif params[:query].match(/^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i)
+      elsif params[:query] =~ /^[A-Z]\d[A-Z]\s*\d[A-Z]\d$/i
         @sources.push "ca_postcode"
         @sources.push "osm_nominatim"
       else
@@ -70,7 +70,7 @@ class GeocoderController < ApplicationController
     response = fetch_text("http://rpc.geocoder.us/service/csv?zip=#{escape_query(query)}")
 
     # parse the response
-    unless response.match(/couldn't find this zip/)
+    unless response =~ /couldn't find this zip/
       data = response.split(/\s*,\s+/) # lat,long,town,state,zip
       @results.push(:lat => data[0], :lon => data[1],
                     :zoom => POSTCODE_ZOOM,
@@ -95,7 +95,7 @@ class GeocoderController < ApplicationController
     response = fetch_text("http://www.npemap.org.uk/cgi/geocoder.fcgi?format=text&postcode=#{escape_query(query)}")
 
     # parse the response
-    unless response.match(/Error/)
+    unless response =~ /Error/
       dataline = response.split(/\n/)[1]
       data = dataline.split(/,/) # easting,northing,postcode,lat,long
       postcode = data[2].delete("'")
@@ -171,11 +171,11 @@ class GeocoderController < ApplicationController
       type = place.attributes["type"].to_s
       name = place.attributes["display_name"].to_s
       min_lat, max_lat, min_lon, max_lon = place.attributes["boundingbox"].to_s.split(",")
-      if type.empty?
-        prefix_name = ""
-      else
-        prefix_name = t "geocoder.search_osm_nominatim.prefix.#{klass}.#{type}", :default => type.tr("_", " ").capitalize
-      end
+      prefix_name = if type.empty?
+                      ""
+                    else
+                      t "geocoder.search_osm_nominatim.prefix.#{klass}.#{type}", :default => type.tr("_", " ").capitalize
+                    end
       if klass == "boundary" && type == "administrative"
         rank = (place.attributes["place_rank"].to_i + 1) / 2
         prefix_name = t "geocoder.search_osm_nominatim.admin_levels.level#{rank}", :default => prefix_name
@@ -339,11 +339,11 @@ class GeocoderController < ApplicationController
   def nsew_to_decdeg(captures)
     begin
       Float(captures[0])
-      captures[2].downcase != "s" ? lat = captures[0].to_f : lat = -(captures[0].to_f)
-      captures[5].downcase != "w" ? lon = captures[3].to_f : lon = -(captures[3].to_f)
+      lat = captures[2].downcase != "s" ? captures[0].to_f : -captures[0].to_f
+      lon = captures[5].downcase != "w" ? captures[3].to_f : -captures[3].to_f
     rescue
-      captures[0].downcase != "s" ? lat = captures[1].to_f : lat = -(captures[1].to_f)
-      captures[3].downcase != "w" ? lon = captures[4].to_f : lon = -(captures[4].to_f)
+      lat = captures[0].downcase != "s" ? captures[1].to_f : -captures[1].to_f
+      lon = captures[3].downcase != "w" ? captures[4].to_f : -captures[4].to_f
     end
     { :lat => lat, :lon => lon }
   end
@@ -351,11 +351,11 @@ class GeocoderController < ApplicationController
   def ddm_to_decdeg(captures)
     begin
       Float(captures[0])
-      captures[3].downcase != "s" ? lat = captures[0].to_f + captures[1].to_f / 60 : lat = -(captures[0].to_f + captures[1].to_f / 60)
-      captures[7].downcase != "w" ? lon = captures[4].to_f + captures[5].to_f / 60 : lon = -(captures[4].to_f + captures[5].to_f / 60)
+      lat = captures[3].downcase != "s" ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60)
+      lon = captures[7].downcase != "w" ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60)
     rescue
-      captures[0].downcase != "s" ? lat = captures[1].to_f + captures[2].to_f / 60 : lat = -(captures[1].to_f + captures[2].to_f / 60)
-      captures[4].downcase != "w" ? lon = captures[5].to_f + captures[6].to_f / 60 : lon = -(captures[5].to_f + captures[6].to_f / 60)
+      lat = captures[0].downcase != "s" ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60)
+      lon = captures[4].downcase != "w" ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60)
     end
     { :lat => lat, :lon => lon }
   end
@@ -363,11 +363,11 @@ class GeocoderController < ApplicationController
   def dms_to_decdeg(captures)
     begin
       Float(captures[0])
-      captures[4].downcase != "s" ? lat = captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : lat = -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60)
-      captures[9].downcase != "w" ? lon = captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : lon = -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60)
+      lat = captures[4].downcase != "s" ? captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60)
+      lon = captures[9].downcase != "w" ? captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60)
     rescue
-      captures[0].downcase != "s" ? lat = captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : lat = -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60)
-      captures[5].downcase != "w" ? lon = captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : lon = -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60)
+      lat = captures[0].downcase != "s" ? captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60)
+      lon = captures[5].downcase != "w" ? captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60)
     end
     { :lat => lat, :lon => lon }
   end
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index e73b2ba03..d996251b3 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -319,21 +319,19 @@ class NotesController < ApplicationController
   # Generate a condition to choose which bugs we want based
   # on their status and the user's request parameters
   def closed_condition(notes)
-    if params[:closed]
-      closed_since = params[:closed].to_i
-    else
-      closed_since = 7
-    end
+    closed_since = if params[:closed]
+                     params[:closed].to_i
+                   else
+                     7
+                   end
 
     if closed_since < 0
-      notes = notes.where("status != 'hidden'")
+      notes.where("status != 'hidden'")
     elsif closed_since > 0
-      notes = notes.where("(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))")
+      notes.where("(status = 'open' OR (status = 'closed' AND closed_at > '#{Time.now - closed_since.days}'))")
     else
-      notes = notes.where("status = 'open'")
+      notes.where("status = 'open'")
     end
-
-    notes
   end
 
   ##
diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb
index 88691f862..59ebfd631 100644
--- a/app/controllers/oauth_controller.rb
+++ b/app/controllers/oauth_controller.rb
@@ -41,38 +41,36 @@ class OauthController < ApplicationController
     if @token.invalidated?
       @message = t "oauth.oauthorize_failure.invalid"
       render :action => "authorize_failure"
-    else
-      if request.post?
-        if user_authorizes_token?
-          @token.authorize!(current_user)
-          if @token.oauth10?
-            callback_url = params[:oauth_callback] || @token.client_application.callback_url
-          else
-            callback_url = @token.oob? ? @token.client_application.callback_url : @token.callback_url
-          end
-          @redirect_url = URI.parse(callback_url) unless callback_url.blank?
+    elsif request.post?
+      if user_authorizes_token?
+        @token.authorize!(current_user)
+        callback_url = if @token.oauth10?
+                         params[:oauth_callback] || @token.client_application.callback_url
+                       else
+                         @token.oob? ? @token.client_application.callback_url : @token.callback_url
+                       end
+        @redirect_url = URI.parse(callback_url) unless callback_url.blank?
 
-          if @redirect_url.to_s.blank?
-            render :action => "authorize_success"
-          else
-            @redirect_url.query = if @redirect_url.query.blank?
-                                    "oauth_token=#{@token.token}"
-                                  else
-                                    @redirect_url.query +
+        if @redirect_url.to_s.blank?
+          render :action => "authorize_success"
+        else
+          @redirect_url.query = if @redirect_url.query.blank?
+                                  "oauth_token=#{@token.token}"
+                                else
+                                  @redirect_url.query +
                                     "&oauth_token=#{@token.token}"
-                                  end
-
-            unless @token.oauth10?
-              @redirect_url.query += "&oauth_verifier=#{@token.verifier}"
-            end
+                                end
 
-            redirect_to @redirect_url.to_s
+          unless @token.oauth10?
+            @redirect_url.query += "&oauth_verifier=#{@token.verifier}"
           end
-        else
-          @token.invalidate!
-          @message = t("oauth.oauthorize_failure.denied", :app_name => @token.client_application.name)
-          render :action => "authorize_failure"
+
+          redirect_to @redirect_url.to_s
         end
+      else
+        @token.invalidate!
+        @message = t("oauth.oauthorize_failure.denied", :app_name => @token.client_application.name)
+        render :action => "authorize_failure"
       end
     end
   end
diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb
index bda0a7942..771b1a032 100644
--- a/app/controllers/trace_controller.rb
+++ b/app/controllers/trace_controller.rb
@@ -30,13 +30,13 @@ class TraceController < ApplicationController
     end
 
     # set title
-    if target_user.nil?
-      @title = t "trace.list.public_traces"
-    elsif @user && @user == target_user
-      @title = t "trace.list.your_traces"
-    else
-      @title = t "trace.list.public_traces_from", :user => target_user.display_name
-    end
+    @title = if target_user.nil?
+               t "trace.list.public_traces"
+             elsif @user && @user == target_user
+               t "trace.list.your_traces"
+             else
+               t "trace.list.public_traces_from", :user => target_user.display_name
+             end
 
     @title += t "trace.list.tagged_with", :tags => params[:tag] if params[:tag]
 
@@ -45,19 +45,17 @@ class TraceController < ApplicationController
     # 2 - all traces, not logged in = all public traces
     # 3 - user's traces, logged in as same user = all user's traces
     # 4 - user's traces, not logged in as that user = all user's public traces
-    if target_user.nil? # all traces
-      if @user
-        @traces = Trace.visible_to(@user) # 1
-      else
-        @traces = Trace.visible_to_all # 2
-      end
-    else
-      if @user && @user == target_user
-        @traces = @user.traces # 3 (check vs user id, so no join + can't pick up non-public traces by changing name)
-      else
-        @traces = target_user.traces.visible_to_all # 4
-      end
-    end
+    @traces = if target_user.nil? # all traces
+                if @user
+                  Trace.visible_to(@user) # 1
+                else
+                  Trace.visible_to_all # 2
+                end
+              elsif @user && @user == target_user
+                @user.traces # 3 (check vs user id, so no join + can't pick up non-public traces by changing name)
+              else
+                target_user.traces.visible_to_all # 4
+              end
 
     @traces = @traces.tagged(params[:tag]) if params[:tag]
 
@@ -315,11 +313,11 @@ class TraceController < ApplicationController
     visibility = params[:visibility]
 
     if visibility.nil?
-      if params[:public] && params[:public].to_i.nonzero?
-        visibility = "public"
-      else
-        visibility = "private"
-      end
+      visibility = if params[:public] && params[:public].to_i.nonzero?
+                     "public"
+                   else
+                     "private"
+                   end
     end
 
     if params[:file].respond_to?(:read)
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 14e8c9460..8520aa324 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -403,13 +403,11 @@ class UserController < ApplicationController
         friend.friend_user_id = @new_friend.id
         if @user.is_friends_with?(@new_friend)
           flash[:warning] = t "user.make_friend.already_a_friend", :name => @new_friend.display_name
+        elsif friend.save
+          flash[:notice] = t "user.make_friend.success", :name => @new_friend.display_name
+          Notifier.friend_notification(friend).deliver_now
         else
-          if friend.save
-            flash[:notice] = t "user.make_friend.success", :name => @new_friend.display_name
-            Notifier.friend_notification(friend).deliver_now
-          else
-            friend.add_error(t("user.make_friend.failed", :name => @new_friend.display_name))
-          end
+          friend.add_error(t("user.make_friend.failed", :name => @new_friend.display_name))
         end
 
         if params[:referer]
@@ -682,11 +680,11 @@ class UserController < ApplicationController
     user.home_lat = params[:user][:home_lat]
     user.home_lon = params[:user][:home_lon]
 
-    if params[:user][:preferred_editor] == "default"
-      user.preferred_editor = nil
-    else
-      user.preferred_editor = params[:user][:preferred_editor]
-    end
+    user.preferred_editor = if params[:user][:preferred_editor] == "default"
+                              nil
+                            else
+                              params[:user][:preferred_editor]
+                            end
 
     if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank?
       user.auth_provider = nil
@@ -778,11 +776,11 @@ class UserController < ApplicationController
   ##
   # check signup acls
   def check_signup_allowed(email = nil)
-    if email.nil?
-      domain = nil
-    else
-      domain = email.split("@").last
-    end
+    domain = if email.nil?
+               nil
+             else
+               email.split("@").last
+             end
 
     if blocked = Acl.no_account_creation(request.remote_ip, domain)
       logger.info "Blocked signup from #{request.remote_ip} for #{email}"
diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index 6c9e108dc..8f2ce5da7 100644
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -2,11 +2,11 @@ require "uri"
 
 module BrowseHelper
   def printable_name(object, version = false)
-    if object.id.is_a?(Array)
-      id = object.id[0]
-    else
-      id = object.id
-    end
+    id = if object.id.is_a?(Array)
+           object.id[0]
+         else
+           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
@@ -92,7 +92,7 @@ module BrowseHelper
 
   private
 
-  ICON_TAGS = %w(aeroway amenity barrier building highway historic landuse leisure man_made natural railway shop tourism waterway)
+  ICON_TAGS = %w(aeroway amenity barrier building highway historic landuse leisure man_made natural railway shop tourism waterway).freeze
 
   def icon_tags(object)
     object.tags.find_all { |k, _v| ICON_TAGS.include? k }.sort
@@ -123,14 +123,14 @@ module BrowseHelper
     if key == "wikipedia"
       # This regex should match Wikipedia language codes, everything
       # from de to zh-classical
-      if value =~ /^([a-z-]{2,12}):(.+)$/i
-        # Value is <lang>:<title> so split it up
-        # Note that value is always left as-is, see: https://trac.openstreetmap.org/ticket/4315
-        lang = $1
-      else
-        # Value is <title> so default to English Wikipedia
-        lang = "en"
-      end
+      lang = if value =~ /^([a-z-]{2,12}):(.+)$/i
+               # Value is <lang>:<title> so split it up
+               # Note that value is always left as-is, see: https://trac.openstreetmap.org/ticket/4315
+               $1
+             else
+               # Value is <title> so default to English Wikipedia
+               "en"
+             end
     elsif key =~ /^wikipedia:(\S+)$/
       # Language is in the key, so assume value is the title
       lang = $1
diff --git a/app/helpers/geocoder_helper.rb b/app/helpers/geocoder_helper.rb
index e135917a9..161bb2d6d 100644
--- a/app/helpers/geocoder_helper.rb
+++ b/app/helpers/geocoder_helper.rb
@@ -2,13 +2,13 @@ module GeocoderHelper
   def result_to_html(result)
     html_options = { :class => "set_position", :data => {} }
 
-    if result[:type] && result[:id]
-      url = url_for(:controller => :browse, :action => result[:type], :id => result[:id])
-    elsif result[:min_lon] && result[:min_lat] && result[:max_lon] && result[:max_lat]
-      url = "/?bbox=#{result[:min_lon]},#{result[:min_lat]},#{result[:max_lon]},#{result[:max_lat]}"
-    else
-      url = "/#map=#{result[:zoom]}/#{result[:lat]}/#{result[:lon]}"
-    end
+    url = if result[:type] && result[:id]
+            url_for(:controller => :browse, :action => result[:type], :id => result[:id])
+          elsif result[:min_lon] && result[:min_lat] && result[:max_lon] && result[:max_lat]
+            "/?bbox=#{result[:min_lon]},#{result[:min_lat]},#{result[:max_lon]},#{result[:max_lat]}"
+          else
+            "/#map=#{result[:zoom]}/#{result[:lat]}/#{result[:lon]}"
+          end
 
     result.each do |key, value|
       html_options[:data][key.to_s.tr("_", "-")] = value
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index bf939e425..691081609 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -146,7 +146,7 @@ class Changeset < ActiveRecord::Base
     # do the changeset update and the changeset tags update in the
     # same transaction to ensure consistency.
     Changeset.transaction do
-      self.save!
+      save!
 
       tags = self.tags
       ChangesetTag.delete_all(:changeset_id => id)
@@ -166,12 +166,12 @@ class Changeset < ActiveRecord::Base
   # that would make it more than 24h long, in which case clip to
   # 24h, as this has been decided is a reasonable time limit.
   def update_closed_at
-    if self.is_open?
-      if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT)
-        self.closed_at = created_at + MAX_TIME_OPEN
-      else
-        self.closed_at = Time.now.getutc + IDLE_TIMEOUT
-      end
+    if is_open?
+      self.closed_at = if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT)
+                         created_at + MAX_TIME_OPEN
+                       else
+                         Time.now.getutc + IDLE_TIMEOUT
+                       end
     end
   end
 
diff --git a/app/models/client_application.rb b/app/models/client_application.rb
index aa7cb1c34..156eeafc7 100644
--- a/app/models/client_application.rb
+++ b/app/models/client_application.rb
@@ -77,7 +77,7 @@ class ClientApplication < ActiveRecord::Base
   # can agree or not agree to each of them.
   PERMISSIONS = [:allow_read_prefs, :allow_write_prefs, :allow_write_diary,
                  :allow_write_api, :allow_read_gpx, :allow_write_gpx,
-                 :allow_write_notes]
+                 :allow_write_notes].freeze
 
   def generate_keys
     self.key = OAuth::Helper.generate_key(40)[0, 40]
diff --git a/app/models/node.rb b/app/models/node.rb
index f1a89e0e2..20c874931 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -117,7 +117,7 @@ class Node < ActiveRecord::Base
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Node.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_node, user)
       ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id)
       fail OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(",")}.") unless ways.empty?
@@ -138,7 +138,7 @@ class Node < ActiveRecord::Base
 
   def update_from(new_node, user)
     Node.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_node, user)
 
       # update changeset first
@@ -184,7 +184,7 @@ class Node < ActiveRecord::Base
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
-    if self.visible?
+    if visible?
       el["lat"] = lat.to_s
       el["lon"] = lon.to_s
     end
@@ -235,7 +235,7 @@ class Node < ActiveRecord::Base
     Node.transaction do
       self.version += 1
       self.timestamp = t
-      self.save!
+      save!
 
       # Create a NodeTag
       tags = self.tags
diff --git a/app/models/note.rb b/app/models/note.rb
index 31056be49..73207af0f 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -59,6 +59,6 @@ class Note < ActiveRecord::Base
 
   # Fill in default values for new notes
   def set_defaults
-    self.status = "open" unless self.attribute_present?(:status)
+    self.status = "open" unless attribute_present?(:status)
   end
 end
diff --git a/app/models/notifier.rb b/app/models/notifier.rb
index a9bc0d101..23f7b9907 100644
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -136,17 +136,17 @@ class Notifier < ActionMailer::Base
       @owner = recipient == comment.note.author
       @event = comment.event
 
-      if comment.author
-        @commenter = comment.author.display_name
-      else
-        @commenter = I18n.t("notifier.note_comment_notification.anonymous")
-      end
-
-      if @owner
-        subject = I18n.t("notifier.note_comment_notification.#{@event}.subject_own", :commenter => @commenter)
-      else
-        subject = I18n.t("notifier.note_comment_notification.#{@event}.subject_other", :commenter => @commenter)
-      end
+      @commenter = if comment.author
+                     comment.author.display_name
+                   else
+                     I18n.t("notifier.note_comment_notification.anonymous")
+                   end
+
+      subject = if @owner
+                  I18n.t("notifier.note_comment_notification.#{@event}.subject_own", :commenter => @commenter)
+                else
+                  I18n.t("notifier.note_comment_notification.#{@event}.subject_other", :commenter => @commenter)
+                end
 
       mail :to => recipient.email, :subject => subject
     end
@@ -162,11 +162,11 @@ class Notifier < ActionMailer::Base
       @time = comment.created_at
       @changeset_author = comment.changeset.user.display_name
 
-      if @owner
-        subject = I18n.t("notifier.changeset_comment_notification.commented.subject_own", :commenter => @commenter)
-      else
-        subject = I18n.t("notifier.changeset_comment_notification.commented.subject_other", :commenter => @commenter)
-      end
+      subject = if @owner
+                  I18n.t("notifier.changeset_comment_notification.commented.subject_own", :commenter => @commenter)
+                else
+                  I18n.t("notifier.changeset_comment_notification.commented.subject_other", :commenter => @commenter)
+                end
 
       mail :to => recipient.email, :subject => subject
     end
diff --git a/app/models/old_node.rb b/app/models/old_node.rb
index 84899d7fb..b103e5c73 100644
--- a/app/models/old_node.rb
+++ b/app/models/old_node.rb
@@ -55,7 +55,7 @@ class OldNode < ActiveRecord::Base
 
     add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
 
-    if self.visible?
+    if visible?
       el["lat"] = lat.to_s
       el["lon"] = lon.to_s
     end
diff --git a/app/models/relation.rb b/app/models/relation.rb
index cb9621822..d244a6d6c 100644
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -33,7 +33,7 @@ class Relation < ActiveRecord::Base
   scope :ways, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Way", :member_id => ids.flatten }) }
   scope :relations, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Relation", :member_id => ids.flatten }) }
 
-  TYPES = %w(node way relation)
+  TYPES = %w(node way relation).freeze
 
   def self.from_xml(xml, create = false)
     p = XML::Parser.string(xml)
@@ -183,7 +183,7 @@ class Relation < ActiveRecord::Base
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Relation.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_relation, user)
       # This will check to see if this relation is used by another relation
       rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
@@ -199,7 +199,7 @@ class Relation < ActiveRecord::Base
 
   def update_from(new_relation, user)
     Relation.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_relation, user)
       unless new_relation.preconditions_ok?(members)
         fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
@@ -215,7 +215,7 @@ class Relation < ActiveRecord::Base
 
   def create_with_history(user)
     check_create_consistency(self, user)
-    unless self.preconditions_ok?
+    unless preconditions_ok?
       fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
     end
     self.version = 0
@@ -289,7 +289,7 @@ class Relation < ActiveRecord::Base
       t = Time.now.getutc
       self.version += 1
       self.timestamp = t
-      self.save!
+      save!
 
       tags = self.tags.clone
       relation_tags.each do |old_tag|
@@ -312,7 +312,7 @@ class Relation < ActiveRecord::Base
       end
       # if there are left-over tags then they are new and will have to
       # be added.
-      tags_changed |= (!tags.empty?)
+      tags_changed |= !tags.empty?
       RelationTag.delete_all(:relation_id => id)
       self.tags.each do |k, v|
         tag = RelationTag.new
@@ -370,7 +370,7 @@ class Relation < ActiveRecord::Base
       # materially change the rest of the relation.
       any_relations =
         changed_members.collect { |_id, type| type == "relation" }
-        .inject(false) { |a, e| a || e }
+                       .inject(false) { |a, e| a || e }
 
       update_members = if tags_changed || any_relations
                          # add all non-relation bounding boxes to the changeset
diff --git a/app/models/request_token.rb b/app/models/request_token.rb
index a1b30abb3..c0f019486 100644
--- a/app/models/request_token.rb
+++ b/app/models/request_token.rb
@@ -35,7 +35,7 @@ class RequestToken < OauthToken
   end
 
   def oob?
-    callback_url.nil? || callback_url.downcase == "oob"
+    callback_url.nil? || callback_url.casecmp("oob").zero?
   end
 
   def oauth10?
diff --git a/app/models/trace.rb b/app/models/trace.rb
index b13ce84d2..ed1bf058c 100644
--- a/app/models/trace.rb
+++ b/app/models/trace.rb
@@ -28,20 +28,20 @@ class Trace < ActiveRecord::Base
   end
 
   def tagstring=(s)
-    if s.include? ","
-      self.tags = s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect {|tag|
-        tt = Tracetag.new
-        tt.tag = tag
-        tt
-      }
-    else
-      # do as before for backwards compatibility:
-      self.tags = s.split.collect {|tag|
-        tt = Tracetag.new
-        tt.tag = tag
-        tt
-      }
-    end
+    self.tags = if s.include? ","
+                  s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect do|tag|
+                    tt = Tracetag.new
+                    tt.tag = tag
+                    tt
+                  end
+                else
+                  # do as before for backwards compatibility:
+                  s.split.collect do|tag|
+                    tt = Tracetag.new
+                    tt.tag = tag
+                    tt
+                  end
+                end
   end
 
   def public?
@@ -101,17 +101,17 @@ class Trace < ActiveRecord::Base
     zipped = filetype =~ /Zip archive/
     tarred = filetype =~ /tar archive/
 
-    if gzipped
-      mimetype = "application/x-gzip"
-    elsif bzipped
-      mimetype = "application/x-bzip2"
-    elsif zipped
-      mimetype = "application/x-zip"
-    elsif tarred
-      mimetype = "application/x-tar"
-    else
-      mimetype = "application/gpx+xml"
-    end
+    mimetype = if gzipped
+                 "application/x-gzip"
+               elsif bzipped
+                 "application/x-bzip2"
+               elsif zipped
+                 "application/x-zip"
+               elsif tarred
+                 "application/x-tar"
+               else
+                 "application/gpx+xml"
+               end
 
     mimetype
   end
@@ -123,21 +123,21 @@ class Trace < ActiveRecord::Base
     zipped = filetype =~ /Zip archive/
     tarred = filetype =~ /tar archive/
 
-    if tarred && gzipped
-      extension = ".tar.gz"
-    elsif tarred && bzipped
-      extension = ".tar.bz2"
-    elsif tarred
-      extension = ".tar"
-    elsif gzipped
-      extension = ".gpx.gz"
-    elsif bzipped
-      extension = ".gpx.bz2"
-    elsif zipped
-      extension = ".zip"
-    else
-      extension = ".gpx"
-    end
+    extension = if tarred && gzipped
+                  ".tar.gz"
+                elsif tarred && bzipped
+                  ".tar.bz2"
+                elsif tarred
+                  ".tar"
+                elsif gzipped
+                  ".gpx.gz"
+                elsif bzipped
+                  ".gpx.bz2"
+                elsif zipped
+                  ".zip"
+                else
+                  ".gpx"
+                end
 
     extension
   end
@@ -156,7 +156,7 @@ class Trace < ActiveRecord::Base
     el1["lon"] = longitude.to_s if inserted
     el1["user"] = user.display_name
     el1["visibility"] = visibility
-    el1["pending"] = (!inserted).to_s
+    el1["pending"] = inserted ? "false" : "true"
     el1["timestamp"] = timestamp.xmlschema
 
     el2 = XML::Node.new "description"
@@ -297,7 +297,7 @@ class Trace < ActiveRecord::Base
       self.icon_picture = gpx.icon(min_lat, min_lon, max_lat, max_lon)
       self.size = gpx.actual_points
       self.inserted = true
-      self.save!
+      save!
     end
 
     logger.info "done trace #{id}"
diff --git a/app/models/user.rb b/app/models/user.rb
index eae917af8..1a0e3415e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -242,7 +242,7 @@ class User < ActiveRecord::Base
   private
 
   def set_defaults
-    self.creation_time = Time.now.getutc unless self.attribute_present?(:creation_time)
+    self.creation_time = Time.now.getutc unless attribute_present?(:creation_time)
   end
 
   def encrypt_password
diff --git a/app/models/user_role.rb b/app/models/user_role.rb
index 9686f7250..6bc7435ae 100644
--- a/app/models/user_role.rb
+++ b/app/models/user_role.rb
@@ -1,7 +1,7 @@
 class UserRole < ActiveRecord::Base
   belongs_to :user
 
-  ALL_ROLES = %w(administrator moderator)
+  ALL_ROLES = %w(administrator moderator).freeze
 
   validates :role, :inclusion => ALL_ROLES, :uniqueness => { :scope => :user_id }
 end
diff --git a/app/models/way.rb b/app/models/way.rb
index 6d49735f1..0f9650760 100644
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -163,7 +163,7 @@ class Way < ActiveRecord::Base
 
   def update_from(new_way, user)
     Way.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_way, user)
       unless new_way.preconditions_ok?(nds)
         fail OSM::APIPreconditionFailedError.new("Cannot update way #{id}: data is invalid.")
@@ -180,7 +180,7 @@ class Way < ActiveRecord::Base
 
   def create_with_history(user)
     check_create_consistency(self, user)
-    unless self.preconditions_ok?
+    unless preconditions_ok?
       fail OSM::APIPreconditionFailedError.new("Cannot create way: data is invalid.")
     end
     self.version = 0
@@ -219,7 +219,7 @@ class Way < ActiveRecord::Base
     # provide repeatable reads for the used-by checks. this means it
     # shouldn't be possible to get race conditions.
     Way.transaction do
-      self.lock!
+      lock!
       check_consistency(self, new_way, user)
       rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Way", :member_id => id }).order(:id)
       fail OSM::APIPreconditionFailedError.new("Way #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty?
@@ -265,7 +265,7 @@ class Way < ActiveRecord::Base
     Way.transaction do
       self.version += 1
       self.timestamp = t
-      self.save!
+      save!
 
       tags = self.tags
       WayTag.delete_all(:way_id => id)
diff --git a/config/initializers/abstract_adapter.rb b/config/initializers/abstract_adapter.rb
index 5613e0d00..89d384567 100644
--- a/config/initializers/abstract_adapter.rb
+++ b/config/initializers/abstract_adapter.rb
@@ -4,7 +4,7 @@ if defined?(ActiveRecord::ConnectionAdaptors::AbstractAdapter)
       class AbstractAdapter
         protected
 
-        alias_method :old_log, :log
+        alias old_log log
 
         def log(sql, name)
           if block_given?
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index a654f2aa2..610053141 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -1,7 +1,7 @@
 require "openid/fetchers"
 require "openid/util"
 
-CA_BUNDLES = ["/etc/ssl/certs/ca-certificates.crt", "/etc/pki/tls/cert.pem"]
+CA_BUNDLES = ["/etc/ssl/certs/ca-certificates.crt", "/etc/pki/tls/cert.pem"].freeze
 
 OpenID.fetcher.ca_file = CA_BUNDLES.find { |f| File.exist?(f) }
 OpenID::Util.logger = Rails.logger
diff --git a/config/initializers/streaming.rb b/config/initializers/streaming.rb
index 3db2a55f9..0c27ba2e7 100644
--- a/config/initializers/streaming.rb
+++ b/config/initializers/streaming.rb
@@ -1,7 +1,7 @@
 # Hack ActionController::DataStreaming to allow streaming from a file handle
 module ActionController
   module DataStreaming
-    alias_method :old_send_file, :send_file
+    alias old_send_file send_file
 
     def send_file(file, options = {})
       if file.is_a?(File) || file.is_a?(Tempfile)
diff --git a/config/preinitializer.rb b/config/preinitializer.rb
index de8367e9f..07f105722 100644
--- a/config/preinitializer.rb
+++ b/config/preinitializer.rb
@@ -1,17 +1,15 @@
 require "yaml"
 
-if defined?(Rake.application) && Rake.application.top_level_tasks.grep(/^(default$|test(:|$))/).any?
-  env = "test"
-else
-  env = ENV["RAILS_ENV"] || "development"
-end
+env = if defined?(Rake.application) && Rake.application.top_level_tasks.grep(/^(default$|test(:|$))/).any?
+        "test"
+      else
+        ENV["RAILS_ENV"] || "development"
+      end
 
 config = YAML.load_file(File.expand_path(env == "test" ? "../example.application.yml" : "../application.yml", __FILE__))
 
 ENV.each do |key, value|
-  if key.match(/^OSM_(.*)$/)
-    Object.const_set(Regexp.last_match(1).upcase, value)
-  end
+  Object.const_set(Regexp.last_match(1).upcase, value) if key =~ /^OSM_(.*)$/
 end
 
 config[env].each do |key, value|
diff --git a/db/migrate/008_remove_segments.rb b/db/migrate/008_remove_segments.rb
index 241a144b0..2ccb0a2ba 100644
--- a/db/migrate/008_remove_segments.rb
+++ b/db/migrate/008_remove_segments.rb
@@ -15,7 +15,7 @@ class RemoveSegments < ActiveRecord::Migration
       end
 
       conn_opts = ActiveRecord::Base.connection
-                  .instance_eval { @connection_options }
+                                    .instance_eval { @connection_options }
       args = conn_opts.map(&:to_s) + [prefix]
       fail "#{cmd} failed" unless system cmd, *args
 
diff --git a/db/migrate/023_add_changesets.rb b/db/migrate/023_add_changesets.rb
index 9039bb5d6..76ef1f747 100644
--- a/db/migrate/023_add_changesets.rb
+++ b/db/migrate/023_add_changesets.rb
@@ -29,7 +29,7 @@ class AddChangesets < ActiveRecord::Migration
     # all the changesets will have the id of the user that made them.
     # We need to generate a changeset for each user in the database
     execute "INSERT INTO changesets (id, user_id, created_at, open)" +
-      "SELECT id, id, creation_time, false from users;"
+    "SELECT id, id, creation_time, false from users;"
 
     @conv_user_tables.each do |tbl|
       rename_column tbl, :user_id, :changeset_id
diff --git a/db/migrate/041_add_fine_o_auth_permissions.rb b/db/migrate/041_add_fine_o_auth_permissions.rb
index dcb77a2b6..598cb3c04 100644
--- a/db/migrate/041_add_fine_o_auth_permissions.rb
+++ b/db/migrate/041_add_fine_o_auth_permissions.rb
@@ -1,6 +1,6 @@
 class AddFineOAuthPermissions < ActiveRecord::Migration
   PERMISSIONS = [:allow_read_prefs, :allow_write_prefs, :allow_write_diary,
-                 :allow_write_api, :allow_read_gpx, :allow_write_gpx]
+                 :allow_write_api, :allow_read_gpx, :allow_write_gpx].freeze
 
   def self.up
     PERMISSIONS.each do |perm|
diff --git a/lib/auth.rb b/lib/auth.rb
index 636173928..b00df09d0 100644
--- a/lib/auth.rb
+++ b/lib/auth.rb
@@ -1,6 +1,7 @@
 module Auth
-  PROVIDERS = { "None" => "", "OpenID" => "openid" }
-  PROVIDERS["Google"] = "google" if defined?(GOOGLE_AUTH_ID)
-  PROVIDERS["Facebook"] = "facebook" if defined?(FACEBOOK_AUTH_ID)
-  PROVIDERS["Windows Live"] = "windowslive" if defined?(WINDOWSLIVE_AUTH_ID)
+  PROVIDERS = { "None" => "", "OpenID" => "openid" }.tap do |providers|
+    providers["Google"] = "google" if defined?(GOOGLE_AUTH_ID)
+    providers["Facebook"] = "facebook" if defined?(FACEBOOK_AUTH_ID)
+    providers["Windows Live"] = "windowslive" if defined?(WINDOWSLIVE_AUTH_ID)
+  end.freeze
 end
diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb
index e2dfcd3ff..c01808f54 100644
--- a/lib/bounding_box.rb
+++ b/lib/bounding_box.rb
@@ -6,8 +6,6 @@ class BoundingBox
   SCALED_LON_LIMIT = LON_LIMIT * GeoRecord::SCALE
   SCALED_LAT_LIMIT = LAT_LIMIT * GeoRecord::SCALE
 
-  public
-
   def initialize(min_lon, min_lat, max_lon, max_lat)
     @min_lon = min_lon.to_f unless min_lon.nil?
     @min_lat = min_lat.to_f unless min_lat.nil?
@@ -158,20 +156,22 @@ class BoundingBox
     "#{min_lon},#{min_lat},#{max_lon},#{max_lat}"
   end
 
-  private
-
-  def self.from_bbox_array(bbox_array)
-    unless bbox_array
-      fail OSM::APIBadUserInput.new(
-        "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat")
+  class << self
+    private
+
+    def from_bbox_array(bbox_array)
+      unless bbox_array
+        fail OSM::APIBadUserInput.new(
+          "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat")
+      end
+      # 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
+      min_lat = [[bbox_array[1].to_f, -LAT_LIMIT].max, +LAT_LIMIT].min
+      max_lon = [[bbox_array[2].to_f, +LON_LIMIT].min, -LON_LIMIT].max
+      max_lat = [[bbox_array[3].to_f, +LAT_LIMIT].min, -LAT_LIMIT].max
+      BoundingBox.new(min_lon, min_lat, max_lon, max_lat)
     end
-    # 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
-    min_lat = [[bbox_array[1].to_f, -LAT_LIMIT].max, +LAT_LIMIT].min
-    max_lon = [[bbox_array[2].to_f, +LON_LIMIT].min, -LON_LIMIT].max
-    max_lat = [[bbox_array[3].to_f, +LAT_LIMIT].min, -LAT_LIMIT].max
-    BoundingBox.new(min_lon, min_lat, max_lon, max_lat)
   end
 
   def update!(bbox)
diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb
index ee0c5b7dc..00db5d337 100644
--- a/lib/classic_pagination/pagination.rb
+++ b/lib/classic_pagination/pagination.rb
@@ -61,7 +61,7 @@ module ActionController
       DEFAULT_OPTIONS[:group] = nil
     else
       # A hash holding options for controllers using macro-style pagination
-      OPTIONS = {}
+      OPTIONS = {}.freeze
 
       # The default options for pagination
       DEFAULT_OPTIONS = {
@@ -78,7 +78,7 @@ module ActionController
         :select     => nil,
         :group      => nil,
         :parameter  => "page"
-      }
+      }.freeze
     end
 
     def self.included(base) #:nodoc:
@@ -253,19 +253,19 @@ module ActionController
       def current_page
         @current_page ||= self[@current_page_number]
       end
-      alias_method :current, :current_page
+      alias current current_page
 
       # Returns a new Page representing the first page in this paginator.
       def first_page
         @first_page ||= self[1]
       end
-      alias_method :first, :first_page
+      alias first first_page
 
       # Returns a new Page representing the last page in this paginator.
       def last_page
         @last_page ||= self[page_count]
       end
-      alias_method :last, :last_page
+      alias last last_page
 
       # Returns the number of pages in this paginator.
       def page_count
@@ -277,7 +277,7 @@ module ActionController
                         end
       end
 
-      alias_method :length, :page_count
+      alias length page_count
 
       # Returns true if this paginator contains the page of index +number+.
       def has_page_number?(number)
@@ -310,7 +310,7 @@ module ActionController
           @number = 1 unless @paginator.has_page_number? @number
         end
         attr_reader :paginator, :number
-        alias_method :to_i, :number
+        alias to_i number
 
         # Compares two Page objects and returns true when they represent the
         # same page (i.e., their paginators are the same and they have the
@@ -416,7 +416,7 @@ module ActionController
         def pages
           (@first.number..@last.number).to_a.collect! { |n| @paginator[n] }
         end
-        alias_method :to_a, :pages
+        alias to_a pages
       end
     end
   end
diff --git a/lib/classic_pagination/pagination_helper.rb b/lib/classic_pagination/pagination_helper.rb
index ae03b82b2..18decb119 100644
--- a/lib/classic_pagination/pagination_helper.rb
+++ b/lib/classic_pagination/pagination_helper.rb
@@ -13,7 +13,7 @@ module ActionView
           :always_show_anchors => true,
           :link_to_current_page => false,
           :params => {}
-        }
+        }.freeze
       end
 
       # Creates a basic HTML link bar for the given +paginator+.  Links will be created
@@ -113,11 +113,11 @@ module ActionView
         end
 
         window_pages.each do |page|
-          if current_page == page && !link_to_current_page
-            html << page.number.to_s
-          else
-            html << yield(page.number)
-          end
+          html << if current_page == page && !link_to_current_page
+                    page.number.to_s
+                  else
+                    yield(page.number)
+                  end
           html << " "
         end
 
diff --git a/lib/diff_reader.rb b/lib/diff_reader.rb
index 6ffe3806a..867e28489 100644
--- a/lib/diff_reader.rb
+++ b/lib/diff_reader.rb
@@ -11,7 +11,7 @@ class DiffReader
     "node"     => Node,
     "way"      => Way,
     "relation" => Relation
-  }
+  }.freeze
 
   ##
   # Construct a diff reader by giving it a bunch of XML +data+ to parse
diff --git a/lib/editors.rb b/lib/editors.rb
index 1c5427b94..21f646228 100644
--- a/lib/editors.rb
+++ b/lib/editors.rb
@@ -1,4 +1,4 @@
 module Editors
-  ALL_EDITORS = %w(potlatch potlatch2 id remote)
-  RECOMMENDED_EDITORS = %w(id potlatch2 remote)
+  ALL_EDITORS = %w(potlatch potlatch2 id remote).freeze
+  RECOMMENDED_EDITORS = %w(id potlatch2 remote).freeze
 end
diff --git a/lib/gpx.rb b/lib/gpx.rb
index f1c82cf92..3ec11c5c4 100644
--- a/lib/gpx.rb
+++ b/lib/gpx.rb
@@ -63,7 +63,7 @@ module GPX
       highlightgc.stroke("#000000")
       highlightgc.fill("#000000")
 
-      images = frames.times.collect do
+      images = Array(frames) do
         Magick::Image.new(width, height) do |image|
           image.background_color = "white"
           image.format = "GIF"
@@ -81,11 +81,11 @@ module GPX
 
         if m > 0
           frames.times do |n|
-            if n == mm
-              gc = highlightgc.dup
-            else
-              gc = linegc.dup
-            end
+            gc = if n == mm
+                   highlightgc.dup
+                 else
+                   linegc.dup
+                 end
 
             gc.line(px, py, oldpx, oldpy)
 
@@ -148,13 +148,11 @@ module GPX
     end
   end
 
-  private
-
   TrkPt = Struct.new(:segment, :latitude, :longitude, :altitude, :timestamp) do
     def valid?
       latitude && longitude && timestamp &&
-      latitude >= -90 && latitude <= 90 &&
-      longitude >= -180 && longitude <= 180
+        latitude >= -90 && latitude <= 90 &&
+        longitude >= -180 && longitude <= 180
     end
   end
 end
diff --git a/lib/password_hash.rb b/lib/password_hash.rb
index fe618ba7a..4faac4da8 100644
--- a/lib/password_hash.rb
+++ b/lib/password_hash.rb
@@ -7,7 +7,7 @@ module PasswordHash
   SALT_BYTE_SIZE = 32
   HASH_BYTE_SIZE = 32
   PBKDF2_ITERATIONS = 1000
-  DIGEST_ALGORITHM = "sha512"
+  DIGEST_ALGORITHM = "sha512".freeze
 
   def self.create(password)
     salt = SecureRandom.base64(SALT_BYTE_SIZE)
@@ -45,8 +45,6 @@ module PasswordHash
     false
   end
 
-  private
-
   def self.hash(password, salt, iterations, size, algorithm)
     digest = OpenSSL::Digest.new(algorithm)
     pbkdf2 = OpenSSL::PKCS5.pbkdf2_hmac(password, salt, iterations, size, digest)
diff --git a/lib/potlatch.rb b/lib/potlatch.rb
index a077d4c4d..6e8c88bb8 100644
--- a/lib/potlatch.rb
+++ b/lib/potlatch.rb
@@ -30,9 +30,7 @@ module Potlatch
 
     # Return numeric array
     def self.getarray(s)
-      getlong(s).times.collect do
-        getvalue(s)
-      end
+      Array.new(getlong(s)) { getvalue(s) }
     end
 
     # Return object/hash
@@ -251,11 +249,11 @@ module Potlatch
           tag = $1
           type = $2
           values = $3
-          if values == "-"
-            autotags[type][tag] = []
-          else
-            autotags[type][tag] = values.split(",").sort.reverse
-          end
+          autotags[type][tag] = if values == "-"
+                                  []
+                                else
+                                  values.split(",").sort.reverse
+                                end
         end
       end
 
diff --git a/lib/potlatch2.rb b/lib/potlatch2.rb
index d2ccd546a..bf2894968 100644
--- a/lib/potlatch2.rb
+++ b/lib/potlatch2.rb
@@ -93,5 +93,5 @@ module Potlatch2
     "yi" => "yi",
     "zh" => "zh_CN",
     "zh-TW" => "zh_TW"
-  }
+  }.freeze
 end
diff --git a/lib/quova.rb b/lib/quova.rb
index b09f8acea..cb6b93654 100644
--- a/lib/quova.rb
+++ b/lib/quova.rb
@@ -16,7 +16,7 @@ end
 module Quova
   ##
   # Access details for WSDL description
-  WSDL_URL = "https://webservices.quova.com/OnDemand/GeoPoint/v1/default.asmx?WSDL"
+  WSDL_URL = "https://webservices.quova.com/OnDemand/GeoPoint/v1/default.asmx?WSDL".freeze
   WSDL_USER = QUOVA_USERNAME
   WSDL_PASS = QUOVA_PASSWORD
 
diff --git a/lib/redactable.rb b/lib/redactable.rb
index a42164ec7..ca98b714e 100644
--- a/lib/redactable.rb
+++ b/lib/redactable.rb
@@ -13,10 +13,10 @@ module Redactable
 
   def redact!(redaction)
     # check that this version isn't the current version
-    fail OSM::APICannotRedactError.new if self.is_latest_version?
+    fail OSM::APICannotRedactError.new if is_latest_version?
 
     # make the change
     self.redaction = redaction
-    self.save!
+    save!
   end
 end
diff --git a/lib/session_persistence.rb b/lib/session_persistence.rb
index 2244a3e63..d4dd1cc1d 100644
--- a/lib/session_persistence.rb
+++ b/lib/session_persistence.rb
@@ -20,13 +20,17 @@
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 module SessionPersistence
-  private
+  class << self
+    private
 
-  # Install filter when we are included
-  def self.included(controller)
-    controller.after_filter :persist_session
+    # Install filter when we are included
+    def included(controller)
+      controller.after_filter :persist_session
+    end
   end
 
+  private
+
   # Override this method if you don't want to use session[:_remember_for].
   def session_persistence_key
     :_remember_for
diff --git a/lib/short_link.rb b/lib/short_link.rb
index 43d64041e..ec83ce33f 100644
--- a/lib/short_link.rb
+++ b/lib/short_link.rb
@@ -10,78 +10,80 @@ module ShortLink
   # URL-friendly.
   ARRAY = ("A".."Z").to_a + ("a".."z").to_a + ("0".."9").to_a + ["_", "~"]
 
-  ##
-  # Given a string encoding a location, returns the [lon, lat, z] tuple of that
-  # location.
-  def self.decode(str)
-    x = 0
-    y = 0
-    z = 0
-    z_offset = 0
+  class << self
+    ##
+    # Given a string encoding a location, returns the [lon, lat, z] tuple of that
+    # location.
+    def decode(str)
+      x = 0
+      y = 0
+      z = 0
+      z_offset = 0
 
-    # keep support for old shortlinks which use the @ character, now
-    # replaced by the ~ character because twitter is horribly broken
-    # and we can't have that.
-    str.tr!("@", "~")
+      # keep support for old shortlinks which use the @ character, now
+      # replaced by the ~ character because twitter is horribly broken
+      # and we can't have that.
+      str.tr!("@", "~")
 
-    str.each_char do |c|
-      t = ARRAY.index c
-      if t.nil?
-        z_offset -= 1
-      else
-        3.times do
-          x <<= 1
-          x |= 1 unless (t & 32).zero?
-          t <<= 1
+      str.each_char do |c|
+        t = ARRAY.index c
+        if t.nil?
+          z_offset -= 1
+        else
+          3.times do
+            x <<= 1
+            x |= 1 unless (t & 32).zero?
+            t <<= 1
 
-          y <<= 1
-          y |= 1 unless (t & 32).zero?
-          t <<= 1
+            y <<= 1
+            y |= 1 unless (t & 32).zero?
+            t <<= 1
+          end
+          z += 3
         end
-        z += 3
       end
+      # pack the coordinates out to their original 32 bits.
+      x <<= (32 - z)
+      y <<= (32 - z)
+
+      # project the parameters back to their coordinate ranges.
+      [(x * 360.0 / 2**32) - 180.0,
+       (y * 180.0 / 2**32) - 90.0,
+       z - 8 - (z_offset % 3)]
     end
-    # pack the coordinates out to their original 32 bits.
-    x <<= (32 - z)
-    y <<= (32 - z)
 
-    # project the parameters back to their coordinate ranges.
-    [(x * 360.0 / 2**32) - 180.0,
-     (y * 180.0 / 2**32) - 90.0,
-     z - 8 - (z_offset % 3)]
-  end
+    ##
+    # given a location and zoom, return a short string representing it.
+    def encode(lon, lat, z)
+      code = interleave_bits(((lon + 180.0) * 2**32 / 360.0).to_i,
+                             ((lat + 90.0) * 2**32 / 180.0).to_i)
+      str = ""
+      # add eight to the zoom level, which approximates an accuracy of
+      # one pixel in a tile.
+      ((z + 8) / 3.0).ceil.times do |i|
+        digit = (code >> (58 - 6 * i)) & 0x3f
+        str << ARRAY[digit]
+      end
+      # append characters onto the end of the string to represent
+      # partial zoom levels (characters themselves have a granularity
+      # of 3 zoom levels).
+      ((z + 8) % 3).times { str << "-" }
 
-  ##
-  # given a location and zoom, return a short string representing it.
-  def self.encode(lon, lat, z)
-    code = interleave_bits(((lon + 180.0) * 2**32 / 360.0).to_i,
-                           ((lat + 90.0) * 2**32 / 180.0).to_i)
-    str = ""
-    # add eight to the zoom level, which approximates an accuracy of
-    # one pixel in a tile.
-    ((z + 8) / 3.0).ceil.times do |i|
-      digit = (code >> (58 - 6 * i)) & 0x3f
-      str << ARRAY[digit]
+      str
     end
-    # append characters onto the end of the string to represent
-    # partial zoom levels (characters themselves have a granularity
-    # of 3 zoom levels).
-    ((z + 8) % 3).times { str << "-" }
 
-    str
-  end
-
-  private
+    private
 
-  ##
-  # interleaves the bits of two 32-bit numbers. the result is known
-  # as a Morton code.
-  def self.interleave_bits(x, y)
-    c = 0
-    31.downto(0) do |i|
-      c = (c << 1) | ((x >> i) & 1)
-      c = (c << 1) | ((y >> i) & 1)
+    ##
+    # interleaves the bits of two 32-bit numbers. the result is known
+    # as a Morton code.
+    def interleave_bits(x, y)
+      c = 0
+      31.downto(0) do |i|
+        c = (c << 1) | ((x >> i) & 1)
+        c = (c << 1) | ((y >> i) & 1)
+      end
+      c
     end
-    c
   end
 end
diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb
index 582d1ee5c..5da65c4c6 100644
--- a/test/controllers/changeset_controller_test.rb
+++ b/test/controllers/changeset_controller_test.rb
@@ -102,16 +102,16 @@ class ChangesetControllerTest < ActionController::TestCase
     basic_authorization users(:normal_user).email, "test"
     # Create the first user's changeset
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_require_public_data
 
     basic_authorization users(:public_user).email, "test"
     # Create the first user's changeset
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
 
     assert_response :success, "Creation of changeset did not return sucess status"
@@ -527,8 +527,8 @@ EOF
 
       # create a temporary changeset
       content "<osm><changeset>" +
-        "<tag k='created_by' v='osm test suite checking changesets'/>" +
-        "</changeset></osm>"
+      "<tag k='created_by' v='osm test suite checking changesets'/>" +
+      "</changeset></osm>"
       assert_difference "Changeset.count", 1 do
         put :create
       end
@@ -1102,8 +1102,8 @@ EOF
     basic_authorization users(:public_user).email, "test"
 
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
@@ -1114,8 +1114,8 @@ EOF
     diff.root = XML::Node.new "osmChange"
     modify = XML::Node.new "modify"
     xml_old_node = old_node.to_xml_node
-    xml_old_node["lat"] = (2.0).to_s
-    xml_old_node["lon"] = (2.0).to_s
+    xml_old_node["lat"] = 2.0.to_s
+    xml_old_node["lon"] = 2.0.to_s
     xml_old_node["changeset"] = changeset_id.to_s
     modify << xml_old_node
     diff.root << modify
@@ -1140,8 +1140,8 @@ EOF
     basic_authorization users(:public_user).email, "test"
 
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
@@ -1225,8 +1225,8 @@ EOF
 
     # create a temporary changeset
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_response :forbidden
 
@@ -1235,8 +1235,8 @@ EOF
 
     # create a temporary changeset
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
@@ -1281,8 +1281,8 @@ EOF
 
     # create a temporary changeset
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
@@ -1340,8 +1340,8 @@ OSMFILE
 
     # create a temporary changeset
     content "<osm><changeset>" +
-      "<tag k='created_by' v='osm test suite checking changesets'/>" +
-      "</changeset></osm>"
+    "<tag k='created_by' v='osm test suite checking changesets'/>" +
+    "</changeset></osm>"
     put :create
     assert_response :success
     changeset_id = @response.body.to_i
@@ -2317,8 +2317,8 @@ EOF
   # check the result of a list
   def check_list_result(changesets)
     changesets = changesets.where("num_changes > 0")
-                 .order(:created_at => :desc)
-                 .limit(20)
+                           .order(:created_at => :desc)
+                           .limit(20)
     assert changesets.size <= 20
 
     assert_select "ol.changesets", :count => [changesets.size, 1].min do
@@ -2334,8 +2334,8 @@ EOF
   # check the result of a feed
   def check_feed_result(changesets)
     changesets = changesets.where("num_changes > 0")
-                 .order(:created_at => :desc)
-                 .limit(20)
+                           .order(:created_at => :desc)
+                           .limit(20)
     assert changesets.size <= 20
 
     assert_select "feed", :count => [changesets.size, 1].min do
diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb
index 8952daade..5fd610e7f 100644
--- a/test/controllers/node_controller_test.rb
+++ b/test/controllers/node_controller_test.rb
@@ -466,8 +466,8 @@ class NodeControllerTest < ActionController::TestCase
     # try and put something into a string that the API might
     # use unquoted and therefore allow code injection...
     content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" +
-      '<tag k="#{@user.inspect}" v="0"/>' +
-      "</node></osm>"
+    '<tag k="#{@user.inspect}" v="0"/>' +
+    "</node></osm>"
     put :create
     assert_require_public_data "Shouldn't be able to create with non-public user"
 
@@ -478,8 +478,8 @@ class NodeControllerTest < ActionController::TestCase
     # try and put something into a string that the API might
     # use unquoted and therefore allow code injection...
     content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" +
-      '<tag k="#{@user.inspect}" v="0"/>' +
-      "</node></osm>"
+    '<tag k="#{@user.inspect}" v="0"/>' +
+    "</node></osm>"
     put :create
     assert_response :success
     nodeid = @response.body
diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb
index bb9286f1d..ae6c94667 100644
--- a/test/controllers/relation_controller_test.rb
+++ b/test/controllers/relation_controller_test.rb
@@ -165,8 +165,8 @@ class RelationControllerTest < ActionController::TestCase
     # This time try with a role attribute in the relation
     nid = current_nodes(:used_node_1).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member  ref='#{nid}' type='node' role='some'/>" +
-      "<tag k='test' v='yes' /></relation></osm>"
+    "<member  ref='#{nid}' type='node' role='some'/>" +
+    "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden due to user
     assert_response :forbidden,
@@ -177,7 +177,7 @@ class RelationControllerTest < ActionController::TestCase
     # need a role attribute to be included
     nid = current_nodes(:used_node_1).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member  ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
+    "<member  ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden due to user
     assert_response :forbidden,
@@ -188,9 +188,9 @@ class RelationControllerTest < ActionController::TestCase
     nid = current_nodes(:used_node_1).id
     wid = current_ways(:used_way).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member type='node' ref='#{nid}' role='some'/>" +
-      "<member type='way' ref='#{wid}' role='other'/>" +
-      "<tag k='test' v='yes' /></relation></osm>"
+    "<member type='node' ref='#{nid}' role='some'/>" +
+    "<member type='way' ref='#{wid}' role='other'/>" +
+    "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for forbidden, due to user
     assert_response :forbidden,
@@ -233,8 +233,8 @@ class RelationControllerTest < ActionController::TestCase
     # This time try with a role attribute in the relation
     nid = current_nodes(:used_node_1).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member  ref='#{nid}' type='node' role='some'/>" +
-      "<tag k='test' v='yes' /></relation></osm>"
+    "<member  ref='#{nid}' type='node' role='some'/>" +
+    "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
     assert_response :success,
@@ -265,7 +265,7 @@ class RelationControllerTest < ActionController::TestCase
     # need a role attribute to be included
     nid = current_nodes(:used_node_1).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member  ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
+    "<member  ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
     assert_response :success,
@@ -296,9 +296,9 @@ class RelationControllerTest < ActionController::TestCase
     nid = current_nodes(:used_node_1).id
     wid = current_ways(:used_way).id
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member type='node' ref='#{nid}' role='some'/>" +
-      "<member type='way' ref='#{wid}' role='other'/>" +
-      "<tag k='test' v='yes' /></relation></osm>"
+    "<member type='node' ref='#{nid}' role='some'/>" +
+    "<member type='way' ref='#{wid}' role='other'/>" +
+    "<tag k='test' v='yes' /></relation></osm>"
     put :create
     # hope for success
     assert_response :success,
@@ -412,8 +412,8 @@ class RelationControllerTest < ActionController::TestCase
 
     # create a relation with non-existing node as member
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member type='node' ref='0'/><tag k='test' v='yes' />" +
-      "</relation></osm>"
+    "<member type='node' ref='0'/><tag k='test' v='yes' />" +
+    "</relation></osm>"
     put :create
     # expect failure
     assert_response :precondition_failed,
@@ -432,8 +432,8 @@ class RelationControllerTest < ActionController::TestCase
 
     # create some xml that should return an error
     content "<osm><relation changeset='#{changeset_id}'>" +
-      "<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" +
-      "<tag k='tester' v='yep'/></relation></osm>"
+    "<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" +
+    "<tag k='tester' v='yep'/></relation></osm>"
     put :create
     # expect failure
     assert_response :bad_request
@@ -963,7 +963,7 @@ OSM
     a_tags.each do |k, v|
       assert_equal v, b_tags[k],
                    "Tags which were not altered should be the same. " +
-                     "#{a_tags.inspect} != #{b_tags.inspect}"
+      "#{a_tags.inspect} != #{b_tags.inspect}"
     end
   end
 
diff --git a/test/controllers/way_controller_test.rb b/test/controllers/way_controller_test.rb
index 8226f435c..45acabece 100644
--- a/test/controllers/way_controller_test.rb
+++ b/test/controllers/way_controller_test.rb
@@ -121,8 +121,8 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with pre-existing nodes
     content "<osm><way changeset='#{changeset_id}'>" +
-      "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
-      "<tag k='test' v='yes' /></way></osm>"
+    "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
+    "<tag k='test' v='yes' /></way></osm>"
     put :create
     # hope for failure
     assert_response :forbidden,
@@ -138,8 +138,8 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with pre-existing nodes
     content "<osm><way changeset='#{changeset_id}'>" +
-      "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
-      "<tag k='test' v='yes' /></way></osm>"
+    "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
+    "<tag k='test' v='yes' /></way></osm>"
     put :create
     # hope for success
     assert_response :success,
@@ -179,7 +179,7 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with non-existing node
     content "<osm><way changeset='#{open_changeset_id}'>" +
-      "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
+    "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
     assert_response :forbidden,
@@ -187,7 +187,7 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with no nodes
     content "<osm><way changeset='#{open_changeset_id}'>" +
-      "<tag k='test' v='yes' /></way></osm>"
+    "<tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
     assert_response :forbidden,
@@ -195,7 +195,7 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way inside a closed changeset
     content "<osm><way changeset='#{closed_changeset_id}'>" +
-      "<nd ref='#{nid1}'/></way></osm>"
+    "<nd ref='#{nid1}'/></way></osm>"
     put :create
     # expect failure
     assert_response :forbidden,
@@ -211,7 +211,7 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with non-existing node
     content "<osm><way changeset='#{open_changeset_id}'>" +
-      "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
+    "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
     assert_response :precondition_failed,
@@ -220,7 +220,7 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with no nodes
     content "<osm><way changeset='#{open_changeset_id}'>" +
-      "<tag k='test' v='yes' /></way></osm>"
+    "<tag k='test' v='yes' /></way></osm>"
     put :create
     # expect failure
     assert_response :precondition_failed,
@@ -229,7 +229,7 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way inside a closed changeset
     content "<osm><way changeset='#{closed_changeset_id}'>" +
-      "<nd ref='#{nid1}'/></way></osm>"
+    "<nd ref='#{nid1}'/></way></osm>"
     put :create
     # expect failure
     assert_response :conflict,
@@ -237,9 +237,9 @@ class WayControllerTest < ActionController::TestCase
 
     # create a way with a tag which is too long
     content "<osm><way changeset='#{open_changeset_id}'>" +
-      "<nd ref='#{nid1}'/>" +
-      "<tag k='foo' v='#{'x' * 256}'/>" +
-      "</way></osm>"
+    "<nd ref='#{nid1}'/>" +
+    "<tag k='foo' v='#{'x' * 256}'/>" +
+    "</way></osm>"
     put :create
     # expect failure
     assert_response :bad_request,
diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb
index aed52f77c..323147030 100644
--- a/test/integration/oauth_test.rb
+++ b/test/integration/oauth_test.rb
@@ -329,6 +329,7 @@ class OAuthTest < ActionDispatch::IntegrationTest
     assert_not_nil token.created_at
     assert_nil token.authorized_at
     assert_nil token.invalidated_at
+    assert_equal options[:oauth_callback], token.callback_url
     assert_allowed token, client.permissions
 
     token
diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb
index e36f85c06..2bc17f711 100644
--- a/test/integration/user_creation_test.rb
+++ b/test/integration/user_creation_test.rb
@@ -81,7 +81,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
       assert_difference("User.count") do
         assert_difference("ActionMailer::Base.deliveries.size", 1) do
           post_via_redirect "/user/save", {},
-                            { "HTTP_ACCEPT_LANGUAGE" => "#{locale}" }
+                            { "HTTP_ACCEPT_LANGUAGE" => locale.to_s }
         end
       end
 
diff --git a/test/models/message_test.rb b/test/models/message_test.rb
index 81290a14a..a6ae3ecce 100644
--- a/test/models/message_test.rb
+++ b/test/models/message_test.rb
@@ -5,7 +5,7 @@ class MessageTest < ActiveSupport::TestCase
   api_fixtures
   fixtures :messages
 
-  EURO = "\xe2\x82\xac" # euro symbol
+  EURO = "\xe2\x82\xac".freeze # euro symbol
 
   # This needs to be updated when new fixtures are added
   # or removed.
diff --git a/test/models/note_comment_test.rb b/test/models/note_comment_test.rb
index e61732904..323a827c0 100644
--- a/test/models/note_comment_test.rb
+++ b/test/models/note_comment_test.rb
@@ -22,8 +22,8 @@ class NoteCommentTest < ActiveSupport::TestCase
   end
 
   def test_body_valid
-    ok = ["Name", "vergrößern", "foo\x0abar",
-          "ルシステムにも対応します", "輕觸搖晃的遊戲"]
+    ok = %W(Name vergrößern foo\nbar
+            ルシステムにも対応します 輕觸搖晃的遊戲)
     bad = ["foo\x00bar", "foo\x08bar", "foo\x1fbar", "foo\x7fbar",
            "foo\ufffebar", "foo\uffffbar"]
 
diff --git a/test/models/request_token_test.rb b/test/models/request_token_test.rb
new file mode 100644
index 000000000..24ad419fa
--- /dev/null
+++ b/test/models/request_token_test.rb
@@ -0,0 +1,12 @@
+require "test_helper"
+
+class RequestTokenTest < ActiveSupport::TestCase
+  api_fixtures
+
+  def test_oob
+    assert_equal true, RequestToken.new.oob?
+    assert_equal true, RequestToken.new(:callback_url => "oob").oob?
+    assert_equal true, RequestToken.new(:callback_url => "OOB").oob?
+    assert_equal false, RequestToken.new(:callback_url => "http://test.host/").oob?
+  end
+end
diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb
index 524ec637a..32d98c9eb 100644
--- a/test/models/trace_test.rb
+++ b/test/models/trace_test.rb
@@ -26,30 +26,30 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_visible
     check_query(Trace.visible, [
-      :public_trace_file, :anon_trace_file, :trackable_trace_file,
-      :identifiable_trace_file, :zipped_trace_file, :tar_trace_file,
-      :tar_gzip_trace_file, :tar_bzip_trace_file, :pending_trace_file
-    ])
+                  :public_trace_file, :anon_trace_file, :trackable_trace_file,
+                  :identifiable_trace_file, :zipped_trace_file, :tar_trace_file,
+                  :tar_gzip_trace_file, :tar_bzip_trace_file, :pending_trace_file
+                ])
   end
 
   def test_visible_to
     check_query(Trace.visible_to(1), [
-      :public_trace_file, :identifiable_trace_file, :pending_trace_file
-    ])
+                  :public_trace_file, :identifiable_trace_file, :pending_trace_file
+                ])
     check_query(Trace.visible_to(2), [
-      :public_trace_file, :anon_trace_file, :trackable_trace_file,
-      :identifiable_trace_file, :pending_trace_file
-    ])
+                  :public_trace_file, :anon_trace_file, :trackable_trace_file,
+                  :identifiable_trace_file, :pending_trace_file
+                ])
     check_query(Trace.visible_to(3), [
-      :public_trace_file, :identifiable_trace_file, :pending_trace_file
-    ])
+                  :public_trace_file, :identifiable_trace_file, :pending_trace_file
+                ])
   end
 
   def test_visible_to_all
     check_query(Trace.visible_to_all, [
-      :public_trace_file, :identifiable_trace_file,
-      :deleted_trace_file, :pending_trace_file
-    ])
+                  :public_trace_file, :identifiable_trace_file,
+                  :deleted_trace_file, :pending_trace_file
+                ])
   end
 
   def test_tagged
diff --git a/test/models/user_preference_test.rb b/test/models/user_preference_test.rb
index ee2dbf2d8..d3400bc9d 100644
--- a/test/models/user_preference_test.rb
+++ b/test/models/user_preference_test.rb
@@ -20,7 +20,7 @@ class UserPreferenceTest < ActiveSupport::TestCase
     new_up.k = up.k
     new_up.v = "some other value"
     assert_not_equal new_up.v, up.v
-    assert_raise (ActiveRecord::RecordNotUnique) { new_up.save }
+    assert_raise(ActiveRecord::RecordNotUnique) { new_up.save }
   end
 
   def test_check_valid_length
diff --git a/test/models/way_test.rb b/test/models/way_test.rb
index 807079f09..6c8b0f80b 100644
--- a/test/models/way_test.rb
+++ b/test/models/way_test.rb
@@ -29,7 +29,7 @@ class WayTest < ActiveSupport::TestCase
     way = Way.find(current_ways(:visible_way).id)
     assert way.valid?
     # it already has 1 node
-    1.upto((MAX_NUMBER_OF_WAY_NODES) / 2) do
+    1.upto(MAX_NUMBER_OF_WAY_NODES / 2) do
       way.add_nd_num(current_nodes(:used_node_1).id)
       way.add_nd_num(current_nodes(:used_node_2).id)
     end
-- 
2.43.2