From 1a568afcbb277f7501b1e6fbd95e9a3611b88ecb Mon Sep 17 00:00:00 2001 From: EMaksy Date: Fri, 12 May 2023 16:24:39 +0200 Subject: [PATCH 1/6] Group all check results in ExecutionResults view --- .../ExecutionResults/ExecutionResults.jsx | 34 +++++++++++++++---- .../ExecutionResults/checksUtils.js | 17 ++++++++++ assets/js/components/Table/Table.jsx | 9 ++++- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/assets/js/components/ExecutionResults/ExecutionResults.jsx b/assets/js/components/ExecutionResults/ExecutionResults.jsx index a1ba3c2012..4c5c4202ae 100644 --- a/assets/js/components/ExecutionResults/ExecutionResults.jsx +++ b/assets/js/components/ExecutionResults/ExecutionResults.jsx @@ -1,19 +1,20 @@ import React, { useState } from 'react'; import Table from '@components/Table'; - +import Accordion from '@components/Accordion'; import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; import Modal from '@components/Modal'; - import { getHostID } from '@state/selectors/cluster'; import PremiumPill from '@components/PremiumPill'; import HealthIcon from '@components/Health'; import { + getCatalogCategoryList, getCheckResults, getCheckDescription, getCheckRemediation, getCheckExpectations, + getCheckGroup, isPremium, } from './checksUtils'; @@ -29,7 +30,7 @@ const resultsTableConfig = { title: 'Id', key: 'checkID', fontSize: 'text-base', - className: 'bg-gray-50 border-b', + className: 'bg-gray-50 border-b w-1/6 h-auto', render: (checkID, { onClick, premium }) => (
( {description} @@ -61,7 +62,7 @@ const resultsTableConfig = { title: 'Result', key: 'result', fontSize: 'text-base', - className: 'bg-gray-50 border-b', + className: 'bg-gray-50 border-b w-1/6 h-auto text-base', render: (_, { result }) => , }, ], @@ -116,7 +117,9 @@ function ExecutionResults({ } }; - const tableData = getCheckResults(executionData) + const checksResults = getCheckResults(executionData); + const catalogCategoryList = getCatalogCategoryList(catalog, checksResults); + const tableData = checksResults .filter((check) => { if (predicates.length === 0) { return true; @@ -135,6 +138,7 @@ function ExecutionResults({ checkID, result, clusterName, + category: getCheckGroup(catalog, checkID), executionState: executionData?.status, description: getCheckDescription(catalog, checkID), expectations: getCheckExpectations(catalog, checkID), @@ -147,6 +151,7 @@ function ExecutionResults({ }, }) ); + return ( - + {catalogCategoryList.map((item) => ( + +
obj.category === item) + .sort((a, b) => a.description.localeCompare(b.description))} + withPadding={false} + /> + + ))} { return false; }; +export const getCatalogCategoryList = (catalog, checksResults = []) => + [ + ...new Set( + checksResults.map( + ({ check_id }) => catalog.find((check) => check.id === check_id).group + ) + ), + ].sort(); + export const getCheckDescription = (catalog, checkID) => { const check = findCheck(catalog, checkID); if (check) { @@ -46,6 +55,14 @@ export const getCheckDescription = (catalog, checkID) => { return null; }; +export const getCheckGroup = (catalog, checkID) => { + const check = findCheck(catalog, checkID); + if (check) { + return check.group; + } + return null; +}; + export const getCheckRemediation = (catalog, checkID) => { const check = findCheck(catalog, checkID); if (check) { diff --git a/assets/js/components/Table/Table.jsx b/assets/js/components/Table/Table.jsx index 17a0e72bd3..ebce1d01f8 100644 --- a/assets/js/components/Table/Table.jsx +++ b/assets/js/components/Table/Table.jsx @@ -60,6 +60,7 @@ function Table({ searchParams, setSearchParams, emptyStateText = 'No data available', + withPadding = true, }) { const { columns, @@ -148,7 +149,13 @@ function Table({ />
-
+
From 26200718d1bee055d198e693d4d2c789e4434cf3 Mon Sep 17 00:00:00 2001 From: EMaksy Date: Fri, 12 May 2023 16:27:24 +0200 Subject: [PATCH 2/6] Add unit tests and update exisitng test --- .../ExecutionResults.test.jsx | 22 ++--------- .../ExecutionResults/checksUtils.test.js | 39 +++++++++++++++++++ 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/assets/js/components/ExecutionResults/ExecutionResults.test.jsx b/assets/js/components/ExecutionResults/ExecutionResults.test.jsx index 4cddf5c640..d144760f3d 100644 --- a/assets/js/components/ExecutionResults/ExecutionResults.test.jsx +++ b/assets/js/components/ExecutionResults/ExecutionResults.test.jsx @@ -3,7 +3,6 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { faker } from '@faker-js/faker'; import { renderWithRouter } from '@lib/test-utils'; - import { catalogFactory, catalogCheckFactory, @@ -180,23 +179,10 @@ describe('ExecutionResults', () => { ).toHaveLength(1); expect(screen.getAllByText('2/2 Expectations met.')).toHaveLength(2); expect(screen.getAllByText('1/2 Expectations met.')).toHaveLength(2); - - const mainTable = screen.getByRole('table'); - const tableRows = mainTable.querySelectorAll('tbody > tr'); - - expect(tableRows).toHaveLength(2 * executionData.check_results.length); - - expect(tableRows[0]).toHaveTextContent(checkID1); - expect(tableRows[1]).toHaveTextContent(clusterName); - expect(tableRows[1]).toHaveTextContent( - /Value `.*` is the same on all targets/ - ); - expect(tableRows[1]).toHaveTextContent(clusterHosts[0].hostname); - expect(tableRows[1]).toHaveTextContent('2/2 Expectations met.'); - - expect(tableRows[2]).toHaveTextContent(checkID2); - expect(tableRows[3]).toHaveTextContent(clusterHosts[1].hostname); - expect(tableRows[3]).toHaveTextContent('1/2 Expectations met'); + expect(screen.getByText(checkID1)).toHaveTextContent(checkID1); + expect(screen.getByText(checkID2)).toHaveTextContent(checkID2); + expect(screen.getByText(catalog[0].description)).toBeInTheDocument(); + expect(screen.getByText(catalog[1].description)).toBeInTheDocument(); }); it('should render the execution starting dialog, when an execution is not started yet', () => { diff --git a/assets/js/components/ExecutionResults/checksUtils.test.js b/assets/js/components/ExecutionResults/checksUtils.test.js index 15b61793e5..ac5cd2b432 100644 --- a/assets/js/components/ExecutionResults/checksUtils.test.js +++ b/assets/js/components/ExecutionResults/checksUtils.test.js @@ -15,7 +15,9 @@ import { import { EXPECT, EXPECT_SAME } from '@lib/model'; import { + getCatalogCategoryList, getCheckDescription, + getCheckGroup, getCheckRemediation, getCheckResults, getCheckExpectations, @@ -49,6 +51,36 @@ describe('checksUtils', () => { }); }); + it('getCatalogCategoryList should return a sorted category list where it is matching', () => { + const fakerListID = [ + faker.datatype.number(), + faker.datatype.number(), + faker.datatype.number(), + ]; + const execution = [ + { check_id: fakerListID[0] }, + { check_id: fakerListID[1] }, + { check_id: fakerListID[2] }, + ]; + const catalog = [ + { id: fakerListID[0], group: 'Category C' }, + { id: fakerListID[1], group: 'Category A' }, + { id: fakerListID[2], group: 'Category B' }, + ]; + const expected = ['Category A', 'Category B', 'Category C']; + expect(getCatalogCategoryList(catalog, execution)).toEqual(expected); + }); + + it('getCatalogCategoryList should return an empty array if checksResults is not provided', () => { + const fakerListID = [faker.datatype.number(), faker.datatype.number()]; + const catalog = [ + { id: fakerListID[0], group: faker.lorem.word() }, + { id: fakerListID[1], group: faker.lorem.word() }, + ]; + const expected = []; + expect(getCatalogCategoryList(catalog, undefined)).toEqual(expected); + }); + it('getDescription should return a check description', () => { const catalog = catalogCheckFactory.buildList(2); const [{ id, description }] = catalog; @@ -56,6 +88,13 @@ describe('checksUtils', () => { expect(getCheckDescription(catalog, id)).toBe(description); }); + it('getCheckGroup should return a check grupp', () => { + const catalog = catalogCheckFactory.buildList(2); + const check = catalog[0]; + + expect(getCheckGroup(catalog, check.id)).toBe(check.group); + }); + it('getCheckRemediation should return a check remediation', () => { const catalog = catalogCheckFactory.buildList(2); const [{ id, remediation }] = catalog; From 1f35e055d76e2e10abec4cedf58703c849066c42 Mon Sep 17 00:00:00 2001 From: EMaksy Date: Fri, 12 May 2023 16:27:48 +0200 Subject: [PATCH 3/6] Update the ExecutionResults story --- .../ExecutionResults.stories.jsx | 223 ++++++++++++++++-- 1 file changed, 208 insertions(+), 15 deletions(-) diff --git a/assets/js/components/ExecutionResults/ExecutionResults.stories.jsx b/assets/js/components/ExecutionResults/ExecutionResults.stories.jsx index 41d115056e..649b746cdc 100644 --- a/assets/js/components/ExecutionResults/ExecutionResults.stories.jsx +++ b/assets/js/components/ExecutionResults/ExecutionResults.stories.jsx @@ -5,7 +5,33 @@ import { hostFactory, clusterFactory } from '@lib/test-utils/factories'; import ExecutionResults from '.'; const executionID = '9fbcaec6-e65e-4adc-9cac-fc542c66717b'; -const agentID = '22248a4c-216f-45d8-90ff-904d27900efc'; +const agentID = [ + '8a2a4977-357d-4b76-b3c0-9b6a0e334d9d', + 'e1b2fc0e-8eae-42bb-81a7-6ddc8d13e05b', + '3f9675e9-2c59-4f0e-a1e8-7ebd4df3d90c', + 'b1dc32a5-9466-4e2d-bd4f-9a462c153c36', +]; +const checkID = ['DC5429', 'FB0E0D', '68626E', '15F7A8']; +const checkNames = [ + 'SBD_PACEMAKER', + 'corosync running 2 ring configuration', + 'SBD msgwait timeout', + 'Check Corosync token_retransmits_before_loss_const during runtime', +]; +const checkResults = ['passing', 'warning', 'critical', 'unknown']; +const checkIsPremium = [true, false]; +const checkGroup = ['Corosync', 'SBD']; +const checkDescription = [ + 'Corosync `token` timeout is set to expected value\n', + 'Corosync is running with consensus timeout set to the recommended value', + 'SBD msgwait timeout value is at least two times the watchdog timeout', + 'Corosync is running with `token_retransmits_before_loss_const` set to the recommended value', +]; +const checkRemediation = [ + 'Additional remediation instructions', + 'Abstract the value of the Corosync `token` timeout is not set as recommended', +]; + const groupID = '02acea9d-9658-4902-9806-0eef2bfbbf5d'; const cloudProvider = 'azure'; @@ -16,7 +42,7 @@ const { name: clusterName, type: clusterScenario } = clusterFactory.build({ const clusterHosts = [ hostFactory.build({ - id: agentID, + id: agentID[0], hostname: 'carbonarahost01', cluster_id: groupID, }), @@ -41,25 +67,90 @@ const completedExecution = { { agents_check_results: [ { - agent_id: agentID, + agent_id: agentID[0], expectation_evaluations: [ { - name: 'expectation_example', + name: checkNames[0], return_value: 123, type: 'expect', }, ], facts: [ - { check_id: '156F64', name: 'lol_this_is_a_fact', value: 123 }, + { check_id: checkID[0], name: 'lol_this_is_a_fact', value: 123 }, + ], + values: [], + }, + ], + check_id: checkID[0], + expectation_results: [ + { name: checkNames[0], result: true, type: 'expect' }, + ], + result: checkResults[0], + }, + + { + agents_check_results: [ + { + agent_id: agentID[1], + expectation_evaluations: [ + { + name: checkNames[1], + return_value: 456, + type: 'expect', + }, + ], + facts: [{ check_id: checkID[1], name: 'new_fact', value: 456 }], + values: [], + }, + ], + check_id: checkID[1], + expectation_results: [ + { name: checkNames[1], result: true, type: 'expect' }, + ], + result: checkResults[1], + }, + + { + agents_check_results: [ + { + agent_id: agentID[2], + expectation_evaluations: [ + { + name: checkNames[2], + return_value: 456, + type: 'expect', + }, ], + facts: [{ check_id: checkID[2], name: 'new_fact', value: 456 }], values: [], }, ], - check_id: '156F64', + check_id: checkID[2], expectation_results: [ - { name: 'expectation_example', result: true, type: 'expect' }, + { name: checkNames[2], result: true, type: 'expect' }, ], - result: 'passing', + result: checkResults[2], + }, + { + agents_check_results: [ + { + agent_id: agentID[3], + expectation_evaluations: [ + { + name: checkNames[3], + return_value: 456, + type: 'expect', + }, + ], + facts: [{ check_id: checkID[3], name: 'new_fact', value: 456 }], + values: [], + }, + ], + check_id: checkID[3], + expectation_results: [ + { name: checkNames[3], result: true, type: 'expect' }, + ], + result: checkResults[3], }, ], completed_at: '2022-11-09T17:02:20.629366Z', @@ -76,7 +167,7 @@ const catalogData = { data: { items: [ { - description: 'Corosync `token` timeout is set to expected value\n', + description: checkDescription[0], expectations: [ { expression: @@ -92,12 +183,11 @@ const catalogData = { name: 'corosync_token_timeout', }, ], - group: 'Corosync', - id: '156F64', - name: 'Corosync configuration file', - remediation: - '## Abstract\nThe value of the Corosync `token` timeout is not set as recommended.\n## Remediation\n...\n', - severity: 'critical', + group: checkGroup[0], + premium: checkIsPremium[1], + id: checkID[0], + name: checkNames[0], + remediation: checkRemediation[1], values: [ { conditions: [ @@ -112,6 +202,109 @@ const catalogData = { }, ], }, + { + id: checkID[1], + description: checkDescription[1], + expectations: [ + { + expression: 'additional_expression', + name: 'additional_expectation', + type: 'expect', + }, + ], + facts: [ + { + argument: 'additional_argument', + gatherer: 'additional_gatherer', + name: 'additional_fact', + }, + ], + group: checkGroup[0], + premium: checkIsPremium[0], + name: checkNames[1], + remediation: checkRemediation[0], + values: [ + { + conditions: [ + { + expression: 'additional_condition_expression', + value: 'additional_condition_value', + }, + ], + default: 'additional_default_value', + name: 'additional_value', + }, + ], + }, + { + id: checkID[2], + description: checkDescription[2], + expectations: [ + { + expression: 'additional_expression', + name: 'additional_expectation', + type: 'expect', + }, + ], + facts: [ + { + argument: 'additional_argument', + gatherer: 'additional_gatherer', + name: 'additional_fact', + }, + ], + group: checkGroup[1], + premium: checkIsPremium[0], + name: checkNames[2], + remediation: checkRemediation[0], + values: [ + { + conditions: [ + { + expression: 'additional_condition_expression', + value: 'additional_condition_value', + }, + ], + default: 'additional_default_value', + name: 'additional_value', + }, + ], + }, + { + id: checkID[3], + description: checkDescription[3], + expectations: [ + { + expression: 'additional_expression', + name: 'additional_expectation', + type: 'expect', + }, + ], + facts: [ + { + argument: 'additional_argument', + gatherer: 'additional_gatherer', + name: 'additional_fact', + }, + ], + group: checkGroup[0], + premium: checkIsPremium[1], + name: checkNames[3], + remediation: checkRemediation[0], + + values: [ + { + conditions: [ + { + expression: 'additional_condition_expression', + value: 'additional_condition_value', + }, + ], + default: 'additional_default_value', + name: 'additional_value', + }, + ], + }, ], }, }; From c8832ca491be918dea72779cfa286e4975d80922 Mon Sep 17 00:00:00 2001 From: EMaksy Date: Fri, 12 May 2023 17:34:25 +0200 Subject: [PATCH 4/6] Fix bug when page is reloaded --- assets/js/components/ExecutionResults/checksUtils.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/assets/js/components/ExecutionResults/checksUtils.js b/assets/js/components/ExecutionResults/checksUtils.js index 012da68e2a..b9ecc4f62d 100644 --- a/assets/js/components/ExecutionResults/checksUtils.js +++ b/assets/js/components/ExecutionResults/checksUtils.js @@ -38,14 +38,18 @@ export const isPremium = (catalog, checkID) => { return false; }; -export const getCatalogCategoryList = (catalog, checksResults = []) => - [ +export const getCatalogCategoryList = (catalog, checksResults = []) => { + if (catalog.length === 0) { + return []; + } + return [ ...new Set( checksResults.map( ({ check_id }) => catalog.find((check) => check.id === check_id).group ) ), ].sort(); +}; export const getCheckDescription = (catalog, checkID) => { const check = findCheck(catalog, checkID); From 479cf239b95ad660d8d2843e0489dfaa547e4c8a Mon Sep 17 00:00:00 2001 From: EMaksy Date: Sun, 14 May 2023 17:01:44 +0200 Subject: [PATCH 5/6] Apply comment suggestions --- .../ExecutionResults/ExecutionResults.jsx | 20 ++++++------- .../ExecutionResults/checksUtils.js | 2 +- .../ExecutionResults/checksUtils.test.js | 28 ++++++++++--------- assets/js/components/Table/Table.jsx | 8 ++---- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/assets/js/components/ExecutionResults/ExecutionResults.jsx b/assets/js/components/ExecutionResults/ExecutionResults.jsx index 4c5c4202ae..983acd1671 100644 --- a/assets/js/components/ExecutionResults/ExecutionResults.jsx +++ b/assets/js/components/ExecutionResults/ExecutionResults.jsx @@ -1,13 +1,12 @@ import React, { useState } from 'react'; -import Table from '@components/Table'; -import Accordion from '@components/Accordion'; import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; -import Modal from '@components/Modal'; import { getHostID } from '@state/selectors/cluster'; -import PremiumPill from '@components/PremiumPill'; +import Accordion from '@components/Accordion'; import HealthIcon from '@components/Health'; - +import Modal from '@components/Modal'; +import PremiumPill from '@components/PremiumPill'; +import Table from '@components/Table'; import { getCatalogCategoryList, getCheckResults, @@ -62,7 +61,7 @@ const resultsTableConfig = { title: 'Result', key: 'result', fontSize: 'text-base', - className: 'bg-gray-50 border-b w-1/6 h-auto text-base', + className: 'bg-gray-50 border-b w-1/6 h-auto', render: (_, { result }) => , }, ], @@ -151,6 +150,10 @@ function ExecutionResults({ }, }) ); + const filterAndSortData = (data, item) => + data + .filter((obj) => obj.category === item) + .sort((a, b) => a.description.localeCompare(b.description)); return (
obj.category === item) - .sort((a, b) => a.description.localeCompare(b.description))} + data={filterAndSortData(tableData, item)} withPadding={false} /> @@ -212,5 +213,4 @@ function ExecutionResults({ ); } - export default ExecutionResults; diff --git a/assets/js/components/ExecutionResults/checksUtils.js b/assets/js/components/ExecutionResults/checksUtils.js index b9ecc4f62d..1d7a1f48f9 100644 --- a/assets/js/components/ExecutionResults/checksUtils.js +++ b/assets/js/components/ExecutionResults/checksUtils.js @@ -39,7 +39,7 @@ export const isPremium = (catalog, checkID) => { }; export const getCatalogCategoryList = (catalog, checksResults = []) => { - if (catalog.length === 0) { + if (!catalog || catalog.length === 0 || !checksResults) { return []; } return [ diff --git a/assets/js/components/ExecutionResults/checksUtils.test.js b/assets/js/components/ExecutionResults/checksUtils.test.js index ac5cd2b432..3642c1275d 100644 --- a/assets/js/components/ExecutionResults/checksUtils.test.js +++ b/assets/js/components/ExecutionResults/checksUtils.test.js @@ -52,33 +52,35 @@ describe('checksUtils', () => { }); it('getCatalogCategoryList should return a sorted category list where it is matching', () => { - const fakerListID = [ + const IDs = [ faker.datatype.number(), faker.datatype.number(), faker.datatype.number(), ]; - const execution = [ - { check_id: fakerListID[0] }, - { check_id: fakerListID[1] }, - { check_id: fakerListID[2] }, + const checksResults = [ + { check_id: IDs[0] }, + { check_id: IDs[1] }, + { check_id: IDs[2] }, ]; const catalog = [ - { id: fakerListID[0], group: 'Category C' }, - { id: fakerListID[1], group: 'Category A' }, - { id: fakerListID[2], group: 'Category B' }, + { id: IDs[0], group: 'Category C' }, + { id: IDs[1], group: 'Category A' }, + { id: IDs[2], group: 'Category B' }, ]; const expected = ['Category A', 'Category B', 'Category C']; - expect(getCatalogCategoryList(catalog, execution)).toEqual(expected); + expect(getCatalogCategoryList(catalog, checksResults)).toEqual(expected); }); it('getCatalogCategoryList should return an empty array if checksResults is not provided', () => { - const fakerListID = [faker.datatype.number(), faker.datatype.number()]; + const IDs = [faker.datatype.number(), faker.datatype.number()]; const catalog = [ - { id: fakerListID[0], group: faker.lorem.word() }, - { id: fakerListID[1], group: faker.lorem.word() }, + { id: IDs[0], group: faker.lorem.word() }, + { id: IDs[1], group: faker.lorem.word() }, ]; const expected = []; - expect(getCatalogCategoryList(catalog, undefined)).toEqual(expected); + [[], undefined, null].forEach((item) => { + expect(getCatalogCategoryList(catalog, item)).toEqual(expected); + }); }); it('getDescription should return a check description', () => { diff --git a/assets/js/components/Table/Table.jsx b/assets/js/components/Table/Table.jsx index ebce1d01f8..4974a51440 100644 --- a/assets/js/components/Table/Table.jsx +++ b/assets/js/components/Table/Table.jsx @@ -150,11 +150,9 @@ function Table({
From 83c6cfa358a69513c7510634644a8c2bd6d492b9 Mon Sep 17 00:00:00 2001 From: EMaksy Date: Mon, 15 May 2023 09:54:01 +0200 Subject: [PATCH 6/6] Add gray color to ChecksResultOutline --- assets/js/components/ExecutionResults/CheckResultOutline.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/ExecutionResults/CheckResultOutline.jsx b/assets/js/components/ExecutionResults/CheckResultOutline.jsx index 52d945de3d..2343113cb5 100644 --- a/assets/js/components/ExecutionResults/CheckResultOutline.jsx +++ b/assets/js/components/ExecutionResults/CheckResultOutline.jsx @@ -75,7 +75,7 @@ function CheckResultOutline({ ); return ( -
+