From 311359da2bdab01885d13c588f2911f3fed8973a Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Wed, 14 Sep 2022 03:14:59 +0200 Subject: [PATCH 2/4] user bulk destroy includes a confirmation page that also gives the opportunity to lock users instead of deleting them. --- app/controllers/users_controller.rb | 15 +++++++ app/views/context_menus/users.html.erb | 6 +++ app/views/users/bulk_destroy.html.erb | 24 ++++++++++ config/locales/en.yml | 2 + config/routes.rb | 3 ++ test/functional/users_controller_test.rb | 57 ++++++++++++++++++++++++ 6 files changed, 107 insertions(+) create mode 100644 app/views/users/bulk_destroy.html.erb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 27d6eb0d3f..d5777cd47f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -218,6 +218,21 @@ class UsersController < ApplicationController end end + def bulk_destroy + @users = User.logged.where(id: params[:ids]).where.not(id: User.current) + (render_404; return) unless @users.any? + + if params[:lock] + @users.update_all status: User::STATUS_LOCKED + flash[:notice] = l(:notice_successful_update) + redirect_to users_path + elsif params[:confirm] == I18n.t(:general_text_Yes) + @users.destroy_all + flash[:notice] = l(:notice_successful_delete) + redirect_to users_path + end + end + private def find_user(logged = true) diff --git a/app/views/context_menus/users.html.erb b/app/views/context_menus/users.html.erb index 2a0c12be1f..3bd4f7d836 100644 --- a/app/views/context_menus/users.html.erb +++ b/app/views/context_menus/users.html.erb @@ -20,5 +20,11 @@ method: :delete, class: 'icon icon-del' %> <% end %> + <% else %> +
  • + <%= context_menu_link l(:button_delete), + {controller: 'users', action: 'bulk_destroy', ids: @users.map(&:id)}, + method: :delete, class: 'icon icon-del' %> +
    • <% end %> diff --git a/app/views/users/bulk_destroy.html.erb b/app/views/users/bulk_destroy.html.erb new file mode 100644 index 0000000000..403314f6f9 --- /dev/null +++ b/app/views/users/bulk_destroy.html.erb @@ -0,0 +1,24 @@ +<%= title l(:label_confirmation) %> + +<%= form_tag(bulk_destroy_users_path(ids: @users.map(&:id)), method: :delete) do %> +
    + +

    <%= simple_format l :text_users_bulk_destroy_head %>

    + +<% @users.each do |user| %> +

    <%= user.name %> (<%= user.login %>)

    +<% end %> + +

    <%= l :text_users_bulk_destroy_confirm, yes: l(:general_text_Yes) %>

    +

    <%= text_field_tag 'confirm' %>

    + +
    + +

    + <%= submit_tag l(:button_delete), class: 'btn-alert btn-small' %> + <%= submit_tag l(:button_lock), class: 'btn', name: 'lock' %> + <%= link_to l(:button_cancel), users_path %> +

    +<% end %> + + diff --git a/config/locales/en.yml b/config/locales/en.yml index 8f80d309e0..335614dfbe 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1230,6 +1230,8 @@ en: text_project_close_confirmation: Are you sure you want to close the '%{value}' project to make it read-only? text_project_reopen_confirmation: Are you sure you want to reopen the '%{value}' project? text_project_archive_confirmation: Are you sure you want to archive the '%{value}' project? + text_users_bulk_destroy_head: 'You are about to delete the following users and remove all references to them. This cannot be undone. Often, locking users instead of deleting them is the better solution.' + text_users_bulk_destroy_confirm: 'To confirm, please enter "%{yes}" below.' text_workflow_edit: Select a role and a tracker to edit the workflow text_are_you_sure: Are you sure? text_journal_changed: "%{label} changed from %{old} to %{new}" diff --git a/config/routes.rb b/config/routes.rb index f2106d90f7..fa65b4417d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,6 +110,9 @@ Rails.application.routes.draw do match '/users/context_menu', to: 'context_menus#users', as: :users_context_menu, via: [:get, :post] resources :users do + collection do + delete 'bulk_destroy' + end resources :memberships, :controller => 'principal_memberships' resources :email_addresses, :only => [:index, :create, :update, :destroy] end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 5044b32120..d888f23115 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -1051,4 +1051,61 @@ class UsersControllerTest < Redmine::ControllerTest assert_response 422 end end + + def test_bulk_destroy + assert_difference 'User.count', -1 do + delete :bulk_destroy, :params => {:ids => [2], :confirm => 'Yes'} + end + assert_redirected_to '/users' + assert_nil User.find_by_id(2) + end + + def test_bulk_destroy_should_not_destroy_current_user + assert_difference 'User.count', -1 do + delete :bulk_destroy, :params => {:ids => [2, 1], :confirm => 'Yes'} + end + assert_redirected_to '/users' + assert_nil User.find_by_id(2) + end + + def test_bulk_destroy_with_lock_param_should_lock_instead + assert_no_difference 'User.count' do + delete :bulk_destroy, :params => {:ids => [2], :lock => 'lock'} + end + assert_redirected_to '/users' + assert User.find_by_id(2).locked? + end + + def test_bulk_destroy_should_require_confirmation + assert_no_difference 'User.count' do + delete :bulk_destroy, :params => {:ids => [2]} + end + assert_response :success + assert_select '.warning', :text => /You are about to delete the following users/ + end + + def test_bulk_destroy_should_require_correct_confirmation + assert_no_difference 'User.count' do + delete :bulk_destroy, :params => {:ids => [2], :confirm => 'wrong'} + end + assert_response :success + assert_select '.warning', :text => /You are about to delete the following users/ + end + + def test_bulk_destroy_should_be_denied_for_non_admin_users + @request.session[:user_id] = 3 + + assert_no_difference 'User.count' do + delete :bulk_destroy, :params => {:ids => [2], :confirm => 'Yes'} + end + assert_response 403 + end + + def test_bulk_destroy_should_be_denied_for_anonymous + assert User.find(6).anonymous? + assert_no_difference 'User.count' do + delete :bulk_destroy, :params => {:ids => [6], :confirm => "Yes"} + end + assert_response 404 + end end -- 2.30.2