Optionaly inherit members from parent project (#5605).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11298 e93f8b46-1217-0410-a6f0-8f06a7374b81

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

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_create :add_role_to_group_users, :add_role_to_subprojects
-  after_destroy :remove_role_from_group_users
+  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

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

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

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

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

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

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

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!

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