Adding connected physical servers to physical switches page #4356
Conversation
douglasgabriel
force-pushed
the
cnt_comp_sys
branch
2 times, most recently
from
b441cb0 to
6ee47de
Compare
| def textual_ports | ||
| ports_count = @record.physical_network_ports.count | ||
| ports = {:label => _("Ports"), :value => ports_count, :icon => "ff ff-network-port"} | ||
| ports = {:label => _("Ports"), :value => ports_count, :icon => PhysicalNetworkPortDecorator.fonticon} |
There was a problem hiding this comment.
I'd say record.decorate.fonticon, what do you think, @skateman ?
There was a problem hiding this comment.
In this case @record is a PhysicalSwitch instance, and here I'm printing the PhysicalNetworkPort icon, so if I put record.decorate.fonticon the wrong icon will be shown. 🤔
There was a problem hiding this comment.
oh, ok. @skateman, if you have a better way to handle this, let's do it in a follow up PR.
There was a problem hiding this comment.
@martinpovolny 🤔 🤔 🤔 I don't have a better idea right now 😕
| physical_servers_count = @record.physical_servers.count | ||
| physical_servers = {:label => _("Physical Servers"), :value => physical_servers_count, :icon => PhysicalServerDecorator.fonticon} | ||
| if physical_servers_count.positive? | ||
| physical_servers[:link] = "/physical_switch/show/#{@record.id}?display=physical_servers" |
There was a problem hiding this comment.
I think you should try to use a helper method to calculate the URLs. I will help if we convert the controllers to restful ones. (The same on line 44). There's show_link in GenericShowMixin that you have already included in your controller. Seems to me you could use that one.
There was a problem hiding this comment.
Just realize this method: url_for_only_path. Do you think that I can use it?
Like:
physical_servers[:link] = url_for_only_path(:action => 'show', :id => @record, :display => 'physical_servers')
|
Last thing: can you, please, add a (simple) spec for the textual summary helper? There is already a number of examples. Thx! |
douglasgabriel
force-pushed
the
cnt_comp_sys
branch
from
6ee47de to
40e6f0a
Compare
|
@douglasgabriel : please, ping me, when you add the specs. Thx! |
douglasgabriel
force-pushed
the
cnt_comp_sys
branch
2 times, most recently
from
4c8cfde to
ef5e6dc
Compare
|
Done @martinpovolny . I don't think that the Travis failures are due to my changes |
douglasgabriel
force-pushed
the
cnt_comp_sys
branch
from
ef5e6dc to
44d07d1
Compare
|
Checked commit douglasgabriel@44d07d1 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
This PR is able to
Depends on
ManageIQ/manageiq#17735- Merged