Skip to content

Commit

Permalink
Show an error message when the page fails to load. fixes edgi-govdata…
Browse files Browse the repository at this point in the history
  • Loading branch information
steryereo committed Mar 25, 2020
1 parent 814096e commit 5009818
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 24 deletions.
32 changes: 26 additions & 6 deletions src/components/__tests__/page-details.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(
<PageDetails
match={match}
/>,
{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);
});
});
50 changes: 32 additions & 18 deletions src/components/page-details/page-details.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,43 +24,48 @@ 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);
}

/**
* @param {PageDetailsProps} previousProps
*/
componentDidUpdate (previousProps) {
componentDidUpdate(previousProps) {
this.setTitle();
const nextPageId = this.props.match.params.pageId;
if (nextPageId !== previousProps.match.params.pageId) {
this._loadPage(nextPageId);
}
}

setTitle (unmounting = false) {
setTitle(unmounting = false) {
document.title = !unmounting && this.state.page ? `Scanner | ${this.state.page.url}` : 'Scanner';
}

Expand All @@ -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('/');
}
Expand All @@ -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,
Expand All @@ -91,7 +96,15 @@ export default class PageDetails extends React.Component {
);
}

render () {
render() {
if (this.state.error) {
return (
<div styleName="baseStyles.alert baseStyles.alert-danger">
{this.state.error.message}
</div>
);
}

if (!this.state.page) {
return (<Loading />);
}
Expand Down Expand Up @@ -122,7 +135,7 @@ export default class PageDetails extends React.Component {
);
}

_renderPager () {
_renderPager() {
if (!this.props.pages) {
return null;
}
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -254,21 +267,22 @@ export default class PageDetails extends React.Component {
page.versions = versions;
this.setState({page});
});
});
})
.catch(error => this.setState({error}));
}

_loadVersions(page, dateFrom, dateTo) {
const capture_time = {'capture_time': `${dateFrom}..${dateTo}`};
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);
}
Expand Down

0 comments on commit 5009818

Please sign in to comment.