storage service edit

[OrGur1987]

belongs to issue: ManageIQ/manageiq-ui-classic#8728

depends upon:

• storage service edit manageiq#22425

adds editing/update option to storage service controller and app yaml

[OrGur1987]

@bdunne hi, this is ready for review
thanks

[kbrock]

this looks great. just that one change

[kbrock]

Also, is there a way to add a spec here?

[OrGur1987]

Also, is there a way to add a spec here?

should I use spec/models/cloud_volume_spec.rb as an example?
the specs there test queuing methods

[OrGur1987]

Also, is there a way to add a spec here?

should I use spec/models/cloud_volume_spec.rb as an example? the specs there test queuing methods

@kbrock

[kbrock]

@OrGur1987 good point. you can't test that the records have been modified because this is just a request for the provider.

Yes, base your tests upon those.

[OrGur1987]

@OrGur1987 good point. you can't test that the records have been modified because this is just a request for the provider.

Yes, base your tests upon those.

added an edit spec to spec/requests/storage_services_spec.rb.

[kbrock]

Suggested change

	storage_service = FactoryBot.create("ManageIQ::Providers::Autosde::StorageManager::StorageService", :name => 'test_service1', :ext_management_system => provider)
	storage_service = FactoryBot.create(:autosde_storage_manager, :ext_management_system => provider)

also noticed that others use :storage_service, not sure if that works for you.

You could probably put the provider and storage_service into a let() {} at the top of this file to reduce some of this duplication... but that is up to you.

[OrGur1987]

extracted provider to let().
extracting storage_service didn't quite work out with the delete and get specs..

in the edit spec, for some reason when I use FactoryBot.create(:storage_service, ... ) the service does not support update, but it does when I use FactoryBot.create("ManageIQ::Providers::Autosde::StorageManager::StorageService",...).

do u have any idea why? maybe it references the core repo storage_service model and not the provider's one?
it supposedly doesn't support create and delete as well, but those spec do pass..

    83:   context 'Storage Services edit action' do
    84:     it "PUT /api/storage_services/:id'" do
    85:       storage_service1 = FactoryBot.create("ManageIQ::Providers::Autosde::StorageManager::StorageService", :name => 'test_service1', :ext_management_system => provider)
    86:       storage_service2 = FactoryBot.create(:storage_service, :name => 'test_service1', :ext_management_system => provider)
    87: 
    88:       require 'byebug'
    89:       byebug
    90: 
=>  91:       api_basic_authorize('storage_service_edit')
    92:       put(api_storage_service_url(nil, storage_service))
    93: 
    94: 
    95:       expect(response.parsed_body["message"]).to include("Updating")
    96:       expect(response).to have_http_status(:ok)
    97:     end
    98:   end
    99: 
   100: end
(byebug) storage_service1.supports_update?
true
(byebug) storage_service2.supports_update?
false
(byebug) storage_service2.supports_create?
false
(byebug) storage_service2.supports_delete?
false
(byebug) storage_service1.supports_delete?
true

[kbrock]

sorry.

We tend to use stub_supports to ensure that you get the expected supports?.
In the far far future, we would like to change all these tests to use a generic providers with a bunch of expectations on them.

In your example, I know you specified the correct :provider for service2, but the class of storage_service is not right. You would need to use a factory like :autosde_storage_manager.

But stub_supports on the storage_service2 would probably get your needs met if that factory does not work for you.

[kbrock]

@OrGur1987 This looks great. Just one or two tweaks and we can get it in right away.

Thanks for doing this

[OrGur1987]

@OrGur1987 This looks great. Just one or two tweaks and we can get it in right away.

Thanks for doing this

@kbrock is spec ok like that?

[OrGur1987]

@kbrock
kind reminder :)

[kbrock]

@OrGur1987 thanks for the ping. I'll leave this tab open and hope it does not slip through the cracks again.

Could you see my notes around the class of the storage_service2?
Also, don't set any attributes (i.e.: name) in the provider if it will work without it set.

Not sure why the factory is a sticking point for me

[miq-bot]

Checked commit Autosde@004d17c with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 🍪

[OrGur1987]

@OrGur1987 thanks for the ping. I'll leave this tab open and hope it does not slip through the cracks again.

Could you see my notes around the class of the storage_service2? Also, don't set any attributes (i.e.: name) in the provider if it will work without it set.

Not sure why the factory is a sticking point for me

@kbrock
stub_supports fixed it thnx :)
removed the name from the provider