From e7f2651a7586d6138a8beb2b1b46d8bf04a7497f Mon Sep 17 00:00:00 2001 From: Andrei Lakatos Date: Wed, 24 Jun 2026 13:02:56 -0400 Subject: [PATCH 1/5] Fixes #39349 - implement new button structure with kebab menu --- .../Components/TaskDetails/Components/Task.js | 70 +++++- .../TaskDetails/Components/TaskButtons.js | 216 +++++++++--------- .../Components/__tests__/Task.test.js | 72 ++++-- .../Components/__tests__/TaskButtons.test.js | 150 ++++++------ .../TaskDetails/__tests__/TaskDetails.test.js | 4 +- 5 files changed, 310 insertions(+), 202 deletions(-) diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js b/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js index 75b00d851..8f65bdc0f 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js @@ -1,12 +1,47 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; +import { + Flex, + FlexItem, + Split, + SplitItem, + Stack, + StackItem, + Title, +} from '@patternfly/react-core'; import TaskInfo from './TaskInfo'; import { ForceUnlockConfirmationModal, UnlockConfirmationModal, } from '../../common/ClickConfirmation'; +import { taskResultIconEl } from '../../common/taskResultIcon'; import { TaskButtons } from './TaskButtons'; +const TitleComponent = ({ action, state, result }) => ( + + + + {action} + + + {taskResultIconEl(state, result)} + +); + +TitleComponent.propTypes = { + action: PropTypes.string.isRequired, + state: PropTypes.string, + result: PropTypes.string, +}; + +TitleComponent.defaultProps = { + state: '', + result: '', +}; + const Task = props => { const { taskReload, @@ -43,13 +78,30 @@ const Task = props => { isOpen={forceUnlockModalOpen} setModalClosed={() => setForceUnlockModalOpen(false)} /> - - + + + + + + + + + + + + + + + ); }; @@ -60,6 +112,8 @@ Task.propTypes = { forceCancelTaskRequest: PropTypes.func, unlockTaskRequest: PropTypes.func, action: PropTypes.string, + state: PropTypes.string, + result: PropTypes.string, taskReloadStart: PropTypes.func, }; @@ -69,6 +123,8 @@ Task.defaultProps = { forceCancelTaskRequest: () => null, unlockTaskRequest: () => null, action: '', + state: '', + result: '', taskReloadStart: () => null, }; diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js index b1c425e42..d89f535db 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js @@ -1,7 +1,8 @@ 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'; export const TaskButtons = ({ @@ -30,120 +31,127 @@ 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/__tests__/Task.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js index 2ca7caf8a..b40f43ee9 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js @@ -4,41 +4,67 @@ import '@testing-library/jest-dom'; import Task from '../Task'; +jest.mock('foremanReact/components/common/dates/RelativeDateTime', () => { + const RelativeDateTime = ({ date, defaultValue }) => ( + {date || defaultValue} + ); + return RelativeDateTime; +}); + +jest.mock('../TaskInfo', () => { + const TaskInfo = () => null; + + return TaskInfo; +}); + +const baseTaskProps = { + id: 'test', + taskReloadStart: jest.fn(), + taskProgressToggle: jest.fn(), + cancelTaskRequest: jest.fn(), + resumeTaskRequest: jest.fn(), +}; + describe('Task', () => { - it('renders task controls from TaskButtons with minimal props', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('renders action heading and exposes cancel, dynflow link, and task actions toggle', () => { render( ); + expect( - screen.getByRole('button', { name: /start auto-reloading/i }) + screen.getByRole('heading', { level: 4, name: 'Refresh hosts' }) + ).toBeInTheDocument(); + 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(); }); - it('renders parent task and sub tasks links when provided', () => { + it('shows running status icon next to the title when task state is running', () => { 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.getByRole('heading', { level: 4, name: 'Running job' }) + ).toBeInTheDocument(); + expect(screen.getByTitle('Running')).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/__tests__/TaskDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js index 9638ffa56..f4c68d563 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js @@ -195,7 +195,9 @@ describe('TaskDetails', () => { it('renders task overview with action name when loaded', () => { renderTaskDetails({ ...taskDetailsWithExecutionTabDefaults }); - expect(screen.getByText('Refresh foo')).toBeInTheDocument(); + expect( + screen.getByRole('heading', { level: 4, name: 'Refresh foo' }) + ).toBeInTheDocument(); }); it('shows execution details panel on the default tab when loaded', () => { From 81b5e4860170882db3c02f081d0ac6fca9d25a43 Mon Sep 17 00:00:00 2001 From: Andrei Lakatos Date: Thu, 25 Jun 2026 10:28:35 -0400 Subject: [PATCH 2/5] Refs #39349 - move buttons to page title, provide state props to the details page --- .../Components/TaskDetails/Components/Task.js | 134 +-------------- .../TaskDetails/Components/TaskInfo.js | 1 - .../Components/__tests__/Task.test.js | 59 +------ .../TaskDetails/__tests__/TaskDetails.test.js | 8 - .../ShowTaskDetails/TaskDetailsHeader.js | 160 ++++++++++++++++++ .../ShowTaskDetails/TaskDetailsHeader.scss | 5 + .../Routes/ShowTaskDetails/TaskDetailsPage.js | 43 ++--- .../__tests__/TaskDetailsHeader.test.js | 84 +++++++++ .../__tests__/TaskDetailsPage.test.js | 152 ++++++++--------- .../ShowTaskDetails}/index.js | 12 +- webpack/Routes/routes.js | 2 +- webpack/Routes/routes.test.js | 2 +- webpack/index.js | 5 - 13 files changed, 349 insertions(+), 318 deletions(-) create mode 100644 webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js create mode 100644 webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.scss create mode 100644 webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js rename webpack/ForemanTasks/{Components/TaskDetails => Routes/ShowTaskDetails}/index.js (85%) diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js b/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js index 8f65bdc0f..861c53919 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/Task.js @@ -1,131 +1,13 @@ -import React, { useState } from 'react'; -import PropTypes from 'prop-types'; -import { - Flex, - FlexItem, - Split, - SplitItem, - Stack, - StackItem, - Title, -} from '@patternfly/react-core'; +import React from 'react'; +import { Stack, StackItem } from '@patternfly/react-core'; import TaskInfo from './TaskInfo'; -import { - ForceUnlockConfirmationModal, - UnlockConfirmationModal, -} from '../../common/ClickConfirmation'; -import { taskResultIconEl } from '../../common/taskResultIcon'; -import { TaskButtons } from './TaskButtons'; -const TitleComponent = ({ action, state, result }) => ( - - - - {action} - - - {taskResultIconEl(state, result)} - +const Task = props => ( + + + + + ); -TitleComponent.propTypes = { - action: PropTypes.string.isRequired, - state: PropTypes.string, - result: PropTypes.string, -}; - -TitleComponent.defaultProps = { - state: '', - result: '', -}; - -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)} - /> - - - - - - - - - - - - - - - - - ); -}; - -Task.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, -}; - -Task.defaultProps = { - taskReload: false, - id: '', - forceCancelTaskRequest: () => null, - unlockTaskRequest: () => null, - action: '', - state: '', - result: '', - taskReloadStart: () => null, -}; - export default Task; 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 b40f43ee9..bd16decf4 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js @@ -8,63 +8,16 @@ jest.mock('foremanReact/components/common/dates/RelativeDateTime', () => { const RelativeDateTime = ({ date, defaultValue }) => ( {date || defaultValue} ); - return RelativeDateTime; -}); - -jest.mock('../TaskInfo', () => { - const TaskInfo = () => null; - return TaskInfo; + return RelativeDateTime; }); -const baseTaskProps = { - id: 'test', - taskReloadStart: jest.fn(), - taskProgressToggle: jest.fn(), - cancelTaskRequest: jest.fn(), - resumeTaskRequest: jest.fn(), -}; - describe('Task', () => { - afterEach(() => { - jest.clearAllMocks(); - }); - - it('renders action heading and exposes cancel, dynflow link, and task actions toggle', () => { - render( - - ); - - expect( - screen.getByRole('heading', { level: 4, name: 'Refresh hosts' }) - ).toBeInTheDocument(); - 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(); - }); - - it('shows running status icon next to the title when task state is running', () => { - render( - - ); + it('renders task overview metadata via TaskInfo', () => { + render(); - expect( - screen.getByRole('heading', { level: 4, name: 'Running job' }) - ).toBeInTheDocument(); - expect(screen.getByTitle('Running')).toBeInTheDocument(); + expect(screen.getByText(/name:/i)).toBeInTheDocument(); + expect(screen.getByText('Refresh hosts')).toBeInTheDocument(); + expect(screen.getByText(/result:/i)).toBeInTheDocument(); }); }); diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js index f4c68d563..203a7c0dd 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js @@ -192,14 +192,6 @@ describe('TaskDetails', () => { expect(taskReloadStop).toHaveBeenCalled(); }); - it('renders task overview with action name when loaded', () => { - renderTaskDetails({ ...taskDetailsWithExecutionTabDefaults }); - - expect( - screen.getByRole('heading', { level: 4, name: 'Refresh foo' }) - ).toBeInTheDocument(); - }); - it('shows execution details panel on the default tab when loaded', () => { renderTaskDetails({ ...taskDetailsWithExecutionTabDefaults }); diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js new file mode 100644 index 000000000..b8d8191ca --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js @@ -0,0 +1,160 @@ +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 { TaskButtons } from '../../Components/TaskDetails/Components/TaskButtons'; +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, + } = props; + 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 ( + + setUnlockModalOpen(false)} + /> + setForceUnlockModalOpen(false)} + /> + + + + + + + + + + ); +}; + +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, + }), +}; + +TaskDetailsHeader.defaultProps = { + taskReload: false, + id: '', + forceCancelTaskRequest: () => null, + unlockTaskRequest: () => null, + action: '', + state: '', + result: '', + taskReloadStart: () => null, + taskReloadStop: () => null, + executionPlan: {}, +}; + +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..47bed17b2 --- /dev/null +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js @@ -0,0 +1,84 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +import TaskDetailsHeader from '../TaskDetailsHeader'; + +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: {}, +}; + +describe('TaskDetailsHeader', () => { + 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(); + 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(); + }); + + 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(); + }); +}); diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js index 0e1d040ec..f1f323986 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js @@ -1,18 +1,35 @@ 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 TaskDetailsPage from '../index'; + +const { renderWithStore } = rtlHelpers; + +const renderWithStoreAndI18n = (component, initialState) => + renderWithStore( + {component}, + initialState + ); + +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 +40,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: { @@ -67,26 +82,10 @@ const createStoreForTaskPayload = overrides => ({ }, }); -const rootReducer = combineReducers({ - API: (state = {}, action) => state, - intervals: intervalsReducer, - breadcrumbBar: breadcrumbBarReducer, - foremanTasks: (state = {}, action) => state, -}); - const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { 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 +93,28 @@ const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { `/foreman_tasks/tasks/${matchDefault.params.id}` ); - return render( + return renderWithStoreAndI18n( - - - - - - - ); -}; - -const breadcrumbTitleHeadings = () => - screen.getAllByRole('heading', { level: 1 }).filter( - heading => heading.getAttribute('data-ouia-component-id') === 'breadcrumb_title' + + , + createStoreForTaskPayload(apiPayloadOverrides) ); - -/** - * 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,10 +131,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( + screen.queryByRole('heading', { name: /Permission denied/i }) + ).not.toBeInTheDocument(); expect(mockUseForemanPermissions).toHaveBeenCalled(); }); @@ -174,7 +160,9 @@ 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(); }); it('denies access when user only has edit_foreman_tasks without view', () => { @@ -191,24 +179,18 @@ 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(); }); }); + it('shows running status icon when action is unset', () => { + renderPage({}); - const expectToolbarHeadingText = substring => { - const headings = breadcrumbTitleHeadings(); - - expect(headings.length).toBeGreaterThan(0); - - headings.forEach(heading => { - expect(heading).toHaveTextContent(substring); - }); - }; - - it('shows generic title and breadcrumb from route id when action is unset', () => { - const page = renderPage({}); - expectToolbarHeadingText('Task Details'); + expect( + screen.getByRole('heading', { level: 1, name: 'Task Details' }) + ).toBeInTheDocument(); expect(screen.getByRole('navigation', { name: 'Breadcrumb' })).toHaveTextContent( 'Tasks' @@ -219,18 +201,19 @@ describe('TaskDetailsPage', () => { ) ).toBeInTheDocument(); - const titleRegion = taskDetailsTitleRegion(page.container); - - expect( - within(titleRegion).getAllByRole('img', { hidden: true }).length - ).toBeGreaterThan(0); - expect(titleRegion.querySelector('[class*="danger"]')).toBeNull(); + expect(screen.getAllByTitle('Running').length).toBeGreaterThan(0); + expect(screen.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 +226,17 @@ 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(screen.getAllByTitle('Error').length).toBeGreaterThan(0); 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 85% rename from webpack/ForemanTasks/Components/TaskDetails/index.js rename to webpack/ForemanTasks/Routes/ShowTaskDetails/index.js index 8c8f6fa26..efcce2cf0 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,12 @@ import { selectIsLoading, selectDependsOn, selectBlocks, -} from './TaskDetailsSelectors'; +} from '../../Components/TaskDetails/TaskDetailsSelectors'; +import { getTaskID } from '../../Components/TaskDetails/TasksDetailsHelper'; +import TaskDetailsPage from './TaskDetailsPage'; const mapStateToProps = state => ({ + id: getTaskID(), startAt: selectStartAt(state), startBefore: selectStartBefore(state), startedAt: selectStartedAt(state), @@ -80,4 +82,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, -}); From 35c742b717b868d2262ab8111d2af212295651b8 Mon Sep 17 00:00:00 2001 From: Andrei Lakatos Date: Fri, 26 Jun 2026 06:51:44 -0400 Subject: [PATCH 3/5] Refs #39349 - route task id via props and align TaskDetails RTL tests --- .../TaskDetails/Components/__tests__/Task.test.js | 6 ++++++ .../Components/TaskDetails/TaskDetails.js | 4 ++-- .../TaskDetails/__tests__/TaskDetails.fixtures.js | 1 + .../Routes/ShowTaskDetails/TaskDetailsHeader.js | 2 +- .../__tests__/TaskDetailsPage.test.js | 15 +++++++++++---- .../ForemanTasks/Routes/ShowTaskDetails/index.js | 5 ++--- 6 files changed, 23 insertions(+), 10 deletions(-) diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js index bd16decf4..d21ff7b67 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Task.test.js @@ -19,5 +19,11 @@ describe('Task', () => { 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/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/__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/Routes/ShowTaskDetails/TaskDetailsHeader.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js index b8d8191ca..2d67f6e18 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js @@ -114,13 +114,13 @@ const TaskDetailsHeader = props => { diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js index f1f323986..1b94be61b 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js @@ -82,6 +82,11 @@ const createStoreForTaskPayload = overrides => ({ }, }); +const getTitleRow = () => + document.querySelector( + '[data-ouia-component-id="foreman-tasks-task-details-title-row"]' + ); + const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { const history = createMemoryHistory({ initialEntries: [`/foreman_tasks/tasks/${matchDefault.params.id}`], @@ -201,8 +206,10 @@ describe('TaskDetailsPage', () => { ) ).toBeInTheDocument(); - expect(screen.getAllByTitle('Running').length).toBeGreaterThan(0); - expect(screen.queryByTitle('Error')).not.toBeInTheDocument(); + const titleRow = getTitleRow(); + + expect(within(titleRow).getByTitle('Running')).toBeInTheDocument(); + expect(within(titleRow).queryByTitle('Error')).not.toBeInTheDocument(); }); it('uses task action for title and breadcrumb when loaded', () => { @@ -212,7 +219,7 @@ describe('TaskDetailsPage', () => { screen.getByRole('heading', { level: 1, name: 'Refresh hosts' }) ).toBeInTheDocument(); expect( - screen.getByRole('button', { name: /cancel/i }) + within(getTitleRow()).getByRole('button', { name: /cancel/i }) ).toBeInTheDocument(); expect(screen.getByRole('link', { name: /^Tasks$/ })).toHaveAttribute( @@ -236,7 +243,7 @@ describe('TaskDetailsPage', () => { expect( screen.getByRole('heading', { level: 1, name: 'Some action' }) ).toBeInTheDocument(); - expect(screen.getAllByTitle('Error').length).toBeGreaterThan(0); + expect(within(getTitleRow()).getByTitle('Error')).toBeInTheDocument(); expect( within(screen.getByRole('navigation', { name: 'Breadcrumb' })).getByText( diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/index.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/index.js index efcce2cf0..59f75ebd7 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/index.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/index.js @@ -38,11 +38,10 @@ import { selectDependsOn, selectBlocks, } from '../../Components/TaskDetails/TaskDetailsSelectors'; -import { getTaskID } from '../../Components/TaskDetails/TasksDetailsHelper'; import TaskDetailsPage from './TaskDetailsPage'; -const mapStateToProps = state => ({ - id: getTaskID(), +const mapStateToProps = (state, ownProps) => ({ + id: ownProps.match.params.id, startAt: selectStartAt(state), startBefore: selectStartBefore(state), startedAt: selectStartedAt(state), From e42cc370d99cec0e2a2bd7777dd073b37a94ab41 Mon Sep 17 00:00:00 2001 From: Andrei Lakatos Date: Fri, 26 Jun 2026 09:25:32 -0400 Subject: [PATCH 4/5] Refs #39349 - hide task header actions without view permission or on API error --- .../TaskDetails/__tests__/TaskDetails.test.js | 21 ++--- .../ShowTaskDetails/TaskDetailsHeader.js | 56 ++++++++----- .../__tests__/TaskDetailsHeader.test.js | 72 ++++++++++++++-- .../__tests__/TaskDetailsPage.test.js | 83 ++++++++++++------- 4 files changed, 164 insertions(+), 68 deletions(-) diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js index 203a7c0dd..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(); @@ -195,9 +199,6 @@ describe('TaskDetails', () => { 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(); @@ -210,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 }) @@ -227,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 index 2d67f6e18..18031c623 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/TaskDetailsHeader.js @@ -8,7 +8,10 @@ import { 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, @@ -56,7 +59,10 @@ const TaskDetailsHeader = props => { 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; @@ -89,16 +95,20 @@ const TaskDetailsHeader = props => { return ( - setUnlockModalOpen(false)} - /> - setForceUnlockModalOpen(false)} - /> + {showActions && ( + <> + setUnlockModalOpen(false)} + /> + setForceUnlockModalOpen(false)} + /> + + )} { result={props.result} /> - - - + {showActions && ( + + + + )} ); @@ -142,6 +154,7 @@ TaskDetailsHeader.propTypes = { state: PropTypes.string, cancellable: PropTypes.bool, }), + apiStatus: PropTypes.oneOf(Object.keys(STATUS)), }; TaskDetailsHeader.defaultProps = { @@ -155,6 +168,7 @@ TaskDetailsHeader.defaultProps = { taskReloadStart: () => null, taskReloadStop: () => null, executionPlan: {}, + apiStatus: STATUS.RESOLVED, }; export default TaskDetailsHeader; diff --git a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js index 47bed17b2..dd95881fd 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsHeader.test.js @@ -1,8 +1,19 @@ 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', @@ -20,9 +31,38 @@ const baseHeaderProps = { 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(); }); @@ -33,13 +73,7 @@ describe('TaskDetailsHeader', () => { expect( screen.getByRole('heading', { level: 1, name: 'Refresh hosts' }) ).toBeInTheDocument(); - 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(); + expectHeaderActionsVisible(); }); it('shows running status icon next to the title when task state is running', () => { @@ -81,4 +115,28 @@ describe('TaskDetailsHeader', () => { 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 1b94be61b..0e873d2bd 100644 --- a/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js +++ b/webpack/ForemanTasks/Routes/ShowTaskDetails/__tests__/TaskDetailsPage.test.js @@ -8,17 +8,14 @@ import { IntlProvider } from 'react-intl'; import { rtlHelpers } from 'foremanReact/common/testHelpers'; import { STATUS } from 'foremanReact/constants'; -import { FOREMAN_TASK_DETAILS, VIEW_FOREMAN_TASKS } from '../../../Components/TaskDetails/TaskDetailsConstants'; +import { + FOREMAN_TASK_DETAILS, + VIEW_FOREMAN_TASKS, +} from '../../../Components/TaskDetails/TaskDetailsConstants'; import TaskDetailsPage from '../index'; const { renderWithStore } = rtlHelpers; -const renderWithStoreAndI18n = (component, initialState) => - renderWithStore( - {component}, - initialState - ); - jest.mock('../../../Components/TaskDetails/TaskDetailsActions', () => { const actual = jest.requireActual( '../../../Components/TaskDetails/TaskDetailsActions' @@ -72,22 +69,34 @@ 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 getTitleRow = () => - document.querySelector( - '[data-ouia-component-id="foreman-tasks-task-details-title-row"]' - ); +const expectHeaderActionsHidden = () => { + expect( + screen.queryByRole('button', { name: /cancel/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /^task actions$/i }) + ).not.toBeInTheDocument(); +}; + +const getHeaderTitleArea = title => + screen.getByRole('heading', { level: 1, name: title }).parentElement + .parentElement; -const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { +const renderPage = ( + apiPayloadOverrides = {}, + propsOverrides = {}, + apiStatus = STATUS.RESOLVED +) => { const history = createMemoryHistory({ initialEntries: [`/foreman_tasks/tasks/${matchDefault.params.id}`], }); @@ -98,16 +107,18 @@ const renderPage = (apiPayloadOverrides = {}, propsOverrides = {}) => { `/foreman_tasks/tasks/${matchDefault.params.id}` ); - return renderWithStoreAndI18n( - - - , - createStoreForTaskPayload(apiPayloadOverrides) + return renderWithStore( + + + + + , + createStoreForTaskPayload(apiPayloadOverrides, apiStatus) ); }; @@ -142,7 +153,6 @@ describe('TaskDetailsPage', () => { expect( screen.queryByRole('heading', { name: /Permission denied/i }) ).not.toBeInTheDocument(); - expect(mockUseForemanPermissions).toHaveBeenCalled(); }); it(`shows ResourceLoadFailedEmptyState and lists ${VIEW_FOREMAN_TASKS} when it is absent`, () => { @@ -168,6 +178,7 @@ describe('TaskDetailsPage', () => { expect( screen.queryByRole('tab', { name: /^task$/i }) ).not.toBeInTheDocument(); + expectHeaderActionsHidden(); }); it('denies access when user only has edit_foreman_tasks without view', () => { @@ -187,6 +198,16 @@ describe('TaskDetailsPage', () => { 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); + + expect( + screen.getByRole('heading', { name: /unable to load task/i }) + ).toBeInTheDocument(); + expectHeaderActionsHidden(); }); }); @@ -206,10 +227,10 @@ describe('TaskDetailsPage', () => { ) ).toBeInTheDocument(); - const titleRow = getTitleRow(); + const titleArea = getHeaderTitleArea('Task Details'); - expect(within(titleRow).getByTitle('Running')).toBeInTheDocument(); - expect(within(titleRow).queryByTitle('Error')).not.toBeInTheDocument(); + expect(within(titleArea).getByTitle('Running')).toBeInTheDocument(); + expect(within(titleArea).queryByTitle('Error')).not.toBeInTheDocument(); }); it('uses task action for title and breadcrumb when loaded', () => { @@ -219,7 +240,7 @@ describe('TaskDetailsPage', () => { screen.getByRole('heading', { level: 1, name: 'Refresh hosts' }) ).toBeInTheDocument(); expect( - within(getTitleRow()).getByRole('button', { name: /cancel/i }) + screen.getByRole('button', { name: /cancel/i }) ).toBeInTheDocument(); expect(screen.getByRole('link', { name: /^Tasks$/ })).toHaveAttribute( @@ -243,7 +264,9 @@ describe('TaskDetailsPage', () => { expect( screen.getByRole('heading', { level: 1, name: 'Some action' }) ).toBeInTheDocument(); - expect(within(getTitleRow()).getByTitle('Error')).toBeInTheDocument(); + expect( + within(getHeaderTitleArea('Some action')).getByTitle('Error') + ).toBeInTheDocument(); expect( within(screen.getByRole('navigation', { name: 'Breadcrumb' })).getByText( From ace21c1fe7fb1af62abb0dfce1f46077f793f1cb Mon Sep 17 00:00:00 2001 From: Andrei Lakatos Date: Tue, 30 Jun 2026 06:10:12 -0400 Subject: [PATCH 5/5] Refs #39349 - move task buttons css to a separate file --- .../TaskDetails/Components/TaskButtons.js | 7 ++- .../TaskDetails/Components/TaskButtons.scss | 45 +++++++++++++++++++ .../Components/TaskDetails/TaskDetails.scss | 39 ---------------- 3 files changed, 50 insertions(+), 41 deletions(-) create mode 100644 webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.scss diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js index d89f535db..19dd4bf9b 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskButtons.js @@ -5,6 +5,8 @@ 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, @@ -37,8 +39,8 @@ export const TaskButtons = ({ ouiaId: 'task-buttons-reload-button', content: ( <> - - + +   {taskReload ? __('Stop auto-reloading') : __('Start auto-reloading')} @@ -100,6 +102,7 @@ export const TaskButtons = ({ return ( span { + pointer-events: auto; + } +} 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;