]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'upstream/pull/4926'
authorTom Hughes <tom@compton.nu>
Sat, 15 Feb 2025 17:07:59 +0000 (17:07 +0000)
committerTom Hughes <tom@compton.nu>
Sat, 15 Feb 2025 17:07:59 +0000 (17:07 +0000)
13 files changed:
app/abilities/api_ability.rb
app/assets/javascripts/index.js
app/assets/javascripts/index/changeset.js
app/assets/javascripts/index/note.js
app/assets/stylesheets/errors.scss
app/controllers/api/user_blocks_controller.rb
app/views/layouts/_head.html.erb
app/views/layouts/_meta.html.erb
app/views/layouts/error.html.erb
config/locales/en.yml
config/routes.rb
lib/oauth.rb
test/controllers/api/user_blocks_controller_test.rb

index 7bbd9889ad53fbbb6d064b115f79eca7a54cad75..ec4de8f8e03b99dc795f88cecf6421aefb11c00e 100644 (file)
@@ -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
index 9b825caad0aa77c76932cc3d0c163f6d4d66537c..810327e3fe15bd48a13dd4f5212bcb3bf932fbb7 100644 (file)
@@ -39,6 +39,14 @@ $(document).ready(function () {
 
     $("#sidebar_loader").show().addClass("delayed-fade-in");
 
+    // Prevent caching the XHR response as a full-page URL
+    // https://github.com/openstreetmap/openstreetmap-website/issues/5663
+    if (content_path.indexOf("?") >= 0) {
+      content_path += "&xhr=1";
+    } else {
+      content_path += "?xhr=1";
+    }
+
     $("#sidebar_content")
       .empty();
 
@@ -300,8 +308,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);
index 39b4abde15bd7fc836e641733bfec9af1dd8ee14..d3e61270bf7dac9945d0c3a84457d821f39d3b82 100644 (file)
@@ -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);
         });
index e9c51f9bf1df571f6814be4999a10517975fd28b..6a0487aaac21a2622386f66257860975357b833d 100644 (file)
@@ -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();
   };
index fd140023218ec93014ca6ce1534e76f7f630ce2e..77b440a889261beb4be326d47782c102669312e1 100644 (file)
@@ -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;
+    }
+  }
 }
index 51f0d26d3e641ee3762357a20a8b085eea505e32..e1fb70a659f3ddea3fee902e7b846e68c3bdbe83 100644 (file)
@@ -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
index e6d709b27f7de74192a36af8760c9c96dc88febe..37d830ef67f3cc2e4ef0174afad8fb77ddeb60ad 100644 (file)
@@ -1,6 +1,5 @@
 <%= tag.head :data => application_data do %>
-  <meta http-equiv="X-UA-Compatible" content="IE=edge" />
-  <meta name="viewport" content="width=device-width, initial-scale=1">
+  <%= 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 %>
index 4c88887f920d4462c1650759a3801c4d0ccb4ec5..48be6e0aa1ee86b31f724c896084febfa4aabb30 100644 (file)
@@ -1,3 +1,5 @@
+<meta http-equiv="X-UA-Compatible" content="IE=edge" />
+<meta name="viewport" content="width=device-width, initial-scale=1">
 <% [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 -%>
index dfcb3cb91c27a7924ded612df5a5596b71c9cb7a..eab764aa3a2a7e0a3a0ed3e7727b774687ab5b35 100644 (file)
@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<html xmlns="http://www.w3.org/1999/xhtml">
+<html lang="<%= I18n.locale %>" dir="<%= dir %>">
   <head>
     <meta charset="utf-8">
     <title>OpenStreetMap</title>
@@ -7,11 +7,13 @@
     <%= render :partial => "layouts/meta" %>
   </head>
   <body>
-    <a href="<%= root_path %>">
-      <%= image_tag "osm_logo.svg", :alt => t("layouts.logo.alt_text"), :class => "logo" %>
-    </a>
-    <div class="details">
-      <%= yield %>
-    </div>
+    <main>
+      <a href="<%= root_path %>" class="logo">
+        <%= image_tag "osm_logo.svg", :alt => t("layouts.logo.alt_text") %>
+      </a>
+      <div class="details">
+        <%= yield %>
+      </div>
+    </main>
   </body>
 </html>
index 5f9f706a0032937d9c8c34add43c3d0495ffbca1..1f9792d39608c15770e62285b2d5185de26b4eaf 100644 (file)
@@ -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
index 0041362666aae10d66f1c8b6e2e1b1d43b325a1b..b3fc5af15900002fe06d4fe8e5cef36315528d5e 100644 (file)
@@ -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
index dfa3a8028f2ac98d818081782dc489eb05ba4f1e..47edba5005244ecd5367429db818b5afac2cd881 100644 (file)
@@ -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
index 169338811d3edeaf11a2210fd0b2d1f1989d8570..2705e332d5a5c81b8d3fddafea5a78af5de71d09 100644 (file)
@@ -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