From fd1dcdc760e182f4a691ca4577caf80d60bb8cdc Mon Sep 17 00:00:00 2001 From: Noah Talerman Date: Wed, 28 Oct 2020 18:36:02 -0700 Subject: [PATCH 1/5] Created errors500 reducer to check for all actions with a payload that contains an errors object. I hooked up the 500 page to state tree. 500 page displays a button if an error message exists and reveals that message. --- frontend/interfaces/errors500.js | 6 ++ frontend/pages/Kolide500/Kolide500.jsx | 75 ++++++++++++++++++- frontend/pages/Kolide500/_styles.scss | 4 + frontend/redux/nodes/errors500/actions.js | 7 ++ frontend/redux/nodes/errors500/reducer.js | 22 ++++++ .../redux/nodes/errors500/reducer.tests.js | 46 ++++++++++++ frontend/redux/reducers.js | 2 + 7 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 frontend/interfaces/errors500.js create mode 100644 frontend/redux/nodes/errors500/actions.js create mode 100644 frontend/redux/nodes/errors500/reducer.js create mode 100644 frontend/redux/nodes/errors500/reducer.tests.js diff --git a/frontend/interfaces/errors500.js b/frontend/interfaces/errors500.js new file mode 100644 index 00000000000..43618340de3 --- /dev/null +++ b/frontend/interfaces/errors500.js @@ -0,0 +1,6 @@ +import PropTypes from 'prop-types'; + +export default PropTypes.shape({ + http_request: PropTypes.number, + base: PropTypes.string, +}); diff --git a/frontend/pages/Kolide500/Kolide500.jsx b/frontend/pages/Kolide500/Kolide500.jsx index dd9282f41ec..0c878596646 100644 --- a/frontend/pages/Kolide500/Kolide500.jsx +++ b/frontend/pages/Kolide500/Kolide500.jsx @@ -1,12 +1,72 @@ import React, { Component } from 'react'; +import PropTypes from 'prop-types'; +import { connect } from 'react-redux'; +import { noop } from 'lodash'; +import { resetErrors } from 'redux/nodes/errors500/actions'; +import errorsInterface from 'interfaces/errors500'; import kolideLogo from '../../../assets/images/kolide-logo-condensed.svg'; import gopher from '../../../assets/images/500.svg'; const baseClass = 'kolide-500'; -class Kolide404 extends Component { +class Kolide500 extends Component { + static propTypes = { + errors: errorsInterface, + dispatch: PropTypes.func, + }; + + static defaultProps = { + dispatch: noop, + }; + + constructor (props) { + super(props); + + this.state = { + showErrorMessage: false, + }; + } + + componentWillUnmount() { + const { dispatch } = this.props; + dispatch(resetErrors()); + } + + onShowErrorMessage = () => { + this.setState({ showErrorMessage: true }); + } + + renderError = () => { + const { errors } = this.props; + const errorMessage = errors ? errors.base : null; + const { showErrorMessage } = this.state; + const { onShowErrorMessage } = this; + + if (errorMessage && !showErrorMessage) { + // We only show the button when errorMessage exists + // and showErrorMessage is set to false + return ( + + ); + } + + if (errorMessage && showErrorMessage) { + // We only show the error message when errorMessage exists + // and showErrorMessage is set to true + return ( +
+

{errorMessage}

+
+ ); + } + + return false; + } + render () { + const { renderError } = this; + return (
@@ -18,10 +78,10 @@ class Kolide404 extends Component {

Uh oh!

Error 500

Something went wrong on our end.

-

We have alerted the engineers and they are working on a solution.

+ {renderError()} +

Need assistance? File an issue.

-

Need assistance? File an issue.

@@ -29,4 +89,11 @@ class Kolide404 extends Component { } } -export default Kolide404; +const mapStateToProps = (state) => { + const { errors } = state.errors; + return { + errors, + }; +}; + +export default connect(mapStateToProps)(Kolide500); diff --git a/frontend/pages/Kolide500/_styles.scss b/frontend/pages/Kolide500/_styles.scss index 62f835200e7..775818d2cef 100644 --- a/frontend/pages/Kolide500/_styles.scss +++ b/frontend/pages/Kolide500/_styles.scss @@ -36,6 +36,10 @@ } } + .error-message-container { + display: inline; + } + main { text-align: center; diff --git a/frontend/redux/nodes/errors500/actions.js b/frontend/redux/nodes/errors500/actions.js new file mode 100644 index 00000000000..2b4c2e4f379 --- /dev/null +++ b/frontend/redux/nodes/errors500/actions.js @@ -0,0 +1,7 @@ +export const RESET_ERRORS = 'RESET_ERRORS'; + +export const resetErrors = () => { + return { + type: RESET_ERRORS, + }; +}; diff --git a/frontend/redux/nodes/errors500/reducer.js b/frontend/redux/nodes/errors500/reducer.js new file mode 100644 index 00000000000..03fe16f081a --- /dev/null +++ b/frontend/redux/nodes/errors500/reducer.js @@ -0,0 +1,22 @@ +import { + RESET_ERRORS, +} from './actions'; + +const initialState = { + errors: null, +}; + +const reducer = (state = initialState, { type, payload }) => { + if (payload && payload.errors) { + return { + errors: payload.errors, + }; + } else if (type === RESET_ERRORS) { + return { + errors: null, + }; + } + return state; +}; + +export default reducer; diff --git a/frontend/redux/nodes/errors500/reducer.tests.js b/frontend/redux/nodes/errors500/reducer.tests.js new file mode 100644 index 00000000000..a816b78f53e --- /dev/null +++ b/frontend/redux/nodes/errors500/reducer.tests.js @@ -0,0 +1,46 @@ +import expect from 'expect'; +import { LOCATION_CHANGE } from 'react-router-redux'; + +import reducer, { initialState } from './reducer'; +import { + hideFlash, + renderFlash, +} from './actions'; + +describe('Notifications - reducer', () => { + it('Updates state with notification info when RENDER_FLASH is dispatched', () => { + const undoAction = { type: 'UNDO' }; + const newState = reducer(initialState, renderFlash('success', 'You did it!', undoAction)); + + expect(newState).toEqual({ + alertType: 'success', + isVisible: true, + message: 'You did it!', + undoAction, + }); + }); + + it('Updates state to hide notifications when HIDE_FLASH is dispatched', () => { + const stateWithFlashDisplayed = reducer(initialState, renderFlash('success', 'You did it!')); + const newState = reducer(stateWithFlashDisplayed, hideFlash); + + expect(newState).toEqual({ + alertType: null, + isVisible: false, + message: null, + undoAction: null, + }); + }); + + it('Updates state to hide notifications during location change', () => { + const stateWithFlashDisplayed = reducer(initialState, renderFlash('success', 'You did it!')); + const newState = reducer(stateWithFlashDisplayed, { type: LOCATION_CHANGE }); + + expect(newState).toEqual({ + alertType: null, + isVisible: false, + message: null, + undoAction: null, + }); + }); +}); diff --git a/frontend/redux/reducers.js b/frontend/redux/reducers.js index 87426a618f1..fb4430031e2 100644 --- a/frontend/redux/reducers.js +++ b/frontend/redux/reducers.js @@ -6,6 +6,7 @@ import app from './nodes/app/reducer'; import auth from './nodes/auth/reducer'; import components from './nodes/components/reducer'; import entities from './nodes/entities/reducer'; +import errors500 from './nodes/errors500/reducer'; import notifications from './nodes/notifications/reducer'; import persistentFlash from './nodes/persistent_flash/reducer'; import redirectLocation from './nodes/redirectLocation/reducer'; @@ -15,6 +16,7 @@ export default combineReducers({ auth, components, entities, + errors500, loadingBar: loadingBarReducer, notifications, persistentFlash, From d0f805f72241e43ff87cb5b96614de75f131bd65 Mon Sep 17 00:00:00 2001 From: Noah Talerman Date: Wed, 28 Oct 2020 18:55:15 -0700 Subject: [PATCH 2/5] Changing messaging for reporting an issue --- frontend/pages/Kolide500/Kolide500.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/pages/Kolide500/Kolide500.jsx b/frontend/pages/Kolide500/Kolide500.jsx index 0c878596646..d9a780d3812 100644 --- a/frontend/pages/Kolide500/Kolide500.jsx +++ b/frontend/pages/Kolide500/Kolide500.jsx @@ -79,7 +79,8 @@ class Kolide500 extends Component {

Error 500

Something went wrong on our end.

{renderError()} -

Need assistance? File an issue.

+

Please file an issue if you believe this is a bug.

+ File an issue
From 15d10b20a847f47ac5eef3367edbe0784f592de2 Mon Sep 17 00:00:00 2001 From: Noah Talerman Date: Thu, 29 Oct 2020 08:43:00 -0700 Subject: [PATCH 3/5] When moving my chnages over from github, I wrongly committed changes to the reducer test file from the persistent_flash tests. Now the errors500 tests are updated and correct. --- .../redux/nodes/errors500/reducer.tests.js | 61 ++++++++----------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/frontend/redux/nodes/errors500/reducer.tests.js b/frontend/redux/nodes/errors500/reducer.tests.js index a816b78f53e..6adada62282 100644 --- a/frontend/redux/nodes/errors500/reducer.tests.js +++ b/frontend/redux/nodes/errors500/reducer.tests.js @@ -1,46 +1,39 @@ import expect from 'expect'; -import { LOCATION_CHANGE } from 'react-router-redux'; -import reducer, { initialState } from './reducer'; -import { - hideFlash, - renderFlash, -} from './actions'; +import reducer from './reducer'; -describe('Notifications - reducer', () => { - it('Updates state with notification info when RENDER_FLASH is dispatched', () => { - const undoAction = { type: 'UNDO' }; - const newState = reducer(initialState, renderFlash('success', 'You did it!', undoAction)); +describe('Errors - reducer', () => { + it('Updates state with errors object when an action that has a payload with an errors object is dispatched', () => { + const payload = { + errors: { + base: "inserting pack: Error 1136: Column count doesn't match value count at row 1", + http_status: 500, + }, + }; + const packsCreateFailureAction = { type: 'packs_CREATE_FAILURE', payload }; + const initialState = { + errors: null, + }; + const newState = reducer(initialState, packsCreateFailureAction); expect(newState).toEqual({ - alertType: 'success', - isVisible: true, - message: 'You did it!', - undoAction, + errors: { + base: "inserting pack: Error 1136: Column count doesn't match value count at row 1", + http_status: 500, + }, }); }); - it('Updates state to hide notifications when HIDE_FLASH is dispatched', () => { - const stateWithFlashDisplayed = reducer(initialState, renderFlash('success', 'You did it!')); - const newState = reducer(stateWithFlashDisplayed, hideFlash); - - expect(newState).toEqual({ - alertType: null, - isVisible: false, - message: null, - undoAction: null, - }); - }); - - it('Updates state to hide notifications during location change', () => { - const stateWithFlashDisplayed = reducer(initialState, renderFlash('success', 'You did it!')); - const newState = reducer(stateWithFlashDisplayed, { type: LOCATION_CHANGE }); - + it('Updates state by setting errors to null when the RESET_ERRORS action is dipatched', () => { + const errorsState = { + errors: { + base: "inserting pack: Error 1136: Column count doesn't match value count at row 1", + http_status: 500, + }, + }; + const newState = reducer(errorsState, { type: 'RESET_ERRORS' }); expect(newState).toEqual({ - alertType: null, - isVisible: false, - message: null, - undoAction: null, + errors: null, }); }); }); From eafe3868b73aa3a035076f3fe84b5e57a0bd9268 Mon Sep 17 00:00:00 2001 From: Noah Talerman Date: Thu, 29 Oct 2020 14:37:05 -0700 Subject: [PATCH 4/5] Changed naming in error500 interface. Fixed incorrect property name in Kolide500 mapStateToProps. --- frontend/interfaces/errors500.js | 2 +- frontend/pages/Kolide500/Kolide500.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/interfaces/errors500.js b/frontend/interfaces/errors500.js index 43618340de3..39b02123c22 100644 --- a/frontend/interfaces/errors500.js +++ b/frontend/interfaces/errors500.js @@ -1,6 +1,6 @@ import PropTypes from 'prop-types'; export default PropTypes.shape({ - http_request: PropTypes.number, + http_status: PropTypes.number, base: PropTypes.string, }); diff --git a/frontend/pages/Kolide500/Kolide500.jsx b/frontend/pages/Kolide500/Kolide500.jsx index d9a780d3812..7ce9152a16a 100644 --- a/frontend/pages/Kolide500/Kolide500.jsx +++ b/frontend/pages/Kolide500/Kolide500.jsx @@ -91,7 +91,7 @@ class Kolide500 extends Component { } const mapStateToProps = (state) => { - const { errors } = state.errors; + const { errors } = state.errors500; return { errors, }; From 0ac12c7f2154b69e71c001497a607b45e4c11e53 Mon Sep 17 00:00:00 2001 From: Noah Talerman Date: Tue, 3 Nov 2020 16:27:31 -0800 Subject: [PATCH 5/5] Changed link to docs on Kolide500 page to reference new fork. Also anchor tag properties so the link opens in a new tab. --- frontend/pages/Kolide500/Kolide500.jsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frontend/pages/Kolide500/Kolide500.jsx b/frontend/pages/Kolide500/Kolide500.jsx index 7ce9152a16a..a0772c432c3 100644 --- a/frontend/pages/Kolide500/Kolide500.jsx +++ b/frontend/pages/Kolide500/Kolide500.jsx @@ -80,7 +80,13 @@ class Kolide500 extends Component {

Something went wrong on our end.

{renderError()}

Please file an issue if you believe this is a bug.

- File an issue + + File an issue +