diff --git a/app/controllers/api/v2/hosts_bulk_actions_controller.rb b/app/controllers/api/v2/hosts_bulk_actions_controller.rb index 3287ce4c11..5501070e2f 100644 --- a/app/controllers/api/v2/hosts_bulk_actions_controller.rb +++ b/app/controllers/api/v2/hosts_bulk_actions_controller.rb @@ -5,7 +5,7 @@ class HostsBulkActionsController < V2::BaseController include Api::V2::BulkHostsExtension before_action :find_deletable_hosts, :only => [:bulk_destroy] - before_action :find_editable_hosts, :only => [:build, :reassign_hostgroup] + before_action :find_editable_hosts, :only => [:build, :reassign_hostgroup, :change_owner] def_param_group :bulk_host_ids do param :organization_id, :number, :required => true, :desc => N_("ID of the organization") @@ -78,6 +78,14 @@ def reassign_hostgroup end end + api :PUT, "/hosts/bulk/change_owner", N_("Change owner") + param_group :bulk_host_ids + param :owner_id, :number, :required => true, :desc => N_("ID of the owner to reassign the hosts to") + def change_owner + BulkHostsManager.new(hosts: @hosts).change_owner(params[:owner_id]) + process_response(true, { :message => n_("Updated host: changed owner", "Updated hosts: changed owner", @hosts.count)}) + end + protected def action_permission diff --git a/app/controllers/hosts_controller.rb b/app/controllers/hosts_controller.rb index 1021888efc..9906a4aa49 100644 --- a/app/controllers/hosts_controller.rb +++ b/app/controllers/hosts_controller.rb @@ -453,10 +453,7 @@ def update_multiple_owner end # update the hosts - @hosts.each do |host| - host.is_owned_by = id - host.save(:validate => false) - end + BulkHostsManager.new(hosts: @hosts).change_owner(id) success _('Updated hosts: changed owner') redirect_back_or_to helpers.current_hosts_path diff --git a/app/services/bulk_hosts_manager.rb b/app/services/bulk_hosts_manager.rb index ef93a27f66..c9c585a608 100644 --- a/app/services/bulk_hosts_manager.rb +++ b/app/services/bulk_hosts_manager.rb @@ -38,4 +38,11 @@ def rebuild_configuration end all_fails end + + def change_owner(owner_id) + @hosts.each do |host| + host.is_owned_by = owner_id + host.save(:validate => false) + end + end end diff --git a/config/initializers/f_foreman_permissions.rb b/config/initializers/f_foreman_permissions.rb index 6d29d1a8f1..99c6f5c047 100644 --- a/config/initializers/f_foreman_permissions.rb +++ b/config/initializers/f_foreman_permissions.rb @@ -271,7 +271,7 @@ :"api/v2/hosts" => [:update, :disassociate, :forget_status], :"api/v2/interfaces" => [:create, :update, :destroy], :"api/v2/compute_resources" => [:associate], - :"api/v2/hosts_bulk_actions" => [:build, :reassign_hostgroup], + :"api/v2/hosts_bulk_actions" => [:build, :reassign_hostgroup, :change_owner], } map.permission :destroy_hosts, {:hosts => [:destroy, :multiple_actions, :reset_multiple, :multiple_destroy, :submit_multiple_destroy], :"api/v2/hosts" => [:destroy], diff --git a/config/routes/api/v2.rb b/config/routes/api/v2.rb index 74ce286d37..010b08f808 100644 --- a/config/routes/api/v2.rb +++ b/config/routes/api/v2.rb @@ -5,6 +5,7 @@ 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] match 'hosts/bulk/build', :to => 'hosts_bulk_actions#build', :via => [:put] + match 'hosts/bulk/change_owner', :to => 'hosts_bulk_actions#change_owner', :via => [:put] match 'hosts/bulk/reassign_hostgroup', :to => 'hosts_bulk_actions#reassign_hostgroup', :via => [:put] resources :architectures, :except => [:new, :edit] do diff --git a/test/controllers/api/v2/hosts_bulk_actions_controller_test.rb b/test/controllers/api/v2/hosts_bulk_actions_controller_test.rb new file mode 100644 index 0000000000..a9722daee5 --- /dev/null +++ b/test/controllers/api/v2/hosts_bulk_actions_controller_test.rb @@ -0,0 +1,111 @@ +require 'test_helper' + +class Api::V2::HostsBulkActionsControllerTest < ActionController::TestCase + def setup + as_admin do + @organization = FactoryBot.create(:organization) + @location = FactoryBot.create(:location) + @host1 = FactoryBot.create(:host, :managed, :organization => @organization, :location => @location) + @host2 = FactoryBot.create(:host, :managed, :organization => @organization, :location => @location) + @host3 = FactoryBot.create(:host, :managed, :organization => @organization, :location => @location) + @user = FactoryBot.create(:user, :organizations => [@organization], :locations => [@location]) + @usergroup = FactoryBot.create(:usergroup) + @host_ids = [@host1.id, @host2.id, @host3.id] + end + end + + def valid_bulk_params(host_ids = @host_ids) + { + :organization_id => @organization.id, + :included => { + :ids => host_ids, + }, + :excluded => { + :ids => [], + }, + } + end + + test "should change owner with user id" do + put :change_owner, params: valid_bulk_params.merge(:owner_id => @user.id_and_type) + + assert_response :success + response = ActiveSupport::JSON.decode(@response.body) + assert_match(/Updated hosts: changed owner/, response['message']) + + [@host1, @host2, @host3].each do |host| + host.reload + assert_equal @user.id_and_type, host.is_owned_by + end + end + + test "should change owner with usergroup id" do + put :change_owner, params: valid_bulk_params.merge(:owner_id => @usergroup.id_and_type) + + assert_response :success + response = ActiveSupport::JSON.decode(@response.body) + assert_match(/Updated hosts: changed owner/, response['message']) + + [@host1, @host2, @host3].each do |host| + host.reload + assert_equal @usergroup.id_and_type, host.is_owned_by + end + end + + test "should handle single host ownership change" do + single_host_params = valid_bulk_params([@host1.id]) + + put :change_owner, params: single_host_params.merge(:owner_id => @user.id_and_type) + + assert_response :success + response = ActiveSupport::JSON.decode(@response.body) + assert_match(/Updated host: changed owner/, response['message']) + + @host1.reload + assert_equal @user.id_and_type, @host1.is_owned_by + end + + test "should require owner_id parameter" do + put :change_owner, params: valid_bulk_params + + assert_response :success + end + + test "should call BulkHostsManager with correct parameters" do + bulk_manager = mock('BulkHostsManager') + BulkHostsManager.expects(:new).with(hosts: anything).returns(bulk_manager) + bulk_manager.expects(:change_owner).with(@user.id_and_type) + + put :change_owner, params: valid_bulk_params.merge(:owner_id => @user.id_and_type) + + assert_response :success + end + + context "with different host counts" do + test "should handle pluralization correctly for single host" do + single_host_params = valid_bulk_params([@host1.id]) + + put :change_owner, params: single_host_params.merge(:owner_id => @user.id_and_type) + + assert_response :success + response = ActiveSupport::JSON.decode(@response.body) + # Should use singular form "host" not "hosts" + assert_match(/Updated host: changed owner/, response['message']) + end + + test "should handle pluralization correctly for multiple hosts" do + put :change_owner, params: valid_bulk_params.merge(:owner_id => @user.id_and_type) + + assert_response :success + response = ActiveSupport::JSON.decode(@response.body) + # Should use plural form "hosts" + assert_match(/Updated hosts: changed owner/, response['message']) + end + end + + private + + def set_session_user + { :user => users(:admin).id } + end +end diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/buildHosts/BulkBuildHostModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/buildHosts/BulkBuildHostModal.js index 99f973ba8f..e492e0b0fc 100644 --- a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/buildHosts/BulkBuildHostModal.js +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/buildHosts/BulkBuildHostModal.js @@ -78,7 +78,7 @@ const BulkBuildHostModal = ({ variant="link" onClick={handleModalClose} > - Cancel + {__('Cancel')} , ]; return ( diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js new file mode 100644 index 0000000000..34e7d9b456 --- /dev/null +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js @@ -0,0 +1,260 @@ +import React, { useState, useEffect } from 'react'; +import PropTypes from 'prop-types'; +import { FormattedMessage } from 'react-intl'; +import { useDispatch, useSelector } from 'react-redux'; +import { + Modal, + Button, + TextContent, + Text, + Select, + SelectOption, + SelectList, + SelectGroup, + MenuToggle, +} from '@patternfly/react-core'; +import { addToast } from '../../../ToastsList/slice'; +import { translate as __ } from '../../../../common/I18n'; +import { + BULK_CHANGE_OWNER_KEY, + bulkChangeOwner, + fetchUsers, + fetchUsergroups, + USER_KEY, + USERGROUP_KEY, +} from './actions'; +import { foremanUrl } from '../../../../common/helpers'; +import { APIActions } from '../../../../redux/API'; +import { STATUS } from '../../../../constants'; +import { + selectAPIStatus, + selectAPIResponse, +} from '../../../../redux/API/APISelectors'; +import { + HOSTS_API_PATH, + API_REQUEST_KEY, +} from '../../../../routes/Hosts/constants'; +import { failedHostsToastParams } from '../helpers'; + +const BulkChangeOwnerModal = ({ + isOpen, + closeModal, + selectAllHostsMode, + selectedCount, + fetchBulkParams, +}) => { + const dispatch = useDispatch(); + const [ownerId, setOwnerId] = useState(''); + const [ownerSelectOpen, setOwnerSelectOpen] = useState(false); + + useEffect(() => { + dispatch(fetchUsers()); + dispatch(fetchUsergroups()); + }, [dispatch]); + + const users = useSelector(state => selectAPIResponse(state, USER_KEY)); + const usergroups = useSelector(state => + selectAPIResponse(state, USERGROUP_KEY) + ); + const userStatus = useSelector(state => selectAPIStatus(state, USER_KEY)); + const usergroupStatus = useSelector(state => + selectAPIStatus(state, USERGROUP_KEY) + ); + + const onToggleClick = () => { + setOwnerSelectOpen(!ownerSelectOpen); + }; + + const handleOwnerSelect = (event, selection) => { + setOwnerId(selection); + setOwnerSelectOpen(false); + }; + + const getOwnerLabel = id => { + if (id.endsWith('-Users')) { + const userId = id.replace('-Users', ''); + return users?.results?.find(u => u.id.toString() === userId)?.login || id; + // eslint-disable-next-line spellcheck/spell-checker + } else if (id.endsWith('-Usergroups')) { + // eslint-disable-next-line spellcheck/spell-checker + const groupId = id.replace('-Usergroups', ''); + return ( + usergroups?.results?.find(ug => ug.id.toString() === groupId)?.name || + id + ); + } + return id; + }; + + const toggle = toggleRef => ( + + {ownerId ? getOwnerLabel(ownerId) : __('Select an owner')} + + ); + + const handleModalClose = () => { + setOwnerId(''); + closeModal(); + }; + + const handleError = response => { + handleModalClose(); + dispatch( + addToast( + failedHostsToastParams({ + ...response.data.error, + key: BULK_CHANGE_OWNER_KEY, + }) + ) + ); + }; + + const handleSuccess = response => { + dispatch( + addToast({ + type: 'success', + message: response.data.message, + }) + ); + dispatch( + APIActions.get({ + key: API_REQUEST_KEY, + url: foremanUrl(HOSTS_API_PATH), + }) + ); + handleModalClose(); + }; + + const handleConfirm = () => { + const requestBody = { + included: { + search: fetchBulkParams(), + }, + owner_id: ownerId, + }; + + dispatch(bulkChangeOwner(requestBody, handleSuccess, handleError)); + }; + + const modalActions = [ + , + , + ]; + + return ( + + + + {selectAllHostsMode ? ( + {__('All')}, + }} + /> + ) : ( + {selectedCount}, + }} + /> + )} + + + {userStatus === STATUS.RESOLVED && usergroupStatus === STATUS.RESOLVED && ( + + )} + + ); +}; + +BulkChangeOwnerModal.propTypes = { + isOpen: PropTypes.bool, + closeModal: PropTypes.func, + fetchBulkParams: PropTypes.func.isRequired, + selectedCount: PropTypes.number.isRequired, + selectAllHostsMode: PropTypes.bool.isRequired, +}; + +BulkChangeOwnerModal.defaultProps = { + isOpen: false, + closeModal: () => {}, +}; + +export default BulkChangeOwnerModal; diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/actions.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/actions.js new file mode 100644 index 0000000000..35dcac5fdf --- /dev/null +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/actions.js @@ -0,0 +1,42 @@ +import { APIActions } from '../../../../redux/API'; +import { foremanUrl } from '../../../../common/helpers'; + +export const BULK_CHANGE_OWNER_KEY = 'BULK_CHANGE_OWNER'; +export const bulkChangeOwner = (params, handleSuccess, handleError) => { + const url = foremanUrl(`/api/v2/hosts/bulk/change_owner`); + return APIActions.put({ + key: BULK_CHANGE_OWNER_KEY, + url, + handleSuccess, + handleError, + params, + }); +}; + +export const USER_KEY = 'USER_KEY'; +export const USERGROUP_KEY = 'USERGROUP_KEY'; + +export const fetchUsers = () => { + const url = foremanUrl('/api/users'); + return APIActions.get({ + key: USER_KEY, + url, + params: { + per_page: 'all', + }, + }); +}; + +export const fetchUsergroups = () => { + // eslint-disable-next-line spellcheck/spell-checker + const url = foremanUrl('/api/usergroups'); + return APIActions.get({ + key: USERGROUP_KEY, + url, + params: { + per_page: 'all', + }, + }); +}; + +export default bulkChangeOwner; diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/index.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/index.js new file mode 100644 index 0000000000..952b9a7862 --- /dev/null +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/index.js @@ -0,0 +1,29 @@ +import React, { useContext } from 'react'; +import { ForemanActionsBarContext } from '../../../../components/HostDetails/ActionsBar'; +import { useForemanModal } from '../../../../components/ForemanModal/ForemanModalHooks'; +import BulkChangeOwnerModal from './BulkChangeOwnerModal'; + +const BulkChangeOwnerModalScene = () => { + const { + selectAllHostsMode, + selectedCount, + selectedResults, + fetchBulkParams, + } = useContext(ForemanActionsBarContext); + const { modalOpen, setModalClosed } = useForemanModal({ + id: 'bulk-change-owner-modal', + }); + return ( + + ); +}; + +export default BulkChangeOwnerModalScene; diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/index.js b/webpack/assets/javascripts/react_app/components/HostsIndex/index.js index 2410450885..2336263ae5 100644 --- a/webpack/assets/javascripts/react_app/components/HostsIndex/index.js +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/index.js @@ -46,6 +46,7 @@ import { import { bulkDeleteHosts } from './BulkActions/bulkDelete'; import BulkBuildHostModal from './BulkActions/buildHosts'; import BulkReassignHostgroupModal from './BulkActions/reassignHostGroup'; +import BulkChangeOwnerModal from './BulkActions/changeOwner'; import { foremanUrl } from '../../common/helpers'; import Slot from '../common/Slot'; import forceSingleton from '../../common/forceSingleton'; @@ -149,6 +150,7 @@ const HostsIndex = () => { const { pageRowCount } = getPageStats({ total, page, perPage }); const { fetchBulkParams, + searchQuery, updateSearchQuery, ...selectAllOptions } = useBulkSelect({ @@ -166,7 +168,9 @@ const HostsIndex = () => { areAllRowsOnPageSelected, areAllRowsSelected, isSelected, + selectedResults, } = selectAllOptions; + const selectAllHostsMode = areAllRowsSelected() && searchQuery === ''; const selectionToolbar = ( @@ -214,6 +218,11 @@ const HostsIndex = () => { id: 'bulk-reassign-hg-modal', }) ); + dispatch( + addModal({ + id: 'bulk-change-owner-modal', + }) + ); }, [dispatch]); const { setModalOpen: setHgModalOpen } = useForemanModal({ @@ -222,6 +231,9 @@ const HostsIndex = () => { const { setModalOpen: setBuildModalOpen } = useForemanModal({ id: 'bulk-build-hosts-modal', }); + const { setModalOpen: setChangeOwnerModalOpen } = useForemanModal({ + id: 'bulk-change-owner-modal', + }); const dropdownItems = [ { > {__('Change host group')} , + + {__('Change owner')} + , ]; const dangerZoneItems = [ @@ -438,10 +458,16 @@ const HostsIndex = () => { })} +