diff --git a/__tests__/fixtures/version-2/broken.json b/__tests__/fixtures/version-2/broken.json new file mode 100644 index 0000000000000000000000000000000000000000..2b84515e2736c7d43339eee33678bb8c9d53e650 --- /dev/null +++ b/__tests__/fixtures/version-2/broken.json @@ -0,0 +1,6 @@ +<html> + <title>Error</title> + <body> + <p>Something went wrong</p> + </body> +</html> diff --git a/__tests__/integration/mirador/invalid-api-response.test.js b/__tests__/integration/mirador/invalid-api-response.test.js index 8bc544ad8f837b71bc26b8c2a889fcfb1776b998..4cec574815a8ae86f09e7b1fa10c88c012c8b812 100644 --- a/__tests__/integration/mirador/invalid-api-response.test.js +++ b/__tests__/integration/mirador/invalid-api-response.test.js @@ -1,5 +1,5 @@ describe('Mirador Invalid API Response Handler Test', () => { - beforeAll(async () => { + beforeEach(async () => { await page.goto('http://127.0.0.1:4488/__tests__/integration/mirador/'); }); it('breaks Mirador', async () => { @@ -14,4 +14,25 @@ describe('Mirador Invalid API Response Handler Test', () => { expect(e.name).toMatch('TimeoutError'); } }); + + it('renders an error message when a manifest cannot be loaded (and allows it to be dismissed)', async () => { + await expect(page).toClick('#addBtn'); + await expect(page).toFill('#manifestURL', 'http://localhost:5000/api/broken'); + await expect(page).toClick('#fetchBtn'); + + await page.waitFor(1000); + + await expect(page).toMatchElement( + 'p', { text: 'The resource cannot be added:' }, + ); + await expect(page).toMatchElement( + 'p', { text: 'http://localhost:5000/api/broken' }, + ); + + await expect(page).toClick('button', { text: 'Dismiss' }); + await expect(page).not.toMatchElement( + 'p', + { text: 'The resource http://localhost:5000/api/broken cannot be added.' }, + ); + }); }); diff --git a/__tests__/src/components/ManifestListItem.test.js b/__tests__/src/components/ManifestListItem.test.js index b6084286d9edeef8100f8609366faa124823f58b..f3c85f267e08c931a273e443c99c6b0e819cb188 100644 --- a/__tests__/src/components/ManifestListItem.test.js +++ b/__tests__/src/components/ManifestListItem.test.js @@ -1,6 +1,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import ManifestListItem from '../../../src/components/ManifestListItem'; +import ManifestListItemError from '../../../src/containers/ManifestListItemError'; /** */ function createWrapper(props) { @@ -34,7 +35,7 @@ describe('ManifestListItem', () => { const wrapper = createWrapper({ error: 'This is an error message' }); expect(wrapper.find('WithStyles(Paper)').length).toBe(1); - expect(wrapper.find('WithStyles(Paper)').children().text()).toEqual('This is an error message'); + expect(wrapper.find(ManifestListItemError).length).toBe(1); }); it('updates and adds window when button clicked', () => { const addWindow = jest.fn(); diff --git a/__tests__/src/components/ManifestListItemError.test.js b/__tests__/src/components/ManifestListItemError.test.js new file mode 100644 index 0000000000000000000000000000000000000000..9606b3de0e3ed051f716e3d8c7d736edfd2dd416 --- /dev/null +++ b/__tests__/src/components/ManifestListItemError.test.js @@ -0,0 +1,54 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import Typography from '@material-ui/core/Typography'; +import ManifestListItemError from '../../../src/components/ManifestListItemError'; + +/** + * Helper function to wrap creating a ManifestListItemError component +*/ +function createWrapper(props) { + return shallow( + <ManifestListItemError + manifestId="http://example.com" + onDismissClick={() => {}} + onTryAgainClick={() => {}} + t={key => key} + {...props} + />, + ).dive(); // unwrap HOC created by withStyles() +} + +describe('ManifestListItemError', () => { + let wrapper; + let mockFn; + + it('renders the failed manifest url and error key', () => { + wrapper = createWrapper(); + + expect( + wrapper.find(Typography).children().first().text(), + ).toEqual('manifestError'); // the i18n key + + expect( + wrapper.find(Typography).children().last().text(), + ).toEqual('http://example.com'); + }); + + it('has a dismiss button that fires the onDismissClick prop', () => { + mockFn = jest.fn(); + wrapper = createWrapper({ onDismissClick: mockFn }); + + wrapper.find('WithStyles(Button)').first().simulate('click'); + expect(mockFn).toHaveBeenCalledTimes(1); + expect(mockFn).toHaveBeenCalledWith('http://example.com'); + }); + + it('has a try again button that fires the onTryAgainClick prop', () => { + mockFn = jest.fn(); + wrapper = createWrapper({ onTryAgainClick: mockFn }); + + wrapper.find('WithStyles(Button)').last().simulate('click'); + expect(mockFn).toHaveBeenCalledTimes(1); + expect(mockFn).toHaveBeenCalledWith('http://example.com'); + }); +}); diff --git a/__tests__/src/reducers/manifests.test.js b/__tests__/src/reducers/manifests.test.js index 21cbe9bc166dd990d9b6451aab00807e773dad93..649f01c786d338e495c4a7f86780054c476ef48e 100644 --- a/__tests__/src/reducers/manifests.test.js +++ b/__tests__/src/reducers/manifests.test.js @@ -18,6 +18,7 @@ describe('manifests reducer', () => { abc123: { id: 'abc123', isFetching: true, + error: 'Error fetching manifest', }, }, { @@ -34,9 +35,11 @@ describe('manifests reducer', () => { id: 'abc123', isFetching: false, manifestation: {}, + error: null, }, }); }); + it('should handle RECEIVE_MANIFEST_FAILURE', () => { expect(manifestsReducer( { diff --git a/locales/en/translation.json b/locales/en/translation.json index cccb1a7cd96c16732278976805381d66fcbebc24..2b316ec58f400c10823d9c1105cc405e566832e1 100644 --- a/locales/en/translation.json +++ b/locales/en/translation.json @@ -16,12 +16,14 @@ "closeMenu": "Close Menu", "closeWindow": "Close window", "dark": "Dark", + "dismiss": "Dismiss", "downloadExport": "Download/Export", "downloadExportWorkspace": "Download/export workspace", "fetchManifest": "Add", "fullScreen": "Full Screen", "light": "Light", "listAllOpenWindows": "List all open windows", + "manifestError": "The resource cannot be added:", "menu": "Menu", "off": "Off", "openInfoCompanionWindow": "Open information companion window", @@ -35,6 +37,7 @@ "theme": "Theme", "thumbnails": "Thumbnails", "toggleWindowSideBar": "Toggle window sidebar", + "tryAgain": "Try again", "untitled": "[Untitled]", "view": "View", "zoomIn": "Zoom in", diff --git a/src/components/ManifestListItem.js b/src/components/ManifestListItem.js index ade9413cd04a8f91e1062293673c9c11d96b07f7..6f91563f5ec041781a797855c4c7339edc3b77c2 100644 --- a/src/components/ManifestListItem.js +++ b/src/components/ManifestListItem.js @@ -7,6 +7,7 @@ import Grid from '@material-ui/core/Grid'; import Typography from '@material-ui/core/Typography'; import ReactPlaceholder from 'react-placeholder'; import { TextBlock, TextRow, RectShape } from 'react-placeholder/lib/placeholders'; +import ManifestListItemError from '../containers/ManifestListItemError'; import WindowIcon from './WindowIcon'; import ns from '../config/css-ns'; import 'react-placeholder/lib/reactPlaceholder.css'; @@ -71,7 +72,7 @@ class ManifestListItem extends React.Component { if (error) { return ( <Paper elevation={1} className={classes.root} data-manifestid={manifestId}> - {error} + <ManifestListItemError manifestId={manifestId} /> </Paper> ); } diff --git a/src/components/ManifestListItemError.js b/src/components/ManifestListItemError.js new file mode 100644 index 0000000000000000000000000000000000000000..2d4c82df387012d7ac2c3317205fb9817ebd16e6 --- /dev/null +++ b/src/components/ManifestListItemError.js @@ -0,0 +1,79 @@ +import React, { Component } from 'react'; +import PropTypes from 'prop-types'; +import Button from '@material-ui/core/Button'; +import ErrorIcon from '@material-ui/icons/ErrorOutline'; +import Grid from '@material-ui/core/Grid'; +import Typography from '@material-ui/core/Typography'; +import { withStyles } from '@material-ui/core/styles'; + +/** + * ManifestListItemError renders a component displaying a + * message to the user about a problem loading a manifest +*/ +class ManifestListItemError extends Component { + /** + * Returns the rendered component + */ + render() { + const { + classes, manifestId, onDismissClick, onTryAgainClick, t, + } = this.props; + + return ( + <Grid container> + <Grid container> + <Grid container item xs={12} sm={6}> + <Grid item xs={4} sm={3}> + <Grid container justify="center"> + <ErrorIcon className={classes.errorIcon} /> + </Grid> + </Grid> + <Grid item xs={8} sm={9}> + <Typography>{t('manifestError')}</Typography> + <Typography className={classes.manifestIdText}>{manifestId}</Typography> + </Grid> + </Grid> + </Grid> + + <Grid container> + <Grid container item xs={12} sm={6} justify="flex-end"> + <Grid item> + <Button onClick={() => { onDismissClick(manifestId); }}> + {t('dismiss')} + </Button> + <Button onClick={() => { onTryAgainClick(manifestId); }}> + {t('tryAgain')} + </Button> + </Grid> + </Grid> + </Grid> + </Grid> + ); + } +} + + +ManifestListItemError.propTypes = { + classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types + manifestId: PropTypes.string.isRequired, + onDismissClick: PropTypes.func.isRequired, + onTryAgainClick: PropTypes.func.isRequired, + t: PropTypes.func.isRequired, +}; + +/** + Material UI styles + @private + */ +const styles = theme => ({ + errorIcon: { + color: theme.palette.error.main, + height: '2rem', + width: '2rem', + }, + manifestIdText: { + wordBreak: 'break-all', + }, +}); + +export default withStyles(styles)(ManifestListItemError); diff --git a/src/containers/ManifestListItemError.js b/src/containers/ManifestListItemError.js new file mode 100644 index 0000000000000000000000000000000000000000..237c17f9b5a3048ec1db3f1e3d8e6bdf264c6c01 --- /dev/null +++ b/src/containers/ManifestListItemError.js @@ -0,0 +1,19 @@ +import { compose } from 'redux'; +import { connect } from 'react-redux'; +import { withNamespaces } from 'react-i18next'; +import { fetchManifest, removeManifest } from '../state/actions/manifest'; +import ManifestListItemError from '../components/ManifestListItemError'; + +/** */ +const mapDispatchToProps = { + onDismissClick: removeManifest, + onTryAgainClick: fetchManifest, +}; + +const enhance = compose( + connect(null, mapDispatchToProps), + withNamespaces(), + // further HOC +); + +export default enhance(ManifestListItemError); diff --git a/src/state/actions/manifest.js b/src/state/actions/manifest.js index beda211f010c8bbde7daeb50c4417d7e5e1cb984..a26e8998113559d6fd69291e4bd264c848c437b8 100644 --- a/src/state/actions/manifest.js +++ b/src/state/actions/manifest.js @@ -58,7 +58,13 @@ export function fetchManifest(manifestId, properties) { return fetch(manifestId) .then(response => response.json()) .then(json => dispatch(receiveManifest(manifestId, json))) - .catch(error => dispatch(receiveManifestFailure(manifestId, error))); + .catch((error) => { + if (typeof error === 'object') { // Returned by JSON parse failure + dispatch(receiveManifestFailure(manifestId, String(error))); + } else { + dispatch(receiveManifestFailure(manifestId, error)); + } + }); }); } diff --git a/src/state/reducers/manifests.js b/src/state/reducers/manifests.js index 5b1c0ba73bae927896e3f926cdd011719b136d2a..c3f52fdd6a57b5f98cb58884a12b6e39ea710df0 100644 --- a/src/state/reducers/manifests.js +++ b/src/state/reducers/manifests.js @@ -23,6 +23,7 @@ export const manifestsReducer = (state = {}, action) => { id: action.manifestId, manifestation: manifesto.create(action.manifestJson), isFetching: false, + error: null, // Explicitly set the error to null in case this is a re-fetch }, }; case ActionTypes.RECEIVE_MANIFEST_FAILURE: