From efb789a4fe239f61f3fb5d73aa89a0c8eba3199b Mon Sep 17 00:00:00 2001 From: Wesley Yee Date: Wed, 22 Apr 2020 14:14:56 -0400 Subject: [PATCH] Move error api to more generic alert api (#605) --- packages/app/src/App.tsx | 6 +- packages/app/src/apis.ts | 11 ++-- .../AlertDisplay.tsx} | 55 ++++++------------- .../{ErrorDisplay => AlertDisplay}/index.ts | 2 +- .../core/src/api/apis/definitions/alert.ts | 38 +++++++++++++ .../core/src/api/apis/definitions/index.ts | 1 + .../apis/implementations/AlertApiForwarder.ts | 35 ++++++++++++ .../apis/implementations/ErrorApiForwarder.ts | 45 +++++++++++++++ .../src/api/apis/implementations/index.ts | 22 ++++++++ packages/core/src/api/apis/index.ts | 1 + yarn.lock | 2 +- 11 files changed, 170 insertions(+), 48 deletions(-) rename packages/app/src/components/{ErrorDisplay/ErrorDisplay.tsx => AlertDisplay/AlertDisplay.tsx} (53%) rename packages/app/src/components/{ErrorDisplay => AlertDisplay}/index.ts (93%) create mode 100644 packages/core/src/api/apis/definitions/alert.ts create mode 100644 packages/core/src/api/apis/implementations/AlertApiForwarder.ts create mode 100644 packages/core/src/api/apis/implementations/ErrorApiForwarder.ts create mode 100644 packages/core/src/api/apis/implementations/index.ts diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index 3b0a8c894b..2988077467 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -25,9 +25,9 @@ import { createApp } from '@backstage/core'; import React, { FC } from 'react'; import { BrowserRouter as Router } from 'react-router-dom'; import Root from './components/Root'; -import ErrorDisplay from './components/ErrorDisplay'; +import AlertDisplay from './components/AlertDisplay'; import * as plugins from './plugins'; -import apis, { errorDialogForwarder } from './apis'; +import apis, { alertApiForwarder } from './apis'; import { ThemeContextType, ThemeContext, useThemeType } from './ThemeContext'; const useStyles = makeStyles(theme => ({ @@ -86,7 +86,7 @@ const App: FC<{}> = () => { - + diff --git a/packages/app/src/apis.ts b/packages/app/src/apis.ts index 88b75c1392..bfa049acfb 100644 --- a/packages/app/src/apis.ts +++ b/packages/app/src/apis.ts @@ -17,7 +17,10 @@ import { ApiHolder, ApiRegistry, + alertApiRef, errorApiRef, + AlertApiForwarder, + ErrorApiForwarder, featureFlagsApiRef, FeatureFlags, } from '@backstage/core'; @@ -26,13 +29,13 @@ import { LighthouseRestApi, } from '@backstage/plugin-lighthouse'; -import { ErrorDisplayForwarder } from './components/ErrorDisplay/ErrorDisplay'; - const builder = ApiRegistry.builder(); -export const errorDialogForwarder = new ErrorDisplayForwarder(); +export const alertApiForwarder = new AlertApiForwarder(); +builder.add(alertApiRef, alertApiForwarder); -builder.add(errorApiRef, errorDialogForwarder); +export const errorApiForwarder = new ErrorApiForwarder(alertApiForwarder); +builder.add(errorApiRef, errorApiForwarder); builder.add(featureFlagsApiRef, new FeatureFlags()); diff --git a/packages/app/src/components/ErrorDisplay/ErrorDisplay.tsx b/packages/app/src/components/AlertDisplay/AlertDisplay.tsx similarity index 53% rename from packages/app/src/components/ErrorDisplay/ErrorDisplay.tsx rename to packages/app/src/components/AlertDisplay/AlertDisplay.tsx index ef24821508..d9dd0b240b 100644 --- a/packages/app/src/components/ErrorDisplay/ErrorDisplay.tsx +++ b/packages/app/src/components/AlertDisplay/AlertDisplay.tsx @@ -19,59 +19,36 @@ import PropTypes from 'prop-types'; import { Snackbar, IconButton } from '@material-ui/core'; import CloseIcon from '@material-ui/icons/Close'; import { Alert } from '@material-ui/lab'; -import { ErrorApi, ErrorContext } from '@backstage/core'; - -type SubscriberFunc = (error: Error) => void; -type Unsubscribe = () => void; - -// TODO: figure out where to put implementations of APIs, both inside apps -// but also in core/separate package. -export class ErrorDisplayForwarder implements ErrorApi { - private readonly subscribers = new Set(); - - post(error: Error, context?: ErrorContext) { - if (context?.hidden) { - return; - } - - this.subscribers.forEach(subscriber => subscriber(error)); - } - - subscribe(func: SubscriberFunc): Unsubscribe { - this.subscribers.add(func); - - return () => { - this.subscribers.delete(func); - }; - } -} +import { AlertApiForwarder, AlertMessage } from '@backstage/core'; type Props = { - forwarder: ErrorDisplayForwarder; + forwarder: AlertApiForwarder; }; // TODO: improve on this and promote to a shared component for use by all apps. -const ErrorDisplay: FC = ({ forwarder }) => { - const [errors, setErrors] = useState>([]); +const AlertDisplay: FC = ({ forwarder }) => { + const [messages, setMessages] = useState>([]); useEffect(() => { - return forwarder.subscribe(error => setErrors(errs => errs.concat(error))); + return forwarder.subscribe((message: AlertMessage) => + setMessages(msgs => msgs.concat(message)), + ); }, [forwarder]); - if (errors.length === 0) { + if (messages.length === 0) { return null; } - const [firstError] = errors; + const [firstMessage] = messages; const handleClose = () => { - setErrors(errs => errs.filter(err => err !== firstError)); + setMessages(msgs => msgs.filter(msg => msg !== firstMessage)); }; return ( = ({ forwarder }) => { } - severity="error" + severity={firstMessage.severity} > - {firstError.toString()} + {firstMessage.message.toString()} ); }; -ErrorDisplay.propTypes = { - forwarder: PropTypes.instanceOf(ErrorDisplayForwarder).isRequired, +AlertDisplay.propTypes = { + forwarder: PropTypes.instanceOf(AlertApiForwarder).isRequired, }; -export default ErrorDisplay; +export default AlertDisplay; diff --git a/packages/app/src/components/ErrorDisplay/index.ts b/packages/app/src/components/AlertDisplay/index.ts similarity index 93% rename from packages/app/src/components/ErrorDisplay/index.ts rename to packages/app/src/components/AlertDisplay/index.ts index 05e414061b..b60bc1776b 100644 --- a/packages/app/src/components/ErrorDisplay/index.ts +++ b/packages/app/src/components/AlertDisplay/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { default } from './ErrorDisplay'; +export { default } from './AlertDisplay'; diff --git a/packages/core/src/api/apis/definitions/alert.ts b/packages/core/src/api/apis/definitions/alert.ts new file mode 100644 index 0000000000..f0d0bb3d72 --- /dev/null +++ b/packages/core/src/api/apis/definitions/alert.ts @@ -0,0 +1,38 @@ +/* + * 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 ApiRef from '../ApiRef'; + +export type AlertMessage = { + message: string; + // Severity will default to success since that is what material ui defaults the value to. + severity?: 'success' | 'info' | 'warning' | 'error'; +}; + +/** + * The alert API is used to report alerts to the app, and display them to the user. + */ + +export type AlertApi = { + /** + * Post an alert for handling by the application. + */ + post(alert: AlertMessage); +}; + +export const alertApiRef = new ApiRef({ + id: 'core.alert', + description: 'Used to report alerts and forward them to the app', +}); diff --git a/packages/core/src/api/apis/definitions/index.ts b/packages/core/src/api/apis/definitions/index.ts index 55923b50b7..ca1290403f 100644 --- a/packages/core/src/api/apis/definitions/index.ts +++ b/packages/core/src/api/apis/definitions/index.ts @@ -20,5 +20,6 @@ // // If you think some API definition is missing, please open an Issue or send a PR! +export * from './alert'; export * from './error'; export * from './featureFlags'; diff --git a/packages/core/src/api/apis/implementations/AlertApiForwarder.ts b/packages/core/src/api/apis/implementations/AlertApiForwarder.ts new file mode 100644 index 0000000000..349512c0f9 --- /dev/null +++ b/packages/core/src/api/apis/implementations/AlertApiForwarder.ts @@ -0,0 +1,35 @@ +/* + * 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 { AlertApi, AlertMessage } from '../../../'; + +type SubscriberFunc = (message: AlertMessage) => void; +type Unsubscribe = () => void; + +export class AlertApiForwarder implements AlertApi { + private readonly subscribers = new Set(); + + post(alert: AlertMessage) { + this.subscribers.forEach(subscriber => subscriber(alert)); + } + + subscribe(func: SubscriberFunc): Unsubscribe { + this.subscribers.add(func); + + return () => { + this.subscribers.delete(func); + }; + } +} diff --git a/packages/core/src/api/apis/implementations/ErrorApiForwarder.ts b/packages/core/src/api/apis/implementations/ErrorApiForwarder.ts new file mode 100644 index 0000000000..8d777ac18f --- /dev/null +++ b/packages/core/src/api/apis/implementations/ErrorApiForwarder.ts @@ -0,0 +1,45 @@ +/* + * 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 { ErrorApi, ErrorContext, AlertApi } from '../../../'; + +type SubscriberFunc = (error: Error) => void; +type Unsubscribe = () => void; + +export class ErrorApiForwarder implements ErrorApi { + private readonly subscribers = new Set(); + private alertApi: AlertApi; + + constructor(alertApi: AlertApi) { + this.alertApi = alertApi; + } + + post(error: Error, context?: ErrorContext) { + if (context?.hidden) { + return; + } + + this.alertApi.post({ message: error.message, severity: 'error' }); + this.subscribers.forEach(subscriber => subscriber(error)); + } + + subscribe(func: SubscriberFunc): Unsubscribe { + this.subscribers.add(func); + + return () => { + this.subscribers.delete(func); + }; + } +} diff --git a/packages/core/src/api/apis/implementations/index.ts b/packages/core/src/api/apis/implementations/index.ts new file mode 100644 index 0000000000..b5ea6467cb --- /dev/null +++ b/packages/core/src/api/apis/implementations/index.ts @@ -0,0 +1,22 @@ +/* + * 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. + */ + +// This folder contains implementations for all core APIs. +// +// Plugins should rely on these APIs for functionality as much as possible. + +export * from './AlertApiForwarder'; +export * from './ErrorApiForwarder'; diff --git a/packages/core/src/api/apis/index.ts b/packages/core/src/api/apis/index.ts index 9214414f5c..8dd982264d 100644 --- a/packages/core/src/api/apis/index.ts +++ b/packages/core/src/api/apis/index.ts @@ -20,3 +20,4 @@ export { default as ApiTestRegistry } from './ApiTestRegistry'; export { default as ApiRef } from './ApiRef'; export * from './types'; export * from './definitions'; +export * from './implementations'; diff --git a/yarn.lock b/yarn.lock index bd57bfaf12..4f35fa649b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17382,7 +17382,7 @@ request@^2.85.0, request@^2.87.0, request@^2.88.0: tunnel-agent "^0.6.0" uuid "^3.3.2" -request@cypress-io/request#b5af0d1fa47eec97ba980cde90a13e69a2afcd16: +"request@github:cypress-io/request#b5af0d1fa47eec97ba980cde90a13e69a2afcd16": version "2.88.1" resolved "https://codeload.github.com/cypress-io/request/tar.gz/b5af0d1fa47eec97ba980cde90a13e69a2afcd16" dependencies: