Fixes #36867 - Add host delete & create

  Add bulk modal with bulk params
  add register/create buttons; fix links
  address UX comments
  Remove icon from delete action in the toolbar’s kebab
  In the delete modal as a primary button use just “Delete”
   (not delete host)
  To the top part:
    Add a kebab with legacy UI button
  Ensure the loading screen doesn't say 'No Results'
  support foreman_remote_execution slot
  Refs #36867 - move action to new controller

app/controllers/api/base_controller.rb

@@ -387,7 +387,7 @@ def action_permission
         'create'
       when 'edit', 'update'
         'edit'
-      when 'destroy'
+      when 'destroy', 'bulk_destroy'
         'destroy'
       when 'index', 'show', 'status'
         'view'

app/controllers/api/v2/hosts_bulk_actions_controller.rb

@@ -0,0 +1,35 @@
+module Api
+  module V2
+    class HostsBulkActionsController < V2::BaseController
+      include Api::Version2
+      include Api::V2::BulkHostsExtension
+
+      before_action :find_deletable_hosts, :only => [:bulk_destroy]
+
+      def_param_group :bulk_host_ids do
+        param :organization_id, :number, :required => true, :desc => N_("ID of the organization")
+        param :included, Hash, :desc => N_("Hosts to include in the action"), :required => true, :action_aware => true do
+          param :search, String, :required => false, :desc => N_("Search string describing which hosts to perform the action on")
+          param :ids, Array, :required => false, :desc => N_("List of host ids to perform the action on")
+        end
+        param :excluded, Hash, :desc => N_("Hosts to explicitly exclude in the action."\
+                                           " All other hosts will be included in the action,"\
+                                           " unless an included parameter is passed as well."), :required => true, :action_aware => true do
+          param :ids, Array, :required => false, :desc => N_("List of host ids to exclude and not perform the action on")
+        end
+      end
+
+      api :DELETE, "/hosts/bulk/", N_("Delete multiple hosts")
+      param_group :bulk_host_ids
+      def bulk_destroy
+        process_response @hosts.destroy_all
+      end
+
+      private
+
+      def find_deletable_hosts
+        find_bulk_hosts(:destroy_hosts, params)
+      end
+    end
+  end
+end

app/controllers/api/v2/hosts_controller.rb

@@ -2,6 +2,7 @@ module Api
   module V2
     class HostsController < V2::BaseController
       include Api::Version2
+      include Api::V2::BulkHostsExtension
       include ScopesPerAction
       include Foreman::Controller::SmartProxyAuth
       include Foreman::Controller::Parameters::Host

app/controllers/concerns/api/v2/bulk_hosts_extension.rb

@@ -0,0 +1,45 @@
+module Api::V2::BulkHostsExtension
+  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] ||= {}
+    search_param = bulk_params[:included][:search] || bulk_params[:search]
+
+    if !params[:install_all] && bulk_params[:included][:ids].blank? && search_param.nil?
+      render_error :custom_error, :status => :bad_request, :locals => { :message => _('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 search_param.present?
+      @hosts = @hosts.search_for(search_param)
+    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
+    if @hosts.empty?
+      render_error :custom_error, :status => :forbidden, :locals => { :message => _('No hosts matched search, or action unauthorized for selected hosts.') }
+    end
+    @hosts
+  end
+
+  def find_organization
+    @organization ||= Organization.find_by_id(params[:organization_id])
+  end
+end

app/registries/foreman/access_permissions.rb

@@ -275,6 +275,7 @@
                                   }
     map.permission :destroy_hosts, {:hosts => [:destroy, :multiple_actions, :reset_multiple, :multiple_destroy, :submit_multiple_destroy],
                                     :"api/v2/hosts" => [:destroy],
+                                    :"api/v2/hosts_bulk_actions" => [:bulk_destroy],
                                     :"api/v2/interfaces" => [:destroy],
                                   }
     map.permission :build_hosts,   {:hosts => [:setBuild, :cancelBuild, :multiple_build, :submit_multiple_build, :review_before_build,

config/routes/api/puppet/api/v2.rb

@@ -3,7 +3,7 @@
     # new v2 routes that point to v2
     scope "(:apiv)", :module => :v2, :defaults => {:apiv => 'v2'}, :apiv => /v2/, :constraints => ApiConstraints.new(:version => 2, :default => true) do
       constraints(:id => /[^\/]+/) do
-        resources :hosts, :except => [:new, :edit] do
+        resources :hosts, :only => [] do # only: [] to avoid adding other api/v2/hosts routes
           put :puppetrun, :on => :member, :to => 'puppet_hosts#puppetrun'
         end
       end

config/routes/api/v2.rb

@@ -3,6 +3,8 @@
   namespace :api, :defaults => {:format => 'json'} do
     # new v2 routes that point to v2
     scope "(:apiv)", :module => :v2, :defaults => {:apiv => 'v2'}, :apiv => /v2/, :constraints => ApiConstraints.new(:version => 2, :default => true) do
+      match 'hosts/bulk', :to => 'hosts_bulk_actions#bulk_destroy', :via => [:delete]
+
       resources :architectures, :except => [:new, :edit] do
         constraints(:id => /[^\/]+/) do
           resources :hosts, :except => [:new, :edit]

test/integration/host_js_test.rb

@@ -112,7 +112,7 @@ class HostJSTest < IntegrationTestWithJavascript
       visit host_details_page_path(host)
       find('#hostdetails-kebab').click
       click_button 'Delete'
-      click_button 'Delete host'
+      find('button.pf-c-button.pf-m-danger').click # the red delete button, not the menu item
       assert_current_path hosts_path
       assert_raises(ActiveRecord::RecordNotFound) do
         Host.find(host.id)

webpack/assets/javascripts/react_app/components/ConfirmModal/index.js

@@ -1,11 +1,12 @@
-import React from 'react';
+import React, { useState } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
-import { Modal, Button, ModalVariant } from '@patternfly/react-core';
+import { Modal, Button, ModalVariant, Checkbox } from '@patternfly/react-core';
 import { translate as __ } from '../../common/I18n';
 import { closeConfirmModal, selectConfirmModal } from './slice';
 
 const ConfirmModal = () => {
   const {
+    id,
     isOpen,
     title,
     message,
@@ -14,11 +15,16 @@ const ConfirmModal = () => {
     onCancel,
     modalProps,
     isWarning,
+    isDireWarning,
   } = useSelector(selectConfirmModal);
 
   const dispatch = useDispatch();
+  const [direWarningChecked, setDireWarningChecked] = useState(false);
 
-  const closeModal = () => dispatch(closeConfirmModal());
+  const closeModal = () => {
+    setDireWarningChecked(false);
+    return dispatch(closeConfirmModal());
+  };
 
   const handleCancel = () => {
     onCancel();
@@ -36,6 +42,7 @@ const ConfirmModal = () => {
       variant={isWarning ? 'danger' : 'primary'}
       onClick={handleConfirm}
       ouiaId="btn-modal-confirm"
+      isDisabled={isDireWarning && !direWarningChecked}
     >
       {confirmButtonText || __('Confirm')}
     </Button>,
@@ -49,12 +56,22 @@ const ConfirmModal = () => {
     </Button>,
   ];
 
+  const direWarningCheckbox = (
+    <Checkbox
+      id="dire-warning-checkbox"
+      ouiaId="dire-warning-checkbox"
+      label={__('I understand that this action cannot be undone.')}
+      isChecked={direWarningChecked}
+      onChange={val => setDireWarningChecked(val)}
+    />
+  );
+
   if (!isOpen) return null;
 
   return (
     <Modal
       ouiaId="app-confirm-modal"
-      id="app-confirm-modal"
+      id={id ?? 'app-confirm-modal'}
       aria-label="application confirm modal"
       variant={ModalVariant.small}
       title={title}
@@ -64,7 +81,10 @@ const ConfirmModal = () => {
       titleIconVariant={isWarning ? 'warning' : null}
       {...modalProps}
     >
-      {message}
+      <>
+        {message}
+        {isDireWarning && direWarningCheckbox}
+      </>
     </Modal>
   );
 };

webpack/assets/javascripts/react_app/components/ConfirmModal/slice.js

@@ -11,20 +11,24 @@ const confirmModalSlice = createSlice({
       const {
         title = '',
         message = '',
+        id = 'app-confirm-modal',
         onConfirm = noop,
         onCancel = noop,
         isWarning = false,
+        isDireWarning = false,
         confirmButtonText = null,
         modalProps = {},
       } = action.payload;
       return {
         isOpen: true,
         title,
         message,
+        id,
         onConfirm,
         onCancel,
         modalProps,
         isWarning,
+        isDireWarning,
         confirmButtonText,
       };
     },

webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/actions.js

@@ -14,8 +14,7 @@ export const deleteHost = (
   compute,
   destroyVmOnHostDelete
 ) => dispatch => {
-  const successToast = () =>
-    sprintf(__('Host %s has been removed successfully'), hostName);
+  const successToast = () => sprintf(__('Host %s deleted'), hostName);
   const errorToast = ({ message }) => message;
   const url = foremanUrl(`/api/hosts/${hostName}`);
 
@@ -37,7 +36,7 @@ export const deleteHost = (
     openConfirmModal({
       isWarning: true,
       title: __('Delete host?'),
-      confirmButtonText: __('Delete host'),
+      confirmButtonText: __('Delete'),
       onConfirm: () =>
         dispatch(
           APIActions.delete({

webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDelete.js

@@ -0,0 +1,88 @@
+import React from 'react';
+import { FormattedMessage } from 'react-intl';
+import { visit } from '../../../../foreman_navigation';
+import { foremanUrl } from '../../../common/helpers';
+import { sprintf, translate as __ } from '../../../common/I18n';
+import { openConfirmModal } from '../../ConfirmModal';
+import { APIActions } from '../../../redux/API';
+import './bulkDeleteModal.scss';
+
+export const bulkDeleteHosts = ({
+  bulkParams,
+  selectedCount,
+  destroyVmOnHostDelete,
+}) => dispatch => {
+  const successToast = () => sprintf(__('%s hosts deleted'), selectedCount);
+  const errorToast = ({ message }) => message;
+  const url = foremanUrl(`/api/v2/hosts/bulk?search=${bulkParams}`);
+
+  // TODO: Replace with a checkbox instead of a global setting for cascade host destroy
+  const cascadeMessage = () =>
+    destroyVmOnHostDelete
+      ? __(
+          'For hosts with compute resources, this will delete the VM and its disks.'
+        )
+      : __(
+          'For hosts with compute resources, VMs and their disks will not be deleted.'
+        );
+
+  dispatch(
+    openConfirmModal({
+      isWarning: true,
+      isDireWarning: true,
+      id: 'bulk-delete-hosts-modal',
+      title: (
+        <FormattedMessage
+          defaultMessage="Delete {count, plural, one {{singular}} other {{plural}}}?"
+          values={{
+            count: selectedCount,
+            singular: __('host'),
+            plural: __('hosts'),
+          }}
+          id="bulk-delete-host-count"
+        />
+      ),
+      confirmButtonText: __('Delete'),
+      onConfirm: () =>
+        dispatch(
+          APIActions.delete({
+            url,
+            key: `BULK-HOSTS-DELETE`,
+            successToast,
+            errorToast,
+            handleSuccess: () => visit(foremanUrl('/new/hosts')),
+          })
+        ),
+      message: (
+        <FormattedMessage
+          id="bulk-delete-hosts"
+          values={{
+            hostsCount: (
+              <strong>
+                <FormattedMessage
+                  defaultMessage="{count, plural, one {# {singular}} other {# {plural}}}"
+                  values={{
+                    count: selectedCount,
+                    singular: __('host'),
+                    plural: __('hosts'),
+                  }}
+                  id="bulk-delete-host-count"
+                />
+              </strong>
+            ),
+            cascade: cascadeMessage(),
+            settings: (
+              <a href={foremanUrl('/settings?search=destroy')}>
+                {__('Provisioning settings')}
+              </a>
+            ),
+            br: <br />,
+          }}
+          defaultMessage={__(
+            '{hostsCount} will be deleted. This action is irreversible. {br}{br} {cascade} {br}{br} This behavior can be changed via global setting "Destroy associated VM on host delete" in {settings}.{br}{br}'
+          )}
+        />
+      ),
+    })
+  );
+};

webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDeleteModal.scss

@@ -0,0 +1,8 @@
+#bulk-delete-hosts-modal {
+  min-height: 21rem;
+  .pf-c-check label {
+    font-size: 14px;
+    position: relative;
+    top: 2px;
+  }
+}