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 index 562d770e31..052b9df990 100644 --- 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 => ( @@ -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 index eea9df3c7e..e77df2e37d 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 @@ -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 index d5f202cea9..e9698e9130 100644 --- 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 index cc29187049..4bab86de55 100644 --- 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 index e51c1ba8cc..def937cff8 100644 --- 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 new file mode 100644 index 0000000000..50ec714f33 --- /dev/null +++ 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 index 373fd54ea1..277af61163 100644 --- 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 index c038155e64..61e852f98c 100644 --- 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 new file mode 100644 index 0000000000..b615bbfd8e --- /dev/null +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/powerState/BulkPowerStateModal.test.js @@ -0,0 +1,172 @@ +import React from 'react'; +import { render, screen, fireEvent, act } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { Provider } from 'react-redux'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; +import BulkPowerStateModal from './BulkPowerStateModal'; +import { HostsPowerRefreshContext } from '../../HostsPowerRefreshContext'; +import { bulkChangePowerState } from './actions'; + +jest.mock('../../../../common/I18n'); +jest.mock('./actions'); + +const mockStore = configureMockStore([thunk]); + +const defaultProps = { + selectedHostsCount: 3, + fetchBulkParams: jest.fn(() => 'id ^ (1,2,3)'), + isOpen: true, + closeModal: jest.fn(), +}; + +const renderModal = (props = {}) => { + const store = mockStore({ API: {} }); + render( + + + + + + ); + return store; +}; + +describe('BulkPowerStateModal', () => { + let capturedHandleError; + + beforeEach(() => { + jest.clearAllMocks(); + bulkChangePowerState.mockImplementation( + (_payload, _handleSuccess, handleError) => { + capturedHandleError = handleError; + return { type: 'MOCK_ACTION' }; + } + ); + }); + + const selectAndSubmit = async () => { + await act(async () => { + fireEvent.click(screen.getByText('Select power state')); + }); + await act(async () => { + fireEvent.click(screen.getByText('Start')); + }); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /apply/i })); + }); + }; + + it('renders modal with title and power state select', () => { + renderModal(); + expect(screen.getByText('Change power state')).toBeInTheDocument(); + expect(screen.getByText('Select power state')).toBeInTheDocument(); + }); + + it('has Apply disabled until a power state is selected', async () => { + renderModal(); + expect(screen.getByRole('button', { name: /apply/i })).toBeDisabled(); + await act(async () => { + fireEvent.click(screen.getByText('Select power state')); + }); + await act(async () => { + fireEvent.click(screen.getByText('Start')); + }); + expect(screen.getByRole('button', { name: /apply/i })).not.toBeDisabled(); + }); + + it('calls closeModal on Cancel', async () => { + const closeModal = jest.fn(); + renderModal({ closeModal }); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /cancel/i })); + }); + expect(closeModal).toHaveBeenCalled(); + }); + + describe('handleError', () => { + it('shows object error as danger toast with failed hosts link', async () => { + const store = renderModal(); + await selectAndSubmit(); + + await act(async () => { + capturedHandleError({ + response: { + data: { + error: { message: 'Some hosts failed', failed_host_ids: [1, 2] }, + }, + }, + }); + }); + + const toast = store + .getActions() + .find(a => a.type === 'toasts/addToast')?.payload.toast; + expect(toast).toMatchObject({ type: 'danger', message: 'Some hosts failed' }); + expect(toast.link).toBeDefined(); + }); + + it('enriches message with provider errors from failed_hosts', async () => { + const store = renderModal(); + await selectAndSubmit(); + + await act(async () => { + capturedHandleError({ + response: { + data: { + error: { + message: 'Partial failure.', + failed_hosts: [ + { id: 1, error: 'Provider A failed' }, + { id: 2, error: 'Provider B failed' }, + ], + }, + }, + }, + }); + }); + + const toast = store + .getActions() + .find(a => a.type === 'toasts/addToast')?.payload.toast; + expect(toast.type).toBe('danger'); + expect(toast.message).toContain('Partial failure.'); + expect(toast.message).toContain('Provider A failed'); + expect(toast.message).toContain('Provider B failed'); + }); + + it('handles a string apiError correctly without a link', async () => { + // regression: old code spread a string into failedHostsToastParams, + // losing the message entirely — the isObject guard fixes this + const store = renderModal(); + await selectAndSubmit(); + + await act(async () => { + capturedHandleError({ + response: { data: { error: 'Something went wrong' } }, + }); + }); + + const toast = store + .getActions() + .find(a => a.type === 'toasts/addToast')?.payload.toast; + expect(toast).toMatchObject({ type: 'danger', message: 'Something went wrong' }); + expect(toast.link).toBeUndefined(); + }); + + it('falls back to error.message for network errors without a response', async () => { + const store = renderModal(); + await selectAndSubmit(); + + await act(async () => { + capturedHandleError({ message: 'Network Error' }); + }); + + const toast = store + .getActions() + .find(a => a.type === 'toasts/addToast')?.payload.toast; + expect(toast).toMatchObject({ type: 'danger', message: 'Network Error' }); + expect(toast.link).toBeUndefined(); + }); + }); +}); 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 index 42d1ee7895..7867564566 100644 --- a/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/reassignHostGroup/BulkReassignHostgroupModal.js +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/reassignHostGroup/BulkReassignHostgroupModal.js @@ -11,7 +11,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, @@ -120,15 +120,10 @@ const BulkReassignHostgroupModal = ({ dispatch(fetchHostgroups()); }, [dispatch]); - const handleError = response => { + const handleError = error => { handleModalClose(); dispatch( - addToast( - failedHostsToastParams({ - ...response.data.error, - key: BULK_REASSIGN_HOSTGROUP_KEY, - }) - ) + addToast(bulkErrorToastParams(error, BULK_REASSIGN_HOSTGROUP_KEY)) ); };