Commit 2ca3cb6f8fd23e6c2d64d804da8a1dec2dc5bafe
Committed by
Antonio Terceiro
1 parent
74a2b88a
Exists in
master
and in
23 other branches
ActionItem1163: organization admins only can add/change roles of members
Showing
2 changed files
with
41 additions
and
6 deletions
Show diff stats
app/controllers/my_profile/profile_members_controller.rb
| ... | ... | @@ -10,8 +10,8 @@ class ProfileMembersController < MyProfileController |
| 10 | 10 | def update_roles |
| 11 | 11 | @roles = params[:roles] ? environment.roles.find(params[:roles]) : [] |
| 12 | 12 | @roles = @roles.select{|r| r.has_kind?('Profile') } |
| 13 | - @person = Person.find(params[:person]) | |
| 14 | - if @person.define_roles(@roles, profile) | |
| 13 | + @person = profile.members.find { |m| m.id == params[:person].to_i } | |
| 14 | + if @person && @person.define_roles(@roles, profile) | |
| 15 | 15 | flash[:notice] = _('Roles successfuly updated') |
| 16 | 16 | else |
| 17 | 17 | flash[:notice] = _('Couldn\'t change the roles') |
| ... | ... | @@ -21,8 +21,12 @@ class ProfileMembersController < MyProfileController |
| 21 | 21 | |
| 22 | 22 | def change_role |
| 23 | 23 | @roles = profile.roles |
| 24 | - @member = Person.find(params[:id]) | |
| 25 | - @associations = @member.find_roles(@profile) | |
| 24 | + @member = profile.members.find { |m| m.id == params[:id].to_i } | |
| 25 | + if @member | |
| 26 | + @associations = @member.find_roles(@profile) | |
| 27 | + else | |
| 28 | + redirect_to :action => :index | |
| 29 | + end | |
| 26 | 30 | end |
| 27 | 31 | |
| 28 | 32 | def add_role | ... | ... |
test/functional/profile_members_controller_test.rb
| ... | ... | @@ -51,7 +51,7 @@ class ProfileMembersControllerTest < Test::Unit::TestCase |
| 51 | 51 | user = create_user_with_permission('test_user', 'manage_memberships', ent) |
| 52 | 52 | login_as :test_user |
| 53 | 53 | |
| 54 | - get 'change_role', :profile => 'test_enterprise' , :id => member | |
| 54 | + get 'change_role', :profile => 'test_enterprise' , :id => member.id | |
| 55 | 55 | |
| 56 | 56 | assert_response :success |
| 57 | 57 | assert_includes assigns(:roles), role |
| ... | ... | @@ -61,6 +61,19 @@ class ProfileMembersControllerTest < Test::Unit::TestCase |
| 61 | 61 | assert_tag :tag => 'label', :content => role.name |
| 62 | 62 | end |
| 63 | 63 | |
| 64 | + should 'not show form to change role if person is not member' do | |
| 65 | + ent = Enterprise.create!(:identifier => 'test_enterprise', :name => 'test enterprise') | |
| 66 | + not_member = create_user('test_member').person | |
| 67 | + user = create_user_with_permission('test_user', 'manage_memberships', ent) | |
| 68 | + login_as :test_user | |
| 69 | + | |
| 70 | + get 'change_role', :profile => 'test_enterprise' , :id => not_member.id | |
| 71 | + | |
| 72 | + assert_nil assigns('member') | |
| 73 | + assert_response :redirect | |
| 74 | + assert_redirected_to :action => 'index' | |
| 75 | + end | |
| 76 | + | |
| 64 | 77 | should 'update roles' do |
| 65 | 78 | ent = Enterprise.create!(:identifier => 'test_enterprise', :name => 'test enterprise') |
| 66 | 79 | role1 = Role.create!(:name => 'member_role', :permissions => ['edit_profile'], :environment => ent.environment) |
| ... | ... | @@ -71,7 +84,7 @@ class ProfileMembersControllerTest < Test::Unit::TestCase |
| 71 | 84 | user = create_user_with_permission('test_user', 'manage_memberships', ent) |
| 72 | 85 | login_as :test_user |
| 73 | 86 | |
| 74 | - post 'update_roles', :profile => 'test_enterprise', :roles => [role2.id], :person => member | |
| 87 | + post 'update_roles', :profile => 'test_enterprise', :roles => [role2.id], :person => member.id | |
| 75 | 88 | |
| 76 | 89 | assert_response :redirect |
| 77 | 90 | member = Person.find(member.id) |
| ... | ... | @@ -80,6 +93,23 @@ class ProfileMembersControllerTest < Test::Unit::TestCase |
| 80 | 93 | assert_not_includes roles, role1 |
| 81 | 94 | end |
| 82 | 95 | |
| 96 | + should 'not update roles if user is not profile member' do | |
| 97 | + ent = Enterprise.create!(:identifier => 'test_enterprise', :name => 'test enterprise') | |
| 98 | + role = Role.create!(:name => 'owner_role', :permissions => ['edit_profile', 'destroy_profile'], :environment => ent.environment) | |
| 99 | + | |
| 100 | + not_member = create_user('test_member').person | |
| 101 | + user = create_user_with_permission('test_user', 'manage_memberships', ent) | |
| 102 | + login_as :test_user | |
| 103 | + | |
| 104 | + post 'update_roles', :profile => 'test_enterprise', :roles => [role.id], :person => not_member.id | |
| 105 | + | |
| 106 | + assert_response :redirect | |
| 107 | + not_member = Person.find(not_member.id) | |
| 108 | + roles = not_member.find_roles(ent).map(&:role) | |
| 109 | + assert_not_includes roles, role | |
| 110 | + end | |
| 111 | + | |
| 112 | + | |
| 83 | 113 | should 'unassociate community member' do |
| 84 | 114 | com = Community.create!(:identifier => 'test_community', :name => 'test community') |
| 85 | 115 | admin = create_user_with_permission('admin_user', 'manage_memberships', com) |
| ... | ... | @@ -108,6 +138,7 @@ class ProfileMembersControllerTest < Test::Unit::TestCase |
| 108 | 138 | login_as :test_user |
| 109 | 139 | get :change_role, :id => p.id, :profile => com.identifier |
| 110 | 140 | |
| 141 | + assert_equal p, assigns(:member) | |
| 111 | 142 | assert_response :success |
| 112 | 143 | assert_not_includes assigns(:roles), role |
| 113 | 144 | end | ... | ... |