jro_apps/redmine Commit - r13911:ed9f00178c65

Ability to limit member management to certain roles (#19707)....

Jean-Philippe Lang -

r13911:ed9f00178c65

parent child

Context file:

r13911:ed9f00178c65

Collapse all files

db/migrate/20150528084820_add_roles_all_roles_managed.rb created 644 +5 0

@@ -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

db/migrate/20150528092912_create_roles_managed_roles.rb created 644 +8 0

@@ -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

db/migrate/20150528093249_add_unique_index_on_roles_managed_roles.rb created 644 +5 0

@@ -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

app/controllers/members_controller.rb +8 -10

               def create
                 members = []
                 if params[:membership]
-                  if params[:membership][:user_ids]
+                  user_ids = Array.wrap(params[:membership][:user_id] || params[:membership][:user_ids])
-                    attrs = params[:membership].dup
+                  user_ids << nil if user_ids.empty?
-                    user_ids = attrs.delete(:user_ids)
+                  user_ids.each do |user_id|
-                    user_ids.each do |user_id|
+                    member = Member.new(:project => @project, :user_id => user_id)
-                      members << Member.new(:role_ids => params[:membership][:role_ids], :user_id => user_id)
+                    member.set_editable_role_ids(params[:membership][:role_ids])
-                    end
+                    members << member
-                  else
-                    members << Member.new(:role_ids => params[:membership][:role_ids], :user_id => params[:membership][:user_id])
                   end
                   @project.members << members
                 end
               def update
                 if params[:membership]
-                  @member.role_ids = params[:membership][:role_ids]
+                  @member.set_editable_role_ids(params[:membership][:role_ids])
                 end
                 saved = @member.save
                 respond_to do |format|
               end
               def destroy
-                if request.delete? && @member.deletable?
+                if @member.deletable?
                   @member.destroy
                 end
                 respond_to do |format|

app/models/member.rb +67 -10

@@ -29,6 +29,12 class Member < ActiveRecord::Base
29		29
30	before_destroy :set_issue_category_nil	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	def role	38	def role
33	end	39	end
34		40
@@ -70,22 +76,52 class Member < ActiveRecord::Base
70	end	76	end
71	end	77	end
72		78
73	def deletable?	79	# Set member role ids ignoring any change to roles that
74	member_roles.detect {\|mr\| mr.inherited_from}.nil?	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	end	87	end
76		88
77	def destroy	89	# Returns true if one of the member roles is inherited
78	if member_roles.reload.present?	90	def any_inherited_role?
79	# destroying the last role will destroy another instance	91	member_roles.any? {\|mr\| mr.inherited_from}
80	# of the same Member record, #super would then trigger callbacks twice	92	end
81	member_roles.destroy_all	93
82	@destroyed = true	94	# Returns true if the member has the role and if it's inherited
83	freeze	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	else	103	else
85	super	104	user.managed_roles(project).include?(role)
86	end	105	end
87	end	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	def include?(user)	125	def include?(user)
90	if principal.is_a?(Group)	126	if principal.is_a?(Group)
91	!user.nil? && user.groups.include?(principal)	127	!user.nil? && user.groups.include?(principal)
@@ -102,6 +138,27 class Member < ActiveRecord::Base
102	end	138	end
103	end	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	# Creates memberships for principal with the attributes	162	# Creates memberships for principal with the attributes
106	# * project_ids : one or more project ids	163	# * project_ids : one or more project ids
107	# * role_ids : ids of the roles to give to each membership	164	# * role_ids : ids of the roles to give to each membership

app/models/member_role.rb +8 -1

                 !inherited_from.nil?
               end
+              # Destroys the MemberRole without destroying its Member if it doesn't have
+              # any other roles
+              def destroy_without_member_removal
+                @member_removal = false
+                destroy
+              end
               private
               def remove_member_if_empty
-                if member.roles.empty?
+                if @member_removal != false && member.roles.empty?
                   member.destroy
                 end
               end

app/models/role.rb +4 0

               end
               has_and_belongs_to_many :custom_fields, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "role_id"
+              has_and_belongs_to_many :managed_roles, :class_name => 'Role',
+                :join_table => "#{table_name_prefix}roles_managed_roles#{table_name_suffix}",
+                :association_foreign_key => "managed_role_id"
               has_many :member_roles, :dependent => :destroy
               has_many :members, :through => :member_roles
               acts_as_list

app/models/user.rb +9 0

@@ -566,6 +566,15 class User < Principal
566	@visible_project_ids \|\|= Project.visible(self).pluck(:id)	566	@visible_project_ids \|\|= Project.visible(self).pluck(:id)
567	end	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	# Returns true if user is arg or belongs to arg	578	# Returns true if user is arg or belongs to arg
570	def is_or_belongs_to?(arg)	579	def is_or_belongs_to?(arg)
571	if arg.is_a?(User)	580	if arg.is_a?(User)

app/views/members/_new_form.html.erb +1 -1

             <fieldset class="box">
               <legend><%= l(:label_role_plural) %> <%= toggle_checkboxes_link('.roles-selection input') %></legend>
               <div class="roles-selection">
-                <% Role.givable.all.each do |role| %>
+                <% User.current.managed_roles(@project).each do |role| %>
                   <label><%= check_box_tag 'membership[role_ids][]', role.id, false, :id => nil %> <%= role %></label>
                 <% end %>
               </div>

app/views/projects/settings/_members.html.erb +1 -3

                         <%= check_box_tag('membership[role_ids][]',
                                           role.id, member.roles.include?(role),
                                           :id => nil,
-                                          :disabled => member.member_roles.detect {
+                                          :disabled => !member.role_editable?(role)) %> <%= role %>
-                                             |mr| mr.role_id == role.id && !mr.inherited_from.nil?
-                                             } ) %> <%= role %>
                       </label><br />
                       <% end %>
                     </p>

app/views/roles/_form.html.erb +32 -1

@@ -16,6 +16,28
16		16
17	<p><%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {\|v\| [l(v.last), v.first]} %></p>	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	<% if @role.new_record? && @roles.any? %>	41	<% if @role.new_record? && @roles.any? %>
20	<p><label for="copy_workflow_from"><%= l(:label_copy_workflow_from) %></label>	42	<p><label for="copy_workflow_from"><%= l(:label_copy_workflow_from) %></label>
21	<%= 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>	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	<fieldset><legend><%= mod.blank? ? l(:label_project) : l_or_humanize(mod, :prefix => 'project_module_') %></legend>	51	<fieldset><legend><%= mod.blank? ? l(:label_project) : l_or_humanize(mod, :prefix => 'project_module_') %></legend>
30	<% perms_by_module[mod].each do \|permission\| %>	52	<% perms_by_module[mod].each do \|permission\| %>
31	<label class="floating">	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	<%= l_or_humanize(permission.name, :prefix => 'permission_') %>	56	<%= l_or_humanize(permission.name, :prefix => 'permission_') %>
34	</label>	57	</label>
35	<% end %>	58	<% end %>
@@ -38,3 +61,11
38	<br /><%= check_all_links 'permissions' %>	61	<br /><%= check_all_links 'permissions' %>
39	<%= hidden_field_tag 'role[permissions][]', '' %>	62	<%= hidden_field_tag 'role[permissions][]', '' %>
40	</div>	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 %>

test/functional/members_controller_test.rb +81 -2

@@ -30,6 +30,20 class MembersControllerTest < ActionController::TestCase
30	assert_response :success	30	assert_response :success
31	end	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	def test_xhr_new	47	def test_xhr_new
34	xhr :get, :new, :project_id => 1	48	xhr :get, :new, :project_id => 1
35	assert_response :success	49	assert_response :success
@@ -52,6 +66,29 class MembersControllerTest < ActionController::TestCase
52	assert User.find(7).member_of?(Project.find(1))	66	assert User.find(7).member_of?(Project.find(1))
53	end	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	def test_xhr_create	92	def test_xhr_create
56	assert_difference 'Member.count', 3 do	93	assert_difference 'Member.count', 3 do
57	xhr :post, :create, :project_id => 1, :membership => {:role_ids => [1], :user_ids => [7, 8, 9]}	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	assert_match /alert/, response.body, "Alert message not sent"	112	assert_match /alert/, response.body, "Alert message not sent"
76	end	113	end
77		114
78	def test_~~edit~~	115	def test_update
79	assert_no_difference 'Member.count' do	116	assert_no_difference 'Member.count' do
80	put :update, :id => 2, :membership => {:role_ids => [1], :user_id => 3}	117	put :update, :id => 2, :membership => {:role_ids => [1], :user_id => 3}
81	end	118	end
82	assert_redirected_to '/projects/ecookbook/settings/members'	119	assert_redirected_to '/projects/ecookbook/settings/members'
83	end	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	assert_no_difference 'Member.count' do	143	assert_no_difference 'Member.count' do
87	xhr :put, :update, :id => 2, :membership => {:role_ids => [1], :user_id => 3}	144	xhr :put, :update, :id => 2, :membership => {:role_ids => [1], :user_id => 3}
88	assert_response :success	145	assert_response :success
@@ -103,6 +160,28 class MembersControllerTest < ActionController::TestCase
103	assert !User.find(3).member_of?(Project.find(1))	160	assert !User.find(3).member_of?(Project.find(1))
104	end	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	def test_xhr_destroy	185	def test_xhr_destroy
107	assert_difference 'Member.count', -1 do	186	assert_difference 'Member.count', -1 do
108	xhr :delete, :destroy, :id => 2	187	xhr :delete, :destroy, :id => 2

test/unit/member_test.rb +31 0

@@ -159,4 +159,35 class MemberTest < ActiveSupport::TestCase
159	assert_equal -1, a <=> b	159	assert_equal -1, a <=> b
160	assert_equal 1, b <=> a	160	assert_equal 1, b <=> a
161	end	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	end	193	end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages