From 0bdd149d939bfed93fae33c591e5722401c5b751 Mon Sep 17 00:00:00 2001 From: Tommaso Scarlatti Date: Fri, 22 Jul 2022 12:53:59 +0200 Subject: [PATCH 1/6] refactor: add i18n in clusters list page --- frontend/locales/en/strings.json | 38 ++++++++++++++ frontend/src/old-pages/Clusters/Clusters.tsx | 54 ++++++++++---------- 2 files changed, 66 insertions(+), 26 deletions(-) diff --git a/frontend/locales/en/strings.json b/frontend/locales/en/strings.json index e66fae35..d63e3085 100644 --- a/frontend/locales/en/strings.json +++ b/frontend/locales/en/strings.json @@ -4,6 +4,44 @@ "home": "Home" } }, + "cluster": { + "list": { + "filtering": { + "empty": { + "title": "No clusters", + "subtitle": "No clusters to display", + "action": "Create Cluster" + }, + "noMatch": { + "title": "No matches", + "subtitle": "No clusters match the filters", + "action": "Create filter" + } + }, + "cols": { + "name": "Name", + "status": "Status", + "version": "Version" + }, + "loadingText": "Loading clusters...", + "countText": "Results:", + "filteringAriaLabel": "Filter cluster", + "splitPanel": { + "preferencesTitle": "Split panel preferences", + "preferencesPositionLabel": "Split panel position", + "preferencesPositionDescription": "Choose the default split panel position for the service.", + "preferencesPositionSide": "Side", + "preferencesPositionBottom": "Bottom", + "preferencesConfirm": "Confirm", + "preferencesCancel": "Cancel", + "closeButtonAriaLabel": "Close panel", + "openButtonAriaLabel": "Open panel", + "resizeHandleAriaLabel": "Resize split panel", + "noClusterSelectedText": "No cluster selected", + "selectClusterText": "Select a cluster to see its details" + } + } + }, "wizard": { "source" : { "validation": { diff --git a/frontend/src/old-pages/Clusters/Clusters.tsx b/frontend/src/old-pages/Clusters/Clusters.tsx index d6d8d016..8c6b0517 100644 --- a/frontend/src/old-pages/Clusters/Clusters.tsx +++ b/frontend/src/old-pages/Clusters/Clusters.tsx @@ -15,6 +15,7 @@ import { ListClusters } from '../../model' import { useState, clearState, getState, setState, isAdmin } from '../../store' import { selectCluster } from './util' import { findFirst } from '../../util' +import { useTranslation } from 'react-i18next'; // UI Elements import { @@ -52,7 +53,6 @@ function updateClusterList(navigate: NavigateFunction) { if((oldStatus === 'CREATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'CREATE_COMPLETE') || (oldStatus === 'UPDATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'UPDATE_COMPLETE')) - // @ts-expect-error TS(2554) FIXME: Expected 2 arguments, but got 1. selectCluster(selectedClusterName); // If the selected cluster is not found, and was in DELETE_IN_PROGRESS status, deselect it } else if (oldStatus === 'DELETE_IN_PROGRESS') { @@ -68,6 +68,8 @@ function ClusterList() { const selectedClusterName = useState(['app', 'clusters', 'selected']); let navigate = useNavigate(); let params = useParams(); + const { t } = useTranslation(); + React.useEffect(() => { const timerId = (setInterval(() => updateClusterList(navigate), 5000)); @@ -89,16 +91,16 @@ function ClusterList() { filtering: { empty: ( Create Cluster} + title={t("cluster.list.filtering.empty.title")} + subtitle={t("cluster.list.filtering.empty.subtitle")} + action={} /> ), noMatch: ( actions.setFiltering('')}>Clear filter} + title={t("cluster.list.filtering.noMatch.title")} + subtitle={t("cluster.list.filtering.noMatch.subtitle")} + action={} /> ), }, @@ -115,19 +117,19 @@ function ClusterList() { } trackBy="clusterName" columnDefinitions={[ { id: "name", - header: "Name", + header: t("cluster.list.cols.name"), cell: item => (item as any).clusterName, sortingField: "clusterName" }, { id: "status", - header: "Status", + header: t("cluster.list.cols.status"), cell: item => || "-", sortingField: "clusterStatus" }, { id: "version", - header: "Version", + header: t("cluster.list.cols.version"), cell: item => (item as any).version || "-" } @@ -135,11 +137,11 @@ function ClusterList() { loading={clusters === null} items={items} selectionType="single" - loadingText="Loading clusters..." + loadingText={t("cluster.list.loadingText")} pagination={} filter={} + countText={`${t("cluster.list.countText")} ${filteredItemsCount}`} + filteringAriaLabel={t("cluster.list.filteringAriaLabel")}/>} selectedItems={(items || []).filter((c) => (c as any).clusterName === selectedClusterName)} // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. onSelectionChange={({ detail }) => { navigate(`/clusters/${detail.selectedItems[0].clusterName}`); }} @@ -153,6 +155,7 @@ export default function Clusters () { const clusters = useState(['clusters', 'list']); let navigate = useNavigate(); const [ splitOpen, setSplitOpen ] = React.useState(true); + const { t } = useTranslation(); const configure = () => { wizardShow(navigate); @@ -176,17 +179,16 @@ export default function Clusters () { }> - {clusterName ? `Cluster: ${clusterName}` : "No cluster selected" } + {clusterName ? `Cluster: ${clusterName}` : t("cluster.list.splitPanel.noClusterSelectedText") } }> - {clusterName ?
:
Select a cluster to see its details.
} + {clusterName ?
:
{t("cluster.list.splitPanel.selectClusterText")}
} } content={ From 1a76d51b5e9827c596afc0b5b67facde9ee910e6 Mon Sep 17 00:00:00 2001 From: Tommaso Scarlatti Date: Fri, 22 Jul 2022 16:05:00 +0200 Subject: [PATCH 2/6] refactor: leverage useQuery in ListClusters() --- frontend/src/model.tsx | 21 ++++--- frontend/src/old-pages/Clusters/Clusters.tsx | 63 ++++++++++---------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/frontend/src/model.tsx b/frontend/src/model.tsx index 3c652f12..f5a519cb 100644 --- a/frontend/src/model.tsx +++ b/frontend/src/model.tsx @@ -163,19 +163,18 @@ function DeleteCluster(clusterName: any, callback=null) { }) } -function ListClusters(callback: any) { +async function ListClusters() { var url = 'api?path=/v3/clusters'; - request('get', url).then((response: any) => { - //console.log("List Success", response) - if(response.status === 200) { - callback && callback(response.data.clusters); - setState(['clusters', 'list'], response.data.clusters); + try { + const { data } = await request('get', url); + setState(['clusters', 'list'], data?.clusters); + return data?.clusters || []; + } catch (error) { + if((error as any).response) { + notify(`Error: ${(error as any).response.data.message}`, 'error'); } - }).catch((error: any) => { - if(error.response) - notify(`Error: ${error.response.data.message}`, 'error'); - console.log(error) - }); + throw error; + } } function GetConfiguration(clusterName: any, callback=null) { diff --git a/frontend/src/old-pages/Clusters/Clusters.tsx b/frontend/src/old-pages/Clusters/Clusters.tsx index 8c6b0517..2e885465 100644 --- a/frontend/src/old-pages/Clusters/Clusters.tsx +++ b/frontend/src/old-pages/Clusters/Clusters.tsx @@ -8,16 +8,13 @@ // OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and // limitations under the License. import React from 'react'; -import { NavigateFunction, useNavigate, useParams } from "react-router-dom" - +import { useNavigate, useParams } from "react-router-dom" import { ListClusters } from '../../model' - -import { useState, clearState, getState, setState, isAdmin } from '../../store' +import { useState, getState, clearState, setState, isAdmin } from '../../store' import { selectCluster } from './util' import { findFirst } from '../../util' import { useTranslation } from 'react-i18next'; - -// UI Elements +import { useQuery } from 'react-query'; import { AppLayout, Button, @@ -28,28 +25,36 @@ import { Table, TextFilter } from "@awsui/components-react"; - import { useCollection } from '@awsui/collection-hooks'; -// Components import EmptyState from '../../components/EmptyState'; import Status from "../../components/Status"; import Actions from './Actions'; import Details from "./Details"; import { wizardShow } from '../Configure/Configure'; -function updateClusterList(navigate: NavigateFunction) { + +interface Cluster { + cloudformationStackArn: string, + cloudformationStackStatus: string, + clusterName: string, + clusterStatus: string, + region: string, + version: string +} + +async function updateClusterList(navigate) { const selectedClusterName = getState(['app', 'clusters', 'selected']); const oldStatus = getState(['app', 'clusters', 'selectedStatus']); - ListClusters((clusterList: any) => { - if(selectedClusterName) - { - const selectedCluster = findFirst(clusterList, (c: any) => c.clusterName === selectedClusterName); - if(selectedCluster) - { - if(oldStatus !== selectedCluster.clusterStatus) + try { + const clusterList = await ListClusters(); + if(selectedClusterName) { + const selectedCluster = findFirst(clusterList, (c: Cluster) => c.clusterName === selectedClusterName); + if(selectedCluster) { + if(oldStatus !== selectedCluster.clusterStatus) { setState(['app', 'clusters', 'selectedStatus'], selectedCluster.clusterStatus); + } if((oldStatus === 'CREATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'CREATE_COMPLETE') || (oldStatus === 'UPDATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'UPDATE_COMPLETE')) @@ -60,17 +65,19 @@ function updateClusterList(navigate: NavigateFunction) { navigate('/clusters'); } } - }) + } catch (error) {} +} + +interface ClusterListProps { + clusters: Cluster[] } -function ClusterList() { - let clusters = useState(['clusters', 'list']); +function ClusterList({ clusters }: ClusterListProps) { const selectedClusterName = useState(['app', 'clusters', 'selected']); let navigate = useNavigate(); let params = useParams(); const { t } = useTranslation(); - React.useEffect(() => { const timerId = (setInterval(() => updateClusterList(navigate), 5000)); return () => { clearInterval(timerId); } @@ -143,8 +150,7 @@ function ClusterList() { countText={`${t("cluster.list.countText")} ${filteredItemsCount}`} filteringAriaLabel={t("cluster.list.filteringAriaLabel")}/>} selectedItems={(items || []).filter((c) => (c as any).clusterName === selectedClusterName)} - // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. - onSelectionChange={({ detail }) => { navigate(`/clusters/${detail.selectedItems[0].clusterName}`); }} + onSelectionChange={({ detail }) => { navigate(`/clusters/${(detail.selectedItems[0] as Cluster).clusterName}`); }} /> ) } @@ -152,20 +158,15 @@ function ClusterList() { export default function Clusters () { const clusterName = useState(['app', 'clusters', 'selected']); const cluster = useState(['clusters', 'index', clusterName]); - const clusters = useState(['clusters', 'list']); let navigate = useNavigate(); const [ splitOpen, setSplitOpen ] = React.useState(true); const { t } = useTranslation(); + const { data } = useQuery('LIST_CLUSTERS', () => ListClusters()); const configure = () => { wizardShow(navigate); } - React.useEffect(() => { - // @ts-expect-error TS(2554) FIXME: Expected 1 arguments, but got 0. - ListClusters(); - }, []) - return ( }> + actions={cluster && }> {clusterName ? `Cluster: ${clusterName}` : t("cluster.list.splitPanel.noClusterSelectedText") } }> {clusterName ?
:
{t("cluster.list.splitPanel.selectClusterText")}
} } - content={ + content={ } /> ); From 6d10626e849e24a2a2d842452532e27bc83dc962 Mon Sep 17 00:00:00 2001 From: Tommaso Scarlatti Date: Tue, 26 Jul 2022 15:29:21 +0200 Subject: [PATCH 3/6] test: add @testing-library/user-event --- frontend/package-lock.json | 21 +++++++++++++++++++++ frontend/package.json | 1 + 2 files changed, 22 insertions(+) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 77a79dc1..3ead8f46 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -37,6 +37,7 @@ "devDependencies": { "@awsui/jest-preset": "^2.0.5", "@testing-library/react": "^12.1.5", + "@testing-library/user-event": "^14.3.0", "@types/jest": "^28.1.4", "@types/lodash": "^4.14.182", "@types/node": "^18.0.0", @@ -3952,6 +3953,19 @@ "react-dom": "<18.0.0" } }, + "node_modules/@testing-library/user-event": { + "version": "14.3.0", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-14.3.0.tgz", + "integrity": "sha512-P02xtBBa8yMaLhK8CzJCIns8rqwnF6FxhR9zs810flHOBXUYCFjLd8Io1rQrAkQRWEmW2PGdZIEdMxf/KLsqFA==", + "dev": true, + "engines": { + "node": ">=12", + "npm": ">=6" + }, + "peerDependencies": { + "@testing-library/dom": ">=7.21.4" + } + }, "node_modules/@tootallnate/once": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-2.0.0.tgz", @@ -17344,6 +17358,13 @@ "@types/react-dom": "<18.0.0" } }, + "@testing-library/user-event": { + "version": "14.3.0", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-14.3.0.tgz", + "integrity": "sha512-P02xtBBa8yMaLhK8CzJCIns8rqwnF6FxhR9zs810flHOBXUYCFjLd8Io1rQrAkQRWEmW2PGdZIEdMxf/KLsqFA==", + "dev": true, + "requires": {} + }, "@tootallnate/once": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-2.0.0.tgz", diff --git a/frontend/package.json b/frontend/package.json index fdabd35b..03cec09c 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -55,6 +55,7 @@ "devDependencies": { "@awsui/jest-preset": "^2.0.5", "@testing-library/react": "^12.1.5", + "@testing-library/user-event": "^14.3.0", "@types/jest": "^28.1.4", "@types/lodash": "^4.14.182", "@types/node": "^18.0.0", From 823b03f26914c3c23f1bf380c49c510a21895220 Mon Sep 17 00:00:00 2001 From: Tommaso Scarlatti Date: Tue, 26 Jul 2022 15:30:48 +0200 Subject: [PATCH 4/6] refactor: remove @ts-expect-errors above ListClusters() usages --- frontend/src/model.tsx | 1 - frontend/src/old-pages/Configure/Create.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/frontend/src/model.tsx b/frontend/src/model.tsx index f5a519cb..3f97b04e 100644 --- a/frontend/src/model.tsx +++ b/frontend/src/model.tsx @@ -802,7 +802,6 @@ async function LoadInitialState() { if(groups && (groups.includes("admin") || groups.includes("user"))) { ListUsers(); - // @ts-expect-error TS(2554) FIXME: Expected 1 arguments, but got 0. ListClusters(); // @ts-expect-error TS(2554) FIXME: Expected 3 arguments, but got 0. ListCustomImages(); diff --git a/frontend/src/old-pages/Configure/Create.tsx b/frontend/src/old-pages/Configure/Create.tsx index 60254ba3..d0050992 100644 --- a/frontend/src/old-pages/Configure/Create.tsx +++ b/frontend/src/old-pages/Configure/Create.tsx @@ -57,7 +57,6 @@ function handleCreate(handleClose: any, navigate: any) { // @ts-expect-error TS(2554) FIXME: Expected 2 arguments, but got 1. DescribeCluster(clusterName) setState(['app', 'clusters', 'selected'], clusterName); - // @ts-expect-error TS(2554) FIXME: Expected 1 arguments, but got 0. ListClusters(); handleClose(); navigate(href); From a995c4a7681db63a7138f773f1687dc9bc7fa41a Mon Sep 17 00:00:00 2001 From: Tommaso Scarlatti Date: Tue, 26 Jul 2022 15:31:19 +0200 Subject: [PATCH 5/6] test: add unit tests for list clusters page --- frontend/src/old-pages/Clusters/Clusters.tsx | 9 +- .../Clusters/__tests__/Clusters.test.tsx | 121 ++++++++++++++++++ 2 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx diff --git a/frontend/src/old-pages/Clusters/Clusters.tsx b/frontend/src/old-pages/Clusters/Clusters.tsx index 2e885465..cb816c24 100644 --- a/frontend/src/old-pages/Clusters/Clusters.tsx +++ b/frontend/src/old-pages/Clusters/Clusters.tsx @@ -34,7 +34,7 @@ import Details from "./Details"; import { wizardShow } from '../Configure/Configure'; -interface Cluster { +export interface Cluster { cloudformationStackArn: string, cloudformationStackStatus: string, clusterName: string, @@ -86,7 +86,7 @@ function ClusterList({ clusters }: ClusterListProps) { React.useEffect(() => { if(params.clusterName && selectedClusterName !== params.clusterName) selectCluster(params.clusterName, navigate); - }, [selectedClusterName, params]) + }, [selectedClusterName, params, navigate]) const configure = () => { wizardShow(navigate); @@ -158,15 +158,10 @@ function ClusterList({ clusters }: ClusterListProps) { export default function Clusters () { const clusterName = useState(['app', 'clusters', 'selected']); const cluster = useState(['clusters', 'index', clusterName]); - let navigate = useNavigate(); const [ splitOpen, setSplitOpen ] = React.useState(true); const { t } = useTranslation(); const { data } = useQuery('LIST_CLUSTERS', () => ListClusters()); - const configure = () => { - wizardShow(navigate); - } - return ( ( + + + + + + + {props.children} + + + + + + +) + +jest.mock('../../../model', () => ({ + ListClusters: jest.fn(), +})); + +jest.mock('../../../store', () => ({ + ...jest.requireActual('../../../store') as any, + isAdmin: () => true, +})); + +const mockedUseNavigate = jest.fn(); + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom') as any, + useNavigate: () => mockedUseNavigate, +})); + +describe('given a component to show the clusters list', () => { + + describe('when the clusters list is available', () => { + beforeEach(() => { + (ListClusters as jest.Mock).mockResolvedValue(mockClusters); + mockedUseNavigate.mockReset(); + }); + + it('should render the clusters', async () => { + const { getByText } = await waitFor(() => render( + + + + )) + + expect(getByText('test-cluster')).toBeTruthy() + expect(getByText('CREATE COMPLETE')).toBeTruthy() + expect(getByText('3.1.4')).toBeTruthy() + }) + + describe('when the user selects a cluster', () => { + it('should populate the split panel', async () => { + const output = await waitFor(() => render( + + + + )) + + await userEvent.click(output.getByRole('radio')) + expect(mockedUseNavigate).toHaveBeenCalledWith('/clusters/test-cluster') + }) + }) + + describe('when the user clicks on "Create Cluster" button', () => { + it('should redirect to configure', async () => { + const output = await waitFor(() => render( + + + + )) + + //await userEvent.click(output.getByRole('button', {name: /Create Cluster/i})) + await userEvent.click(output.getByText('Create Cluster')) + expect(mockedUseNavigate).toHaveBeenCalledWith('/configure') + }) + }) + }) + + describe('when there are no clusters available', () => { + beforeEach(() => { + (ListClusters as jest.Mock).mockResolvedValue([]); + }); + + it('should show the empty state', async () => { + const { getByText } = await waitFor(() => render( + + + + )) + + expect(getByText('No clusters to display')).toBeTruthy() + }) + }) +}) \ No newline at end of file From 7060fb8132440203135a6b10186ba8dd65c5488c Mon Sep 17 00:00:00 2001 From: Tommaso Scarlatti Date: Wed, 27 Jul 2022 11:55:35 +0200 Subject: [PATCH 6/6] refactor: add useCallback in clusters onSelectionChange and remove unnecessary comments --- frontend/src/old-pages/Clusters/Clusters.tsx | 23 +++++++++++-------- .../Clusters/__tests__/Clusters.test.tsx | 1 - 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/frontend/src/old-pages/Clusters/Clusters.tsx b/frontend/src/old-pages/Clusters/Clusters.tsx index cb816c24..75991173 100644 --- a/frontend/src/old-pages/Clusters/Clusters.tsx +++ b/frontend/src/old-pages/Clusters/Clusters.tsx @@ -8,7 +8,7 @@ // OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and // limitations under the License. import React from 'react'; -import { useNavigate, useParams } from "react-router-dom" +import { NavigateFunction, useNavigate, useParams } from "react-router-dom" import { ListClusters } from '../../model' import { useState, getState, clearState, setState, isAdmin } from '../../store' import { selectCluster } from './util' @@ -43,7 +43,7 @@ export interface Cluster { version: string } -async function updateClusterList(navigate) { +async function updateClusterList(navigate: NavigateFunction) { const selectedClusterName = getState(['app', 'clusters', 'selected']); const oldStatus = getState(['app', 'clusters', 'selectedStatus']); @@ -55,20 +55,18 @@ async function updateClusterList(navigate) { if(oldStatus !== selectedCluster.clusterStatus) { setState(['app', 'clusters', 'selectedStatus'], selectedCluster.clusterStatus); } - - if((oldStatus === 'CREATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'CREATE_COMPLETE') || - (oldStatus === 'UPDATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'UPDATE_COMPLETE')) - selectCluster(selectedClusterName); - // If the selected cluster is not found, and was in DELETE_IN_PROGRESS status, deselect it - } else if (oldStatus === 'DELETE_IN_PROGRESS') { + if((oldStatus === 'CREATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'CREATE_COMPLETE') || (oldStatus === 'UPDATE_IN_PROGRESS' && selectedCluster.clusterStatus === 'UPDATE_COMPLETE')) { + selectCluster(selectedClusterName, null); + } else if (oldStatus === 'DELETE_IN_PROGRESS') { clearState(['app', 'clusters', 'selected']); navigate('/clusters'); + } } } } catch (error) {} } -interface ClusterListProps { +type ClusterListProps = { clusters: Cluster[] } @@ -88,6 +86,11 @@ function ClusterList({ clusters }: ClusterListProps) { selectCluster(params.clusterName, navigate); }, [selectedClusterName, params, navigate]) + const onSelectionChangeCallback = React.useCallback( + ({ detail }) => { + navigate(`/clusters/${(detail.selectedItems[0] as Cluster).clusterName}`); + }, [navigate]); + const configure = () => { wizardShow(navigate); } @@ -150,7 +153,7 @@ function ClusterList({ clusters }: ClusterListProps) { countText={`${t("cluster.list.countText")} ${filteredItemsCount}`} filteringAriaLabel={t("cluster.list.filteringAriaLabel")}/>} selectedItems={(items || []).filter((c) => (c as any).clusterName === selectedClusterName)} - onSelectionChange={({ detail }) => { navigate(`/clusters/${(detail.selectedItems[0] as Cluster).clusterName}`); }} + onSelectionChange={onSelectionChangeCallback} /> ) } diff --git a/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx b/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx index 1cc09e6d..97c30e3f 100644 --- a/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx +++ b/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx @@ -96,7 +96,6 @@ describe('given a component to show the clusters list', () => { )) - //await userEvent.click(output.getByRole('button', {name: /Create Cluster/i})) await userEvent.click(output.getByText('Create Cluster')) expect(mockedUseNavigate).toHaveBeenCalledWith('/configure') })