From 2744afc339bfcd71100275844d840072bc8eab49 Mon Sep 17 00:00:00 2001 From: arbulu89 Date: Thu, 3 Nov 2022 14:28:07 +0100 Subject: [PATCH 1/6] Duplicate current catalog view in a new route --- .../ChecksCatalog/ChecksCatalogNew.jsx | 138 ++++++++++++++++++ assets/js/components/ChecksCatalog/index.js | 2 + assets/js/trento.jsx | 2 + 3 files changed, 142 insertions(+) create mode 100644 assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx diff --git a/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx b/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx new file mode 100644 index 0000000000..8c7af956d5 --- /dev/null +++ b/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx @@ -0,0 +1,138 @@ +import React, { useState, useEffect } from 'react'; +import { useSelector, useDispatch } from 'react-redux'; + +import { Disclosure, Transition } from '@headlessui/react'; + +import ReactMarkdown from 'react-markdown'; +import remarkGfm from 'remark-gfm'; + +import ProviderSelection from './ProviderSelection'; +import NotificationBox from '@components/NotificationBox'; +import LoadingBox from '@components/LoadingBox'; + +import { EOS_ERROR } from 'eos-icons-react'; + +export const ChecksCatalogNew = () => { + const dispatch = useDispatch(); + + const [catalogData, catalogError, loading] = useSelector((state) => [ + state.catalog.data, + state.catalog.error, + state.catalog.loading, + ]); + + const providers = catalogData.map((provider) => provider.provider); + const [selected, setSelected] = useState(providers[0]); + + const dispatchUpdateCatalog = () => { + dispatch({ + type: 'UPDATE_CATALOG', + payload: {}, + }); + }; + + useEffect(() => { + setSelected(providers[0]); + }, [providers[0]]); + + useEffect(() => { + dispatchUpdateCatalog(); + }, [dispatch]); + + if (loading) { + return ; + } + + if (catalogError) { + return ( + } + text={catalogError} + buttonText="Try again" + buttonOnClick={dispatchUpdateCatalog} + /> + ); + } + + return ( +
+ + {catalogData + .filter((provider) => provider.provider == selected) + .map(({ _, groups }) => + groups?.map(({ group, checks }, idx) => ( +
+
+

+ {group} +

+
+
    + {checks.map((check) => ( +
  • + + +
    +
    +

    + {check.id} +

    + {check.premium > 0 && ( +

    + Premium +

    + )} +
    +
    +
    + + {check.description} + +
    +
    +
    +     + + +
    +
    + + {check.remediation} + +
    +
    +     +     +     +
    • + ))} +
+
+ )) + )} +
+ ); +}; \ No newline at end of file diff --git a/assets/js/components/ChecksCatalog/index.js b/assets/js/components/ChecksCatalog/index.js index e2133b19cf..28776e7e6b 100644 --- a/assets/js/components/ChecksCatalog/index.js +++ b/assets/js/components/ChecksCatalog/index.js @@ -1,3 +1,5 @@ import ChecksCatalog from './ChecksCatalog'; +export { ChecksCatalogNew } from './ChecksCatalogNew'; + export default ChecksCatalog; diff --git a/assets/js/trento.jsx b/assets/js/trento.jsx index 85904fc9d6..7931067081 100644 --- a/assets/js/trento.jsx +++ b/assets/js/trento.jsx @@ -24,6 +24,7 @@ import DatabasesOverview from '@components/DatabasesOverview'; import SapSystemDetails from './components/SapSystemDetails/SapSystemDetails'; import DatabaseDetails from './components/DatabaseDetails'; import ChecksCatalog from '@components/ChecksCatalog'; +import { ChecksCatalogNew } from '@components/ChecksCatalog'; import NotFound from '@components/NotFound'; import SomethingWentWrong from '@components/SomethingWentWrong'; import Settings from '@components/Settings'; @@ -49,6 +50,7 @@ const App = () => { } /> } /> } /> + } /> } /> } /> Date: Mon, 7 Nov 2022 14:25:14 +0100 Subject: [PATCH 2/6] Implement new catalog container --- .../ChecksCatalog/CatalogContainer.jsx | 112 +++++++++++++ .../ChecksCatalog/ChecksCatalogNew.jsx | 156 ++++-------------- 2 files changed, 141 insertions(+), 127 deletions(-) create mode 100644 assets/js/components/ChecksCatalog/CatalogContainer.jsx diff --git a/assets/js/components/ChecksCatalog/CatalogContainer.jsx b/assets/js/components/ChecksCatalog/CatalogContainer.jsx new file mode 100644 index 0000000000..7eb4f53e35 --- /dev/null +++ b/assets/js/components/ChecksCatalog/CatalogContainer.jsx @@ -0,0 +1,112 @@ +import React from 'react'; + +import { Disclosure, Transition } from '@headlessui/react'; + +import ReactMarkdown from 'react-markdown'; +import remarkGfm from 'remark-gfm'; + +import NotificationBox from '@components/NotificationBox'; +import LoadingBox from '@components/LoadingBox'; + +import { groupBy } from '@lib/lists'; + +import { EOS_ERROR } from 'eos-icons-react'; + +const CatalogContainer = ({ + getCatalog = () => {}, + catalogData = [], + catalogError = null, + loading = false, +}) => { + if (loading) { + return ; + } + + if (catalogError) { + return ( + } + text={catalogError} + buttonText="Try again" + buttonOnClick={getCatalog} + /> + ); + } + + return ( +
+ {Object.entries(groupBy(catalogData.items, 'group')).map( + ([group, checks], idx) => ( +
+
+

+ {group} +

+
+
    + {checks.map((check) => ( +
  • + + +
    +
    +

    + {check.id} +

    + {check.premium > 0 && ( +

    + Premium +

    + )} +
    +
    +
    + + {check.description} + +
    +
    +
    +     + + +
    +
    + + {check.remediation} + +
    +
    +     +     +     +
    • + ))} +
+
+ ) + )} +
+ ); +}; + +export default CatalogContainer; diff --git a/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx b/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx index 8c7af956d5..f7da317904 100644 --- a/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx +++ b/assets/js/components/ChecksCatalog/ChecksCatalogNew.jsx @@ -1,138 +1,40 @@ import React, { useState, useEffect } from 'react'; -import { useSelector, useDispatch } from 'react-redux'; -import { Disclosure, Transition } from '@headlessui/react'; +import axios from 'axios'; -import ReactMarkdown from 'react-markdown'; -import remarkGfm from 'remark-gfm'; +import CatalogContainer from './CatalogContainer'; -import ProviderSelection from './ProviderSelection'; -import NotificationBox from '@components/NotificationBox'; -import LoadingBox from '@components/LoadingBox'; - -import { EOS_ERROR } from 'eos-icons-react'; +const wandaURL = process.env.WANDA_URL; export const ChecksCatalogNew = () => { - const dispatch = useDispatch(); - - const [catalogData, catalogError, loading] = useSelector((state) => [ - state.catalog.data, - state.catalog.error, - state.catalog.loading, - ]); - - const providers = catalogData.map((provider) => provider.provider); - const [selected, setSelected] = useState(providers[0]); - - const dispatchUpdateCatalog = () => { - dispatch({ - type: 'UPDATE_CATALOG', - payload: {}, - }); - }; + const [catalogError, setError] = useState(null); + const [loading, setLoaded] = useState(true); + const [catalogData, setCatalog] = useState([]); useEffect(() => { - setSelected(providers[0]); - }, [providers[0]]); - - useEffect(() => { - dispatchUpdateCatalog(); - }, [dispatch]); - - if (loading) { - return ; - } - - if (catalogError) { - return ( - } - text={catalogError} - buttonText="Try again" - buttonOnClick={dispatchUpdateCatalog} - /> - ); - } + getCatalog(); + }, []); + + const getCatalog = () => { + setLoaded(true); + axios + .get(`${wandaURL}/api/checks/catalog`) + .then((catalog) => { + setLoaded(false); + setCatalog(catalog.data); + }) + .catch(function (error) { + setLoaded(false); + setError(error.message); + }); + }; return ( -
- - {catalogData - .filter((provider) => provider.provider == selected) - .map(({ _, groups }) => - groups?.map(({ group, checks }, idx) => ( -
-
-

- {group} -

-
-
    - {checks.map((check) => ( -
  • - - -
    -
    -

    - {check.id} -

    - {check.premium > 0 && ( -

    - Premium -

    - )} -
    -
    -
    - - {check.description} - -
    -
    -
    -     - - -
    -
    - - {check.remediation} - -
    -
    -     -     -     -
    • - ))} -
-
- )) - )} -
+ ); -}; \ No newline at end of file +}; From 896fe47145d1782589c4b976aed41c61a5b7c7e4 Mon Sep 17 00:00:00 2001 From: arbulu89 Date: Mon, 7 Nov 2022 14:25:35 +0100 Subject: [PATCH 3/6] Add tests to the catalog container --- .../ChecksCatalog/CatalogContainer.test.jsx | 84 +++++++++++++++++++ assets/js/lib/test-utils/factories.js | 8 ++ 2 files changed, 92 insertions(+) create mode 100644 assets/js/components/ChecksCatalog/CatalogContainer.test.jsx diff --git a/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx b/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx new file mode 100644 index 0000000000..4df08f6d81 --- /dev/null +++ b/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx @@ -0,0 +1,84 @@ +import React from 'react'; + +import { screen, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import userEvent from '@testing-library/user-event'; + +import { faker } from '@faker-js/faker'; +import { renderWithRouter } from '@lib/test-utils'; +import { catalogCheckFactory } from '@lib/test-utils/factories'; + +import CatalogContainer from './CatalogContainer'; + +describe('ChecksCatalog CatalogContainer component', () => { + it('should render the notification box', () => { + renderWithRouter(); + + expect(screen.getByRole('button')).toHaveTextContent('Try again'); + }); + + it('should render the loading box', () => { + renderWithRouter(); + + expect(screen.getByText('Loading checks catalog...')).toBeVisible(); + }); + + it('should render the checks catalog', () => { + const groupName1 = faker.animal.cat(); + const groupName2 = faker.animal.cat(); + const group1 = catalogCheckFactory.buildList(5, { group: groupName1 }); + const group2 = catalogCheckFactory.buildList(5, { group: groupName2 }); + const catalog = { items: group1.concat(group2) }; + + renderWithRouter( + + ); + + const groups = screen.getAllByRole('list'); + expect(groups.length).toBe(2); + + for (let group of groups) { + let { getAllByRole } = within(group); + let checks = getAllByRole('listitem'); + expect(checks.length).toBe(5); + } + }); + + it('should show check remediation when the row is clicked', () => { + const groupName = faker.animal.cat(); + const catalogChecks = catalogCheckFactory.buildList(2, { + group: groupName, + }); + const checkRemediation1 = catalogChecks[0].remediation; + const checkRemediation2 = catalogChecks[1].remediation; + const catalog = { items: catalogChecks }; + + renderWithRouter( + + ); + + const groups = screen.getAllByRole('list'); + const { getAllByRole } = within(groups[0]); + let checks = getAllByRole('listitem'); + const check1 = checks[0].querySelector('div'); + const check2 = checks[1].querySelector('div'); + + expect(screen.queryByText(checkRemediation1)).not.toBeInTheDocument(); + userEvent.click(check1); + expect(screen.getByText(checkRemediation1)).toBeVisible(); + userEvent.click(check1); + expect(screen.queryByText(checkRemediation1)).not.toBeInTheDocument(); + + expect(screen.queryByText(checkRemediation2)).not.toBeInTheDocument(); + userEvent.click(check2); + expect(screen.getByText(checkRemediation2)).toBeVisible(); + }); +}); diff --git a/assets/js/lib/test-utils/factories.js b/assets/js/lib/test-utils/factories.js index b9841c65a9..2bf400327c 100644 --- a/assets/js/lib/test-utils/factories.js +++ b/assets/js/lib/test-utils/factories.js @@ -27,3 +27,11 @@ export const healthSummaryFactory = Factory.define(() => ({ casing: 'upper', }), })); + +export const catalogCheckFactory = Factory.define(() => ({ + id: faker.datatype.uuid(), + name: faker.animal.cat(), + group: faker.animal.cat(), + description: faker.lorem.paragraph(), + remediation: faker.lorem.paragraph(), +})); From 0d2e52f20b9d46b862ec3c42b05cac8813162e64 Mon Sep 17 00:00:00 2001 From: arbulu89 Date: Mon, 7 Nov 2022 15:02:33 +0100 Subject: [PATCH 4/6] Include process as global variable --- assets/.eslintrc.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/.eslintrc.js b/assets/.eslintrc.js index 95080d8bd8..d98c1fc48b 100644 --- a/assets/.eslintrc.js +++ b/assets/.eslintrc.js @@ -6,6 +6,9 @@ module.exports = { jquery: true, 'jest/globals': true, }, + globals: { + process: true, + }, extends: ['eslint:recommended', 'plugin:react/recommended'], parserOptions: { ecmaFeatures: { From 0e626774227ad98c09621a9877a84193ff21895a Mon Sep 17 00:00:00 2001 From: arbulu89 Date: Tue, 8 Nov 2022 09:33:23 +0100 Subject: [PATCH 5/6] Show error message if the checks catalog is empty --- .../ChecksCatalog/CatalogContainer.jsx | 17 ++++++++++++++--- .../ChecksCatalog/CatalogContainer.test.jsx | 14 +++++++++++--- .../ChecksCatalog/ChecksCatalogNew.jsx | 11 ++++++----- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/assets/js/components/ChecksCatalog/CatalogContainer.jsx b/assets/js/components/ChecksCatalog/CatalogContainer.jsx index 7eb4f53e35..22734abdb6 100644 --- a/assets/js/components/ChecksCatalog/CatalogContainer.jsx +++ b/assets/js/components/ChecksCatalog/CatalogContainer.jsx @@ -13,7 +13,7 @@ import { groupBy } from '@lib/lists'; import { EOS_ERROR } from 'eos-icons-react'; const CatalogContainer = ({ - getCatalog = () => {}, + onRefresh = () => {}, catalogData = [], catalogError = null, loading = false, @@ -28,14 +28,25 @@ const CatalogContainer = ({ icon={} text={catalogError} buttonText="Try again" - buttonOnClick={getCatalog} + buttonOnClick={onRefresh} + /> + ); + } + + if (catalogData.length === 0) { + return ( + } + text="Checks catalog is empty." + buttonText="Try again" + buttonOnClick={onRefresh} /> ); } return (
- {Object.entries(groupBy(catalogData.items, 'group')).map( + {Object.entries(groupBy(catalogData, 'group')).map( ([group, checks], idx) => (
{ it('should render the notification box', () => { - renderWithRouter(); + renderWithRouter(); + expect(screen.getByText('some error')).toBeVisible(); expect(screen.getByRole('button')).toHaveTextContent('Try again'); }); @@ -23,12 +24,19 @@ describe('ChecksCatalog CatalogContainer component', () => { expect(screen.getByText('Loading checks catalog...')).toBeVisible(); }); + it('should render an error message if the checks catalog is empty', () => { + renderWithRouter(); + + expect(screen.getByText('Checks catalog is empty.')).toBeVisible(); + expect(screen.getByRole('button')).toHaveTextContent('Try again'); + }); + it('should render the checks catalog', () => { const groupName1 = faker.animal.cat(); const groupName2 = faker.animal.cat(); const group1 = catalogCheckFactory.buildList(5, { group: groupName1 }); const group2 = catalogCheckFactory.buildList(5, { group: groupName2 }); - const catalog = { items: group1.concat(group2) }; + const catalog = group1.concat(group2); renderWithRouter( { }); const checkRemediation1 = catalogChecks[0].remediation; const checkRemediation2 = catalogChecks[1].remediation; - const catalog = { items: catalogChecks }; + const catalog = catalogChecks; renderWithRouter( { axios .get(`${wandaURL}/api/checks/catalog`) .then((catalog) => { - setLoaded(false); - setCatalog(catalog.data); + setCatalog(catalog.data.items); }) - .catch(function (error) { - setLoaded(false); + .catch((error) => { setError(error.message); + }) + .finally(() => { + setLoaded(false); }); }; return ( getCatalog()} catalogData={catalogData} catalogError={catalogError} loading={loading} From 470b64c10625ad015c6df1a4294aa53f6ed00ad2 Mon Sep 17 00:00:00 2001 From: arbulu89 Date: Tue, 8 Nov 2022 09:59:16 +0100 Subject: [PATCH 6/6] Extract check item to individual component --- .../ChecksCatalog/CatalogContainer.jsx | 66 +++---------------- .../ChecksCatalog/CatalogContainer.test.jsx | 35 ---------- .../js/components/ChecksCatalog/CheckItem.jsx | 62 +++++++++++++++++ .../ChecksCatalog/CheckItem.test.jsx | 66 +++++++++++++++++++ assets/js/trento.jsx | 3 +- 5 files changed, 138 insertions(+), 94 deletions(-) create mode 100644 assets/js/components/ChecksCatalog/CheckItem.jsx create mode 100644 assets/js/components/ChecksCatalog/CheckItem.test.jsx diff --git a/assets/js/components/ChecksCatalog/CatalogContainer.jsx b/assets/js/components/ChecksCatalog/CatalogContainer.jsx index 22734abdb6..9c7f72fc5b 100644 --- a/assets/js/components/ChecksCatalog/CatalogContainer.jsx +++ b/assets/js/components/ChecksCatalog/CatalogContainer.jsx @@ -1,10 +1,5 @@ import React from 'react'; -import { Disclosure, Transition } from '@headlessui/react'; - -import ReactMarkdown from 'react-markdown'; -import remarkGfm from 'remark-gfm'; - import NotificationBox from '@components/NotificationBox'; import LoadingBox from '@components/LoadingBox'; @@ -12,6 +7,8 @@ import { groupBy } from '@lib/lists'; import { EOS_ERROR } from 'eos-icons-react'; +import CheckItem from './CheckItem'; + const CatalogContainer = ({ onRefresh = () => {}, catalogData = [], @@ -59,58 +56,13 @@ const CatalogContainer = ({
    {checks.map((check) => ( -
  • - - -
    -
    -

    - {check.id} -

    - {check.premium > 0 && ( -

    - Premium -

    - )} -
    -
    -
    - - {check.description} - -
    -
    -
    -     - - -
    -
    - - {check.remediation} - -
    -
    -     -     -     -
    • + ))}
diff --git a/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx b/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx index 84d3308f41..39428f280b 100644 --- a/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx +++ b/assets/js/components/ChecksCatalog/CatalogContainer.test.jsx @@ -2,7 +2,6 @@ import React from 'react'; import { screen, within } from '@testing-library/react'; import '@testing-library/jest-dom'; -import userEvent from '@testing-library/user-event'; import { faker } from '@faker-js/faker'; import { renderWithRouter } from '@lib/test-utils'; @@ -55,38 +54,4 @@ describe('ChecksCatalog CatalogContainer component', () => { expect(checks.length).toBe(5); } }); - - it('should show check remediation when the row is clicked', () => { - const groupName = faker.animal.cat(); - const catalogChecks = catalogCheckFactory.buildList(2, { - group: groupName, - }); - const checkRemediation1 = catalogChecks[0].remediation; - const checkRemediation2 = catalogChecks[1].remediation; - const catalog = catalogChecks; - - renderWithRouter( - - ); - - const groups = screen.getAllByRole('list'); - const { getAllByRole } = within(groups[0]); - let checks = getAllByRole('listitem'); - const check1 = checks[0].querySelector('div'); - const check2 = checks[1].querySelector('div'); - - expect(screen.queryByText(checkRemediation1)).not.toBeInTheDocument(); - userEvent.click(check1); - expect(screen.getByText(checkRemediation1)).toBeVisible(); - userEvent.click(check1); - expect(screen.queryByText(checkRemediation1)).not.toBeInTheDocument(); - - expect(screen.queryByText(checkRemediation2)).not.toBeInTheDocument(); - userEvent.click(check2); - expect(screen.getByText(checkRemediation2)).toBeVisible(); - }); }); diff --git a/assets/js/components/ChecksCatalog/CheckItem.jsx b/assets/js/components/ChecksCatalog/CheckItem.jsx new file mode 100644 index 0000000000..0f274815c6 --- /dev/null +++ b/assets/js/components/ChecksCatalog/CheckItem.jsx @@ -0,0 +1,62 @@ +import React from 'react'; + +import { Disclosure, Transition } from '@headlessui/react'; + +import ReactMarkdown from 'react-markdown'; +import remarkGfm from 'remark-gfm'; + +const CheckItem = ({ checkID, premium = false, description, remediation }) => { + return ( +
  • + + +
    +
    +

    + {checkID} +

    + {premium > 0 && ( +

    + Premium +

    + )} +
    +
    +
    + + {description} + +
    +
    +
    +     + + +
    +
    + + {remediation} + +
    +
    +     +     +     +
    • + ); +}; + +export default CheckItem; diff --git a/assets/js/components/ChecksCatalog/CheckItem.test.jsx b/assets/js/components/ChecksCatalog/CheckItem.test.jsx new file mode 100644 index 0000000000..f13440178b --- /dev/null +++ b/assets/js/components/ChecksCatalog/CheckItem.test.jsx @@ -0,0 +1,66 @@ +import React from 'react'; + +import { screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import userEvent from '@testing-library/user-event'; + +import { renderWithRouter } from '@lib/test-utils'; +import { catalogCheckFactory } from '@lib/test-utils/factories'; + +import CheckItem from './CheckItem'; + +describe('ChecksCatalog CheckItem component', () => { + it('should show check information', () => { + const check = catalogCheckFactory.build(); + + renderWithRouter( + + ); + + expect(screen.getByText(check.id)).toBeVisible(); + expect(screen.getByText(check.description)).toBeVisible(); + }); + + it('should show premium badge if the check is premium', () => { + const check = catalogCheckFactory.build(); + + renderWithRouter( + + ); + + expect(screen.getByText('Premium')).toBeVisible(); + }); + + it('should show check remediation when the row is clicked', () => { + const check = catalogCheckFactory.build(); + + renderWithRouter( + + ); + + const checks = screen.getAllByRole('listitem'); + const checkDiv = checks[0].querySelector('div'); + + expect(screen.queryByText(check.remediation)).not.toBeInTheDocument(); + userEvent.click(checkDiv); + expect(screen.getByText(check.remediation)).toBeVisible(); + userEvent.click(checkDiv); + expect(screen.queryByText(check.remediation)).not.toBeInTheDocument(); + }); +}); diff --git a/assets/js/trento.jsx b/assets/js/trento.jsx index 7931067081..96a1588fff 100644 --- a/assets/js/trento.jsx +++ b/assets/js/trento.jsx @@ -23,8 +23,7 @@ import ClusterDetails from '@components/ClusterDetails'; import DatabasesOverview from '@components/DatabasesOverview'; import SapSystemDetails from './components/SapSystemDetails/SapSystemDetails'; import DatabaseDetails from './components/DatabaseDetails'; -import ChecksCatalog from '@components/ChecksCatalog'; -import { ChecksCatalogNew } from '@components/ChecksCatalog'; +import ChecksCatalog, { ChecksCatalogNew } from '@components/ChecksCatalog'; import NotFound from '@components/NotFound'; import SomethingWentWrong from '@components/SomethingWentWrong'; import Settings from '@components/Settings';