From: Andy Allan Date: Wed, 3 Oct 2018 12:04:12 +0000 (+0200) Subject: Merge branch 'master' into messages X-Git-Tag: live~3929^2 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/5e407dfb34f47e6fbbbf3c11c1a8318256abb5cd?hp=f62f0547bb6d215cf0339c9855ddf8b3597b2006 Merge branch 'master' into messages --- diff --git a/.rubocop.yml b/.rubocop.yml index 24d57fb37..31c14773c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,8 @@ inherit_from: .rubocop_todo.yml +AllCops: + TargetRubyVersion: 2.3 + Rails: Enabled: true @@ -27,6 +30,9 @@ Rails/ApplicationRecord: Rails/CreateTableWithTimestamps: Enabled: false +Rails/FindEach: + Enabled: false + Rails/HasManyOrHasOneDependent: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 78ed84ac3..d3e51600d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,23 +1,11 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2018-06-19 09:02:55 +0100 using RuboCop version 0.57.2. +# on 2018-09-19 14:24:02 +0100 using RuboCop version 0.58.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 34 -Lint/AmbiguousOperator: - Exclude: - - 'test/controllers/amf_controller_test.rb' - - 'test/controllers/changeset_controller_test.rb' - - 'test/lib/bounding_box_test.rb' - - 'test/lib/country_test.rb' - -# Offense count: 96 -Lint/AmbiguousRegexpLiteral: - Enabled: false - # Offense count: 32 # Configuration parameters: AllowSafeAssignment. Lint/AssignmentInCondition: @@ -42,17 +30,13 @@ Lint/HandleExceptions: - 'app/controllers/amf_controller.rb' - 'app/controllers/user_controller.rb' -# Offense count: 2 -Lint/ShadowingOuterLocalVariable: - Exclude: - - 'app/views/changeset/list.atom.builder' - -# Offense count: 690 +# Offense count: 692 Metrics/AbcSize: Max: 280 -# Offense count: 41 +# Offense count: 40 # Configuration parameters: CountComments, ExcludedMethods. +# ExcludedMethods: refine Metrics/BlockLength: Max: 262 @@ -70,7 +54,7 @@ Metrics/ClassLength: Metrics/CyclomaticComplexity: Max: 20 -# Offense count: 688 +# Offense count: 691 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 179 @@ -85,7 +69,7 @@ Metrics/ModuleLength: Metrics/ParameterLists: Max: 9 -# Offense count: 71 +# Offense count: 72 Metrics/PerceivedComplexity: Max: 23 @@ -178,10 +162,17 @@ Style/AsciiComments: Exclude: - 'test/models/message_test.rb' -# Offense count: 229 +# Offense count: 230 Style/Documentation: Enabled: false +# Offense count: 462 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle. +# SupportedStyles: when_needed, always, never +Style/FrozenStringLiteralComment: + Enabled: false + # Offense count: 2 # Cop supports --auto-correct. Style/IfUnlessModifier: @@ -194,7 +185,7 @@ Style/IfUnlessModifier: Style/NumericLiterals: MinDigits: 11 -# Offense count: 3064 +# Offense count: 3080 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: diff --git a/Gemfile.lock b/Gemfile.lock index 2f6eb8406..76a31e169 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,7 +2,7 @@ GEM remote: https://rubygems.org/ specs: SystemTimer (1.2.3) - aasm (5.0.0) + aasm (5.0.1) concurrent-ruby (~> 1.0) actioncable (5.2.0) actionpack (= 5.2.0) @@ -63,7 +63,7 @@ GEM bigdecimal (1.1.0) binding_of_caller (0.8.0) debug_inspector (>= 0.0.1) - bootsnap (1.3.1) + bootsnap (1.3.2) msgpack (~> 1.0) builder (3.2.3) canonical-rails (0.2.4) @@ -104,10 +104,10 @@ GEM erubi (1.7.1) execjs (2.7.0) exifr (1.3.4) - factory_bot (4.11.0) + factory_bot (4.11.1) activesupport (>= 3.0.0) - factory_bot_rails (4.11.0) - factory_bot (~> 4.11.0) + factory_bot_rails (4.11.1) + factory_bot (~> 4.11.1) railties (>= 3.0.0) faraday (0.12.2) multipart-post (>= 1.2, < 3) @@ -169,7 +169,7 @@ GEM nokogiri (>= 1.5.9) mail (2.7.0) mini_mime (>= 0.1.1) - marcel (0.3.2) + marcel (0.3.3) mimemagic (~> 0.3.2) method_source (0.9.0) mime-types (3.2.2) @@ -301,7 +301,7 @@ GEM rack (>= 1.4) rinku (2.0.4) rotp (3.3.1) - rubocop (0.58.2) + rubocop (0.59.1) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) @@ -317,7 +317,7 @@ GEM crass (~> 1.0.2) nokogiri (>= 1.4.4) nokogumbo (~> 1.4) - sass (3.5.7) + sass (3.6.0) sass-listen (~> 4.0.0) sass-listen (4.0.0) rb-fsevent (~> 0.9, >= 0.9.4) @@ -358,7 +358,7 @@ GEM tins (1.16.3) tzinfo (1.2.5) thread_safe (~> 0.1) - uglifier (4.1.18) + uglifier (4.1.19) execjs (>= 0.3.0, < 3) unicode-display_width (1.4.0) validates_email_format_of (1.6.3) diff --git a/INSTALL.md b/INSTALL.md index 768cbd540..8e47cb266 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -5,7 +5,7 @@ If you want to deploy the software for your own project, then see the notes at t You can install the software directly on your machine, which is the traditional and probably best-supported approach. However, there is an alternative which may be easier: Vagrant. This installs the software into a virtual machine, which makes it easier to get a consistent development environment and may avoid installation difficulties. For Vagrant instructions, see [VAGRANT.md](VAGRANT.md). -These instructions are based on Ubuntu 12.04 LTS, which is the platform used by the OSMF servers. +These instructions are based on Ubuntu 16.04 LTS, which is the platform used by the OSMF servers. The instructions also work, with only minor amendments, for all other current Ubuntu releases, Fedora and MacOSX We don't recommend attempting to develop or deploy this software on Windows. If you need to use Windows, then try developing this software using Ubuntu in a virtual machine, or use [Vagrant](VAGRANT.md). diff --git a/Vagrantfile b/Vagrantfile index d10934b37..fcb4790fe 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -4,23 +4,26 @@ Vagrant.configure("2") do |config| # use official ubuntu image for virtualbox config.vm.provider "virtualbox" do |vb, override| - override.vm.box = "ubuntu/xenial64" + override.vm.box = "ubuntu/bionic64" override.vm.synced_folder ".", "/srv/openstreetmap-website" vb.customize ["modifyvm", :id, "--memory", "1024"] vb.customize ["modifyvm", :id, "--cpus", "2"] vb.customize ["modifyvm", :id, "--uartmode1", "disconnected"] end - # use third party image and NFS sharing for lxc + # Use sshfs sharing if available, otherwise NFS sharing + sharing_type = Vagrant.has_plugin?("vagrant-sshfs") ? "sshfs" : "nfs" + + # use third party image and sshfs or NFS sharing for lxc config.vm.provider "lxc" do |_, override| - override.vm.box = "generic/ubuntu1604" - override.vm.synced_folder ".", "/srv/openstreetmap-website", :type => "nfs" + override.vm.box = "generic/ubuntu1804" + override.vm.synced_folder ".", "/srv/openstreetmap-website", :type => sharing_type end - # use third party image and NFS sharing for libvirt + # use third party image and sshfs or NFS sharing for libvirt config.vm.provider "libvirt" do |_, override| - override.vm.box = "generic/ubuntu1604" - override.vm.synced_folder ".", "/srv/openstreetmap-website", :type => "nfs" + override.vm.box = "generic/ubuntu1804" + override.vm.synced_folder ".", "/srv/openstreetmap-website", :type => sharing_type end # configure shared package cache if possible diff --git a/Vendorfile b/Vendorfile index 5303d738f..15a8eba1b 100644 --- a/Vendorfile +++ b/Vendorfile @@ -11,13 +11,13 @@ folder 'vendor/assets' do end folder 'leaflet' do - file 'leaflet.js', 'https://unpkg.com/leaflet@1.3.3/dist/leaflet-src.js' - file 'leaflet.css', 'https://unpkg.com/leaflet@1.3.3/dist/leaflet.css' + file 'leaflet.js', 'https://unpkg.com/leaflet@1.3.4/dist/leaflet-src.js' + file 'leaflet.css', 'https://unpkg.com/leaflet@1.3.4/dist/leaflet.css' [ 'layers.png', 'layers-2x.png', 'marker-icon.png', 'marker-icon-2x.png', 'marker-shadow.png' ].each do |image| - file "images/#{image}", "https://unpkg.com/leaflet@1.3.3/dist/images/#{image}" + file "images/#{image}", "https://unpkg.com/leaflet@1.3.4/dist/images/#{image}" end from 'git://github.com/aratcliffe/Leaflet.contextmenu.git', :tag => 'v1.5.0' do @@ -69,7 +69,7 @@ folder 'vendor/assets' do folder 'javascripts' do file 'html5shiv.js', 'https://raw.githubusercontent.com/aFarkas/html5shiv/master/src/html5shiv.js' - file 'bowser.js', 'https://github.com/lancedikson/bowser/releases/download/1.9.3/bowser.js' + file 'bowser.js', 'https://github.com/lancedikson/bowser/releases/download/1.9.4/bowser.js' end folder 'swfobject' do diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index ccf4e561c..e8e566f53 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -187,12 +187,12 @@ $(document).ready(function () { $.cookie('_osm_location', OSM.locationCookie(map), { expires: expiry, path: '/' }); }); - if ($.cookie('_osm_welcome') === 'hide') { - $('.welcome').hide(); + if ($.cookie('_osm_welcome') !== 'hide') { + $('.welcome').addClass('visible'); } $('.welcome .close-wrap').on('click', function() { - $('.welcome').hide(); + $('.welcome').removeClass('visible'); $.cookie('_osm_welcome', 'hide', { expires: expiry, path: '/' }); }); diff --git a/app/assets/javascripts/leaflet.map.js b/app/assets/javascripts/leaflet.map.js index ba88e5904..e712a8d1f 100644 --- a/app/assets/javascripts/leaflet.map.js +++ b/app/assets/javascripts/leaflet.map.js @@ -61,6 +61,12 @@ L.OSM.Map = L.Map.extend({ code: "G", name: I18n.t("javascripts.map.base.gps") }); + + this.on("layeradd", function (event) { + if (this.baseLayers.indexOf(event.layer) >= 0) { + this.setMaxZoom(event.layer.options.maxZoom); + } + }); }, updateLayers: function(layerParam) { diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index f9214b24d..4b8a80c54 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -611,11 +611,18 @@ body.compact { height: auto; overflow: hidden; - #banner, - .welcome { + #banner { display: block; } + .welcome { + display: none; + + &.visible { + display: block; + } + } + #sidebar_content { display: none; } @@ -971,6 +978,8 @@ header .search_forms, img.button { display: block; + width: 20px; + height: 20px; } span.force_width { @@ -1501,7 +1510,7 @@ tr.turn:hover { /* Rules for the trace view */ -.trace-view { +.trace-show { .trace_pending { color: red; } @@ -1585,7 +1594,7 @@ tr.turn:hover { margin-left: 70px; } -.user-view { +.user-show { // Silly exception; remove when user page is redesigned. .content-inner { max-width: none; @@ -1712,7 +1721,7 @@ tr.turn:hover { } } -.diary_entry-view img.user_thumbnail { +.diary_entry-show img.user_thumbnail { float: left; } diff --git a/app/assets/stylesheets/small.scss b/app/assets/stylesheets/small.scss index 984364736..12e21c7d8 100644 --- a/app/assets/stylesheets/small.scss +++ b/app/assets/stylesheets/small.scss @@ -126,9 +126,12 @@ body.small { } } - #sidebar .welcome, - #sidebar #banner { - display: none !important; + .overlay-sidebar #sidebar .welcome.visible { + display: none; + } + + .overlay-sidebar #sidebar #banner { + display: none; } .leaflet-top.leaflet-right { diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 07e7669b1..4f6adae5d 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -143,6 +143,7 @@ class AmfController < ApplicationController if cstags return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(cstags) + cstags = strip_non_xml_chars cstags end @@ -471,7 +472,7 @@ class AmfController < ApplicationController return -1, t("application.setup_user_auth.blocked") if user.blocks.active.exists? query = Trace.visible_to(user) - query = if searchterm.to_i > 0 + query = if searchterm.to_i.positive? query.where(:id => searchterm.to_i) else query.where("MATCH(name) AGAINST (?)", searchterm).limit(21) @@ -497,6 +498,7 @@ class AmfController < ApplicationController rel = Relation.where(:id => relid).first return [-4, "relation", relid] if rel.nil? || !rel.visible + [0, "", relid, rel.tags, rel.members, rel.version] end end @@ -506,9 +508,9 @@ class AmfController < ApplicationController def findrelations(searchterm) rels = [] - if searchterm.to_i > 0 + if searchterm.to_i.positive? rel = Relation.where(:id => searchterm.to_i).first - rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel && rel.visible + rels.push([rel.id, rel.tags, rel.members, rel.version]) if rel&.visible else RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t| rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) if t.relation.visible @@ -533,6 +535,7 @@ class AmfController < ApplicationController return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(tags) + tags = strip_non_xml_chars tags relid = relid.to_i @@ -542,7 +545,7 @@ class AmfController < ApplicationController relation = nil Relation.transaction do # create a new relation, or find the existing one - relation = Relation.find(relid) if relid > 0 + relation = Relation.find(relid) if relid.positive? # We always need a new node, based on the data that has been sent to us new_relation = Relation.new @@ -550,7 +553,7 @@ class AmfController < ApplicationController typedmembers = [] members.each do |m| mid = m[1].to_i - if mid < 0 + if mid.negative? mid = renumberednodes[mid] if m[0] == "Node" mid = renumberedways[mid] if m[0] == "Way" end @@ -622,6 +625,7 @@ class AmfController < ApplicationController return -2, "Server error - way is only #{pointlist.length} points long." if pointlist.length < 2 return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(attributes) + attributes = strip_non_xml_chars attributes originalway = originalway.to_i @@ -651,6 +655,7 @@ class AmfController < ApplicationController # fixup node tags in a way as well return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(node.tags) + node.tags = strip_non_xml_chars node.tags node.tags.delete("created_by") @@ -728,6 +733,7 @@ class AmfController < ApplicationController return -1, "You must accept the contributor terms before you can edit." if REQUIRE_TERMS_AGREED && user.terms_agreed.nil? return -1, "One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." unless tags_ok(tags) + tags = strip_non_xml_chars tags id = id.to_i @@ -735,7 +741,7 @@ class AmfController < ApplicationController node = nil new_node = nil Node.transaction do - if id > 0 + if id.positive? begin node = Node.find(id) rescue ActiveRecord::RecordNotFound @@ -883,12 +889,10 @@ class AmfController < ApplicationController # in the +tags+ hash. def strip_non_xml_chars(tags) new_tags = {} - unless tags.nil? - tags.each do |k, v| - new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015" - new_v = v.delete "\000-\037\ufffe\uffff", "^\011\012\015" - new_tags[new_k] = new_v - end + tags&.each do |k, v| + new_k = k.delete "\000-\037\ufffe\uffff", "^\011\012\015" + new_v = v.delete "\000-\037\ufffe\uffff", "^\011\012\015" + new_tags[new_k] = new_v end new_tags end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index db4ae9ad3..f35493b26 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -283,8 +283,7 @@ class ApplicationController < ActionController::Base # TODO: some sort of escaping of problem characters in the message response.headers["Error"] = message - if request.headers["X-Error-Format"] && - request.headers["X-Error-Format"].casecmp("xml").zero? + if 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]}") @@ -310,7 +309,7 @@ class ApplicationController < ActionController::Base helper_method :preferred_languages def set_locale(reset = false) - if current_user && current_user.languages.empty? && !http_accept_language.user_preferred_languages.empty? + if current_user&.languages&.empty? && !http_accept_language.user_preferred_languages.empty? current_user.languages = http_accept_language.user_preferred_languages current_user.save end @@ -435,7 +434,7 @@ class ApplicationController < ActionController::Base def preferred_editor editor = if params[:editor] params[:editor] - elsif current_user && current_user.preferred_editor + elsif current_user&.preferred_editor current_user.preferred_editor else DEFAULT_EDITOR diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index b4cb4594f..89b9c6ca3 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -58,7 +58,7 @@ class BrowseController < ApplicationController def changeset @type = "changeset" @changeset = Changeset.find(params[:id]) - @comments = if current_user && current_user.moderator? + @comments = if current_user&.moderator? @changeset.comments.unscope(:where => :visible).includes(:author) else @changeset.comments.includes(:author) @@ -77,7 +77,7 @@ class BrowseController < ApplicationController def note @type = "note" - if current_user && current_user.moderator? + if current_user&.moderator? @note = Note.find(params[:id]) @note_comments = @note.comments.unscope(:where => :visible) else diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 56fab0fc4..c03ed9056 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -4,19 +4,19 @@ class ChangesetController < ApplicationController layout "site" require "xml/libxml" - 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] + skip_before_action :verify_authenticity_token, :except => [:index] + before_action :authorize_web, :only => [:index, :feed, :comments_feed] + before_action :set_locale, :only => [:index, :feed, :comments_feed] 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, :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] - around_action :web_timeout, :only => [:list, :feed, :comments_feed] + before_action :check_api_readable, :except => [:create, :update, :upload, :download, :query, :index, :feed, :comment, :subscribe, :unsubscribe, :comments_feed] + before_action(:only => [:index, :feed, :comments_feed]) { |c| c.check_database_readable(true) } + around_action :api_call_handle_error, :except => [:index, :feed, :comments_feed] + around_action :api_call_timeout, :except => [:index, :feed, :comments_feed, :upload] + around_action :web_timeout, :only => [:index, :feed, :comments_feed] # Helper methods for checking consistency include ConsistencyValidations @@ -255,7 +255,7 @@ class ChangesetController < ApplicationController ## # list non-empty changesets in reverse chronological order - def list + def index @params = params.permit(:display_name, :bbox, :friends, :nearby, :max_id, :list) if request.format == :atom && @params[:max_id] @@ -300,14 +300,14 @@ class ChangesetController < ApplicationController @edits = changesets.order("changesets.id DESC").limit(20).preload(:user, :changeset_tags, :comments) - render :action => :list, :layout => false + render :action => :index, :layout => false end end ## # list edits as an atom feed def feed - list + index end ## @@ -481,6 +481,7 @@ class ChangesetController < ApplicationController u = if name.nil? # user input checking, we don't have any UIDs < 1 raise OSM::APIBadUserInput, "invalid user ID" if user.to_i < 1 + u = User.find(user.to_i) else u = User.find_by(:display_name => name) @@ -581,7 +582,7 @@ class ChangesetController < ApplicationController # Get the maximum number of comments to return def comments_limit if params[:limit] - if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 + if params[:limit].to_i.positive? && params[:limit].to_i <= 10000 params[:limit].to_i else raise OSM::APIBadUserInput, "Comments limit must be between 1 and 10000" diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb index f2c11fc74..723fff17e 100644 --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@ -4,11 +4,11 @@ class DiaryEntryController < ApplicationController before_action :authorize_web before_action :set_locale before_action :require_user, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe] - before_action :lookup_user, :only => [:view, :comments] + before_action :lookup_user, :only => [:show, :comments] before_action :check_database_readable before_action :check_database_writable, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe] before_action :require_administrator, :only => [:hide, :hidecomment] - before_action :allow_thirdparty_images, :only => [:new, :edit, :list, :view, :comments] + before_action :allow_thirdparty_images, :only => [:new, :edit, :index, :show, :comments] def new @title = t "diary_entry.new.title" @@ -29,7 +29,7 @@ class DiaryEntryController < ApplicationController # Subscribe user to diary comments @diary_entry.subscriptions.create(:user => current_user) - redirect_to :action => "list", :display_name => current_user.display_name + redirect_to :action => "index", :display_name => current_user.display_name else render :action => "edit" end @@ -47,9 +47,9 @@ class DiaryEntryController < ApplicationController @diary_entry = DiaryEntry.find(params[:id]) if current_user != @diary_entry.user - redirect_to :action => "view", :id => params[:id] + redirect_to diary_entry_path(@diary_entry.user, @diary_entry) elsif params[:diary_entry] && @diary_entry.update(entry_params) - redirect_to :action => "view", :id => params[:id] + redirect_to diary_entry_path(@diary_entry.user, @diary_entry) end set_map_location @@ -71,9 +71,9 @@ class DiaryEntryController < ApplicationController # Add the commenter to the subscribers if necessary @entry.subscriptions.create(:user => current_user) unless @entry.subscribers.exists?(current_user.id) - redirect_to :action => "view", :display_name => @entry.user.display_name, :id => @entry.id + redirect_to diary_entry_path(@entry.user, @entry) else - render :action => "view" + render :action => "show" end rescue ActiveRecord::RecordNotFound render :action => "no_such_entry", :status => :not_found @@ -84,7 +84,7 @@ class DiaryEntryController < ApplicationController diary_entry.subscriptions.create(:user => current_user) unless diary_entry.subscribers.exists?(current_user.id) - redirect_to :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id + redirect_to diary_entry_path(diary_entry.user, diary_entry) rescue ActiveRecord::RecordNotFound render :action => "no_such_entry", :status => :not_found end @@ -94,17 +94,17 @@ class DiaryEntryController < ApplicationController diary_entry.subscriptions.where(:user => current_user).delete_all if diary_entry.subscribers.exists?(current_user.id) - redirect_to :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id + redirect_to diary_entry_path(diary_entry.user, diary_entry) rescue ActiveRecord::RecordNotFound render :action => "no_such_entry", :status => :not_found end - def list + def index if params[:display_name] @user = User.active.find_by(:display_name => params[:display_name]) if @user - @title = t "diary_entry.list.user_title", :user => @user.display_name + @title = t "diary_entry.index.user_title", :user => @user.display_name @entries = @user.diary_entries else render_unknown_user params[:display_name] @@ -112,7 +112,7 @@ class DiaryEntryController < ApplicationController end elsif params[:friends] if current_user - @title = t "diary_entry.list.title_friends" + @title = t "diary_entry.index.title_friends" @entries = DiaryEntry.where(:user_id => current_user.friend_users) else require_user @@ -120,7 +120,7 @@ class DiaryEntryController < ApplicationController end elsif params[:nearby] if current_user - @title = t "diary_entry.list.title_nearby" + @title = t "diary_entry.index.title_nearby" @entries = DiaryEntry.where(:user_id => current_user.nearby) else require_user @@ -130,10 +130,10 @@ class DiaryEntryController < ApplicationController @entries = DiaryEntry.joins(:user).where(:users => { :status => %w[active confirmed] }) if params[:language] - @title = t "diary_entry.list.in_language_title", :language => Language.find(params[:language]).english_name + @title = t "diary_entry.index.in_language_title", :language => Language.find(params[:language]).english_name @entries = @entries.where(:language_code => params[:language]) else - @title = t "diary_entry.list.title" + @title = t "diary_entry.index.title" end end @@ -157,7 +157,7 @@ class DiaryEntryController < ApplicationController @entries = user.diary_entries @title = t("diary_entry.feed.user.title", :user => user.display_name) @description = t("diary_entry.feed.user.description", :user => user.display_name) - @link = url_for :controller => "diary_entry", :action => "list", :display_name => user.display_name, :host => SERVER_URL, :protocol => SERVER_PROTOCOL + @link = url_for :controller => "diary_entry", :action => "index", :display_name => user.display_name, :host => SERVER_URL, :protocol => SERVER_PROTOCOL else head :not_found return @@ -169,21 +169,21 @@ class DiaryEntryController < ApplicationController @entries = @entries.where(:language_code => params[:language]) @title = t("diary_entry.feed.language.title", :language_name => Language.find(params[:language]).english_name) @description = t("diary_entry.feed.language.description", :language_name => Language.find(params[:language]).english_name) - @link = url_for :controller => "diary_entry", :action => "list", :language => params[:language], :host => SERVER_URL, :protocol => SERVER_PROTOCOL + @link = url_for :controller => "diary_entry", :action => "index", :language => params[:language], :host => SERVER_URL, :protocol => SERVER_PROTOCOL else @title = t("diary_entry.feed.all.title") @description = t("diary_entry.feed.all.description") - @link = url_for :controller => "diary_entry", :action => "list", :host => SERVER_URL, :protocol => SERVER_PROTOCOL + @link = url_for :controller => "diary_entry", :action => "index", :host => SERVER_URL, :protocol => SERVER_PROTOCOL end end @entries = @entries.visible.includes(:user).order("created_at DESC").limit(20) end - def view + def show @entry = @user.diary_entries.visible.where(:id => params[:id]).first if @entry - @title = t "diary_entry.view.title", :user => params[:display_name], :title => @entry.title + @title = t "diary_entry.show.title", :user => params[:display_name], :title => @entry.title else @title = t "diary_entry.no_such_entry.title", :id => params[:id] render :action => "no_such_entry", :status => :not_found @@ -193,13 +193,13 @@ class DiaryEntryController < ApplicationController def hide entry = DiaryEntry.find(params[:id]) entry.update(:visible => false) - redirect_to :action => "list", :display_name => entry.user.display_name + redirect_to :action => "index", :display_name => entry.user.display_name end def hidecomment comment = DiaryComment.find(params[:comment]) comment.update(:visible => false) - redirect_to :action => "view", :display_name => comment.diary_entry.user.display_name, :id => comment.diary_entry.id + redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry) end def comments @@ -235,7 +235,7 @@ class DiaryEntryController < ApplicationController def require_administrator unless current_user.administrator? flash[:error] = t("user.filter.not_an_administrator") - redirect_to :action => "view" + redirect_to :action => "show" end end diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index d4e9a3bdf..ad38454f0 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -18,7 +18,7 @@ class IssuesController < ApplicationController @issues = Issue.visible_to(current_user) # If search - if params[:search_by_user] && params[:search_by_user].present? + if params[:search_by_user]&.present? @find_user = User.find_by(:display_name => params[:search_by_user]) if @find_user @issues = @issues.where(:reported_user_id => @find_user.id) @@ -28,11 +28,11 @@ class IssuesController < ApplicationController end end - @issues = @issues.where(:status => params[:status]) if params[:status] && params[:status].present? + @issues = @issues.where(:status => params[:status]) if params[:status]&.present? - @issues = @issues.where(:reportable_type => params[:issue_type]) if params[:issue_type] && params[:issue_type].present? + @issues = @issues.where(:reportable_type => params[:issue_type]) if params[:issue_type]&.present? - if params[:last_updated_by] && params[:last_updated_by].present? + if params[:last_updated_by]&.present? last_updated_by = params[:last_updated_by].to_s == "nil" ? nil : params[:last_updated_by].to_i @issues = @issues.where(:updated_by => last_updated_by) end diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 20baf6bb4..84b814a34 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -54,6 +54,7 @@ class NodeController < ApplicationController new_node = Node.from_xml(request.raw_post) 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 @@ -65,6 +66,7 @@ class NodeController < ApplicationController ids = params["nodes"].split(",").collect(&:to_i) 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| diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 94d0fdb55..19ff725dc 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -211,7 +211,7 @@ class NotesController < ApplicationController # Find the note and check it is valid @note = Note.find(params[:id]) raise OSM::APINotFoundError unless @note - raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || (current_user && current_user.moderator?) + raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || current_user&.moderator? # Render the result respond_to do |format| @@ -286,7 +286,7 @@ class NotesController < ApplicationController @page = (params[:page] || 1).to_i @page_size = 10 @notes = @user.notes - @notes = @notes.visible unless current_user && current_user.moderator? + @notes = @notes.visible unless current_user&.moderator? @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author).to_a else @title = t "user.no_such_user.title" @@ -307,7 +307,7 @@ class NotesController < ApplicationController # Get the maximum number of results to return def result_limit if params[:limit] - if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 + if params[:limit].to_i.positive? && params[:limit].to_i <= 10000 params[:limit].to_i else raise OSM::APIBadUserInput, "Note limit must be between 1 and 10000" @@ -327,9 +327,9 @@ class NotesController < ApplicationController 7 end - if closed_since < 0 + if closed_since.negative? notes.where.not(:status => "hidden") - elsif closed_since > 0 + elsif closed_since.positive? notes.where(:status => "open") .or(notes.where(:status => "closed") .where(notes.arel_table[:closed_at].gt(Time.now - closed_since.days))) diff --git a/app/controllers/old_controller.rb b/app/controllers/old_controller.rb index 9adf141d9..4f01b1e2a 100644 --- a/app/controllers/old_controller.rb +++ b/app/controllers/old_controller.rb @@ -70,6 +70,6 @@ class OldController < ApplicationController private def show_redactions? - current_user && current_user.moderator? && params[:show_redactions] == "true" + current_user&.moderator? && params[:show_redactions] == "true" end end diff --git a/app/controllers/traces_controller.rb b/app/controllers/traces_controller.rb index c77d884b3..b78ae2959 100644 --- a/app/controllers/traces_controller.rb +++ b/app/controllers/traces_controller.rb @@ -92,8 +92,8 @@ class TracesController < ApplicationController def show @trace = Trace.find(params[:id]) - if @trace && @trace.visible? && - (@trace.public? || @trace.user == current_user) + if @trace&.visible? && + (@trace&.public? || @trace&.user == current_user) @title = t ".title", :name => @trace.name else flash[:error] = t ".trace_not_found" @@ -318,7 +318,7 @@ class TracesController < ApplicationController visibility = params[:visibility] if visibility.nil? - visibility = if params[:public] && params[:public].to_i.nonzero? + visibility = if params[:public]&.to_i&.nonzero? "public" else "private" diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 7e22c63b4..78299dccf 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -14,11 +14,11 @@ class UserController < ApplicationController before_action :require_allow_read_prefs, :only => [:api_details] before_action :require_allow_read_gpx, :only => [:api_gpx_files] before_action :require_cookies, :only => [:new, :login, :confirm] - before_action :require_administrator, :only => [:set_status, :delete, :list] + before_action :require_administrator, :only => [:set_status, :delete, :index] around_action :api_call_handle_error, :only => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :lookup_user_by_id, :only => [:api_read] before_action :lookup_user_by_name, :only => [:set_status, :delete] - before_action :allow_thirdparty_images, :only => [:view, :account] + before_action :allow_thirdparty_images, :only => [:show, :account] def terms @legale = params[:legale] || OSM.ip_to_country(request.remote_ip) || DEFAULT_LEGALE @@ -29,7 +29,7 @@ class UserController < ApplicationController else @title = t "user.terms.title" - if current_user && current_user.terms_agreed? + if current_user&.terms_agreed? # Already agreed to terms, so just show settings redirect_to :action => :account, :display_name => current_user.display_name elsif current_user.nil? && session[:new_user].nil? @@ -276,7 +276,7 @@ class UserController < ApplicationController if params[:session] == session.id if session[:token] token = UserToken.find_by(:token => session[:token]) - token.destroy if token + token&.destroy session.delete(:token) end session.delete(:user) @@ -292,7 +292,7 @@ class UserController < ApplicationController def confirm if request.post? token = UserToken.find_by(:token => params[:confirm_string]) - if token && token.user.active? + if token&.user&.active? flash[:error] = t("user.confirm.already active") redirect_to :action => "login" elsif !token || token.expired? @@ -349,7 +349,7 @@ class UserController < ApplicationController def confirm_email if request.post? token = UserToken.find_by(:token => params[:confirm_string]) - if token && token.user.new_email? + if token&.user&.new_email? self.current_user = token.user current_user.email = current_user.new_email current_user.new_email = nil @@ -409,11 +409,11 @@ class UserController < ApplicationController render :xml => doc.to_s end - def view + def show @user = User.find_by(:display_name => params[:display_name]) if @user && - (@user.visible? || (current_user && current_user.administrator?)) + (@user.visible? || (current_user&.administrator?)) @title = @user.display_name else render_unknown_user params[:display_name] @@ -440,7 +440,7 @@ class UserController < ApplicationController if params[:referer] redirect_to params[:referer] else - redirect_to :action => "view" + redirect_to :action => "show" end end else @@ -463,7 +463,7 @@ class UserController < ApplicationController if params[:referer] redirect_to params[:referer] else - redirect_to :action => "view" + redirect_to :action => "show" end end else @@ -476,19 +476,19 @@ class UserController < ApplicationController def set_status @user.status = params[:status] @user.save - redirect_to :action => "view", :display_name => params[:display_name] + redirect_to user_path(:display_name => params[:display_name]) end ## # delete a user, marking them as deleted and removing personal data def delete @user.delete - redirect_to :action => "view", :display_name => params[:display_name] + redirect_to user_path(:display_name => params[:display_name]) end ## # display a list of users matching specified criteria - def list + def index if request.post? ids = params[:user].keys.collect(&:to_i) @@ -552,7 +552,7 @@ class UserController < ApplicationController if user.nil? && provider == "google" openid_url = auth_info[:extra][:id_info]["openid_id"] user = User.find_by(:auth_provider => "openid", :auth_uid => openid_url) if openid_url - user.update(:auth_provider => provider, :auth_uid => uid) if user + user&.update(:auth_provider => provider, :auth_uid => uid) end if user @@ -757,7 +757,7 @@ class UserController < ApplicationController flash[:error] = t("user.filter.not_an_administrator") if params[:display_name] - redirect_to :action => "view", :display_name => params[:display_name] + redirect_to user_path(:display_name => params[:display_name]) else redirect_to :action => "login", :referer => request.fullpath end @@ -826,6 +826,7 @@ class UserController < ApplicationController def gravatar_enable(user) # code from example https://en.gravatar.com/site/implement/images/ruby/ return false if user.image.present? + hash = Digest::MD5.hexdigest(user.email.downcase) url = "https://www.gravatar.com/avatar/#{hash}?d=404" # without d=404 we will always get an image back response = OSM.http_client.get(URI.parse(url)) diff --git a/app/helpers/banner_helper.rb b/app/helpers/banner_helper.rb index 661369454..fef6eaa5e 100644 --- a/app/helpers/banner_helper.rb +++ b/app/helpers/banner_helper.rb @@ -26,7 +26,7 @@ module BannerHelper # rotate all banner queue positions index = cval.to_i - cookies[ckey] = index - 1 if index > 0 + cookies[ckey] = index - 1 if index.positive? # pick banner with mininum queue position next if index > min_index diff --git a/app/helpers/changeset_helper.rb b/app/helpers/changeset_helper.rb index 7eb389023..bc19bc857 100644 --- a/app/helpers/changeset_helper.rb +++ b/app/helpers/changeset_helper.rb @@ -3,7 +3,7 @@ module ChangesetHelper if changeset.user.status == "deleted" t("user.no_such_user.deleted") elsif changeset.user.data_public? - link_to(changeset.user.display_name, user_path(changeset.user.display_name)) + link_to(changeset.user.display_name, user_path(changeset.user)) else t("browse.anonymous") end @@ -32,15 +32,15 @@ module ChangesetHelper end end - def changeset_list_title(params, user) + def changeset_index_title(params, user) if params[:friends] && user - t "changeset.list.title_friend" + t "changeset.index.title_friend" elsif params[:nearby] && user - t "changeset.list.title_nearby" + t "changeset.index.title_nearby" elsif params[:display_name] - t "changeset.list.title_user", :user => params[:display_name] + t "changeset.index.title_user", :user => params[:display_name] else - t "changeset.list.title" + t "changeset.index.title" end end end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index f9a84ba7d..43ca98c91 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -2,11 +2,11 @@ module IssuesHelper def reportable_url(reportable) case reportable when DiaryEntry - url_for(:controller => reportable.class.name.underscore, :action => :view, :display_name => reportable.user.display_name, :id => reportable.id) + diary_entry_url(reportable.user, reportable) when User - url_for(:controller => reportable.class.name.underscore, :action => :view, :display_name => reportable.display_name) + user_url(reportable) when DiaryComment - url_for(:controller => reportable.diary_entry.class.name.underscore, :action => :view, :display_name => reportable.diary_entry.user.display_name, :id => reportable.diary_entry.id, :anchor => "comment#{reportable.id}") + diary_entry_url(reportable.diary_entry.user, reportable.diary_entry, :anchor => "comment#{reportable.id}") when Note url_for(:controller => :browse, :action => :note, :id => reportable.id) end @@ -29,7 +29,7 @@ module IssuesHelper count = Issue.visible_to(current_user).open.limit(100).size if count > 99 content_tag(:span, "99+", :class => "count-number") - elsif count > 0 + elsif count.positive? content_tag(:span, count, :class => "count-number") end end diff --git a/app/helpers/note_helper.rb b/app/helpers/note_helper.rb index 6ebd18345..efe2346c9 100644 --- a/app/helpers/note_helper.rb +++ b/app/helpers/note_helper.rb @@ -18,7 +18,7 @@ module NoteHelper elsif author.status == "deleted" t("user.no_such_user.deleted") else - link_to h(author.display_name), link_options.merge(:controller => "user", :action => "view", :display_name => author.display_name) + link_to h(author.display_name), link_options.merge(:controller => "user", :action => "show", :display_name => author.display_name) end end end diff --git a/app/helpers/notifier_helper.rb b/app/helpers/notifier_helper.rb index dbd2d245d..2915a0880 100644 --- a/app/helpers/notifier_helper.rb +++ b/app/helpers/notifier_helper.rb @@ -29,6 +29,6 @@ module NotifierHelper # Because we can't use stylesheets in HTML emails, we need to inline the # styles. Rather than copy-paste the same string of CSS into every message, # we apply it once here, after the message has been composed. - html.gsub /

/, '

' + html.gsub(/

/, '

') end end diff --git a/app/helpers/user_roles_helper.rb b/app/helpers/user_roles_helper.rb index 805abb58f..0b7e9c616 100644 --- a/app/helpers/user_roles_helper.rb +++ b/app/helpers/user_roles_helper.rb @@ -6,24 +6,24 @@ module UserRolesHelper end def role_icon(user, role) - if current_user && current_user.administrator? + if current_user&.administrator? if user.has_role?(role) image = "roles/#{role}" - alt = t("user.view.role.revoke.#{role}") - title = t("user.view.role.revoke.#{role}") + alt = t("user.show.role.revoke.#{role}") + title = t("user.show.role.revoke.#{role}") url = revoke_role_path(:display_name => user.display_name, :role => role) confirm = t("user_role.revoke.are_you_sure", :name => user.display_name, :role => role) else image = "roles/blank_#{role}" - alt = t("user.view.role.grant.#{role}") - title = t("user.view.role.grant.#{role}") + alt = t("user.show.role.grant.#{role}") + title = t("user.show.role.grant.#{role}") url = grant_role_path(:display_name => user.display_name, :role => role) confirm = t("user_role.grant.are_you_sure", :name => user.display_name, :role => role) end elsif user.has_role?(role) image = "roles/#{role}" - alt = t("user.view.role.#{role}") - title = t("user.view.role.#{role}") + alt = t("user.show.role.#{role}") + title = t("user.show.role.#{role}") end if image diff --git a/app/models/changeset.rb b/app/models/changeset.rb index 1aa1e1fd3..5cdfeb994 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -106,6 +106,7 @@ class Changeset < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag["v"].nil? + cs.add_tag_keyval(tag["k"], tag["v"]) end @@ -207,7 +208,7 @@ class Changeset < ActiveRecord::Base user_display_name_cache = {} if user_display_name_cache.nil? - if user_display_name_cache && user_display_name_cache.key?(user_id) + if user_display_name_cache&.key?(user_id) # use the cache if available elsif user.data_public? user_display_name_cache[user_id] = user.display_name diff --git a/app/models/client_application.rb b/app/models/client_application.rb index ef1a0110e..d11942beb 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -51,12 +51,13 @@ class ClientApplication < ActiveRecord::Base def self.find_token(token_key) token = OauthToken.includes(:client_application).find_by(:token => token_key) - token if token && token.authorized? + token if token&.authorized? end def self.verify_request(request, options = {}, &block) signature = OAuth::Signature.build(request, options, &block) return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp) + value = signature.verify value rescue OAuth::Signature::UnknownSignatureMethod diff --git a/app/models/node.rb b/app/models/node.rb index c09fcbd67..989cdee5c 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -88,19 +88,23 @@ class Node < ActiveRecord::Base raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt["lat"].nil? raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt["lon"].nil? + node.lat = OSM.parse_float(pt["lat"], OSM::APIBadXMLError, "node", pt, "lat not a number") node.lon = OSM.parse_float(pt["lon"], OSM::APIBadXMLError, "node", pt, "lon not a number") raise OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt["changeset"].nil? + node.changeset_id = pt["changeset"].to_i raise OSM::APIBadUserInput, "The node is outside this world" unless node.in_world? # version must be present unless creating raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt["version"].nil? + node.version = create ? 0 : pt["version"].to_i unless create raise OSM::APIBadXMLError.new("node", pt, "ID is required when updating.") if pt["id"].nil? + node.id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -119,6 +123,7 @@ class Node < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag["v"].nil? + node.add_tag_key_val(tag["k"], tag["v"]) end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 11658cfd9..01b807607 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -87,16 +87,8 @@ class Notifier < ActionMailer::Base @from_user = comment.user.display_name @text = comment.body @title = comment.diary_entry.title - @readurl = url_for(:controller => "diary_entry", - :action => "view", - :display_name => comment.diary_entry.user.display_name, - :id => comment.diary_entry.id, - :anchor => "comment#{comment.id}") - @commenturl = url_for(:controller => "diary_entry", - :action => "view", - :display_name => comment.diary_entry.user.display_name, - :id => comment.diary_entry.id, - :anchor => "newcomment") + @readurl = diary_entry_url(comment.diary_entry.user, comment.diary_entry, :anchor => "comment#{comment.id}") + @commenturl = diary_entry_url(comment.diary_entry.user, comment.diary_entry, :anchor => "newcomment") @replyurl = new_message_url(comment.user, :message => { :title => "Re: #{comment.diary_entry.title}" }) @author = @from_user @@ -112,8 +104,7 @@ class Notifier < ActionMailer::Base def friend_notification(friend) with_recipient_locale friend.befriendee do @friend = friend - @viewurl = url_for(:controller => "user", :action => "view", - :display_name => @friend.befriender.display_name) + @viewurl = user_url(@friend.befriender) @friendurl = url_for(:controller => "user", :action => "make_friend", :display_name => @friend.befriender.display_name) @author = @friend.befriender.display_name @@ -190,8 +181,8 @@ class Notifier < ActionMailer::Base end def user_avatar_file_path(user) - image = user && user.image - if image && image.file? + image = user&.image + if image&.file? return image.path(:small) else return Rails.root.join("app", "assets", "images", "users", "images", "small.png") diff --git a/app/models/oauth_nonce.rb b/app/models/oauth_nonce.rb index 0952f068e..9d2773e8f 100644 --- a/app/models/oauth_nonce.rb +++ b/app/models/oauth_nonce.rb @@ -22,8 +22,10 @@ class OauthNonce < ActiveRecord::Base # Remembers a nonce and it's associated timestamp. It returns false if it has already been used def self.remember(nonce, timestamp) return false if Time.now.to_i - timestamp.to_i > 86400 + oauth_nonce = OauthNonce.create(:nonce => nonce, :timestamp => timestamp.to_i) return false if oauth_nonce.new_record? + oauth_nonce end end diff --git a/app/models/relation.rb b/app/models/relation.rb index 2495830ee..202db12da 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -70,12 +70,15 @@ class Relation < ActiveRecord::Base relation = Relation.new raise OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create || !pt["version"].nil? + relation.version = pt["version"] raise OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt["changeset"].nil? + relation.changeset_id = pt["changeset"] unless create raise OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt["id"].nil? + relation.id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -94,6 +97,7 @@ class Relation < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag["v"].nil? + relation.add_tag_keyval(tag["k"], tag["v"]) end @@ -106,6 +110,7 @@ class Relation < ActiveRecord::Base pt.find("member").each do |member| # member_type = raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"] + # member_ref = member['ref'] # member_role member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string. @@ -207,6 +212,7 @@ class Relation < ActiveRecord::Base lock! check_consistency(self, new_relation, user) 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 @@ -219,6 +225,7 @@ class Relation < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) raise OSM::APIPreconditionFailedError, "Cannot create relation: data or member data is invalid." unless preconditions_ok? + self.version = 0 self.visible = true save_with_history! @@ -253,7 +260,8 @@ class Relation < ActiveRecord::Base element = model.lock("for share").find_by(:id => m[1]) # and check that it is OK to use. - 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? + raise OSM::APIPreconditionFailedError, "Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}" unless element&.visible? && element&.preconditions_ok? + hash[m[1]] = true end @@ -267,9 +275,10 @@ class Relation < ActiveRecord::Base def fix_placeholders!(id_map, placeholder_id = nil) members.map! do |type, id, role| old_id = id.to_i - if old_id < 0 + if old_id.negative? new_id = id_map[type.downcase.to_sym][old_id] raise OSM::APIBadUserInput, "Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}." if new_id.nil? + [type, new_id, role] else [type, id, role] @@ -372,7 +381,7 @@ class Relation < ActiveRecord::Base # reasonable on the assumption that adding or removing members doesn't # materially change the rest of the relation. any_relations = - changed_members.collect { |_id, type| type == "relation" } + changed_members.collect { |type, _id, _role| type == "Relation" } .inject(false) { |acc, elem| acc || elem } update_members = if tags_changed || any_relations diff --git a/app/models/request_token.rb b/app/models/request_token.rb index 335a735bc..ed0cc3ae4 100644 --- a/app/models/request_token.rb +++ b/app/models/request_token.rb @@ -40,6 +40,7 @@ class RequestToken < OauthToken def authorize!(user) return false if authorized? + self.user = user self.authorized_at = Time.now self.verifier = OAuth::Helper.generate_key(20)[0, 20] unless oauth10? diff --git a/app/models/trace.rb b/app/models/trace.rb index 214b0b647..5096a81aa 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -214,10 +214,12 @@ class Trace < ActiveRecord::Base def update_from_xml_node(pt, create = false) raise OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt["visibility"].nil? + self.visibility = pt["visibility"] unless create raise OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt["id"].nil? + id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -232,6 +234,7 @@ class Trace < ActiveRecord::Base description = pt.find("description").first raise OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil? + self.description = description.content self.tags = pt.find("tag").collect do |tag| @@ -303,7 +306,7 @@ class Trace < ActiveRecord::Base tp.save! end - if gpx.actual_points > 0 + if gpx.actual_points.positive? max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude) min_lat = Tracepoint.where(:gpx_id => id).minimum(:latitude) max_lon = Tracepoint.where(:gpx_id => id).maximum(:longitude) diff --git a/app/models/way.rb b/app/models/way.rb index e5b73ceaa..c95a12122 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -68,12 +68,15 @@ class Way < ActiveRecord::Base way = Way.new raise OSM::APIBadXMLError.new("way", pt, "Version is required when updating") unless create || !pt["version"].nil? + way.version = pt["version"] raise OSM::APIBadXMLError.new("way", pt, "Changeset id is missing") if pt["changeset"].nil? + way.changeset_id = pt["changeset"] unless create raise OSM::APIBadXMLError.new("way", pt, "ID is required when updating") if pt["id"].nil? + way.id = pt["id"].to_i # .to_i will return 0 if there is no number that can be parsed. # We want to make sure that there is no id with zero anyway @@ -92,6 +95,7 @@ class Way < ActiveRecord::Base pt.find("tag").each do |tag| raise OSM::APIBadXMLError.new("way", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("way", pt, "tag is missing value") if tag["v"].nil? + way.add_tag_keyval(tag["k"], tag["v"]) end @@ -123,7 +127,7 @@ class Way < ActiveRecord::Base 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 - ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node && nd.node.visible? + ordered_nodes[nd.sequence_id] = nd.node_id.to_s if nd.node&.visible? end end @@ -194,6 +198,7 @@ class Way < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) raise OSM::APIPreconditionFailedError, "Cannot create way: data is invalid." unless preconditions_ok? + self.version = 0 self.visible = true save_with_history! @@ -249,9 +254,10 @@ class Way < ActiveRecord::Base # to IDs +id_map+. def fix_placeholders!(id_map, placeholder_id = nil) nds.map! do |node_id| - if node_id < 0 + if node_id.negative? new_id = id_map[:node][node_id] raise OSM::APIBadUserInput, "Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}" if new_id.nil? + new_id else node_id diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb index 85b9515df..545b6d072 100644 --- a/app/views/browse/changeset.html.erb +++ b/app/views/browse/changeset.html.erb @@ -127,7 +127,7 @@ <% end %> <%= user = (@prev_by_user || @next_by_user).user.display_name - link_to content_tag(:bdi, user), :controller => "changeset", :action => "list", :display_name => user + link_to content_tag(:bdi, user), :controller => "changeset", :action => "index", :display_name => user %> <% if @next_by_user %> · diff --git a/app/views/changeset/_changeset.html.erb b/app/views/changeset/_changeset.html.erb index f27b0bc91..03ba013a3 100644 --- a/app/views/changeset/_changeset.html.erb +++ b/app/views/changeset/_changeset.html.erb @@ -14,7 +14,7 @@ <%= content_tag "li", :id => "changeset_#{changeset.id}", :data => {:changeset => changeset_data} do %>

- + <%= changeset.tags['comment'].to_s.presence || t('browse.no_comment') %>

diff --git a/app/views/changeset/history.html.erb b/app/views/changeset/history.html.erb index 17faf7f8d..ba745cb03 100644 --- a/app/views/changeset/history.html.erb +++ b/app/views/changeset/history.html.erb @@ -5,9 +5,9 @@ <% end -%> <% - set_title(changeset_list_title(params, current_user)) + set_title(changeset_index_title(params, current_user)) if params[:display_name] - @heading = t('changeset.list.title_user', :user => link_to(params[:display_name], :controller => "user", :action => "view", :display_name => params[:display_name])).html_safe + @heading = t('changeset.index.title_user', :user => link_to(params[:display_name], user_path(:display_name => params[:display_name]))).html_safe else @heading = @title end diff --git a/app/views/changeset/list.atom.builder b/app/views/changeset/index.atom.builder similarity index 81% rename from app/views/changeset/list.atom.builder rename to app/views/changeset/index.atom.builder index 86dd966cc..57befe117 100644 --- a/app/views/changeset/list.atom.builder +++ b/app/views/changeset/index.atom.builder @@ -1,8 +1,8 @@ atom_feed(:language => I18n.locale, :schema_date => 2009, :id => url_for(@params.merge(:only_path => false)), - :root_url => url_for(@params.merge(:action => :list, :format => nil, :only_path => false)), + :root_url => url_for(@params.merge(:action => :index, :format => nil, :only_path => false)), "xmlns:georss" => "http://www.georss.org/georss") do |feed| - feed.title changeset_list_title(params, current_user) + feed.title changeset_index_title(params, current_user) feed.updated @edits.map { |e| [e.created_at, e.closed_at].max }.max feed.icon image_url("favicon.ico") @@ -32,7 +32,7 @@ atom_feed(:language => I18n.locale, :schema_date => 2009, if changeset.user.data_public? entry.author do |author| author.name changeset.user.display_name - author.uri url_for(:controller => "user", :action => "view", :display_name => changeset.user.display_name, :only_path => false) + author.uri user_url(changeset.user, :only_path => false) end end @@ -51,7 +51,7 @@ atom_feed(:language => I18n.locale, :schema_date => 2009, table.tr do |tr| tr.th t("browse.changeset.belongs_to") tr.td do |td| - td.a h(changeset.user.display_name), :href => url_for(:controller => "user", :action => "view", :display_name => changeset.user.display_name, :only_path => false) + td.a h(changeset.user.display_name), :href => user_url(changeset.user, :only_path => false) end end end @@ -59,10 +59,10 @@ atom_feed(:language => I18n.locale, :schema_date => 2009, table.tr do |tr| tr.th t("browse.tag_details.tags") tr.td do |td| - td.table :cellpadding => "0" do |table| + td.table :cellpadding => "0" do |tag_table| changeset.tags.sort.each do |tag| - table.tr do |tr| - tr.td << "#{h(tag[0])} = #{linkify(h(tag[1]))}" + tag_table.tr do |tag_tr| + tag_tr.td << "#{h(tag[0])} = #{linkify(h(tag[1]))}" end end end diff --git a/app/views/changeset/list.html.erb b/app/views/changeset/index.html.erb similarity index 100% rename from app/views/changeset/list.html.erb rename to app/views/changeset/index.html.erb diff --git a/app/views/diary_entry/_diary_entry.html.erb b/app/views/diary_entry/_diary_entry.html.erb index 930f20e5e..ec2f22601 100644 --- a/app/views/diary_entry/_diary_entry.html.erb +++ b/app/views/diary_entry/_diary_entry.html.erb @@ -4,10 +4,10 @@ <%= user_thumbnail diary_entry.user %> <% end %> -

<%= link_to h(diary_entry.title), :action => 'view', :display_name => diary_entry.user.display_name, :id => diary_entry.id %>

+

<%= link_to h(diary_entry.title), diary_entry_path(diary_entry.user, diary_entry) %>

- <%= raw(t '.posted_by', :link_user => (link_to h(diary_entry.user.display_name), user_path(diary_entry.user)), :created => l(diary_entry.created_at, :format => :blog), :language_link => (link_to h(diary_entry.language.name), :controller => 'diary_entry', :action => 'list', :display_name => nil, :language => diary_entry.language_code)) %> + <%= raw(t '.posted_by', :link_user => (link_to h(diary_entry.user.display_name), user_path(diary_entry.user)), :created => l(diary_entry.created_at, :format => :blog), :language_link => (link_to h(diary_entry.language.name), :controller => 'diary_entry', :action => 'index', :display_name => nil, :language => diary_entry.language_code)) %> @@ -21,10 +21,10 @@ <% end %>