diff --git a/app/services/foreman_tasks/troubleshooting_help_generator.rb b/app/services/foreman_tasks/troubleshooting_help_generator.rb index 0c39f8a22..5db3f3bde 100644 --- a/app/services/foreman_tasks/troubleshooting_help_generator.rb +++ b/app/services/foreman_tasks/troubleshooting_help_generator.rb @@ -67,7 +67,7 @@ def generic_info i.add_line _('A paused task represents a process that has not finished properly. '\ 'Any task in paused state can lead to potential inconsistency '\ 'and needs to be resolved.') - i.add_line _("The recommended approach is to investigate the error messages below and in 'errors' tab, "\ + i.add_line _("The recommended approach is to investigate the error messages below and in 'Execution details' tab, "\ 'address the primary cause of the issue and resume the task.') if (link = troubleshooting_link) i.add_link(link) diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js index 3c4a6145b..5c4dfead8 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/TaskInfo.js @@ -54,7 +54,6 @@ const TaskInfo = props => { state, help, output, - errors, progress, username, usernamePath, @@ -173,12 +172,6 @@ const TaskInfo = props => { 
{output}
)} - {errors && errors.length > 0 && ( - - {__('Errors:')} - 
{errors}
- - )} ); }; @@ -192,7 +185,6 @@ TaskInfo.propTypes = { startedAt: PropTypes.string, state: PropTypes.string, help: PropTypes.string, - errors: PropTypes.array, progress: PropTypes.number, username: PropTypes.string, usernamePath: PropTypes.string, @@ -208,7 +200,6 @@ TaskInfo.defaultProps = { startedAt: '', state: '', help: '', - errors: [], progress: 0, username: '', usernamePath: '', diff --git a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskInfo.test.js b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskInfo.test.js index a83c92757..821d71621 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskInfo.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/TaskInfo.test.js @@ -33,7 +33,7 @@ describe('TaskInfo', () => { result="error" progress={0.5} help={ - "A paused task represents a process that has not finished properly. Any task in paused state can lead to potential inconsistency and needs to be resolved.\nThe recommended approach is to investigate the error messages below and in 'errors' tab, address the primary cause of the issue and resume the task." + "A paused task represents a process that has not finished properly. Any task in paused state can lead to potential inconsistency and needs to be resolved.\nThe recommended approach is to investigate the error messages below and in 'Execution details' tab, address the primary cause of the issue and resume the task." } output={{}} usernamePath="" diff --git a/webpack/ForemanTasks/Components/TaskDetails/ExecutionDetails.js b/webpack/ForemanTasks/Components/TaskDetails/ExecutionDetails.js new file mode 100644 index 000000000..853304a02 --- /dev/null +++ b/webpack/ForemanTasks/Components/TaskDetails/ExecutionDetails.js @@ -0,0 +1,65 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import RunningSteps from './Components/RunningSteps'; +import Errors from './Components/Errors'; + +const ExecutionDetails = ({ + state, + runningSteps, + cancelStep, + id, + taskReload, + taskReloadStart, + executionPlan, + failedSteps, + result, +}) => { + const showingRunningSteps = + state === 'running' || + state === 'pending' || + state === 'paused' || + runningSteps.length > 0; + + return ( +
+ {showingRunningSteps ? ( + + ) : ( + + )} +
+ ); +}; + +ExecutionDetails.propTypes = { + state: PropTypes.string, + result: PropTypes.string, + runningSteps: PropTypes.array, + cancelStep: PropTypes.func.isRequired, + id: PropTypes.string.isRequired, + taskReload: PropTypes.bool.isRequired, + taskReloadStart: PropTypes.func.isRequired, + executionPlan: PropTypes.shape({}), + failedSteps: PropTypes.array, +}; + +ExecutionDetails.defaultProps = { + state: '', + result: undefined, + runningSteps: [], + executionPlan: {}, + failedSteps: [], +}; + +export default ExecutionDetails; diff --git a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js index cab955487..a71603ed6 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js +++ b/webpack/ForemanTasks/Components/TaskDetails/TaskDetails.js @@ -6,10 +6,9 @@ import { STATUS } from 'foremanReact/constants'; import { usePermissions } from 'foremanReact/common/hooks/Permissions/permissionHooks'; import { ResourceLoadFailedEmptyState } from 'foremanReact/components/common/EmptyState'; import Task from './Components/Task'; -import RunningSteps from './Components/RunningSteps'; -import Errors from './Components/Errors'; import Locks from './Components/Locks'; 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'; @@ -17,6 +16,13 @@ import { TaskSkeleton } from './Components/TaskSkeleton'; import './TaskDetails.scss'; +export const TASK_DETAILS_TAB_KEYS = Object.freeze({ + EXECUTION: 'execution', + DEPENDENCIES: 'dependencies', + LOCKS: 'locks', + RAW: 'raw', +}); + const TaskDetails = ({ executionPlan, failedSteps, @@ -34,8 +40,8 @@ const TaskDetails = ({ ...props }) => { const id = getTaskID(); - const { taskReload, isLoading, result } = props; - const [activeTabKey, setActiveTabKey] = useState(1); + const { taskReload, isLoading } = props; + const [activeTab, setActiveTab] = useState(TASK_DETAILS_TAB_KEYS.EXECUTION); const hasViewPermission = usePermissions([VIEW_FOREMAN_TASKS]); useEffect(() => { @@ -76,7 +82,7 @@ const TaskDetails = ({ const cancellable = executionPlan ? executionPlan.cancellable : false; const lockRecords = locks.concat(links); - const taskComponentProps = { + const taskProps = { ...props, cancellable, resumable, @@ -87,49 +93,49 @@ const TaskDetails = ({ return (
+
+ {isLoading ? : } +
setActiveTabKey(tabKey)} + activeKey={activeTab} mountOnEnter + onSelect={(_e, tabKey) => setActiveTab(tabKey)} > {__('Task')}} - aria-label={__('Task')} - ouiaId="task-details-tab-task" - > - {isLoading ? : } - - {__('Running Steps')}} isDisabled={isLoading} - aria-label={__('Running Steps')} - ouiaId="task-details-tab-running-steps" + eventKey={TASK_DETAILS_TAB_KEYS.EXECUTION} + title={{__('Execution details')}} + aria-label={__('Execution details')} + ouiaId="task-details-tab-execution-details" > - + {!isLoading && ( + + )} {__('Errors')}} + eventKey={TASK_DETAILS_TAB_KEYS.DEPENDENCIES} isDisabled={isLoading} - aria-label={__('Errors')} - ouiaId="task-details-tab-errors" + title={{__('Dependencies')}} + aria-label={__('Dependencies')} + ouiaId="task-details-tab-dependencies" > - + {__('Locks')}} isDisabled={isLoading} aria-label={__('Locks')} @@ -138,16 +144,7 @@ const TaskDetails = ({ {__('Dependencies')}} - isDisabled={isLoading} - aria-label={__('Dependencies')} - ouiaId="task-details-tab-dependencies" - > - - - {__('Raw')}} isDisabled={isLoading} aria-label={__('Raw')} @@ -170,6 +167,7 @@ const TaskDetails = ({ TaskDetails.propTypes = { label: PropTypes.string, + result: PropTypes.string, runningSteps: PropTypes.array, cancelStep: PropTypes.func.isRequired, taskReload: PropTypes.bool.isRequired, @@ -181,10 +179,10 @@ TaskDetails.propTypes = { links: PropTypes.array, dependsOn: PropTypes.array, blocks: PropTypes.array, + executionPlan: PropTypes.shape({}), + failedSteps: PropTypes.array, ...Task.propTypes, - ...Errors.propTypes, ...Locks.propTypes, - ...Dependencies.propTypes, ...Raw.propTypes, }; TaskDetails.defaultProps = { @@ -194,11 +192,10 @@ TaskDetails.defaultProps = { links: [], dependsOn: [], blocks: [], + failedSteps: [], + executionPlan: {}, ...Task.defaultProps, - ...RunningSteps.defaultProps, - ...Errors.defaultProps, ...Locks.defaultProps, - ...Dependencies.defaultProps, ...Raw.defaultProps, }; diff --git a/webpack/ForemanTasks/Components/TaskDetails/TaskDetailsSelectors.js b/webpack/ForemanTasks/Components/TaskDetails/TaskDetailsSelectors.js index 4a3265db6..7de5c080e 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/TaskDetailsSelectors.js +++ b/webpack/ForemanTasks/Components/TaskDetails/TaskDetailsSelectors.js @@ -39,11 +39,6 @@ export const selectResumable = state => export const selectCancellable = state => selectTaskDetailsResponse(state).cancellable || false; -export const selectErrors = state => { - const { humanized } = selectTaskDetailsResponse(state); - return humanized ? humanized.errors : []; -}; - export const selectProgress = state => selectTaskDetailsResponse(state).progress ? Math.trunc(selectTaskDetailsResponse(state).progress * 100) diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/ExecutionDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/ExecutionDetails.test.js new file mode 100644 index 000000000..b312fa7d3 --- /dev/null +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/ExecutionDetails.test.js @@ -0,0 +1,194 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +import ExecutionDetails from '../ExecutionDetails'; +import { + fixtureFailedExecutionDetail, + fixtureRunningExecutionDetail, +} from './TaskDetails.fixtures'; + +const rtlBaseProps = { + cancelStep: jest.fn(), + taskReloadStart: jest.fn(), + id: 'a15dd820-32f1-4ced-9ab7-c0fab8234c47', + taskReload: false, +}; + +describe('ExecutionDetails', () => { + it('renders execution details panel with OUIA id', () => { + render( + + ); + + expect( + document.querySelector('[data-ouia-component-id="execution-details-panel"]') + ).toBeInTheDocument(); + expect(document.getElementById('execution-details-panel')).toBeInTheDocument(); + }); + + it('shows Errors pane when stopped with no running steps', () => { + render( + + ); + + expect(screen.getByRole('heading', { name: /^no errors found$/i })).toBeInTheDocument(); + expect( + screen.getByText(/the task finished with no errors or warnings/i) + ).toBeInTheDocument(); + }); + + it('shows RunningSteps when stopped but runningSteps list is non-empty', () => { + render( + + ); + + expect( + screen.getByRole('heading', { name: 'Warning alert: Running step 1' }) + ).toBeInTheDocument(); + }); + + it('forwards executionPlan and result to RunningSteps when state is pending without steps', () => { + render( + + ); + + expect( + screen.getByRole('heading', { level: 2, name: /planned task/i }) + ).toBeInTheDocument(); + }); + + it('shows temporarily suspended messaging when pending, plan running, result pending', () => { + render( + + ); + + expect( + screen.getByRole('heading', { + level: 4, + name: /temporarily suspended step/i, + }) + ).toBeInTheDocument(); + }); + + it('routes failed steps through Errors pane when stopped', () => { + render( + + ); + + expect( + screen.getByRole('tab', { + name: /action actions::katello::eventqueue::monitor is already active/i, + }) + ).toBeInTheDocument(); + expect( + screen.getByLabelText(/failed task errors/i) + ).toBeInTheDocument(); + expect(screen.getByText('Exception:')).toBeInTheDocument(); + }); + + it('renders RunningSteps with cancel when task is running', () => { + const cancelStep = jest.fn(); + + render( + + ); + + expect(screen.getByRole('button', { name: /cancel/i })).toBeInTheDocument(); + }); + + it('shows RunningSteps when state is paused without running steps', () => { + render( + + ); + + expect(screen.getByText(/no running steps/i)).toBeInTheDocument(); + expect( + screen.queryByRole('heading', { name: /^no errors found$/i }) + ).not.toBeInTheDocument(); + }); + + it('shows RunningSteps when state is paused with running steps', () => { + render( + + ); + + expect( + screen.getByRole('heading', { name: 'Warning alert: Running step 1' }) + ).toBeInTheDocument(); + expect(screen.getByText('paused')).toBeInTheDocument(); + }); +}); diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js index 209bfcccc..f5d87ffc8 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.fixtures.js @@ -6,3 +6,101 @@ export const minProps = { taskProgressToggle: jest.fn(), apiStatus: 'RESOLVED', }; + +/** Props for snapshots that exercise the Execution details tab alongside the overview. */ +export const taskDetailsWithExecutionTabDefaults = { + ...minProps, + locks: [ + { + exclusive: false, + resource_type: 'User', + resource_id: 4, + link: null, + }, + ], + links: [], + executionPlan: { state: 'stopped', cancellable: false }, + failedSteps: [], + runningSteps: [], + dependsOn: [], + blocks: [], + action: 'Refresh foo', + state: 'stopped', + result: 'success', + progress: 100, + startAt: '', + startedAt: '', + endedAt: '', + startBefore: '', +}; + +const SAMPLE_FAILED_STEP = { + error: { + exception_class: 'RuntimeError', + message: + 'Action Actions::Katello::EventQueue::Monitor is already active', + backtrace: [ + "/home/example/gems/dynflow-1.2.3/lib/dynflow/action/singleton.rb:15:in `rescue in singleton_lock!'", + ], + }, + action_class: 'Actions::Katello::EventQueue::Monitor', + state: 'error', + input: + '{"locale"=>"en",\n "current_request_id"=>nil,\n "current_user_id"=>4}\n', + output: '{}\n', +}; + +const SAMPLE_RUNNING_STEP = { + cancellable: false, + id: 1, + action_class: 'Actions::DynflowExample', + state: 'running', + input: '{}', + output: '{}', +}; + +const SAMPLE_DEPENDS_ON = [ + { + id: '123', + action: 'Actions::FooBar', + humanized: 'Foo Bar Action', + state: 'stopped', + result: 'success', + }, +]; + +export const fixtureFailedExecutionDetail = { + ...taskDetailsWithExecutionTabDefaults, + result: 'error', + state: 'stopped', + failedSteps: [SAMPLE_FAILED_STEP], +}; + +export const fixtureRunningExecutionDetail = { + ...taskDetailsWithExecutionTabDefaults, + state: 'running', + runningSteps: [SAMPLE_RUNNING_STEP], +}; + +export const fixtureWithOverviewMessages = { + ...taskDetailsWithExecutionTabDefaults, + help: 'See logs for more.', + output: { + messages: ['partial output'], + result: 'error', + }, +}; + +export const fixtureWithDependencies = { + ...taskDetailsWithExecutionTabDefaults, + dependsOn: SAMPLE_DEPENDS_ON, + blocks: [ + { + id: '789', + action: 'Actions::Test', + humanized: 'Test Action', + state: 'paused', + result: 'warning', + }, + ], +}; diff --git a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js index 412f6787a..9638ffa56 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js +++ b/webpack/ForemanTasks/Components/TaskDetails/__tests__/TaskDetails.test.js @@ -1,5 +1,5 @@ import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; import { configureStore } from '@reduxjs/toolkit'; import { Provider } from 'react-redux'; @@ -7,7 +7,13 @@ import { createMemoryHistory } from 'history'; import { Router } from 'react-router-dom'; import TaskDetails from '../TaskDetails'; -import { minProps } from './TaskDetails.fixtures'; +import { + fixtureFailedExecutionDetail, + fixtureWithDependencies, + fixtureWithOverviewMessages, + minProps, + taskDetailsWithExecutionTabDefaults, +} from './TaskDetails.fixtures'; import { VIEW_FOREMAN_TASKS } from '../TaskDetailsConstants'; const mockUseForemanPermissions = jest.fn( @@ -111,25 +117,141 @@ describe('TaskDetails', () => { expect(screen.queryByRole('tab', { name: /^task$/i })).not.toBeInTheDocument(); }); - it('shows skeleton while loading on the Task tab', () => { + it('shows skeleton in the overview while loading', () => { const { container } = renderTaskDetails({ isLoading: true }); expect( container.querySelector('.react-loading-skeleton') ).toBeInTheDocument(); }); - it('renders six tabs with expected labels', () => { + it('renders four tabs with expected labels', () => { renderTaskDetails(); expect(document.getElementById('task-details-tabs')).toBeInTheDocument(); - expect(screen.getByRole('tab', { name: /^task$/i })).toBeInTheDocument(); expect( - screen.getByRole('tab', { name: /running steps/i }) + screen.getByRole('tab', { name: /execution details/i }) ).toBeInTheDocument(); - expect(screen.getByRole('tab', { name: /errors/i })).toBeInTheDocument(); - expect(screen.getByRole('tab', { name: /locks/i })).toBeInTheDocument(); expect( screen.getByRole('tab', { name: /dependencies/i }) ).toBeInTheDocument(); + expect(screen.getByRole('tab', { name: /locks/i })).toBeInTheDocument(); expect(screen.getByRole('tab', { name: /raw/i })).toBeInTheDocument(); }); + + it('exposes stable ouiaIds on tabs for automation', () => { + renderTaskDetails(); + + expect( + document.querySelector( + '[data-ouia-component-id="task-details-tab-execution-details"]' + ) + ).toBeInTheDocument(); + expect( + document.querySelector( + '[data-ouia-component-id="task-details-tab-dependencies"]' + ) + ).toBeInTheDocument(); + expect( + document.querySelector( + '[data-ouia-component-id="task-details-tab-locks"]' + ) + ).toBeInTheDocument(); + expect( + document.querySelector('[data-ouia-component-id="task-details-tab-raw"]') + ).toBeInTheDocument(); + }); + + it('shows failed step errors on the default execution details tab', () => { + renderTaskDetails({ + ...fixtureFailedExecutionDetail, + executionPlan: fixtureFailedExecutionDetail.executionPlan, + failedSteps: fixtureFailedExecutionDetail.failedSteps, + }); + + expect( + screen.getByRole('tab', { + name: /action actions::katello::eventqueue::monitor is already active/i, + }) + ).toBeInTheDocument(); + expect(screen.getByLabelText(/failed task errors/i)).toBeInTheDocument(); + }); + + it('starts task reload on mount and stops on unmount', () => { + const taskReloadStart = jest.fn(); + const taskReloadStop = jest.fn(); + + const { unmount } = renderTaskDetails({ + ...taskDetailsWithExecutionTabDefaults, + taskReloadStart, + taskReloadStop, + }); + + expect(taskReloadStart).toHaveBeenCalledWith( + 'a15dd820-32f1-4ced-9ab7-c0fab8234c47' + ); + unmount(); + 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(); + }); + + it('disables tabs and hides execution panel while loading', () => { + renderTaskDetails({ + ...taskDetailsWithExecutionTabDefaults, + isLoading: true, + }); + + expect( + document.getElementById('execution-details-panel') + ).not.toBeInTheDocument(); + expect( + screen.getByRole('tab', { name: /execution details/i }) + ).toBeDisabled(); + expect(screen.getByRole('tab', { name: /dependencies/i })).toBeDisabled(); + expect(screen.getByRole('tab', { name: /locks/i })).toBeDisabled(); + expect(screen.getByRole('tab', { name: /raw/i })).toBeDisabled(); + }); + + it('renders troubleshooting help in the overview section', () => { + renderTaskDetails({ ...fixtureWithOverviewMessages }); + + expect(screen.getByText(/troubleshooting/i)).toBeInTheDocument(); + expect(screen.getByText('See logs')).toBeInTheDocument(); + }); + + it('renders raw output when the Raw tab is selected', () => { + renderTaskDetails({ ...fixtureWithOverviewMessages }); + + fireEvent.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', () => { + renderTaskDetails({ ...fixtureWithDependencies }); + + fireEvent.click(screen.getByRole('tab', { name: /dependencies/i })); + + expect( + screen.getByRole('link', { name: 'Foo Bar Action' }) + ).toBeInTheDocument(); + expect( + screen.getByRole('link', { name: 'Test Action' }) + ).toBeInTheDocument(); + }); }); diff --git a/webpack/ForemanTasks/Components/TaskDetails/index.js b/webpack/ForemanTasks/Components/TaskDetails/index.js index 58c0ed7c1..8c8f6fa26 100644 --- a/webpack/ForemanTasks/Components/TaskDetails/index.js +++ b/webpack/ForemanTasks/Components/TaskDetails/index.js @@ -15,7 +15,6 @@ import { selectResumable, selectCancellable, selectHelp, - selectErrors, selectProgress, selectUsername, selectLabel, @@ -50,7 +49,6 @@ const mapStateToProps = state => ({ output: selectOutput(state), resumable: selectResumable(state), cancellable: selectCancellable(state), - errors: selectErrors(state), progress: selectProgress(state), username: selectUsername(state), label: selectLabel(state),