From f03e699ea076feb508c7e523a3f95a3bfc83f829 Mon Sep 17 00:00:00 2001 From: Jeremy Lenz Date: Thu, 26 Oct 2023 10:57:47 -0400 Subject: [PATCH] =?UTF-8?q?Fixes=20#36867=20-=20Add=20host=20delete=20&=20?= =?UTF-8?q?create=20=20=20Add=20bulk=20modal=20with=20bulk=20params=20=20?= =?UTF-8?q?=20add=20register/create=20buttons;=20fix=20links=20=20=20addre?= =?UTF-8?q?ss=20UX=20comments=20=20=20Remove=20icon=20from=20delete=20acti?= =?UTF-8?q?on=20in=20the=20toolbar=E2=80=99s=20kebab=20=20=20In=20the=20de?= =?UTF-8?q?lete=20modal=20as=20a=20primary=20button=20use=20just=20?= =?UTF-8?q?=E2=80=9CDelete=E2=80=9D=20=20=20=20(not=20delete=20host)=20=20?= =?UTF-8?q?=20To=20the=20top=20part:=20=20=20=20=20Add=20a=20kebab=20with?= =?UTF-8?q?=20legacy=20UI=20button=20=20=20Ensure=20the=20loading=20screen?= =?UTF-8?q?=20doesn't=20say=20'No=20Results'=20=20=20support=20foreman=5Fr?= =?UTF-8?q?emote=5Fexecution=20slot=20=20=20Refs=20#36867=20-=20move=20act?= =?UTF-8?q?ion=20to=20new=20controller=20=20=20move=20bulk=20hosts=20exten?= =?UTF-8?q?sion=20test=20to=20Foreman?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/api/base_controller.rb | 2 +- .../api/v2/hosts_bulk_actions_controller.rb | 35 ++++ app/controllers/api/v2/hosts_controller.rb | 1 + .../concerns/api/v2/bulk_hosts_extension.rb | 45 +++++ app/registries/foreman/access_permissions.rb | 1 + config/routes/api/puppet/api/v2.rb | 2 +- config/routes/api/v2.rb | 2 + .../concerns/bulk_hosts_extension_test.rb | 168 ++++++++++++++++++ test/integration/host_js_test.rb | 2 +- test/test_helper.rb | 4 + .../components/ConfirmModal/index.js | 30 +++- .../components/ConfirmModal/slice.js | 4 + .../HostDetails/ActionsBar/actions.js | 5 +- .../HostsIndex/BulkActions/bulkDelete.js | 88 +++++++++ .../BulkActions/bulkDeleteModal.scss | 8 + .../react_app/components/HostsIndex/index.js | 167 +++++++++++++++-- .../components/HostsIndex/index.scss | 15 ++ .../PF4/TableIndexPage/Table/Table.js | 4 +- .../PF4/TableIndexPage/Table/Table.test.js | 2 +- .../PF4/TableIndexPage/Table/TableHooks.js | 2 +- .../PF4/TableIndexPage/TableIndexPage.js | 24 ++- .../PF4/TableIndexPage/TableIndexPage.scss | 10 ++ .../routes/common/EmptyPage/index.js | 24 ++- 23 files changed, 603 insertions(+), 42 deletions(-) create mode 100644 app/controllers/api/v2/hosts_bulk_actions_controller.rb create mode 100644 app/controllers/concerns/api/v2/bulk_hosts_extension.rb create mode 100644 test/controllers/concerns/bulk_hosts_extension_test.rb create mode 100644 webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDelete.js create mode 100644 webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDeleteModal.scss create mode 100644 webpack/assets/javascripts/react_app/components/HostsIndex/index.scss diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 667670fbdb5..ee5cae45788 100644 --- a/app/controllers/api/base_controller.rb +++ b/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' diff --git a/app/controllers/api/v2/hosts_bulk_actions_controller.rb b/app/controllers/api/v2/hosts_bulk_actions_controller.rb new file mode 100644 index 00000000000..0b548e8b4a7 --- /dev/null +++ b/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 diff --git a/app/controllers/api/v2/hosts_controller.rb b/app/controllers/api/v2/hosts_controller.rb index 45fe9f5ce32..0d5f91ca81d 100644 --- a/app/controllers/api/v2/hosts_controller.rb +++ b/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 diff --git a/app/controllers/concerns/api/v2/bulk_hosts_extension.rb b/app/controllers/concerns/api/v2/bulk_hosts_extension.rb new file mode 100644 index 00000000000..b3a374d1dc5 --- /dev/null +++ b/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 diff --git a/app/registries/foreman/access_permissions.rb b/app/registries/foreman/access_permissions.rb index 8f998430b1e..e7893b63fef 100644 --- a/app/registries/foreman/access_permissions.rb +++ b/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, diff --git a/config/routes/api/puppet/api/v2.rb b/config/routes/api/puppet/api/v2.rb index 061f37e3dfe..013847c8e53 100644 --- a/config/routes/api/puppet/api/v2.rb +++ b/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 diff --git a/config/routes/api/v2.rb b/config/routes/api/v2.rb index 04526285595..db47e07f49c 100644 --- a/config/routes/api/v2.rb +++ b/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] diff --git a/test/controllers/concerns/bulk_hosts_extension_test.rb b/test/controllers/concerns/bulk_hosts_extension_test.rb new file mode 100644 index 00000000000..78ab343131d --- /dev/null +++ b/test/controllers/concerns/bulk_hosts_extension_test.rb @@ -0,0 +1,168 @@ +require 'test_helper' + +class BulkHostsExtensionTestController < ::Api::BaseController + include Api::Version2 + include ::Api::V2::BulkHostsExtension + + def initialize(params = {}) + @params = params + end + + attr_reader :params +end + +class BulkHostsExtensionTest < ActiveSupport::TestCase + def models + as_admin do + @organization = FactoryBot.create(:organization) + @host1 = FactoryBot.create(:host, :managed, :organization => @organization) + @host2 = FactoryBot.create(:host, :organization => @organization) + @host3 = FactoryBot.create(:host, :organization => @organization) + @host4 = FactoryBot.create(:host, :organization => @organization) + @host5 = FactoryBot.create(:host, :organization => @organization) + @host6 = FactoryBot.create(:host, :organization => @organization) + @host_ids = [@host1, @host2, @host3, @host4, @host5, @host6].map(&:id) + end + end + + def permissions + @edit = :edit_hosts + end + + def setup + # set_user + models + permissions + @controller = BulkHostsExtensionTestController.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 = ->(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_no_hosts_specified + bulk_params = { + :included => {}, + } + @controller.expects(:render_error).with(:custom_error, :status => :bad_request, :locals => { :message => _('No hosts have been specified') }) + @controller.find_bulk_hosts(@edit, bulk_params) + 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 = ->(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], + }, + } + @controller.expects(:render_error).with(:custom_error, :status => :forbidden, :locals => { :message => _('No hosts matched search, or action unauthorized for selected hosts.') }) + @controller.find_bulk_hosts(@edit, bulk_params) + end + + def test_empty_params + @controller.expects(:render_error).with(:custom_error, :status => :bad_request, :locals => { :message => _('No hosts have been specified') }) + @controller.find_bulk_hosts(@edit, {}) + end +end diff --git a/test/integration/host_js_test.rb b/test/integration/host_js_test.rb index dab16c91dfd..b2020fc2343 100644 --- a/test/integration/host_js_test.rb +++ b/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) diff --git a/test/test_helper.rb b/test/test_helper.rb index 9829a05562c..34cffb7ff56 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -129,6 +129,10 @@ def assert_sql_queries(num_of_queries, match = /SELECT/) ActiveSupport::Notifications.unsubscribe("sql.active_record") assert_equal num_of_queries, queries.size, "Expected #{num_of_queries} queries, but got #{queries.size}" end + + def assert_equal_arrays(array1, array2) + assert_equal array1.sort, array2.sort + end end class ActionView::TestCase diff --git a/webpack/assets/javascripts/react_app/components/ConfirmModal/index.js b/webpack/assets/javascripts/react_app/components/ConfirmModal/index.js index a0b6bc5adf0..d60b0593172 100644 --- a/webpack/assets/javascripts/react_app/components/ConfirmModal/index.js +++ b/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')} , @@ -49,12 +56,22 @@ const ConfirmModal = () => { , ]; + const direWarningCheckbox = ( + setDireWarningChecked(val)} + /> + ); + if (!isOpen) return null; return ( { titleIconVariant={isWarning ? 'warning' : null} {...modalProps} > - {message} + <> + {message} + {isDireWarning && direWarningCheckbox} + ); }; diff --git a/webpack/assets/javascripts/react_app/components/ConfirmModal/slice.js b/webpack/assets/javascripts/react_app/components/ConfirmModal/slice.js index cb5035755b6..41271e143e1 100644 --- a/webpack/assets/javascripts/react_app/components/ConfirmModal/slice.js +++ b/webpack/assets/javascripts/react_app/components/ConfirmModal/slice.js @@ -11,9 +11,11 @@ const confirmModalSlice = createSlice({ const { title = '', message = '', + id = 'app-confirm-modal', onConfirm = noop, onCancel = noop, isWarning = false, + isDireWarning = false, confirmButtonText = null, modalProps = {}, } = action.payload; @@ -21,10 +23,12 @@ const confirmModalSlice = createSlice({ isOpen: true, title, message, + id, onConfirm, onCancel, modalProps, isWarning, + isDireWarning, confirmButtonText, }; }, diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/actions.js b/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/actions.js index b8652bc102d..726aff33ec7 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/actions.js +++ b/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({ diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDelete.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDelete.js new file mode 100644 index 00000000000..e6d04bff2a8 --- /dev/null +++ b/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: ( + + ), + confirmButtonText: __('Delete'), + onConfirm: () => + dispatch( + APIActions.delete({ + url, + key: `BULK-HOSTS-DELETE`, + successToast, + errorToast, + handleSuccess: () => visit(foremanUrl('/new/hosts')), + }) + ), + message: ( + + + + ), + cascade: cascadeMessage(), + settings: ( + + {__('Provisioning settings')} + + ), + 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}' + )} + /> + ), + }) + ); +}; diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDeleteModal.scss b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/bulkDeleteModal.scss new file mode 100644 index 00000000000..2bea7a5a854 --- /dev/null +++ b/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; + } +} \ No newline at end of file diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/index.js b/webpack/assets/javascripts/react_app/components/HostsIndex/index.js index 0ffa832786c..5df6b4056a8 100644 --- a/webpack/assets/javascripts/react_app/components/HostsIndex/index.js +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/index.js @@ -1,8 +1,20 @@ import React, { createContext } from 'react'; +import { useHistory, Link } from 'react-router-dom'; import PropTypes from 'prop-types'; -import { useSelector, shallowEqual } from 'react-redux'; +import { useSelector, useDispatch, shallowEqual } from 'react-redux'; import { Td } from '@patternfly/react-table'; -import { ToolbarItem } from '@patternfly/react-core'; +import { + ToolbarItem, + Dropdown, + DropdownItem, + KebabToggle, + Flex, + FlexItem, + Button, + Split, + SplitItem, +} from '@patternfly/react-core'; +import { UndoIcon } from '@patternfly/react-icons'; import { translate as __ } from '../../common/I18n'; import TableIndexPage from '../PF4/TableIndexPage/TableIndexPage'; import { ActionKebab } from './ActionKebab'; @@ -12,6 +24,13 @@ import { useAPI } from '../../common/hooks/API/APIHooks'; import { useBulkSelect } from '../PF4/TableIndexPage/Table/TableHooks'; import SelectAllCheckbox from '../PF4/TableIndexPage/Table/SelectAllCheckbox'; import { getPageStats } from '../PF4/TableIndexPage/Table/helpers'; +import { deleteHost } from '../HostDetails/ActionsBar/actions'; +import { useForemanSettings } from '../../Root/Context/ForemanContext'; +import { getURIsearch } from '../../common/urlHelpers'; +import { bulkDeleteHosts } from './BulkActions/bulkDelete'; +import { foremanUrl } from '../../common/helpers'; +import Slot from '../common/Slot'; +import './index.scss'; export const ForemanHostsIndexActionsBarContext = createContext({}); @@ -19,11 +38,19 @@ const HostsIndex = () => { const columns = { name: { title: __('Name'), - wrapper: ({ id, name }) => {name}, + wrapper: ({ name }) => {name}, isSorted: true, }, }; - const defaultParams = { search: '' }; // search || + + const history = useHistory(); + const { location: { search: historySearch } = {} } = history || {}; + const urlParams = new URLSearchParams(historySearch); + const urlParamsSearch = urlParams.get('search') || ''; + const searchFromUrl = urlParamsSearch || getURIsearch(); + const initialSearchQuery = apiSearchQuery || searchFromUrl || ''; + + const defaultParams = { search: initialSearchQuery }; const response = useAPI('get', `${HOSTS_API_PATH}?include_permissions=true`, { key: API_REQUEST_KEY, @@ -34,6 +61,7 @@ const HostsIndex = () => { response: { search: apiSearchQuery, results, + subtotal, total, per_page: perPage, page, @@ -42,10 +70,14 @@ const HostsIndex = () => { const { pageRowCount } = getPageStats({ total, page, perPage }); - const { fetchBulkParams, ...selectAllOptions } = useBulkSelect({ + const { + fetchBulkParams, + updateSearchQuery, + ...selectAllOptions + } = useBulkSelect({ results, - metadata: { total, page }, - initialSearchQuery: apiSearchQuery || '', + metadata: { total, page, selectable: subtotal }, + initialSearchQuery, }); const { @@ -81,7 +113,7 @@ const HostsIndex = () => { select={{ rowIndex: rowData.id, onSelect: (_event, isSelecting) => { - selectOne(isSelecting, rowData.id); + selectOne(isSelecting, rowData.id, rowData); }, isSelected: isSelected(rowData.id), disable: false, @@ -92,24 +124,133 @@ const HostsIndex = () => { RowSelectTd.propTypes = { rowData: PropTypes.object.isRequired, }; + const dispatch = useDispatch(); + const { destroyVmOnHostDelete } = useForemanSettings(); + const deleteHostHandler = ({ hostName, computeId }) => + dispatch(deleteHost(hostName, computeId, destroyVmOnHostDelete)); + const handleBulkDelete = () => { + const bulkParams = fetchBulkParams(); + dispatch( + bulkDeleteHosts({ + bulkParams, + selectedCount, + destroyVmOnHostDelete, + }) + ); + }; - const actionNode = []; + const dropdownItems = [ + + {__('Delete')} + , + ]; const registeredItems = useSelector(selectKebabItems, shallowEqual); const customToolbarItems = ( - + ); + const rowKebabItems = ({ + id, + name: hostName, + compute_id: computeId, + canDelete, + }) => [ + { + title: __('Delete'), + onClick: () => deleteHostHandler({ id, hostName, computeId }), + isDisabled: !canDelete, + }, + ]; + + const [legacyUIKebabOpen, setLegacyUIKebabOpen] = React.useState(false); + const legacyUIKebab = ( + + } + isOpen={legacyUIKebabOpen} + isPlain + dropdownItems={[ + } + > + {__('Legacy UI')} + , + ]} + /> + ); + + const hostsIndexHeader = ( + + +

{__('Hosts')}

+ + + + + + + + + + + + + {legacyUIKebab} + + + + ); + return ( { selectionToolbar={selectionToolbar} showCheckboxes rowSelectTd={RowSelectTd} + rowKebabItems={rowKebabItems} + updateSearchQuery={updateSearchQuery} /> ); }; diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/index.scss b/webpack/assets/javascripts/react_app/components/HostsIndex/index.scss new file mode 100644 index 00000000000..08a9029bac8 --- /dev/null +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/index.scss @@ -0,0 +1,15 @@ +#legacy-ui-kebab ul.pf-c-dropdown__menu { + padding-left: 0; + li { + display: unset; + a { + font-size: 16px; + color: var(--pf-c-dropdown__menu-item--Color); + font-weight: 300; + } + } +} + +#header-text .pf-l-flex { + align-items: center; +} diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js index b6e889971c9..af7197023db 100644 --- a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js +++ b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js @@ -69,7 +69,7 @@ export const Table = ({ onClick: () => onDeleteClick({ id, name }), isDisabled: !canDelete, }, - getActions && getActions({ id, name, ...item }), + ...((getActions && getActions({ id, name, canDelete, ...item })) ?? []), ].filter(Boolean); const columnNamesKeys = Object.keys(columns); const RowSelectTd = rowSelectTd; @@ -105,7 +105,7 @@ export const Table = ({ diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.test.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.test.js index 471fda2e832..8a45ac45e16 100644 --- a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.test.js +++ b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.test.js @@ -205,7 +205,7 @@ describe('Table', () => { ); expect(screen.queryAllByText('items')).toHaveLength(0); - expect(screen.queryAllByText('No Results')).toHaveLength(1); + expect(screen.queryAllByText('No Results')).toHaveLength(0); expect(screen.queryAllByText('Loading...')).toHaveLength(1); }); }); diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableHooks.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableHooks.js index f00865dcbf7..1cd91788f3d 100644 --- a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableHooks.js +++ b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableHooks.js @@ -227,7 +227,7 @@ export const useBulkSelect = ({ const fetchBulkParams = ({ idColumnName = idColumn, selectAllQuery = '', - }) => { + } = {}) => { const searchQueryWithExclusionSet = () => { const query = [ searchQuery, diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/TableIndexPage.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/TableIndexPage.js index f81f1b58253..a154b50e61b 100644 --- a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/TableIndexPage.js +++ b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/TableIndexPage.js @@ -55,7 +55,8 @@ A page component that displays a table with data fetched from an API. It provide @param {Array} {customToolbarItems} - an array of custom toolbar items to be displayed @param {boolean} {exportable} - whether or not to show export button @param {boolean} {hasHelpPage} - whether or not to show documentation button -@param {string}{header} - the header text for the page +@param {string}{headerText} - the header text for the page +@param {string}{header} - header node; default is {headerText} @param {boolean} {isDeleteable} - whether or not entries can be deleted @param {boolean} {searchable} - whether or not the table can be searched @param {React.ReactNode} {children} - optional children to be rendered inside the page instead of the table @@ -77,6 +78,7 @@ const TableIndexPage = ({ customToolbarItems, exportable, hasHelpPage, + headerText, header, isDeleteable, searchable, @@ -85,6 +87,8 @@ const TableIndexPage = ({ replacementResponse, showCheckboxes, rowSelectTd, + rowKebabItems, + updateSearchQuery, }) => { const history = useHistory(); const { location: { search: historySearch } = {} } = history || {}; @@ -153,6 +157,7 @@ const TableIndexPage = ({ const setSearch = newSearch => { const uri = new URI(); uri.setSearch(newSearch); + updateSearchQuery(newSearch.search); history.push({ search: uri.search() }); setParamsAndAPI({ ...params, ...newSearch }); }; @@ -185,9 +190,7 @@ const TableIndexPage = ({ return (
- - {header} - + {headerText} {breadcrumbOptions && ( @@ -199,7 +202,7 @@ const TableIndexPage = ({ > - {header} + {header ?? {headerText}} @@ -259,6 +262,7 @@ const TableIndexPage = ({ ( - -); +const EmptyPage = ({ message: { type, text } }) => { + const headerTextMap = { + empty: __('No Results'), + error: __('Error'), + loading: __('Loading'), + }; + const headerText = headerTextMap[type]; + return ( + + ); +}; EmptyPage.propTypes = { message: PropTypes.shape({ - type: PropTypes.oneOf(['empty', 'error']), + type: PropTypes.oneOf(['empty', 'error', 'loading']), text: PropTypes.string, }), };