From 95771c53cba3410645af0b3f3dd9bd9504f2993e Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Thu, 7 Sep 2023 17:50:29 +0200 Subject: [PATCH 01/10] Add tooltip above `Start executions` button --- .../HostDetails/HostChecksSelection.jsx | 23 ++++++---- .../HostDetails/HostChecksSelection.test.jsx | 42 +++++++++++++++++++ .../js/components/HostDetails/HostDetails.jsx | 27 ++++++++---- .../HostDetails/HostDetails.test.jsx | 2 + .../HostDetails/HostSettingsPage.jsx | 8 +++- 5 files changed, 85 insertions(+), 17 deletions(-) diff --git a/assets/js/components/HostDetails/HostChecksSelection.jsx b/assets/js/components/HostDetails/HostChecksSelection.jsx index 3964d612aa..fd726e8f13 100644 --- a/assets/js/components/HostDetails/HostChecksSelection.jsx +++ b/assets/js/components/HostDetails/HostChecksSelection.jsx @@ -5,6 +5,7 @@ import PageHeader from '@components/PageHeader'; import BackButton from '@components/BackButton'; import Button from '@components/Button'; import ChecksSelection from '@components/ChecksSelection'; +import Tooltip from '@components/Tooltip'; import HostInfoBox from './HostInfoBox'; @@ -23,6 +24,7 @@ function HostChecksSelection({ onSelectedChecksChange, hostChecksExecutionEnabled, onStartExecution = () => {}, + checksExecutionTooltipVisible, }) { return (
@@ -44,15 +46,20 @@ function HostChecksSelection({ > Save Checks Selection - + +
diff --git a/assets/js/components/HostDetails/HostChecksSelection.test.jsx b/assets/js/components/HostDetails/HostChecksSelection.test.jsx index f7703b893d..7328efabb5 100644 --- a/assets/js/components/HostDetails/HostChecksSelection.test.jsx +++ b/assets/js/components/HostDetails/HostChecksSelection.test.jsx @@ -42,6 +42,7 @@ describe('HostChecksSelection component', () => { catalogLoading={false} onUpdateCatalog={onUpdateCatalog} hostSelectedChecks={selectedChecks} + checksExecutionTooltipVisible /> ); @@ -58,6 +59,47 @@ describe('HostChecksSelection component', () => { expect( screen.getByRole('button', { name: 'Save Checks Selection' }) ).toBeVisible(); + expect(screen.getByRole('tooltip')).toBeVisible(); + expect(onUpdateCatalog).toHaveBeenCalled(); + }); + + it('should not render tooltip when render host check selection', async () => { + const group0 = faker.animal.cat(); + const group1 = faker.animal.dog(); + const group2 = faker.lorem.word(); + const catalog = [ + ...catalogCheckFactory.buildList(2, { group: group0 }), + ...catalogCheckFactory.buildList(2, { group: group1 }), + ...catalogCheckFactory.buildList(2, { group: group2 }), + ]; + + const onUpdateCatalog = jest.fn(); + + const { + id: hostID, + hostname: hostName, + provider, + agent_version: agentVersion, + selected_checks: selectedChecks, + } = hostFactory.build({ provider: 'azure' }); + + renderWithRouter( + + ); + + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); expect(onUpdateCatalog).toHaveBeenCalled(); }); }); diff --git a/assets/js/components/HostDetails/HostDetails.jsx b/assets/js/components/HostDetails/HostDetails.jsx index 3f2a02dc4f..8f82bfafce 100644 --- a/assets/js/components/HostDetails/HostDetails.jsx +++ b/assets/js/components/HostDetails/HostDetails.jsx @@ -15,6 +15,7 @@ import WarningBanner from '@components/Banners/WarningBanner'; import CleanUpButton from '@components/CleanUpButton'; import DeregistrationModal from '@components/DeregistrationModal'; import { canStartExecution } from '@components/ChecksSelection'; +import Tooltip from '@components/Tooltip'; import SuseLogo from '@static/suse_logo.svg'; import ChecksComingSoon from '@static/checks-coming-soon.svg'; @@ -51,6 +52,8 @@ function HostDetails({ navigate, }) { const [cleanUpModalOpen, setCleanUpModalOpen] = useState(false); + const [checksExecutionTooltipVisible, setChecksExecutionTooltipVisible] = + useState(true); const versionWarningMessage = agentVersionWarning(agentVersion); @@ -129,15 +132,23 @@ function HostDetails({ Show Results - + +
diff --git a/assets/js/components/HostDetails/HostDetails.test.jsx b/assets/js/components/HostDetails/HostDetails.test.jsx index 59ad8b8a8b..34635ffc71 100644 --- a/assets/js/components/HostDetails/HostDetails.test.jsx +++ b/assets/js/components/HostDetails/HostDetails.test.jsx @@ -31,6 +31,7 @@ describe('HostDetails component', () => { ); + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); const startExecutionButton = screen.getByText('Start Execution'); expect(startExecutionButton).toBeDisabled(); }); @@ -42,6 +43,7 @@ describe('HostDetails component', () => { ); + expect(screen.getByRole('tooltip')).toBeVisible(); const startExecutionButton = screen.getByText('Start Execution'); expect(startExecutionButton).toBeEnabled(); }); diff --git a/assets/js/components/HostDetails/HostSettingsPage.jsx b/assets/js/components/HostDetails/HostSettingsPage.jsx index 9826862c3a..29699f2f24 100644 --- a/assets/js/components/HostDetails/HostSettingsPage.jsx +++ b/assets/js/components/HostDetails/HostSettingsPage.jsx @@ -29,6 +29,8 @@ function HostSettingsPage() { setSelection(hostSelectedChecks); } }, [hostSelectedChecks]); + const [checksExecutionTooltipVisible, setChecksExecutionTooltipVisible] = + useState(hostSelectedChecks?.length > 0); const { data: catalog, @@ -55,7 +57,7 @@ function HostSettingsPage() { }) ); - const saveSelection = (newSelection, targetID, targetName) => + const saveSelection = (newSelection, targetID, targetName) => { dispatch( hostChecksSelected({ hostID: targetID, @@ -63,9 +65,12 @@ function HostSettingsPage() { checks: newSelection, }) ); + setChecksExecutionTooltipVisible(newSelection?.length > 0); + }; const requestHostChecksExecution = () => { dispatch(hostExecutionRequested(host, hostSelectedChecks, navigate)); + setChecksExecutionTooltipVisible(false); }; return ( @@ -84,6 +89,7 @@ function HostSettingsPage() { hostChecksExecutionEnabled={hostChecksExecutionEnabled} onSelectedChecksChange={setSelection} onStartExecution={requestHostChecksExecution} + checksExecutionTooltipVisible={checksExecutionTooltipVisible} /> ); } From 67eaecf9066a57468986f82f58831ab12ee9d92c Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Mon, 11 Sep 2023 11:55:49 +0200 Subject: [PATCH 02/10] Remove stateful code as clicking `Start Execution` redirects to new page --- assets/js/components/HostDetails/HostDetails.jsx | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/assets/js/components/HostDetails/HostDetails.jsx b/assets/js/components/HostDetails/HostDetails.jsx index 8f82bfafce..0b370e91dd 100644 --- a/assets/js/components/HostDetails/HostDetails.jsx +++ b/assets/js/components/HostDetails/HostDetails.jsx @@ -52,8 +52,6 @@ function HostDetails({ navigate, }) { const [cleanUpModalOpen, setCleanUpModalOpen] = useState(false); - const [checksExecutionTooltipVisible, setChecksExecutionTooltipVisible] = - useState(true); const versionWarningMessage = agentVersionWarning(agentVersion); @@ -134,15 +132,12 @@ function HostDetails({ diff --git a/assets/js/components/HostDetails/HostDetails.jsx b/assets/js/components/HostDetails/HostDetails.jsx index 0b370e91dd..fcefd6753b 100644 --- a/assets/js/components/HostDetails/HostDetails.jsx +++ b/assets/js/components/HostDetails/HostDetails.jsx @@ -131,6 +131,7 @@ function HostDetails({ From 2c578c5f773a94b62c689809f4951e94d60c4340 Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Tue, 12 Sep 2023 15:01:25 +0200 Subject: [PATCH 04/10] Use consistent style for `Start Execution` button in `Cluster Details` page --- .../ClusterDetails/HanaClusterDetails.jsx | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/assets/js/components/ClusterDetails/HanaClusterDetails.jsx b/assets/js/components/ClusterDetails/HanaClusterDetails.jsx index 194c44664b..b14a118503 100644 --- a/assets/js/components/ClusterDetails/HanaClusterDetails.jsx +++ b/assets/js/components/ClusterDetails/HanaClusterDetails.jsx @@ -1,7 +1,6 @@ import React from 'react'; import { groupBy } from 'lodash'; -import classNames from 'classnames'; import PageHeader from '@components/PageHeader'; import BackButton from '@components/BackButton'; import Button from '@components/Button'; @@ -9,7 +8,6 @@ import Button from '@components/Button'; import ListView from '@components/ListView'; import Table from '@components/Table'; import Tooltip from '@components/Tooltip'; -import TriggerChecksExecutionRequest from '@components/TriggerChecksExecutionRequest'; import ClusterNodeLink from '@components/ClusterDetails/ClusterNodeLink'; import ChecksResultOverview from '@components/ClusterDetails/ChecksResultOverview'; import ProviderLabel from '@components/ProviderLabel'; @@ -133,21 +131,17 @@ function HanaClusterDetails({ content="Select some Checks first!" place="bottom" > - { + onStartExecution(clusterID, hosts, selectedChecks, navigate); + }} disabled={startExecutionDisabled} - hosts={hosts.map(({ id }) => id)} - checks={selectedChecks} - onStartExecution={onStartExecution} > - {' '} - Start Execution - + {' '} + Start Execution +
From 4ae121eb674ba2f24340cc3cb21ab5e57c707f3d Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Tue, 12 Sep 2023 16:17:46 +0200 Subject: [PATCH 05/10] Use correct tooltip in `Host Details` page --- assets/js/components/HostDetails/HostDetails.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/js/components/HostDetails/HostDetails.jsx b/assets/js/components/HostDetails/HostDetails.jsx index fcefd6753b..fee4611d61 100644 --- a/assets/js/components/HostDetails/HostDetails.jsx +++ b/assets/js/components/HostDetails/HostDetails.jsx @@ -131,9 +131,9 @@ function HostDetails({ - + + From a7864f5e45c6328fed8f76dcdc53c6bee47bbf5a Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Wed, 13 Sep 2023 10:24:28 +0200 Subject: [PATCH 09/10] Linting issue --- .../components/HostDetails/HostChecksSelection.stories.jsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/assets/js/components/HostDetails/HostChecksSelection.stories.jsx b/assets/js/components/HostDetails/HostChecksSelection.stories.jsx index 2d091f445f..2b08849c0f 100644 --- a/assets/js/components/HostDetails/HostChecksSelection.stories.jsx +++ b/assets/js/components/HostDetails/HostChecksSelection.stories.jsx @@ -110,8 +110,9 @@ export default { description: 'Starts the host checks execution', }, checksExecutionTooltipVisible: { - description: 'Whether to display the tooltip above the start execution button', - } + description: + 'Whether to display the tooltip above the start execution button', + }, }, }; From b4ff564fb94101a1b27e3f075d765fa4792f9601 Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Wed, 13 Sep 2023 15:58:01 +0200 Subject: [PATCH 10/10] Update tests and move UI state in `Host Checks Selection` --- .../HanaClusterDetails.test.jsx | 63 ++++++++++++++++++- .../ClusterSettingsPage.test.jsx | 43 +++++++++++++ .../HostDetails/HostChecksSelection.jsx | 21 ++++--- .../HostChecksSelection.stories.jsx | 5 -- .../HostDetails/HostChecksSelection.test.jsx | 48 ++------------ .../HostDetails/HostDetails.test.jsx | 6 +- .../HostDetails/HostSettingsPage.jsx | 16 +---- 7 files changed, 127 insertions(+), 75 deletions(-) diff --git a/assets/js/components/ClusterDetails/HanaClusterDetails.test.jsx b/assets/js/components/ClusterDetails/HanaClusterDetails.test.jsx index c976639d90..1c2f22d01e 100644 --- a/assets/js/components/ClusterDetails/HanaClusterDetails.test.jsx +++ b/assets/js/components/ClusterDetails/HanaClusterDetails.test.jsx @@ -100,9 +100,7 @@ describe('HanaClusterDetails component', () => { /> ); - expect( - screen.getByText(`Start Execution`).closest('button') - ).toBeDisabled(); + expect(screen.getByText('Start Execution')).toBeDisabled(); } ); @@ -285,4 +283,63 @@ describe('HanaClusterDetails component', () => { }); }); }); + + const suggestionScenarios = [ + { + selectedChecks: [], + hasSelectedChecks: false, + suggestionExpectation: (tooltipSuggestion) => { + tooltipSuggestion.toBeVisible(); + }, + }, + { + selectedChecks: [faker.datatype.uuid()], + hasSelectedChecks: true, + suggestionExpectation: (tooltipSuggestion) => { + tooltipSuggestion.not.toBeInTheDocument(); + }, + }, + ]; + + it.each(suggestionScenarios)( + 'should suggest to the user to select some checks only when the selection is empty', + async ({ selectedChecks, hasSelectedChecks, suggestionExpectation }) => { + const user = userEvent.setup(); + + const { + clusterID, + clusterName, + cib_last_written: cibLastWritten, + type: clusterType, + sid, + provider, + details, + } = clusterFactory.build(); + + const hosts = hostFactory.buildList(2, { cluster_id: clusterID }); + + renderWithRouter( + + ); + + const startExecutionButton = screen.getByText('Start Execution'); + await user.hover(startExecutionButton); + suggestionExpectation( + expect(screen.queryByText('Select some Checks first!')) + ); + } + ); }); diff --git a/assets/js/components/ClusterSettingsPage/ClusterSettingsPage.test.jsx b/assets/js/components/ClusterSettingsPage/ClusterSettingsPage.test.jsx index bc577b960c..13099651d9 100644 --- a/assets/js/components/ClusterSettingsPage/ClusterSettingsPage.test.jsx +++ b/assets/js/components/ClusterSettingsPage/ClusterSettingsPage.test.jsx @@ -102,4 +102,47 @@ describe('ClusterDetails ClusterSettings component', () => { ) ).toBeVisible(); }); + + const suggestionScenarios = [ + { + cluster: clusterFactory.build({ + selected_checks: [], + }), + suggestionExpectation: (tooltipSuggestion) => { + tooltipSuggestion.not.toBeInTheDocument(); + }, + }, + { + cluster: clusterFactory.build({ + selected_checks: [faker.datatype.uuid()], + }), + suggestionExpectation: (tooltipSuggestion) => { + tooltipSuggestion.toBeVisible(); + }, + }, + ]; + + it.each(suggestionScenarios)( + 'should suggest to the user to start an execution when the selection is not empty', + ({ cluster, suggestionExpectation }) => { + const { id: clusterID } = cluster; + const [StatefulClusterSettings] = withState(, { + ...defaultInitialState, + clustersList: { clusters: [cluster] }, + }); + + renderWithRouterMatch(StatefulClusterSettings, { + path: 'clusters/:clusterID/settings', + route: `/clusters/${clusterID}/settings`, + }); + + suggestionExpectation( + expect( + screen.queryByText( + 'Click Start Execution or wait for Trento to periodically run checks.' + ) + ) + ); + } + ); }); diff --git a/assets/js/components/HostDetails/HostChecksSelection.jsx b/assets/js/components/HostDetails/HostChecksSelection.jsx index 309770e555..67b9a26cbe 100644 --- a/assets/js/components/HostDetails/HostChecksSelection.jsx +++ b/assets/js/components/HostDetails/HostChecksSelection.jsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect, useState } from 'react'; import { EOS_PLAY_CIRCLE } from 'eos-icons-react'; import PageHeader from '@components/PageHeader'; @@ -9,23 +9,28 @@ import Tooltip from '@components/Tooltip'; import HostInfoBox from './HostInfoBox'; +const defaultSavedSelection = []; + function HostChecksSelection({ hostID, hostName, provider, agentVersion, - selectedChecks, catalog, catalogError, catalogLoading, onUpdateCatalog, isSavingSelection, onSaveSelection, - onSelectedChecksChange, hostChecksExecutionEnabled, onStartExecution = () => {}, - checksExecutionTooltipVisible, + savedHostSelection = defaultSavedSelection, }) { + const [selection, setSelection] = useState([]); + useEffect(() => { + setSelection(savedHostSelection); + }, [savedHostSelection]); + return (
Back to Host Details @@ -41,7 +46,7 @@ function HostChecksSelection({
); diff --git a/assets/js/components/HostDetails/HostChecksSelection.stories.jsx b/assets/js/components/HostDetails/HostChecksSelection.stories.jsx index 2b08849c0f..ffcf993b65 100644 --- a/assets/js/components/HostDetails/HostChecksSelection.stories.jsx +++ b/assets/js/components/HostDetails/HostChecksSelection.stories.jsx @@ -109,10 +109,6 @@ export default { onStartExecution: { description: 'Starts the host checks execution', }, - checksExecutionTooltipVisible: { - description: - 'Whether to display the tooltip above the start execution button', - }, }, }; @@ -128,6 +124,5 @@ export const Default = { catalogLoading: false, isSavingSelection: false, hostChecksExecutionEnabled: false, - checksExecutionTooltipVisible: false, }, }; diff --git a/assets/js/components/HostDetails/HostChecksSelection.test.jsx b/assets/js/components/HostDetails/HostChecksSelection.test.jsx index 7328efabb5..3abd6a271b 100644 --- a/assets/js/components/HostDetails/HostChecksSelection.test.jsx +++ b/assets/js/components/HostDetails/HostChecksSelection.test.jsx @@ -36,13 +36,11 @@ describe('HostChecksSelection component', () => { hostName={hostName} provider={provider} agentVersion={agentVersion} - selectedChecks={selectedChecks} catalog={catalog} catalogError={null} catalogLoading={false} onUpdateCatalog={onUpdateCatalog} hostSelectedChecks={selectedChecks} - checksExecutionTooltipVisible /> ); @@ -59,47 +57,11 @@ describe('HostChecksSelection component', () => { expect( screen.getByRole('button', { name: 'Save Checks Selection' }) ).toBeVisible(); - expect(screen.getByRole('tooltip')).toBeVisible(); - expect(onUpdateCatalog).toHaveBeenCalled(); - }); - - it('should not render tooltip when render host check selection', async () => { - const group0 = faker.animal.cat(); - const group1 = faker.animal.dog(); - const group2 = faker.lorem.word(); - const catalog = [ - ...catalogCheckFactory.buildList(2, { group: group0 }), - ...catalogCheckFactory.buildList(2, { group: group1 }), - ...catalogCheckFactory.buildList(2, { group: group2 }), - ]; - - const onUpdateCatalog = jest.fn(); - - const { - id: hostID, - hostname: hostName, - provider, - agent_version: agentVersion, - selected_checks: selectedChecks, - } = hostFactory.build({ provider: 'azure' }); - - renderWithRouter( - - ); - - expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + expect( + screen.queryByText( + 'Click Start Execution or wait for Trento to periodically run checks.' + ) + ).not.toBeInTheDocument(); expect(onUpdateCatalog).toHaveBeenCalled(); }); }); diff --git a/assets/js/components/HostDetails/HostDetails.test.jsx b/assets/js/components/HostDetails/HostDetails.test.jsx index 9d76708839..dbea4d944f 100644 --- a/assets/js/components/HostDetails/HostDetails.test.jsx +++ b/assets/js/components/HostDetails/HostDetails.test.jsx @@ -37,7 +37,7 @@ describe('HostDetails component', () => { expect(startExecutionButton).toBeDisabled(); await user.hover(startExecutionButton); - expect(screen.getByRole('tooltip')).toBeInTheDocument(); + expect(screen.getByText('Select some Checks first!')).toBeInTheDocument(); }); it('should enable start execution button when checks are selected', async () => { @@ -52,7 +52,9 @@ describe('HostDetails component', () => { expect(startExecutionButton).toBeEnabled(); await user.hover(startExecutionButton); - expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + expect( + screen.queryByText('Select some Checks first!') + ).not.toBeInTheDocument(); }); }); diff --git a/assets/js/components/HostDetails/HostSettingsPage.jsx b/assets/js/components/HostDetails/HostSettingsPage.jsx index 29699f2f24..de42eff554 100644 --- a/assets/js/components/HostDetails/HostSettingsPage.jsx +++ b/assets/js/components/HostDetails/HostSettingsPage.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { useNavigate, useParams } from 'react-router-dom'; @@ -23,14 +23,6 @@ function HostSettingsPage() { const hostSelectedChecks = useSelector((state) => getHostSelectedChecks(state, hostID) ); - const [selection, setSelection] = useState([]); - useEffect(() => { - if (host) { - setSelection(hostSelectedChecks); - } - }, [hostSelectedChecks]); - const [checksExecutionTooltipVisible, setChecksExecutionTooltipVisible] = - useState(hostSelectedChecks?.length > 0); const { data: catalog, @@ -65,12 +57,10 @@ function HostSettingsPage() { checks: newSelection, }) ); - setChecksExecutionTooltipVisible(newSelection?.length > 0); }; const requestHostChecksExecution = () => { dispatch(hostExecutionRequested(host, hostSelectedChecks, navigate)); - setChecksExecutionTooltipVisible(false); }; return ( @@ -85,11 +75,9 @@ function HostSettingsPage() { onUpdateCatalog={refreshCatalog} isSavingSelection={saving} onSaveSelection={saveSelection} - selectedChecks={selection} hostChecksExecutionEnabled={hostChecksExecutionEnabled} - onSelectedChecksChange={setSelection} onStartExecution={requestHostChecksExecution} - checksExecutionTooltipVisible={checksExecutionTooltipVisible} + savedHostSelection={hostSelectedChecks} /> ); }