Add bulk lock feature to user list context menu (#40913).

Patch by Mizuki ISHIKAWA (@ishikawa999).

git-svn-id: https://svn.redmine.org/redmine/trunk@22898 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index e32a8ddb85c66d889539be7c7db2f7ca4211baa8..26e9151d993994dfac81b29c42bb3a1422b8f512 100644 (file)
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -231,17 +231,22 @@ class UsersController < ApplicationController
     @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)
+    if params[:confirm] == I18n.t(:general_text_Yes)
       @users.destroy_all
       flash[:notice] = l(:notice_successful_delete)
       redirect_to users_path
     end
   end
 
+  def bulk_lock
+    @users = User.logged.where(id: params[:ids]).where.not(id: User.current)
+    (render_404; return) unless @users.any?
+
+    @users.update_all status: User::STATUS_LOCKED
+    flash[:notice] = l(:notice_successful_update)
+    redirect_to users_path
+  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 3bd4f7d836f55e3500d1cb7da2647527ccc785d2..c47b55fd24e756312494b3c6725643f43482afb1 100644 (file)
--- a/app/views/context_menus/users.html.erb
+++ b/app/views/context_menus/users.html.erb
@@ -21,6 +21,11 @@
       </li>
     <% end %>
   <% else %>
+    <% unless @users.all?(&:locked?) %>
+      <li>
+        <%= context_menu_link l(:button_lock), bulk_lock_users_path(ids: @users.map(&:id)), method: :post, class: 'icon icon-lock' %>
+      </li>
+    <% end %>
     <li>
       <%= context_menu_link l(:button_delete),
         {controller: 'users', action: 'bulk_destroy', ids: @users.map(&:id)},

diff --git a/app/views/users/bulk_destroy.html.erb b/app/views/users/bulk_destroy.html.erb
index f18eb815f4df74848d2ebd6288aeb4641d134eb6..24a757b9d85efea150f5686363eae5d4b366fda7 100644 (file)
--- a/app/views/users/bulk_destroy.html.erb
+++ b/app/views/users/bulk_destroy.html.erb
@@ -14,9 +14,7 @@
 
 </div>
 
-<p>
-  <%= 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 %>
-</p>
+<%= submit_tag l(:button_delete), class: 'btn-alert btn-small' %>
 <% end %>
+<%= button_to l(:button_lock), bulk_lock_users_path(ids: @users.map(&:id)), method: :post, class: 'btn', name: 'lock' %>
+<%= link_to l(:button_cancel), users_path %>
\ No newline at end of file

diff --git a/app/views/users/destroy.html.erb b/app/views/users/destroy.html.erb
index 6478519b14dcc010456367f346985144dc3509a1..23d33becfdabdad002987a2cc884402074112f5d 100644 (file)
--- a/app/views/users/destroy.html.erb
+++ b/app/views/users/destroy.html.erb
@@ -12,9 +12,7 @@
   </p>
 </div>
 
-<p>
-  <%= submit_tag l(:button_delete) %>
-  <%= submit_tag l(:button_lock), name: 'lock' unless @user.locked? %>
-  <%= link_to l(:button_cancel), users_path %>
-</p>
+<%= submit_tag l(:button_delete) %>
 <% end %>
+<%= button_to l(:button_lock), bulk_lock_users_path(ids: [@user.id]), method: :post, class: 'btn', name: 'lock' unless @user.locked? %>
+<%= link_to l(:button_cancel), users_path %>
\ No newline at end of file

diff --git a/config/routes.rb b/config/routes.rb
index d884d62db30dc981f3aa50e4ea08f72ed2cf2ec8..f7cc3ac142b71d935d9c8d4a5ac51e3dd09d641e 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -112,6 +112,7 @@ Rails.application.routes.draw do
   resources :users do
     collection do
       delete 'bulk_destroy'
+      post :bulk_lock
     end
     resources :memberships, :controller => 'principal_memberships'
     resources :email_addresses, :only => [:index, :create, :update, :destroy]

diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index b869db0d09f2c27ad9712790a65ce6b2a2ffd3a4..b56fb9108125c14783d7726d9f57c7e1d8f3f61f 100644 (file)
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -1145,14 +1145,6 @@ class UsersControllerTest < Redmine::ControllerTest
     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]}
@@ -1185,4 +1177,38 @@ class UsersControllerTest < Redmine::ControllerTest
     end
     assert_response :not_found
   end
+
+  def test_bulk_lock
+    assert_difference 'User.status(User::STATUS_LOCKED).count', 1 do
+      delete :bulk_lock, :params => {:ids => [2]}
+    end
+    assert_redirected_to '/users'
+    assert User.find_by_id(2).locked?
+  end
+
+  def test_bulk_lock_should_not_lock_current_user
+    assert_difference 'User.status(User::STATUS_LOCKED).count', 1 do
+      delete :bulk_lock, :params => {:ids => [2, 1]}
+    end
+    assert_redirected_to '/users'
+    assert_not User.find_by_id(1).locked?
+    assert User.find_by_id(2).locked?
+  end
+
+  def test_bulk_lock_should_be_denied_for_non_admin_users
+    @request.session[:user_id] = 3
+
+    assert_no_difference 'User.status(User::STATUS_LOCKED).count' do
+      delete :bulk_lock, :params => {:ids => [2]}
+    end
+    assert_response :forbidden
+  end
+
+  def test_bulk_lock_should_be_denied_for_anonymous
+    assert User.find(6).anonymous?
+    assert_no_difference 'User.status(User::STATUS_LOCKED).count' do
+      delete :bulk_lock, :params => {:ids => [6]}
+    end
+    assert_response :not_found
+  end
 end