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

Change toolbar delete buttons for Button and Button Group to use generic code #6230

Merged
merged 5 commits into from
Oct 23, 2019

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Sep 24, 2019

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

Go to Automation -> Automate -> Generic Objects - > delete a custom button(with or without Custom Button Group it shouldn't matter)

Actual result:
There's an error popup and message for a sec. See Before pic.

Expected result:
No error message/popup. See After pic.

According to the BZ it happens in 70%.

This fix should work for all possible delete actions on Generic Object Definition page.

Before:
Screenshot 2019-09-24 at 14 58 14
After:
image

@miq-bot add_label bug, generic objects, ivanchuk/yes, changelog/no, wip

Depends on #6289

@miq-bot miq-bot changed the title Use correct id for delete action [WIP] Use correct id for delete action Sep 24, 2019
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@romanblanco please have a look, thanks :)

@miq-bot miq-bot changed the title [WIP] Use correct id for delete action Use correct id for delete action Sep 24, 2019
@miq-bot miq-bot removed the wip label Sep 24, 2019
@himdel
Copy link
Contributor

himdel commented Sep 24, 2019

app/assets/javascripts/components/generic_object/generic-object-definition-toolbar.js
21:      if (event.entity && (toolbar.recordId || ManageIQ.gridChecks.length > 0)) {
24:        if (toolbar.recordId) {
25:          toolbar.genericObjectDefinitions = _.union(toolbar.genericObjectDefinitions, [toolbar.recordId]);
40:    var currentRecordId = toolbar.recordId || ManageIQ.record.recordId;

It's unlikely that only one toolbar.recordId is wrong ;).

They should probably all go away, including the recordId entry in bindings?

@miq-bot
Copy link
Member

miq-bot commented Sep 25, 2019

@ZitaNemeckova unrecognized command 'add_albel', ignoring...

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

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label wip

@miq-bot miq-bot changed the title Use correct id for delete action [WIP] Use correct id for delete action Sep 25, 2019
@miq-bot miq-bot added the wip label Sep 25, 2019
@ZitaNemeckova ZitaNemeckova force-pushed the fix_GOD_again branch 3 times, most recently from cba786e to 238453d Compare October 7, 2019 10:27
@ZitaNemeckova ZitaNemeckova force-pushed the fix_GOD_again branch 4 times, most recently from 44e4c88 to f414d49 Compare October 7, 2019 14:00
@ZitaNemeckova ZitaNemeckova changed the title [WIP] Use correct id for delete action [WIP] Change toolbar delete buttons for Button and Button Group to use generic code Oct 7, 2019
@ZitaNemeckova ZitaNemeckova force-pushed the fix_GOD_again branch 2 times, most recently from 8505dea to 3e1e654 Compare October 7, 2019 14:14
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@romanblanco please have a look, thanks :)

@miq-bot miq-bot changed the title [WIP] Change toolbar delete buttons for Button and Button Group to use generic code Change toolbar delete buttons for Button and Button Group to use generic code Oct 7, 2019
@miq-bot miq-bot removed the wip label Oct 7, 2019
@ZitaNemeckova ZitaNemeckova force-pushed the fix_GOD_again branch 2 times, most recently from 65564e6 to 8bce4d5 Compare October 23, 2019 10:26
@ZitaNemeckova
Copy link
Contributor Author

@himdel It's ready for re-review :) Thanks :)

@ZitaNemeckova ZitaNemeckova requested a review from himdel October 23, 2019 11:38
@himdel
Copy link
Contributor

himdel commented Oct 23, 2019

Thanks, I like the toolbar much more now :)

One more bit needed:

app/assets/javascripts/components/generic_object/generic-object-definition-toolbar.js
49:    } else if (toolbar.action === 'delete_custom_button_set' && currentRecordId) {
51:    } else if (toolbar.action === 'delete_custom_button' && currentRecordId) {

those are dead code now ;)

@ZitaNemeckova
Copy link
Contributor Author

Removed :)

@miq-bot
Copy link
Member

miq-bot commented Oct 23, 2019

Checked commits ZitaNemeckova/manageiq-ui-classic@8f94aa7~...3073ae1 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
6 files checked, 0 offenses detected
Everything looks fine. 🍪

@himdel himdel self-assigned this Oct 23, 2019
@himdel himdel added this to the Sprint 123 Ending Oct 28, 2019 milestone Oct 23, 2019
@himdel himdel merged commit 6c5707c into ManageIQ:master Oct 23, 2019
@ZitaNemeckova ZitaNemeckova deleted the fix_GOD_again branch October 24, 2019 08:06
@simaishi
Copy link
Contributor

simaishi commented Nov 4, 2019

@ZitaNemeckova Trying to backport this to ivanchuk branch and have 2 questions..

@ZitaNemeckova
Copy link
Contributor Author

@simaishi Thanks for catching this one.

Should I tag this one as ivanchuk/conflict and 6214 as ivanchuk/no?

@simaishi
Copy link
Contributor

simaishi commented Nov 5, 2019

@ZitaNemeckova yes, that will work. Thanks!

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label ivanchuk/yes
@miq-bot add_label ivanchuk/conflict

@simaishi
Copy link
Contributor

Backported to Ivanchuk via #6380

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