Fixes #39463 - show bulk actions error toasts correctly

sbernhard · codex · sbernhard · commit fcd128af34c7 · 2026-07-23T15:08:55.000+02:00
Co-authored-by: Codex &lt;codex@openai.com&gt;
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/assignTaxonomy/BulkAssignTaxonomyModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/assignTaxonomy/BulkAssignTaxonomyModal.js
@@ -12,6 +12,7 @@ import {
   TreeView,
 } from '@patternfly/react-core';
 import { addToast } from '../../../ToastsList/slice';
+import { buildBulkRequestBody, bulkErrorToastParams } from '../helpers';
 import { translate as __ } from '../../../../common/I18n';
 import { STATUS } from '../../../../constants';
 import {
@@ -32,7 +33,6 @@ import {
   MODAL_TYPES,
 } from './BulkAssignTaxonomyConstants';
 import TaxonomySelect from './TaxonomySelect';
-import { buildBulkRequestBody } from '../helpers';
 
 export const BulkAssignOrganizationModal = props => (
   <BulkAssignTaxonomyModal modalType={MODAL_TYPES.ORGANIZATION} {...props} />
@@ -53,6 +53,9 @@ const BulkAssignTaxonomyModal = ({
   onSuccess: onSuccessCallback,
 }) => {
   const org = modalType === MODAL_TYPES.ORGANIZATION;
+  const actionKey = org
+    ? BULK_ASSIGN_ORGANIZATION_KEY
+    : BULK_ASSIGN_LOCATION_KEY;
   const taxType = org ? 'organization' : 'location';
   const dispatch = useDispatch();
   const [taxId, setTaxId] = useState('');
@@ -69,9 +72,7 @@ const BulkAssignTaxonomyModal = ({
       : selectAPIStatus(state, LOCATION_KEY)
   );
   const hostUpdateStatus = useSelector(state =>
-    org
-      ? selectAPIStatus(state, BULK_ASSIGN_ORGANIZATION_KEY)
-      : selectAPIStatus(state, BULK_ASSIGN_LOCATION_KEY)
+    selectAPIStatus(state, actionKey)
   );
   const handleModalClose = () => {
     setTaxId('');
@@ -106,15 +107,8 @@ const BulkAssignTaxonomyModal = ({
     taxonomy.results.find(t => t.id === id)?.name;
 
   const handleError = error => {
-    const {
-      response: {
-        data: {
-          error: { message },
-        },
-      },
-    } = error;
-    dispatch(addToast({ type: 'danger', message }));
     handleModalClose();
+    dispatch(addToast(bulkErrorToastParams(error, actionKey)));
   };
 
   const handleSuccess = response => {
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
@@ -12,7 +12,7 @@ import {
 } from '@patternfly/react-core';
 import { addToast } from '../../../ToastsList/slice';
 import { translate as __ } from '../../../../common/I18n';
-import { buildBulkRequestBody, failedHostsToastParams } from '../helpers';
+import { buildBulkRequestBody, bulkErrorToastParams } from '../helpers';
 import { STATUS } from '../../../../constants';
 import { selectAPIStatus } from '../../../../redux/API/APISelectors';
 import { bulkBuildHosts, HOST_BUILD_KEY } from './actions';
@@ -37,13 +37,9 @@ const BulkBuildHostModal = ({
     closeModal();
   };
 
-  const handleError = ({ response }) => {
+  const handleError = error => {
     handleModalClose();
-    dispatch(
-      addToast(
-        failedHostsToastParams({ ...response.data.error, key: HOST_BUILD_KEY })
-      )
-    );
+    dispatch(addToast(bulkErrorToastParams(error, HOST_BUILD_KEY)));
   };
   const handleSave = () => {
     const requestBody = buildBulkRequestBody({
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
@@ -28,7 +28,7 @@ import {
   selectAPIStatus,
   selectAPIResponse,
 } from '../../../../redux/API/APISelectors';
-import { buildBulkRequestBody, failedHostsToastParams } from '../helpers';
+import { buildBulkRequestBody, bulkErrorToastParams } from '../helpers';
 
 const BulkChangeOwnerModal = ({
   isOpen,
@@ -99,16 +99,9 @@ const BulkChangeOwnerModal = ({
     closeModal();
   };
 
-  const handleError = response => {
+  const handleError = error => {
     handleModalClose();
-    dispatch(
-      addToast(
-        failedHostsToastParams({
-          ...response.data.error,
-          key: BULK_CHANGE_OWNER_KEY,
-        })
-      )
-    );
+    dispatch(addToast(bulkErrorToastParams(error, BULK_CHANGE_OWNER_KEY)));
   };
 
   const handleSuccess = response => {
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/disassociate/BulkDisassociateModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/disassociate/BulkDisassociateModal.js
@@ -12,7 +12,7 @@ import {
 import { addToast } from '../../../ToastsList/slice';
 import { translate as __ } from '../../../../common/I18n';
 import { BULK_DISASSOCIATE_KEY, bulkDisassociate } from './actions';
-import { buildBulkRequestBody, failedHostsToastParams } from '../helpers';
+import { buildBulkRequestBody, bulkErrorToastParams } from '../helpers';
 
 const BulkDisassociateModal = ({
   isOpen,
@@ -56,16 +56,9 @@ const BulkDisassociateModal = ({
     },
   ];
 
-  const handleError = response => {
+  const handleError = error => {
     closeModal();
-    dispatch(
-      addToast(
-        failedHostsToastParams({
-          ...response.data.error,
-          key: BULK_DISASSOCIATE_KEY,
-        })
-      )
-    );
+    dispatch(addToast(bulkErrorToastParams(error, BULK_DISASSOCIATE_KEY)));
   };
 
   const handleSuccess = response => {
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/helpers.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/helpers.js
@@ -52,3 +52,16 @@ export const failedHostsToastParams = ({
 
   return toastParams;
 };
+
+export const bulkErrorToastParams = (error, key) => {
+  const fallback = error?.message || __('Unexpected error occurred.');
+  const apiError = error?.response?.data?.error;
+  const isObject = apiError && typeof apiError === 'object';
+  const message = isObject ? apiError.message : apiError;
+
+  return failedHostsToastParams({
+    message: message || fallback,
+    failed_host_ids: isObject ? apiError.failed_host_ids : undefined,
+    key,
+  });
+};
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/helpers.test.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/helpers.test.js
@@ -0,0 +1,142 @@
+import { buildBulkRequestBody, bulkErrorToastParams } from './helpers';
+
+jest.mock('../../../common/I18n');
+
+describe('BulkActions helpers', () => {
+  describe('bulkErrorToastParams', () => {
+    it('uses API error details from the response', () => {
+      const toast = bulkErrorToastParams(
+        {
+          response: {
+            data: {
+              error: {
+                message: 'Some hosts failed',
+                failed_host_ids: [1, 2],
+              },
+            },
+          },
+        },
+        'BULK_ACTION_KEY'
+      );
+
+      expect(toast).toMatchObject({
+        type: 'danger',
+        message: 'Some hosts failed',
+      });
+      expect(toast.link.children).toBe('Failed hosts');
+      expect(toast.link.href).toContain('/new/hosts');
+      expect(toast.link.href).toContain('search=');
+    });
+
+    it('uses the error message when no response exists', () => {
+      const toast = bulkErrorToastParams(
+        { message: 'Network Error' },
+        'BULK_ACTION_KEY'
+      );
+
+      expect(toast).toMatchObject({
+        type: 'danger',
+        message: 'Network Error',
+      });
+      expect(toast.link).toBeUndefined();
+    });
+
+    it('uses a response error string as the message', () => {
+      const toast = bulkErrorToastParams(
+        {
+          response: {
+            data: {
+              error: 'Bulk action failed',
+            },
+          },
+        },
+        'BULK_ACTION_KEY'
+      );
+
+      expect(toast).toMatchObject({
+        type: 'danger',
+        message: 'Bulk action failed',
+      });
+      expect(toast.link).toBeUndefined();
+    });
+
+    it('falls back to a generic message when no usable message exists', () => {
+      const toast = bulkErrorToastParams({}, 'BULK_ACTION_KEY');
+
+      expect(toast).toMatchObject({
+        type: 'danger',
+        message: 'Unexpected error occurred.',
+      });
+      expect(toast.link).toBeUndefined();
+    });
+
+    it('keeps failed hosts link when response error has no message', () => {
+      const toast = bulkErrorToastParams(
+        {
+          response: {
+            data: {
+              error: {
+                failed_host_ids: [1, 2],
+              },
+            },
+          },
+        },
+        'BULK_ACTION_KEY'
+      );
+
+      expect(toast).toMatchObject({
+        type: 'danger',
+        message: 'Unexpected error occurred.',
+      });
+      expect(toast.link.children).toBe('Failed hosts');
+    });
+  });
+
+  describe('buildBulkRequestBody', () => {
+    it('builds an included search from fetchBulkParams', () => {
+      const body = buildBulkRequestBody({
+        fetchBulkParams: () => 'id ^ (1,2,3)',
+        reboot: true,
+      });
+
+      expect(body).toEqual({
+        included: {
+          search: 'id ^ (1,2,3)',
+        },
+        reboot: true,
+      });
+    });
+
+    it('adds taxonomy params when present', () => {
+      const body = buildBulkRequestBody({
+        fetchBulkParams: () => 'id ^ (1,2,3)',
+        organizationId: 1,
+        locationId: 2,
+      });
+
+      expect(body).toMatchObject({
+        included: {
+          search: 'id ^ (1,2,3)',
+        },
+        organization_id: 1,
+        location_id: 2,
+      });
+    });
+
+    it('uses includedSearch instead of calling fetchBulkParams', () => {
+      const fetchBulkParams = jest.fn();
+
+      const body = buildBulkRequestBody({
+        fetchBulkParams,
+        includedSearch: 'name ~ test',
+      });
+
+      expect(body).toEqual({
+        included: {
+          search: 'name ~ test',
+        },
+      });
+      expect(fetchBulkParams).not.toHaveBeenCalled();
+    });
+  });
+});
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/manageNotifications/BulkManageNotificationsModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/manageNotifications/BulkManageNotificationsModal.js
@@ -15,7 +15,7 @@ import { FormattedMessage } from 'react-intl';
 import { translate as __ } from '../../../../common/I18n';
 import { bulkManageNotifications } from './actions';
 import { BULK_MANAGE_NOTIFICATIONS_KEY } from './constants';
-import { failedHostsToastParams } from '../helpers';
+import { bulkErrorToastParams } from '../helpers';
 import { addToast } from '../../../ToastsList/slice';
 
 const BulkManageNotificationsModal = ({
@@ -48,17 +48,9 @@ const BulkManageNotificationsModal = ({
   };
 
   const handleError = error => {
-    const apiError = error?.response?.data?.error;
-    if (apiError) {
-      dispatch(
-        addToast(
-          failedHostsToastParams({
-            ...apiError,
-            key: BULK_MANAGE_NOTIFICATIONS_KEY,
-          })
-        )
-      );
-    }
+    dispatch(
+      addToast(bulkErrorToastParams(error, BULK_MANAGE_NOTIFICATIONS_KEY))
+    );
     handleModalClose();
   };
 
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/powerState/BulkPowerStateModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/powerState/BulkPowerStateModal.js
@@ -19,7 +19,11 @@ import './BulkPowerStateModal.scss';
 import { HostsPowerRefreshContext } from '../../HostsPowerRefreshContext';
 import { POWER_STATES, BULK_POWER_STATE_KEY } from './constants';
 import { bulkChangePowerState } from './actions';
-import { buildBulkRequestBody, failedHostsToastParams } from '../helpers';
+import {
+  buildBulkRequestBody,
+  failedHostsToastParams,
+  bulkErrorToastParams,
+} from '../helpers';
 import { addToast } from '../../../ToastsList/slice';
 
 const BulkPowerStateModal = ({
@@ -61,8 +65,9 @@ const BulkPowerStateModal = ({
 
   const handleError = error => {
     const apiError = error?.response?.data?.error;
+    const isObject = apiError && typeof apiError === 'object';
 
-    if (apiError) {
+    if (isObject) {
       let enhancedError = apiError;
 
       if (apiError.failed_hosts && apiError.failed_hosts.length > 0) {
@@ -86,6 +91,8 @@ const BulkPowerStateModal = ({
           })
         )
       );
+    } else {
+      dispatch(addToast(bulkErrorToastParams(error, BULK_POWER_STATE_KEY)));
     }
 
     cleanup();
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/powerState/BulkPowerStateModal.test.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/powerState/BulkPowerStateModal.test.js
diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/reassignHostGroup/BulkReassignHostgroupModal.js b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/reassignHostGroup/BulkReassignHostgroupModal.js
