From: Anton Khorev Date: Sat, 15 Feb 2025 17:03:42 +0000 (+0300) Subject: Merge branch 'pull/5678' X-Git-Tag: live~133 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/3bdbc35bbd8f44bab813adb5c9afe505868e4a09?hp=b1397d9fc34ef0e7da061d94963d7610d1b9cf34 Merge branch 'pull/5678' --- diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 7bbd9889a..ec4de8f8e 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -43,7 +43,9 @@ class ApiAbility can :destroy, Note if scopes.include?("write_notes") - can :redact, [OldNode, OldWay, OldRelation] if user&.terms_agreed? && scopes.include?("write_redactions") + can :redact, [OldNode, OldWay, OldRelation] if user.terms_agreed? && scopes.include?("write_redactions") + + can :create, UserBlock if scopes.include?("write_blocks") end end end diff --git a/app/assets/javascripts/index.js b/app/assets/javascripts/index.js index 2cf45e9ad..a5916d3ed 100644 --- a/app/assets/javascripts/index.js +++ b/app/assets/javascripts/index.js @@ -317,8 +317,9 @@ $(document).ready(function () { }; function addObject(type, id, center) { + var hashParams = OSM.parseHash(window.location.hash); map.addObject({ type: type, id: parseInt(id, 10) }, function (bounds) { - if (!window.location.hash && bounds.isValid() && + if (!hashParams.center && bounds.isValid() && (center || !map.getBounds().contains(bounds))) { OSM.router.withoutMoveListener(function () { map.fitBounds(bounds); diff --git a/app/assets/javascripts/index/changeset.js b/app/assets/javascripts/index/changeset.js index 39b4abde1..d3e61270b 100644 --- a/app/assets/javascripts/index/changeset.js +++ b/app/assets/javascripts/index/changeset.js @@ -12,9 +12,10 @@ OSM.Changeset = function (map) { const changesetData = content.find("[data-changeset]").data("changeset"); changesetData.type = "changeset"; + var hashParams = OSM.parseHash(window.location.hash); initialize(); map.addObject(changesetData, function (bounds) { - if (!window.location.hash && bounds.isValid()) { + if (!hashParams.center && bounds.isValid()) { OSM.router.withoutMoveListener(function () { map.fitBounds(bounds); }); diff --git a/app/assets/javascripts/index/note.js b/app/assets/javascripts/index/note.js index e9c51f9bf..6a0487aaa 100644 --- a/app/assets/javascripts/index/note.js +++ b/app/assets/javascripts/index/note.js @@ -27,13 +27,16 @@ OSM.Note = function (map) { var data = $(".details").data(); if (!data) return; var latLng = L.latLng(data.coordinates.split(",")); - if (!map.getBounds().contains(latLng)) moveToNote(); + if (!map.getBounds().contains(latLng)) { + OSM.router.withoutMoveListener(function () { + map.setView(latLng, 15, { reset: true }); + }); + } }); }; page.load = function (path, id) { initialize(path, id); - moveToNote(); }; function initialize(path, id) { @@ -48,7 +51,6 @@ OSM.Note = function (map) { success: () => { OSM.loadSidebarContent(path, () => { initialize(path, id); - moveToNote(); }); }, error: (xhr) => { @@ -77,11 +79,19 @@ OSM.Note = function (map) { var data = $(".details").data(); if (data) { + var hashParams = OSM.parseHash(window.location.hash); map.addObject({ type: "note", id: parseInt(id, 10), latLng: L.latLng(data.coordinates.split(",")), icon: noteIcons[data.status] + }, function () { + if (!hashParams.center) { + var latLng = L.latLng(data.coordinates.split(",")); + OSM.router.withoutMoveListener(function () { + map.setView(latLng, 15, { reset: true }); + }); + } }); } } @@ -99,18 +109,6 @@ OSM.Note = function (map) { } } - function moveToNote() { - var data = $(".details").data(); - if (!data) return; - var latLng = L.latLng(data.coordinates.split(",")); - - if (!window.location.hash || window.location.hash.match(/^#?c[0-9]+$/)) { - OSM.router.withoutMoveListener(function () { - map.setView(latLng, 15, { reset: true }); - }); - } - } - page.unload = function () { map.removeObject(); }; diff --git a/app/assets/stylesheets/errors.scss b/app/assets/stylesheets/errors.scss index fd1400232..77b440a88 100644 --- a/app/assets/stylesheets/errors.scss +++ b/app/assets/stylesheets/errors.scss @@ -1,8 +1,43 @@ -.logo { - float: left; - margin: 10px; +body { + margin: 1rem; + margin-top: 2rem; + font-family: system-ui; } -.details { - float: left; +main { + display: flex; + flex-direction: column; + align-items: center; + gap: 1rem 2rem; + max-width: 960px; + + .logo { + flex-shrink: 0; + + img { + display: block; + max-width: 100%; + height: auto; + } + } + + .details { + h1 { + margin-top: 0; + } + } +} + +@media (min-width: 640px) { + body { + margin: 2rem; + } + + main { + flex-direction: row; + + .logo { + align-self: start; + } + } } diff --git a/app/controllers/api/user_blocks_controller.rb b/app/controllers/api/user_blocks_controller.rb index 51f0d26d3..e1fb70a65 100644 --- a/app/controllers/api/user_blocks_controller.rb +++ b/app/controllers/api/user_blocks_controller.rb @@ -1,5 +1,8 @@ module Api class UserBlocksController < ApiController + before_action :check_api_writable, :only => :create + before_action :authorize, :only => :create + authorize_resource before_action :set_request_formats @@ -11,5 +14,33 @@ module Api rescue ActiveRecord::RecordNotFound raise OSM::APINotFoundError end + + def create + raise OSM::APIBadUserInput, "No user was given" unless params[:user] + + user = User.visible.find_by(:id => params[:user]) + raise OSM::APINotFoundError unless user + raise OSM::APIBadUserInput, "No reason was given" unless params[:reason] + raise OSM::APIBadUserInput, "No period was given" unless params[:period] + + period = Integer(params[:period], :exception => false) + raise OSM::APIBadUserInput, "Period should be a number of hours" unless period + + max_period = UserBlock::PERIODS.max + raise OSM::APIBadUserInput, "Period must be between 0 and #{max_period}" if period.negative? || period > max_period + raise OSM::APIBadUserInput, "Needs_view must be true if provided" unless params[:needs_view].nil? || params[:needs_view] == "true" + + ends_at = Time.now.utc + period.hours + needs_view = params[:needs_view] == "true" + @user_block = UserBlock.create( + :user => user, + :creator => current_user, + :reason => params[:reason], + :ends_at => ends_at, + :deactivates_at => (ends_at unless needs_view), + :needs_view => needs_view + ) + render :show + end end end diff --git a/app/views/layouts/_head.html.erb b/app/views/layouts/_head.html.erb index e6d709b27..37d830ef6 100644 --- a/app/views/layouts/_head.html.erb +++ b/app/views/layouts/_head.html.erb @@ -1,6 +1,5 @@ <%= tag.head :data => application_data do %> - - + <%= render :partial => "layouts/meta" %> <%= javascript_include_tag "turbo", :type => "module" %> <%= javascript_include_tag "application" %> <%= javascript_include_tag "i18n/#{I18n.locale}" %> @@ -11,7 +10,6 @@ <% end %> <%= stylesheet_link_tag "print-#{dir}", :media => "print" %> <%= stylesheet_link_tag "leaflet-all", :media => "screen, print" %> - <%= render :partial => "layouts/meta" %> <%= yield :head %> <%= yield :auto_discovery_link_tag %> <%= csrf_meta_tag %> diff --git a/app/views/layouts/_meta.html.erb b/app/views/layouts/_meta.html.erb index 4c88887f9..48be6e0aa 100644 --- a/app/views/layouts/_meta.html.erb +++ b/app/views/layouts/_meta.html.erb @@ -1,3 +1,5 @@ + + <% [57, 60, 72, 76, 114, 120, 144, 152, 180].each do |size| -%> <%= favicon_link_tag "apple-touch-icon-#{size}x#{size}.png", :rel => "apple-touch-icon", :sizes => "#{size}x#{size}", :type => "image/png" %> <% end -%> diff --git a/app/views/layouts/error.html.erb b/app/views/layouts/error.html.erb index dfcb3cb91..eab764aa3 100644 --- a/app/views/layouts/error.html.erb +++ b/app/views/layouts/error.html.erb @@ -1,5 +1,5 @@ - + OpenStreetMap @@ -7,11 +7,13 @@ <%= render :partial => "layouts/meta" %> - - <%= image_tag "osm_logo.svg", :alt => t("layouts.logo.alt_text"), :class => "logo" %> - -
- <%= yield %> -
+
+ +
+ <%= yield %> +
+
diff --git a/config/locales/en.yml b/config/locales/en.yml index eaf0fdf15..b52bd137f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2713,6 +2713,7 @@ en: write_gpx: Upload GPS traces write_notes: Modify notes write_redactions: Redact map data + write_blocks: Create and revoke user blocks read_email: Read user email address consume_messages: Read, update status and delete user messages send_messages: Send private messages to other users diff --git a/config/routes.rb b/config/routes.rb index 3971494aa..7e9cd8dfd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -121,7 +121,7 @@ OpenStreetMap::Application.routes.draw do resource :subscription, :only => [:create, :destroy], :controller => "note_subscriptions" end - resources :user_blocks, :only => :show, :id => /\d+/, :controller => "user_blocks" + resources :user_blocks, :only => [:show, :create], :id => /\d+/, :controller => "user_blocks" namespace :user_blocks, :path => "user/blocks" do resource :active_list, :path => "active", :only => :show end diff --git a/config/settings/test.yml b/config/settings/test.yml index b0e2f4613..b7cffcc27 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -53,3 +53,5 @@ doorkeeper_signing_key: | cK1+/2V+OkM/0nXjxPwPj7LiOediUyZNUn48r29uGOL1S83PSUdyST207CP6mZjc K8aJmnGsVEAcWPzbpNh14q/c -----END PRIVATE KEY----- +# Override Firefox binary used in system tests +#system_test_firefox_binary: diff --git a/lib/gpx.rb b/lib/gpx.rb index 45a4dcf5f..921dce12c 100644 --- a/lib/gpx.rb +++ b/lib/gpx.rb @@ -31,6 +31,8 @@ module GPX point.altitude ||= 0 yield point @actual_points += 1 + @lats << point.latitude + @lons << point.longitude elsif reader.name == "trkseg" @tracksegs += 1 end @@ -44,6 +46,8 @@ module GPX @possible_points = 0 @actual_points = 0 @tracksegs = 0 + @lats = [] + @lons = [] begin Archive::Reader.open_filename(@file).each_entry_with_data do |entry, data| @@ -94,9 +98,9 @@ module GPX first = true - points.each_with_index do |p, pt| - px = proj.x(p.longitude) - py = proj.y(p.latitude) + @actual_points.times do |pt| + px = proj.x @lons[pt] + py = proj.y @lats[pt] if (pt >= (points_per_frame * n)) && (pt <= (points_per_frame * (n + 1))) pen.thickness = 3 @@ -151,9 +155,9 @@ module GPX first = true - points do |p| - px = proj.x(p.longitude) - py = proj.y(p.latitude) + @actual_points.times do |pt| + px = proj.x @lons[pt] + py = proj.y @lats[pt] pen.line(px, py, oldpx, oldpy) unless first diff --git a/lib/oauth.rb b/lib/oauth.rb index dfa3a8028..47edba500 100644 --- a/lib/oauth.rb +++ b/lib/oauth.rb @@ -1,11 +1,11 @@ module Oauth SCOPES = %w[ read_prefs write_prefs write_diary - write_api write_changeset_comments read_gpx write_gpx write_notes write_redactions + write_api write_changeset_comments read_gpx write_gpx write_notes write_redactions write_blocks consume_messages send_messages openid ].freeze PRIVILEGED_SCOPES = %w[read_email skip_authorization].freeze - MODERATOR_SCOPES = %w[write_redactions].freeze + MODERATOR_SCOPES = %w[write_redactions write_blocks].freeze class Scope attr_reader :name diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 0ddb8a87a..496f37c16 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -9,6 +9,7 @@ end class ApplicationSystemTestCase < ActionDispatch::SystemTestCase driven_by :selenium, :using => :headless_firefox do |options| options.add_preference("intl.accept_languages", "en") + options.binary = Settings.system_test_firefox_binary if Settings.system_test_firefox_binary end def before_setup diff --git a/test/controllers/api/user_blocks_controller_test.rb b/test/controllers/api/user_blocks_controller_test.rb index 169338811..2705e332d 100644 --- a/test/controllers/api/user_blocks_controller_test.rb +++ b/test/controllers/api/user_blocks_controller_test.rb @@ -3,6 +3,10 @@ require "test_helper" module Api class UserBlocksControllerTest < ActionDispatch::IntegrationTest def test_routes + assert_routing( + { :path => "/api/0.6/user_blocks", :method => :post }, + { :controller => "api/user_blocks", :action => "create" } + ) assert_routing( { :path => "/api/0.6/user_blocks/1", :method => :get }, { :controller => "api/user_blocks", :action => "show", :id => "1" } @@ -14,11 +18,22 @@ module Api end def test_show - block = create(:user_block) + blocked_user = create(:user) + creator_user = create(:moderator_user) + block = create(:user_block, :user => blocked_user, :creator => creator_user, :reason => "because running tests") get api_user_block_path(block) assert_response :success - assert_select "user_block[id='#{block.id}']", 1 + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because running tests" + end get api_user_block_path(block, :format => "json") assert_response :success @@ -32,5 +47,165 @@ module Api assert_response :not_found assert_equal "text/plain", @response.media_type end + + def test_create_no_permission + blocked_user = create(:user) + assert_empty blocked_user.blocks + + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1) + assert_response :unauthorized + assert_empty blocked_user.blocks + + regular_creator_user = create(:user) + auth_header = bearer_authorization_header(regular_creator_user, :scopes => %w[read_prefs]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + + auth_header = bearer_authorization_header(regular_creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + + moderator_creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(moderator_creator_user, :scopes => %w[read_prefs]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :forbidden + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_no_user + blocked_user = create(:user, :deleted) + assert_empty blocked_user.blocks + + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:reason => "because", :period => 1), :headers => auth_header + assert_response :bad_request + assert_equal "text/plain", @response.media_type + assert_equal "No user was given", @response.body + + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_user_is_unknown + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => 0, :reason => "because", :period => 1), :headers => auth_header + assert_response :not_found + assert_equal "text/plain", @response.media_type + end + + def test_create_invalid_because_user_is_deleted + blocked_user = create(:user, :deleted) + assert_empty blocked_user.blocks + + creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :not_found + assert_equal "text/plain", @response.media_type + + assert_empty blocked_user.blocks + end + + def test_create_invalid_because_missing_reason + create_with_params_and_assert_bad_request("No reason was given", :period => "10") + end + + def test_create_invalid_because_missing_period + create_with_params_and_assert_bad_request("No period was given", :reason => "because") + end + + def test_create_invalid_because_non_numeric_period + create_with_params_and_assert_bad_request("Period should be a number of hours", :reason => "because", :period => "one hour") + end + + def test_create_invalid_because_negative_period + create_with_params_and_assert_bad_request("Period must be between 0 and #{UserBlock::PERIODS.max}", :reason => "go away", :period => "-1") + end + + def test_create_invalid_because_excessive_period + create_with_params_and_assert_bad_request("Period must be between 0 and #{UserBlock::PERIODS.max}", :reason => "go away", :period => "10000000") + end + + def test_create_invalid_because_unknown_needs_view + create_with_params_and_assert_bad_request("Needs_view must be true if provided", :reason => "because", :period => "1", :needs_view => "maybe") + end + + def test_create_success + blocked_user = create(:user) + creator_user = create(:moderator_user) + + assert_empty blocked_user.blocks + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => 1), :headers => auth_header + assert_response :success + assert_equal 1, blocked_user.blocks.length + + block = blocked_user.blocks.take + assert_predicate block, :active? + assert_equal "because", block.reason + assert_equal creator_user, block.creator + + assert_equal "application/xml", @response.media_type + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">@needs_view", "false" + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because" + end + end + + def test_create_success_with_needs_view + blocked_user = create(:user) + creator_user = create(:moderator_user) + + assert_empty blocked_user.blocks + auth_header = bearer_authorization_header(creator_user, :scopes => %w[read_prefs write_blocks]) + post api_user_blocks_path(:user => blocked_user.id, :reason => "because", :period => "1", :needs_view => "true"), :headers => auth_header + assert_response :success + assert_equal 1, blocked_user.blocks.length + + block = blocked_user.blocks.take + assert_predicate block, :active? + assert_equal "because", block.reason + assert_equal creator_user, block.creator + + assert_equal "application/xml", @response.media_type + assert_select "osm>user_block", 1 do + assert_select ">@id", block.id.to_s + assert_select ">@needs_view", "true" + assert_select ">user", 1 + assert_select ">user>@uid", blocked_user.id.to_s + assert_select ">creator", 1 + assert_select ">creator>@uid", creator_user.id.to_s + assert_select ">revoker", 0 + assert_select ">reason", 1 + assert_select ">reason", "because" + end + end + + private + + def create_with_params_and_assert_bad_request(message, **params) + blocked_user = create(:user) + assert_empty blocked_user.blocks + + moderator_creator_user = create(:moderator_user) + auth_header = bearer_authorization_header(moderator_creator_user, :scopes => %w[read_prefs write_blocks]) + + post api_user_blocks_path({ :user => blocked_user.id }.merge(params)), :headers => auth_header + assert_response :bad_request + assert_equal "text/plain", @response.media_type + assert_equal message, @response.body + + assert_empty blocked_user.blocks + end end end diff --git a/test/teaspoon_env.rb b/test/teaspoon_env.rb index 8a9dc001f..c14045b1c 100644 --- a/test/teaspoon_env.rb +++ b/test/teaspoon_env.rb @@ -100,10 +100,12 @@ Teaspoon.configure do |config| # Capybara Webkit: https://github.com/jejacks0n/teaspoon/wiki/Using-Capybara-Webkit require "selenium-webdriver" config.driver = :selenium + firefox_options = Selenium::WebDriver::Firefox::Options.new(:args => ["-headless"]) + firefox_options.binary = Settings.system_test_firefox_binary if Settings.system_test_firefox_binary config.driver_options = { :client_driver => :firefox, :selenium_options => { - :options => Selenium::WebDriver::Firefox::Options.new(:args => ["-headless"]) + :options => firefox_options } }