Skip to content
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

Support array of objects for custom button support #14930

Merged
merged 4 commits into from
Jun 16, 2017

Conversation

mkanoor
Copy link
Contributor

@mkanoor mkanoor commented Apr 27, 2017

https://www.pivotaltracker.com/n/projects/1613907/stories/140944305

These are changes in the Automate side to handle being passed in an array of objects.
The objects are not loaded by the Automate Engine, they are loaded by the Automate Method based on the passed in target_object_type and target_object_ids.

Links

  • Sample Automate Method that gets passed in array of objects -
$evm.log(:info, "Object Type: #{$evm.root['target_object_type']}")
$evm.log(:info, "Object IDS: #{$evm.root['target_object_ids']}")
klass = $evm.vmdb($evm.root['target_object_type'])
$evm.root['target_object_ids'].each do |id|
  obj = klass.find(id)
  $evm.log(:info, "Object name is #{obj.name}")
end

https://www.pivotaltracker.com/n/projects/1613907/stories/140944305

This is changes in the Automate side to handle being passed
in an array of objects.
@tinaafitz
Copy link
Member

@mkanoor Looks good.

@mkanoor
Copy link
Contributor Author

mkanoor commented Apr 27, 2017

@gmcculloug
Since the caller could be passing in vms which could be in different zones, what should be the value of the zone when putting into the queue. (nil or default).
I have implemented it with the value being nil
https://github.com/ManageIQ/manageiq/blob/master/app/models/custom_button.rb#L76

klass = target.first.id.class
object_ids = target.collect { |t| "#{klass}::#{t.id}" }.join(",")
override_attrs = {:array_object_type => target.first.class.base_class.name,
'Array::object_ids' => object_ids}.merge(override_attrs || {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like override_attrs is a required hash field so this could be:

override_attrs.merge!(:array_object_type => target.first.class.base_class.name,
                     'Array::object_ids' => object_ids)

Also, what do you think about making the naming here a little more unique/consistent? target_object_type and target_object_ids

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmcculloug the override_attrs can also be nil so we have to initialize it before we do the merge

@gmcculloug gmcculloug requested a review from Fryguy April 28, 2017 14:55
@gmcculloug
Copy link
Member

@Fryguy Looking for your thoughts on this one. The idea is to avoid overloading the automation process when passed an array of objects to load by passing the class/IDs into the method and letting the automate method process them as needed.

The current usage is from custom buttons on a list-view so this is only handling a single object type.

@miq-bot
Copy link
Member

miq-bot commented Apr 28, 2017

Checked commits mkanoor/manageiq@1c94dc0~...9a60fbd with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@martinpovolny
Copy link
Member

ping @Fryguy?

Can you delegate someone if you don't have time for this one, please?

I'd like to use this from the UI. Thx!

@chessbyte
Copy link
Member

@Fryguy bump

@Fryguy Fryguy merged commit 5792a10 into ManageIQ:master Jun 16, 2017
@Fryguy Fryguy added this to the Sprint 63 Ending Jun 19, 2017 milestone Jun 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants