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

TextualSummary: unify power states. #4085

Merged
merged 1 commit into from
Jun 11, 2018
Merged

TextualSummary: unify power states. #4085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions app/helpers/host_helper/textual_summary.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module HostHelper::TextualSummary
include TextualMixins::TextualDevices
include TextualMixins::TextualOsInfo
include TextualMixins::TextualVmmInfo
include TextualMixins::TextualPowerState
# TODO: Determine if DoNav + url_for + :title is the right way to do links, or should it be link_to with :title

#
Expand Down Expand Up @@ -163,9 +164,7 @@ def textual_service_tag
end

def textual_power_state
state = @record.state.to_s.downcase
state = "unknown" if state.blank?
{:label => _("Power State"), :image => "svg/currentstate-#{state}.svg", :value => state}
textual_power_state_whitelisted(@record.state)
end

def textual_lockdown_mode
Expand Down
4 changes: 3 additions & 1 deletion app/helpers/physical_server_helper/textual_summary.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module PhysicalServerHelper::TextualSummary
include TextualMixins::TextualPowerState

def textual_group_properties
TextualGroup.new(
_("Properties"),
Expand Down Expand Up @@ -105,7 +107,7 @@ def textual_cores
end

def textual_power_state
{:label => _("Power State"), :value => @record.power_state}
textual_power_state_whitelisted(@record.power_state)
end

def textual_mac
Expand Down
4 changes: 3 additions & 1 deletion app/helpers/physical_switch_helper/textual_summary.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module PhysicalSwitchHelper::TextualSummary
include TextualMixins::TextualPowerState

def textual_group_properties
TextualGroup.new(
_("Properties"),
Expand Down Expand Up @@ -69,7 +71,7 @@ def textual_description
end

def textual_power_state
{:label => _("Power State"), :value => @record.power_state}
textual_power_state_whitelisted(@record.power_state)
end

def management_networks
Expand Down
25 changes: 19 additions & 6 deletions app/helpers/textual_mixins/textual_power_state.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
module TextualMixins::TextualPowerState
def textual_power_state
state = @record.current_state.downcase
state = "unknown" if state.blank?
h = {:label => _("Power State"), :value => state}
h[:image] = "svg/currentstate-#{@record.template? ? 'template' : state}.svg"
h
VALID_POWER_STATE = %w(
Copy link
Contributor

@himdel himdel Jun 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This looks suspiciously similar to QuadiconHelper#MACHINE_STATE_QUADRANT (except that one uses fonticons for those same states)

Should we try to unify those two as well?

Copy link
Member Author

@martinpovolny martinpovolny Jun 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Similar, but not the same. Yes, we could and should unify that. But I don't have an authoritative set of values for this. That would have to come from the backend. Or do we know what are the valid values? @skateman ?

I see two options:

  • leave this as it is now. Possibly do some unification in the user, after we somehow figure what are the valid values.
  • use @skateman 's list from QuadionHelper ignoring the couple of extra values (powering*).

I don't have a strong opinion here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using this for now, eventually we should be using fonticons instead of svgs here as well.

archived connecting disconnected image_locked install_failed maintenance
migrating never non_operational not_responding off on orphaned paused
powering_down powering-down powering_up powering-up preparing_for_maintenance
reboot_in_progress retired shelved_offloaded shelved standby suspended
template terminated unknown wait_for_launch
).freeze

def power_state_to_image(state)
"svg/currentstate-#{state}.svg" if VALID_POWER_STATE.include?(state)
end

def textual_power_state_whitelisted(state)
state = state.blank? ? 'unknown' : state.downcase
{:label => _('Power State'), :value => state, :image => power_state_to_image(state)}
end

def textual_power_state_whitelisted_with_template
textual_power_state_whitelisted(@record.template? ? 'template' : @record.current_state)
end
end
4 changes: 4 additions & 0 deletions app/helpers/vm_cloud_helper/textual_summary.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ def textual_group_security
#
# Items
#
def textual_power_state
textual_power_state_whitelisted_with_template
end

def textual_ipaddress
return nil if @record.template?
ips = @record.ipaddresses
Expand Down
4 changes: 4 additions & 0 deletions app/helpers/vm_helper/textual_summary.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ def textual_group_normal_operating_ranges
#
# Items
#
def textual_power_state
textual_power_state_whitelisted_with_template
end

def textual_hostname
hostnames = @record.hostnames
{:label => n_("Hostname", "Hostnames", hostnames.size), :value => hostnames.join(", ")}
Expand Down