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

Expose Custom Button visability/enablement #15911

Merged

Conversation

imtayadeway
Copy link
Contributor

@imtayadeway imtayadeway commented Aug 30, 2017

Resolves https://www.pivotaltracker.com/story/show/149699953

Does a couple of things:

  1. When asking for a Service's custom_actions or custom_action_buttons, will only return things whose visibility_expression evaluates to true for the object at hand
  2. When asking for a Service's custom_actions, serializes the result of the enablement_expression evaluated in the context of the object at hand, along with the rest of the button's attributes

From what I understand, buttons have an attribute disabled_text which I am assuming is there to explain the reason for its being disabled - if correct then this is already serialized and returned in the response, so nothing further needs to be done here.

/cc @AllenBW
@miq-bot add-label enhancement
@miq-bot assign @gtanzillo

@imtayadeway imtayadeway force-pushed the custom-button-visability-enablement branch 2 times, most recently from 0b7d159 to 98216b9 Compare August 30, 2017 22:39
@imtayadeway imtayadeway force-pushed the custom-button-visability-enablement branch from 98216b9 to 906d915 Compare August 30, 2017 22:46
@miq-bot
Copy link
Member

miq-bot commented Aug 30, 2017

Checked commits imtayadeway/manageiq@e40f7d4~...906d915 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@AllenBW
Copy link
Member

AllenBW commented Aug 31, 2017

ALL HAIL 💃 🕺

@lpichler
Copy link
Contributor

From what I understand, buttons have an attribute disabled_text which I am assuming is there to explain the reason for its being disabled.

yes you are right 👍

@imtayadeway
Copy link
Contributor Author

@lpichler it was my understanding that https://github.com/ManageIQ/manageiq/blob/master/app/models/custom_button_set.rb#L29-L62 was made in support of this PR, but as you can see it's not being utilized here, and AFAICT anywhere else yet. Did I miss something?

@lpichler
Copy link
Contributor

@imtayadeway

was made in support of this PR, ...

yes, probably you can use it (if it would be helpful) but now, it used only in controller repo
https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/helpers/application_helper/toolbar_builder.rb#L343

@imtayadeway
Copy link
Contributor Author

@lpichler thanks! 🙇

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

Looks really good 👍

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.

5 participants