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 error for Datastore custom button #4745

Merged
merged 3 commits into from Oct 22, 2018

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Oct 9, 2018

  • Add open_url to local_options for non-explorer screens
  • Fix error handling when Automate does not return url

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

Create custom button with dialog but without Open URL for Datastore with Display for set to Single.

Automation -> Automate -> Customization -> Buttons

Go to Datastore summary page and click the new button.

Before:

FATAL -- : Error caught: [ActionView::Template::Error] undefined local variable or method `open_url' for #<#<Class:0x00007fab97d529e8>:0x00007fab89690820>
Did you mean?  ops_url
manageiq-ui-classic/app/views/shared/dialogs/_dialog_provision.html.haml:45:in `__anage___manageiq_ui_classic_app_views_shared_dialogs__dialog_provision_html_haml__1101056942674079458_70187113809480'

screen shot 2018-10-15 at 4 08 59 pm

After:
Without Open URL
screen shot 2018-10-16 at 10 30 25 am
With Open URL
screen shot 2018-10-16 at 4 39 26 pm

@miq-bot add_label wip, bug, hammer/yes, gaprindashvili/yes

@miq-bot miq-bot changed the title Fix error for Datastore custom button [WIP] Fix error for Datastore custom button Oct 9, 2018
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Fix error for Datastore custom button Fix error for Datastore custom button Oct 15, 2018
@miq-bot miq-bot removed the wip label Oct 15, 2018
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label blocker

@romanblanco
Copy link
Member

@miq-bot assign @romanblanco

@ZitaNemeckova
Copy link
Contributor Author

@romanblanco Error handling changed as discussed.

@mzazrivec
Copy link
Contributor

I guess the codecliemate issues are worth investigating / fixing.

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.

I'm seeing the same results in UI after setting Open URL: true, but there's still an error:

[----] F, [2018-10-18T13:39:24.849133 #24431:2b018ad638bc] FATAL -- : Error caught: [NoMethodError] undefined method `url' for nil:NilClass
/home/rblanco/devel/manageiq-ui-classic/app/controllers/application_controller/buttons.rb:252:in `open_url_after_dialog'

@ZitaNemeckova
Copy link
Contributor Author

ZitaNemeckova commented Oct 18, 2018

Yep it's broken. APi.wait_for_task gets

actions: (2) [{…}, {…}]
context_data: null
created_on: "2018-10-18T14:52:56Z"
href: "http://localhost:3000/api/tasks/10000000054786"
id: "10000000054786"
identifier: null
message: "MiqTask has been queued."
miq_server_id: "10000000000014"
name: "Automate method task for open_url"
pct_complete: null
results: null
started_on: "2018-10-18T14:52:57Z"
state: "Finished"
status: "Ok"
updated_on: "2018-10-18T14:52:57Z"
userid: "admin"
zone: null

@miq-bot add_label wip

Edit: Blamed wrong thing. If remote_console_url isn't set in automate method it never creates correct SystemConsole. Added check for that in the last commit.

@miq-bot miq-bot changed the title Fix error for Datastore custom button [WIP] Fix error for Datastore custom button Oct 18, 2018
@miq-bot miq-bot added the wip label Oct 18, 2018
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@romanblanco Now it has to work even with your InspectMe method. Please give it another try.

@miq-bot
Copy link
Member

miq-bot commented Oct 22, 2018

Checked commits ZitaNemeckova/manageiq-ui-classic@77b692a~...c6ff820 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@romanblanco Now it has to work even with your InspectMe method. Please give it another try.

@miq-bot miq-bot changed the title [WIP] Fix error for Datastore custom button Fix error for Datastore custom button Oct 22, 2018
@miq-bot miq-bot removed the wip label Oct 22, 2018
@miq-bot miq-bot changed the title Fix error for Datastore custom button [WIP] Fix error for Datastore custom button Oct 22, 2018
@miq-bot miq-bot added the wip label Oct 22, 2018
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.

The GitHub page is currently not getting updated correctly.

After the last changes, the fix works:

diff --git a/app/assets/javascripts/controllers/dialog_user/dialog_user_controller.js b/app/assets/javascripts/controllers/dialog_user/dialog_user_controller.js
index f52fcf90e..cff19f0ba 100644
--- a/app/assets/javascripts/controllers/dialog_user/dialog_user_controller.js
+++ b/app/assets/javascripts/controllers/dialog_user/dialog_user_controller.js
@@ -79,8 +79,13 @@ ManageIQ.angular.app.controller('dialogUserController', ['API', 'dialogFieldRefr
               return $http.post("open_url_after_dialog", {targetId: vm.targetId});
             })
             .then(function(response) {
-              window.open(response.data.open_url);
-              miqService.redirectBack(__('Order Request was Submitted'), 'success', finishSubmitEndpoint);
+              if (response.data.open_url) {
+                window.open(response.data.open_url);
+                miqService.redirectBack(__('Order Request was Submitted'), 'success', finishSubmitEndpoint);
+              } else {
+                miqService.miqFlash('error', __('Automate failed to obtain URL.'));
+                miqService.sparkleOff();
+              };
             })
             .catch(function() {
               return Promise.reject({data: {error: {message: '-'.concat(__('Automate failed to obtain URL.')) }}});
diff --git a/app/controllers/application_controller/buttons.rb b/app/controllers/application_controller/buttons.rb
index 1da917ec2..5669e6ae7 100644
--- a/app/controllers/application_controller/buttons.rb
+++ b/app/controllers/application_controller/buttons.rb
@@ -254,7 +254,8 @@ module ApplicationController::Buttons
   end
   def open_url_after_dialog
-    url = SystemConsole.find_by(:vm_id => params[:targetId]).url
+    system_console = SystemConsole.find_by(:vm_id => params[:targetId])
+    url = system_console.try(:url)
     render :json => {:open_url => url}
   end

Whoever merges this, please make sure the @ZitaNemeckova's commits with changes are in place

@JPrause
Copy link
Member

JPrause commented Oct 22, 2018

@ZitaNemeckova as this is for a 5.9.5 blocker, is this still a WIP.

@romanblanco
Copy link
Member

@JPrause It's not WIP anymore, but because of the website error the label probably is not getting updated as well as the commited changes.

@ZitaNemeckova
Copy link
Contributor Author

@JPrause I tried to remove wip via miq-bot #4745 (comment) .

@himdel
Copy link
Contributor

himdel commented Oct 22, 2018

@miq-bot remove_label wip

EDIT: ..because I didn't see the comments :) .. also removed manually, hopefully it sticks

@himdel himdel removed the wip label Oct 22, 2018
@himdel himdel changed the title [WIP] Fix error for Datastore custom button Fix error for Datastore custom button Oct 22, 2018
@himdel himdel added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 22, 2018
@himdel himdel merged commit fe7f419 into ManageIQ:master Oct 22, 2018
@himdel
Copy link
Contributor

himdel commented Oct 22, 2018

Merged. Tested locally, because checks don't seem to be responding consistently today.

@miq-bot miq-bot changed the title Fix error for Datastore custom button [WIP] Fix error for Datastore custom button Oct 22, 2018
@miq-bot miq-bot added the wip label Oct 22, 2018
@simaishi simaishi changed the title [WIP] Fix error for Datastore custom button Fix error for Datastore custom button Oct 22, 2018
@simaishi simaishi removed the wip label Oct 22, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 6669028a9b8f05213fac26eb481277fdb50e7ded
Author: Martin Hradil <himdel@seznam.cz>
Date:   Mon Oct 22 14:41:32 2018 +0200

    Merge pull request #4745 from ZitaNemeckova/fix_error_handling_open_url
    
    Fix error for Datastore custom button
    
    (cherry picked from commit fe7f419cc67c659a3bcabadc63a2cb334c1d4aed)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641669
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641670

@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 75b755f67a943735f71dc492fb3a37e214e47f4d
Author: Martin Hradil <himdel@seznam.cz>
Date:   Mon Oct 22 14:41:32 2018 +0200

    Merge pull request #4745 from ZitaNemeckova/fix_error_handling_open_url
    
    Fix error for Datastore custom button
    
    (cherry picked from commit fe7f419cc67c659a3bcabadc63a2cb334c1d4aed)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1574403
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1635797

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.

None yet

7 participants