From ea880db04a1eb799f0e3fe05598fd9c6c864e052 Mon Sep 17 00:00:00 2001 From: Jose Balanza Martinez <62359443+braulio-balanza@users.noreply.github.com> Date: Mon, 27 Apr 2020 09:59:52 -0500 Subject: [PATCH] Use useInterval() hook to refetch audit status in lighthouse plugin WIP (#623) * Fix error when using yarn lint * Update logic for create app * Add eslint-react-hooks * Move common utils to utils.ts * Add AuditListRow that refetches the audit * Fixed Audit state using a react component * Change AuditListRow to AuditRow * Fixed some eslint-react-hooks warnings * Change AuditListRow to AuditRow * Merge * Fixes tests in AuditListTable.test.tsx * Refactored based on freben's review * Remove eslint-plugin-react-hooks --- plugins/lighthouse/package.json | 2 +- .../src/components/Audit/AuditRow.tsx | 86 +++++++++++++++++++ .../lighthouse/src/components/Audit/index.tsx | 54 ++++++++++++ .../AuditList/AuditListTable.test.tsx | 38 +++++--- .../components/AuditList/AuditListTable.tsx | 81 ++--------------- .../src/components/AuditList/index.tsx | 2 +- .../src/components/CreateAudit/index.tsx | 2 +- plugins/lighthouse/src/utils.ts | 38 +++++++- 8 files changed, 216 insertions(+), 87 deletions(-) create mode 100644 plugins/lighthouse/src/components/Audit/AuditRow.tsx create mode 100644 plugins/lighthouse/src/components/Audit/index.tsx diff --git a/plugins/lighthouse/package.json b/plugins/lighthouse/package.json index 6852d67754..01e3e5c81b 100644 --- a/plugins/lighthouse/package.json +++ b/plugins/lighthouse/package.json @@ -31,8 +31,8 @@ "@material-ui/lab": "4.0.0-alpha.45", "react": "^16.13.1", "react-dom": "^16.13.1", - "react-router-dom": "^5.1.2", "react-markdown": "^4.3.1", + "react-router-dom": "^5.1.2", "react-use": "^13.24.0" }, "files": [ diff --git a/plugins/lighthouse/src/components/Audit/AuditRow.tsx b/plugins/lighthouse/src/components/Audit/AuditRow.tsx new file mode 100644 index 0000000000..c42e6a8538 --- /dev/null +++ b/plugins/lighthouse/src/components/Audit/AuditRow.tsx @@ -0,0 +1,86 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { FC } from 'react'; +import { + Link, + TableCell, + TableRow, +} from '@material-ui/core'; +import { makeStyles } from '@material-ui/core/styles'; +import { TrendLine } from '@backstage/core'; +import { + Website, +} from '../../api'; +import { formatTime, CATEGORIES, CATEGORY_LABELS, SparklinesDataByCategory } from '../../utils'; +import AuditStatusIcon from '../AuditStatusIcon'; + +const useStyles = makeStyles(theme => ({ + table: { + minWidth: 650, + }, + status: { + textTransform: 'capitalize', + }, + link: { + paddingTop: theme.spacing(2), + paddingBottom: theme.spacing(2), + display: 'inline-block', + }, + statusCell: { whiteSpace: 'nowrap' }, + sparklinesCell: { minWidth: 120 }, +})); + +export const AuditRow: FC<{ + website: Website; + categorySparkline: SparklinesDataByCategory; +}> = ({ website, categorySparkline }) => { + const classes = useStyles(); + + return ( + + + + {website.url} + + + {CATEGORIES.map(category => ( + + + + ))} + + {' '} + + {website.lastAudit.status.toLowerCase()} + + + + {formatTime(website.lastAudit.timeCreated)} + + + ); +}; + +export default AuditRow; diff --git a/plugins/lighthouse/src/components/Audit/index.tsx b/plugins/lighthouse/src/components/Audit/index.tsx new file mode 100644 index 0000000000..349bdf5ee9 --- /dev/null +++ b/plugins/lighthouse/src/components/Audit/index.tsx @@ -0,0 +1,54 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { FC, useState } from 'react'; +import { useInterval } from 'react-use'; +import { Website, lighthouseApiRef } from '../../api'; +import { useApi } from '@backstage/core'; +import { + SparklinesDataByCategory, + buildSparklinesDataForItem, +} from '../../utils'; +import { AuditRow } from './AuditRow'; +export const LIMIT = 10; + +export const Audit: FC<{ + website: Website; + categorySparkline: SparklinesDataByCategory; +}> = ({ website, categorySparkline }) => { + const lighthouseApi = useApi(lighthouseApiRef); + const [websiteState, setWebsiteState] = useState(website); + const [sparklineState, setSparklineState] = useState(categorySparkline); + + const runRefresh = async () => { + const response = await lighthouseApi.getWebsiteForAuditId( + websiteState.lastAudit.id, + ); + const auditStatus = response.lastAudit.status; + if (auditStatus === 'COMPLETED' || auditStatus === 'FAILED') { + setSparklineState(buildSparklinesDataForItem(response)); + setWebsiteState(response); + } + }; + + useInterval( + runRefresh, + websiteState?.lastAudit.status === 'RUNNING' ? 5000 : null, + ); + + return ; +}; + +export default Audit; diff --git a/plugins/lighthouse/src/components/AuditList/AuditListTable.test.tsx b/plugins/lighthouse/src/components/AuditList/AuditListTable.test.tsx index 3b82b27645..3ab07434d7 100644 --- a/plugins/lighthouse/src/components/AuditList/AuditListTable.test.tsx +++ b/plugins/lighthouse/src/components/AuditList/AuditListTable.test.tsx @@ -17,19 +17,41 @@ import React from 'react'; import { render } from '@testing-library/react'; import { wrapInThemedTestApp } from '@backstage/test-utils'; +import { ApiRegistry, ApiProvider } from '@backstage/core'; import AuditListTable from './AuditListTable'; -import { WebsiteListResponse } from '../../api'; +import { + WebsiteListResponse, + lighthouseApiRef, + LighthouseRestApi, +} from '../../api'; import { formatTime } from '../../utils'; +import mockFetch from 'jest-fetch-mock'; import * as data from '../../__fixtures__/website-list-response.json'; const websiteListResponse = data as WebsiteListResponse; describe('AuditListTable', () => { + let apis: ApiRegistry; + + beforeEach(() => { + apis = ApiRegistry.from([ + [lighthouseApiRef, new LighthouseRestApi('http://lighthouse')], + ]); + mockFetch.mockResponse(JSON.stringify(websiteListResponse)); + }); + + const auditList = (websiteList: WebsiteListResponse) => { + return ( + + + + ); + }; it('renders the link to each website', () => { const rendered = render( - wrapInThemedTestApp(), + wrapInThemedTestApp(auditList(websiteListResponse)), ); const link = rendered.queryByText('https://anchor.fm'); const website = websiteListResponse.items.find( @@ -46,7 +68,7 @@ describe('AuditListTable', () => { it('renders the dates that are available for a given row', () => { const rendered = render( - wrapInThemedTestApp(), + wrapInThemedTestApp(auditList(websiteListResponse)), ); const website = websiteListResponse.items.find( w => w.url === 'https://anchor.fm', @@ -60,7 +82,7 @@ describe('AuditListTable', () => { it('renders the status for a given row', async () => { const rendered = render( - wrapInThemedTestApp(), + wrapInThemedTestApp(auditList(websiteListResponse)), ); const completed = await rendered.findAllByText('completed'); @@ -85,9 +107,7 @@ describe('AuditListTable', () => { describe('sparklines', () => { it('correctly maps the data from the website payload', () => { const rendered = render( - wrapInThemedTestApp( - , - ), + wrapInThemedTestApp(auditList(websiteListResponse)), ); const backstageSEO = rendered.getByTitle( 'trendline for SEO category of https://backstage.io', @@ -97,9 +117,7 @@ describe('AuditListTable', () => { it('does not break when no data is available', () => { const rendered = render( - wrapInThemedTestApp( - , - ), + wrapInThemedTestApp(auditList(websiteListResponse)), ); const anchorSEO = rendered.queryByTitle( 'trendline for SEO category of https://anchor.fm', diff --git a/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx b/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx index ba0e79f066..778eb71fb5 100644 --- a/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx +++ b/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx @@ -15,7 +15,6 @@ */ import React, { FC, useMemo } from 'react'; import { - Link, Table, TableBody, TableCell, @@ -24,31 +23,11 @@ import { TableRow, } from '@material-ui/core'; import { makeStyles } from '@material-ui/core/styles'; -import { TrendLine } from '@backstage/core'; - import { - Audit, - AuditCompleted, - LighthouseCategoryId, Website, } from '../../api'; -import { formatTime } from '../../utils'; -import AuditStatusIcon from '../AuditStatusIcon'; - -export const CATEGORIES: LighthouseCategoryId[] = [ - 'accessibility', - 'performance', - 'seo', - 'best-practices', -]; - -export const CATEGORY_LABELS: Record = { - accessibility: 'Accessibility', - performance: 'Performance', - seo: 'SEO', - 'best-practices': 'Best Practices', - pwa: 'Progressive Web App', -}; +import { CATEGORIES, CATEGORY_LABELS, SparklinesDataByCategory, buildSparklinesDataForItem } from '../../utils'; +import Audit from '../Audit' const useStyles = makeStyles(theme => ({ table: { @@ -66,34 +45,13 @@ const useStyles = makeStyles(theme => ({ sparklinesCell: { minWidth: 120 }, })); -type SparklinesDataByCategory = Record; -function buildSparklinesDataForItem(item: Website): SparklinesDataByCategory { - return item.audits - .filter( - (audit: Audit): audit is AuditCompleted => audit.status === 'COMPLETED', - ) - .reduce((scores, audit) => { - Object.values(audit.categories).forEach(category => { - scores[category.id] = scores[category.id] || []; - scores[category.id].unshift(category.score); - }); - - // edge case: if only one audit exists, force a "flat" sparkline - Object.values(scores).forEach(arr => { - if (arr.length === 1) arr.push(arr[0]); - }); - - return scores; - }, {} as SparklinesDataByCategory); -} - export const AuditListTable: FC<{ items: Website[] }> = ({ items }) => { const classes = useStyles(); const categorySparklines: Record = useMemo( () => items.reduce( (res, item) => ({ - ...res, + ...res, [item.url]: buildSparklinesDataForItem(item), }), {}, @@ -118,34 +76,11 @@ export const AuditListTable: FC<{ items: Website[] }> = ({ items }) => { {items.map(website => ( - - - - {website.url} - - - {CATEGORIES.map(category => ( - - - - ))} - - {' '} - - {website.lastAudit.status.toLowerCase()} - - - {formatTime(website.lastAudit.timeCreated)} - + ))} diff --git a/plugins/lighthouse/src/components/AuditList/index.tsx b/plugins/lighthouse/src/components/AuditList/index.tsx index 4dbbb05295..85be31cc80 100644 --- a/plugins/lighthouse/src/components/AuditList/index.tsx +++ b/plugins/lighthouse/src/components/AuditList/index.tsx @@ -63,7 +63,7 @@ const AuditList: FC<{}> = () => { if (value?.total && value?.limit) return Math.ceil(value?.total / value?.limit); return 0; - }, [value?.total, value?.limit]); + }, [value]); const history = useHistory(); diff --git a/plugins/lighthouse/src/components/CreateAudit/index.tsx b/plugins/lighthouse/src/components/CreateAudit/index.tsx index 7e74661319..8ff0ce5ea8 100644 --- a/plugins/lighthouse/src/components/CreateAudit/index.tsx +++ b/plugins/lighthouse/src/components/CreateAudit/index.tsx @@ -84,7 +84,7 @@ const CreateAudit: FC<{}> = () => { } finally { setSubmitting(false); } - }, [url, emulatedFormFactor, lighthouseApi, setSubmitting]); + }, [url, emulatedFormFactor, lighthouseApi, setSubmitting, errorApi, history]); return ( diff --git a/plugins/lighthouse/src/utils.ts b/plugins/lighthouse/src/utils.ts index 48cc67a88b..1546521083 100644 --- a/plugins/lighthouse/src/utils.ts +++ b/plugins/lighthouse/src/utils.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { useLocation } from 'react-router-dom'; - +import {Website, Audit, LighthouseCategoryId, AuditCompleted} from './api' export function useQuery(): URLSearchParams { return new URLSearchParams(useLocation().search); } @@ -28,3 +28,39 @@ export function formatTime(timestamp: string | Date) { } return date.toUTCString(); } + +export const CATEGORIES: LighthouseCategoryId[] = [ + 'accessibility', + 'performance', + 'seo', + 'best-practices', +]; + +export const CATEGORY_LABELS: Record = { + accessibility: 'Accessibility', + performance: 'Performance', + seo: 'SEO', + 'best-practices': 'Best Practices', + pwa: 'Progressive Web App', +}; + +export type SparklinesDataByCategory = Record; +export function buildSparklinesDataForItem(item: Website): SparklinesDataByCategory { + return item.audits + .filter( + (audit: Audit): audit is AuditCompleted => audit.status === 'COMPLETED', + ) + .reduce((scores, audit) => { + Object.values(audit.categories).forEach(category => { + scores[category.id] = scores[category.id] || []; + scores[category.id].unshift(category.score); + }); + + // edge case: if only one audit exists, force a "flat" sparkline + Object.values(scores).forEach(arr => { + if (arr.length === 1) arr.push(arr[0]); + }); + + return scores; + }, {} as SparklinesDataByCategory); +} \ No newline at end of file