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.

Sign up for GitHub

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

Jump to bottom

Fix open_url with dialog #4334

Merged
merged 2 commits into from
Aug 17, 2018
Merged

Fix open_url with dialog #4334

merged 2 commits into from
Aug 17, 2018

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Jul 23, 2018 •
edited by martinpovolny
Loading

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

Create custom button with Automate method:

  1. Automation -> Automate -> Explorer:

    a) create a new domain with a name

    b) select ManageIQ/System/Request

    c) Configuration -> Copy this Class (copy it to your new domain from step 1a)

    d) select Request Class in your new domain -> Configuration -> Add a new Instance

    • name TestOpenUrl
    • to meth1 write test_open_url

    e) Select Request class in your new domain, click Methods -> Configuration -> Add a new Method

    • type: inline
    • name: test_open_url
    • data should be (or anything that sets vm.remote_console_url to something):
      vm = $evm.root['vm']
      $evm.log(:info, "VM to launch SSH for is #{vm.hostnames[0]}")
      vm.remote_console_url = "https://www.google.com"

important: use URL including the protocol (https) or the browser will ignore the request
also disable URL pop-up blocking in your browser to make this work

Automation -> Automate -> Customization -> Buttons

Create a new button for VMs with Open Url checked and with a Dialog(doesn't matter which one) and Request has to be TestOpenUrl

Go to VM summary page and click the new button

After submitting the dialog you should be redirected to VM summary page and a new page should be open in a tab.
image

@miq-bot add_label wip

Related PRs: ManageIQ/manageiq#17802 (MERGED) and ManageIQ/manageiq-automation_engine#205 (MERGED) and ManageIQ/manageiq-api#444 (MERGED)

@miq-bot miq-bot changed the title Trying to fix scrutinizer [WIP] Trying to fix scrutinizer Jul 23, 2018
@miq-bot miq-bot added the wip label Jul 23, 2018
@ZitaNemeckova ZitaNemeckova force-pushed the fix_dialog_with_open_url branch from 7be26fd to 93dfcd3 Compare July 24, 2018 07:49
@ZitaNemeckova ZitaNemeckova changed the title [WIP] Trying to fix scrutinizer [WIP] Fix open_url with dialog Jul 24, 2018
@ZitaNemeckova ZitaNemeckova force-pushed the fix_dialog_with_open_url branch 5 times, most recently from 73e211b to c21cf0c Compare August 1, 2018 12:30
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label pending core

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label gaprindashvili/yes, automation/automate, blocker, bug

@ZitaNemeckova ZitaNemeckova force-pushed the fix_dialog_with_open_url branch 3 times, most recently from bba0e1c to 39bea79 Compare August 2, 2018 12:05
This was referenced Aug 7, 2018
Merged
Merged
@ZitaNemeckova ZitaNemeckova force-pushed the fix_dialog_with_open_url branch 3 times, most recently from a651121 to 7e04682 Compare August 7, 2018 13:01
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Fix open_url with dialog Fix open_url with dialog Aug 7, 2018
@miq-bot miq-bot removed the wip label Aug 7, 2018
martinpovolny
martinpovolny reviewed Aug 8, 2018
View reviewed changes
app/controllers/application_controller/buttons.rb Outdated
@@ -248,6 +248,12 @@ def ab_group_delete
end
end

def open_url_after_dialog
url = SystemConsole.find_by(:vm_id => params[:targetId]).url
# url = "www.redhat.com"
Copy link
Member

Choose a reason for hiding this comment

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

forgotten comment?

@miq-bot
Copy link
Member

miq-bot commented Aug 9, 2018

Checked commits ZitaNemeckova/manageiq-ui-classic@fce0833~...171214d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@ZitaNemeckova ZitaNemeckova reopened this Aug 13, 2018
@ZitaNemeckova ZitaNemeckova mentioned this pull request Aug 14, 2018
Merged
@lfu
Copy link
Member

lfu commented Aug 16, 2018

@ZitaNemeckova What page am I expecting after submitting the dialog following your steps in the description? Could you add a screenshot? Thanks.

@ZitaNemeckova
Copy link
Contributor Author

@lfu I updated the description. Let me know if you need more info :)

@martinpovolny
Copy link
Member

@lfu: I suppose you are testing this one. Thanks! Ping me, please, once you verify that this works in the UI.

@lfu
Copy link
Member

lfu commented Aug 16, 2018

@ZitaNemeckova Thanks for the update.
I was redirected to the VM summary page but I did not see a new tab with the specified URL.
A SystemConsole record was created in VMDB. Seems everything went well.
Did I miss any configuration to see the new page?

@martinpovolny
Copy link
Member

Pop-ups unblocked in the browser?

@lfu
Copy link
Member

lfu commented Aug 16, 2018

@martinpovolny Of course it is blocked! And now I got:

screen shot 2018-08-16 at 2 48 41 pm

@martinpovolny
Copy link
Member

what about an url starting with 'http://' or 'https://" ? might be anything else is considered relative?

@lfu
Copy link
Member

lfu commented Aug 16, 2018

@martinpovolny 👍 Worked with https://www.google.com.

@martinpovolny martinpovolny merged commit 5fc5098 into ManageIQ:master Aug 17, 2018
@martinpovolny martinpovolny self-assigned this Aug 17, 2018
@martinpovolny martinpovolny added this to the Sprint 93 Ending Aug 27, 2018 milestone Aug 17, 2018
@ZitaNemeckova
Copy link
Contributor Author

@martinpovolny Two related PRs aren't merged yet. ManageIQ/manageiq#17802 and ManageIQ/manageiq-api#444 Is it ok, it's merged without them?

@martinpovolny
Copy link
Member

@ZitaNemeckova : tests pass --> no breakage. Also I pinged both the core and API PRs. We need to move!

@ZitaNemeckova ZitaNemeckova deleted the fix_dialog_with_open_url branch August 17, 2018 13:31
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label pending core

@ZitaNemeckova ZitaNemeckova mentioned this pull request Aug 22, 2018
Merged
h-kataria
h-kataria reviewed Aug 22, 2018
View reviewed changes
app/views/shared/dialogs/_dialog_provision.html.haml
@@ -50,4 +50,5 @@
:finish_submit_endpoint => finish_submit_endpoint,
:api_submit_endpoint => api_submit_endpoint,
:api_action => api_action,
:cancel_endpoint => cancel_endpoint}
:cancel_endpoint => cancel_endpoint,
:open_url => open_url}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ZitaNemeckova is causing an error on upstream, please take a look
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in #4521

@simaishi
Copy link
Contributor

@ZitaNemeckova As discussed, I tried to backport #4036 as well but that's conflicting too. I think it's best that you create a separate PR for gaprindashvili for this and #4036 combined. Feel free to include #4521 as well, since that's a follow up to this PR.

@ZitaNemeckova ZitaNemeckova mentioned this pull request Sep 21, 2018
Merged
@simaishi
Copy link
Contributor

Backported to Gaprindashvili via #4691

@martinpovolny martinpovolny mentioned this pull request Jun 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@h-kataria h-kataria h-kataria left review comments

@martinpovolny martinpovolny martinpovolny left review comments

Assignees

@martinpovolny martinpovolny

Labels
automation/automate blocker bug gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ZitaNemeckova @miq-bot @lfu @martinpovolny @simaishi @h-kataria