diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js b/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js index 75b00d851..861c53919 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js @@ -1,75 +1,13 @@ -import React, { useState } from 'react'; -import PropTypes from 'prop-types'; +import React from 'react'; +import { Stack, StackItem } from '@patternfly/react-core'; import TaskInfo from './TaskInfo'; -import { - ForceUnlockConfirmationModal, - UnlockConfirmationModal, -} from '../../common/ClickConfirmation'; -import { TaskButtons } from './TaskButtons'; -const Task = props => { - const { - taskReload, - id, - forceCancelTaskRequest, - unlockTaskRequest, - action, - taskReloadStart, - } = props; - const forceUnlock = () => { - if (!taskReload) { - taskReloadStart(id); - } - forceCancelTaskRequest(id, action); - }; - const unlock = () => { - if (!taskReload) { - taskReloadStart(id); - } - unlockTaskRequest(id, action); - }; - const [unlockModalOpen, setUnlockModalOpen] = useState(false); - const [forceUnlockModalOpen, setForceUnlockModalOpen] = useState(false); - - return ( - - setUnlockModalOpen(false)} - /> - setForceUnlockModalOpen(false)} - /> - +const Task = props => ( + + - - ); -}; - -Task.propTypes = { - taskReload: PropTypes.bool, - id: PropTypes.string, - forceCancelTaskRequest: PropTypes.func, - unlockTaskRequest: PropTypes.func, - action: PropTypes.string, - taskReloadStart: PropTypes.func, -}; - -Task.defaultProps = { - taskReload: false, - id: '', - forceCancelTaskRequest: () => null, - unlockTaskRequest: () => null, - action: '', - taskReloadStart: () => null, -}; + + +); export default Task; diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js index b1c425e42..19dd4bf9b 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js @@ -1,9 +1,12 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { Button, Icon } from '@patternfly/react-core'; -import { SyncAltIcon } from '@patternfly/react-icons'; +import { Button, Flex, FlexItem, Icon } from '@patternfly/react-core'; +import { SimpleDropdown } from '@patternfly/react-templates'; +import { EllipsisVIcon, SyncAltIcon } from '@patternfly/react-icons'; import { translate as __ } from 'foremanReact/common/I18n'; +import './TaskButtons.scss'; + export const TaskButtons = ({ canEdit, dynflowEnableConsole, @@ -30,120 +33,128 @@ export const TaskButtons = ({ ? undefined : `dynflow_enable_console ${__('Setting is off')}`; + const overflowItems = [ + { + value: 'reload', + ouiaId: 'task-buttons-reload-button', + content: ( + <> + + + +   + {taskReload ? __('Stop auto-reloading') : __('Start auto-reloading')} + + ), + onClick: taskProgressToggle, + }, + { + value: 'resume', + ouiaId: 'task-buttons-resume-button', + content: __('Resume'), + onClick: () => { + if (!taskReload) { + taskReloadStart(id); + } + + resumeTaskRequest(id, action); + }, + isDisabled: !canEdit || !resumable, + tooltip: editActionsTitle, + }, + ...(parentTask + ? [ + { + value: 'parent', + ouiaId: 'task-buttons-parent-button', + content: __('Parent task'), + to: `/foreman_tasks/tasks/${parentTask}`, + }, + ] + : []), + ...(hasSubTasks + ? [ + { + value: 'subtasks', + ouiaId: 'task-buttons-subtask-button', + content: __('Sub tasks'), + to: `/foreman_tasks/tasks/${id}/sub_tasks`, + }, + ] + : []), + { + value: 'unlock', + ouiaId: 'task-buttons-unlock-button', + content: __('Unlock'), + onClick: () => setUnlockModalOpen(true), + isDisabled: !canEdit || state !== 'paused', + tooltip: editActionsTitle, + }, + { + value: 'force-unlock', + ouiaId: 'task-buttons-force-unlock-button', + content: __('Force Unlock'), + onClick: () => setForceUnlockModalOpen(true), + isDisabled: !canEdit || state === 'stopped', + tooltip: editActionsTitle, + }, + ]; + return ( - <> - - - - - {parentTask && ( + + - )} - {hasSubTasks && ( + + - )} - - - + + + + ); }; diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.scss b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.scss new file mode 100644 index 000000000..7f4701bbe --- /dev/null +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.scss @@ -0,0 +1,45 @@ +.task-buttons { + .spin { + -webkit-animation: spin 1s infinite linear; + -moz-animation: spin 1s infinite linear; + -o-animation: spin 1s infinite linear; + animation: spin 1s infinite linear; + -webkit-transform-origin: 50% 50%; + transform-origin: 50% 50%; + -ms-transform-origin: 50% 50%; /* IE 9 */ + } + + @-moz-keyframes spin { + from { + -moz-transform: rotate(0deg); + } + + to { + -moz-transform: rotate(360deg); + } + } + + @-webkit-keyframes spin { + from { + -webkit-transform: rotate(0deg); + } + + to { + -webkit-transform: rotate(360deg); + } + } + + @keyframes spin { + from { + transform: rotate(0deg); + } + + to { + transform: rotate(360deg); + } + } + + .dynflow-button > span { + pointer-events: auto; + } +} diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js index c71a185af..affab7a53 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js @@ -119,7 +119,6 @@ const TaskInfo = props => { return ( - {details.map((items, key) => ( diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js index 2ca7caf8a..d21ff7b67 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js @@ -4,41 +4,26 @@ import '@testing-library/jest-dom'; import Task from '../Task'; +jest.mock('foremanReact/components/common/dates/RelativeDateTime', () => { + const RelativeDateTime = ({ date, defaultValue }) => ( + {date || defaultValue} + ); + + return RelativeDateTime; +}); + describe('Task', () => { - it('renders task controls from TaskButtons with minimal props', () => { - render( - - ); - expect( - screen.getByRole('button', { name: /start auto-reloading/i }) - ).toBeInTheDocument(); - }); + it('renders task overview metadata via TaskInfo', () => { + render(); - it('renders parent task and sub tasks links when provided', () => { - render( - - ); - expect(screen.getByRole('link', { name: /parent task/i })).toHaveAttribute( - 'href', - '/foreman_tasks/tasks/parent-id' - ); - expect(screen.getByRole('link', { name: /sub tasks/i })).toHaveAttribute( - 'href', - '/foreman_tasks/tasks/test/sub_tasks' - ); + expect(screen.getByText(/name:/i)).toBeInTheDocument(); + expect(screen.getByText('Refresh hosts')).toBeInTheDocument(); + expect(screen.getByText(/result:/i)).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /cancel/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /^task actions$/i }) + ).not.toBeInTheDocument(); }); }); diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js index b0ecd89d8..68611de94 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskButtons.test.js @@ -1,5 +1,6 @@ import React from 'react'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import '@testing-library/jest-dom'; import { TaskButtons } from '../TaskButtons'; @@ -16,29 +17,42 @@ const defaultProps = { setForceUnlockModalOpen, }; +const openTaskActionsMenu = async () => { + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /^task actions$/i })); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + + await waitFor(() => { + expect(screen.getByRole('menu')).toBeInTheDocument(); + }); +}; + describe('TaskButtons', () => { beforeEach(() => { jest.clearAllMocks(); }); describe('rendering', () => { - it('renders reload button with correct text when taskReload is false', () => { + it('shows start auto-reloading in overflow when taskReload is false', async () => { render(); + await openTaskActionsMenu(); expect( - screen.getByRole('button', { name: /start auto-reloading/i }) + screen.getByRole('menuitem', { name: /start auto-reloading/i }) ).toBeInTheDocument(); expect( - screen.queryByRole('button', { name: /stop auto-reloading/i }) + screen.queryByRole('menuitem', { name: /stop auto-reloading/i }) ).not.toBeInTheDocument(); }); - it('renders reload button with correct text when taskReload is true', () => { + it('shows stop auto-reloading in overflow when taskReload is true', async () => { render(); + await openTaskActionsMenu(); expect( - screen.getByRole('button', { name: /stop auto-reloading/i }) + screen.getByRole('menuitem', { name: /stop auto-reloading/i }) ).toBeInTheDocument(); expect( - screen.queryByRole('button', { name: /start auto-reloading/i }) + screen.queryByRole('menuitem', { name: /start auto-reloading/i }) ).not.toBeInTheDocument(); }); @@ -54,15 +68,21 @@ describe('TaskButtons', () => { expect(dynflowLink).toHaveAttribute('target', '_blank'); }); - it('disables dynflow console link when dynflowEnableConsole is false', () => { - render(); + it('marks dynflow console link disabled when dynflowEnableConsole is false', () => { + render( + + ); const dynflowLink = screen.getByRole('link', { name: /dynflow console/i, }); - expect(dynflowLink).not.toBeDisabled(); + expect(dynflowLink).toHaveAttribute('aria-disabled', 'true'); }); - it('enables dynflow console link when dynflowEnableConsole is true', () => { + it('does not disable dynflow console link when dynflowEnableConsole is true', () => { render( { const dynflowLink = screen.getByRole('link', { name: /dynflow console/i, }); - expect(dynflowLink).not.toBeDisabled(); + expect(dynflowLink).not.toHaveAttribute('aria-disabled', 'true'); }); - it('disables resume and cancel buttons when canEdit is false', () => { + it('disables resume overflow item and cancel button when canEdit is false', async () => { render(); - expect(screen.getByRole('button', { name: /resume/i })).toBeDisabled(); expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled(); + await openTaskActionsMenu(); + expect(screen.getByRole('menuitem', { name: /^resume$/i })).toBeDisabled(); }); - it('disables resume button when resumable is false', () => { + it('disables resume overflow item when resumable is false', async () => { render(); - expect(screen.getByRole('button', { name: /resume/i })).toBeDisabled(); + await openTaskActionsMenu(); + expect(screen.getByRole('menuitem', { name: /^resume$/i })).toBeDisabled(); }); it('disables cancel button when cancellable is false', () => { @@ -92,66 +114,63 @@ describe('TaskButtons', () => { expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled(); }); - it('disables unlock button when state is not paused', () => { + it('disables unlock overflow item when state is not paused', async () => { render(); - expect(screen.getByRole('button', { name: /^unlock$/i })).toBeDisabled(); + await openTaskActionsMenu(); + expect(screen.getByRole('menuitem', { name: /^unlock$/i })).toBeDisabled(); }); - it('enables unlock button when state is paused and canEdit is true', () => { + it('enables unlock overflow item when state is paused and canEdit is true', async () => { render(); - expect( - screen.getByRole('button', { name: /^unlock$/i }) - ).not.toBeDisabled(); + await openTaskActionsMenu(); + expect(screen.getByRole('menuitem', { name: /^unlock$/i })).not.toBeDisabled(); }); - it('disables force unlock button when state is stopped', () => { + it('disables force unlock overflow item when state is stopped', async () => { render(); - expect( - screen.getByRole('button', { name: /force unlock/i }) - ).toBeDisabled(); + await openTaskActionsMenu(); + expect(screen.getByRole('menuitem', { name: /force unlock/i })).toBeDisabled(); }); - it('enables force unlock button when state is not stopped and canEdit is true', () => { + it('enables force unlock overflow item when state is not stopped and canEdit is true', async () => { render(); - expect( - screen.getByRole('button', { name: /force unlock/i }) - ).not.toBeDisabled(); + await openTaskActionsMenu(); + expect(screen.getByRole('menuitem', { name: /force unlock/i })).not.toBeDisabled(); }); - it('renders parent task button when parentTask is provided', () => { + it('includes parent task overflow link when parentTask is provided', async () => { render(); - const parentButton = screen.getByRole('link', { name: /parent task/i }); - expect(parentButton).toBeInTheDocument(); - expect(parentButton).toHaveAttribute( - 'href', - '/foreman_tasks/tasks/parent-123' - ); + await openTaskActionsMenu(); + expect( + screen.getByRole('menuitem', { name: /parent task/i }) + ).toHaveAttribute('href', '/foreman_tasks/tasks/parent-123'); }); - it('does not render parent task button when parentTask is not provided', () => { + it('does not include parent overflow item when parentTask is not provided', async () => { render(); + await openTaskActionsMenu(); expect( - screen.queryByRole('link', { name: /parent task/i }) + screen.queryByRole('menuitem', { name: /parent task/i }) ).not.toBeInTheDocument(); }); - it('renders sub tasks button when hasSubTasks is true', () => { + it('includes sub tasks overflow link when hasSubTasks is true', async () => { render(); - const subTasksButton = screen.getByRole('link', { name: /sub tasks/i }); - expect(subTasksButton).toBeInTheDocument(); - expect(subTasksButton).toHaveAttribute( - 'href', - '/foreman_tasks/tasks/task-123/sub_tasks' - ); + await openTaskActionsMenu(); + expect( + screen.getByRole('menuitem', { name: /sub tasks/i }) + ).toHaveAttribute('href', '/foreman_tasks/tasks/task-123/sub_tasks'); }); - it('does not render sub tasks button when hasSubTasks is false', () => { + it('does not include sub tasks overflow item when hasSubTasks is false', async () => { render(); + await openTaskActionsMenu(); expect( - screen.queryByRole('link', { name: /sub tasks/i }) + screen.queryByRole('menuitem', { name: /sub tasks/i }) ).not.toBeInTheDocument(); }); }); + describe('user interactions', () => { const cancelTaskRequest = jest.fn(); const resumeTaskRequest = jest.fn(); @@ -174,44 +193,41 @@ describe('TaskButtons', () => { state: 'paused', }; - it('calls taskProgressToggle when reload button is clicked', () => { + it('calls taskProgressToggle when overflow reload item is clicked', async () => { render(); - const reloadButton = screen.getByRole('button', { - name: /start auto-reloading/i, - }); - fireEvent.click(reloadButton); + await openTaskActionsMenu(); + fireEvent.click( + screen.getByRole('menuitem', { name: /start auto-reloading/i }) + ); expect(taskProgressToggle).toHaveBeenCalled(); }); - it('calls taskReloadStart and resumeTaskRequest when resume button is clicked', () => { + it('calls taskReloadStart and resumeTaskRequest when resume menu item is clicked', async () => { render(); - const resumeButton = screen.getByRole('button', { name: /resume/i }); - fireEvent.click(resumeButton); + await openTaskActionsMenu(); + fireEvent.click(screen.getByRole('menuitem', { name: /^resume$/i })); expect(taskReloadStart).toHaveBeenCalledWith(id); expect(resumeTaskRequest).toHaveBeenCalledWith(id, action); }); - it('calls taskReloadStart and cancelTaskRequest when cancel button is clicked', () => { + it('calls taskReloadStart and cancelTaskRequest when cancel button is clicked', async () => { render(); - const cancelButton = screen.getByRole('button', { name: /cancel/i }); - fireEvent.click(cancelButton); + await userEvent.click(screen.getByRole('button', { name: /cancel/i })); expect(taskReloadStart).toHaveBeenCalledWith(id); expect(cancelTaskRequest).toHaveBeenCalledWith(id, action); }); - it('calls setUnlockModalOpen when unlock button is clicked', () => { + it('calls setUnlockModalOpen when unlock menu item is clicked', async () => { render(); - const unlockButton = screen.getByRole('button', { name: /^unlock$/i }); - fireEvent.click(unlockButton); + await openTaskActionsMenu(); + fireEvent.click(screen.getByRole('menuitem', { name: /^unlock$/i })); expect(setUnlockModalOpen).toHaveBeenCalledWith(true); }); - it('calls setForceUnlockModalOpen when force unlock button is clicked', () => { + it('calls setForceUnlockModalOpen when force unlock menu item is clicked', async () => { render(); - const forceUnlockButton = screen.getByRole('button', { - name: /force unlock/i, - }); - fireEvent.click(forceUnlockButton); + await openTaskActionsMenu(); + fireEvent.click(screen.getByRole('menuitem', { name: /force unlock/i })); expect(setForceUnlockModalOpen).toHaveBeenCalledWith(true); }); }); diff --git a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js index a71603ed6..131ab99e2 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js +++ b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js @@ -11,7 +11,6 @@ import Raw from './Components/Raw'; import ExecutionDetails from './ExecutionDetails'; import Dependencies from './Components/Dependencies'; import { TASKS_PATH, VIEW_FOREMAN_TASKS } from './TaskDetailsConstants'; -import { getTaskID } from './TasksDetailsHelper'; import { TaskSkeleton } from './Components/TaskSkeleton'; import './TaskDetails.scss'; @@ -37,9 +36,9 @@ const TaskDetails = ({ apiStatus, apiErrorMessage, apiErrorCode, + id, ...props }) => { - const id = getTaskID(); const { taskReload, isLoading } = props; const [activeTab, setActiveTab] = useState(TASK_DETAILS_TAB_KEYS.EXECUTION); const hasViewPermission = usePermissions([VIEW_FOREMAN_TASKS]); @@ -166,6 +165,7 @@ const TaskDetails = ({ }; TaskDetails.propTypes = { + id: PropTypes.string.isRequired, label: PropTypes.string, result: PropTypes.string, runningSteps: PropTypes.array, diff --git a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.scss b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.scss index d7025f6e9..275c0d170 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.scss +++ b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.scss @@ -6,45 +6,6 @@ .container { margin: 0; } - .spin { - -webkit-animation: spin 1s infinite linear; - -moz-animation: spin 1s infinite linear; - -o-animation: spin 1s infinite linear; - animation: spin 1s infinite linear; - -webkit-transform-origin: 50% 50%; - transform-origin: 50% 50%; - -ms-transform-origin: 50% 50%; /* IE 9 */ - } - - @-moz-keyframes spin { - from { - -moz-transform: rotate(0deg); - } - to { - -moz-transform: rotate(360deg); - } - } - - @-webkit-keyframes spin { - from { - -webkit-transform: rotate(0deg); - } - to { - -webkit-transform: rotate(360deg); - } - } - - @keyframes spin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } - } - .dynflow-button > span { - pointer-events: auto; - } pre { white-space: pre-wrap; diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js index f5d87ffc8..ab09d5d51 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js @@ -1,4 +1,5 @@ export const minProps = { + id: 'a15dd820-32f1-4ced-9ab7-c0fab8234c47', cancelStep: jest.fn(), taskReload: false, taskReloadStop: jest.fn(), diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js index 9638ffa56..b046ad0d1 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js @@ -1,5 +1,6 @@ import React from 'react'; -import { fireEvent, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; import { configureStore } from '@reduxjs/toolkit'; import { Provider } from 'react-redux'; @@ -51,6 +52,10 @@ describe('TaskDetails', () => { ); }); + afterEach(() => { + jest.clearAllMocks(); + }); + it(`shows ResourceLoadFailedEmptyState when ${VIEW_FOREMAN_TASKS} is absent`, () => { mockUseForemanPermissions.mockImplementation(() => new Set()); @@ -126,7 +131,6 @@ describe('TaskDetails', () => { it('renders four tabs with expected labels', () => { renderTaskDetails(); - expect(document.getElementById('task-details-tabs')).toBeInTheDocument(); expect( screen.getByRole('tab', { name: /execution details/i }) ).toBeInTheDocument(); @@ -192,18 +196,9 @@ describe('TaskDetails', () => { expect(taskReloadStop).toHaveBeenCalled(); }); - it('renders task overview with action name when loaded', () => { - renderTaskDetails({ ...taskDetailsWithExecutionTabDefaults }); - - expect(screen.getByText('Refresh foo')).toBeInTheDocument(); - }); - it('shows execution details panel on the default tab when loaded', () => { renderTaskDetails({ ...taskDetailsWithExecutionTabDefaults }); - expect( - document.getElementById('execution-details-panel') - ).toBeInTheDocument(); expect( screen.getByRole('heading', { name: /^no errors found$/i }) ).toBeInTheDocument(); @@ -216,7 +211,7 @@ describe('TaskDetails', () => { }); expect( - document.getElementById('execution-details-panel') + screen.queryByRole('heading', { name: /^no errors found$/i }) ).not.toBeInTheDocument(); expect( screen.getByRole('tab', { name: /execution details/i }) @@ -233,19 +228,19 @@ describe('TaskDetails', () => { expect(screen.getByText('See logs')).toBeInTheDocument(); }); - it('renders raw output when the Raw tab is selected', () => { + it('renders raw output when the Raw tab is selected', async () => { renderTaskDetails({ ...fixtureWithOverviewMessages }); - fireEvent.click(screen.getByRole('tab', { name: /raw/i })); + await userEvent.click(screen.getByRole('tab', { name: /raw/i })); expect(screen.getByText(/raw output/i)).toBeInTheDocument(); expect(screen.getByText(/partial output/i)).toBeInTheDocument(); }); - it('renders dependency tables when the Dependencies tab is selected', () => { + it('renders dependency tables when the Dependencies tab is selected', async () => { renderTaskDetails({ ...fixtureWithDependencies }); - fireEvent.click(screen.getByRole('tab', { name: /dependencies/i })); + await userEvent.click(screen.getByRole('tab', { name: /dependencies/i })); expect( screen.getByRole('link', { name: 'Foo Bar Action' }) diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js new file mode 100644 index 000000000..18031c623 --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js @@ -0,0 +1,174 @@ +import React, { useState } from 'react'; +import PropTypes from 'prop-types'; +import { + Flex, + FlexItem, + Split, + SplitItem, + Title, +} from '@patternfly/react-core'; +import { translate as __ } from 'foremanReact/common/I18n'; +import { STATUS } from 'foremanReact/constants'; +import { usePermissions } from 'foremanReact/common/hooks/Permissions/permissionHooks'; +import { TaskButtons } from '../../Components/TaskDetails/Components/TaskButtons'; +import { VIEW_FOREMAN_TASKS } from '../../Components/TaskDetails/TaskDetailsConstants'; +import { + ForceUnlockConfirmationModal, + UnlockConfirmationModal, +} from '../../Components/common/ClickConfirmation'; +import { taskResultIconEl } from '../../Components/common/taskResultIcon'; +import './TaskDetailsHeader.scss'; + +const TitleComponent = ({ action, state, result }) => ( + + + + {action || __('Task Details')} + + + {taskResultIconEl(state, result)} + +); + +TitleComponent.propTypes = { + action: PropTypes.string, + state: PropTypes.string, + result: PropTypes.string, +}; + +TitleComponent.defaultProps = { + action: '', + state: '', + result: '', +}; + +const TaskDetailsHeader = props => { + const { + taskReload, + id, + forceCancelTaskRequest, + unlockTaskRequest, + action, + taskReloadStart, + taskReloadStop, + executionPlan, + apiStatus, + } = props; + const hasViewPermission = usePermissions([VIEW_FOREMAN_TASKS]); + const showActions = hasViewPermission && apiStatus !== STATUS.ERROR; + const resumable = executionPlan ? executionPlan.state === 'paused' : false; + const cancellable = executionPlan ? executionPlan.cancellable : false; + + const forceUnlock = () => { + if (!taskReload) { + taskReloadStart(id); + } + + forceCancelTaskRequest(id, action); + }; + + const unlock = () => { + if (!taskReload) { + taskReloadStart(id); + } + + unlockTaskRequest(id, action); + }; + + const taskProgressToggle = () => { + if (taskReload) { + taskReloadStop(); + } else { + taskReloadStart(id); + } + }; + + const [unlockModalOpen, setUnlockModalOpen] = useState(false); + const [forceUnlockModalOpen, setForceUnlockModalOpen] = useState(false); + + return ( + + {showActions && ( + <> + setUnlockModalOpen(false)} + /> + setForceUnlockModalOpen(false)} + /> + + )} + + + + + {showActions && ( + + + + )} + + + ); +}; + +TaskDetailsHeader.propTypes = { + taskReload: PropTypes.bool, + id: PropTypes.string, + forceCancelTaskRequest: PropTypes.func, + unlockTaskRequest: PropTypes.func, + action: PropTypes.string, + state: PropTypes.string, + result: PropTypes.string, + taskReloadStart: PropTypes.func, + taskReloadStop: PropTypes.func, + executionPlan: PropTypes.shape({ + state: PropTypes.string, + cancellable: PropTypes.bool, + }), + apiStatus: PropTypes.oneOf(Object.keys(STATUS)), +}; + +TaskDetailsHeader.defaultProps = { + taskReload: false, + id: '', + forceCancelTaskRequest: () => null, + unlockTaskRequest: () => null, + action: '', + state: '', + result: '', + taskReloadStart: () => null, + taskReloadStop: () => null, + executionPlan: {}, + apiStatus: STATUS.RESOLVED, +}; + +export default TaskDetailsHeader; diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.scss b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.scss new file mode 100644 index 000000000..2824cb904 --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.scss @@ -0,0 +1,5 @@ +.foreman-tasks-task-details-header { + .foreman-tasks-task-details-header__title { + margin: 0; + } +} diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js index 31187a2e6..439c7f238 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage.js @@ -1,48 +1,20 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { useSelector } from 'react-redux'; -import { Flex, FlexItem, TextContent, Text } from '@patternfly/react-core'; import PageLayout from 'foremanReact/routes/common/PageLayout/PageLayout'; import { translate as __, sprintf } from 'foremanReact/common/I18n'; -import TaskDetails from '../../Components/TaskDetails'; -import { - selectAction, - selectResult, - selectState, -} from '../../Components/TaskDetails/TaskDetailsSelectors'; -import { taskResultIconEl } from '../../Components/common/taskResultIcon'; +import TaskDetails from '../../Components/TaskDetails/TaskDetails'; +import TaskDetailsHeader from './TaskDetailsHeader'; const TaskDetailsPage = props => { - const { id } = props.match.params; - const action = useSelector(selectAction); - const taskState = useSelector(selectState); - const taskResult = useSelector(selectResult); + const { match, action } = props; + const { id } = match.params; const headerText = action ? sprintf(__('Details of %s task'), action) : __('Task Details'); - const header = ( - - - - {headerText} - - - - {taskResultIconEl(taskState, taskResult)} - - - ); - return ( } header={headerText} searchable={false} breadcrumbOptions={{ @@ -69,6 +41,11 @@ TaskDetailsPage.propTypes = { id: PropTypes.string.isRequired, }).isRequired, }).isRequired, + action: PropTypes.string, +}; + +TaskDetailsPage.defaultProps = { + action: '', }; export default TaskDetailsPage; diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js new file mode 100644 index 000000000..dd95881fd --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js @@ -0,0 +1,142 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { STATUS } from 'foremanReact/constants'; + +import TaskDetailsHeader from '../TaskDetailsHeader'; +import { VIEW_FOREMAN_TASKS } from '../../../Components/TaskDetails/TaskDetailsConstants'; + +const mockUseForemanPermissions = jest.fn( + () => new Set(['view_foreman_tasks']) +); + +jest.mock('foremanReact/Root/Context/ForemanContext', () => ({ + ...jest.requireActual('foremanReact/Root/Context/ForemanContext'), + useForemanPermissions: (...args) => mockUseForemanPermissions(...args), +})); + +const baseHeaderProps = { + id: 'test-id', + action: 'Refresh hosts', + state: 'running', + result: 'pending', + taskReload: false, + taskReloadStart: jest.fn(), + taskReloadStop: jest.fn(), + forceCancelTaskRequest: jest.fn(), + unlockTaskRequest: jest.fn(), + cancelTaskRequest: jest.fn(), + resumeTaskRequest: jest.fn(), + canEdit: true, + dynflowEnableConsole: true, + externalId: 'ext-99', + executionPlan: {}, + apiStatus: STATUS.RESOLVED, +}; + +const expectHeaderActionsVisible = () => { + expect(screen.getByRole('button', { name: /cancel/i })).toBeInTheDocument(); + expect( + screen.getByRole('link', { name: /dynflow console/i }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /^task actions$/i }) + ).toBeInTheDocument(); +}; + +const expectHeaderActionsHidden = () => { + expect( + screen.queryByRole('button', { name: /cancel/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('link', { name: /dynflow console/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /^task actions$/i }) + ).not.toBeInTheDocument(); +}; + +describe('TaskDetailsHeader', () => { + beforeEach(() => { + mockUseForemanPermissions.mockImplementation( + () => new Set(['view_foreman_tasks']) + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('renders action heading and exposes cancel, dynflow link, and task actions toggle', () => { + render(); + + expect( + screen.getByRole('heading', { level: 1, name: 'Refresh hosts' }) + ).toBeInTheDocument(); + expectHeaderActionsVisible(); + }); + + it('shows running status icon next to the title when task state is running', () => { + render( + + ); + + expect( + screen.getByRole('heading', { level: 1, name: 'Running job' }) + ).toBeInTheDocument(); + expect(screen.getByTitle('Running')).toBeInTheDocument(); + }); + + it('shows error status icon next to the title when task stopped with error', () => { + render( + + ); + + expect( + screen.getByRole('heading', { level: 1, name: 'Failed job' }) + ).toBeInTheDocument(); + expect(screen.getByTitle('Error')).toBeInTheDocument(); + }); + + it('uses generic title when action is unset', () => { + render(); + + expect( + screen.getByRole('heading', { level: 1, name: 'Task Details' }) + ).toBeInTheDocument(); + }); + + it.each([ + [ + `${VIEW_FOREMAN_TASKS} is absent`, + () => { + mockUseForemanPermissions.mockImplementation(() => new Set()); + }, + {}, + ], + [ + 'apiStatus is ERROR', + () => {}, + { apiStatus: STATUS.ERROR }, + ], + ])('hides task actions when %s', (_label, setupPermissions, propOverrides) => { + setupPermissions(); + + render(); + + expect( + screen.getByRole('heading', { level: 1, name: 'Refresh hosts' }) + ).toBeInTheDocument(); + expectHeaderActionsHidden(); + }); +}); diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js index 0e1d040ec..0e873d2bd 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js @@ -1,18 +1,32 @@ import React from 'react'; -import { render, screen, within } from '@testing-library/react'; +import { screen, within } from '@testing-library/react'; import '@testing-library/jest-dom'; -import { combineReducers, configureStore } from '@reduxjs/toolkit'; import { createMemoryHistory } from 'history'; import { Router } from 'react-router-dom'; import { IntlProvider } from 'react-intl'; -import { Provider } from 'react-redux'; -import breadcrumbBarReducer from 'foremanReact/components/BreadcrumbBar/BreadcrumbBarReducer'; +import { rtlHelpers } from 'foremanReact/common/testHelpers'; import { STATUS } from 'foremanReact/constants'; -import intervalsReducer from 'foremanReact/redux/middlewares/IntervalMiddleware/IntervalReducer'; -import { FOREMAN_TASK_DETAILS, VIEW_FOREMAN_TASKS } from '../../../Components/TaskDetails/TaskDetailsConstants'; -import TaskDetailsPage from '../TaskDetailsPage'; +import { + FOREMAN_TASK_DETAILS, + VIEW_FOREMAN_TASKS, +} from '../../../Components/TaskDetails/TaskDetailsConstants'; +import TaskDetailsPage from '../index'; + +const { renderWithStore } = rtlHelpers; + +jest.mock('../../../Components/TaskDetails/TaskDetailsActions', () => { + const actual = jest.requireActual( + '../../../Components/TaskDetails/TaskDetailsActions' + ); + + return { + ...actual, + taskReloadStart: jest.fn(() => () => undefined), + taskReloadStop: jest.fn(() => () => undefined), + }; +}); const mockUseForemanPermissions = jest.fn( () => new Set(['view_foreman_tasks']) @@ -23,8 +37,6 @@ jest.mock('foremanReact/Root/Context/ForemanContext', () => ({ useForemanPermissions: (...args) => mockUseForemanPermissions(...args), })); -const TASK_DETAILS_TITLE_ROW_OUIA_ID = 'foreman-tasks-task-details-title-row'; - const routerPropsBase = { history: { push: jest.fn(), replace: jest.fn(), go: jest.fn() }, location: { @@ -57,36 +69,37 @@ const baseTaskPayload = { result: '', }; -const createStoreForTaskPayload = overrides => ({ +const createStoreForTaskPayload = (overrides, apiStatus = STATUS.RESOLVED) => ({ API: { [FOREMAN_TASK_DETAILS]: { response: { ...baseTaskPayload, ...overrides }, - status: STATUS.RESOLVED, + status: apiStatus, payload: {}, }, }, }); -const rootReducer = combineReducers({ - API: (state = {}, action) => state, - intervals: intervalsReducer, - breadcrumbBar: breadcrumbBarReducer, - foremanTasks: (state = {}, action) => state, -}); +const expectHeaderActionsHidden = () => { + expect( + screen.queryByRole('button', { name: /cancel/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /^task actions$/i }) + ).not.toBeInTheDocument(); +}; -const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { +const getHeaderTitleArea = title => + screen.getByRole('heading', { level: 1, name: title }).parentElement + .parentElement; + +const renderPage = ( + apiPayloadOverrides = {}, + propsOverrides = {}, + apiStatus = STATUS.RESOLVED +) => { const history = createMemoryHistory({ initialEntries: [`/foreman_tasks/tasks/${matchDefault.params.id}`], }); - const store = configureStore({ - reducer: rootReducer, - preloadedState: createStoreForTaskPayload(apiPayloadOverrides), - middleware: getDefaultMiddleware => - getDefaultMiddleware({ - immutableCheck: false, - serializableCheck: false, - }), - }); window.history.pushState( {}, @@ -94,43 +107,30 @@ const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { `/foreman_tasks/tasks/${matchDefault.params.id}` ); - return render( - - - - - - - + return renderWithStore( + + + + + , + createStoreForTaskPayload(apiPayloadOverrides, apiStatus) ); }; -const breadcrumbTitleHeadings = () => - screen.getAllByRole('heading', { level: 1 }).filter( - heading => heading.getAttribute('data-ouia-component-id') === 'breadcrumb_title' - ); - -/** - * Title row (`customHeader` root `Flex`): same OUIA pattern as `Locks.test.js`. - */ -const taskDetailsTitleRegion = container => { - const el = container.querySelector( - `[data-ouia-component-id="${TASK_DETAILS_TITLE_ROW_OUIA_ID}"]` - ); - - expect(el).toBeTruthy(); - - return el; -}; - describe('TaskDetailsPage', () => { beforeEach(() => { - mockUseForemanPermissions.mockImplementation(() => new Set(['view_foreman_tasks'])); + mockUseForemanPermissions.mockImplementation( + () => new Set(['view_foreman_tasks']) + ); + }); + + afterEach(() => { + jest.clearAllMocks(); }); describe('permissions', () => { @@ -147,11 +147,12 @@ describe('TaskDetailsPage', () => { expect( screen.getByRole('heading', { level: 1, - name: /Details of Run job task/, + name: 'Run job', }) ).toBeInTheDocument(); - expect(screen.queryByRole('heading', { name: /Permission denied/i })).not.toBeInTheDocument(); - expect(mockUseForemanPermissions).toHaveBeenCalled(); + expect( + screen.queryByRole('heading', { name: /Permission denied/i }) + ).not.toBeInTheDocument(); }); it(`shows ResourceLoadFailedEmptyState and lists ${VIEW_FOREMAN_TASKS} when it is absent`, () => { @@ -174,7 +175,10 @@ describe('TaskDetailsPage', () => { expect( screen.getByRole('navigation', { name: 'Breadcrumb' }) ).toBeInTheDocument(); - expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + expect( + screen.queryByRole('tab', { name: /^task$/i }) + ).not.toBeInTheDocument(); + expectHeaderActionsHidden(); }); it('denies access when user only has edit_foreman_tasks without view', () => { @@ -191,24 +195,28 @@ describe('TaskDetailsPage', () => { expect( screen.getByRole('navigation', { name: 'Breadcrumb' }) ).toBeInTheDocument(); - expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); + expect( + screen.queryByRole('tab', { name: /^task$/i }) + ).not.toBeInTheDocument(); + expectHeaderActionsHidden(); }); - }); + it('hides header actions when the task API returns an error', () => { + renderPage({ action: 'Failed load' }, {}, STATUS.ERROR); - const expectToolbarHeadingText = substring => { - const headings = breadcrumbTitleHeadings(); - - expect(headings.length).toBeGreaterThan(0); - - headings.forEach(heading => { - expect(heading).toHaveTextContent(substring); + expect( + screen.getByRole('heading', { name: /unable to load task/i }) + ).toBeInTheDocument(); + expectHeaderActionsHidden(); }); - }; + }); - it('shows generic title and breadcrumb from route id when action is unset', () => { - const page = renderPage({}); - expectToolbarHeadingText('Task Details'); + it('shows running status icon when action is unset', () => { + renderPage({}); + + expect( + screen.getByRole('heading', { level: 1, name: 'Task Details' }) + ).toBeInTheDocument(); expect(screen.getByRole('navigation', { name: 'Breadcrumb' })).toHaveTextContent( 'Tasks' @@ -219,18 +227,21 @@ describe('TaskDetailsPage', () => { ) ).toBeInTheDocument(); - const titleRegion = taskDetailsTitleRegion(page.container); + const titleArea = getHeaderTitleArea('Task Details'); - expect( - within(titleRegion).getAllByRole('img', { hidden: true }).length - ).toBeGreaterThan(0); - expect(titleRegion.querySelector('[class*="danger"]')).toBeNull(); + expect(within(titleArea).getByTitle('Running')).toBeInTheDocument(); + expect(within(titleArea).queryByTitle('Error')).not.toBeInTheDocument(); }); it('uses task action for title and breadcrumb when loaded', () => { renderPage({ action: 'Refresh hosts' }); - expectToolbarHeadingText('Details of Refresh hosts task'); + expect( + screen.getByRole('heading', { level: 1, name: 'Refresh hosts' }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /cancel/i }) + ).toBeInTheDocument(); expect(screen.getByRole('link', { name: /^Tasks$/ })).toHaveAttribute( 'href', @@ -243,18 +254,19 @@ describe('TaskDetailsPage', () => { ).toBeInTheDocument(); }); - it('shows error status styling in the heading when task is stopped with error result', () => { - const page = renderPage({ + it('shows error status icon when task is stopped with error result', () => { + renderPage({ action: 'Some action', state: 'stopped', result: 'error', }); - expectToolbarHeadingText('Details of Some action task'); - expect( - taskDetailsTitleRegion(page.container).querySelector('[class*="danger"]') - ).toBeTruthy(); + screen.getByRole('heading', { level: 1, name: 'Some action' }) + ).toBeInTheDocument(); + expect( + within(getHeaderTitleArea('Some action')).getByTitle('Error') + ).toBeInTheDocument(); expect( within(screen.getByRole('navigation', { name: 'Breadcrumb' })).getByText( diff --git a/webpack/ForemanTasks/Components/TaskDetails/index.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/index.js similarity index 86% rename from webpack/ForemanTasks/Components/TaskDetails/index.js rename to webpack/ForemanTasks/Routes/ShowTaskDetails/index.js index 8c8f6fa26..59f75ebd7 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/index.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/index.js @@ -1,8 +1,7 @@ import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; -import TaskDetails from './TaskDetails'; -import * as taskDetailsActions from './TaskDetailsActions'; -import * as taskActions from '../TaskActions'; +import * as taskDetailsActions from '../../Components/TaskDetails/TaskDetailsActions'; +import * as taskActions from '../../Components/TaskActions'; import { selectEndedAt, selectStartAt, @@ -38,9 +37,11 @@ import { selectIsLoading, selectDependsOn, selectBlocks, -} from './TaskDetailsSelectors'; +} from '../../Components/TaskDetails/TaskDetailsSelectors'; +import TaskDetailsPage from './TaskDetailsPage'; -const mapStateToProps = state => ({ +const mapStateToProps = (state, ownProps) => ({ + id: ownProps.match.params.id, startAt: selectStartAt(state), startBefore: selectStartBefore(state), startedAt: selectStartedAt(state), @@ -80,4 +81,4 @@ const mapStateToProps = state => ({ const mapDispatchToProps = dispatch => bindActionCreators({ ...taskActions, ...taskDetailsActions }, dispatch); -export default connect(mapStateToProps, mapDispatchToProps)(TaskDetails); +export default connect(mapStateToProps, mapDispatchToProps)(TaskDetailsPage); diff --git a/webpack/Routes/routes.js b/webpack/Routes/routes.js index c5e786412..a10b718bc 100644 --- a/webpack/Routes/routes.js +++ b/webpack/Routes/routes.js @@ -1,6 +1,6 @@ import React from 'react'; import TasksTableIndexPage from '../ForemanTasks/Components/TasksTable/TasksIndexPage'; -import TaskDetailsPage from '../ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage'; +import TaskDetailsPage from '../ForemanTasks/Routes/ShowTaskDetails'; const ForemanTasksRoutes = [ { diff --git a/webpack/Routes/routes.test.js b/webpack/Routes/routes.test.js index 62553363d..d6c00d84c 100644 --- a/webpack/Routes/routes.test.js +++ b/webpack/Routes/routes.test.js @@ -13,7 +13,7 @@ jest.mock( ); jest.mock( - '../ForemanTasks/Routes/ShowTaskDetails/TaskDetailsPage', + '../ForemanTasks/Routes/ShowTaskDetails', () => function TaskDetailsPageStub() { return
; diff --git a/webpack/index.js b/webpack/index.js index 8c1e3aba9..a45ef52b7 100644 --- a/webpack/index.js +++ b/webpack/index.js @@ -3,13 +3,8 @@ /* eslint-disable import/extensions */ import componentRegistry from 'foremanReact/components/componentRegistry'; import TasksDashboard from './ForemanTasks/Components/TasksDashboard'; -import TaskDetails from './ForemanTasks/Components/TaskDetails'; componentRegistry.register({ name: 'TasksDashboard', type: TasksDashboard, }); -componentRegistry.register({ - name: 'TaskDetails', - type: TaskDetails, -});