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.

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

Fixes #36878 - Override host bulk destroy #10782

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +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
Expand Down
47 changes: 0 additions & 47 deletions app/controllers/katello/concerns/api/v2/bulk_hosts_extensions.rb

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module Katello
module Concerns
# overrides Foreman Api::V2::HostsBulkActionsController
module Api::V2::HostsBulkActionsControllerExtensions
extend ActiveSupport::Concern
require 'active_support/core_ext/string/inflections'

module Overrides
def bulk_destroy
destroyed_count = @hosts.count
@hosts.in_batches.each_record do |host|
Katello::RegistrationManager.unregister_host(host, :unregistering => false)
end
process_response(true, { :message => _("Deleted %{host_count} %{hosts}") % { :host_count => destroyed_count, :hosts => 'host'.pluralize(destroyed_count) }})
end
end

included do
prepend Overrides
end
end
end
end
2 changes: 1 addition & 1 deletion app/controllers/katello/remote_execution_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/katello/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ 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
::Api::V2::RegistrationController.include ::Foreman::Controller::SmartProxyAuth
Expand Down
141 changes: 4 additions & 137 deletions test/controllers/api/v2/concerns/bulk_hosts_extensions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

module Katello
class TestController
include Concerns::Api::V2::BulkHostsExtensions
# this now tests the bulk hosts extension that was moved to Foreman
include ::Api::V2::BulkHostsExtension

def initialize(params = {})
@params = params
Expand Down Expand Up @@ -35,48 +36,8 @@ def setup
@controller = TestController.new(organization_id: @organization.id)
end

def test_search
bulk_params = {
:included => {
:search => "name = #{@host1.name}"
}
}
result = @controller.find_bulk_hosts(@edit, bulk_params)

assert_equal result, [@host1]
end

def test_search_restrict
bulk_params = {
:included => {
:search => "name ~ host"
}
}
restrict = lambda { |hosts| hosts.where("id != #{@host2.id}") }
result = @controller.find_bulk_hosts(@edit, bulk_params, restrict)

assert_includes result, @host1
refute_includes result, @host2
assert_includes result, @host3
end

def test_search_exclude
bulk_params = {
:included => {
:search => "name ~ host"
},
:excluded => {
:ids => [@host1.id]
}
}
result = @controller.find_bulk_hosts(@edit, bulk_params)

refute_includes result, @host1
assert_includes result, @host2
assert_includes result, @host3
end

def test_select_all_hosts_for_errata_apply
def test_select_all_hosts_for_errata_apply_bastion
# bastion sends the install_all param
@controller.instance_variable_set(
:@params,
{
Expand All @@ -86,99 +47,5 @@ def test_select_all_hosts_for_errata_apply

assert_equal_arrays [@host1, @host2, @host3, @host4, @host5, @host6], result
end

def test_no_hosts_specified
bulk_params = {
:included => {}
}

assert_raises(HttpErrors::BadRequest) do
@controller.find_bulk_hosts(@edit, bulk_params)
end
end

def test_ids
bulk_params = {
:included => {
:ids => [@host1.id, @host2.id]
}
}
result = @controller.find_bulk_hosts(@edit, bulk_params)

assert_equal [@host1, @host2].sort, result.sort
end

def test_ids_excluded
bulk_params = {
:included => {
:ids => [@host1.id, @host2.id]
},
:excluded => {
:ids => [@host2.id]
}
}
result = @controller.find_bulk_hosts(@edit, bulk_params)

assert_equal result, [@host1]
end

def test_ids_restricted
bulk_params = {
:included => {
:ids => [@host1.id, @host2.id]
}
}
restrict = lambda { |hosts| hosts.where("id != #{@host2.id}") }
result = @controller.find_bulk_hosts(@edit, bulk_params, restrict)

assert_equal result, [@host1]
end

def test_included_ids_with_nil_scoped_search
bulk_params = {
:included => {
:ids => [@host1.id, @host2.id],
:search => nil
}
}

result = @controller.find_bulk_hosts(@edit, bulk_params)

assert_equal [@host1, @host2].sort, result.sort
end

def test_ids_with_scoped_search
bulk_params = {
:included => {
:ids => [@host1.id, @host2.id],
:search => "name != #{@host2.name}"
}
}

result = @controller.find_bulk_hosts(@edit, bulk_params)

assert_equal result, [@host1]
end

def test_forbidden
bulk_params = {
:included => {
:ids => [@host1.id]
},
:excluded => {
:ids => [@host1.id]
}
}

assert_raises(HttpErrors::Forbidden) do
@controller.find_bulk_hosts(@edit, bulk_params)
end
end

def test_empty_params
assert_raises(HttpErrors::BadRequest) do
@controller.find_bulk_hosts(@edit, {})
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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 = hosts(:one)
@host2 = hosts(:two)
end

def test_bulk_destroy
Katello::RegistrationManager.expects(:unregister_host).twice
delete :bulk_destroy, params: { :search => "id ^ (#{[@host1.id, @host2.id].join(',')})" }
assert_response :success
end
end
end
6 changes: 1 addition & 5 deletions webpack/components/extensions/Hosts/ActionsBar/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { useContext } from 'react';
import { DropdownItem } from '@patternfly/react-core';
import { CubeIcon } from '@patternfly/react-icons';
import { translate as __ } from 'foremanReact/common/I18n';
import { foremanUrl } from 'foremanReact/common/helpers';
import { ForemanHostsIndexActionsBarContext } from 'foremanReact/components/HostsIndex';
Expand All @@ -20,18 +19,15 @@ const HostActionsBar = () => {
href = foremanUrl(`/change_host_content_source?search=${fetchBulkParams({})}`);
}

const title = __('Change content source');
return (
<>
<DropdownItem
ouiaId="change-content-s-dropdown-item"
key="change-content-source-dropdown-item"
title={title}
href={href}
isDisabled={selectedCount === 0}
icon={<CubeIcon />}
>
{title}
{__('Change content source')}
</DropdownItem>
</>
);
Expand Down
Loading