From 50098181a0fc8d3d6a3535215ed7b7e7fd5cdfc8 Mon Sep 17 00:00:00 2001 From: Nicholas Echols Date: Sun, 22 Mar 2020 21:32:24 -0700 Subject: [PATCH] Show an error message when the page fails to load. fixes #166 --- .../__tests__/page-details.test.jsx | 32 +++++++++--- src/components/page-details/page-details.jsx | 50 ++++++++++++------- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/components/__tests__/page-details.test.jsx b/src/components/__tests__/page-details.test.jsx index 169a282b..33304e8a 100644 --- a/src/components/__tests__/page-details.test.jsx +++ b/src/components/__tests__/page-details.test.jsx @@ -10,13 +10,16 @@ describe('page-details', () => { simplePage.versions.forEach(version => { version.capture_time = new Date(version.capture_time); }); - const match = { params: { pageId: simplePage.uuid }}; - const createMockApi = () => { - return Object.assign(Object.create(WebMonitoringDb.prototype), { + const match = {params: {pageId: simplePage.uuid}}; + + const createMockApi = overrides => Object.assign( + Object.create(WebMonitoringDb.prototype), + { getPage: jest.fn().mockResolvedValue(simplePage), - getVersions: jest.fn().mockResolvedValue(simplePage.versions) - }); - }; + getVersions: jest.fn().mockResolvedValue(simplePage.versions), + }, + overrides + ); it('can render', () => { const mockApi = createMockApi(); @@ -49,4 +52,21 @@ describe('page-details', () => { pageDetails.unmount(); expect(document.title).toBe('Scanner'); }); + + it('shows an error message if api.getPage throws an error', async () => { + const error = new Error('Page does not exist'); + const mockApi = createMockApi({getPage: jest.fn().mockRejectedValue(error)}); + + const pageDetails = shallow( + , + {context: {api: mockApi}} + ); + + await expect(mockApi.getPage.mock.results[0].value).rejects.toThrow(); + await Promise.resolve(); // wait a tick for the error message to render + + expect(pageDetails.find('[className*="danger"]').text()).toBe(error.message); + }); }); diff --git a/src/components/page-details/page-details.jsx b/src/components/page-details/page-details.jsx index 5081d791..b91d4456 100644 --- a/src/components/page-details/page-details.jsx +++ b/src/components/page-details/page-details.jsx @@ -24,27 +24,32 @@ const cutoffDate = '2000-01-01'; * @param {PageDetailsProps} props */ export default class PageDetails extends React.Component { - static getDerivedStateFromProps (props, state) { + static getDerivedStateFromProps(props, state) { // Clear existing content when switching pages if (state.page && state.page.uuid !== props.match.params.pageId) { - return { page: null }; + return { + page: null + }; } return null; } - constructor (props) { + constructor(props) { super(props); - this.state = { page: null }; + this.state = { + page: null, + error: null + }; this._annotateChange = this._annotateChange.bind(this); this._navigateToChange = this._navigateToChange.bind(this); } - componentDidMount () { + componentDidMount() { window.addEventListener('keydown', this); this._loadPage(this.props.match.params.pageId); } - componentWillUnmount () { + componentWillUnmount() { window.removeEventListener('keydown', this); this.setTitle(true); } @@ -52,7 +57,7 @@ export default class PageDetails extends React.Component { /** * @param {PageDetailsProps} previousProps */ - componentDidUpdate (previousProps) { + componentDidUpdate(previousProps) { this.setTitle(); const nextPageId = this.props.match.params.pageId; if (nextPageId !== previousProps.match.params.pageId) { @@ -60,7 +65,7 @@ export default class PageDetails extends React.Component { } } - setTitle (unmounting = false) { + setTitle(unmounting = false) { document.title = !unmounting && this.state.page ? `Scanner | ${this.state.page.url}` : 'Scanner'; } @@ -69,7 +74,7 @@ export default class PageDetails extends React.Component { * @private * @param {DOMEvent} event */ - handleEvent (event) { + handleEvent(event) { if (event.keyCode === 27) { this.props.history.push('/'); } @@ -82,7 +87,7 @@ export default class PageDetails extends React.Component { * @param {string} toVersion ID of the `to` version of the change * @param {Object} annotation */ - _annotateChange (fromVersion, toVersion, annotation) { + _annotateChange(fromVersion, toVersion, annotation) { return this.context.api.annotateChange( this.state.page.uuid, fromVersion, @@ -91,7 +96,15 @@ export default class PageDetails extends React.Component { ); } - render () { + render() { + if (this.state.error) { + return ( +
+ {this.state.error.message} +
+ ); + } + if (!this.state.page) { return (); } @@ -122,7 +135,7 @@ export default class PageDetails extends React.Component { ); } - _renderPager () { + _renderPager() { if (!this.props.pages) { return null; } @@ -159,7 +172,7 @@ export default class PageDetails extends React.Component { * @private * @returns {JSX.Element} */ - _renderChange () { + _renderChange() { /** TODO: should we show 404 for bad versions? */ const versionData = this._versionsToRender(); @@ -195,7 +208,7 @@ export default class PageDetails extends React.Component { * @private * @returns {Object} */ - _versionsToRender () { + _versionsToRender() { const [fromId, toId] = (this.props.match.params.change || '').split('..'); const versions = this.state.page.versions; let from, to, shouldRedirect = false; @@ -236,7 +249,7 @@ export default class PageDetails extends React.Component { return {from, to, shouldRedirect}; } - _loadPage (pageId) { + _loadPage(pageId) { // TODO: handle the missing `.versions` collection problem better const fromList = this.props.pages && this.props.pages.find( (page) => page.uuid === pageId && !!page.versions); @@ -254,7 +267,8 @@ export default class PageDetails extends React.Component { page.versions = versions; this.setState({page}); }); - }); + }) + .catch(error => this.setState({error})); } _loadVersions(page, dateFrom, dateTo) { @@ -262,13 +276,13 @@ export default class PageDetails extends React.Component { return this.context.api.getVersions(page.uuid, capture_time, Infinity); } - _getChangeUrl (from, to, page) { + _getChangeUrl(from, to, page) { const changeId = (from && to) ? `${from.uuid}..${to.uuid}` : ''; const pageId = page && page.uuid || this.props.match.params.pageId; return `/page/${pageId}/${changeId}`; } - _navigateToChange (from, to, page, replace = false) { + _navigateToChange(from, to, page, replace = false) { const url = this._getChangeUrl(from, to, page); this.props.history[replace ? 'replace' : 'push'](url); }