From 7a4115f090c6a6f923b2fe31b505982ac006e992 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Tue, 24 Dec 2024 02:51:45 +0300 Subject: [PATCH 1/1] Create issued blocks resource --- .rubocop_todo.yml | 1 + app/abilities/ability.rb | 2 +- app/controllers/user_blocks_controller.rb | 17 +--- .../users/issued_blocks_controller.rb | 31 ++++++ app/views/user_blocks/_navigation.html.erb | 8 +- app/views/user_blocks/_page.html.erb | 3 +- .../issued_blocks/show.html.erb} | 4 +- app/views/users/show.html.erb | 4 +- config/locales/en.yml | 9 +- config/routes.rb | 4 +- .../user_blocks_controller_test.rb | 88 ----------------- .../users/issued_blocks_controller_test.rb | 95 +++++++++++++++++++ 12 files changed, 147 insertions(+), 119 deletions(-) create mode 100644 app/controllers/users/issued_blocks_controller.rb rename app/views/{user_blocks/blocks_by.html.erb => users/issued_blocks/show.html.erb} (77%) create mode 100644 test/controllers/users/issued_blocks_controller_test.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 071abc44c..100a199c8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -27,6 +27,7 @@ FactoryBot/ExcessiveCreateList: - 'test/controllers/notes_controller_test.rb' - 'test/controllers/traces_controller_test.rb' - 'test/controllers/user_blocks_controller_test.rb' + - 'test/controllers/users/issued_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 e4b9dcf6b..d0b4997b2 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, :blocks_by], UserBlock + can [:read, :blocks_on], UserBlock end if user&.active? diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb index 551371794..ccb07ea59 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, :blocks_by] + before_action :lookup_user, :only => [:new, :create, :revoke_all, :blocks_on] before_action :lookup_user_block, :only => [:show, :edit, :update] before_action :require_valid_params, :only => [:create, :update] before_action :check_database_readable @@ -130,21 +130,6 @@ class UserBlocksController < ApplicationController render :partial => "page" if turbo_frame_request_id == "pagination" end - ## - # shows a list of all the blocks by the given user. - def blocks_by - @params = params.permit(:display_name) - - user_blocks = UserBlock.where(:creator => @user) - - @user_blocks, @newer_user_blocks_id, @older_user_blocks_id = get_page_items(user_blocks, :includes => [:user, :creator, :revoker]) - - @show_user_name = true - @show_creator_name = false - - render :partial => "page" if turbo_frame_request_id == "pagination" - end - private ## diff --git a/app/controllers/users/issued_blocks_controller.rb b/app/controllers/users/issued_blocks_controller.rb new file mode 100644 index 000000000..b533b5f6d --- /dev/null +++ b/app/controllers/users/issued_blocks_controller.rb @@ -0,0 +1,31 @@ +module Users + class IssuedBlocksController < 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 by the given user. + def show + @params = params.permit(:user_display_name) + + user_blocks = UserBlock.where(:creator => @user) + + @user_blocks, @newer_user_blocks_id, @older_user_blocks_id = get_page_items(user_blocks, :includes => [:user, :creator, :revoker]) + + @show_user_name = true + @show_creator_name = false + + 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 0770e331f..cd79e4f22 100644 --- a/app/views/user_blocks/_navigation.html.erb +++ b/app/views/user_blocks/_navigation.html.erb @@ -22,16 +22,16 @@ <% if current_user&.blocks_created&.exists? %>
  • <%= link_to t(".blocks_by_me"), - user_blocks_by_path(current_user), - :class => ["nav-link", { :active => action_name == "blocks_by" && current_user == @user }] %> + user_issued_blocks_path(current_user), + :class => ["nav-link", { :active => controller_name == "issued_blocks" && current_user == @user }] %>
    • <% end %> <% by_user = @user || @user_block&.creator %> <% if by_user != current_user && by_user&.blocks_created&.exists? %>
  • <%= link_to t(".blocks_by_user_html", :user => tag.span(by_user.display_name, :class => "username text-truncate d-inline-block align-bottom", :dir => "auto")), - user_blocks_by_path(by_user), - :class => ["nav-link", { :active => action_name == "blocks_by" }] %> + user_issued_blocks_path(by_user), + :class => ["nav-link", { :active => controller_name == "issued_blocks" }] %>
    • <% end %> <% if @user_block&.persisted? %> diff --git a/app/views/user_blocks/_page.html.erb b/app/views/user_blocks/_page.html.erb index c2e516170..fa9f0b2d2 100644 --- a/app/views/user_blocks/_page.html.erb +++ b/app/views/user_blocks/_page.html.erb @@ -16,10 +16,11 @@ - <%= render :partial => "block", :collection => @user_blocks %> + <%= render :partial => "user_blocks/block", :collection => @user_blocks %> <%= render "shared/pagination", + :translation_scope => "shared.pagination.user_blocks", :newer_id => @newer_user_blocks_id, :older_id => @older_user_blocks_id %> diff --git a/app/views/user_blocks/blocks_by.html.erb b/app/views/users/issued_blocks/show.html.erb similarity index 77% rename from app/views/user_blocks/blocks_by.html.erb rename to app/views/users/issued_blocks/show.html.erb index 73748e557..ada2a568f 100644 --- a/app/views/user_blocks/blocks_by.html.erb +++ b/app/views/users/issued_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 f39224502..5e8398b16 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -42,7 +42,7 @@ <% if can?(:create, UserBlock) and current_user.blocks_created.exists? %>
  • - <%= link_to t(".blocks by me"), user_blocks_by_path(current_user) %> + <%= link_to t(".blocks by me"), user_issued_blocks_path(current_user) %> <%= number_with_delimiter(current_user.blocks_created.active.size) %>
    • <% end %> @@ -100,7 +100,7 @@ <% if @user.moderator? and @user.blocks_created.exists? %>
  • - <%= link_to t(".moderator_history"), user_blocks_by_path(@user) %> + <%= link_to t(".moderator_history"), user_issued_blocks_path(@user) %> <%= number_with_delimiter(@user.blocks_created.active.size) %>
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 7faf3bebc..387f82757 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2846,6 +2846,11 @@ en: report: Report this User go_public: flash success: "All your edits are now public, and you are now allowed to edit." + issued_blocks: + show: + title: "Blocks by %{name}" + heading_html: "List of Blocks by %{name}" + empty: "%{name} has not made any blocks yet." lists: show: title: Users @@ -2966,10 +2971,6 @@ en: title: "Blocks on %{name}" heading_html: "List of Blocks on %{name}" empty: "%{name} has not been blocked yet." - blocks_by: - title: "Blocks by %{name}" - heading_html: "List of Blocks by %{name}" - empty: "%{name} has not made any blocks 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 ccc32c773..7ecb200da 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -271,6 +271,9 @@ OpenStreetMap::Application.routes.draw do # user pages resources :users, :path => "user", :param => :display_name, :only => [:show, :destroy] do resource :role, :controller => "user_roles", :path => "roles/:role", :only => [:create, :destroy] + scope :module => :users do + resource :issued_blocks, :path => "blocks_by", :only => :show + end end get "/user/:display_name/account", :to => redirect(:path => "/account/edit") post "/user/:display_name/set_status" => "users#set_status", :as => :set_status_user @@ -331,7 +334,6 @@ OpenStreetMap::Application.routes.draw do # banning pages get "/user/:display_name/blocks" => "user_blocks#blocks_on", :as => "user_blocks_on" - get "/user/:display_name/blocks_by" => "user_blocks#blocks_by", :as => "user_blocks_by" 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 2b2238b61..1c48448e0 100644 --- a/test/controllers/user_blocks_controller_test.rb +++ b/test/controllers/user_blocks_controller_test.rb @@ -41,10 +41,6 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest { :path => "/user/username/blocks", :method => :get }, { :controller => "user_blocks", :action => "blocks_on", :display_name => "username" } ) - assert_routing( - { :path => "/user/username/blocks_by", :method => :get }, - { :controller => "user_blocks", :action => "blocks_by", :display_name => "username" } - ) assert_routing( { :path => "/user/username/blocks/revoke_all", :method => :get }, { :controller => "user_blocks", :action => "revoke_all", :display_name => "username" } @@ -895,90 +891,6 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest end end - ## - # test the blocks_by action - def test_blocks_by - moderator_user = create(:moderator_user) - second_moderator_user = create(:moderator_user) - normal_user = create(:user) - active_block = create(:user_block, :creator => moderator_user) - expired_block = create(:user_block, :expired, :creator => second_moderator_user) - revoked_block = create(:user_block, :revoked, :creator => second_moderator_user) - - # Asking for a list of blocks with a bogus user name should fail - get user_blocks_by_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 given by one moderator - get user_blocks_by_path(moderator_user) - assert_response :success - assert_select "h1 a[href='#{user_path moderator_user}']", :text => moderator_user.display_name - assert_select "a.active[href='#{user_blocks_by_path moderator_user}']" - assert_select "table#block_list tbody", :count => 1 do - assert_select "tr", 1 - assert_select "a[href='#{user_block_path(active_block)}']", 1 - end - - # Check the list of blocks given by a different moderator - get user_blocks_by_path(second_moderator_user) - assert_response :success - assert_select "h1 a[href='#{user_path second_moderator_user}']", :text => second_moderator_user.display_name - assert_select "a.active[href='#{user_blocks_by_path second_moderator_user}']" - assert_select "table#block_list tbody", :count => 1 do - assert_select "tr", 2 - assert_select "a[href='#{user_block_path(expired_block)}']", 1 - assert_select "a[href='#{user_block_path(revoked_block)}']", 1 - end - - # Check the list of blocks (not) given by a normal user - get user_blocks_by_path(normal_user) - assert_response :success - assert_select "table#block_list", false - assert_select "p", "#{normal_user.display_name} has not made any blocks yet." - end - - ## - # test the blocks_by action with multiple pages - def test_blocks_by_paged - user = create(:moderator_user) - user_blocks = create_list(:user_block, 50, :creator => user).reverse - next_path = user_blocks_by_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_by action with invalid pages - def test_blocks_by_invalid_paged - user = create(:moderator_user) - - %w[-1 0 fred].each do |id| - get user_blocks_by_path(user, :before => id) - assert_redirected_to :controller => :errors, :action => :bad_request - - get user_blocks_by_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/issued_blocks_controller_test.rb b/test/controllers/users/issued_blocks_controller_test.rb new file mode 100644 index 000000000..4fab264e9 --- /dev/null +++ b/test/controllers/users/issued_blocks_controller_test.rb @@ -0,0 +1,95 @@ +require "test_helper" +require_relative "../user_blocks/table_test_helper" + +module Users + class IssuedBlocksControllerTest < ActionDispatch::IntegrationTest + include UserBlocks::TableTestHelper + + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/user/username/blocks_by", :method => :get }, + { :controller => "users/issued_blocks", :action => "show", :user_display_name => "username" } + ) + end + + def test_show + moderator_user = create(:moderator_user) + second_moderator_user = create(:moderator_user) + normal_user = create(:user) + active_block = create(:user_block, :creator => moderator_user) + expired_block = create(:user_block, :expired, :creator => second_moderator_user) + revoked_block = create(:user_block, :revoked, :creator => second_moderator_user) + + # Asking for a list of blocks with a bogus user name should fail + get user_issued_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 given by one moderator + get user_issued_blocks_path(moderator_user) + assert_response :success + assert_select "h1 a[href='#{user_path moderator_user}']", :text => moderator_user.display_name + assert_select "a.active[href='#{user_issued_blocks_path moderator_user}']" + assert_select "table#block_list tbody", :count => 1 do + assert_select "tr", 1 + assert_select "a[href='#{user_block_path(active_block)}']", 1 + end + + # Check the list of blocks given by a different moderator + get user_issued_blocks_path(second_moderator_user) + assert_response :success + assert_select "h1 a[href='#{user_path second_moderator_user}']", :text => second_moderator_user.display_name + assert_select "a.active[href='#{user_issued_blocks_path second_moderator_user}']" + assert_select "table#block_list tbody", :count => 1 do + assert_select "tr", 2 + assert_select "a[href='#{user_block_path(expired_block)}']", 1 + assert_select "a[href='#{user_block_path(revoked_block)}']", 1 + end + + # Check the list of blocks (not) given by a normal user + get user_issued_blocks_path(normal_user) + assert_response :success + assert_select "table#block_list", false + assert_select "p", "#{normal_user.display_name} has not made any blocks yet." + end + + def test_show_paged + user = create(:moderator_user) + user_blocks = create_list(:user_block, 50, :creator => user).reverse + next_path = user_issued_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(:moderator_user) + + %w[-1 0 fred].each do |id| + get user_issued_blocks_path(user, :before => id) + assert_redirected_to :controller => "/errors", :action => :bad_request + + get user_issued_blocks_path(user, :after => id) + assert_redirected_to :controller => "/errors", :action => :bad_request + end + end + end +end -- 2.39.5