##// END OF EJS Templates
jro_apps » redmine
RSS
Ability to limit member management to certain roles (#19707)....
Jean-Philippe Lang -
r13911:ed9f00178c65
parent child
Show More
Side by Side Unified
Context file:
r13911:ed9f00178c65 Loading diff...:
Show file before | Show file after | Hide comments
@@ -0,0 +1,5
1 class AddRolesAllRolesManaged < ActiveRecord::Migration
2 def change
3 add_column :roles, :all_roles_managed, :boolean, :default => true, :null => false
4 end
5 end
Show file before | Show file after | Hide comments
@@ -0,0 +1,8
1 class CreateRolesManagedRoles < ActiveRecord::Migration
2 def change
3 create_table :roles_managed_roles, :id => false do |t|
4 t.integer :role_id, :null => false
5 t.integer :managed_role_id, :null => false
6 end
7 end
8 end
Show file before | Show file after | Hide comments
@@ -0,0 +1,5
1 class AddUniqueIndexOnRolesManagedRoles < ActiveRecord::Migration
2 def change
3 add_index :roles_managed_roles, [:role_id, :managed_role_id], :unique => true
4 end
5 end
Show file before | Show file after | Hide comments
@@ -53,14 +53,12 class MembersController < ApplicationController
53 53 def create
54 54 members = []
55 55 if params[:membership]
56 if params[:membership][:user_ids]
57 attrs = params[:membership].dup
58 user_ids = attrs.delete(:user_ids)
59 user_ids.each do |user_id|
60 members << Member.new(:role_ids => params[:membership][:role_ids], :user_id => user_id)
61 end
62 else
63 members << Member.new(:role_ids => params[:membership][:role_ids], :user_id => params[:membership][:user_id])
56 user_ids = Array.wrap(params[:membership][:user_id] || params[:membership][:user_ids])
57 user_ids << nil if user_ids.empty?
58 user_ids.each do |user_id|
59 member = Member.new(:project => @project, :user_id => user_id)
60 member.set_editable_role_ids(params[:membership][:role_ids])
61 members << member
64 62 end
65 63 @project.members << members
66 64 end
@@ -84,7 +82,7 class MembersController < ApplicationController
84 82
85 83 def update
86 84 if params[:membership]
87 @member.role_ids = params[:membership][:role_ids]
85 @member.set_editable_role_ids(params[:membership][:role_ids])
88 86 end
89 87 saved = @member.save
90 88 respond_to do |format|
@@ -101,7 +99,7 class MembersController < ApplicationController
101 99 end
102 100
103 101 def destroy
104 if request.delete? && @member.deletable?
102 if @member.deletable?
105 103 @member.destroy
106 104 end
107 105 respond_to do |format|
Show file before | Show file after | Hide comments
@@ -29,6 +29,12 class Member < ActiveRecord::Base
29 29
30 30 before_destroy :set_issue_category_nil
31 31
32 alias :base_reload :reload
33 def reload(*args)
34 @managed_roles = nil
35 base_reload(*args)
36 end
37
32 38 def role
33 39 end
34 40
@@ -70,22 +76,52 class Member < ActiveRecord::Base
70 76 end
71 77 end
72 78
73 def deletable?
74 member_roles.detect {|mr| mr.inherited_from}.nil?
79 # Set member role ids ignoring any change to roles that
80 # user is not allowed to manage
81 def set_editable_role_ids(ids, user=User.current)
82 ids = (ids || []).collect(&:to_i) - [0]
83 editable_role_ids = user.managed_roles(project).map(&:id)
84 untouched_role_ids = self.role_ids - editable_role_ids
85 touched_role_ids = ids & editable_role_ids
86 self.role_ids = untouched_role_ids + touched_role_ids
75 87 end
76 88
77 def destroy
78 if member_roles.reload.present?
79 # destroying the last role will destroy another instance
80 # of the same Member record, #super would then trigger callbacks twice
81 member_roles.destroy_all
82 @destroyed = true
83 freeze
89 # Returns true if one of the member roles is inherited
90 def any_inherited_role?
91 member_roles.any? {|mr| mr.inherited_from}
92 end
93
94 # Returns true if the member has the role and if it's inherited
95 def has_inherited_role?(role)
96 member_roles.any? {|mr| mr.role_id == role.id && mr.inherited_from.present?}
97 end
98
99 # Returns true if the member's role is editable by user
100 def role_editable?(role, user=User.current)
101 if has_inherited_role?(role)
102 false
84 103 else
85 super
104 user.managed_roles(project).include?(role)
86 105 end
87 106 end
88 107
108 # Returns true if the member is deletable by user
109 def deletable?(user=User.current)
110 if any_inherited_role?
111 false
112 else
113 roles & user.managed_roles(project) == roles
114 end
115 end
116
117 # Destroys the member
118 def destroy
119 member_roles.reload.each(&:destroy_without_member_removal)
120 super
121 end
122
123 # Returns true if the member is user or is a group
124 # that includes user
89 125 def include?(user)
90 126 if principal.is_a?(Group)
91 127 !user.nil? && user.groups.include?(principal)
@@ -102,6 +138,27 class Member < ActiveRecord::Base
102 138 end
103 139 end
104 140
141 # Returns the roles that the member is allowed to manage
142 # in the project the member belongs to
143 def managed_roles
144 @managed_roles ||= begin
145 if principal.try(:admin?)
146 Role.givable.to_a
147 else
148 members_management_roles = roles.select do |role|
149 role.has_permission?(:manage_members)
150 end
151 if members_management_roles.empty?
152 []
153 elsif members_management_roles.any?(&:all_roles_managed?)
154 Role.givable.to_a
155 else
156 members_management_roles.map(&:managed_roles).reduce(&:|)
157 end
158 end
159 end
160 end
161
105 162 # Creates memberships for principal with the attributes
106 163 # * project_ids : one or more project ids
107 164 # * role_ids : ids of the roles to give to each membership
Show file before | Show file after | Hide comments
@@ -36,10 +36,17 class MemberRole < ActiveRecord::Base
36 36 !inherited_from.nil?
37 37 end
38 38
39 # Destroys the MemberRole without destroying its Member if it doesn't have
40 # any other roles
41 def destroy_without_member_removal
42 @member_removal = false
43 destroy
44 end
45
39 46 private
40 47
41 48 def remove_member_if_empty
42 if member.roles.empty?
49 if @member_removal != false && member.roles.empty?
43 50 member.destroy
44 51 end
45 52 end
Show file before | Show file after | Hide comments
@@ -64,6 +64,10 class Role < ActiveRecord::Base
64 64 end
65 65 has_and_belongs_to_many :custom_fields, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "role_id"
66 66
67 has_and_belongs_to_many :managed_roles, :class_name => 'Role',
68 :join_table => "#{table_name_prefix}roles_managed_roles#{table_name_suffix}",
69 :association_foreign_key => "managed_role_id"
70
67 71 has_many :member_roles, :dependent => :destroy
68 72 has_many :members, :through => :member_roles
69 73 acts_as_list
Show file before | Show file after | Hide comments
@@ -566,6 +566,15 class User < Principal
566 566 @visible_project_ids ||= Project.visible(self).pluck(:id)
567 567 end
568 568
569 # Returns the roles that the user is allowed to manage for the given project
570 def managed_roles(project)
571 if admin?
572 Role.givable.to_a
573 else
574 membership(project).try(:managed_roles) || []
575 end
576 end
577
569 578 # Returns true if user is arg or belongs to arg
570 579 def is_or_belongs_to?(arg)
571 580 if arg.is_a?(User)
Show file before | Show file after | Hide comments
@@ -9,7 +9,7
9 9 <fieldset class="box">
10 10 <legend><%= l(:label_role_plural) %> <%= toggle_checkboxes_link('.roles-selection input') %></legend>
11 11 <div class="roles-selection">
12 <% Role.givable.all.each do |role| %>
12 <% User.current.managed_roles(@project).each do |role| %>
13 13 <label><%= check_box_tag 'membership[role_ids][]', role.id, false, :id => nil %> <%= role %></label>
14 14 <% end %>
15 15 </div>
Show file before | Show file after | Hide comments
@@ -32,9 +32,7
32 32 <%= check_box_tag('membership[role_ids][]',
33 33 role.id, member.roles.include?(role),
34 34 :id => nil,
35 :disabled => member.member_roles.detect {
36 |mr| mr.role_id == role.id && !mr.inherited_from.nil?
37 } ) %> <%= role %>
35 :disabled => !member.role_editable?(role)) %> <%= role %>
38 36 </label><br />
39 37 <% end %>
40 38 </p>
Show file before | Show file after | Hide comments
@@ -16,6 +16,28
16 16
17 17 <p><%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p>
18 18
19 <% unless @role.builtin? %>
20 <p id="manage_members_options">
21 <label>Gestion des membres</label>
22 <label class="block">
23 <%= radio_button_tag 'role[all_roles_managed]', 1, @role.all_roles_managed?, :id => 'role_all_roles_managed_on',
24 :data => {:disables => '.role_managed_role input'} %>
25 tous les rôles
26 </label>
27 <label class="block">
28 <%= radio_button_tag 'role[all_roles_managed]', 0, !@role.all_roles_managed?, :id => 'role_all_roles_managed_off',
29 :data => {:enables => '.role_managed_role input'} %>
30 ces rôles uniquement:
31 </label>
32 <% Role.givable.sorted.each do |role| %>
33 <label class="block role_managed_role" style="padding-left:2em;">
34 <%= check_box_tag 'role[managed_role_ids][]', role.id, @role.managed_roles.include?(role), :id => nil %>
35 <%= role.name %>
36 </label>
37 <% end %>
38 <%= hidden_field_tag 'role[managed_role_ids][]', '' %>
39 <% end %>
40
19 41 <% if @role.new_record? && @roles.any? %>
20 42 <p><label for="copy_workflow_from"><%= l(:label_copy_workflow_from) %></label>
21 43 <%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name, params[:copy_workflow_from] || @copy_from.try(:id))) %></p>
@@ -29,7 +51,8
29 51 <fieldset><legend><%= mod.blank? ? l(:label_project) : l_or_humanize(mod, :prefix => 'project_module_') %></legend>
30 52 <% perms_by_module[mod].each do |permission| %>
31 53 <label class="floating">
32 <%= check_box_tag 'role[permissions][]', permission.name, (@role.permissions.include? permission.name), :id => nil %>
54 <%= check_box_tag 'role[permissions][]', permission.name, (@role.permissions.include? permission.name),
55 :id => "role_permissions_#{permission.name}" %>
33 56 <%= l_or_humanize(permission.name, :prefix => 'permission_') %>
34 57 </label>
35 58 <% end %>
@@ -38,3 +61,11
38 61 <br /><%= check_all_links 'permissions' %>
39 62 <%= hidden_field_tag 'role[permissions][]', '' %>
40 63 </div>
64
65 <%= javascript_tag do %>
66 $(document).ready(function(){
67 $("#role_permissions_manage_members").change(function(){
68 $("#manage_members_options").toggle($(this).is(":checked"));
69 }).change();
70 });
71 <% end %>
Show file before | Show file after | Hide comments
@@ -30,6 +30,20 class MembersControllerTest < ActionController::TestCase
30 30 assert_response :success
31 31 end
32 32
33 def test_new_should_propose_managed_roles_only
34 role = Role.find(1)
35 role.update! :all_roles_managed => false
36 role.managed_roles = Role.where(:id => [2, 3]).to_a
37
38 get :new, :project_id => 1
39 assert_response :success
40 assert_select 'div.roles-selection' do
41 assert_select 'label', :text => 'Manager', :count => 0
42 assert_select 'label', :text => 'Developer'
43 assert_select 'label', :text => 'Reporter'
44 end
45 end
46
33 47 def test_xhr_new
34 48 xhr :get, :new, :project_id => 1
35 49 assert_response :success
@@ -52,6 +66,29 class MembersControllerTest < ActionController::TestCase
52 66 assert User.find(7).member_of?(Project.find(1))
53 67 end
54 68
69 def test_create_should_ignore_unmanaged_roles
70 role = Role.find(1)
71 role.update! :all_roles_managed => false
72 role.managed_roles = Role.where(:id => [2, 3]).to_a
73
74 assert_difference 'Member.count' do
75 post :create, :project_id => 1, :membership => {:role_ids => [1, 2], :user_id => 7}
76 end
77 member = Member.order(:id => :desc).first
78 assert_equal [2], member.role_ids
79 end
80
81 def test_create_should_be_allowed_for_admin_without_role
82 User.find(1).members.delete_all
83 @request.session[:user_id] = 1
84
85 assert_difference 'Member.count' do
86 post :create, :project_id => 1, :membership => {:role_ids => [1, 2], :user_id => 7}
87 end
88 member = Member.order(:id => :desc).first
89 assert_equal [1, 2], member.role_ids
90 end
91
55 92 def test_xhr_create
56 93 assert_difference 'Member.count', 3 do
57 94 xhr :post, :create, :project_id => 1, :membership => {:role_ids => [1], :user_ids => [7, 8, 9]}
@@ -75,14 +112,34 class MembersControllerTest < ActionController::TestCase
75 112 assert_match /alert/, response.body, "Alert message not sent"
76 113 end
77 114
78 def test_edit
115 def test_update
79 116 assert_no_difference 'Member.count' do
80 117 put :update, :id => 2, :membership => {:role_ids => [1], :user_id => 3}
81 118 end
82 119 assert_redirected_to '/projects/ecookbook/settings/members'
83 120 end
84 121
85 def test_xhr_edit
122 def test_update_should_not_add_unmanaged_roles
123 role = Role.find(1)
124 role.update! :all_roles_managed => false
125 role.managed_roles = Role.where(:id => [2, 3]).to_a
126 member = Member.create!(:user => User.find(9), :role_ids => [3], :project_id => 1)
127
128 put :update, :id => member.id, :membership => {:role_ids => [1, 2, 3]}
129 assert_equal [2, 3], member.reload.role_ids.sort
130 end
131
132 def test_update_should_not_remove_unmanaged_roles
133 role = Role.find(1)
134 role.update! :all_roles_managed => false
135 role.managed_roles = Role.where(:id => [2, 3]).to_a
136 member = Member.create!(:user => User.find(9), :role_ids => [1, 3], :project_id => 1)
137
138 put :update, :id => member.id, :membership => {:role_ids => [2]}
139 assert_equal [1, 2], member.reload.role_ids.sort
140 end
141
142 def test_xhr_update
86 143 assert_no_difference 'Member.count' do
87 144 xhr :put, :update, :id => 2, :membership => {:role_ids => [1], :user_id => 3}
88 145 assert_response :success
@@ -103,6 +160,28 class MembersControllerTest < ActionController::TestCase
103 160 assert !User.find(3).member_of?(Project.find(1))
104 161 end
105 162
163 def test_destroy_should_fail_with_unmanaged_roles
164 role = Role.find(1)
165 role.update! :all_roles_managed => false
166 role.managed_roles = Role.where(:id => [2, 3]).to_a
167 member = Member.create!(:user => User.find(9), :role_ids => [1, 3], :project_id => 1)
168
169 assert_no_difference 'Member.count' do
170 delete :destroy, :id => member.id
171 end
172 end
173
174 def test_destroy_should_succeed_with_managed_roles_only
175 role = Role.find(1)
176 role.update! :all_roles_managed => false
177 role.managed_roles = Role.where(:id => [2, 3]).to_a
178 member = Member.create!(:user => User.find(9), :role_ids => [3], :project_id => 1)
179
180 assert_difference 'Member.count', -1 do
181 delete :destroy, :id => member.id
182 end
183 end
184
106 185 def test_xhr_destroy
107 186 assert_difference 'Member.count', -1 do
108 187 xhr :delete, :destroy, :id => 2
Show file before | Show file after | Hide comments
@@ -159,4 +159,35 class MemberTest < ActiveSupport::TestCase
159 159 assert_equal -1, a <=> b
160 160 assert_equal 1, b <=> a
161 161 end
162
163 def test_managed_roles_should_return_all_roles_for_role_with_all_roles_managed
164 member = Member.new
165 member.roles << Role.generate!(:permissions => [:manage_members], :all_roles_managed => true)
166 assert_equal Role.givable.all, member.managed_roles
167 end
168
169 def test_managed_roles_should_return_all_roles_for_admins
170 member = Member.new(:user => User.find(1))
171 member.roles << Role.generate!
172 assert_equal Role.givable.all, member.managed_roles
173 end
174
175 def test_managed_roles_should_return_limited_roles_for_role_without_all_roles_managed
176 member = Member.new
177 member.roles << Role.generate!(:permissions => [:manage_members], :all_roles_managed => false, :managed_role_ids => [2, 3])
178 assert_equal [2, 3], member.managed_roles.map(&:id).sort
179 end
180
181 def test_managed_roles_should_cumulated_managed_roles
182 member = Member.new
183 member.roles << Role.generate!(:permissions => [:manage_members], :all_roles_managed => false, :managed_role_ids => [3])
184 member.roles << Role.generate!(:permissions => [:manage_members], :all_roles_managed => false, :managed_role_ids => [2])
185 assert_equal [2, 3], member.managed_roles.map(&:id).sort
186 end
187
188 def test_managed_roles_should_return_no_roles_for_role_without_permission
189 member = Member.new
190 member.roles << Role.generate!(:all_roles_managed => true)
191 assert_equal [], member.managed_roles
192 end
162 193 end
General Comments 0
You need to be logged in to leave comments. Login now