From 3bd33291b2525b245652496831e01cf24a7f14f2 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:33:35 -0700 Subject: [PATCH 01/14] Adding env variables and loading support for middleware plugins --- env.js | 14 +++++++++++++- index.js | 6 ++++++ plugins.js | 11 +++++++++++ src/config/types.ts | 1 + 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 plugins.js diff --git a/env.js b/env.js index d135ef065..89c73d772 100644 --- a/env.js +++ b/env.js @@ -15,6 +15,15 @@ const CORS_PROXY_PREFIX = process.env.CORS_PROXY_PREFIX || '/cors_proxy'; const CONFIG_DIR = process.env.CONFIG_DIR || './config'; const CONFIG_CACHE_TTL_SECONDS = process.env.CONFIG_CACHE_TTL_SECONDS || '60'; +// Defines a file to be required which will provide implementations for +// any user-definable code. +const PLUGINS_MODULE = process.env.PLUGINS_MODULE; + +// Optional URL which will provide information about system status. This is used +// to show informational banners to the user. +// If it has no protocol, it will be treated as relative to window.location.origin +const STATUS_URL = process.env.STATUS_URL; + module.exports = { ADMIN_API_URL, BASE_URL, @@ -23,10 +32,13 @@ module.exports = { CONFIG_DIR, CORS_PROXY_PREFIX, NODE_ENV, + PLUGINS_MODULE, + STATUS_URL, processEnv: { ADMIN_API_URL, BASE_URL, CORS_PROXY_PREFIX, - NODE_ENV + NODE_ENV, + STATUS_URL } }; diff --git a/index.js b/index.js index 7df78d5ff..23ebb6006 100644 --- a/index.js +++ b/index.js @@ -8,6 +8,7 @@ const fs = require('fs'); const morgan = require('morgan'); const express = require('express'); const env = require('./env'); +const { applyMiddleware } = require('./plugins'); const corsProxy = require('./corsProxy.js'); @@ -19,6 +20,11 @@ app.use(morgan('combined')); app.get(`${env.BASE_URL}/healthz`, (_req, res) => res.status(200).send()); app.use(corsProxy(`${env.BASE_URL}${env.CORS_PROXY_PREFIX}`)); +if (typeof applyMiddleware === 'function') { + console.log('Found middleware plugins, applying...'); + applyMiddleware(app); +} + if (process.env.NODE_ENV === 'production') { const path = require('path'); const expressStaticGzip = require('express-static-gzip'); diff --git a/plugins.js b/plugins.js new file mode 100644 index 000000000..789249752 --- /dev/null +++ b/plugins.js @@ -0,0 +1,11 @@ +const env = require('./env'); + +const { middleware } = env.PLUGINS_MODULE ? require(env.PLUGINS_MODULE) : {}; + +if (Array.isArray(middleware)) { + module.exports.applyMiddleware = app => middleware.forEach(m => m(app)); +} else if (middleware !== undefined) { + console.log( + `Expected middleware to be of type Array, but got ${middleware} instead` + ); +} diff --git a/src/config/types.ts b/src/config/types.ts index 78a2a81b0..d7762cffe 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -12,4 +12,5 @@ export interface Env extends NodeJS.ProcessEnv { BASE_URL?: string; CORS_PROXY_PREFIX: string; DISABLE_ANALYTICS?: string; + STATUS_URL?: string; } From 1b7061d32311fc13a051a022af5ffb3071910a45 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Thu, 12 Mar 2020 15:32:58 -0700 Subject: [PATCH 02/14] Adds json parsing middleware to ensure that custom POST route handlers can read their payloads --- index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/index.js b/index.js index 23ebb6006..44e34e25c 100644 --- a/index.js +++ b/index.js @@ -16,6 +16,7 @@ const app = express(); // Enable logging for HTTP access app.use(morgan('combined')); +app.use(express.json()); app.get(`${env.BASE_URL}/healthz`, (_req, res) => res.status(200).send()); app.use(corsProxy(`${env.BASE_URL}${env.CORS_PROXY_PREFIX}`)); From 61cb82cbaf0538233bf0ae5cd21c47fc8d9aa538 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Thu, 12 Mar 2020 15:50:09 -0700 Subject: [PATCH 03/14] Also ignoring docker file and dockerignore --- .dockerignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.dockerignore b/.dockerignore index 6043e867d..3b0113d6a 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,6 +1,8 @@ .git .env node_modules +Dockerfile +.dockerignore npm-debug.log .storybook .vscode From d15f877fa5dff8d142ef818cca37238402411553 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 1 Apr 2020 15:53:47 -0700 Subject: [PATCH 04/14] Adding plumbing for fetching system status --- .../Notifications/useSystemStatus.ts | 14 +++++++++ src/models/Common/api.ts | 31 ++++++++++++++++++- src/models/Common/constants.ts | 4 ++- src/models/Common/types.ts | 5 +++ 4 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 src/components/Notifications/useSystemStatus.ts diff --git a/src/components/Notifications/useSystemStatus.ts b/src/components/Notifications/useSystemStatus.ts new file mode 100644 index 000000000..647580eeb --- /dev/null +++ b/src/components/Notifications/useSystemStatus.ts @@ -0,0 +1,14 @@ +import { useAPIContext } from 'components/data/apiContext'; +import { useFetchableData } from 'components/hooks'; +import { defaultSystemStatus } from 'models/Common/constants'; + +/** Hook for fetching the current system status. Defaults to a safe value + * indicating normal system status. + */ +export function useSystemStatus() { + const { getSystemStatus } = useAPIContext(); + return useFetchableData({ + defaultValue: defaultSystemStatus, + doFetch: getSystemStatus + }); +} diff --git a/src/models/Common/api.ts b/src/models/Common/api.ts index ae879b54d..76fae69c5 100644 --- a/src/models/Common/api.ts +++ b/src/models/Common/api.ts @@ -9,16 +9,22 @@ import { RequestConfig } from 'models/AdminEntity'; +import { env } from 'common/env'; import { log } from 'common/log'; import { createCorsProxyURL } from 'common/utils'; import { transformRequestError } from 'models/AdminEntity/transformRequestError'; -import { defaultAxiosConfig, identifierPrefixes } from './constants'; +import { + defaultAxiosConfig, + defaultSystemStatus, + identifierPrefixes +} from './constants'; import { IdentifierScope, LiteralMap, NamedEntity, NamedEntityIdentifier, ResourceType, + SystemStatus, UserProfile } from './types'; import { makeIdentifierPath, makeNamedEntityPath } from './utils'; @@ -127,6 +133,29 @@ export const getUserProfile = async () => { } }; +/** If env.STATUS_URL is set, will issue a fetch to retrieve the current system + * status. If not, will resolve immediately with a default value indicating + * normal system status. + */ +export const getSystemStatus = async () => { + if (!env.STATUS_URL) { + return defaultSystemStatus; + } + const path = env.STATUS_URL; + + try { + const { data } = await axios.get( + path, + defaultAxiosConfig + ); + return data; + } catch (e) { + const { message } = transformRequestError(e, path); + log.error(`Failed to fetch system status: ${message}`); + return null; + } +}; + /** Given a url to a `LiteralMap` stored at a remote location, will fetch and * decode the resulting object. */ diff --git a/src/models/Common/constants.ts b/src/models/Common/constants.ts index 9b0522712..f91825ebe 100644 --- a/src/models/Common/constants.ts +++ b/src/models/Common/constants.ts @@ -2,7 +2,7 @@ import axios, { AxiosRequestConfig, AxiosTransformer } from 'axios'; import * as camelcaseKeys from 'camelcase-keys'; import * as snakecaseKeys from 'snakecase-keys'; import { isObject } from 'util'; -import { LiteralMapBlob, ResourceType } from './types'; +import { LiteralMapBlob, ResourceType, SystemStatus } from './types'; export const endpointPrefixes = { execution: '/executions', @@ -44,3 +44,5 @@ export const defaultAxiosConfig: AxiosRequestConfig = { ], withCredentials: true }; + +export const defaultSystemStatus: SystemStatus = { status: 'normal' }; diff --git a/src/models/Common/types.ts b/src/models/Common/types.ts index bde98d504..3b2581765 100644 --- a/src/models/Common/types.ts +++ b/src/models/Common/types.ts @@ -176,3 +176,8 @@ export interface UserProfile { email: string; picture: string; } + +export interface SystemStatus { + message?: string; + status: 'normal' | 'degraded' | 'down'; +} From 71b4fa3ee2df042d4b587a514bb7c56ab0ad4de9 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Fri, 3 Apr 2020 11:00:29 -0700 Subject: [PATCH 05/14] Adding basic unstyled banner and stories --- src/components/App/App.tsx | 2 + .../Notifications/SystemStatusBanner.tsx | 59 +++++++++++++++++++ .../SystemStatusBanner.stories.tsx | 40 +++++++++++++ .../Notifications/useSystemStatus.ts | 5 +- src/models/Common/api.ts | 3 +- 5 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 src/components/Notifications/SystemStatusBanner.tsx create mode 100644 src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx diff --git a/src/components/App/App.tsx b/src/components/App/App.tsx index fb58111ca..1b5af2f2d 100644 --- a/src/components/App/App.tsx +++ b/src/components/App/App.tsx @@ -4,6 +4,7 @@ import { env } from 'common/env'; import { debug, debugPrefix } from 'common/log'; import { APIContext, useAPIState } from 'components/data/apiContext'; import { LoginExpiredHandler } from 'components/Errors/LoginExpiredHandler'; +import { SystemStatusBanner } from 'components/Notifications/SystemStatusBanner'; import { skeletonColor, skeletonHighlightColor } from 'components/Theme'; import { muiTheme } from 'components/Theme/muiTheme'; import * as React from 'react'; @@ -41,6 +42,7 @@ export const AppComponent: React.StatelessComponent<{}> = () => { + diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx new file mode 100644 index 000000000..5f8049f6d --- /dev/null +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -0,0 +1,59 @@ +import Paper from '@material-ui/core/Paper'; +import { makeStyles, Theme } from '@material-ui/core/styles'; +import { WaitForData } from 'components/common'; +import { SystemStatus } from 'models/Common/types'; +import * as React from 'react'; +import { useSystemStatus } from './useSystemStatus'; + +const useStyles = makeStyles((theme: Theme) => ({ + statusPaper: { + bottom: theme.spacing(3), + margin: theme.spacing(3), + position: 'absolute' + }, + statusContentContainer: { + display: 'flex' + }, + statusClose: { + flex: '0 0 auto' + }, + statusIcon: { + flex: '0 0 auto' + }, + statusMessage: { + flex: '1 1 auto' + } +})); + +const RenderSystemStatusBanner: React.FC<{ + status: SystemStatus; + onClose: () => void; +}> = ({ status, onClose }) => { + const styles = useStyles(); + return ( + +
+
{status.message}
+
+ + ); +}; + +export const SystemStatusBanner: React.FC<{}> = () => { + const systemStatus = useSystemStatus(); + const [dismissed, setDismissed] = React.useState(false); + const onClose = () => setDismissed(true); + if (dismissed) { + return null; + } + return ( + + {systemStatus.value.message && ( + + )} + + ); +}; diff --git a/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx b/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx new file mode 100644 index 000000000..b30d5e1cc --- /dev/null +++ b/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx @@ -0,0 +1,40 @@ +import { storiesOf } from '@storybook/react'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext } from 'components/data/apiContext'; +import { SystemStatus } from 'models'; +import * as React from 'react'; +import { SystemStatusBanner } from '../SystemStatusBanner'; + +const normalStatus: SystemStatus = { + status: 'normal', + message: + 'This is a test. It is only a test. Check out https://flyte.org for more information.' +}; + +const degradedStatus: SystemStatus = { + status: 'degraded', + message: + 'Something is a bit wrong with the system. We can probably still do some things, but may be slow about it.' +}; + +const downStatus: SystemStatus = { + status: 'down', + message: + 'We are down. You should probably go get a cup of coffee while the administrators attempt to troubleshoot.' +}; + +function renderBanner(status: SystemStatus) { + const mockApi = mockAPIContextValue({ + getSystemStatus: () => Promise.resolve(status) + }); + return ( + + + + ); +} + +const stories = storiesOf('Notifications/SystemStatusBanner', module); +stories.add('Normal Status', () => renderBanner(normalStatus)); +stories.add('Degraded Status', () => renderBanner(degradedStatus)); +stories.add('Down Status', () => renderBanner(downStatus)); diff --git a/src/components/Notifications/useSystemStatus.ts b/src/components/Notifications/useSystemStatus.ts index 647580eeb..267b6a22e 100644 --- a/src/components/Notifications/useSystemStatus.ts +++ b/src/components/Notifications/useSystemStatus.ts @@ -1,11 +1,12 @@ import { useAPIContext } from 'components/data/apiContext'; -import { useFetchableData } from 'components/hooks'; +import { FetchableData, useFetchableData } from 'components/hooks'; +import { SystemStatus } from 'models'; import { defaultSystemStatus } from 'models/Common/constants'; /** Hook for fetching the current system status. Defaults to a safe value * indicating normal system status. */ -export function useSystemStatus() { +export function useSystemStatus(): FetchableData { const { getSystemStatus } = useAPIContext(); return useFetchableData({ defaultValue: defaultSystemStatus, diff --git a/src/models/Common/api.ts b/src/models/Common/api.ts index 76fae69c5..48a464361 100644 --- a/src/models/Common/api.ts +++ b/src/models/Common/api.ts @@ -151,8 +151,7 @@ export const getSystemStatus = async () => { return data; } catch (e) { const { message } = transformRequestError(e, path); - log.error(`Failed to fetch system status: ${message}`); - return null; + throw new Error(`Failed to fetch system status: ${message}`); } }; From e96500c4dfd40ba3920cf6913592ffb41ca1418e Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Fri, 3 Apr 2020 13:19:23 -0700 Subject: [PATCH 06/14] Filling out styling for notification banner --- .../Notifications/SystemStatusBanner.tsx | 97 ++++++++++++++++--- .../SystemStatusBanner.stories.tsx | 8 ++ src/components/Theme/constants.ts | 5 + src/models/Common/types.ts | 4 +- 4 files changed, 101 insertions(+), 13 deletions(-) diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index 5f8049f6d..f46647620 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -1,39 +1,112 @@ +import { ButtonBase, SvgIcon, Typography } from '@material-ui/core'; import Paper from '@material-ui/core/Paper'; import { makeStyles, Theme } from '@material-ui/core/styles'; +import Close from '@material-ui/icons/Close'; +import Info from '@material-ui/icons/Info'; +import Warning from '@material-ui/icons/Warning'; import { WaitForData } from 'components/common'; -import { SystemStatus } from 'models/Common/types'; +import { + infoIconColor, + mutedButtonColor, + mutedButtonHoverColor, + warningIconColor +} from 'components/Theme'; +import { StatusString, SystemStatus } from 'models/Common/types'; import * as React from 'react'; import { useSystemStatus } from './useSystemStatus'; const useStyles = makeStyles((theme: Theme) => ({ + closeButton: { + alignItems: 'center', + color: mutedButtonColor, + '&:hover': { + color: mutedButtonHoverColor + }, + display: 'flex', + height: theme.spacing(3) + }, statusPaper: { - bottom: theme.spacing(3), - margin: theme.spacing(3), - position: 'absolute' + bottom: theme.spacing(2), + display: 'flex', + left: '50%', + padding: theme.spacing(2), + position: 'fixed', + transform: 'translateX(-50%)' }, statusContentContainer: { - display: 'flex' + alignItems: 'flex-start', + display: 'flex', + maxWidth: theme.spacing(131) }, statusClose: { + alignItems: 'flex-start', + display: 'flex', flex: '0 0 auto' }, statusIcon: { - flex: '0 0 auto' + alignItems: 'center', + display: 'flex', + flex: '0 0 auto', + lineHeight: `${theme.spacing(3)}px` }, statusMessage: { - flex: '1 1 auto' + flex: '1 1 auto', + fontWeight: 'normal', + lineHeight: `${theme.spacing(3)}px`, + marginLeft: theme.spacing(2), + marginRight: theme.spacing(2) } })); +interface StatusConstantValues { + color: string; + IconComponent: typeof SvgIcon; +} + +const statusConstants: Record = { + normal: { + color: infoIconColor, + IconComponent: Info + }, + degraded: { + color: warningIconColor, + IconComponent: Warning + }, + down: { + color: warningIconColor, + IconComponent: Warning + } +}; + +const StatusIcon: React.FC<{ status: StatusString }> = ({ status }) => { + const { color, IconComponent } = + statusConstants[status] || statusConstants.normal; + return ; +}; + const RenderSystemStatusBanner: React.FC<{ - status: SystemStatus; + systemStatus: SystemStatus; onClose: () => void; -}> = ({ status, onClose }) => { +}> = ({ systemStatus: { message, status }, onClose }) => { const styles = useStyles(); return ( - + +
+ +
-
{status.message}
+ + {message} + +
+
+ + +
); @@ -50,7 +123,7 @@ export const SystemStatusBanner: React.FC<{}> = () => { {systemStatus.value.message && ( )} diff --git a/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx b/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx index b30d5e1cc..21c29056d 100644 --- a/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx +++ b/src/components/Notifications/__stories__/SystemStatusBanner.stories.tsx @@ -11,6 +11,13 @@ const normalStatus: SystemStatus = { 'This is a test. It is only a test. Check out https://flyte.org for more information.' }; +// Max length for a status should be 500 characters +const longTextStatus: SystemStatus = { + status: 'normal', + message: + 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. Nullam dictum felis eu pede mollis pretium. Integer tincidunt. Cras dapibu' +}; + const degradedStatus: SystemStatus = { status: 'degraded', message: @@ -38,3 +45,4 @@ const stories = storiesOf('Notifications/SystemStatusBanner', module); stories.add('Normal Status', () => renderBanner(normalStatus)); stories.add('Degraded Status', () => renderBanner(degradedStatus)); stories.add('Down Status', () => renderBanner(downStatus)); +stories.add('Long Message', () => renderBanner(longTextStatus)); diff --git a/src/components/Theme/constants.ts b/src/components/Theme/constants.ts index c37630f43..0af47716c 100644 --- a/src/components/Theme/constants.ts +++ b/src/components/Theme/constants.ts @@ -38,9 +38,14 @@ export const nestedListColor = COLOR_SPECTRUM.gray0.color; export const buttonHoverColor = COLOR_SPECTRUM.gray0.color; export const inputFocusBorderColor = COLOR_SPECTRUM.blue60.color; +export const warningIconColor = COLOR_SPECTRUM.sunset60.color; +export const infoIconColor = COLOR_SPECTRUM.blue40.color; + export const dangerousButtonBorderColor = COLOR_SPECTRUM.red20.color; export const dangerousButtonColor = COLOR_SPECTRUM.red30.color; export const dangerousButtonHoverColor = COLOR_SPECTRUM.red40.color; +export const mutedButtonColor = COLOR_SPECTRUM.gray30.color; +export const mutedButtonHoverColor = COLOR_SPECTRUM.gray60.color; export const errorBackgroundColor = '#FBFBFC'; diff --git a/src/models/Common/types.ts b/src/models/Common/types.ts index 3b2581765..45b2ba67a 100644 --- a/src/models/Common/types.ts +++ b/src/models/Common/types.ts @@ -177,7 +177,9 @@ export interface UserProfile { picture: string; } +export type StatusString = 'normal' | 'degraded' | 'down'; + export interface SystemStatus { message?: string; - status: 'normal' | 'degraded' | 'down'; + status: StatusString; } From afd3bcb5dcee2f57b19e2671f6d520a8c45f1865 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Fri, 3 Apr 2020 13:58:38 -0700 Subject: [PATCH 07/14] docs --- src/components/Notifications/SystemStatusBanner.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index f46647620..ea76a40a7 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -112,6 +112,10 @@ const RenderSystemStatusBanner: React.FC<{ ); }; +/** Fetches and renders the system status returned by issuing a GET to + * `env.STATUS_URL`. If the status includes a message, a dismissable toast + * will be rendered. Otherwise, nothing will be rendered. + */ export const SystemStatusBanner: React.FC<{}> = () => { const systemStatus = useSystemStatus(); const [dismissed, setDismissed] = React.useState(false); From 30b31d9ab81728c7a70c8ff41ec50355d87a0c94 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Fri, 3 Apr 2020 14:39:45 -0700 Subject: [PATCH 08/14] return null if status message does not exist --- src/components/Notifications/SystemStatusBanner.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index ea76a40a7..458142b52 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -125,12 +125,12 @@ export const SystemStatusBanner: React.FC<{}> = () => { } return ( - {systemStatus.value.message && ( + {systemStatus.value.message ? ( - )} + ) : null} ); }; From fa06213b0e8c87288c43b996d87f24d15c54f240 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 6 Apr 2020 13:30:08 -0700 Subject: [PATCH 09/14] Unit tests for status banner --- .../Notifications/SystemStatusBanner.tsx | 37 +++++---- .../test/SystemStatusBanner.test.tsx | 81 +++++++++++++++++++ 2 files changed, 101 insertions(+), 17 deletions(-) create mode 100644 src/components/Notifications/test/SystemStatusBanner.test.tsx diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index 458142b52..8d621a6ff 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -63,25 +63,23 @@ interface StatusConstantValues { IconComponent: typeof SvgIcon; } -const statusConstants: Record = { - normal: { - color: infoIconColor, - IconComponent: Info - }, - degraded: { - color: warningIconColor, - IconComponent: Warning - }, - down: { - color: warningIconColor, - IconComponent: Warning - } +const InfoIcon = () => ( + +); + +const WarningIcon = () => ( + +); + +const statusIcons: Record = { + normal: InfoIcon, + degraded: WarningIcon, + down: WarningIcon }; const StatusIcon: React.FC<{ status: StatusString }> = ({ status }) => { - const { color, IconComponent } = - statusConstants[status] || statusConstants.normal; - return ; + const IconComponent = statusIcons[status] || statusIcons.normal; + return ; }; const RenderSystemStatusBanner: React.FC<{ @@ -90,7 +88,12 @@ const RenderSystemStatusBanner: React.FC<{ }> = ({ systemStatus: { message, status }, onClose }) => { const styles = useStyles(); return ( - +
diff --git a/src/components/Notifications/test/SystemStatusBanner.test.tsx b/src/components/Notifications/test/SystemStatusBanner.test.tsx new file mode 100644 index 000000000..28f76bc75 --- /dev/null +++ b/src/components/Notifications/test/SystemStatusBanner.test.tsx @@ -0,0 +1,81 @@ +import { fireEvent, render, wait } from '@testing-library/react'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext, APIContextValue } from 'components/data/apiContext'; +import { StatusString, SystemStatus } from 'models'; +import * as React from 'react'; +import { SystemStatusBanner } from '../SystemStatusBanner'; + +describe('SystemStatusBanner', () => { + let systemStatus: SystemStatus; + let apiContext: APIContextValue; + + beforeEach(() => { + systemStatus = { + status: 'normal', + message: 'Everything is fine.' + }; + apiContext = mockAPIContextValue({ + getSystemStatus: jest + .fn() + .mockImplementation(() => Promise.resolve(systemStatus)) + }); + }); + + const renderStatusBanner = () => + render( + + + + ); + + it('should display an info icon for normal status', async () => { + const { getByTestId } = renderStatusBanner(); + await wait(); + + expect(getByTestId('info-icon')).toBeInTheDocument(); + }); + + it('should display a waring icon for degraded status', async () => { + systemStatus.status = 'degraded'; + const { getByTestId } = renderStatusBanner(); + await wait(); + + expect(getByTestId('warning-icon')).toBeInTheDocument(); + }); + + it('should display a warning icon for down status', async () => { + systemStatus.status = 'down'; + const { getByTestId } = renderStatusBanner(); + await wait(); + + expect(getByTestId('warning-icon')).toBeInTheDocument(); + }); + + it('should render normal status icon for any unknown status', async () => { + systemStatus.status = 'unknown' as StatusString; + const { getByTestId } = renderStatusBanner(); + await wait(); + + expect(getByTestId('info-icon')).toBeInTheDocument(); + }); + + it('should only display if a `message` property is present', async () => { + delete systemStatus.message; + const { queryByRole } = renderStatusBanner(); + await wait(); + expect(queryByRole('banner')).toBeNull(); + }); + + it('should hide when dismissed by user', async () => { + const { getByRole, queryByRole } = renderStatusBanner(); + await wait(); + + expect(getByRole('banner')).toBeInTheDocument(); + + const closeButton = getByRole('button'); + fireEvent.click(closeButton); + await wait(); + + expect(queryByRole('banner')).toBeNull(); + }); +}); From 01095dc52dc506cbc3c94a73825c55d337b59897 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 6 Apr 2020 15:57:11 -0700 Subject: [PATCH 10/14] Adding linkification and cleaning up some layout issues --- package.json | 2 + src/common/linkify.ts | 41 ++++++++++++ src/common/test/linkify.test.ts | 40 +++++++++++ .../Notifications/SystemStatusBanner.tsx | 66 +++++++++++-------- .../test/SystemStatusBanner.test.tsx | 10 +++ src/components/common/LinkifiedText.tsx | 22 +++++++ src/components/common/useLinkifiedChunks.ts | 6 ++ yarn.lock | 17 +++++ 8 files changed, 175 insertions(+), 29 deletions(-) create mode 100644 src/common/linkify.ts create mode 100644 src/common/test/linkify.test.ts create mode 100644 src/components/common/LinkifiedText.tsx create mode 100644 src/components/common/useLinkifiedChunks.ts diff --git a/package.json b/package.json index 9947556a1..0d4319ffa 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "@types/html-webpack-plugin": "^2.28.0", "@types/jest": "^25.1.2", "@types/js-yaml": "^3.10.1", + "@types/linkify-it": "^2.1.0", "@types/lodash": "^4.14.68", "@types/long": "^3.0.32", "@types/lossless-json": "^1.0.0", @@ -143,6 +144,7 @@ "identity-obj-proxy": "^3.0.0", "intersection-observer": "^0.7.0", "jest": "^24.9.0", + "linkify-it": "^2.2.0", "lint-staged": "^7.0.4", "lossless-json": "^1.0.3", "memoize-one": "^5.0.0", diff --git a/src/common/linkify.ts b/src/common/linkify.ts new file mode 100644 index 000000000..bbabf6de8 --- /dev/null +++ b/src/common/linkify.ts @@ -0,0 +1,41 @@ +import * as LinkifyIt from 'linkify-it'; +export const linkify = new LinkifyIt(); + +export type LinkifiedTextChunkType = 'text' | 'link'; +export interface LinkifiedTextChunk { + type: LinkifiedTextChunkType; + text: string; + url?: string; +} + +export function getLinkifiedTextChunks(text: string): LinkifiedTextChunk[] { + const matches = linkify.match(text); + if (matches === null) { + return [{ text, type: 'text' }]; + } + + const chunks: LinkifiedTextChunk[] = []; + let lastMatchEndIndex = 0; + matches.forEach(match => { + if (lastMatchEndIndex !== match.index) { + chunks.push({ + text: text.substring(lastMatchEndIndex, match.index), + type: 'text' + }); + } + chunks.push({ + text: match.text, + type: 'link', + url: match.url + }); + lastMatchEndIndex = match.lastIndex; + }); + if (lastMatchEndIndex !== text.length) { + chunks.push({ + text: text.substring(lastMatchEndIndex, text.length), + type: 'text' + }); + } + + return chunks; +} diff --git a/src/common/test/linkify.test.ts b/src/common/test/linkify.test.ts new file mode 100644 index 000000000..4154c82f2 --- /dev/null +++ b/src/common/test/linkify.test.ts @@ -0,0 +1,40 @@ +import { getLinkifiedTextChunks, LinkifiedTextChunk } from 'common/linkify'; + +function text(text: string): LinkifiedTextChunk { + return { text, type: 'text' }; +} + +function link(text: string): LinkifiedTextChunk { + return { text, url: text, type: 'link' }; +} + +describe('linkify/getLinkifiedTextChunks', () => { + const testCases: [string, LinkifiedTextChunk[]][] = [ + ['No match expected', [text('No match expected')]], + [ + 'Points to http://example.com', + [text('Points to '), link('http://example.com')] + ], + [ + 'https://example.com link is at beginning', + [link('https://example.com'), text(' link is at beginning')] + ], + [ + 'A link to http://example.com is in the middle', + [ + text('A link to '), + link('http://example.com'), + text(' is in the middle') + ] + ], + [ + 'A link at the end to http://example.com', + [text('A link at the end to '), link('http://example.com')] + ] + ]; + testCases.forEach(([input, expected]) => + it(`correctly splits: ${input}`, () => { + expect(getLinkifiedTextChunks(input)).toEqual(expected); + }) + ); +}); diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index 8d621a6ff..a282bf8c4 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -5,6 +5,7 @@ import Close from '@material-ui/icons/Close'; import Info from '@material-ui/icons/Info'; import Warning from '@material-ui/icons/Warning'; import { WaitForData } from 'components/common'; +import { LinkifiedText } from 'components/common/LinkifiedText'; import { infoIconColor, mutedButtonColor, @@ -16,6 +17,15 @@ import * as React from 'react'; import { useSystemStatus } from './useSystemStatus'; const useStyles = makeStyles((theme: Theme) => ({ + container: { + bottom: 0, + display: 'flex', + justifyContent: 'center', + left: 0, + position: 'fixed', + padding: theme.spacing(2), + right: 0 + }, closeButton: { alignItems: 'center', color: mutedButtonColor, @@ -26,12 +36,8 @@ const useStyles = makeStyles((theme: Theme) => ({ height: theme.spacing(3) }, statusPaper: { - bottom: theme.spacing(2), display: 'flex', - left: '50%', - padding: theme.spacing(2), - position: 'fixed', - transform: 'translateX(-50%)' + padding: theme.spacing(2) }, statusContentContainer: { alignItems: 'flex-start', @@ -88,30 +94,32 @@ const RenderSystemStatusBanner: React.FC<{ }> = ({ systemStatus: { message, status }, onClose }) => { const styles = useStyles(); return ( - -
- -
-
- - {message} - -
-
- - - -
- +
+ +
+ +
+
+ + + +
+
+ + + +
+ +
); }; diff --git a/src/components/Notifications/test/SystemStatusBanner.test.tsx b/src/components/Notifications/test/SystemStatusBanner.test.tsx index 28f76bc75..4a88ee431 100644 --- a/src/components/Notifications/test/SystemStatusBanner.test.tsx +++ b/src/components/Notifications/test/SystemStatusBanner.test.tsx @@ -78,4 +78,14 @@ describe('SystemStatusBanner', () => { expect(queryByRole('banner')).toBeNull(); }); + + it('should render inline urls as links', async () => { + systemStatus.message = 'Check out http://flyte.org for more info'; + const { getByText } = renderStatusBanner(); + await wait(); + expect(getByText('http://flyte.org').closest('a')).toHaveAttribute( + 'href', + 'http://flyte.org' + ); + }); }); diff --git a/src/components/common/LinkifiedText.tsx b/src/components/common/LinkifiedText.tsx new file mode 100644 index 000000000..ff872415a --- /dev/null +++ b/src/components/common/LinkifiedText.tsx @@ -0,0 +1,22 @@ +import * as React from 'react'; +import { NewTargetLink } from './NewTargetLink'; +import { useLinkifiedChunks } from './useLinkifiedChunks'; + +export const LinkifiedText: React.FC<{ text?: string }> = ({ text = '' }) => { + const chunks = useLinkifiedChunks(text); + return ( + <> + {chunks.map((chunk, idx) => { + const key = `${chunk.type}-${idx}`; + if (chunk.type === 'link') { + return ( + + {chunk.text} + + ); + } + return {chunk.text}; + })} + + ); +}; diff --git a/src/components/common/useLinkifiedChunks.ts b/src/components/common/useLinkifiedChunks.ts new file mode 100644 index 000000000..3fa39eccf --- /dev/null +++ b/src/components/common/useLinkifiedChunks.ts @@ -0,0 +1,6 @@ +import { getLinkifiedTextChunks } from 'common/linkify'; +import { useMemo } from 'react'; + +export function useLinkifiedChunks(text: string) { + return useMemo(() => getLinkifiedTextChunks(text), [text]); +} diff --git a/yarn.lock b/yarn.lock index 2d2b165ea..548b8e14f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2831,6 +2831,11 @@ resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.4.tgz#38fd73ddfd9b55abb1e1b2ed578cb55bd7b7d339" integrity sha512-8+KAKzEvSUdeo+kmqnKrqgeE+LcA0tjYWFY7RPProVYwnqDjukzO+3b6dLD56rYX5TdWejnEOLJYOIeh4CXKuA== +"@types/linkify-it@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@types/linkify-it/-/linkify-it-2.1.0.tgz#ea3dd64c4805597311790b61e872cbd1ed2cd806" + integrity sha512-Q7DYAOi9O/+cLLhdaSvKdaumWyHbm7HAk/bFwwyTuU0arR5yyCeW5GOoqt4tJTpDRxhpx9Q8kQL6vMpuw9hDSw== + "@types/lodash@^4.14.68": version "4.14.149" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.149.tgz#1342d63d948c6062838fbf961012f74d4e638440" @@ -8998,6 +9003,13 @@ lines-and-columns@^1.1.6: resolved "https://registry.yarnpkg.com/lines-and-columns/-/lines-and-columns-1.1.6.tgz#1c00c743b433cd0a4e80758f7b64a57440d9ff00" integrity sha1-HADHQ7QzzQpOgHWPe2SldEDZ/wA= +linkify-it@^2.2.0: + version "2.2.0" + resolved "https://registry.yarnpkg.com/linkify-it/-/linkify-it-2.2.0.tgz#e3b54697e78bf915c70a38acd78fd09e0058b1cf" + integrity sha512-GnAl/knGn+i1U/wjBz3akz2stz+HrHLsxMwHQGofCDfPvlf+gDKN58UtfmUquTY4/MXeE2x7k19KQmeoZi94Iw== + dependencies: + uc.micro "^1.0.1" + lint-staged@^7.0.4: version "7.3.0" resolved "https://registry.yarnpkg.com/lint-staged/-/lint-staged-7.3.0.tgz#90ff33e5ca61ed3dbac35b6f6502dbefdc0db58d" @@ -13420,6 +13432,11 @@ ua-parser-js@^0.7.18: resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.21.tgz#853cf9ce93f642f67174273cc34565ae6f308777" integrity sha512-+O8/qh/Qj8CgC6eYBVBykMrNtp5Gebn4dlGD/kKXVkJNDwyrAwSIqwz8CDf+tsAIWVycKcku6gIXJ0qwx/ZXaQ== +uc.micro@^1.0.1: + version "1.0.6" + resolved "https://registry.yarnpkg.com/uc.micro/-/uc.micro-1.0.6.tgz#9c411a802a409a91fc6cf74081baba34b24499ac" + integrity sha512-8Y75pvTYkLJW2hWQHXxoqRgV7qb9B+9vFEtidML+7koHUFapnVJAZ6cKs+Qjz5Aw3aZWHMC6u0wJE3At+nSGwA== + uglify-es@^3.3.4: version "3.3.9" resolved "https://registry.yarnpkg.com/uglify-es/-/uglify-es-3.3.9.tgz#0c1c4f0700bed8dbc124cdb304d2592ca203e677" From 6c04006782deed8845252bcce242a41308ea0110 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 6 Apr 2020 16:06:29 -0700 Subject: [PATCH 11/14] docs --- src/common/linkify.ts | 4 ++++ src/components/common/LinkifiedText.tsx | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/common/linkify.ts b/src/common/linkify.ts index bbabf6de8..4ae30a92a 100644 --- a/src/common/linkify.ts +++ b/src/common/linkify.ts @@ -8,6 +8,10 @@ export interface LinkifiedTextChunk { url?: string; } +/** Detects any links in the given text and splits the string on the + * link boundaries, returning an array of text/link entries for each + * portion of the string. + */ export function getLinkifiedTextChunks(text: string): LinkifiedTextChunk[] { const matches = linkify.match(text); if (matches === null) { diff --git a/src/components/common/LinkifiedText.tsx b/src/components/common/LinkifiedText.tsx index ff872415a..71b546001 100644 --- a/src/components/common/LinkifiedText.tsx +++ b/src/components/common/LinkifiedText.tsx @@ -2,6 +2,9 @@ import * as React from 'react'; import { NewTargetLink } from './NewTargetLink'; import { useLinkifiedChunks } from './useLinkifiedChunks'; +/** Renders the given text with any inline valid URLs rendered as + * `NewTargetLink`s. + */ export const LinkifiedText: React.FC<{ text?: string }> = ({ text = '' }) => { const chunks = useLinkifiedChunks(text); return ( From 2ff9f33fb17e28fd493646e186002561982dd93c Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 6 Apr 2020 16:08:24 -0700 Subject: [PATCH 12/14] Adding additional test case for linkify --- src/common/test/linkify.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/common/test/linkify.test.ts b/src/common/test/linkify.test.ts index 4154c82f2..ee1f734bb 100644 --- a/src/common/test/linkify.test.ts +++ b/src/common/test/linkify.test.ts @@ -30,6 +30,16 @@ describe('linkify/getLinkifiedTextChunks', () => { [ 'A link at the end to http://example.com', [text('A link at the end to '), link('http://example.com')] + ], + [ + 'A link to http://example.com and another link to https://flyte.org in the middle.', + [ + text('A link to '), + link('http://example.com'), + text(' and another link to '), + link('https://flyte.org'), + text(' in the middle.') + ] ] ]; testCases.forEach(([input, expected]) => From 7398f866b894df4ff0ba3d5a61b63ebd3cb54659 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 6 Apr 2020 16:12:07 -0700 Subject: [PATCH 13/14] Prevent parent container from capturing clicks --- src/components/Notifications/SystemStatusBanner.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index a282bf8c4..a67390fcf 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -22,6 +22,7 @@ const useStyles = makeStyles((theme: Theme) => ({ display: 'flex', justifyContent: 'center', left: 0, + pointerEvents: 'none', position: 'fixed', padding: theme.spacing(2), right: 0 @@ -37,7 +38,8 @@ const useStyles = makeStyles((theme: Theme) => ({ }, statusPaper: { display: 'flex', - padding: theme.spacing(2) + padding: theme.spacing(2), + pointerEvents: 'initial' }, statusContentContainer: { alignItems: 'flex-start', From d26259a4b020ea5aead0744ea1651bf6d276047f Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Mon, 6 Apr 2020 16:30:21 -0700 Subject: [PATCH 14/14] PR feedback and an extra comment --- src/components/Notifications/SystemStatusBanner.tsx | 2 ++ src/components/Notifications/test/SystemStatusBanner.test.tsx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Notifications/SystemStatusBanner.tsx b/src/components/Notifications/SystemStatusBanner.tsx index a67390fcf..0f1f2e9a8 100644 --- a/src/components/Notifications/SystemStatusBanner.tsx +++ b/src/components/Notifications/SystemStatusBanner.tsx @@ -22,6 +22,8 @@ const useStyles = makeStyles((theme: Theme) => ({ display: 'flex', justifyContent: 'center', left: 0, + // The parent container extends the full width of the page. + // We don't want it intercepting pointer events for visible items below it. pointerEvents: 'none', position: 'fixed', padding: theme.spacing(2), diff --git a/src/components/Notifications/test/SystemStatusBanner.test.tsx b/src/components/Notifications/test/SystemStatusBanner.test.tsx index 4a88ee431..9d08db007 100644 --- a/src/components/Notifications/test/SystemStatusBanner.test.tsx +++ b/src/components/Notifications/test/SystemStatusBanner.test.tsx @@ -35,7 +35,7 @@ describe('SystemStatusBanner', () => { expect(getByTestId('info-icon')).toBeInTheDocument(); }); - it('should display a waring icon for degraded status', async () => { + it('should display a warning icon for degraded status', async () => { systemStatus.status = 'degraded'; const { getByTestId } = renderStatusBanner(); await wait();