From d755f992e420d1a7d908d9f4fd7fd965b30cf400 Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Mon, 20 Mar 2017 12:44:33 -0400 Subject: [PATCH 1/4] add multiple resources to a service https://www.pivotaltracker.com/story/show/142057167 add add_resource to the collection --- app/controllers/api/services_controller.rb | 13 +++++ config/api.yml | 4 ++ spec/requests/api/custom_actions_spec.rb | 4 +- spec/requests/api/services_spec.rb | 67 ++++++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/services_controller.rb b/app/controllers/api/services_controller.rb index 27e7e4c00bb..bc2ed075ac7 100644 --- a/app/controllers/api/services_controller.rb +++ b/app/controllers/api/services_controller.rb @@ -18,6 +18,19 @@ def edit_resource(type, id, data) super(type, id, attributes) end + def add_resource_resource(type, id, data) + svc = resource_search(id, type, collection_class(type)) + data['resources'].each do |resource_ref| + resource_type, resource_id = parse_href(resource_ref['href']) + resource = resource_search(resource_id, resource_type, collection_class(resource_type)) + svc.add_resource(resource) + end + svc.save! + action_result(true, "Assigned resources to #{service_ident(svc)}") + rescue => err + action_result(false, err.to_s) + end + def reconfigure_resource(type, id = nil, data = nil) raise BadRequestError, "Must specify an id for Reconfiguring a #{type} resource" unless id diff --git a/config/api.yml b/config/api.yml index 111d76a87d3..8c65ed798b3 100644 --- a/config/api.yml +++ b/config/api.yml @@ -1911,6 +1911,8 @@ :identifier: service_tag - :name: unassign_tags :identifier: service_tag + - :name: add_resource + :identifier: service_edit :resource_actions: :get: - :name: read @@ -1934,6 +1936,8 @@ :identifier: service_admin - :name: delete :identifier: service_delete + - :name: add_resource + :identifier: service_edit :delete: - :name: delete :identifier: service_delete diff --git a/spec/requests/api/custom_actions_spec.rb b/spec/requests/api/custom_actions_spec.rb index e42d39a599c..1d9f1d9ac17 100644 --- a/spec/requests/api/custom_actions_spec.rb +++ b/spec/requests/api/custom_actions_spec.rb @@ -75,7 +75,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit add_resource)) end end @@ -91,7 +91,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3 add_resource)) end it "supports the custom_actions attribute" do diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 89cded552d7..bd5b0d8c26b 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -667,4 +667,71 @@ def expect_svc_with_vms expect(response).to have_http_status(:forbidden) end end + + describe 'add_resource' do + let(:vm1) { FactoryGirl.create(:vm_vmware) } + let(:vm2) { FactoryGirl.create(:vm_vmware) } + let(:vm3) { FactoryGirl.create(:vm_vmware) } + + it 'can add multiple vms by href with an appropriate role' do + api_basic_authorize(action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resources' => [ + { 'href' => vms_url(vm1.id) }, + { 'href' => vms_url(vm2.id) } + ] + } + + run_post(services_url(svc.id), request) + + expected = { + 'success' => true, + 'message' => "Assigned resources to Service id:#{svc.id} name:'#{svc.name}'" + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + expect(svc.reload.vms).to eq([vm1, vm2]) + end + + it 'cannot add multiple vms by href without an appropriate role' do + api_basic_authorize + + run_post(services_url(svc.id), 'action' => 'add_resource') + + expect(response).to have_http_status(:forbidden) + end + + it 'can add multiple vms to multiple services by href with an appropriate role' do + api_basic_authorize(action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resources' => [ + { 'href' => services_url(svc.id), 'resources' => [{'href' => vms_url(vm1.id)}, {'href' => vms_url(vm2.id)}] }, + { 'href' => services_url(svc1.id), 'resources' => [{'href' => vms_url(vm3.id)}] } + ] + } + + run_post(services_url, request) + + expected = { + 'results' => [ + { 'success' => true, 'message' => "Assigned resources to Service id:#{svc.id} name:'#{svc.name}'"}, + { 'success' => true, 'message' => "Assigned resources to Service id:#{svc1.id} name:'#{svc1.name}'"} + ] + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + expect(svc.reload.vms).to eq([vm1, vm2]) + expect(svc1.reload.vms).to eq([vm3]) + end + + it 'cannot add multiple vms to multiple services by href without an appropriate role' do + api_basic_authorize + + run_post(services_url, 'action' => 'add_resource') + + expect(response).to have_http_status(:forbidden) + end + end end From b82284207b7fbc4471b97be3d3acd1cf8800c0e6 Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Mon, 20 Mar 2017 18:30:24 -0400 Subject: [PATCH 2/4] use add_to_resource --- app/controllers/api/services_controller.rb | 3 ++- spec/requests/api/services_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/services_controller.rb b/app/controllers/api/services_controller.rb index bc2ed075ac7..46935b000e9 100644 --- a/app/controllers/api/services_controller.rb +++ b/app/controllers/api/services_controller.rb @@ -23,7 +23,8 @@ def add_resource_resource(type, id, data) data['resources'].each do |resource_ref| resource_type, resource_id = parse_href(resource_ref['href']) resource = resource_search(resource_id, resource_type, collection_class(resource_type)) - svc.add_resource(resource) + raise "Cannot assign #{resource_type} to #{service_ident(svc)}" unless resource.respond_to? :add_to_service + resource.add_to_service(svc) end svc.save! action_result(true, "Assigned resources to #{service_ident(svc)}") diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index bd5b0d8c26b..8cdb2af4f64 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -733,5 +733,25 @@ def expect_svc_with_vms expect(response).to have_http_status(:forbidden) end + + it 'raises an error if the service cannot be added' do + user = FactoryGirl.create(:user) + api_basic_authorize(action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resources' => [ + { 'href' => users_url(user.id) } + ] + } + + run_post(services_url(svc.id), request) + + expected = { + 'success' => false, + 'message' => "Cannot assign users to Service id:#{svc.id} name:'#{svc.name}'" + } + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + end end end From d6008f01393d0d7b7ad11949af3f446e9c0eecbb Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Wed, 22 Mar 2017 09:55:01 -0400 Subject: [PATCH 3/4] limit to collection and return individual results --- app/controllers/api/services_controller.rb | 17 +++--- config/api.yml | 2 - spec/requests/api/custom_actions_spec.rb | 4 +- spec/requests/api/services_spec.rb | 62 ++++++++-------------- 4 files changed, 33 insertions(+), 52 deletions(-) diff --git a/app/controllers/api/services_controller.rb b/app/controllers/api/services_controller.rb index 46935b000e9..3f3a368ac24 100644 --- a/app/controllers/api/services_controller.rb +++ b/app/controllers/api/services_controller.rb @@ -20,14 +20,17 @@ def edit_resource(type, id, data) def add_resource_resource(type, id, data) svc = resource_search(id, type, collection_class(type)) - data['resources'].each do |resource_ref| - resource_type, resource_id = parse_href(resource_ref['href']) - resource = resource_search(resource_id, resource_type, collection_class(resource_type)) - raise "Cannot assign #{resource_type} to #{service_ident(svc)}" unless resource.respond_to? :add_to_service - resource.add_to_service(svc) + data['resources'].collect do |resource_ref| + begin + resource_type, resource_id = parse_href(resource_ref['href']) + resource = resource_search(resource_id, resource_type, collection_class(resource_type)) + raise "Cannot assign #{resource_type} to #{service_ident(svc)}" unless resource.respond_to? :add_to_service + resource.add_to_service(svc) + action_result(true, "Assigned resource #{resource_type} id:#{resource_id} to #{service_ident(svc)}") + rescue => err + action_result(false, err.to_s) + end end - svc.save! - action_result(true, "Assigned resources to #{service_ident(svc)}") rescue => err action_result(false, err.to_s) end diff --git a/config/api.yml b/config/api.yml index 8c65ed798b3..4e07b8e507e 100644 --- a/config/api.yml +++ b/config/api.yml @@ -1936,8 +1936,6 @@ :identifier: service_admin - :name: delete :identifier: service_delete - - :name: add_resource - :identifier: service_edit :delete: - :name: delete :identifier: service_delete diff --git a/spec/requests/api/custom_actions_spec.rb b/spec/requests/api/custom_actions_spec.rb index 1d9f1d9ac17..e42d39a599c 100644 --- a/spec/requests/api/custom_actions_spec.rb +++ b/spec/requests/api/custom_actions_spec.rb @@ -75,7 +75,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit add_resource)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit)) end end @@ -91,7 +91,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3 add_resource)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3)) end it "supports the custom_actions attribute" do diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 8cdb2af4f64..003373d7b23 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -673,41 +673,39 @@ def expect_svc_with_vms let(:vm2) { FactoryGirl.create(:vm_vmware) } let(:vm3) { FactoryGirl.create(:vm_vmware) } - it 'can add multiple vms by href with an appropriate role' do - api_basic_authorize(action_identifier(:services, :add_resource)) + it 'can add multiple vms to multiple services by href with an appropriate role' do + api_basic_authorize(collection_action_identifier(:services, :add_resource)) request = { 'action' => 'add_resource', 'resources' => [ - { 'href' => vms_url(vm1.id) }, - { 'href' => vms_url(vm2.id) } + { 'href' => services_url(svc.id), 'resources' => [{'href' => vms_url(vm1.id)}, {'href' => vms_url(vm2.id)}] }, + { 'href' => services_url(svc1.id), 'resources' => [{'href' => vms_url(vm3.id)}] } ] } - run_post(services_url(svc.id), request) + run_post(services_url, request) expected = { - 'success' => true, - 'message' => "Assigned resources to Service id:#{svc.id} name:'#{svc.name}'" + 'results' => [ + { 'success' => true, 'message' => "Assigned resource vms id:#{vm1.id} to Service id:#{svc.id} name:'#{svc.name}'"}, + { 'success' => true, 'message' => "Assigned resource vms id:#{vm2.id} to Service id:#{svc.id} name:'#{svc.name}'"}, + { 'success' => true, 'message' => "Assigned resource vms id:#{vm3.id} to Service id:#{svc1.id} name:'#{svc1.name}'"} + ] } + expect(response).to have_http_status(:ok) expect(response.parsed_body).to eq(expected) expect(svc.reload.vms).to eq([vm1, vm2]) + expect(svc1.reload.vms).to eq([vm3]) end - it 'cannot add multiple vms by href without an appropriate role' do - api_basic_authorize - - run_post(services_url(svc.id), 'action' => 'add_resource') - - expect(response).to have_http_status(:forbidden) - end - - it 'can add multiple vms to multiple services by href with an appropriate role' do - api_basic_authorize(action_identifier(:services, :add_resource)) + it 'returns individual success and failures' do + user = FactoryGirl.create(:user) + api_basic_authorize(collection_action_identifier(:services, :add_resource)) request = { 'action' => 'add_resource', 'resources' => [ - { 'href' => services_url(svc.id), 'resources' => [{'href' => vms_url(vm1.id)}, {'href' => vms_url(vm2.id)}] }, + { 'href' => services_url(svc.id), 'resources' => [{'href' => vms_url(vm1.id)}, {'href' => users_url(user.id)}] }, { 'href' => services_url(svc1.id), 'resources' => [{'href' => vms_url(vm3.id)}] } ] } @@ -716,13 +714,15 @@ def expect_svc_with_vms expected = { 'results' => [ - { 'success' => true, 'message' => "Assigned resources to Service id:#{svc.id} name:'#{svc.name}'"}, - { 'success' => true, 'message' => "Assigned resources to Service id:#{svc1.id} name:'#{svc1.name}'"} + { 'success' => true, 'message' => "Assigned resource vms id:#{vm1.id} to Service id:#{svc.id} name:'#{svc.name}'"}, + { 'success' => false, 'message' => "Cannot assign users to Service id:#{svc.id} name:'#{svc.name}'"}, + { 'success' => true, 'message' => "Assigned resource vms id:#{vm3.id} to Service id:#{svc1.id} name:'#{svc1.name}'"} ] } + expect(response).to have_http_status(:ok) expect(response.parsed_body).to eq(expected) - expect(svc.reload.vms).to eq([vm1, vm2]) + expect(svc.reload.vms).to eq([vm1]) expect(svc1.reload.vms).to eq([vm3]) end @@ -733,25 +733,5 @@ def expect_svc_with_vms expect(response).to have_http_status(:forbidden) end - - it 'raises an error if the service cannot be added' do - user = FactoryGirl.create(:user) - api_basic_authorize(action_identifier(:services, :add_resource)) - request = { - 'action' => 'add_resource', - 'resources' => [ - { 'href' => users_url(user.id) } - ] - } - - run_post(services_url(svc.id), request) - - expected = { - 'success' => false, - 'message' => "Cannot assign users to Service id:#{svc.id} name:'#{svc.name}'" - } - expect(response).to have_http_status(:ok) - expect(response.parsed_body).to eq(expected) - end end end From 573c8c112ab9b0a7b89d53af28c31fcee214269e Mon Sep 17 00:00:00 2001 From: Jillian Tullo Date: Fri, 24 Mar 2017 17:20:05 -0400 Subject: [PATCH 4/4] add to resource more specs --- app/controllers/api/services_controller.rb | 24 ++++--- config/api.yml | 2 + spec/requests/api/custom_actions_spec.rb | 4 +- spec/requests/api/services_spec.rb | 84 ++++++++++++++++++---- 4 files changed, 88 insertions(+), 26 deletions(-) diff --git a/app/controllers/api/services_controller.rb b/app/controllers/api/services_controller.rb index 3f3a368ac24..3b25a1f9923 100644 --- a/app/controllers/api/services_controller.rb +++ b/app/controllers/api/services_controller.rb @@ -19,18 +19,20 @@ def edit_resource(type, id, data) end def add_resource_resource(type, id, data) + raise "Must specify a service href or id to add_resource to" unless id svc = resource_search(id, type, collection_class(type)) - data['resources'].collect do |resource_ref| - begin - resource_type, resource_id = parse_href(resource_ref['href']) - resource = resource_search(resource_id, resource_type, collection_class(resource_type)) - raise "Cannot assign #{resource_type} to #{service_ident(svc)}" unless resource.respond_to? :add_to_service - resource.add_to_service(svc) - action_result(true, "Assigned resource #{resource_type} id:#{resource_id} to #{service_ident(svc)}") - rescue => err - action_result(false, err.to_s) - end - end + + resource_href = data.fetch_path("resource", "href") + raise "Must specify a resource reference" unless resource_href + + resource_type, resource_id = parse_href(resource_href) + raise "Invalid resource href specified #{resource_href}" unless resource_type && resource_id + + resource = resource_search(resource_id, resource_type, collection_class(resource_type)) + raise "Cannot assign #{resource_type} to #{service_ident(svc)}" unless resource.respond_to? :add_to_service + + resource.add_to_service(svc) + action_result(true, "Assigned resource #{resource_type} id:#{resource_id} to #{service_ident(svc)}") rescue => err action_result(false, err.to_s) end diff --git a/config/api.yml b/config/api.yml index 4e07b8e507e..8c65ed798b3 100644 --- a/config/api.yml +++ b/config/api.yml @@ -1936,6 +1936,8 @@ :identifier: service_admin - :name: delete :identifier: service_delete + - :name: add_resource + :identifier: service_edit :delete: - :name: delete :identifier: service_delete diff --git a/spec/requests/api/custom_actions_spec.rb b/spec/requests/api/custom_actions_spec.rb index e42d39a599c..1d9f1d9ac17 100644 --- a/spec/requests/api/custom_actions_spec.rb +++ b/spec/requests/api/custom_actions_spec.rb @@ -75,7 +75,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit add_resource)) end end @@ -91,7 +91,7 @@ def expect_result_to_have_custom_actions_hash run_get services_url(svc1.id) expect_result_to_have_keys(%w(id href actions)) - expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3)) + expect(response.parsed_body["actions"].collect { |a| a["name"] }).to match_array(%w(edit button1 button2 button3 add_resource)) end it "supports the custom_actions attribute" do diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 003373d7b23..1aae8105600 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -671,15 +671,14 @@ def expect_svc_with_vms describe 'add_resource' do let(:vm1) { FactoryGirl.create(:vm_vmware) } let(:vm2) { FactoryGirl.create(:vm_vmware) } - let(:vm3) { FactoryGirl.create(:vm_vmware) } - it 'can add multiple vms to multiple services by href with an appropriate role' do + it 'can add vm to services by href with an appropriate role' do api_basic_authorize(collection_action_identifier(:services, :add_resource)) request = { 'action' => 'add_resource', 'resources' => [ - { 'href' => services_url(svc.id), 'resources' => [{'href' => vms_url(vm1.id)}, {'href' => vms_url(vm2.id)}] }, - { 'href' => services_url(svc1.id), 'resources' => [{'href' => vms_url(vm3.id)}] } + { 'href' => services_url(svc.id), 'resource' => {'href' => vms_url(vm1.id)} }, + { 'href' => services_url(svc1.id), 'resource' => {'href' => vms_url(vm2.id)} } ] } @@ -688,15 +687,14 @@ def expect_svc_with_vms expected = { 'results' => [ { 'success' => true, 'message' => "Assigned resource vms id:#{vm1.id} to Service id:#{svc.id} name:'#{svc.name}'"}, - { 'success' => true, 'message' => "Assigned resource vms id:#{vm2.id} to Service id:#{svc.id} name:'#{svc.name}'"}, - { 'success' => true, 'message' => "Assigned resource vms id:#{vm3.id} to Service id:#{svc1.id} name:'#{svc1.name}'"} + { 'success' => true, 'message' => "Assigned resource vms id:#{vm2.id} to Service id:#{svc1.id} name:'#{svc1.name}'"} ] } expect(response).to have_http_status(:ok) expect(response.parsed_body).to eq(expected) - expect(svc.reload.vms).to eq([vm1, vm2]) - expect(svc1.reload.vms).to eq([vm3]) + expect(svc.reload.vms).to eq([vm1]) + expect(svc1.reload.vms).to eq([vm2]) end it 'returns individual success and failures' do @@ -705,8 +703,8 @@ def expect_svc_with_vms request = { 'action' => 'add_resource', 'resources' => [ - { 'href' => services_url(svc.id), 'resources' => [{'href' => vms_url(vm1.id)}, {'href' => users_url(user.id)}] }, - { 'href' => services_url(svc1.id), 'resources' => [{'href' => vms_url(vm3.id)}] } + { 'href' => services_url(svc.id), 'resource' => {'href' => vms_url(vm1.id)} }, + { 'href' => services_url(svc1.id), 'resource' => {'href' => users_url(user.id)} } ] } @@ -715,15 +713,75 @@ def expect_svc_with_vms expected = { 'results' => [ { 'success' => true, 'message' => "Assigned resource vms id:#{vm1.id} to Service id:#{svc.id} name:'#{svc.name}'"}, - { 'success' => false, 'message' => "Cannot assign users to Service id:#{svc.id} name:'#{svc.name}'"}, - { 'success' => true, 'message' => "Assigned resource vms id:#{vm3.id} to Service id:#{svc1.id} name:'#{svc1.name}'"} + { 'success' => false, 'message' => "Cannot assign users to Service id:#{svc1.id} name:'#{svc1.name}'"} ] } expect(response).to have_http_status(:ok) expect(response.parsed_body).to eq(expected) expect(svc.reload.vms).to eq([vm1]) - expect(svc1.reload.vms).to eq([vm3]) + end + + it 'requires a valid resource' do + api_basic_authorize(collection_action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resource' => { 'resource' => { 'href' => '1' } } + } + + run_post(services_url(svc.id), request) + + expected = { 'success' => false, 'message' => "Invalid resource href specified 1"} + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + end + + it 'requires the resource to respond to add_to_service' do + user = FactoryGirl.create(:user) + api_basic_authorize(collection_action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resource' => { 'resource' => { 'href' => users_url(user.id) } } + } + + run_post(services_url(svc.id), request) + + expected = { 'success' => false, 'message' => "Cannot assign users to Service id:#{svc.id} name:'#{svc.name}'"} + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + end + + it 'requires a resource reference' do + api_basic_authorize(collection_action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resource' => { 'resource' => {} } + } + + run_post(services_url(svc.id), request) + + expected = { 'success' => false, 'message' => "Must specify a resource reference"} + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + end + + it 'can add a vm to a resource with appropriate role' do + api_basic_authorize(collection_action_identifier(:services, :add_resource)) + request = { + 'action' => 'add_resource', + 'resource' => { 'resource' => {'href' => vms_url(vm1.id)} } + } + + run_post(services_url(svc.id), request) + + expected = { 'success' => true, 'message' => "Assigned resource vms id:#{vm1.id} to Service id:#{svc.id} name:'#{svc.name}'"} + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(expected) + expect(svc.reload.vms).to eq([vm1]) end it 'cannot add multiple vms to multiple services by href without an appropriate role' do