From ed9f00178c65cc4b37ad2ab56cc89b1c79c4fb8b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 31 May 2015 07:16:23 +0000 Subject: Ability to limit member management to certain roles (#19707). git-svn-id: http://svn.redmine.org/redmine/trunk@14293 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/members_controller.rb | 18 +++---- app/models/member.rb | 77 +++++++++++++++++++++++---- app/models/member_role.rb | 9 +++- app/models/role.rb | 4 ++ app/models/user.rb | 9 ++++ app/views/members/_new_form.html.erb | 2 +- app/views/projects/settings/_members.html.erb | 4 +- app/views/roles/_form.html.erb | 33 +++++++++++- 8 files changed, 130 insertions(+), 26 deletions(-) (limited to 'app') diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index da580fd83..5e5bee822 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -53,14 +53,12 @@ class MembersController < ApplicationController def create members = [] if params[:membership] - if params[:membership][:user_ids] - attrs = params[:membership].dup - user_ids = attrs.delete(:user_ids) - user_ids.each do |user_id| - members << Member.new(:role_ids => params[:membership][:role_ids], :user_id => user_id) - end - else - members << Member.new(:role_ids => params[:membership][:role_ids], :user_id => params[:membership][:user_id]) + user_ids = Array.wrap(params[:membership][:user_id] || params[:membership][:user_ids]) + user_ids << nil if user_ids.empty? + user_ids.each do |user_id| + member = Member.new(:project => @project, :user_id => user_id) + member.set_editable_role_ids(params[:membership][:role_ids]) + members << member end @project.members << members end @@ -84,7 +82,7 @@ class MembersController < ApplicationController 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| @@ -101,7 +99,7 @@ class MembersController < ApplicationController end def destroy - if request.delete? && @member.deletable? + if @member.deletable? @member.destroy end respond_to do |format| diff --git a/app/models/member.rb b/app/models/member.rb index 72a92db53..037a5a057 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -29,6 +29,12 @@ class Member < ActiveRecord::Base before_destroy :set_issue_category_nil + alias :base_reload :reload + def reload(*args) + @managed_roles = nil + base_reload(*args) + end + def role end @@ -70,22 +76,52 @@ class Member < ActiveRecord::Base end end - def deletable? - member_roles.detect {|mr| mr.inherited_from}.nil? + # Set member role ids ignoring any change to roles that + # user is not allowed to manage + def set_editable_role_ids(ids, user=User.current) + ids = (ids || []).collect(&:to_i) - [0] + editable_role_ids = user.managed_roles(project).map(&:id) + untouched_role_ids = self.role_ids - editable_role_ids + touched_role_ids = ids & editable_role_ids + self.role_ids = untouched_role_ids + touched_role_ids end - def destroy - if member_roles.reload.present? - # destroying the last role will destroy another instance - # of the same Member record, #super would then trigger callbacks twice - member_roles.destroy_all - @destroyed = true - freeze + # Returns true if one of the member roles is inherited + def any_inherited_role? + member_roles.any? {|mr| mr.inherited_from} + end + + # Returns true if the member has the role and if it's inherited + def has_inherited_role?(role) + member_roles.any? {|mr| mr.role_id == role.id && mr.inherited_from.present?} + end + + # Returns true if the member's role is editable by user + def role_editable?(role, user=User.current) + if has_inherited_role?(role) + false else - super + user.managed_roles(project).include?(role) end end + # Returns true if the member is deletable by user + def deletable?(user=User.current) + if any_inherited_role? + false + else + roles & user.managed_roles(project) == roles + end + end + + # Destroys the member + def destroy + member_roles.reload.each(&:destroy_without_member_removal) + super + end + + # Returns true if the member is user or is a group + # that includes user def include?(user) if principal.is_a?(Group) !user.nil? && user.groups.include?(principal) @@ -102,6 +138,27 @@ class Member < ActiveRecord::Base end end + # Returns the roles that the member is allowed to manage + # in the project the member belongs to + def managed_roles + @managed_roles ||= begin + if principal.try(:admin?) + Role.givable.to_a + else + members_management_roles = roles.select do |role| + role.has_permission?(:manage_members) + end + if members_management_roles.empty? + [] + elsif members_management_roles.any?(&:all_roles_managed?) + Role.givable.to_a + else + members_management_roles.map(&:managed_roles).reduce(&:|) + end + end + end + end + # Creates memberships for principal with the attributes # * project_ids : one or more project ids # * role_ids : ids of the roles to give to each membership diff --git a/app/models/member_role.rb b/app/models/member_role.rb index afa5d2e6a..526daf9cf 100644 --- a/app/models/member_role.rb +++ b/app/models/member_role.rb @@ -36,10 +36,17 @@ class MemberRole < ActiveRecord::Base !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 diff --git a/app/models/role.rb b/app/models/role.rb index 43f32b726..42dddaf47 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -64,6 +64,10 @@ class Role < ActiveRecord::Base 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 diff --git a/app/models/user.rb b/app/models/user.rb index 70610159f..9ff706178 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -566,6 +566,15 @@ class User < Principal @visible_project_ids ||= Project.visible(self).pluck(:id) end + # Returns the roles that the user is allowed to manage for the given project + def managed_roles(project) + if admin? + Role.givable.to_a + else + membership(project).try(:managed_roles) || [] + end + end + # Returns true if user is arg or belongs to arg def is_or_belongs_to?(arg) if arg.is_a?(User) diff --git a/app/views/members/_new_form.html.erb b/app/views/members/_new_form.html.erb index 9140da7c4..3fe9187d1 100644 --- a/app/views/members/_new_form.html.erb +++ b/app/views/members/_new_form.html.erb @@ -9,7 +9,7 @@
<%= l(:label_role_plural) %> <%= toggle_checkboxes_link('.roles-selection input') %>
- <% Role.givable.all.each do |role| %> + <% User.current.managed_roles(@project).each do |role| %> <% end %>
diff --git a/app/views/projects/settings/_members.html.erb b/app/views/projects/settings/_members.html.erb index 63a4bfba0..c9a91e3b1 100644 --- a/app/views/projects/settings/_members.html.erb +++ b/app/views/projects/settings/_members.html.erb @@ -32,9 +32,7 @@ <%= check_box_tag('membership[role_ids][]', role.id, member.roles.include?(role), :id => nil, - :disabled => member.member_roles.detect { - |mr| mr.role_id == role.id && !mr.inherited_from.nil? - } ) %> <%= role %> + :disabled => !member.role_editable?(role)) %> <%= role %>
<% end %>

diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb index 03cbe58d2..8feab5d92 100644 --- a/app/views/roles/_form.html.erb +++ b/app/views/roles/_form.html.erb @@ -16,6 +16,28 @@

<%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

+ <% unless @role.builtin? %> +

+ + + + <% Role.givable.sorted.each do |role| %> + + <% end %> + <%= hidden_field_tag 'role[managed_role_ids][]', '' %> + <% end %> + <% if @role.new_record? && @roles.any? %>

<%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name, params[:copy_workflow_from] || @copy_from.try(:id))) %>

@@ -29,7 +51,8 @@
<%= mod.blank? ? l(:label_project) : l_or_humanize(mod, :prefix => 'project_module_') %> <% perms_by_module[mod].each do |permission| %> <% end %> @@ -38,3 +61,11 @@
<%= check_all_links 'permissions' %> <%= hidden_field_tag 'role[permissions][]', '' %> + +<%= javascript_tag do %> +$(document).ready(function(){ + $("#role_permissions_manage_members").change(function(){ + $("#manage_members_options").toggle($(this).is(":checked")); + }).change(); +}); +<% end %> -- cgit v1.2.3