From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Fri, 25 Oct 2019 16:31:57 +0000 (+0000)
Subject: Specific action for updating role permissions.
X-Git-Tag: 4.1.0~241
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b4138257f0f541850f0f1dc77d02ab93d2e4d619;p=redmine.git

Specific action for updating role permissions.

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

diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb
index 985953377..0e29e8cfd 100644
--- a/app/controllers/roles_controller.rb
+++ b/app/controllers/roles_controller.rb
@@ -109,14 +109,16 @@ class RolesController < ApplicationController
     end
     @roles = scope.to_a
     @permissions = Redmine::AccessControl.permissions.select { |p| !p.public? }
-    if request.post?
-      @roles.each do |role|
-        role.permissions = params[:permissions][role.id.to_s]
-        role.save
-      end
-      flash[:notice] = l(:notice_successful_update)
-      redirect_to roles_path
+  end
+
+  def update_permissions
+    @roles = Role.where(:id => params[:permissions].keys)
+    @roles.each do |role|
+      role.permissions = params[:permissions][role.id.to_s]
+      role.save
     end
+    flash[:notice] = l(:notice_successful_update)
+    redirect_to roles_path
   end
 
   private
diff --git a/app/views/roles/permissions.html.erb b/app/views/roles/permissions.html.erb
index 16e78f877..34a8b8c04 100644
--- a/app/views/roles/permissions.html.erb
+++ b/app/views/roles/permissions.html.erb
@@ -21,8 +21,9 @@
 </div>
 
 <%= form_tag(permissions_roles_path, :id => 'permissions_form') do %>
-<%= hidden_field_tag 'permissions[0]', '', :id => nil %>
-<%= hidden_field_tag 'ids[]', @roles.map(&:id) %>
+<% @roles.each do |role| %>
+    <%= hidden_field_tag "permissions[#{role.id}][]", '', :id => nil %>
+<% end %>
 <div class="autoscroll">
 <table class="list permissions">
 <thead>
diff --git a/config/routes.rb b/config/routes.rb
index 37eb86ecf..d8e5fd710 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -317,7 +317,8 @@ Rails.application.routes.draw do
   end
   resources :roles do
     collection do
-      match 'permissions', :via => [:get, :post]
+      get 'permissions'
+      post 'permissions', :to => 'roles#update_permissions'
     end
   end
   resources :enumerations, :except => :show
diff --git a/test/functional/roles_controller_test.rb b/test/functional/roles_controller_test.rb
index 34be7c7b2..2bce1aa52 100644
--- a/test/functional/roles_controller_test.rb
+++ b/test/functional/roles_controller_test.rb
@@ -228,7 +228,7 @@ class RolesControllerTest < Redmine::ControllerTest
     assert_not_nil Role.find_by_id(1)
   end
 
-  def test_get_permissions
+  def test_permissions
     get :permissions
     assert_response :success
 
@@ -236,7 +236,7 @@ class RolesControllerTest < Redmine::ControllerTest
     assert_select 'input[name=?][type=checkbox][value=delete_issues]:not([checked])', 'permissions[3][]'
   end
 
-  def test_get_permissions_with_filter
+  def test_permissions_with_filter
     get :permissions, :params => {
         :ids => ['2', '3']
       }
@@ -247,10 +247,9 @@ class RolesControllerTest < Redmine::ControllerTest
     assert_select 'input[name=?][type=checkbox][value=delete_issues]:not([checked])', 'permissions[3][]'
   end
 
-  def test_post_permissions
-    post :permissions, :params => {
+  def test_update_permissions
+    post :update_permissions, :params => {
       :permissions => {
-        '0' => '',
         '1' => ['edit_issues'],
         '3' => ['add_issues', 'delete_issues']
       }
@@ -259,13 +258,18 @@ class RolesControllerTest < Redmine::ControllerTest
 
     assert_equal [:edit_issues], Role.find(1).permissions
     assert_equal [:add_issues, :delete_issues], Role.find(3).permissions
-    assert Role.find(2).permissions.empty?
   end
 
-  def test_clear_all_permissions
-    post :permissions, :params => {:permissions => { '0' => '' }}
-    assert_redirected_to '/roles'
-    assert Role.find(1).permissions.empty?
+  def test_update_permissions_should_not_update_other_roles
+    assert_no_changes -> { Role.find(2).permissions } do
+      assert_changes -> { Role.find(1).permissions } do
+        post :update_permissions, :params => {
+            :permissions => {
+              '1' => ['edit_issues']
+            }
+          }
+      end
+    end
   end
 
   def test_move_highest
diff --git a/test/integration/routing/roles_test.rb b/test/integration/routing/roles_test.rb
index 40d995992..1928c7ef5 100644
--- a/test/integration/routing/roles_test.rb
+++ b/test/integration/routing/roles_test.rb
@@ -30,6 +30,6 @@ class RoutingRolesTest < Redmine::RoutingTest
     should_route 'DELETE /roles/2' => 'roles#destroy', :id => '2'
 
     should_route 'GET /roles/permissions' => 'roles#permissions'
-    should_route 'POST /roles/permissions' => 'roles#permissions'
+    should_route 'POST /roles/permissions' => 'roles#update_permissions'
   end
 end