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

GOD's button group can have buttons assign/unassign during create/edit #4913

Merged
merged 22 commits into from
Aug 2, 2019

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Nov 14, 2018

Before:
screen shot 2018-11-14 at 1 59 57 pm
After:
screen shot 2019-01-09 at 4 12 37 pm

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1534529

@miq-bot add_label bug, wip

@miq-bot miq-bot changed the title GOD's button group can have buttons assign/unassign during create/edit [WIP] GOD's button group can have buttons assign/unassign during create/edit Nov 14, 2018
@ZitaNemeckova ZitaNemeckova force-pushed the god_buttons branch 3 times, most recently from 1bf1ed5 to 7d58993 Compare January 9, 2019 15:18
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label generic objects, automation/automate, hammer/yes, gaprindashvili/yes

@ZitaNemeckova
Copy link
Contributor Author

TODO: - fix includes or use includes from lodash

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label pending core

Depends on ManageIQ/manageiq#18368

@himdel
Copy link
Contributor

himdel commented Jan 23, 2019

Depends on ManageIQ/manageiq#18368

Probably shouldn't, given gaprindashvili/yes.
If that core PR ever gets reviewed/merged, we should use it, but probably shouldn't block this one.

@ZitaNemeckova
Copy link
Contributor Author

@himdel I'll remove related commit to that dependency and remove the label. Thanks 👍

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip, pending core

@miq-bot miq-bot changed the title [WIP] GOD's button group can have buttons assign/unassign during create/edit GOD's button group can have buttons assign/unassign during create/edit Jan 24, 2019
@miq-bot miq-bot removed the wip label Jan 24, 2019
@@ -148,6 +149,28 @@ def add_button_in_group
custom_button_set.save!
end

def add_custom_buttons_in_set
custom_button_set = CustomButtonSet.find(params[:custom_button_set_id])
Copy link
Contributor

Choose a reason for hiding this comment

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

find_record_with_rbac

@@ -94,7 +117,17 @@ function mainCustomButtonGroupFormController(API, miqService) {
vm.saveWithAPI = function(method, url, saveObject, saveMsg) {
miqService.sparkleOn();
API[method](url, saveObject)
.then(miqService.redirectBack.bind(vm, saveMsg, 'success', vm.redirectUrl))
// TODO remove once API supports adding/removing buttons from button set
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is done in ManageIQ/manageiq#18368

@@ -148,6 +149,28 @@ def add_button_in_group
custom_button_set.save!
end

def add_custom_buttons_in_set
Copy link
Contributor

Choose a reason for hiding this comment

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

(probably not needed - the API now does it too)


def custom_buttons_in_set
assigned_buttons = if params[:custom_button_set_id].present?
button_set = CustomButtonSet.find(params[:custom_button_set_id])
Copy link
Contributor

Choose a reason for hiding this comment

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

find_record_with_rbac

else
[]
end
generic_object_definition = GenericObjectDefinition.find(params[:generic_object_definition_id])
Copy link
Contributor

Choose a reason for hiding this comment

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

rbac

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@romanblanco please review, thanks :)

@ZitaNemeckova
Copy link
Contributor Author

ZitaNemeckova commented Jul 31, 2019

@miq-bot add_label ivanchuk/yes

@miq-bot miq-bot changed the title [WIP] GOD's button group can have buttons assign/unassign during create/edit GOD's button group can have buttons assign/unassign during create/edit Jul 31, 2019
@miq-bot miq-bot added ivanchuk/yes and removed wip labels Jul 31, 2019
@romanblanco
Copy link
Member

@miq-bot add_reviewer @romanblanco

@miq-bot miq-bot requested a review from romanblanco July 31, 2019 17:33
Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

Tested, works 👍

I only have a comment about adding move inside the ManageIQ scope or make it as a library

app/javascript/packs/globals.js Outdated Show resolved Hide resolved
@@ -0,0 +1,61 @@
/* globals move */
Copy link
Contributor

Choose a reason for hiding this comment

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

no longer needed

@himdel himdel self-assigned this Aug 1, 2019
@himdel
Copy link
Contributor

himdel commented Aug 1, 2019

LGTM, merging when green :)

Note: if/when backporting to gaprindashvili or hammer, this will either need an additional commit to update not only button_order but also CustomButtonSet#children, or be backported with ManageIQ/manageiq#18368

@miq-bot
Copy link
Member

miq-bot commented Aug 1, 2019

Checked commits ZitaNemeckova/manageiq-ui-classic@831bda5~...03d6c5f with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@himdel himdel added this to the Sprint 117 Ending Aug 5, 2019 milestone Aug 2, 2019
@himdel himdel merged commit 1fc6e61 into ManageIQ:master Aug 2, 2019
@himdel
Copy link
Contributor

himdel commented Aug 2, 2019

One note - I'm not sure we're ever showing the custom buttons now for generic objects.

I was not able to see any custom button toolbars in GOD > Instances under definition, or in GO > instance detail, or in GO > show_list.

(Neither for buttons added here, or for buttons added in ae customization)

But the button group form seems to be working fine now :)

simaishi pushed a commit that referenced this pull request Aug 2, 2019
GOD's button group can have buttons assign/unassign during create/edit

(cherry picked from commit 1fc6e61)

https://bugzilla.redhat.com/show_bug.cgi?id=1534529
@simaishi
Copy link
Contributor

simaishi commented Aug 2, 2019

Ivanchuk backport details:

$ git log -1
commit 9ed0f565ff438a2c9b2a549a680fb28bc0cf8a55
Author: Martin Hradil <[email protected]>
Date:   Fri Aug 2 13:04:13 2019 +0200

    Merge pull request #4913 from ZitaNemeckova/god_buttons
    
    GOD's button group can have buttons assign/unassign during create/edit
    
    (cherry picked from commit 1fc6e6197f273a4719af75817f0e0a6be8f627f1)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1534529

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.

6 participants