From a896b2f595e84f7385298f12d6f822bce39d195a Mon Sep 17 00:00:00 2001 From: Quinn James Date: Wed, 10 Jan 2024 22:02:55 +0000 Subject: [PATCH] Fixes #36897 - Hosts now follow existing facet standard for hostgroup inheritance This is a complicated topic; more info in Katello PR #10841. Added logic to have host edit form always return values for CV and LE, regardless of whether they can be edited. --- ...ntent_facet_hosts_controller_extensions.rb | 22 ------------ .../concerns/host_managed_extensions.rb | 22 ------------ .../katello/concerns/hostgroup_extensions.rb | 4 ++- .../_host_environment_select.html.erb | 36 ++++++++++--------- lib/katello/engine.rb | 2 -- .../api/v2/hosts_controller_test.rb | 4 +-- 6 files changed, 25 insertions(+), 65 deletions(-) delete mode 100644 app/controllers/katello/concerns/content_facet_hosts_controller_extensions.rb diff --git a/app/controllers/katello/concerns/content_facet_hosts_controller_extensions.rb b/app/controllers/katello/concerns/content_facet_hosts_controller_extensions.rb deleted file mode 100644 index 00d5efdb8e4..00000000000 --- a/app/controllers/katello/concerns/content_facet_hosts_controller_extensions.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Katello - module Concerns - module ContentFacetHostsControllerExtensions - extend ActiveSupport::Concern - included do - before_action :set_up_content_view_environment, only: [:update] - - def set_up_content_view_environment - return unless @host&.content_facet.present? && params[:host]&.[](:content_facet_attributes)&.present? - cv_id = params[:host][:content_facet_attributes].delete(:content_view_id) - env_id = params[:host][:content_facet_attributes].delete(:lifecycle_environment_id) - Rails.logger.info "#{__method__}: cv_id=#{cv_id}, env_id=#{env_id}" - @host.content_facet.assign_single_environment( - lifecycle_environment_id: env_id, - content_view_id: cv_id - ) - Rails.logger.info "#{__method__}: done" - end - end - end - end -end diff --git a/app/models/katello/concerns/host_managed_extensions.rb b/app/models/katello/concerns/host_managed_extensions.rb index 1bc376e9837..18a917977d5 100644 --- a/app/models/katello/concerns/host_managed_extensions.rb +++ b/app/models/katello/concerns/host_managed_extensions.rb @@ -48,28 +48,6 @@ def inherited_attributes inherited_attrs end - def apply_inherited_attributes(attributes, initialized = true) - attributes = super(attributes, initialized) || {} - facet_attrs = attributes&.[]('content_facet_attributes') - return attributes if facet_attrs.blank? - cv_id = facet_attrs['content_view_id'] - lce_id = facet_attrs['lifecycle_environment_id'] - if initialized && (cv_id.blank? || lce_id.blank?) - if cv_id.blank? - Rails.logger.info "Hostgroup has no content view assigned; using host's existing content view" - facet_attrs['content_view_id'] = content_facet&.single_content_view&.id - end - if lce_id.blank? - Rails.logger.info "Hostgroup has no lifecycle environment assigned; using host's existing lifecycle environment" - facet_attrs['lifecycle_environment_id'] = content_facet&.single_lifecycle_environment&.id - end - attributes['content_facet_attributes'] = facet_attrs - else - Rails.logger.debug "Hostgroup has content view and lifecycle environment assigned; using those" - end - attributes - end - def smart_proxy_ids ids = super ids << content_source_id diff --git a/app/models/katello/concerns/hostgroup_extensions.rb b/app/models/katello/concerns/hostgroup_extensions.rb index e08b8d07ae2..4bb38591e27 100644 --- a/app/models/katello/concerns/hostgroup_extensions.rb +++ b/app/models/katello/concerns/hostgroup_extensions.rb @@ -122,7 +122,9 @@ def inherited_ancestry_attribute(attribute, facet) # it will return the value 2. facet_model = Facets.registered_facets[facet].hostgroup_configuration.model value = facet_model.where.not(attribute => nil).joins(:hostgroup).merge( - ::Hostgroup.where(id: self.ancestor_ids).reorder(ancestry: :desc) + ::Hostgroup.where(id: self.ancestor_ids).reorder( + "#{::Hostgroup.table_name}.ancestry desc nulls last" + ) ).pick(attribute) end value 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 a5da5a8e03b..1f0dcce4406 100644 --- a/app/views/overrides/activation_keys/_host_environment_select.html.erb +++ b/app/views/overrides/activation_keys/_host_environment_select.html.erb @@ -29,23 +29,27 @@ end %> <% 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.single_lifecycle_environment' %> -<%= field(f, env_select_attr, {:label => _("Lifecycle Environment")}) do - 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 - select_tag env_select_id, lifecycle_environment_options(@host, :selected_host_group => @hostgroup || @host.hostgroup, :include_blank => blank_or_inherit_with_id(f, :lifecycle_environment)), :class => 'form-control', :name => env_select_name, :disabled => cv_lce_disabled? - end -end %> +<%= field(f, env_select_attr, {:label => _("Lifecycle Environment")}) do %> + <% 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 %> + <% elsif cv_lce_disabled? %> + <%= hidden_field_tag 'host[content_facet_attributes][lifecycle_environment_id]', fetch_lifecycle_environment(@host).try(:id) %> + <%= select_tag env_select_id, lifecycle_environment_options(@host, :selected_host_group => @hostgroup || @host.hostgroup, :include_blank => blank_or_inherit_with_id(f, :lifecycle_environment)), :class => 'form-control', :name => env_select_name, :disabled => true %> + <% else %> + <%= select_tag env_select_id, lifecycle_environment_options(@host, :selected_host_group => @hostgroup || @host.hostgroup, :include_blank => blank_or_inherit_with_id(f, :lifecycle_environment)), :class => 'form-control', :name => env_select_name %> + <% 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.single_content_view' %> -<%= field(f, cv_select_attr, {:label => _("Content View")}) do - 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 - select_tag cv_select_id, content_views_for_host(@host, :selected_host_group => @hostgroup || @host.hostgroup, :include_blank => blank_or_inherit_with_id(f, :content_view)), :data => {"spinner_path" => spinner_path}, :class => 'form-control', :name => cv_select_name, :disabled => cv_lce_disabled? - end -end %> +<%= field(f, cv_select_attr, {:label => _("Content View")}) do %> + <% 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 %> + <% elsif cv_lce_disabled? %> + <%= hidden_field_tag 'host[content_facet_attributes][content_view_id]', fetch_content_view(@host).try(:id) %> + <%= select_tag cv_select_id, content_views_for_host(@host, :selected_host_group => @hostgroup || @host.hostgroup, :include_blank => blank_or_inherit_with_id(f, :content_view)), :data => {"spinner_path" => spinner_path}, :class => 'form-control', :name => cv_select_name, :disabled => true %> + <% else %> + <%= select_tag cv_select_id, content_views_for_host(@host, :selected_host_group => @hostgroup || @host.hostgroup, :include_blank => blank_or_inherit_with_id(f, :content_view)), :data => {"spinner_path" => spinner_path}, :class => 'form-control', :name => cv_select_name %> + <% end %> +<% end %> diff --git a/lib/katello/engine.rb b/lib/katello/engine.rb index 2c67d94b1d3..7c452b9e2ef 100644 --- a/lib/katello/engine.rb +++ b/lib/katello/engine.rb @@ -156,7 +156,6 @@ class Engine < ::Rails::Engine #Controller extensions ::HostsController.include Katello::Concerns::HostsControllerExtensions - ::HostsController.include Katello::Concerns::ContentFacetHostsControllerExtensions ::SmartProxiesController.include Katello::Concerns::SmartProxiesControllerExtensions ::RegistrationCommandsController.prepend Katello::Concerns::RegistrationCommandsControllerExtensions @@ -194,7 +193,6 @@ class Engine < ::Rails::Engine #Api controller extensions ::Api::V2::HostsController.include Katello::Concerns::Api::V2::HostsControllerExtensions - ::Api::V2::HostsController.include Katello::Concerns::ContentFacetHostsControllerExtensions ::Api::V2::HostsBulkActionsController.include Katello::Concerns::Api::V2::HostsBulkActionsControllerExtensions ::Api::V2::HostgroupsController.include Katello::Concerns::Api::V2::HostgroupsControllerExtensions ::Api::V2::SmartProxiesController.include Katello::Concerns::Api::V2::SmartProxiesControllerExtensions diff --git a/test/controllers/api/v2/hosts_controller_test.rb b/test/controllers/api/v2/hosts_controller_test.rb index 7ed9e25a579..4d88ca5744e 100644 --- a/test/controllers/api/v2/hosts_controller_test.rb +++ b/test/controllers/api/v2/hosts_controller_test.rb @@ -80,7 +80,7 @@ def test_host_update_with_env_only :lifecycle_environment_id => @dev.id } }, session: set_session_user - assert_response :error + assert_response 422 end def test_host_update_with_cv_only @@ -92,7 +92,7 @@ def test_host_update_with_cv_only :content_view_id => @cv2.id } }, session: set_session_user - assert_response :error + assert_response 422 end def test_host_update_with_invalid_env