From d38d4780423ddd038044374d0ce11742710bd931 Mon Sep 17 00:00:00 2001 From: kmalyjur Date: Tue, 11 Nov 2025 16:37:41 +0000 Subject: [PATCH] Fixes #38806 - Stabilize job detail table and fix unresponsive actions --- .../JobInvocationDetail.scss | 4 + .../JobInvocationHostTable.js | 185 +++++++++++------ .../JobInvocationDetail/TemplateInvocation.js | 34 ++-- .../OutputToggleGroup.js | 68 ++++--- .../TemplateInvocationPage.js | 17 +- .../__tests__/TemplateInvocation.test.js | 186 +++++++++++------- 6 files changed, 319 insertions(+), 175 deletions(-) diff --git a/webpack/JobInvocationDetail/JobInvocationDetail.scss b/webpack/JobInvocationDetail/JobInvocationDetail.scss index 38f0615b0..0d0c5fa9c 100644 --- a/webpack/JobInvocationDetail/JobInvocationDetail.scss +++ b/webpack/JobInvocationDetail/JobInvocationDetail.scss @@ -74,6 +74,10 @@ section.job-additional-info { margin-left: 10px; margin-right: 15px; } + + .pf-v5-c-table__tbody > tr.row-hidden { + display: none; + } } .template-invocation { diff --git a/webpack/JobInvocationDetail/JobInvocationHostTable.js b/webpack/JobInvocationDetail/JobInvocationHostTable.js index 33cde9700..26636c436 100644 --- a/webpack/JobInvocationDetail/JobInvocationHostTable.js +++ b/webpack/JobInvocationDetail/JobInvocationHostTable.js @@ -61,9 +61,47 @@ const JobInvocationHostTable = ({ const [allHostsIds, setAllHostsIds] = useState([]); // Expansive items - const [expandedHost, setExpandedHost] = useState([]); + const [expandedHost, setExpandedHost] = useState(new Set()); const prevStatusLabel = useRef(statusLabel); + const [hostInvocationStates, setHostInvocationStates] = useState({}); + + const getInvocationState = hostId => + hostInvocationStates[hostId] || { + showOutputType: { stderr: true, stdout: true, debug: true }, + showTemplatePreview: false, + showCommand: false, + }; + + const updateInvocationState = (hostId, stateKey, value) => { + setHostInvocationStates(prevStates => { + const currentHostState = getInvocationState(hostId); + + const newValue = + typeof value === 'function' ? value(currentHostState[stateKey]) : value; + + return { + ...prevStates, + [hostId]: { + ...currentHostState, + [stateKey]: newValue, + }, + }; + }); + }; + + const isHostExpanded = hostId => expandedHost.has(hostId); + const setHostExpanded = (hostId, isExpanding = true) => + setExpandedHost(prevExpandedSet => { + const newSet = new Set(prevExpandedSet); + if (isExpanding) { + newSet.add(hostId); + } else { + newSet.delete(hostId); + } + return newSet; + }); + // Page table params // Parse URL const { @@ -307,29 +345,18 @@ const JobInvocationHostTable = ({ ); - const isHostExpanded = host => expandedHost.includes(host.id); - - const setHostExpanded = (host, isExpanding = true) => - setExpandedHost(prevExpanded => { - const otherExpandedHosts = prevExpanded.filter(h => h !== host.id); - return isExpanding - ? [...otherExpandedHosts, host.id] - : otherExpandedHosts; - }); - const pageHostIds = results.map(h => h.id); const areAllPageRowsExpanded = pageHostIds.length > 0 && - pageHostIds.every(hostId => expandedHost.includes(hostId)); + pageHostIds.every(hostId => expandedHost.has(hostId)); const onExpandAll = () => { setExpandedHost(() => { if (areAllPageRowsExpanded) { - return []; + return new Set(); } - - return pageHostIds; + return new Set(pageHostIds); }); }; @@ -393,54 +420,88 @@ const JobInvocationHostTable = ({ isDeleteable={false} childrenOutsideTbody > - {results.map((result, rowIndex) => ( - - - - setHostExpanded(result, !isHostExpanded(result)), - expandId: 'host-expandable', - }} - /> - - {columnNamesKeys.map(k => ( - {columns[k].wrapper(result)} - ))} - - - - - - { + const currentInvocationState = getInvocationState(result.id); + return ( + + + + setHostExpanded(result.id, !isHostExpanded(result.id)), + expandId: 'host-expandable', + }} + /> + + {columnNamesKeys.map(k => ( + {columns[k].wrapper(result)} + ))} + + + + + - - {result.job_status === 'cancelled' || - result.job_status === 'N/A' ? ( -
- {__('A task for this host has not been started')} -
- ) : ( - - )} - - - - - ))} + + + {result.job_status === 'cancelled' || + result.job_status === 'N/A' ? ( +
+ {__('A task for this host has not been started')} +
+ ) : ( + + updateInvocationState( + result.id, + 'showOutputType', + value + ) + } + setShowTemplatePreview={value => + updateInvocationState( + result.id, + 'showTemplatePreview', + value + ) + } + setShowCommand={value => + updateInvocationState( + result.id, + 'showCommand', + value + ) + } + /> + )} + + + + + ); + })} diff --git a/webpack/JobInvocationDetail/TemplateInvocation.js b/webpack/JobInvocationDetail/TemplateInvocation.js index 175b41061..9045be218 100644 --- a/webpack/JobInvocationDetail/TemplateInvocation.js +++ b/webpack/JobInvocationDetail/TemplateInvocation.js @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useRef } from 'react'; +import React, { useEffect, useRef } from 'react'; import { isEmpty } from 'lodash'; import PropTypes from 'prop-types'; import { ClipboardCopyButton, Alert, Skeleton } from '@patternfly/react-core'; @@ -60,6 +60,12 @@ export const TemplateInvocation = ({ isExpanded, hostName, hostProxy, + showOutputType, + setShowOutputType, + showTemplatePreview, + setShowTemplatePreview, + showCommand, + setShowCommand, }) => { const intervalRef = useRef(null); const templateURL = showTemplateInvocationUrl(hostID, jobID); @@ -74,14 +80,6 @@ export const TemplateInvocation = ({ responseRef.current = response; }, [response]); - const [showOutputType, setShowOutputType] = useState({ - stderr: true, - stdout: true, - debug: true, - }); - const [showTemplatePreview, setShowTemplatePreview] = useState(false); - const [showCommand, setShowCommand] = useState(false); - useEffect(() => { const dispatchFetch = () => { dispatch( @@ -123,12 +121,8 @@ export const TemplateInvocation = ({ }; }, [isExpanded, dispatch, templateURL, hostID]); - if (!isExpanded) { - return null; - } - - if ((status === STATUS.PENDING && isEmpty(response)) || !response) { - return ; + if (!response || (status === STATUS.PENDING && isEmpty(response))) { + return ; } const errorMessage = @@ -239,6 +233,16 @@ TemplateInvocation.propTypes = { jobID: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, isInTableView: PropTypes.bool, isExpanded: PropTypes.bool, + showOutputType: PropTypes.shape({ + stderr: PropTypes.bool, + stdout: PropTypes.bool, + debug: PropTypes.bool, + }).isRequired, + setShowOutputType: PropTypes.func.isRequired, + showTemplatePreview: PropTypes.bool.isRequired, + setShowTemplatePreview: PropTypes.func.isRequired, + showCommand: PropTypes.bool.isRequired, + setShowCommand: PropTypes.func.isRequired, }; TemplateInvocation.defaultProps = { diff --git a/webpack/JobInvocationDetail/TemplateInvocationComponents/OutputToggleGroup.js b/webpack/JobInvocationDetail/TemplateInvocationComponents/OutputToggleGroup.js index 8d0e6e1e5..6f2591316 100644 --- a/webpack/JobInvocationDetail/TemplateInvocationComponents/OutputToggleGroup.js +++ b/webpack/JobInvocationDetail/TemplateInvocationComponents/OutputToggleGroup.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useCallback } from 'react'; import PropTypes from 'prop-types'; import { ToggleGroup, @@ -27,31 +27,49 @@ export const OutputToggleGroup = ({ taskCancellable, permissions, }) => { - const handleSTDERRClick = _isSelected => { - setShowOutputType(prevShowOutputType => ({ - ...prevShowOutputType, - stderr: _isSelected, - })); - }; + const handleSTDERRClick = useCallback( + _isSelected => { + setShowOutputType(prevShowOutputType => ({ + ...prevShowOutputType, + stderr: _isSelected, + })); + }, + [setShowOutputType] + ); - const handleSTDOUTClick = _isSelected => { - setShowOutputType(prevShowOutputType => ({ - ...prevShowOutputType, - stdout: _isSelected, - })); - }; - const handleDEBUGClick = _isSelected => { - setShowOutputType(prevShowOutputType => ({ - ...prevShowOutputType, - debug: _isSelected, - })); - }; - const handlePreviewTemplateClick = _isSelected => { - setShowTemplatePreview(_isSelected); - }; - const handleCommandClick = _isSelected => { - setShowCommand(_isSelected); - }; + const handleSTDOUTClick = useCallback( + _isSelected => { + setShowOutputType(prevShowOutputType => ({ + ...prevShowOutputType, + stdout: _isSelected, + })); + }, + [setShowOutputType] + ); + + const handleDEBUGClick = useCallback( + _isSelected => { + setShowOutputType(prevShowOutputType => ({ + ...prevShowOutputType, + debug: _isSelected, + })); + }, + [setShowOutputType] + ); + + const handlePreviewTemplateClick = useCallback( + _isSelected => { + setShowTemplatePreview(_isSelected); + }, + [setShowTemplatePreview] + ); + + const handleCommandClick = useCallback( + _isSelected => { + setShowCommand(_isSelected); + }, + [setShowCommand] + ); const toggleGroupItems = { stderr: { diff --git a/webpack/JobInvocationDetail/TemplateInvocationPage.js b/webpack/JobInvocationDetail/TemplateInvocationPage.js index 741a4a591..2c60b934d 100644 --- a/webpack/JobInvocationDetail/TemplateInvocationPage.js +++ b/webpack/JobInvocationDetail/TemplateInvocationPage.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; import PageLayout from 'foremanReact/routes/common/PageLayout/PageLayout'; @@ -26,6 +26,15 @@ const TemplateInvocationPage = ({ ], isPf4: true, }; + + const [showOutputType, setShowOutputType] = useState({ + stderr: true, + stdout: true, + debug: true, + }); + const [showTemplatePreview, setShowTemplatePreview] = useState(false); + const [showCommand, setShowCommand] = useState(false); + return ( ); diff --git a/webpack/JobInvocationDetail/__tests__/TemplateInvocation.test.js b/webpack/JobInvocationDetail/__tests__/TemplateInvocation.test.js index 1634c8ea3..3e4dee3f0 100644 --- a/webpack/JobInvocationDetail/__tests__/TemplateInvocation.test.js +++ b/webpack/JobInvocationDetail/__tests__/TemplateInvocation.test.js @@ -1,7 +1,7 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { Provider } from 'react-redux'; -import { render, screen, act, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent } from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; import * as api from 'foremanReact/redux/API'; import * as selectors from '../JobInvocationSelectors'; @@ -10,12 +10,7 @@ import { mockTemplateInvocationResponse } from './fixtures'; jest.spyOn(api, 'get'); jest.mock('../JobInvocationSelectors'); -selectors.selectTemplateInvocationStatus.mockImplementation(() => () => - 'RESOLVED' -); -selectors.selectTemplateInvocation.mockImplementation(() => () => - mockTemplateInvocationResponse -); + const mockStore = configureMockStore([]); const store = mockStore({ HOSTS_API: { @@ -24,79 +19,122 @@ const store = mockStore({ }, }, }); + +Object.assign(navigator, { + clipboard: { + writeText: jest.fn().mockResolvedValue(undefined), + }, +}); + +const mockProps = { + hostID: '1', + jobID: '1', + isInTableView: false, + isExpanded: true, + hostName: 'example-host', + hostProxy: { name: 'example-proxy', href: '#' }, + showOutputType: { stderr: true, stdout: true, debug: true }, + setShowOutputType: jest.fn(), + showTemplatePreview: false, + setShowTemplatePreview: jest.fn(), + showCommand: false, + setShowCommand: jest.fn(), +}; + describe('TemplateInvocation', () => { - test('render', async () => { + beforeEach(() => { + selectors.selectTemplateInvocationStatus.mockImplementation(() => () => + 'RESOLVED' + ); + selectors.selectTemplateInvocation.mockImplementation(() => () => + mockTemplateInvocationResponse + ); + }); + + test('render', () => { render( - + ); expect(screen.getByText('example-host')).toBeInTheDocument(); expect(screen.getByText('example-proxy')).toBeInTheDocument(); - expect(screen.getByText(/using Smart Proxy/)).toBeInTheDocument(); expect(screen.getByText(/Target:/)).toBeInTheDocument(); - expect(screen.getByText('This is red text')).toBeInTheDocument(); expect(screen.getByText('This is default text')).toBeInTheDocument(); + expect(screen.getByLabelText('Copy to clipboard')).toBeInTheDocument(); }); - test('filtering toggles', () => { - render( + + test('shows "No output" message when all toggles are off', () => { + const { rerender } = render( - + ); - act(() => { - fireEvent.click(screen.getByText('STDOUT')); - fireEvent.click(screen.getByText('DEBUG')); - fireEvent.click(screen.getByText('STDERR')); - }); expect( - screen.queryAllByText('No output for the selected filters') - ).toHaveLength(1); - expect(screen.queryAllByText('Exit status: 1')).toHaveLength(0); - expect( - screen.queryAllByText('StandardError: Job execution failed') - ).toHaveLength(0); + screen.queryByText('No output for the selected filters') + ).not.toBeInTheDocument(); + + const newProps = { + ...mockProps, + showOutputType: { stderr: false, stdout: false, debug: false }, + }; + + rerender( + + + + ); - act(() => { - fireEvent.click(screen.getByText('STDOUT')); - }); expect( - screen.queryAllByText('No output for the selected filters') - ).toHaveLength(0); - expect(screen.queryAllByText('Exit status: 1')).toHaveLength(1); + screen.getByText('No output for the selected filters') + ).toBeInTheDocument(); + }); + + test('correctly filters specific output types', () => { + const { rerender } = render( + + + + ); + + expect(screen.getByText('Exit status: 1')).toBeInTheDocument(); // stdout expect( - screen.queryAllByText('StandardError: Job execution failed') - ).toHaveLength(0); + screen.getByText('StandardError: Job execution failed') + ).toBeInTheDocument(); // debug - act(() => { - fireEvent.click(screen.getByText('DEBUG')); - }); + // Turn off stdout + rerender( + + + + ); + expect(screen.queryByText('Exit status: 1')).not.toBeInTheDocument(); expect( - screen.queryAllByText('No output for the selected filters') - ).toHaveLength(0); - expect(screen.queryAllByText('Exit status: 1')).toHaveLength(1); + screen.getByText('StandardError: Job execution failed') + ).toBeInTheDocument(); + + // Turn off debug + rerender( + + + + ); + expect(screen.queryByText('Exit status: 1')).not.toBeInTheDocument(); expect( - screen.queryAllByText('StandardError: Job execution failed') - ).toHaveLength(1); + screen.queryByText('StandardError: Job execution failed') + ).not.toBeInTheDocument(); }); + test('displays an error alert when there is an error', async () => { selectors.selectTemplateInvocationStatus.mockImplementation(() => () => 'ERROR' @@ -106,14 +144,7 @@ describe('TemplateInvocation', () => { })); render( - + ); @@ -129,19 +160,30 @@ describe('TemplateInvocation', () => { selectors.selectTemplateInvocationStatus.mockImplementation(() => () => 'PENDING' ); - selectors.selectTemplateInvocation.mockImplementation(() => () => ({})); + selectors.selectTemplateInvocation.mockImplementation(() => () => null); render( - + ); - expect(document.querySelectorAll('.pf-v5-c-skeleton')).toHaveLength(1); + expect( + screen.getByTestId('template-invocation-skeleton') + ).toBeInTheDocument(); + }); + + test('copies text to clipboard when clicked', async () => { + render( + + + + ); + + const copyButton = screen.getByLabelText('Copy to clipboard'); + fireEvent.click(copyButton); + expect(navigator.clipboard.writeText).toHaveBeenCalledTimes(1); + expect( + await screen.findByText('Successfully copied to clipboard!') + ).toBeInTheDocument(); }); });