Fixed showing back to back error notifications

CityOfZion · Jan 22, 2018 · dc14e89 · dc14e89
1 parent f1fc3cd
commit dc14e89
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 25 deletions.
diff --git a/__tests__/components/__snapshots__/LoginNep2.test.js.snap b/__tests__/components/__snapshots__/LoginNep2.test.js.snap
@@ -1,7 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`LoginNep2 renders without crashing 1`] = `
-<withActions(LoginNep2)
+<withActions(Connect(Connect(withError(Connect(withProgressProp(ErrorNotifier))))))
   loginNep2={[Function]}
   store={
     Object {

diff --git a/app/hocs/api/progressStrategies/alreadyLoadedStrategy.js b/app/hocs/api/progressStrategies/alreadyLoadedStrategy.js
@@ -14,7 +14,7 @@ function alreadyLoaded (actionStates: Array<Object>): boolean {
 export default function alreadyLoadedStrategy (actions: Array<Object>): ProgressState {
   if (anyFailed(actions)) {
     return FAILED
-  } else if (alreadyLoaded(actions)) {
+  } else if (alreadyLoaded(actions) && actions.length > 0) {
     return LOADED
   } else {
     return LOADING

diff --git a/app/hocs/api/progressStrategies/defaultStrategy.js b/app/hocs/api/progressStrategies/defaultStrategy.js
@@ -14,7 +14,7 @@ function allLoaded (actionStates: Array<Object>): boolean {
 export default function defaultStrategy (actions: Array<Object>): ProgressState {
   if (anyFailed(actions)) {
     return FAILED
-  } else if (allLoaded(actions)) {
+  } else if (allLoaded(actions) && actions.length > 0) {
     return LOADED
   } else {
     return LOADING

diff --git a/app/hocs/api/withProgressProp.js b/app/hocs/api/withProgressProp.js
@@ -21,15 +21,21 @@ export default function withProgressProp (
 ) {
   const mapProgressToProps = (actionStates: Array<ActionState>) => ({ [propName]: strategy(actionStates) })
 
-  const mapStateToProps: MapStateToProps<*, *, *> = (state: Object): Object => {
+  // TODO: this doesn't account for batch within a batch, need to make this recursive
+  const getActionStates = (state: Object): Array<ActionState> => {
     const actionState = get(state, `${prefix}.${actions.id}`)
 
-    // TODO: this doesn't account for batch within a batch, need to make this recursive
-    const actionStates: Array<ActionState> = actionState.batch
-      ? map(actionState.mapping, (key) => get(state, `${prefix}.${key}`))
-      : castArray(actionState)
+    if (!actionState) {
+      return []
+    } else if (actionState.batch) {
+      return map(actionState.mapping, (key) => get(state, `${prefix}.${key}`))
+    } else {
+      return castArray(actionState)
+    }
+  }
 
-    return mapProgressToProps(actionStates)
+  const mapStateToProps: MapStateToProps<*, *, *> = (state: Object): Object => {
+    return mapProgressToProps(getActionStates(state))
   }
 
   return (Component: Class<React.Component<*>>) => {

diff --git a/app/hocs/withFailureNotification.js b/app/hocs/withFailureNotification.js
@@ -5,50 +5,53 @@ import { compose } from 'recompose'
 import { omit } from 'lodash'
 
 import withError from './api/withError'
+import withProgressProp from './api/withProgressProp'
 import { type Actions } from '../values/api'
+import { FAILED, type ProgressState } from '../values/state'
 import { showErrorNotification } from '../modules/notifications'
 
 type Props = {
   __error__: string,
+  __progress__: ProgressState,
   __showErrorNotification__: Function
 }
 
-type Options = {
-  errorPropName: string,
-  notificationPropName: string
-}
-
 const ERROR_PROP = '__error__'
+const PROGRESS_PROP = '__progress__'
 const NOTIFICATION_PROP = '__showErrorNotification__'
 
-export default function withFailureNotifications (
-  actions: Actions,
-  { errorPropName = ERROR_PROP, notificationPropName = NOTIFICATION_PROP }: Options = {}
-) {
-  const mapErrorToProps = (error) => ({ [errorPropName]: error })
+export default function withFailureNotifications (actions: Actions) {
+  const mapErrorToProps = (error) => ({ [ERROR_PROP]: error })
 
   const mapDisptchToProps = (dispatch, ownProps) => ({
-    [notificationPropName]: (...args) => dispatch(showErrorNotification(...args))
+    [NOTIFICATION_PROP]: (...args) => dispatch(showErrorNotification(...args))
   })
 
   return (Component: Class<React.Component<*>>): Class<React.Component<*>> => {
+    const hasError = (props: Props) => !!props[ERROR_PROP]
+
+    const progressChangedToError = (prevProps: Props, nextProps: Props) => {
+      return prevProps[PROGRESS_PROP] !== FAILED && nextProps[PROGRESS_PROP] === FAILED
+    }
+
     class ErrorNotifier extends React.Component<Props> {
       componentWillReceiveProps (nextProps) {
-        if (nextProps[errorPropName] && !this.props[errorPropName]) {
-          const showErrorNotification = nextProps[notificationPropName]
-          showErrorNotification({ message: nextProps[errorPropName] })
+        if (hasError(nextProps) && (!hasError(this.props) || progressChangedToError(this.props, nextProps))) {
+          const showErrorNotification = nextProps[NOTIFICATION_PROP]
+          showErrorNotification({ message: nextProps[ERROR_PROP] })
         }
       }
 
       render () {
-        const passDownProps = omit(this.props, errorPropName)
+        const passDownProps = omit(this.props, ERROR_PROP, PROGRESS_PROP, NOTIFICATION_PROP)
         return <Component {...passDownProps} />
       }
     }
 
     return compose(
       connect(null, mapDisptchToProps),
-      withError(actions, mapErrorToProps)
+      withError(actions, mapErrorToProps),
+      withProgressProp(actions, { propName: PROGRESS_PROP })
     )(ErrorNotifier)
   }
 }