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

Fix missing name of deleted GOD Button/ButtonGroup #6214

Merged
merged 2 commits into from
Sep 30, 2019

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Sep 19, 2019

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

Go to Automation -> Automate -> Generic Objects -> delete a custom button (delete a custom button group to make sure it works as well)

Before:
image
or
Screenshot 2019-09-23 at 15 45 02

After:
Screenshot 2019-09-19 at 14 26 16

if you see 404 error during deleting it's related to this https://bugzilla.redhat.com/show_bug.cgi?id=1753388 and will be fixed in separated PR #6230 .

@miq-bot add_lable wip, bug, generic objects, ivanchuk/yes

@miq-bot
Copy link
Member

miq-bot commented Sep 19, 2019

@ZitaNemeckova unrecognized command 'add_lable', 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, bug, generic objects, ivanchuk/yes

@miq-bot miq-bot changed the title Fix missing name of deleted GOD Button/ButtonGroup [WIP] Fix missing name of deleted GOD Button/ButtonGroup Sep 19, 2019
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Fix missing name of deleted GOD Button/ButtonGroup Fix missing name of deleted GOD Button/ButtonGroup Sep 23, 2019
@miq-bot miq-bot removed the wip label Sep 23, 2019
@ZitaNemeckova
Copy link
Contributor Author

@romanblanco please review, thanks :)

@romanblanco
Copy link
Member

@miq-bot add_label hammer/yes

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.

@ZitaNemeckova I'm getting error while removing the created button:

[----] F, [2019-09-24T10:53:01.402862 #17080:2ad202befa9c] FATAL -- :
[----] F, [2019-09-24T10:53:01.402905 #17080:2ad202befa9c] FATAL -- : ActiveRecord::RecordNotFound (Couldn't find CustomButton with 'id'=1):
[----] F, [2019-09-24T10:53:01.402935 #17080:2ad202befa9c] FATAL -- :
[----] F, [2019-09-24T10:53:01.403012 #17080:2ad202befa9c] FATAL -- : activerecord (5.1.7) lib/active_record/core.rb:189:in `find'
activesupport (5.1.7) lib/active_support/core_ext/object/try.rb:17:in `public_send'
activesupport (5.1.7) lib/active_support/core_ext/object/try.rb:17:in `try!'
activesupport (5.1.7) lib/active_support/core_ext/object/try.rb:6:in `try'
/home/rblanco/devel/manageiq-ui-classic/app/controllers/mixins/breadcrumbs_mixin.rb:60:in `data_for_breadcrumbs'
actionpack (5.1.7) lib/abstract_controller/helpers.rb:68:in `data_for_breadcrumbs'
/home/rblanco/devel/manageiq-ui-classic/app/views/layouts/_breadcrumbs.html.haml:1:in `__home_rblanco_devel_manageiq_ui_classic_app_views_layouts__breadcrumbs_html_haml___125
509737557929557_69932076881800'
actionview (5.1.7) lib/action_view/template.rb:157:in `block in render'
activesupport (5.1.7) lib/active_support/notifications.rb:168:in `instrument'
actionview (5.1.7) lib/action_view/template.rb:352:in `instrument_render_template'
actionview (5.1.7) lib/action_view/template.rb:155:in `render'
actionview (5.1.7) lib/action_view/renderer/partial_renderer.rb:342:in `block in render_partial'
actionview (5.1.7) lib/action_view/renderer/abstract_renderer.rb:42:in `block in instrument'
activesupport (5.1.7) lib/active_support/notifications.rb:166:in `block in instrument'
activesupport (5.1.7) lib/active_support/notifications/instrumenter.rb:21:in `instrument'
activesupport (5.1.7) lib/active_support/notifications.rb:166:in `instrument'
actionview (5.1.7) lib/action_view/renderer/abstract_renderer.rb:41:in `instrument'
actionview (5.1.7) lib/action_view/renderer/partial_renderer.rb:331:in `render_partial'
actionview (5.1.7) lib/action_view/renderer/partial_renderer.rb:310:in `render'
actionview (5.1.7) lib/action_view/renderer/renderer.rb:47:in `render_partial'
actionview (5.1.7) lib/action_view/renderer/renderer.rb:21:in `render'
actionview (5.1.7) lib/action_view/helpers/rendering_helper.rb:31:in `render'
/home/rblanco/devel/manageiq-ui-classic/app/views/layouts/_center_div_no_listnav.html.haml:4:in `__home_rblanco_devel_manageiq_ui_classic_app_views_layouts__center_div_no_lis
tnav_html_haml___2456221316301853596_69932005648960'
actionview (5.1.7) lib/action_view/template.rb:157:in `block in render'
activesupport (5.1.7) lib/active_support/notifications.rb:168:in `instrument'
actionview (5.1.7) lib/action_view/template.rb:352:in `instrument_render_template'
actionview (5.1.7) lib/action_view/template.rb:155:in `render'
actionview (5.1.7) lib/action_view/renderer/partial_renderer.rb:342:in `block in render_partial'
actionview (5.1.7) lib/action_view/renderer/abstract_renderer.rb:42:in `block in instrument'

heading to

The page you were looking for doesn't exist.

tested also with an empty DB

@ZitaNemeckova
Copy link
Contributor Author

@romanblanco You are doing it at wrong screen :)

Go to Automation -> Automate -> Generic Objects -> delete a custom button (delete a custom button group to make sure it works as well)

@romanblanco
Copy link
Member

@ZitaNemeckova Interesting - I can't reproduce the error anymore, the action ends with a successfull flash message, but I'm still getting errors, see:

Screencast from 2019-09-24 13-44-08

@ZitaNemeckova
Copy link
Contributor Author

ZitaNemeckova commented Sep 24, 2019

@romanblanco Fixed in #6230 . It's kinda "separate" issue.

@miq-bot
Copy link
Member

miq-bot commented Sep 25, 2019

Checked commits ZitaNemeckova/manageiq-ui-classic@7be7595~...abbdddd with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@romanblanco
Copy link
Member

romanblanco commented Sep 25, 2019

@ZitaNemeckova still, even after applying

diff --git a/app/assets/javascripts/components/generic_object/generic-object-definition-toolbar.js b/app/assets/javascripts/components/generic_object/generic-object-definition-toolbar.js
index cd2bd648873..caf90ecaa54 100644
--- a/app/assets/javascripts/components/generic_object/generic-object-definition-toolbar.js
+++ b/app/assets/javascripts/components/generic_object/generic-object-definition-toolbar.js
@@ -37,7 +37,7 @@ function genericObjectDefinitionToolbarController(API, miqService, $window) {

   // private functions
   function postGenericObjectDefinitionAction() {
-    var currentRecordId = toolbar.recordId || ManageIQ.record.recordId;
+    var currentRecordId = ManageIQ.record.recordId;
     if (toolbar.action === 'delete' && !currentRecordId) {
       _.forEach(toolbar.genericObjectDefinitions, function(recordId) {
         API.get('/api/generic_object_definitions/' + recordId + '?attributes=generic_objects_count')

I'm seeing a network error while removing the button:
Screencast from 2019-09-25 11-24-15

also, removing Generic Object Class gives me error:
Screencast from 2019-09-25 11-12-28

Update:

Actually, I feel like these errors belong more to PR #6230, as the point of this PR is to correct the name in flash message.

I'm giving this PR +1 as it does what it should and will keep testing #6230

@mzazrivec mzazrivec self-assigned this Sep 30, 2019
@mzazrivec mzazrivec added this to the Sprint 121 Ending Sep 30, 2019 milestone Sep 30, 2019
@mzazrivec mzazrivec merged commit 0f01273 into ManageIQ:master Sep 30, 2019
@ZitaNemeckova ZitaNemeckova deleted the another_fix_GOD branch September 30, 2019 06:42
ZitaNemeckova added a commit to ZitaNemeckova/manageiq-ui-classic that referenced this pull request Oct 7, 2019
ZitaNemeckova added a commit to ZitaNemeckova/manageiq-ui-classic that referenced this pull request Oct 7, 2019
ZitaNemeckova added a commit to ZitaNemeckova/manageiq-ui-classic that referenced this pull request Oct 8, 2019
ZitaNemeckova added a commit to ZitaNemeckova/manageiq-ui-classic that referenced this pull request Oct 10, 2019
ZitaNemeckova added a commit to ZitaNemeckova/manageiq-ui-classic that referenced this pull request Oct 23, 2019
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label ivanchuk/yes, hammer/yes

@miq-bot add_label ivanchuk/no

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.

4 participants