Fixing Provider PhysicalSwitches page summary show #3954
Conversation
EsdrasVP
changed the title
app/helpers/quadicon_helper.rb
Outdated
| @@ -70,8 +70,6 @@ def self.settings_key(klass, layout) | |||
| when "ems_physical_infra", "ems_cloud", "ems_network", "ems_container" | |||
| layout.to_sym | |||
| end | |||
| elsif klass.base_model.name != klass.name.demodulize | |||
There was a problem hiding this comment.
Sorry, can't remove this - this will break Vm for example :).
But you can add a special case for switch vs physical switch, the same as it is done for ExtManagementSystem.
EDIT: never mind, it's the other line, this was introduced in #3699 and should be safe to remove
There was a problem hiding this comment.
Ok, I put back the demodulize and added a condition for "physical_switch".
There was a problem hiding this comment.
Yes, it is safe to remove. It was introduced by me at https://github.com/ManageIQ/manageiq-ui-classic/pull/3699/files#diff-6525cb9600835fcc2ba062fde39a2292R73
There was a problem hiding this comment.
@himdel @saulotoledo I removed both demodulize and the elsif for physical_switch.
There was a problem hiding this comment.
@himdel this actually fixes VMs 😉
There was a problem hiding this comment.
But as I stated it here this might not be The Right Way™ to fix this...
| @@ -54,7 +54,12 @@ def textual_port | |||
| end | |||
|
|
|||
| def textual_physical_switches | |||
| textual_link(@record.physical_switches) | |||
There was a problem hiding this comment.
most likely, adding :as => PhysicalSwitch would have the same effect
More to the point, the new code does not use role_allows?, so it can't stay as is.
There was a problem hiding this comment.
Sorry for that, this was an old solution that I put before fixing the base model for PhysicalSwitch. I put back the textual link call, which is working with new solution.
EsdrasVP
force-pushed
the
fix_provider_switches_listing
branch
from
a3a0e13 to
4b69907
Compare
EsdrasVP
changed the title
EsdrasVP
force-pushed
the
fix_provider_switches_listing
branch
from
4b69907 to
16278d9
Compare
EsdrasVP
changed the title
EsdrasVP
changed the title
EsdrasVP
changed the title
app/helpers/quadicon_helper.rb
Outdated
| @@ -70,6 +70,8 @@ def self.settings_key(klass, layout) | |||
| when "ems_physical_infra", "ems_cloud", "ems_network", "ems_container" | |||
| layout.to_sym | |||
| end | |||
| elsif klass.base_model.to_s.underscore == "physical_switch" | |||
| klass.base_model.name.underscore.to_sym | |||
| elsif klass.base_model.name != klass.name.demodulize | |||
| klass.name.demodulize.underscore.to_sym | |||
There was a problem hiding this comment.
I suggest to remove the demodulize. It was introduced by me at https://github.com/ManageIQ/manageiq-ui-classic/pull/3699/files#diff-6525cb9600835fcc2ba062fde39a2292R73 and it is causing an issue fixed at #3980
Removing the demodulize will help in both cases.
There was a problem hiding this comment.
@saulotoledo I removed this, now it should work fine without both conditions.
EsdrasVP
force-pushed
the
fix_provider_switches_listing
branch
from
16278d9 to
306d1fb
Compare
|
This pull request is not mergeable. Please rebase and repush. |
EsdrasVP
force-pushed
the
fix_provider_switches_listing
branch
from
306d1fb to
b5364ba
Compare
EsdrasVP
changed the title
EsdrasVP
force-pushed
the
fix_provider_switches_listing
branch
from
b5364ba to
12de9e7
Compare
|
Checked commit EsdrasVP@12de9e7 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
EsdrasVP
changed the title
EsdrasVP
changed the title
|
@himdel Could you check if this solution is ok? |
|
@EsdrasVP looks good, thanks! :) Is this still |
|
@himdel There is no more |
|
👍 Seeing switches both in provider summary and in relationships now. |
This PR is able to:
The problems:
When trying to list the Physical Switches from a Physical Infra Provider (
/ems_physical_infra/:id?display=physical_switches)An error is returned
This happens because the application controller from the UI calls
paged_view_search, which is a method from Search class inside ManageIQ core that, internally, uses the base model for the class passed. The result is that, instead ofPhysicalSwitch, its parentSwitchis called.