From 2ca3cb6f8fd23e6c2d64d804da8a1dec2dc5bafe Mon Sep 17 00:00:00 2001
From: Moises Machado <moises@colivre.coop.br>
Date: Wed, 22 Jul 2009 19:14:03 -0300
Subject: [PATCH] ActionItem1163: organization admins only can add/change roles of members

---
 app/controllers/my_profile/profile_members_controller.rb | 12 ++++++++----
 test/functional/profile_members_controller_test.rb       | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/app/controllers/my_profile/profile_members_controller.rb b/app/controllers/my_profile/profile_members_controller.rb
index 0223918..cad2cac 100644
--- a/app/controllers/my_profile/profile_members_controller.rb
+++ b/app/controllers/my_profile/profile_members_controller.rb
@@ -10,8 +10,8 @@ class ProfileMembersController < MyProfileController
   def update_roles
     @roles = params[:roles] ? environment.roles.find(params[:roles]) : []
     @roles = @roles.select{|r| r.has_kind?('Profile') }
-    @person = Person.find(params[:person])      
-    if @person.define_roles(@roles, profile)
+    @person = profile.members.find { |m| m.id == params[:person].to_i }
+    if @person && @person.define_roles(@roles, profile)
       flash[:notice] = _('Roles successfuly updated')
     else
       flash[:notice] = _('Couldn\'t change the roles')
@@ -21,8 +21,12 @@ class ProfileMembersController < MyProfileController
   
   def change_role
     @roles = profile.roles
-    @member = Person.find(params[:id])
-    @associations = @member.find_roles(@profile)
+    @member = profile.members.find { |m| m.id == params[:id].to_i }
+    if @member
+      @associations = @member.find_roles(@profile)
+    else
+      redirect_to :action => :index
+    end
   end
 
   def add_role
diff --git a/test/functional/profile_members_controller_test.rb b/test/functional/profile_members_controller_test.rb
index 116f5c7..933b3b7 100644
--- a/test/functional/profile_members_controller_test.rb
+++ b/test/functional/profile_members_controller_test.rb
@@ -51,7 +51,7 @@ class ProfileMembersControllerTest < Test::Unit::TestCase
     user = create_user_with_permission('test_user', 'manage_memberships', ent)
     login_as :test_user
 
-    get 'change_role', :profile => 'test_enterprise' , :id => member
+    get 'change_role', :profile => 'test_enterprise' , :id => member.id
 
     assert_response :success
     assert_includes assigns(:roles), role
@@ -61,6 +61,19 @@ class ProfileMembersControllerTest < Test::Unit::TestCase
     assert_tag :tag => 'label', :content => role.name
   end
 
+  should 'not show form to change role if person is not member' do
+    ent = Enterprise.create!(:identifier => 'test_enterprise', :name => 'test enterprise')
+    not_member = create_user('test_member').person
+    user = create_user_with_permission('test_user', 'manage_memberships', ent)
+    login_as :test_user
+
+    get 'change_role', :profile => 'test_enterprise' , :id => not_member.id
+
+    assert_nil assigns('member')
+    assert_response :redirect
+    assert_redirected_to :action => 'index'
+  end
+
   should 'update roles' do
     ent = Enterprise.create!(:identifier => 'test_enterprise', :name => 'test enterprise')
     role1 = Role.create!(:name => 'member_role', :permissions => ['edit_profile'], :environment => ent.environment)
@@ -71,7 +84,7 @@ class ProfileMembersControllerTest < Test::Unit::TestCase
     user = create_user_with_permission('test_user', 'manage_memberships', ent)
     login_as :test_user
 
-    post 'update_roles', :profile => 'test_enterprise', :roles => [role2.id], :person => member
+    post 'update_roles', :profile => 'test_enterprise', :roles => [role2.id], :person => member.id
 
     assert_response :redirect
     member = Person.find(member.id)
@@ -80,6 +93,23 @@ class ProfileMembersControllerTest < Test::Unit::TestCase
     assert_not_includes roles, role1
   end
 
+  should 'not update roles if user is not profile member' do
+    ent = Enterprise.create!(:identifier => 'test_enterprise', :name => 'test enterprise')
+    role = Role.create!(:name => 'owner_role', :permissions => ['edit_profile', 'destroy_profile'], :environment => ent.environment)
+
+    not_member = create_user('test_member').person
+    user = create_user_with_permission('test_user', 'manage_memberships', ent)
+    login_as :test_user
+
+    post 'update_roles', :profile => 'test_enterprise', :roles => [role.id], :person => not_member.id
+
+    assert_response :redirect
+    not_member = Person.find(not_member.id)
+    roles = not_member.find_roles(ent).map(&:role)
+    assert_not_includes  roles, role
+  end
+
+
   should 'unassociate community member' do
     com = Community.create!(:identifier => 'test_community', :name => 'test community')
     admin = create_user_with_permission('admin_user', 'manage_memberships', com)
@@ -108,6 +138,7 @@ class ProfileMembersControllerTest < Test::Unit::TestCase
     login_as :test_user
     get :change_role, :id => p.id, :profile => com.identifier
 
+    assert_equal p, assigns(:member)
     assert_response :success
     assert_not_includes assigns(:roles), role
   end
--
libgit2 0.21.2