Server summary page changes #3754

skovic · 2018-04-11T20:30:15Z

This PR makes the following changes to the Physical Server Summary page:

Make the IPv4 address in the Management Networks table a link so that it's clickable.
Change the name of the Assets table to Asset Details.

skovic · 2018-04-11T20:30:56Z

@miq-bot add_label WIP

skovic · 2018-04-11T20:40:35Z

@AparnaKarve I would like to make external links for the IPv4 addresses, but it doesn't seem to be interpreting the tag correctly. See the screenshot below:

Any idea on how I can get this to work? Thanks

AparnaKarve · 2018-04-11T23:46:21Z

app/helpers/physical_server_helper/textual_summary.rb

@@ -109,7 +109,7 @@ def textual_ipv4
    # It is possible for guest devices not to have network data (or a network
    # hash). As a result, we need to exclude guest devices that don't have
    # network data to prevent a nil class error from occurring.
-    {:label =>  _("IPv4 Address"), :value => @record.hardware.guest_devices.reject { |device| device.network.nil? }.collect { |device| device.network.ipaddress }.join(", ") }
+    {:label =>  _("IPv4 Address"), :value => @record.hardware.guest_devices.reject { |device| device.network.nil? }.collect { |device| create_https_url(device.network.ipaddress) }.join(", ") }


Change this to -

{:label => _("IPv4 Address"), :value => @record.hardware.guest_devices.reject { |device| device.network.nil? }.collect { |device| create_https_url(device.network.ipaddress) }.join(", ").html_safe }

AparnaKarve · 2018-04-11T23:54:00Z

app/helpers/physical_server_helper/textual_summary.rb

+    url = ""
+
+    unless ip.nil?
+      url = link_to(ip, "https://#{ip}")


change this to -

url = link_to(ip, "https://#{ip}", :target => "_blank")

to open the link in a new browser tab

AparnaKarve · 2018-04-11T23:55:57Z

@skovic Please take a look at the suggested inline solution.

… are displayed properly

skovic · 2018-04-13T19:26:22Z

@miq-bot remove_label wip

skovic · 2018-04-13T19:28:27Z

@AparnaKarve Thank you for your suggestions. I got the links to work.

AparnaKarve · 2018-04-13T19:40:54Z

@skovic Great! Thanks for the update.

AparnaKarve · 2018-04-13T23:35:18Z

app/helpers/physical_server_helper/textual_summary.rb

+    # with a comma. Then split this string back into an array using a comma, possibly
+    # followed by whitespace as the delimiter. Finally, iterate through the array
+    # and convert each element into a URL containing an IP address.
+    ip_addresses = ip_addresses.join(",").split(/,\s*/)


@skovic Can you write a spec for textual_ipv4 for some of these IP address related use cases (comma separated multiple IP addresses, single IP addresses etc) ?

@AparnaKarve As requested, I wrote a spec test for textual_ipv4. Thanks

@skovic Thanks - the spec looks good!

martinpovolny · 2018-04-16T09:46:23Z

app/helpers/physical_server_helper/textual_summary.rb

+    end
+
+    url
+  end


def create_https_url(id) ip.present? ? link_to(ip, "https://#{ip}", :target => "_blank") : '' end

or better use the URI module

def create_https_url(id) ip.present? ? link_to(ip, URI::HTTPS.build(:host => ip), :target => "_blank") : '' end

@martinpovolny I tried using the URI::HTTPS.build method, but it's causing the following error:

undefined method 'to_model' for #<URI::HTTPS https://10.243.6.68> [physical_server/show]

@martinpovolny Never mind, I got past the error by calling the to_s method on the object that's returned by the build method.

… so that test works

AparnaKarve · 2018-04-17T20:59:57Z

@skovic It looks like there is a security risk warning with html_safe.
Let me check if there are other alternatives to using html_safe

AparnaKarve · 2018-04-17T21:08:18Z

app/helpers/physical_server_helper/textual_summary.rb

+    ip_addresses = ip_addresses.join(",").split(/,\s*/)
+    ip_address_urls = ip_addresses.collect { |ip_address| create_https_url(ip_address) }
+
+    {:label => _("IPv4 Address"), :value => ip_address_urls.join(", ").html_safe }


Using the sanitize method as shown below would be a good substitute for html_safe and would take care of the rubocop security risk warning -

{:label => _("IPv4 Address"), :value => sanitize(ip_address_urls.join(", "), :attributes => %w(href target)) }

miq-bot · 2018-04-18T15:03:40Z

Checked commits skovic/manageiq-ui-classic@77dd19c~...d6a7f4d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🍪

skovic · 2018-04-18T16:27:09Z

@AparnaKarve Looks like using sanitize took care of the security warning. Thanks

AparnaKarve

@skovic LGTM!

skovic added 2 commits April 11, 2018 16:00

Make the management network IPv4 addresses links

77dd19c

Rename the Asset table to Asset Details

af85883

miq-bot changed the title ~~Server summary page changes~~ [WIP] Server summary page changes Apr 11, 2018

miq-bot added the wip label Apr 11, 2018

AparnaKarve reviewed Apr 11, 2018

View reviewed changes

Update the way that URLs to IPv4 addresses are formatted so that they…

afaa612

… are displayed properly

miq-bot changed the title ~~[WIP] Server summary page changes~~ Server summary page changes Apr 13, 2018

miq-bot removed the wip label Apr 13, 2018

AparnaKarve reviewed Apr 13, 2018

View reviewed changes

martinpovolny reviewed Apr 16, 2018

View reviewed changes

skovic added 4 commits April 16, 2018 17:02

Refactor the create_https_url method based on review feedback

8179edb

Access hardware through physical_server's computer_system association…

b27fa7b

… so that test works

Add spec test for textual_ipv4

801a378

Remove extra whitespace from spec test

ba49244

AparnaKarve reviewed Apr 17, 2018

View reviewed changes

Use sanitize instead of html_safe in textual_ipv4

d6a7f4d

AparnaKarve approved these changes Apr 18, 2018

View reviewed changes

h-kataria self-assigned this Apr 19, 2018

h-kataria added enhancement compute/infrastructure labels Apr 19, 2018

h-kataria added compute/physical infrastructure gaprindashvili/no and removed compute/infrastructure labels Apr 19, 2018

h-kataria added this to the Sprint 84 Ending Apr 23, 2018 milestone Apr 19, 2018

h-kataria merged commit 6f3c954 into ManageIQ:master Apr 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Server summary page changes #3754

Server summary page changes #3754

skovic commented Apr 11, 2018 •

edited

Loading

skovic commented Apr 11, 2018

skovic commented Apr 11, 2018

AparnaKarve Apr 11, 2018

AparnaKarve Apr 11, 2018

AparnaKarve commented Apr 11, 2018

skovic commented Apr 13, 2018

skovic commented Apr 13, 2018

AparnaKarve commented Apr 13, 2018

AparnaKarve Apr 13, 2018

skovic Apr 17, 2018

AparnaKarve Apr 17, 2018

martinpovolny Apr 16, 2018 •

edited

Loading

skovic Apr 16, 2018

skovic Apr 16, 2018

AparnaKarve commented Apr 17, 2018

AparnaKarve Apr 17, 2018

miq-bot commented Apr 18, 2018

skovic commented Apr 18, 2018

AparnaKarve left a comment

+                  end
+                  url
+                end

Server summary page changes #3754

Server summary page changes #3754

Conversation

skovic commented Apr 11, 2018 • edited Loading

skovic commented Apr 11, 2018

skovic commented Apr 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AparnaKarve commented Apr 11, 2018

skovic commented Apr 13, 2018

skovic commented Apr 13, 2018

AparnaKarve commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpovolny Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AparnaKarve commented Apr 17, 2018

Choose a reason for hiding this comment

miq-bot commented Apr 18, 2018

skovic commented Apr 18, 2018

AparnaKarve left a comment

Choose a reason for hiding this comment

skovic commented Apr 11, 2018 •

edited

Loading

martinpovolny Apr 16, 2018 •

edited

Loading