diff options
| -rw-r--r-- | app/models/member.rb | 10 | ||||
| -rw-r--r-- | app/models/member_role.rb | 20 | ||||
| -rw-r--r-- | app/models/project.rb | 46 | ||||
| -rw-r--r-- | app/views/projects/_form.html.erb | 13 | ||||
| -rw-r--r-- | config/locales/en.yml | 1 | ||||
| -rw-r--r-- | config/locales/fr.yml | 1 | ||||
| -rw-r--r-- | db/migrate/20130202090625_add_projects_inherit_members.rb | 9 | ||||
| -rw-r--r-- | test/functional/projects_controller_test.rb | 19 | ||||
| -rw-r--r-- | test/object_helpers.rb | 6 | ||||
| -rw-r--r-- | test/unit/project_members_inheritance_test.rb | 260 |
10 files changed, 376 insertions, 9 deletions
diff --git a/app/models/member.rb b/app/models/member.rb index 649edb2eb..5ff31f316 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -97,6 +97,16 @@ class Member < ActiveRecord::Base @membership end + # Finds or initilizes a Member for the given project and principal + def self.find_or_new(project, principal) + project_id = project.is_a?(Project) ? project.id : project + principal_id = principal.is_a?(Principal) ? principal.id : principal + + member = Member.find_by_project_id_and_user_id(project_id, principal_id) + member ||= Member.new(:project_id => project_id, :user_id => principal_id) + member + end + protected def validate_role diff --git a/app/models/member_role.rb b/app/models/member_role.rb index 4f97e35af..f344696be 100644 --- a/app/models/member_role.rb +++ b/app/models/member_role.rb @@ -21,8 +21,8 @@ class MemberRole < ActiveRecord::Base after_destroy :remove_member_if_empty - after_create :add_role_to_group_users - after_destroy :remove_role_from_group_users + after_create :add_role_to_group_users, :add_role_to_subprojects + after_destroy :remove_inherited_roles validates_presence_of :role validate :validate_role_member @@ -44,16 +44,26 @@ class MemberRole < ActiveRecord::Base end def add_role_to_group_users - if member.principal.is_a?(Group) + if member.principal.is_a?(Group) && !inherited? member.principal.users.each do |user| - user_member = Member.find_by_project_id_and_user_id(member.project_id, user.id) || Member.new(:project_id => member.project_id, :user_id => user.id) + user_member = Member.find_or_new(member.project_id, user.id) user_member.member_roles << MemberRole.new(:role => role, :inherited_from => id) user_member.save! end end end - def remove_role_from_group_users + def add_role_to_subprojects + member.project.children.each do |subproject| + if subproject.inherit_members? + child_member = Member.find_or_new(subproject.id, member.user_id) + child_member.member_roles << MemberRole.new(:role => role, :inherited_from => id) + child_member.save! + end + end + end + + def remove_inherited_roles MemberRole.where(:inherited_from => id).all.group_by(&:member).each do |member, member_roles| member_roles.each(&:destroy) if member && member.user diff --git a/app/models/project.rb b/app/models/project.rb index 27c962fbc..1ba7bcff3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -82,6 +82,7 @@ class Project < ActiveRecord::Base validates_exclusion_of :identifier, :in => %w( new ) after_save :update_position_under_parent, :if => Proc.new {|project| project.name_changed?} + after_save :update_inherited_members, :if => Proc.new {|project| project.inherit_members_changed?} before_destroy :delete_all_members scope :has_module, lambda {|mod| @@ -651,6 +652,9 @@ class Project < ActiveRecord::Base safe_attributes 'enabled_module_names', :if => lambda {|project, user| project.new_record? || user.allowed_to?(:select_project_modules, project) } + safe_attributes 'inherit_members', + :if => lambda {|project, user| project.parent.nil? || project.parent.visible?(:user)} + # Returns an array of projects that are in this project's hierarchy # # Example: parents, children, siblings @@ -726,6 +730,44 @@ class Project < ActiveRecord::Base private + def after_parent_changed(parent_was) + remove_inherited_member_roles + add_inherited_member_roles + end + + def update_inherited_members + if parent + if inherit_members? && !inherit_members_was + remove_inherited_member_roles + add_inherited_member_roles + elsif !inherit_members? && inherit_members_was + remove_inherited_member_roles + end + end + end + + def remove_inherited_member_roles + member_roles = memberships.map(&:member_roles).flatten + member_role_ids = member_roles.map(&:id) + member_roles.each do |member_role| + if member_role.inherited_from && !member_role_ids.include?(member_role.inherited_from) + member_role.destroy + end + end + end + + def add_inherited_member_roles + if inherit_members? && parent + parent.memberships.each do |parent_member| + member = Member.find_or_new(self.id, parent_member.user_id) + parent_member.member_roles.each do |parent_member_role| + member.member_roles << MemberRole.new(:role => parent_member_role.role, :inherited_from => parent_member_role.id) + end + member.save! + end + end + end + # Copies wiki from +project+ def copy_wiki(project) # Check that the source project has a wiki first @@ -951,6 +993,7 @@ class Project < ActiveRecord::Base # Inserts/moves the project so that target's children or root projects stay alphabetically sorted def set_or_update_position_under(target_parent) + parent_was = parent sibs = (target_parent.nil? ? self.class.roots : target_parent.children) to_be_inserted_before = sibs.sort_by {|c| c.name.to_s.downcase}.detect {|c| c.name.to_s.downcase > name.to_s.downcase } @@ -967,5 +1010,8 @@ class Project < ActiveRecord::Base # move_to_child_of adds the project in last (ie.right) position move_to_child_of(target_parent) end + if parent_was != target_parent + after_parent_changed(parent_was) + end end end diff --git a/app/views/projects/_form.html.erb b/app/views/projects/_form.html.erb index 27328d638..6c853fa2a 100644 --- a/app/views/projects/_form.html.erb +++ b/app/views/projects/_form.html.erb @@ -4,10 +4,6 @@ <!--[form:project]--> <p><%= f.text_field :name, :required => true, :size => 60 %></p> -<% unless @project.allowed_parents.compact.empty? %> - <p><%= label(:project, :parent_id, l(:field_parent)) %><%= parent_project_select_tag(@project) %></p> -<% end %> - <p><%= f.text_area :description, :rows => 5, :class => 'wiki-edit' %></p> <p><%= f.text_field :identifier, :required => true, :size => 60, :disabled => @project.identifier_frozen?, :maxlength => Project::IDENTIFIER_MAX_LENGTH %> <% unless @project.identifier_frozen? %> @@ -15,6 +11,15 @@ <% end %></p> <p><%= f.text_field :homepage, :size => 60 %></p> <p><%= f.check_box :is_public %></p> + +<% unless @project.allowed_parents.compact.empty? %> + <p><%= label(:project, :parent_id, l(:field_parent)) %><%= parent_project_select_tag(@project) %></p> +<% end %> + +<% if @project.safe_attribute? 'priority_id' %> +<p><%= f.check_box :inherit_members %></p> +<% end %> + <%= wikitoolbar_for 'project_description' %> <% @project.custom_field_values.each do |value| %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 05ec1d32f..95793fc4b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -329,6 +329,7 @@ en: field_timeout: "Timeout (in seconds)" field_board_parent: Parent forum field_private_notes: Private notes + field_inherit_members: Inherit members setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f39943472..cf7280fec 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -329,6 +329,7 @@ fr: field_timeout: "Timeout (en secondes)" field_board_parent: Forum parent field_private_notes: Notes privées + field_inherit_members: Hériter les membres setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/db/migrate/20130202090625_add_projects_inherit_members.rb b/db/migrate/20130202090625_add_projects_inherit_members.rb new file mode 100644 index 000000000..9cf5bade3 --- /dev/null +++ b/db/migrate/20130202090625_add_projects_inherit_members.rb @@ -0,0 +1,9 @@ +class AddProjectsInheritMembers < ActiveRecord::Migration + def up + add_column :projects, :inherit_members, :boolean, :default => false, :null => false + end + + def down + remove_column :projects, :inherit_members + end +end diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index af421310f..3927e41b0 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -288,6 +288,25 @@ class ProjectsControllerTest < ActionController::TestCase end end + def test_create_subproject_with_inherit_members_should_inherit_members + Role.find_by_name('Manager').add_permission! :add_subprojects + parent = Project.find(1) + @request.session[:user_id] = 2 + + assert_difference 'Project.count' do + post :create, :project => { + :name => 'inherited', :identifier => 'inherited', :parent_id => parent.id, :inherit_members => '1' + } + assert_response 302 + end + + project = Project.order('id desc').first + assert_equal 'inherited', project.name + assert_equal parent, project.parent + assert project.memberships.count > 0 + assert_equal parent.memberships.count, project.memberships.count + end + def test_create_should_preserve_modules_on_validation_failure with_settings :default_projects_modules => ['issue_tracking', 'repository'] do @request.session[:user_id] = 1 diff --git a/test/object_helpers.rb b/test/object_helpers.rb index 4b5fbdbbe..335828de5 100644 --- a/test/object_helpers.rb +++ b/test/object_helpers.rb @@ -39,6 +39,12 @@ module ObjectHelpers project end + def Project.generate_with_parent!(parent, attributes={}) + project = Project.generate!(attributes) + project.set_parent!(parent) + project + end + def Tracker.generate!(attributes={}) @generated_tracker_name ||= 'Tracker 0' @generated_tracker_name.succ! diff --git a/test/unit/project_members_inheritance_test.rb b/test/unit/project_members_inheritance_test.rb new file mode 100644 index 000000000..3ae133838 --- /dev/null +++ b/test/unit/project_members_inheritance_test.rb @@ -0,0 +1,260 @@ +# Redmine - project management software +# Copyright (C) 2006-2013 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.expand_path('../../test_helper', __FILE__) + +class ProjectMembersInheritanceTest < ActiveSupport::TestCase + fixtures :roles, :users + + def setup + @parent = Project.generate! + @member = Member.create!(:principal => User.find(2), :project => @parent, :role_ids => [1, 2]) + assert_equal 2, @member.reload.roles.size + end + + def test_project_created_with_inherit_members_disabled_should_not_inherit_members + assert_no_difference 'Member.count' do + project = Project.generate_with_parent!(@parent, :inherit_members => false) + + assert_equal 0, project.memberships.count + end + end + + def test_project_created_with_inherit_members_should_inherit_members + assert_difference 'Member.count', 1 do + project = Project.generate_with_parent!(@parent, :inherit_members => true) + project.reload + + assert_equal 1, project.memberships.count + member = project.memberships.first + assert_equal @member.principal, member.principal + assert_equal @member.roles.sort, member.roles.sort + end + end + + def test_turning_on_inherit_members_should_inherit_members + Project.generate_with_parent!(@parent, :inherit_members => false) + + assert_difference 'Member.count', 1 do + project = Project.order('id desc').first + project.inherit_members = true + project.save! + project.reload + + assert_equal 1, project.memberships.count + member = project.memberships.first + assert_equal @member.principal, member.principal + assert_equal @member.roles.sort, member.roles.sort + end + end + + def test_turning_off_inherit_members_should_inherit_members + Project.generate_with_parent!(@parent, :inherit_members => true) + + assert_difference 'Member.count', -1 do + project = Project.order('id desc').first + project.inherit_members = false + project.save! + project.reload + + assert_equal 0, project.memberships.count + end + end + + def test_moving_a_root_project_under_a_parent_should_inherit_members + Project.generate!(:inherit_members => true) + project = Project.order('id desc').first + + assert_difference 'Member.count', 1 do + project.set_parent!(@parent) + project.reload + + assert_equal 1, project.memberships.count + member = project.memberships.first + assert_equal @member.principal, member.principal + assert_equal @member.roles.sort, member.roles.sort + end + end + + def test_moving_a_subproject_as_root_should_loose_inherited_members + Project.generate_with_parent!(@parent, :inherit_members => true) + project = Project.order('id desc').first + + assert_difference 'Member.count', -1 do + project.set_parent!(nil) + project.reload + + assert_equal 0, project.memberships.count + end + end + + def test_moving_a_subproject_to_another_parent_should_change_inherited_members + other_parent = Project.generate! + other_member = Member.create!(:principal => User.find(4), :project => other_parent, :role_ids => [3]) + + Project.generate_with_parent!(@parent, :inherit_members => true) + project = Project.order('id desc').first + project.set_parent!(other_parent.reload) + project.reload + + assert_equal 1, project.memberships.count + member = project.memberships.first + assert_equal other_member.principal, member.principal + assert_equal other_member.roles.sort, member.roles.sort + end + + def test_inheritance_should_propagate_to_subprojects + project = Project.generate_with_parent!(@parent, :inherit_members => false) + subproject = Project.generate_with_parent!(project, :inherit_members => true) + project.reload + + assert_difference 'Member.count', 2 do + project.inherit_members = true + project.save + project.reload + subproject.reload + + assert_equal 1, project.memberships.count + assert_equal 1, subproject.memberships.count + member = subproject.memberships.first + assert_equal @member.principal, member.principal + assert_equal @member.roles.sort, member.roles.sort + end + end + + def test_inheritance_removal_should_propagate_to_subprojects + project = Project.generate_with_parent!(@parent, :inherit_members => true) + subproject = Project.generate_with_parent!(project, :inherit_members => true) + project.reload + + assert_difference 'Member.count', -2 do + project.inherit_members = false + project.save + project.reload + subproject.reload + + assert_equal 0, project.memberships.count + assert_equal 0, subproject.memberships.count + end + end + + def test_adding_a_member_should_propagate + project = Project.generate_with_parent!(@parent, :inherit_members => true) + + assert_difference 'Member.count', 2 do + member = Member.create!(:principal => User.find(4), :project => @parent, :role_ids => [1, 3]) + + inherited_member = project.memberships.order('id desc').first + assert_equal member.principal, inherited_member.principal + assert_equal member.roles.sort, inherited_member.roles.sort + end + end + + def test_adding_a_member_should_not_propagate_if_child_does_not_inherit + project = Project.generate_with_parent!(@parent, :inherit_members => false) + + assert_difference 'Member.count', 1 do + member = Member.create!(:principal => User.find(4), :project => @parent, :role_ids => [1, 3]) + + assert_nil project.reload.memberships.detect {|m| m.principal == member.principal} + end + end + + def test_removing_a_member_should_propagate + project = Project.generate_with_parent!(@parent, :inherit_members => true) + + assert_difference 'Member.count', -2 do + @member.reload.destroy + project.reload + + assert_equal 0, project.memberships.count + end + end + + def test_adding_a_group_member_should_propagate_with_its_users + project = Project.generate_with_parent!(@parent, :inherit_members => true) + group = Group.generate! + user = User.find(4) + group.users << user + + assert_difference 'Member.count', 4 do + assert_difference 'MemberRole.count', 8 do + member = Member.create!(:principal => group, :project => @parent, :role_ids => [1, 3]) + project.reload + + inherited_group_member = project.memberships.detect {|m| m.principal == group} + assert_not_nil inherited_group_member + assert_equal member.roles.sort, inherited_group_member.roles.sort + + inherited_user_member = project.memberships.detect {|m| m.principal == user} + assert_not_nil inherited_user_member + assert_equal member.roles.sort, inherited_user_member.roles.sort + end + end + end + + def test_removing_a_group_member_should_propagate + project = Project.generate_with_parent!(@parent, :inherit_members => true) + group = Group.generate! + user = User.find(4) + group.users << user + member = Member.create!(:principal => group, :project => @parent, :role_ids => [1, 3]) + + assert_difference 'Member.count', -4 do + assert_difference 'MemberRole.count', -8 do + member.destroy + project.reload + + inherited_group_member = project.memberships.detect {|m| m.principal == group} + assert_nil inherited_group_member + + inherited_user_member = project.memberships.detect {|m| m.principal == user} + assert_nil inherited_user_member + end + end + end + + def test_adding_user_who_use_is_already_a_member_to_parent_project_should_merge_roles + project = Project.generate_with_parent!(@parent, :inherit_members => true) + user = User.find(4) + Member.create!(:principal => user, :project => project, :role_ids => [1, 2]) + + assert_difference 'Member.count', 1 do + Member.create!(:principal => User.find(4), :project => @parent.reload, :role_ids => [1, 3]) + + member = project.reload.memberships.detect {|m| m.principal == user} + assert_not_nil member + assert_equal [1, 2, 3], member.roles.uniq.sort.map(&:id) + end + end + + def test_turning_on_inheritance_with_user_who_is_already_a_member_should_merge_roles + project = Project.generate_with_parent!(@parent) + user = @member.user + Member.create!(:principal => user, :project => project, :role_ids => [1, 3]) + project.reload + + assert_no_difference 'Member.count' do + project.inherit_members = true + project.save! + + member = project.reload.memberships.detect {|m| m.principal == user} + assert_not_nil member + assert_equal [1, 2, 3], member.roles.uniq.sort.map(&:id) + end + end +end |