Merged r23449 from trunk to 6.0-stable (#42106).

git-svn-id: https://svn.redmine.org/redmine/branches/6.0-stable@23456 e93f8b46-1217-0410-a6f0-8f06a7374b81

2 files changed, 34 insertions, 1 deletions

diff --git a/app/models/member.rb b/app/models/member.rb
index ffad20754..f4fedc0db 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -64,7 +64,7 @@ class Member < ApplicationRecord
   def role_ids=(arg)
     ids = (arg || []).collect(&:to_i) - [0]
     # Keep inherited roles
-    ids += member_roles.select {|mr| !mr.inherited_from.nil?}.collect(&:role_id)
+    ids |= member_roles.select {|mr| !mr.inherited_from.nil?}.collect(&:role_id)
 
     new_role_ids = ids - role_ids
     # Add new roles
diff --git a/test/unit/member_test.rb b/test/unit/member_test.rb
index a7b1843dc..f92841b76 100644
--- a/test/unit/member_test.rb
+++ b/test/unit/member_test.rb
@@ -58,6 +58,39 @@ class MemberTest < ActiveSupport::TestCase
     assert_equal 2, @jsmith.reload.roles.size
   end
 
+  def test_update_roles_with_inherited_roles
+    User.current = User.find(1)
+
+    project = Project.find(1)
+    group_a = Group.generate!
+    group_b = Group.generate!
+    test_user = User.generate!
+    group_a.users << test_user
+    group_b.users << test_user
+
+    # Verify that inherited roles are correctly assigned
+    group_a_member = Member.new(project: project, user_id: group_a.id)
+    group_a_member.set_editable_role_ids([1]) # Add Manager role to Group A
+    group_b_member = Member.new(project: project, user_id: group_b.id)
+    group_b_member.set_editable_role_ids([1, 2]) # Add Manager and Developer roles to Group B
+    project.members << [group_a_member, group_b_member]
+    test_user_member = test_user.members.find_by(project_id: project.id)
+    assert_equal [ # [role_id, inherited_from]
+      [1, group_a_member.member_roles.find_by(role_id: 1).id],
+      [1, group_b_member.member_roles.find_by(role_id: 1).id],
+      [2, group_b_member.member_roles.find_by(role_id: 2).id],
+    ], test_user_member.member_roles.map{|r| [r.role_id, r.inherited_from]}
+
+    # Verify that a new non-inherited role is added and inherited roles are maintained
+    test_user_member.set_editable_role_ids([3]) # Add Reporter role to test_user
+    assert_equal [ # [role_id, inherited_from]
+      [1, group_a_member.member_roles.find_by(role_id: 1).id],
+      [1, group_b_member.member_roles.find_by(role_id: 1).id],
+      [2, group_b_member.member_roles.find_by(role_id: 2).id],
+      [3, nil]
+    ], test_user_member.member_roles.map{|r| [r.role_id, r.inherited_from]}
+  end
+
   def test_validate
     member = Member.new(:project_id => 1, :user_id => 2, :role_ids => [2])
     # same use cannot have more than one membership for a project