Skip to content
This repository has been archived by the owner on Apr 11, 2019. It is now read-only.

Commit

Permalink
Issue #135 - Handle better the diff viewer not having coverage data
Browse files Browse the repository at this point in the history
Sometimes, we would need to visit the diff viewer for old changesets.
Unfortunately, the backend would not have data available and the viewer
would fail ungracefully.

Before this, the console would be full of errors even if the UI
would not fail dramatically.
  • Loading branch information
armenzg committed Jun 6, 2018
1 parent 12311fd commit 2695903
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 11 deletions.
3 changes: 1 addition & 2 deletions src/components/diffViewer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Link } from 'react-router-dom';
import DiffFile from './diffFile';
import CoverageMeta from './coverageMeta';
import CoverageFooter from './coverageFooter';
import settings from '../settings';

const DiffViewer = ({
appError, changeset, coverage, parsedDiff,
Expand All @@ -12,7 +11,7 @@ const DiffViewer = ({
<Link to="/" href="/">Return to main page</Link>
</div>
<span className="error_message">{appError}</span>
{(coverage && coverage.summary !== settings.STRINGS.PENDING) && (
{(coverage && coverage.show) && (
<div>
<CoverageMeta
changeset={changeset}
Expand Down
29 changes: 20 additions & 9 deletions src/containers/diffViewer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ export default class DiffViewerContainer extends Component {
constructor(props) {
super(props);
this.state = {
appError: undefined,
coverage: undefined,
appError: '',
changeset: {},
coverage: {},
parsedDiff: [],
supportedExtensions: [],
};
}

Expand All @@ -53,12 +55,14 @@ export default class DiffViewerContainer extends Component {
async fetchSetCoverageData(node) {
try {
const coverage = await getChangesetCoverage(node);
let appError = '';
if (coverage.summary === settings.STRINGS.PENDING) {
this.setState({
appError: 'The coverage data is still pending. Try again later.',
});
appError = 'The coverage data is still pending. Please try again later.';
}
this.setState({ coverage });
if (!coverage.show) {
appError = 'The coverage backend has had an internal error.';
}
this.setState({ appError, coverage });
} catch (error) {
console.error(error);
this.setState({
Expand Down Expand Up @@ -94,10 +98,17 @@ export default class DiffViewerContainer extends Component {

render() {
const {
appError, changeset, coverage, parsedDiff, supportedExtensions,
appError,
changeset,
coverage,
parsedDiff,
supportedExtensions,
} = this.state;
const onlySupportedExtensions = filterUnsupportedExtensions(parsedDiff, supportedExtensions);
const sortedDiff = sortByPercent(onlySupportedExtensions, coverage);
let sortedDiff = [];
if (coverage && coverage.show) {
const onlySupportedExtensions = filterUnsupportedExtensions(parsedDiff, supportedExtensions);
sortedDiff = sortByPercent(onlySupportedExtensions, coverage);
}

return (
<DiffViewer
Expand Down

0 comments on commit 2695903

Please sign in to comment.