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

Add custom action support for models already exposed in api that need it #163

Merged
merged 2 commits into from
Nov 20, 2017

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Oct 31, 2017

Adds model custom action for models that support custom buttons.

In wip because it requires UI components to be added to all these as well

Related to
Host:
eclarizio/manageiq@8ca5858
eclarizio/manageiq-ui-classic@f7d05b4
eclarizio@e637616

Everything else:
ManageIQ/manageiq#16369

@d-m-u d-m-u changed the title Add custom action support for models already exposed in api that need it [WIP] Add custom action support for models already exposed in api that need it Oct 31, 2017
@miq-bot miq-bot added the wip label Oct 31, 2017
@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch 2 times, most recently from 5b3afc9 to 47587e3 Compare November 1, 2017 15:55
@d-m-u d-m-u changed the title [WIP] Add custom action support for models already exposed in api that need it Add custom action support for models already exposed in api that need it Nov 1, 2017
@miq-bot miq-bot removed the wip label Nov 1, 2017
@abellotti
Copy link
Member

@d-m-u needs specs for those, maybe enhance custom_actions_spec.rb to handle all these updated collections in a common way for custom actions ?

@d-m-u d-m-u closed this Nov 2, 2017
@d-m-u d-m-u reopened this Nov 2, 2017
@d-m-u d-m-u changed the title Add custom action support for models already exposed in api that need it [WIP] Add custom action support for models already exposed in api that need it Nov 2, 2017
@miq-bot miq-bot added the wip label Nov 2, 2017
@abellotti
Copy link
Member

@d-m-u see the tests I wrote here for availability_zones custom actions:
abellotti@ebf4e58

You can use that file and extend it similarly for all other collections we're exposing the custom actions for in this PR.

Thanks,

/cc'ing @jntullo & @gtanzillo

@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch 3 times, most recently from 00a99de to e0e089e Compare November 7, 2017 20:35
@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch 4 times, most recently from 9aa769a to 1e50c80 Compare November 8, 2017 19:51
@d-m-u d-m-u changed the title [WIP] Add custom action support for models already exposed in api that need it Add custom action support for models already exposed in api that need it Nov 8, 2017
@d-m-u
Copy link
Contributor Author

d-m-u commented Nov 8, 2017

@miq-bot assign @gmcculloug
@miq-bot remove_label wip

@d-m-u
Copy link
Contributor Author

d-m-u commented Nov 8, 2017

@miq-bot unassign @gmcculloug

@miq-bot
Copy link
Member

miq-bot commented Nov 8, 2017

@d-m-u unrecognized command 'unassign', ignoring...

Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@gmcculloug
Copy link
Member

@gtanzillo @abellotti Who should this be assigned to? (Hint: not me 🙏 )

@eclarizio
Copy link
Member

Aside from @abellotti's performance concern, the custom actions being supported on all the new objects looks good to me.

@abellotti
Copy link
Member

Thanks @eclarizio for testing them out 🙇

@abellotti
Copy link
Member

abellotti commented Nov 15, 2017

@d-m-u I need to bother you to create a second PR 🙇

As per ManageIQ/manageiq#16467 not all of the collections targeted here are meant for the G-release, so the separate PR needs to be created for the collections meant to have custom_actions for the G-release.

these include:

  • cloud_tenants
  • cloud_volumes
  • clusters
  • container_nodes
  • data_stores
  • hosts
  • providers
  • templates

Thank you.

/cc @gmcculloug

@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch 2 times, most recently from 215292f to 7cbdf7d Compare November 16, 2017 15:12
@d-m-u
Copy link
Contributor Author

d-m-u commented Nov 16, 2017

@jntullo or @abellotti can you give this and the counterpart #213 a 👀 just to make sure I didn't done goof on something silly once more please?

@miq-bot
Copy link
Member

miq-bot commented Nov 17, 2017

This pull request is not mergeable. Please rebase and repush.

@abellotti
Copy link
Member

Can't seem to find the custom_actions tests for these 8 collections.

@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch from 61cb77a to 3daaf6a Compare November 20, 2017 12:49
@miq-bot
Copy link
Member

miq-bot commented Nov 20, 2017

This pull request is not mergeable. Please rebase and repush.

@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch 2 times, most recently from 49834b9 to 68b3b35 Compare November 20, 2017 14:49
@d-m-u d-m-u force-pushed the adding_dialog_custom_support_actions branch from 68b3b35 to 08f7b28 Compare November 20, 2017 17:02
@miq-bot
Copy link
Member

miq-bot commented Nov 20, 2017

Checked commits d-m-u/manageiq-api@a9aa00d~...08f7b28 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

@imtayadeway
Copy link
Contributor

@abellotti before hitting the button on this (custom button?), have we considered the particularly thorny performance issues surrounding custom actions currently, i.e. #146 ?

@abellotti
Copy link
Member

LGTM!! Thanks @d-m-u for updating this PR. 👍

@abellotti abellotti added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 20, 2017
@abellotti abellotti merged commit 55187fa into ManageIQ:master Nov 20, 2017
@d-m-u d-m-u deleted the adding_dialog_custom_support_actions branch November 20, 2017 17:29
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.

8 participants