From: Anton Khorev Date: Tue, 24 Dec 2024 03:07:59 +0000 (+0300) Subject: Create received blocks resource X-Git-Tag: live~342^2~1 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/2df389c97e52437f964e4b4ee22c8493a9cc7fbc Create received blocks resource --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 100a199c8..041b7ef5e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -28,6 +28,7 @@ FactoryBot/ExcessiveCreateList: - 'test/controllers/traces_controller_test.rb' - 'test/controllers/user_blocks_controller_test.rb' - 'test/controllers/users/issued_blocks_controller_test.rb' + - 'test/controllers/users/received_blocks_controller_test.rb' - 'test/system/users_test.rb' # Offense count: 635 diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index d0b4997b2..fe899d09d 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -24,7 +24,7 @@ class Ability can [:create, :destroy], :session can [:read, :data, :georss], Trace can [:read, :terms, :create, :save, :suspended, :auth_success, :auth_failure], User - can [:read, :blocks_on], UserBlock + can :read, UserBlock end if user&.active? diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index ccb07ea59..81b0cbcba 100644 --- a/app/controllers/user_blocks_controller.rb +++ b/app/controllers/user_blocks_controller.rb @@ -9,7 +9,7 @@ class UserBlocksController < ApplicationController authorize_resource - before_action :lookup_user, :only => [:new, :create, :revoke_all, :blocks_on] + before_action :lookup_user, :only => [:new, :create, :revoke_all] before_action :lookup_user_block, :only => [:show, :edit, :update] before_action :require_valid_params, :only => [:create, :update] before_action :check_database_readable @@ -111,25 +111,10 @@ class UserBlocksController < ApplicationController if request.post? && params[:confirm] @user.blocks.active.each { |block| block.revoke!(current_user) } flash[:notice] = t ".flash" - redirect_to user_blocks_on_path(@user) + redirect_to user_received_blocks_path(@user) end end - ## - # shows a list of all the blocks on the given user - def blocks_on - @params = params.permit(:display_name) - - user_blocks = UserBlock.where(:user => @user) - - @user_blocks, @newer_user_blocks_id, @older_user_blocks_id = get_page_items(user_blocks, :includes => [:user, :creator, :revoker]) - - @show_user_name = false - @show_creator_name = true - - render :partial => "page" if turbo_frame_request_id == "pagination" - end - private ## diff --git a/app/controllers/users/received_blocks_controller.rb b/app/controllers/users/received_blocks_controller.rb new file mode 100644 index 000000000..a47b56a58 --- /dev/null +++ b/app/controllers/users/received_blocks_controller.rb @@ -0,0 +1,31 @@ +module Users + class ReceivedBlocksController < ApplicationController + include UserMethods + include PaginationMethods + + layout "site" + + before_action :authorize_web + before_action :set_locale + + authorize_resource :class => UserBlock + + before_action :lookup_user + before_action :check_database_readable + + ## + # shows a list of all the blocks on the given user + def show + @params = params.permit(:user_display_name) + + user_blocks = UserBlock.where(:user => @user) + + @user_blocks, @newer_user_blocks_id, @older_user_blocks_id = get_page_items(user_blocks, :includes => [:user, :creator, :revoker]) + + @show_user_name = false + @show_creator_name = true + + render :partial => "user_blocks/page" if turbo_frame_request_id == "pagination" + end + end +end diff --git a/app/views/user_blocks/_navigation.html.erb b/app/views/user_blocks/_navigation.html.erb index cd79e4f22..891315e6e 100644 --- a/app/views/user_blocks/_navigation.html.erb +++ b/app/views/user_blocks/_navigation.html.erb @@ -7,16 +7,16 @@ <% if current_user&.blocks&.exists? %>
  • <%= link_to t(".blocks_on_me"), - user_blocks_on_path(current_user), - :class => ["nav-link", { :active => action_name == "blocks_on" && current_user == @user }] %> + user_received_blocks_path(current_user), + :class => ["nav-link", { :active => controller_name == "received_blocks" && current_user == @user }] %>
    • <% end %> <% on_user = @user || @user_block&.user %> <% if on_user != current_user && on_user&.blocks&.exists? %>
  • <%= link_to t(".blocks_on_user_html", :user => tag.span(on_user.display_name, :class => "username text-truncate d-inline-block align-bottom", :dir => "auto")), - user_blocks_on_path(on_user), - :class => ["nav-link", { :active => action_name == "blocks_on" }] %> + user_received_blocks_path(on_user), + :class => ["nav-link", { :active => controller_name == "received_blocks" }] %>
    • <% end %> <% if current_user&.blocks_created&.exists? %> diff --git a/app/views/user_blocks/blocks_on.html.erb b/app/views/users/received_blocks/show.html.erb similarity index 77% rename from app/views/user_blocks/blocks_on.html.erb rename to app/views/users/received_blocks/show.html.erb index 73748e557..ada2a568f 100644 --- a/app/views/user_blocks/blocks_on.html.erb +++ b/app/views/users/received_blocks/show.html.erb @@ -3,11 +3,11 @@ <% content_for :heading_class, "pb-0" %> <% content_for :heading do %>

    <%= t(".heading_html", :name => link_to(@user.display_name, @user)) %>

    - <%= render :partial => "navigation" %> + <%= render :partial => "user_blocks/navigation" %> <% end %> <% unless @user_blocks.empty? %> -<%= render :partial => "page" %> +<%= render :partial => "user_blocks/page" %> <% else %>

    <%= t ".empty", :name => @user.display_name %>

    <% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 5e8398b16..8b09c751e 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -35,7 +35,7 @@ <% if current_user.blocks.exists? %>
  • - <%= link_to t(".blocks on me"), user_blocks_on_path(current_user) %> + <%= link_to t(".blocks on me"), user_received_blocks_path(current_user) %> <%= number_with_delimiter(current_user.blocks.active.size) %>
    • <% end %> @@ -93,7 +93,7 @@ <% if @user.blocks.exists? %>
  • - <%= link_to t(".block_history"), user_blocks_on_path(@user) %> + <%= link_to t(".block_history"), user_received_blocks_path(@user) %> <%= number_with_delimiter(@user.blocks.active.size) %>
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 387f82757..e4489ff60 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2851,6 +2851,11 @@ en: title: "Blocks by %{name}" heading_html: "List of Blocks by %{name}" empty: "%{name} has not made any blocks yet." + received_blocks: + show: + title: "Blocks on %{name}" + heading_html: "List of Blocks on %{name}" + empty: "%{name} has not been blocked yet." lists: show: title: Users @@ -2967,10 +2972,6 @@ en: read_html: "read at %{time}" time_in_future_title: "%{time_absolute}; in %{time_relative}" time_in_past_title: "%{time_absolute}; %{time_relative}" - blocks_on: - title: "Blocks on %{name}" - heading_html: "List of Blocks on %{name}" - empty: "%{name} has not been blocked yet." show: title: "%{block_on} blocked by %{block_by}" heading_html: "%{block_on} blocked by %{block_by}" diff --git a/config/routes.rb b/config/routes.rb index 7ecb200da..806e4ba85 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -273,6 +273,7 @@ OpenStreetMap::Application.routes.draw do resource :role, :controller => "user_roles", :path => "roles/:role", :only => [:create, :destroy] scope :module => :users do resource :issued_blocks, :path => "blocks_by", :only => :show + resource :received_blocks, :path => "blocks", :only => :show end end get "/user/:display_name/account", :to => redirect(:path => "/account/edit") @@ -333,7 +334,6 @@ OpenStreetMap::Application.routes.draw do resources :user_mutes, :only => [:index] # banning pages - get "/user/:display_name/blocks" => "user_blocks#blocks_on", :as => "user_blocks_on" resources :user_blocks, :path_names => { :new => "new/:display_name" } match "/user/:display_name/blocks/revoke_all" => "user_blocks#revoke_all", :via => [:get, :post], :as => "revoke_all_user_blocks" diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb index 1c48448e0..0b9c82f6a 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -37,10 +37,6 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest { :controller => "user_blocks", :action => "destroy", :id => "1" } ) - assert_routing( - { :path => "/user/username/blocks", :method => :get }, - { :controller => "user_blocks", :action => "blocks_on", :display_name => "username" } - ) assert_routing( { :path => "/user/username/blocks/revoke_all", :method => :get }, { :controller => "user_blocks", :action => "revoke_all", :display_name => "username" } @@ -609,7 +605,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest create(:user_block, :user => blocked_user) # Asking for the revoke all blocks page with a bogus user name should fail - get user_blocks_on_path("non_existent_user") + get user_received_blocks_path("non_existent_user") assert_response :not_found # Check that the revoke all blocks page requires us to login @@ -668,7 +664,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest # Check that revoking blocks works using POST post revoke_all_user_blocks_path(blocked_user, :confirm => true) - assert_redirected_to user_blocks_on_path(blocked_user) + assert_redirected_to user_received_blocks_path(blocked_user) blocks.each(&:reload) assert_not_predicate active_block1, :active? @@ -807,90 +803,6 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest end end - ## - # test the blocks_on action - def test_blocks_on - blocked_user = create(:user) - unblocked_user = create(:user) - normal_user = create(:user) - active_block = create(:user_block, :user => blocked_user) - revoked_block = create(:user_block, :revoked, :user => blocked_user) - expired_block = create(:user_block, :expired, :user => unblocked_user) - - # Asking for a list of blocks with a bogus user name should fail - get user_blocks_on_path("non_existent_user") - assert_response :not_found - assert_template "users/no_such_user" - assert_select "h1", "The user non_existent_user does not exist" - - # Check the list of blocks for a user that has never been blocked - get user_blocks_on_path(normal_user) - assert_response :success - assert_select "table#block_list", false - assert_select "p", "#{normal_user.display_name} has not been blocked yet." - - # Check the list of blocks for a user that is currently blocked - get user_blocks_on_path(blocked_user) - assert_response :success - assert_select "h1 a[href='#{user_path blocked_user}']", :text => blocked_user.display_name - assert_select "a.active[href='#{user_blocks_on_path blocked_user}']" - assert_select "table#block_list tbody", :count => 1 do - assert_select "tr", 2 - assert_select "a[href='#{user_block_path(active_block)}']", 1 - assert_select "a[href='#{user_block_path(revoked_block)}']", 1 - end - - # Check the list of blocks for a user that has previously been blocked - get user_blocks_on_path(unblocked_user) - assert_response :success - assert_select "h1 a[href='#{user_path unblocked_user}']", :text => unblocked_user.display_name - assert_select "a.active[href='#{user_blocks_on_path unblocked_user}']" - assert_select "table#block_list tbody", :count => 1 do - assert_select "tr", 1 - assert_select "a[href='#{user_block_path(expired_block)}']", 1 - end - end - - ## - # test the blocks_on action with multiple pages - def test_blocks_on_paged - user = create(:user) - user_blocks = create_list(:user_block, 50, :user => user).reverse - next_path = user_blocks_on_path(user) - - get next_path - assert_response :success - check_user_blocks_table user_blocks[0...20] - check_no_page_link "Newer Blocks" - next_path = check_page_link "Older Blocks" - - get next_path - assert_response :success - check_user_blocks_table user_blocks[20...40] - check_page_link "Newer Blocks" - next_path = check_page_link "Older Blocks" - - get next_path - assert_response :success - check_user_blocks_table user_blocks[40...50] - check_page_link "Newer Blocks" - check_no_page_link "Older Blocks" - end - - ## - # test the blocks_on action with invalid pages - def test_blocks_on_invalid_paged - user = create(:user) - - %w[-1 0 fred].each do |id| - get user_blocks_on_path(user, :before => id) - assert_redirected_to :controller => :errors, :action => :bad_request - - get user_blocks_on_path(user, :after => id) - assert_redirected_to :controller => :errors, :action => :bad_request - end - end - private def check_block_buttons(block, edit: 0) diff --git a/test/controllers/users/received_blocks_controller_test.rb b/test/controllers/users/received_blocks_controller_test.rb new file mode 100644 index 000000000..b254f9ec3 --- /dev/null +++ b/test/controllers/users/received_blocks_controller_test.rb @@ -0,0 +1,95 @@ +require "test_helper" +require_relative "../user_blocks/table_test_helper" + +module Users + class ReceivedBlocksControllerTest < ActionDispatch::IntegrationTest + include UserBlocks::TableTestHelper + + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/user/username/blocks", :method => :get }, + { :controller => "users/received_blocks", :action => "show", :user_display_name => "username" } + ) + end + + def test_show + blocked_user = create(:user) + unblocked_user = create(:user) + normal_user = create(:user) + active_block = create(:user_block, :user => blocked_user) + revoked_block = create(:user_block, :revoked, :user => blocked_user) + expired_block = create(:user_block, :expired, :user => unblocked_user) + + # Asking for a list of blocks with a bogus user name should fail + get user_received_blocks_path("non_existent_user") + assert_response :not_found + assert_template "users/no_such_user" + assert_select "h1", "The user non_existent_user does not exist" + + # Check the list of blocks for a user that has never been blocked + get user_received_blocks_path(normal_user) + assert_response :success + assert_select "table#block_list", false + assert_select "p", "#{normal_user.display_name} has not been blocked yet." + + # Check the list of blocks for a user that is currently blocked + get user_received_blocks_path(blocked_user) + assert_response :success + assert_select "h1 a[href='#{user_path blocked_user}']", :text => blocked_user.display_name + assert_select "a.active[href='#{user_received_blocks_path blocked_user}']" + assert_select "table#block_list tbody", :count => 1 do + assert_select "tr", 2 + assert_select "a[href='#{user_block_path(active_block)}']", 1 + assert_select "a[href='#{user_block_path(revoked_block)}']", 1 + end + + # Check the list of blocks for a user that has previously been blocked + get user_received_blocks_path(unblocked_user) + assert_response :success + assert_select "h1 a[href='#{user_path unblocked_user}']", :text => unblocked_user.display_name + assert_select "a.active[href='#{user_received_blocks_path unblocked_user}']" + assert_select "table#block_list tbody", :count => 1 do + assert_select "tr", 1 + assert_select "a[href='#{user_block_path(expired_block)}']", 1 + end + end + + def test_show_paged + user = create(:user) + user_blocks = create_list(:user_block, 50, :user => user).reverse + next_path = user_received_blocks_path(user) + + get next_path + assert_response :success + check_user_blocks_table user_blocks[0...20] + check_no_page_link "Newer Blocks" + next_path = check_page_link "Older Blocks" + + get next_path + assert_response :success + check_user_blocks_table user_blocks[20...40] + check_page_link "Newer Blocks" + next_path = check_page_link "Older Blocks" + + get next_path + assert_response :success + check_user_blocks_table user_blocks[40...50] + check_page_link "Newer Blocks" + check_no_page_link "Older Blocks" + end + + def test_show_invalid_paged + user = create(:user) + + %w[-1 0 fred].each do |id| + get user_received_blocks_path(user, :before => id) + assert_redirected_to :controller => "/errors", :action => :bad_request + + get user_received_blocks_path(user, :after => id) + assert_redirected_to :controller => "/errors", :action => :bad_request + end + end + end +end