Disable Web Console button when VM's platform is Windows #1282
Conversation
|
@miq-bot add_labels bug, compute/infrastructure, compute/cloud |
bmclaughlin
force-pushed
the
cockpit-doesnt-support-windows
branch
from
6fe3e52
to
9fb8dd9
Compare
| @@ -4,6 +4,7 @@ class ApplicationHelper::Button::CockpitConsole < ApplicationHelper::Button::Bas | |||
| def disabled? | |||
| record_type = @record.respond_to?(:current_state) ? _('VM') : _('Container Node') | |||
| @error_message = _("The web-based console is not available because the %{record_type} is not powered on" % {:record_type => record_type}) unless on? | |||
| @error_message = _('The web-based console is not available because the Windows platform is not supported') unless platform_supported? && @record.respond_to?(:current_state) | |||
There was a problem hiding this comment.
:current_state is already being looked at in platform_supported? right? There's no need to test it twice.
| @@ -13,4 +14,8 @@ def on? | |||
| return @record.current_state == 'on' if @record.respond_to?(:current_state) # VM status | |||
| @record.ready_condition_status == 'True' if @record.respond_to?(:ready_condition_status) # Container status | |||
| end | |||
|
|
|||
| def platform_supported? | |||
| @record.platform.downcase != 'windows' if @record.respond_to?(:current_state) | |||
There was a problem hiding this comment.
I don't quite understand how :current_state and @record.platform come together.
As in if we don't know the current state, we also do not know the platform? I'm not saying this
is incorrect, I'm just curious.
Also, if the above is correct, I'd rather change the above line to:
@record.respond_to?(:current_state) && @record.platform.downcase != 'windows'There was a problem hiding this comment.
@mzazrivec, I'm checking if it responds to current state to verify it's a VM so that the platform can be checked. In my research I was unable to determine a reliable way to verify the platform of a Container Node.
spec/helpers/application_helper/buttons/cockpit_console_spec.rb
bmclaughlin
force-pushed
the
cockpit-doesnt-support-windows
branch
from
9fb8dd9
to
b31810b
Compare
|
Checked commits bmclaughlin/manageiq-ui-classic@58666d4~...b31810b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Disable Web Console button when VM's platform is Windows (cherry picked from commit 5ea3f51) https://bugzilla.redhat.com/show_bug.cgi?id=1460807
|
Fine backport details: |
|
@miq-bot add_label euwe/yes |
|
Euwe backport details: |
|
The logic here is a bit wrong and ignores container nodes that also use this button. A machine or vm that doesn't respond to current_status should not be treated as a windows machine |
|
Opened #1988 to fix the logic error, please take a look. |
https://bugzilla.redhat.com/show_bug.cgi?id=1447100