Skip to content

Commit

Permalink
Fix/error handling 404 (#17)
Browse files Browse the repository at this point in the history 
* Add fix for 404 no results error

* Fix display of error message

- use () => method so closeModal isn't called every render

* Refactor scroll to top feature, reduce duplication

* Fix errorModal on Login form

* Add fix for range search

- Use proper encoded [ and ]

* Fix npm cover script, use correct server test
  • Loading branch information
@ONS-Tom @ONS-Anthony
ONS-Tom authored and ONS-Anthony committed Dec 7, 2017
1 parent eeedeea commit 722ea55
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 37 deletions.
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@
"start:react": "REACT_APP_ENV=local REACT_APP_AUTH_URL=http://localhost:3001 REACT_APP_API_URL=http://localhost:3001/api react-scripts start",
"build": "react-scripts build",
"eject": "react-scripts eject",
"cover": "NODE_ENV=test SERVE_HTML=true ENV=local ./node_modules/istanbul/lib/cli.js cover --report cobertura ./node_modules/mocha/bin/_mocha -- -R spec test/server.test.js",
"test": "npm run test:unit; npm run test:server; npm run test:server",
"cover": "NODE_ENV=test SERVE_HTML=true ./node_modules/istanbul/lib/cli.js cover --report cobertura ./node_modules/mocha/bin/_mocha -- -R spec test/server-test-spec --recursive",
"test": "npm run test:unit; npm run test:server; npm run test:load",
"test:unit": "./node_modules/babel-cli/bin/babel-node.js test/utils-unit-tests.js",
"test:load": "./node_modules/babel-cli/bin/babel-node.js test/loadtest-unit-tests.js",
"test:server": "./node_modules/mocha/bin/mocha test/server.test.js; ./node_modules/mocha/bin/mocha test/server-test-spec --recursive",
Expand Down
51 changes: 33 additions & 18 deletions src/actions/ApiActions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,17 @@ export function matchSearch(query) {
apiSearch.match(formattedQuery, (success, data) => {
dispatch(sendingRequest(SENDING_MATCH_REQUEST, false));
if (success) {
dispatch(setResults(SET_MATCH_RESULTS, {
results: data.results,
}));
dispatch(setHeaders(SET_MATCH_HEADERS, {
headers: data.response,
}));
// This is a workaround for the API returning 200 {} for no results, should be 404
if (Object.keys(data.results).length === 0 && data.results.constructor === Object) {
dispatch(setErrorMessage(SET_MATCH_ERROR_MESSAGE, '404: No results found.', Math.floor(new Date() / 1000)));
} else {
dispatch(setResults(SET_MATCH_RESULTS, {
results: data.results,
}));
dispatch(setHeaders(SET_MATCH_HEADERS, {
headers: data.response,
}));
}
} else {
dispatch(setErrorMessage(SET_MATCH_ERROR_MESSAGE, data.message, Math.floor(new Date() / 1000)));
}
Expand All @@ -52,12 +57,17 @@ export function rangeSearch(query) {
apiSearch.match(formattedQuery, (success, data) => {
dispatch(sendingRequest(SENDING_RANGE_REQUEST, false));
if (success) {
dispatch(setResults(SET_RANGE_RESULTS, {
results: data.results,
}));
dispatch(setHeaders(SET_RANGE_HEADERS, {
headers: data.response,
}));
// This is a workaround for the API returning 200 {} for no results, should be 404
if (Object.keys(data.results).length === 0 && data.results.constructor === Object) {
dispatch(setErrorMessage(SET_RANGE_ERROR_MESSAGE, '404: No results found.', Math.floor(new Date() / 1000)));
} else {
dispatch(setResults(SET_RANGE_RESULTS, {
results: data.results,
}));
dispatch(setHeaders(SET_RANGE_HEADERS, {
headers: data.response,
}));
}
} else {
dispatch(setErrorMessage(SET_RANGE_ERROR_MESSAGE, data.message, Math.floor(new Date() / 1000)));
}
Expand All @@ -81,12 +91,17 @@ export function ubrnSearch(id) {
apiSearch.ubrn(id, (success, data) => {
dispatch(sendingRequest(SENDING_UBRN_REQUEST, false));
if (success) {
dispatch(setResults(SET_UBRN_RESULTS, {
results: data.results,
}));
dispatch(setHeaders(SET_UBRN_HEADERS, {
headers: data.response,
}));
// This is a workaround for the API returning 200 {} for no results, should be 404
if (Object.keys(data.results).length === 0 && data.results.constructor === Object) {
dispatch(setErrorMessage(SET_UBRN_ERROR_MESSAGE, '404: No results found.', Math.floor(new Date() / 1000)));
} else {
dispatch(setResults(SET_UBRN_RESULTS, {
results: data.results,
}));
dispatch(setHeaders(SET_UBRN_HEADERS, {
headers: data.response,
}));
}
} else {
dispatch(setErrorMessage(SET_UBRN_ERROR_MESSAGE, data.message, Math.floor(new Date() / 1000)));
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/ErrorModal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ class ErrorModal extends React.Component {
window.removeEventListener('keydown', this.props.close);
}
render() {
const modal = (this.props.show) ? (<ModalContainer onClose={this.props.close}>
<ModalDialog style={{ width: '80%' }} onClose={this.props.close}>
const modal = (this.props.show) ? (<ModalContainer onClose={this.props.close()}>
<ModalDialog style={{ width: '80%' }} onClose={this.props.close()}>
<h1 style={{ margin: '10px' }}>{this.props.message}</h1>
<br />
<button className="btn btn--primary" autoFocus onClick={() => this.props.close()}>Close</button>
<button className="btn btn--primary" autoFocus onClick={this.props.close()}>Close</button>
</ModalDialog>
</ModalContainer>) : <div></div>;
return (
Expand Down
2 changes: 1 addition & 1 deletion src/utils/formQuery.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function formRangeQuery(query) {
const queryArr = Object.keys(query).map(param => {
if (param === 'IndustryCode') {
// ES format: IndustryCode:[${min} TO ${max}]
return `${param}${SEPERATOR}%5B1${query[param][0]} TO ${query[param][1]}%5D`;
return `${param}${SEPERATOR}%5B${query[param][0]} TO ${query[param][1]}%5D`;
} else if (param === 'PostCode') {
// ES format: PostCode:(${postCode})
return `${param}${SEPERATOR}(${query[param]})`;
Expand Down
3 changes: 2 additions & 1 deletion src/views/Login.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Login extends React.Component {
}
closeModal() {
this.setState({ show: false, errorMessage: '' });
this.usernameInput.focus();
}
render() {
return (
Expand All @@ -36,7 +37,7 @@ class Login extends React.Component {
<input type="text" placeholder="username" ref={(ref) => (this.usernameInput = ref)} />
<input type="password" placeholder="password" ref={(ref) => (this.passwordInput = ref)} />
<Button id="loginButton" size="wide" text="Login" onClick={!this.props.data.currentlySending ? this.onSubmit : null} ariaLabel="Login Button" type="submit" loading={this.props.data.currentlySending} />
<ErrorModal show={this.state.show && this.props.data.errorMessage !== ''} message={this.props.data.errorMessage} close={this.closeModal} />
<ErrorModal show={this.state.show && this.props.data.errorMessage !== ''} message={this.props.data.errorMessage} close={() => this.closeModal} />
</form>
</div>
</div>
Expand Down
12 changes: 8 additions & 4 deletions src/views/Match.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,20 @@ class Match extends React.Component {
});
}
}
focusAndScroll() {
// Scroll to the top of the page and focus on the first input
document.getElementsByClassName('wrapper')[0].scrollIntoView(false);
this.child.childTextInput.myInput.focus();
}
closeModal() {
this.setState({ show: false, errorMessage: '' });
this.focusAndScroll();
}
clearQuery() {
this.props.dispatch(setQuery(SET_MATCH_QUERY, {}));
this.props.dispatch(setResults(SET_MATCH_RESULTS, { results: [] }));
this.setState({ formValues: {}, showFilter: false });
// Scroll to the top of the page and focus on the first input
document.getElementsByClassName('wrapper')[0].scrollIntoView(false);
this.child.childTextInput.myInput.focus();
this.focusAndScroll();
}
changeFilter() {
this.setState({ showFilter: !this.state.showFilter });
Expand Down Expand Up @@ -144,7 +148,7 @@ class Match extends React.Component {
<ErrorModal
show={this.state.show}
message={this.state.errorMessage}
close={this.closeModal}
close={() => this.closeModal}
/>
<br />
{this.props.data.results.length !== 0 &&
Expand Down
12 changes: 8 additions & 4 deletions src/views/RangeQuery.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,16 @@ class RangeQuery extends React.Component {
});
}
}
focusAndScroll() {
// Scroll to the top of the page and focus on the first input
document.getElementsByClassName('wrapper')[0].scrollIntoView(false);
this.child.childTextInput.myInput.focus();
}
clearQuery() {
this.props.dispatch(setQuery(SET_RANGE_QUERY, {}));
this.props.dispatch(setResults(SET_RANGE_RESULTS, { results: [] }));
this.setState({ formValues: {}, showFilter: false });
// Scroll to the top of the page and focus on the first input
document.getElementsByClassName('wrapper')[0].scrollIntoView(false);
this.child.childTextInput.myInput.focus();
this.focusAndScroll();
}
changeFilter() {
this.setState({ showFilter: !this.state.showFilter });
Expand All @@ -100,6 +103,7 @@ class RangeQuery extends React.Component {
}
closeModal() {
this.setState({ show: false, errorMessage: '' });
this.focusAndScroll();
}
changeQuery(evt) {
// if setting to empty, delete
Expand Down Expand Up @@ -145,7 +149,7 @@ class RangeQuery extends React.Component {
<ErrorModal
show={this.state.show}
message={this.state.errorMessage}
close={this.closeModal}
close={() => this.closeModal}
/>
<br />
{this.props.data.results.length !== 0 &&
Expand Down
12 changes: 8 additions & 4 deletions src/views/UBRNLookup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,19 @@ class UBRNLookup extends React.Component {
});
}
}
clearQuery() {
this.props.dispatch(setQuery(SET_UBRN_QUERY, ''));
this.props.dispatch(setResults(SET_UBRN_RESULTS, { results: [] }));
focusAndScroll() {
// Scroll to the top of the page and focus on the first input
document.getElementsByClassName('wrapper')[0].scrollIntoView(false);
this.child.childTextInput.myInput.focus();
}
clearQuery() {
this.props.dispatch(setQuery(SET_UBRN_QUERY, ''));
this.props.dispatch(setResults(SET_UBRN_RESULTS, { results: [] }));
this.focusAndScroll();
}
closeModal() {
this.setState({ show: false, errorMessage: '' });
this.focusAndScroll();
}
changeQuery(evt) {
// Store the query in Redux store, so we can access it again if a user
Expand Down Expand Up @@ -99,7 +103,7 @@ class UBRNLookup extends React.Component {
<ErrorModal
show={this.state.show}
message={this.state.errorMessage}
close={this.closeModal}
close={() => this.closeModal}
/>
<br />
{this.props.data.results.length !== 0 &&
Expand Down

0 comments on commit 722ea55

Please sign in to comment.