Fixes #39437 - Apply taxonomies to UI bulk actions

Before this patch, the Foreman UI did not send any information about
taxonomies on the 'All Hosts' page for bulk actions. This could cause
bulk actions to be applied to different hosts than the ones that were
actually selected in the UI because the session in the backend would
calculate the hosts independent from UI input.

Assisted-by: OpenAI Codex
(cherry picked from commit 34ee2c5)

Author: nadjaheitmann

app/controllers/api/v2/hosts_bulk_actions_controller.rb

@@ -9,7 +9,6 @@ class HostsBulkActionsController < V2::BaseController
       before_action :validate_power_action, :only => [:change_power_state]
 
       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")

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

@@ -1,9 +1,10 @@
 module Api::V2::BulkHostsExtension
   extend ActiveSupport::Concern
 
-  def bulk_hosts_relation(permission, org)
+  def bulk_hosts_relation(permission, org, location)
     relation = ::Host::Managed.authorized(permission)
     relation = relation.where(organization: org) if org
+    relation = relation.where(location: location) if location
     relation
   end
 
@@ -18,7 +19,8 @@ def find_bulk_hosts(permission, bulk_params, restrict_to = nil)
     end
 
     find_organization
-    @hosts = bulk_hosts_relation(permission, @organization)
+    find_location
+    @hosts = bulk_hosts_relation(permission, @organization, @location)
 
     if bulk_params[:included][:ids].present?
       @hosts = @hosts.where(id: bulk_params[:included][:ids])
@@ -42,4 +44,8 @@ def find_bulk_hosts(permission, bulk_params, restrict_to = nil)
   def find_organization
     @organization ||= Organization.find_by_id(params[:organization_id])
   end
+
+  def find_location
+    @location ||= Location.find_by_id(params[:location_id])
+  end
 end

test/controllers/api/v2/hosts_bulk_actions_controller_test.rb

@@ -17,6 +17,7 @@ def setup
   def valid_bulk_params(host_ids = @host_ids)
     {
       :organization_id => @organization.id,
+      :location_id => @location.id,
       :included => {
         :ids => host_ids,
       },
@@ -107,6 +108,33 @@ def valid_power_params(host_ids = @host_ids, action = 'start')
     end
   end
 
+  test "should scope searched hosts by organization and location" do
+    other_location = FactoryBot.create(:location)
+    other_host = FactoryBot.create(:host, :managed, :organization => @organization, :location => other_location)
+
+    put :change_owner, params: {
+      :organization_id => @organization.id,
+      :location_id => @location.id,
+      :included => {
+        :search => 'name ~ *',
+      },
+      :excluded => {
+        :ids => [],
+      },
+      :owner_id => @user.id_and_type,
+    }
+
+    assert_response :success
+
+    [@host1, @host2, @host3].each do |host|
+      host.reload
+      assert_equal @user.id_and_type, host.is_owned_by
+    end
+
+    other_host.reload
+    refute_equal @user.id_and_type, other_host.is_owned_by
+  end
+
   context "change_power_state" do
     test "successfully changes power state for all hosts" do
       Host.any_instance.stubs(:supports_power?).returns(true)

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

@@ -0,0 +1,161 @@
+import { bulkDeleteHosts } from '../bulkDelete';
+import { openConfirmModal } from '../../../ConfirmModal';
+import { APIActions } from '../../../../redux/API';
+import { visit } from '../../../../common/helpers';
+
+jest.mock('../../../ConfirmModal', () => ({
+  openConfirmModal: jest.fn(payload => ({
+    type: 'OPEN_CONFIRM_MODAL',
+    payload,
+  })),
+}));
+
+jest.mock('../../../../redux/API', () => ({
+  APIActions: {
+    delete: jest.fn(params => ({
+      type: 'API_DELETE',
+      params,
+    })),
+  },
+}));
+
+jest.mock('../../../../common/helpers', () => ({
+  foremanUrl: jest.fn(path => path),
+  visit: jest.fn(),
+}));
+
+describe('bulkDeleteHosts', () => {
+  const dispatch = jest.fn(action => {
+    if (typeof action === 'function') {
+      return action(dispatch);
+    }
+    return action;
+  });
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  const bulkParams = 'id ^ (1,2,3)';
+  const organizationId = 1;
+  const locationId = 2;
+  const selectedCount = 3;
+  const destroyVmOnHostDelete = true;
+
+  it('dispatches openConfirmModal with correct parameters', () => {
+    bulkDeleteHosts({
+      bulkParams,
+      organizationId,
+      locationId,
+      selectedCount,
+      destroyVmOnHostDelete,
+    })(dispatch);
+
+    expect(openConfirmModal).toHaveBeenCalledTimes(1);
+    const modalPayload = openConfirmModal.mock.calls[0][0];
+
+    expect(modalPayload.isWarning).toBe(true);
+    expect(modalPayload.isDireWarning).toBe(true);
+    expect(modalPayload.id).toBe('bulk-delete-hosts-modal');
+    expect(modalPayload.confirmButtonText).toBe('Delete');
+    expect(typeof modalPayload.onConfirm).toBe('function');
+  });
+
+  describe('onConfirm callback', () => {
+    it('calls visit with /new/hosts when onDeleteSuccess is not provided', () => {
+      bulkDeleteHosts({
+        bulkParams,
+        organizationId,
+        locationId,
+        selectedCount,
+        destroyVmOnHostDelete,
+      })(dispatch);
+
+      const modalPayload = openConfirmModal.mock.calls[0][0];
+      modalPayload.onConfirm();
+
+      expect(APIActions.delete).toHaveBeenCalledTimes(1);
+      const deleteParams = APIActions.delete.mock.calls[0][0];
+      const requestUrl = new URL(deleteParams.url, 'https://example.test');
+
+      expect(requestUrl.pathname).toBe('/api/v2/hosts/bulk');
+      expect(requestUrl.searchParams.get('search')).toBe(bulkParams);
+      expect(requestUrl.searchParams.get('organization_id')).toBe(
+        String(organizationId)
+      );
+      expect(requestUrl.searchParams.get('location_id')).toBe(
+        String(locationId)
+      );
+      expect(deleteParams.key).toBe('BULK-HOSTS-DELETE');
+      expect(typeof deleteParams.successToast).toBe('function');
+      expect(typeof deleteParams.errorToast).toBe('function');
+      expect(typeof deleteParams.handleSuccess).toBe('function');
+
+      deleteParams.handleSuccess();
+      expect(visit).toHaveBeenCalledWith('/new/hosts');
+    });
+
+    it('calls onDeleteSuccess callback when provided instead of visit', () => {
+      const onDeleteSuccess = jest.fn();
+
+      bulkDeleteHosts({
+        bulkParams,
+        organizationId,
+        locationId,
+        selectedCount,
+        destroyVmOnHostDelete,
+        onDeleteSuccess,
+      })(dispatch);
+
+      const modalPayload = openConfirmModal.mock.calls[0][0];
+      modalPayload.onConfirm();
+
+      const deleteParams = APIActions.delete.mock.calls[0][0];
+      deleteParams.handleSuccess();
+
+      expect(onDeleteSuccess).toHaveBeenCalledTimes(1);
+      expect(visit).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('successToast', () => {
+    it('returns formatted success message with host count', () => {
+      bulkDeleteHosts({
+        bulkParams,
+        organizationId,
+        locationId,
+        selectedCount,
+        destroyVmOnHostDelete,
+      })(dispatch);
+
+      const modalPayload = openConfirmModal.mock.calls[0][0];
+      modalPayload.onConfirm();
+
+      const deleteParams = APIActions.delete.mock.calls[0][0];
+      const toastMessage = deleteParams.successToast();
+
+      expect(toastMessage).toContain(String(selectedCount));
+    });
+  });
+
+  describe('errorToast', () => {
+    it('returns error message from response', () => {
+      bulkDeleteHosts({
+        bulkParams,
+        organizationId,
+        locationId,
+        selectedCount,
+        destroyVmOnHostDelete,
+      })(dispatch);
+
+      const modalPayload = openConfirmModal.mock.calls[0][0];
+      modalPayload.onConfirm();
+
+      const deleteParams = APIActions.delete.mock.calls[0][0];
+      const errorMessage = 'Bulk delete failed';
+      const toastMessage = deleteParams.errorToast({ message: errorMessage });
+
+      expect(toastMessage).toBe(errorMessage);
+    });
+  });
+});

webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/assignTaxonomy/BulkAssignTaxonomyModal.js

@@ -38,6 +38,7 @@ import {
   API_REQUEST_KEY,
 } from '../../../../routes/Hosts/constants';
 import TaxonomySelect from './TaxonomySelect';
+import { buildBulkRequestBody } from '../helpers';
 
 export const BulkAssignOrganizationModal = props => (
   <BulkAssignTaxonomyModal modalType={MODAL_TYPES.ORGANIZATION} {...props} />
@@ -52,6 +53,8 @@ const BulkAssignTaxonomyModal = ({
   selectAllHostsMode,
   selectedCount,
   fetchBulkParams,
+  organizationId,
+  locationId,
   modalType,
 }) => {
   const org = modalType === MODAL_TYPES.ORGANIZATION;
@@ -136,13 +139,13 @@ const BulkAssignTaxonomyModal = ({
   };
 
   const handleSave = () => {
-    const requestBody = {
-      included: {
-        search: fetchBulkParams(),
-      },
+    const requestBody = buildBulkRequestBody({
+      fetchBulkParams,
+      organizationId,
+      locationId,
       id: taxId,
       mismatch_setting: fixRadioChecked,
-    };
+    });
 
     org
       ? dispatch(
@@ -243,10 +246,14 @@ BulkAssignTaxonomyModal.propTypes = {
   selectedCount: PropTypes.number.isRequired,
   selectAllHostsMode: PropTypes.bool.isRequired,
   fetchBulkParams: PropTypes.func.isRequired,
+  organizationId: PropTypes.number,
+  locationId: PropTypes.number,
   modalType: PropTypes.string.isRequired,
 };
 
 BulkAssignTaxonomyModal.defaultProps = {
   isOpen: false,
   closeModal: () => {},
+  organizationId: undefined,
+  locationId: undefined,
 };

webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/assignTaxonomy/index.js

@@ -7,31 +7,43 @@ import {
 } from './BulkAssignTaxonomyModal';
 
 export const BulkAssignOrganizationModalScene = ({ isOpen, closeModal }) => {
-  const { selectAllHostsMode, selectedCount, fetchBulkParams } = useContext(
-    ForemanActionsBarContext
-  );
+  const {
+    selectAllHostsMode,
+    selectedCount,
+    fetchBulkParams,
+    organizationId,
+    locationId,
+  } = useContext(ForemanActionsBarContext);
   return (
     <BulkAssignOrganizationModal
       key="bulk-assign-organization-modal"
       selectAllHostsMode={selectAllHostsMode}
       selectedCount={selectedCount}
       fetchBulkParams={fetchBulkParams}
+      organizationId={organizationId}
+      locationId={locationId}
       isOpen={isOpen}
       closeModal={closeModal}
     />
   );
 };
 
 export const BulkAssignLocationModalScene = ({ isOpen, closeModal }) => {
-  const { selectAllHostsMode, selectedCount, fetchBulkParams } = useContext(
-    ForemanActionsBarContext
-  );
+  const {
+    selectAllHostsMode,
+    selectedCount,
+    fetchBulkParams,
+    organizationId,
+    locationId,
+  } = useContext(ForemanActionsBarContext);
   return (
     <BulkAssignLocationModal
       key="bulk-assign-location-modal"
       selectAllHostsMode={selectAllHostsMode}
       selectedCount={selectedCount}
       fetchBulkParams={fetchBulkParams}
+      organizationId={organizationId}
+      locationId={locationId}
       isOpen={isOpen}
       closeModal={closeModal}
     />

webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/buildHosts/BulkBuildHostModal.js

@@ -12,7 +12,7 @@ import {
 } from '@patternfly/react-core';
 import { addToast } from '../../../ToastsList/slice';
 import { translate as __ } from '../../../../common/I18n';
-import { failedHostsToastParams } from '../helpers';
+import { buildBulkRequestBody, failedHostsToastParams } from '../helpers';
 import { STATUS } from '../../../../constants';
 import { selectAPIStatus } from '../../../../redux/API/APISelectors';
 import { bulkBuildHosts, HOST_BUILD_KEY } from './actions';
@@ -22,6 +22,8 @@ const BulkBuildHostModal = ({
   closeModal,
   selectedCount,
   fetchBulkParams,
+  organizationId,
+  locationId,
 }) => {
   const dispatch = useDispatch();
   const [buildRadioChecked, setBuildRadioChecked] = useState(true);
@@ -44,13 +46,13 @@ const BulkBuildHostModal = ({
     );
   };
   const handleSave = () => {
-    const requestBody = {
-      included: {
-        search: fetchBulkParams(),
-      },
+    const requestBody = buildBulkRequestBody({
+      fetchBulkParams,
+      organizationId,
+      locationId,
       reboot: rebootChecked,
       rebuild_configuration: !buildRadioChecked,
-    };
+    });
 
     dispatch(bulkBuildHosts(requestBody, handleModalClose, handleError));
   };
@@ -156,11 +158,15 @@ BulkBuildHostModal.propTypes = {
   closeModal: PropTypes.func,
   selectedCount: PropTypes.number.isRequired,
   fetchBulkParams: PropTypes.func.isRequired,
+  organizationId: PropTypes.number,
+  locationId: PropTypes.number,
 };
 
 BulkBuildHostModal.defaultProps = {
   isOpen: false,
   closeModal: () => {},
+  organizationId: undefined,
+  locationId: undefined,
 };
 
 export default BulkBuildHostModal;