Fixes #36878 - Override host bulk destroy

[jeremylenz]

What are the changes introduced in this pull request?

We're adding bulk host deletion to the Foreman V2 API, to support the new All Hosts index page. Need to add overrides to Katello to ensure hosts are unregistered and artifacts are properly destroyed.

Also includes some misc. items:

• removed the icon from the 'Change content source' menu item per UX guidance
• Consume bulk host extensions from Foreman instead of duplicating it in Katello

Considerations taken when implementing this change?

What are the testing steps for this pull request?

Check out theforeman/foreman#9878
Delete some hosts and ensure they're properly unregistered and artifacts removed (content facet, etc. -check registration_manager.rb#remove_host_artifacts)

[parthaa]

APJ

[parthaa]

Please remove bulk_hosts_extensions.rb and move test/controllers/api/v2/hosts_bulk_actions_controller_extensions_test.rb to foreman

From 392797402ad34213ace9ac571dcde08a06db0050 Mon Sep 17 00:00:00 2001
From: Partha Aji <[email protected]>
Date: Fri, 3 Nov 2023 16:44:49 -0400
Subject: [PATCH] more

more
---
 .../v2/content_view_versions_controller.rb    |  2 +-
 .../api/v2/hosts_bulk_actions_controller.rb   |  2 +-
 .../concerns/api/v2/bulk_hosts_extensions.rb  | 47 -------------------
 .../katello/remote_execution_controller.rb    |  2 +-
 .../v2/concerns/bulk_hosts_extensions_test.rb |  2 +-
 ...bulk_actions_controller_extensions_test.rb |  7 +--
 6 files changed, 8 insertions(+), 54 deletions(-)
 delete mode 100644 app/controllers/katello/concerns/api/v2/bulk_hosts_extensions.rb

diff --git a/app/controllers/katello/api/v2/content_view_versions_controller.rb b/app/controllers/katello/api/v2/content_view_versions_controller.rb
index 662292d13d..851813dccb 100644
--- a/app/controllers/katello/api/v2/content_view_versions_controller.rb
+++ b/app/controllers/katello/api/v2/content_view_versions_controller.rb
@@ -1,6 +1,6 @@
 module Katello
   class Api::V2::ContentViewVersionsController < Api::V2::ApiController
-    include Concerns::Api::V2::BulkHostsExtensions
+    include ::Api::V2::BulkHostsExtension
     include Katello::Concerns::FilteredAutoCompleteSearch
 
     before_action :find_authorized_katello_resource, :only => [:show, :update, :promote, :destroy, :republish_repositories]
diff --git a/app/controllers/katello/api/v2/hosts_bulk_actions_controller.rb b/app/controllers/katello/api/v2/hosts_bulk_actions_controller.rb
index c63c2a96e1..155646db65 100644
--- a/app/controllers/katello/api/v2/hosts_bulk_actions_controller.rb
+++ b/app/controllers/katello/api/v2/hosts_bulk_actions_controller.rb
@@ -1,7 +1,7 @@
 module Katello
   # this is Katello's host bulk actions controller, not to be confused with Foreman's
   class Api::V2::HostsBulkActionsController < Api::V2::ApiController
-    include Concerns::Api::V2::BulkHostsExtensions
+    include ::Api::V2::BulkHostsExtension
     include Katello::Concerns::Api::V2::ContentOverridesController
     include Katello::ContentSourceHelper
     include ::Foreman::Renderer::Scope::Macros::Base
diff --git a/app/controllers/katello/concerns/api/v2/bulk_hosts_extensions.rb b/app/controllers/katello/concerns/api/v2/bulk_hosts_extensions.rb
deleted file mode 100644
index a90b7a2d6b..0000000000
--- a/app/controllers/katello/concerns/api/v2/bulk_hosts_extensions.rb
+++ /dev/null
@@ -1,47 +0,0 @@
-module Katello
-  module Concerns
-    module Api::V2::BulkHostsExtensions
-      extend ActiveSupport::Concern
-
-      def bulk_hosts_relation(permission, org)
-        relation = ::Host::Managed.authorized(permission)
-        relation = relation.where(organization: org) if org
-        relation
-      end
-
-      def find_bulk_hosts(permission, bulk_params, restrict_to = nil)
-        #works on a structure of param_group bulk_params and transforms it into a list of systems
-        bulk_params[:included] ||= {}
-        bulk_params[:excluded] ||= {}
-
-        if !params[:install_all] && bulk_params[:included][:ids].blank? && bulk_params[:included][:search].nil?
-          fail HttpErrors::BadRequest, _("No hosts have been specified.")
-        end
-
-        find_organization
-        @hosts = bulk_hosts_relation(permission, @organization)
-
-        if bulk_params[:included][:ids].present?
-          @hosts = @hosts.where(id: bulk_params[:included][:ids])
-        end
-
-        if bulk_params[:included][:search].present?
-          @hosts = @hosts.search_for(bulk_params[:included][:search])
-        end
-
-        @hosts = restrict_to.call(@hosts) if restrict_to
-
-        if bulk_params[:excluded][:ids].present?
-          @hosts = @hosts.where.not(id: bulk_params[:excluded][:ids])
-        end
-        fail HttpErrors::Forbidden, _("No hosts matched search, or action unauthorized for selected hosts.") if @hosts.empty?
-
-        @hosts
-      end
-
-      def find_organization
-        @organization ||= Organization.find_by_id(params[:organization_id])
-      end
-    end
-  end
-end
diff --git a/app/controllers/katello/remote_execution_controller.rb b/app/controllers/katello/remote_execution_controller.rb
index 08b61ea550..909e2b54dc 100644
--- a/app/controllers/katello/remote_execution_controller.rb
+++ b/app/controllers/katello/remote_execution_controller.rb
@@ -1,7 +1,7 @@
 module Katello
   if Katello.with_remote_execution?
     class RemoteExecutionController < JobInvocationsController
-      include Concerns::Api::V2::BulkHostsExtensions
+      include ::Api::V2::BulkHostsExtension
       include Concerns::Api::V2::HostErrataExtensions
 
       def new
diff --git a/test/controllers/api/v2/concerns/bulk_hosts_extensions_test.rb b/test/controllers/api/v2/concerns/bulk_hosts_extensions_test.rb
index 5de02ac5a6..be23dce337 100644
--- a/test/controllers/api/v2/concerns/bulk_hosts_extensions_test.rb
+++ b/test/controllers/api/v2/concerns/bulk_hosts_extensions_test.rb
@@ -4,7 +4,7 @@ require "katello_test_helper"
 
 module Katello
   class TestController
-    include Concerns::Api::V2::BulkHostsExtensions
+    include ::Api::V2::BulkHostsExtension
 
     def initialize(params = {})
       @params = params
diff --git a/test/controllers/api/v2/hosts_bulk_actions_controller_extensions_test.rb b/test/controllers/api/v2/hosts_bulk_actions_controller_extensions_test.rb
index cc2ff2fbd7..a18a51ad75 100644
--- a/test/controllers/api/v2/hosts_bulk_actions_controller_extensions_test.rb
+++ b/test/controllers/api/v2/hosts_bulk_actions_controller_extensions_test.rb
@@ -3,9 +3,10 @@ require 'katello_test_helper'
 module Katello
   # testing Katello's overrides of Foreman's HostsBulkActionsController
   class Api::V2::HostsBulkActionsControllerExtensionsTest < ActionController::TestCase
+    tests ::Api::V2::HostsBulkActionsController
     def setup
-      @host1 = FactoryBot.create(:host, :with_subscription, :with_content, :organization => @view.organization, :content_view => @view, :lifecycle_environment => @library)
-      @host2 = FactoryBot.create(:host, :with_subscription, :with_content, :organization => @view.organization, :content_view => @view, :lifecycle_environment => @library)
+      @host1 = hosts(:one)
+      @host2 = hosts(:two)
     end
 
     def test_bulk_destroy
@@ -14,4 +15,4 @@ module Katello
       assert_response :success
     end
   end
-end
+end
\ No newline at end of file
-- 
2.41.0

[jeremylenz]

looks like tests will be failing until the Foreman PR is merged, since that's what adds ::Api::V2::HostsBulkActionsController

[jeremylenz]

[test katello]