From 91a406dc69b05dfe5e6906fc67263c3da850aa3d Mon Sep 17 00:00:00 2001 From: Partha Aji Date: Mon, 25 Apr 2016 13:49:20 -0400 Subject: [PATCH] Fixes #14802 - Correctly determines if working with Host/Hostgroups page Prior to this commit the hosts and hostroups pages used the same erbs They were just identifying what constitutes a host page vs hostgroup page in the wrong manner. It used -> @host.nil? to determine this which doesnt work for all situations. This commit instead centralizes that logic in a nice helper and uses a simpler 'check the controller name' approach --- .../katello/hosts_and_hostgroups_helper.rb | 5 +++++ .../_host_environment_select.html.erb | 19 +++++++++---------- .../_host_media_type_select.html.erb | 5 ++--- .../_host_synced_content_select.html.erb | 14 +++++++------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/app/helpers/katello/hosts_and_hostgroups_helper.rb b/app/helpers/katello/hosts_and_hostgroups_helper.rb index 81e30521cb5..2ce087b4bf0 100644 --- a/app/helpers/katello/hosts_and_hostgroups_helper.rb +++ b/app/helpers/katello/hosts_and_hostgroups_helper.rb @@ -4,6 +4,10 @@ def kt_ak_label "kt_activation_keys" end + def using_hostgroups_page? + controller.controller_name == "hostgroups" + end + def blank_or_inherit_with_id(f, attr) return true unless f.object.respond_to?(:parent_id) && f.object.parent_id inherited_value = f.object.send(attr).try(:id) || '' @@ -35,6 +39,7 @@ def lifecycle_environment(host) end def use_install_media(host, options = {}) + return true if host && host.errors && host.errors.include?(:medium_id) kickstart_repository_id(host, options).blank? end diff --git a/app/views/overrides/activation_keys/_host_environment_select.html.erb b/app/views/overrides/activation_keys/_host_environment_select.html.erb index c78dc83d39b..ccffe79f0de 100644 --- a/app/views/overrides/activation_keys/_host_environment_select.html.erb +++ b/app/views/overrides/activation_keys/_host_environment_select.html.erb @@ -4,14 +4,13 @@ option.kt-cv { margin-left: 1em; } <% spinner_path = asset_path('spinner.gif') %> -<% using_hostgroups_page = @host.nil? %> -<% env_select_id = using_hostgroups_page ? :hostgroup_lifecycle_environment_id : :host_lifecycle_environment_id %> -<% env_select_name = using_hostgroups_page ? 'hostgroup[lifecycle_environment_id]' : 'host[content_facet_attributes][lifecycle_environment_id]' %> -<% env_select_attr = using_hostgroups_page ? 'lifecycle_environment' : 'content_facet.lifecycle_environment' %> +<% env_select_id = using_hostgroups_page? ? :hostgroup_lifecycle_environment_id : :host_lifecycle_environment_id %> +<% env_select_name = using_hostgroups_page? ? 'hostgroup[lifecycle_environment_id]' : 'host[content_facet_attributes][lifecycle_environment_id]' %> +<% env_select_attr = using_hostgroups_page? ? 'lifecycle_environment' : 'content_facet.lifecycle_environment' %> <%= field(f, env_select_attr, {:label => _("Lifecycle Environment")}) do - if using_hostgroups_page + if using_hostgroups_page? select_tag env_select_id, lifecycle_environment_options(@hostgroup, :include_blank => blank_or_inherit_with_id(f, :lifecycle_environment)), :class => 'form-control', :name => env_select_name else @@ -19,11 +18,11 @@ end end %> -<% cv_select_id = using_hostgroups_page ? :hostgroup_content_view_id : :host_content_view_id %> -<% cv_select_name = using_hostgroups_page ? 'hostgroup[content_view_id]' : 'host[content_facet_attributes][content_view_id]' %> -<% cv_select_attr = using_hostgroups_page ? 'content_view' : 'content_facet.content_view' %> +<% cv_select_id = using_hostgroups_page? ? :hostgroup_content_view_id : :host_content_view_id %> +<% cv_select_name = using_hostgroups_page? ? 'hostgroup[content_view_id]' : 'host[content_facet_attributes][content_view_id]' %> +<% cv_select_attr = using_hostgroups_page? ? 'content_view' : 'content_facet.content_view' %> <%= field(f, cv_select_attr, {:label => _("Content View")}) do - if using_hostgroups_page + if using_hostgroups_page? select_tag cv_select_id, content_views_for_host(@hostgroup, :include_blank => blank_or_inherit_with_id(f, :content_view)), :data => {"spinner_path" => spinner_path}, :class => 'form-control', :name => cv_select_name else @@ -35,7 +34,7 @@ end %> <% end %> -<% data_url = using_hostgroups_page ? environment_selected_hostgroups_path : hostgroup_or_environment_selected_hosts_path %> +<% data_url = using_hostgroups_page? ? environment_selected_hostgroups_path : hostgroup_or_environment_selected_hosts_path %> <%= select_f f, :environment_id, Environment.all, :id, :to_label, {:include_blank => blank_or_inherit_f(f, :environment)}, {:label => _("Puppet Environment"), :onchange => 'update_puppetclasses(this);', 'data-url'=> data_url, 'data-content_puppet_match' => (@host || @hostgroup).new_record? || (@host || @hostgroup).content_and_puppet_match?, diff --git a/app/views/overrides/activation_keys/_host_media_type_select.html.erb b/app/views/overrides/activation_keys/_host_media_type_select.html.erb index 38e4e665c79..b3c5d5cb32e 100644 --- a/app/views/overrides/activation_keys/_host_media_type_select.html.erb +++ b/app/views/overrides/activation_keys/_host_media_type_select.html.erb @@ -1,10 +1,9 @@ -<% using_hostgroups_page = @host.nil? %> -<% media_selection = using_hostgroups_page ? use_install_media(@hostgroup) : use_install_media(@host, :selected_host_group => @hostgroup) +<% media_selection = using_hostgroups_page? ? use_install_media(@hostgroup) : use_install_media(@host, :selected_host_group => @hostgroup) install_media_radio = media_selection ? 'checked="checked"':'' synced_content_radio = media_selection ? '' : 'checked="checked"' install_media_disabled = os_media.empty? ? 'disabled = true' : '' - kickstart_options = using_hostgroups_page ? kickstart_repository_options(@hostgroup) : kickstart_repository_options(@host, :selected_host_group => @hostgroup) + kickstart_options = using_hostgroups_page? ? kickstart_repository_options(@hostgroup) : kickstart_repository_options(@host, :selected_host_group => @hostgroup) synced_content_disabled = kickstart_options.empty? ? 'disabled = true' : '' %> diff --git a/app/views/overrides/activation_keys/_host_synced_content_select.html.erb b/app/views/overrides/activation_keys/_host_synced_content_select.html.erb index b0c8f94d2cb..d785c58f8e7 100644 --- a/app/views/overrides/activation_keys/_host_synced_content_select.html.erb +++ b/app/views/overrides/activation_keys/_host_synced_content_select.html.erb @@ -1,16 +1,16 @@ -<% using_hostgroups_page = @host.nil? -kickstart_repo_id = using_hostgroups_page ? kickstart_repository_id(@hostgroup) : kickstart_repository_id(@host, :selected_host_group => @hostgroup) -host = using_hostgroups_page ? @hostgroup : @host +<% +kickstart_repo_id = using_hostgroups_page? ? kickstart_repository_id(@hostgroup) : kickstart_repository_id(@host, :selected_host_group => @hostgroup) +host = using_hostgroups_page? ? @hostgroup : @host kickstart_options = kickstart_repository_options(host, :selected_host_group => @hostgroup) -ks_repo_select_id = using_hostgroups_page ? :host_group_kickstart_repository_id : :host_kickstart_repository_id -ks_repo_select_name = using_hostgroups_page ? 'hostgroup[kickstart_repository_id]' : 'host[content_facet_attributes][kickstart_repository_id]' -ks_repo_select_attr = using_hostgroups_page ? 'kickstart_repository' : 'content_facet.kickstart_repository' +ks_repo_select_id = using_hostgroups_page? ? :host_group_kickstart_repository_id : :host_kickstart_repository_id +ks_repo_select_name = using_hostgroups_page? ? 'hostgroup[kickstart_repository_id]' : 'host[content_facet_attributes][kickstart_repository_id]' +ks_repo_select_attr = using_hostgroups_page? ? 'kickstart_repository' : 'content_facet.kickstart_repository' %> <% spinner_path = asset_path('spinner.gif') %> <%= field(f, ks_repo_select_attr, {:label => _("Synced Content")}) do - if using_hostgroups_page + if using_hostgroups_page? select_tag ks_repo_select_id, view_to_options(kickstart_options, kickstart_repo_id, blank_or_inherit_with_id(f, :kickstart_repository)), :data => {"spinner_path" => spinner_path, "kickstart-repository-id" => kickstart_repo_id}, :class => 'form-control', :name => ks_repo_select_name, :disabled => kickstart_options.empty? else