-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass multiple objects to automate by object class #18088
Conversation
@tinaafitz @bdunne can ya'll review please? |
app/models/resource_action.rb
Outdated
@@ -20,8 +20,7 @@ def automate_queue_hash(target, override_attrs, user, open_url_task_id = nil) | |||
elsif target.kind_of?(Hash) | |||
override_values = target | |||
elsif target.kind_of?(Array) || target.kind_of?(ActiveRecord::Relation) | |||
klass = target.first.id.class | |||
object_ids = target.collect { |t| "#{klass}::#{t.id}" }.join(",") | |||
object_ids = target.collect { |t| "#{t.class.to_s.demodulize}::#{t.id}" }.join(",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer t.class.name.demodulize
6b9fe7e
to
999e01b
Compare
Checked commit d-m-u@999e01b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@tinaafitz can you please review? |
@miq-bot assign @gmcculloug |
@@ -20,8 +20,7 @@ def automate_queue_hash(target, override_attrs, user, open_url_task_id = nil) | |||
elsif target.kind_of?(Hash) | |||
override_values = target | |||
elsif target.kind_of?(Array) || target.kind_of?(ActiveRecord::Relation) | |||
klass = target.first.id.class | |||
object_ids = target.collect { |t| "#{klass}::#{t.id}" }.join(",") | |||
object_ids = target.collect { |t| "#{t.class.name.demodulize}::#{t.id}" }.join(",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be using Automate engine methods here. See https://github.com/ManageIQ/manageiq-automation_engine/blob/master/lib/miq_automation_engine/engine/miq_ae_engine.rb#L162-L190
@@ -62,8 +62,7 @@ | |||
it "validates queue entry" do | |||
targets = [FactoryGirl.create(:vm_vmware), FactoryGirl.create(:vm_vmware)] | |||
ae_attributes[:target_object_type] = targets.first.class.base_class.name | |||
klass = targets.first.id.class | |||
ae_attributes['Array::target_object_ids'] = targets.collect { |t| "#{klass}::#{t.id}" }.join(",") | |||
ae_attributes['Array::target_object_ids'] = targets.collect { |t| "#{t.class.name.demodulize}::#{t.id}" }.join(",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@d-m-u
Is there a way to test and see if its broken by doing a multi select and invoke a custom button on the, does Automate complain about invalid objects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bug open about this very thing so I think that kinda proves it's currently broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would mean that this feature has not been QE'ed if its that broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since QE is the one who opened this bug, that sounds about right.
This isn't the bug I was looking for :) |
I think we should be passing the array of multiple objects from a custom button run on many vms or hosts or whatevers as the class name rather than running class on an id and getting an Integer which will fail on https://github.com/ManageIQ/manageiq-automation_engine/blob/1975fe20dbcd622c4e8b0363e793d2a477c4c430/lib/miq_automation_engine/engine/miq_ae_engine/miq_ae_object.rb#L562.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1628224