From: Tom Hughes Date: Sun, 4 Nov 2018 14:49:27 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2042' X-Git-Tag: live~3805 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/acfbc091982fbf74baeef7d16ff7d84f63c84951?hp=d9a48d66f977afbe9d5e196e9e22d882b16b3566 Merge remote-tracking branch 'upstream/pull/2042' --- diff --git a/.mailmap b/.mailmap index 29f21fde6..761478b56 100644 --- a/.mailmap +++ b/.mailmap @@ -7,4 +7,4 @@ Kai Krueger Michael Glanznig Petr Kadlec Richard Fairhurst -Simon Poole +Simon Poole diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8fc701cb3..7d2c583eb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -19,7 +19,7 @@ Lint/AssignmentInCondition: - 'app/helpers/application_helper.rb' - 'app/helpers/browse_helper.rb' - 'app/models/client_application.rb' - - 'app/models/notifier.rb' + - 'app/mailers/notifier.rb' - 'lib/nominatim.rb' - 'lib/osm.rb' - 'script/deliver-message' @@ -32,7 +32,7 @@ Lint/HandleExceptions: # Offense count: 692 Metrics/AbcSize: - Max: 280 + Max: 283 # Offense count: 40 # Configuration parameters: CountComments, ExcludedMethods. @@ -48,7 +48,7 @@ Metrics/BlockNesting: # Offense count: 63 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 1795 + Max: 1801 # Offense count: 72 Metrics/CyclomaticComplexity: diff --git a/.travis.yml b/.travis.yml index 8ce7f5f30..3b56d2b4b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: false language: ruby rvm: - - 2.3.1 + - 2.3.3 cache: bundler addons: postgresql: 9.5 diff --git a/Gemfile b/Gemfile index 9ba270313..05bfc6cbd 100644 --- a/Gemfile +++ b/Gemfile @@ -45,7 +45,9 @@ gem "image_optim_rails" # Load rails plugins gem "actionpack-page_caching" +gem "cancancan" gem "composite_primary_keys", "~> 11.0.0" +gem "delayed_job_active_record" gem "dynamic_form" gem "http_accept_language", "~> 2.0.0" gem "i18n-js", ">= 3.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index 76a31e169..72f769929 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -21,7 +21,7 @@ GEM rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionpack-page_caching (1.1.0) + actionpack-page_caching (1.1.1) actionpack (>= 4.0.0, < 6) actionview (5.2.0) activesupport (= 5.2.0) @@ -66,6 +66,7 @@ GEM bootsnap (1.3.2) msgpack (~> 1.0) builder (3.2.3) + cancancan (2.1.3) canonical-rails (0.2.4) rails (>= 4.1, < 5.3) capybara (2.18.0) @@ -97,8 +98,13 @@ GEM crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) - dalli (2.7.8) + dalli (2.7.9) debug_inspector (0.0.3) + delayed_job (4.1.5) + activesupport (>= 3.0, < 5.3) + delayed_job_active_record (4.1.3) + activerecord (>= 3.0, < 5.3) + delayed_job (>= 3.0, < 5) docile (1.3.1) dynamic_form (1.1.4) erubi (1.7.1) @@ -109,7 +115,7 @@ GEM factory_bot_rails (4.11.1) factory_bot (~> 4.11.1) railties (>= 3.0.0) - faraday (0.12.2) + faraday (0.15.3) multipart-post (>= 1.2, < 3) ffi (1.9.25) fspath (3.1.0) @@ -122,9 +128,9 @@ GEM http_accept_language (2.0.5) i18n (0.9.5) concurrent-ruby (~> 1.0) - i18n-js (3.0.11) + i18n-js (3.1.0) i18n (>= 0.6.6, < 2) - image_optim (0.26.2) + image_optim (0.26.3) exifr (~> 1.2, >= 1.2.2) fspath (~> 3.0) image_size (>= 1.5, < 3) @@ -160,14 +166,14 @@ GEM rb-inotify (~> 0.9, >= 0.9.7) ruby_dep (~> 1.2) logstash-event (1.2.02) - logstasher (1.2.2) + logstasher (1.3.0) activesupport (>= 4.0) logstash-event (~> 1.2.0) request_store - loofah (2.2.2) + loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) - mail (2.7.0) + mail (2.7.1) mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) @@ -184,19 +190,19 @@ GEM multi_xml (0.6.0) multipart-post (2.0.0) nio4r (2.3.1) - nokogiri (1.8.4) + nokogiri (1.8.5) mini_portile2 (~> 2.3.0) - nokogumbo (1.5.0) - nokogiri + nokogumbo (2.0.0) + nokogiri (~> 1.8, >= 1.8.4) oauth (0.4.7) oauth-plugin (0.5.1) multi_json oauth (~> 0.4.4) oauth2 (>= 0.5.0) rack - oauth2 (1.4.0) - faraday (>= 0.8, < 0.13) - jwt (~> 1.0) + oauth2 (1.4.1) + faraday (>= 0.8, < 0.16.0) + jwt (>= 1.0, < 3.0) multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) @@ -235,7 +241,7 @@ GEM mimemagic (~> 0.3.0) terrapin (~> 0.6.0) parallel (1.12.1) - parser (2.5.1.2) + parser (2.5.3.0) ast (~> 2.4.0) pg (0.21.0) poltergeist (1.18.1) @@ -243,8 +249,8 @@ GEM cliver (~> 0.3.1) websocket-driver (>= 0.2.0) powerpack (0.1.2) - progress (3.4.0) - psych (3.0.2) + progress (3.5.0) + psych (3.0.3) public_suffix (3.0.3) puma (3.12.0) quad_tile (1.0.1) @@ -300,36 +306,31 @@ GEM request_store (1.4.1) rack (>= 1.4) rinku (2.0.4) - rotp (3.3.1) - rubocop (0.59.1) + rotp (4.0.2) + addressable (~> 2.5) + rubocop (0.60.0) jaro_winkler (~> 1.5.1) parallel (~> 1.10) parser (>= 2.5, != 2.5.1.1) powerpack (~> 0.1) rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) - unicode-display_width (~> 1.0, >= 1.0.1) + unicode-display_width (~> 1.4.0) ruby-openid (2.7.0) ruby-progressbar (1.10.0) ruby_dep (1.5.0) safe_yaml (1.0.4) - sanitize (4.6.6) + sanitize (5.0.0) crass (~> 1.0.2) - nokogiri (>= 1.4.4) - nokogumbo (~> 1.4) - sass (3.6.0) - sass-listen (~> 4.0.0) - sass-listen (4.0.0) - rb-fsevent (~> 0.9, >= 0.9.4) - rb-inotify (~> 0.9, >= 0.9.7) - sassc (1.12.1) + nokogiri (>= 1.8.0) + nokogumbo (~> 2.0) + sassc (2.0.0) ffi (~> 1.9.6) - sass (>= 3.3.0) - sassc-rails (1.3.0) + rake + sassc-rails (2.0.0) railties (>= 4.0.0) - sass - sassc (~> 1.9) - sprockets (> 2.11) + sassc (>= 2.0) + sprockets (> 3.0) sprockets-rails tilt secure_headers (6.0.0) @@ -355,7 +356,7 @@ GEM thor (0.19.4) thread_safe (0.3.6) tilt (2.0.8) - tins (1.16.3) + tins (1.17.0) tzinfo (1.2.5) thread_safe (~> 0.1) uglifier (4.1.19) @@ -363,7 +364,7 @@ GEM unicode-display_width (1.4.0) validates_email_format_of (1.6.3) i18n - vendorer (0.1.16) + vendorer (0.2.0) webmock (3.4.2) addressable (>= 2.3.6) crack (>= 0.3.2) @@ -371,7 +372,7 @@ GEM websocket-driver (0.7.0) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.3) - xpath (3.1.0) + xpath (3.2.0) nokogiri (~> 1.8) PLATFORMS @@ -387,12 +388,14 @@ DEPENDENCIES bigdecimal (~> 1.1.0) binding_of_caller bootsnap (>= 1.1.0) + cancancan canonical-rails capybara (~> 2.13) coffee-rails (~> 4.2) composite_primary_keys (~> 11.0.0) coveralls dalli + delayed_job_active_record dynamic_form factory_bot_rails faraday diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb new file mode 100644 index 000000000..f55f19e4e --- /dev/null +++ b/app/abilities/ability.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +class Ability + include CanCan::Ability + + def initialize(user) + can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id], :site + can [:index, :rss, :show, :comments], DiaryEntry + can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim, + :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder + + if user + can :welcome, :site + can [:create, :edit, :comment, :subscribe, :unsubscribe], DiaryEntry + can [:new, :create], Report + can [:read, :read_one, :update, :update_one, :delete_one], UserPreference + + if user.moderator? + can [:index, :show, :resolve, :ignore, :reopen], Issue + can :create, IssueComment + end + + if user.administrator? + can [:hide, :hidecomment], [DiaryEntry, DiaryComment] + can [:index, :show, :resolve, :ignore, :reopen], Issue + can :create, IssueComment + end + end + + # Define abilities for the passed in user here. For example: + # + # user ||= User.new # guest user (not logged in) + # if user.admin? + # can :manage, :all + # else + # can :read, :all + # end + # + # The first argument to `can` is the action you are giving the user + # permission to do. + # If you pass :manage it will apply to every action. Other common actions + # here are :read, :create, :update and :destroy. + # + # The second argument is the resource the user can perform the action on. + # If you pass :all it will apply to every resource. Otherwise pass a Ruby + # class of the resource. + # + # The third argument is an optional hash of conditions to further filter the + # objects. + # For example, here the user can only update published articles. + # + # can :update, Article, :published => true + # + # See the wiki for details: + # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities + end +end diff --git a/app/abilities/capability.rb b/app/abilities/capability.rb new file mode 100644 index 000000000..2a5c92774 --- /dev/null +++ b/app/abilities/capability.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class Capability + include CanCan::Ability + + def initialize(token) + can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs) + can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs) + end + + private + + def capability?(token, cap) + token&.read_attribute(cap) + end +end diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index 56c4515d0..a16f7d85a 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -1324,11 +1324,11 @@ tr.turn:hover { #maxlat { margin-top: -1px; } #minlon { float: left; - margin-left: -1px; + /* no-r2 */ margin-left: -1px; } #maxlon { float: right; - margin-right: -1px; + /* no-r2 */ margin-right: -1px; } #minlat { margin-bottom: 0; } } @@ -2314,6 +2314,7 @@ a.button { font-size: 13px; background: #e8e8e8; padding: 2px 3px; + white-space: pre-wrap; code { padding: 0; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bd1995014..1df6dd7d1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,8 @@ class ApplicationController < ActionController::Base protect_from_forgery :with => :exception + rescue_from CanCan::AccessDenied, :with => :deny_access + before_action :fetch_body around_action :better_errors_allow_inline, :if => proc { Rails.env.development? } @@ -22,7 +24,7 @@ class ApplicationController < ActionController::Base # don't allow access to any auth-requiring part of the site unless # the new CTs have been seen (and accept/decline chosen). elsif !current_user.terms_seen && flash[:skip_terms].nil? - flash[:notice] = t "user.terms.you need to accept or decline" + flash[:notice] = t "users.terms.you need to accept or decline" if params[:referer] redirect_to :controller => "users", :action => "terms", :referer => params[:referer] else @@ -466,6 +468,29 @@ class ApplicationController < ActionController::Base raise end + def current_ability + # Add in capabilities from the oauth token if it exists and is a valid access token + if Authenticator.new(self, [:token]).allow? + Ability.new(current_user).merge(Capability.new(current_token)) + else + Ability.new(current_user) + end + end + + def deny_access(_exception) + if current_token + set_locale + report_error t("oauth.permissions.missing"), :forbidden + elsif current_user + set_locale + report_error t("application.permission_denied"), :forbidden + elsif request.get? + redirect_to :controller => "users", :action => "login", :referer => request.fullpath + else + head :forbidden + end + end + private # extract authorisation credentials from headers, returns user = nil if none diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index c03ed9056..4ce205fd1 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -332,7 +332,7 @@ class ChangesetController < ApplicationController # Notify current subscribers of the new comment changeset.subscribers.visible.each do |user| - Notifier.changeset_comment_notification(comment, user).deliver_now if current_user != user + Notifier.changeset_comment_notification(comment, user).deliver_later if current_user != user end # Add the commenter to the subscribers if necessary diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb index 723fff17e..70cb1654d 100644 --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@ -3,11 +3,12 @@ class DiaryEntryController < ApplicationController before_action :authorize_web before_action :set_locale - before_action :require_user, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe] + + authorize_resource + 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, :index, :show, :comments] def new @@ -65,7 +66,7 @@ class DiaryEntryController < ApplicationController # Notify current subscribers of the new comment @entry.subscribers.visible.each do |user| - Notifier.diary_comment_notification(@diary_comment, user).deliver_now if current_user != user + Notifier.diary_comment_notification(@diary_comment, user).deliver_later if current_user != user end # Add the commenter to the subscribers if necessary @@ -215,6 +216,22 @@ class DiaryEntryController < ApplicationController private + # This is required because, being a default-deny system, cancancan + # _cannot_ tell you the reason you were denied access; and so + # the "nice" feedback presenting next steps can't be gleaned from + # the exception + ## + # for the hide actions, require that the user is a administrator, or fill out + # a helpful error message and return them to the user page. + def deny_access(exception) + if current_user && exception.action.in?([:hide, :hidecomment]) + flash[:error] = t("users.filter.not_an_administrator") + redirect_to :action => "show" + else + super + end + end + ## # return permitted diary entry parameters def entry_params @@ -229,16 +246,6 @@ class DiaryEntryController < ApplicationController params.require(:diary_comment).permit(:body) end - ## - # require that the user is a administrator, or fill out a helpful error message - # and return them to the user page. - def require_administrator - unless current_user.administrator? - flash[:error] = t("user.filter.not_an_administrator") - redirect_to :action => "show" - end - end - ## # decide on a location for the diary entry map def set_map_location diff --git a/app/controllers/issue_comments_controller.rb b/app/controllers/issue_comments_controller.rb index 8d1acec75..0e4a7079e 100644 --- a/app/controllers/issue_comments_controller.rb +++ b/app/controllers/issue_comments_controller.rb @@ -3,8 +3,8 @@ class IssueCommentsController < ApplicationController before_action :authorize_web before_action :set_locale - before_action :require_user - before_action :check_permission + + authorize_resource def create @issue = Issue.find(params[:issue_id]) @@ -22,10 +22,12 @@ class IssueCommentsController < ApplicationController params.require(:issue_comment).permit(:body) end - def check_permission - unless current_user.administrator? || current_user.moderator? + def deny_access(_exception) + if current_user flash[:error] = t("application.require_moderator_or_admin.not_a_moderator_or_admin") redirect_to root_path + else + super end end diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index ad38454f0..8943f2d4a 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -3,8 +3,9 @@ class IssuesController < ApplicationController before_action :authorize_web before_action :set_locale - before_action :require_user - before_action :check_permission + + authorize_resource + before_action :find_issue, :only => [:show, :resolve, :reopen, :ignore] def index @@ -82,10 +83,12 @@ class IssuesController < ApplicationController @issue = Issue.find(params[:id]) end - def check_permission - unless current_user.administrator? || current_user.moderator? + def deny_access(_exception) + if current_user flash[:error] = t("application.require_moderator_or_admin.not_a_moderator_or_admin") redirect_to root_path + else + super end end end diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 13a395da8..c93c998f0 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -29,7 +29,7 @@ class MessagesController < ApplicationController render :action => "new" elsif @message.save flash[:notice] = t ".message_sent" - Notifier.message_notification(@message).deliver_now + Notifier.message_notification(@message).deliver_later redirect_to :action => :inbox else render :action => "new" diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 95566a1a1..9cdc38446 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -387,7 +387,7 @@ class NotesController < ApplicationController comment = note.comments.create!(attributes) note.comments.map(&:author).uniq.each do |user| - Notifier.note_comment_notification(comment, user).deliver_now if notify && user && user != current_user && user.visible? + Notifier.note_comment_notification(comment, user).deliver_later if notify && user && user != current_user && user.visible? end end end diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index ef87a8699..808726819 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -3,7 +3,8 @@ class ReportsController < ApplicationController before_action :authorize_web before_action :set_locale - before_action :require_user + + authorize_resource def new if required_new_report_params_present? diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index efb77e2f5..4b960e4e2 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -6,10 +6,11 @@ class SiteController < ApplicationController before_action :set_locale before_action :redirect_browse_params, :only => :index before_action :redirect_map_params, :only => [:index, :edit, :export] - before_action :require_user, :only => [:welcome] before_action :require_oauth, :only => [:index] before_action :update_totp, :only => [:index] + authorize_resource :class => false + def index session[:location] ||= OSM.ip_location(request.env["REMOTE_ADDR"]) unless STATUS == :database_readonly || STATUS == :database_offline end diff --git a/app/controllers/user_preferences_controller.rb b/app/controllers/user_preferences_controller.rb index 0aa2e8d52..915c847de 100644 --- a/app/controllers/user_preferences_controller.rb +++ b/app/controllers/user_preferences_controller.rb @@ -2,8 +2,9 @@ class UserPreferencesController < ApplicationController skip_before_action :verify_authenticity_token before_action :authorize - before_action :require_allow_read_prefs, :only => [:read_one, :read] - before_action :require_allow_write_prefs, :except => [:read_one, :read] + + authorize_resource + around_action :api_call_handle_error ## diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d18cf188c..016d7c87d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -107,7 +107,7 @@ class UsersController < ApplicationController successful_login(current_user) else session[:token] = current_user.tokens.create.token - Notifier.signup_confirm(current_user, current_user.tokens.create(:referer => referer)).deliver_now + Notifier.signup_confirm(current_user, current_user.tokens.create(:referer => referer)).deliver_later redirect_to :action => "confirm", :display_name => current_user.display_name end else @@ -158,7 +158,7 @@ class UsersController < ApplicationController if user token = user.tokens.create - Notifier.lost_password(user, token).deliver_now + Notifier.lost_password(user, token).deliver_later flash[:notice] = t "users.lost_password.notice email on way" redirect_to :action => "login" else @@ -339,7 +339,7 @@ class UsersController < ApplicationController if user.nil? || token.nil? || token.user != user flash[:error] = t "users.confirm_resend.failure", :name => params[:display_name] else - Notifier.signup_confirm(user, user.tokens.create).deliver_now + Notifier.signup_confirm(user, user.tokens.create).deliver_later flash[:notice] = t("users.confirm_resend.success", :email => user.email, :sender => SUPPORT_EMAIL).html_safe end @@ -432,7 +432,7 @@ class UsersController < ApplicationController flash[:warning] = t "users.make_friend.already_a_friend", :name => @new_friend.display_name elsif friend.save flash[:notice] = t "users.make_friend.success", :name => @new_friend.display_name - Notifier.friend_notification(friend).deliver_now + Notifier.friend_notification(friend).deliver_later else friend.add_error(t("users.make_friend.failed", :name => @new_friend.display_name)) end @@ -735,7 +735,7 @@ class UsersController < ApplicationController flash.now[:notice] = t "users.account.flash update success confirm needed" begin - Notifier.email_confirm(user, user.tokens.create).deliver_now + Notifier.email_confirm(user, user.tokens.create).deliver_later rescue StandardError # Ignore errors sending email end diff --git a/app/models/notifier.rb b/app/mailers/notifier.rb similarity index 100% rename from app/models/notifier.rb rename to app/mailers/notifier.rb diff --git a/lib/geo_record.rb b/app/models/concerns/geo_record.rb similarity index 89% rename from lib/geo_record.rb rename to app/models/concerns/geo_record.rb index e02734ec9..06049c295 100644 --- a/lib/geo_record.rb +++ b/app/models/concerns/geo_record.rb @@ -1,6 +1,8 @@ require "delegate" module GeoRecord + extend ActiveSupport::Concern + # Ensure that when coordinates are printed that they are always in decimal degrees, # and not e.g. 4.0e-05 # Unfortunately you can't extend Numeric classes directly (e.g. `Coord < Float`). @@ -19,9 +21,9 @@ module GeoRecord # the database. SCALE = 10000000 - def self.included(base) - base.scope :bbox, ->(bbox) { base.where(OSM.sql_for_area(bbox)) } - base.before_save :update_tile + included do + scope :bbox, ->(bbox) { where(OSM.sql_for_area(bbox)) } + before_save :update_tile end # Is this node within -90 >= latitude >= 90 and -180 >= longitude >= 180 diff --git a/lib/not_redactable.rb b/app/models/concerns/not_redactable.rb similarity index 78% rename from lib/not_redactable.rb rename to app/models/concerns/not_redactable.rb index 6a5773296..2d721530f 100644 --- a/lib/not_redactable.rb +++ b/app/models/concerns/not_redactable.rb @@ -1,6 +1,6 @@ -require "osm" - module NotRedactable + extend ActiveSupport::Concern + def redacted? false end diff --git a/lib/object_metadata.rb b/app/models/concerns/object_metadata.rb similarity index 97% rename from lib/object_metadata.rb rename to app/models/concerns/object_metadata.rb index c765df526..dcfde889c 100644 --- a/lib/object_metadata.rb +++ b/app/models/concerns/object_metadata.rb @@ -1,4 +1,6 @@ module ObjectMetadata + extend ActiveSupport::Concern + def add_metadata_to_xml_node(el, osm, changeset_cache, user_display_name_cache) el["changeset"] = osm.changeset_id.to_s el["redacted"] = osm.redaction.id.to_s if osm.redacted? diff --git a/app/models/relation.rb b/app/models/relation.rb index 202db12da..f55711a69 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -384,7 +384,9 @@ class Relation < ActiveRecord::Base changed_members.collect { |type, _id, _role| type == "Relation" } .inject(false) { |acc, elem| acc || elem } - update_members = if tags_changed || any_relations + # if the relation is being deleted tags_changed will be true and members empty + # so we need to use changed_members to create a correct bounding box + update_members = if visible && (tags_changed || any_relations) # add all non-relation bounding boxes to the changeset # FIXME: check for tag changes along with element deletions and # make sure that the deleted element's bounding box is hit. diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index e17c6a77b..946f95feb 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -54,7 +54,7 @@