Merge pull request #3835 from benjiwheeler/alert-safe-errors

avoid create/save/error loops

src/lib/alerts/index.jsx

@@ -48,6 +48,7 @@ const alerts = [
     },
     {
         alertId: 'creatingError',
+        clearList: ['creating', 'createSuccess'],
         content: (
             <FormattedMessage
                 defaultMessage="Could not create the project. Please try again!"
@@ -59,6 +60,7 @@ const alerts = [
     },
     {
         alertId: 'savingError',
+        clearList: ['saving', 'saveSuccess'],
         content: (
             <FormattedMessage
                 defaultMessage="Could not save the project. Please try again!"

src/lib/project-saver-hoc.jsx

@@ -22,7 +22,6 @@ import {
     getIsShowingWithId,
     getIsShowingWithoutId,
     getIsUpdating,
-    manualUpdateProject,
     projectError
 } from '../reducers/project-state';
 
@@ -51,22 +50,25 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 this.createRemixToStorage();
             }
 
-            // check if the project state, and user capabilities, have changed so as to indicate
-            // that we should create or update project
+            // see if we should "create" the current project on the server
             //
-            // if we're newly able to create this project on the server, create it!
-            const showingCreateable = this.props.canCreateNew && this.props.isShowingWithoutId;
-            const prevShowingCreateable = prevProps.canCreateNew && prevProps.isShowingWithoutId;
-            if (showingCreateable && !prevShowingCreateable) {
+            // don't try to create or save immediately after trying to create
+            if (prevProps.isCreatingNew) return;
+            // if we're newly able to create this project, create it!
+            if (this.isShowingCreatable(this.props) && !this.isShowingCreatable(prevProps)) {
                 this.props.onCreateProject();
-            } else {
-                // if we're newly able to save this project, save it!
-                const showingSaveable = this.props.canSave && this.props.isShowingWithId;
-                const becameAbleToSave = this.props.canSave && !prevProps.canSave;
-                const becameShared = this.props.isShared && !prevProps.isShared;
-                if (showingSaveable && (becameAbleToSave || becameShared)) {
-                    this.props.onAutoUpdateProject();
-                }
+            }
+
+            // see if we should save/update the current project on the server
+            //
+            // don't try to save immediately after trying to save
+            if (prevProps.isUpdating) return;
+            // if we're newly able to save this project, save it!
+            const showingSaveable = this.props.canSave && this.props.isShowingWithId;
+            const becameAbleToSave = this.props.canSave && !prevProps.canSave;
+            const becameShared = this.props.isShared && !prevProps.isShared;
+            if (showingSaveable && (becameAbleToSave || becameShared)) {
+                this.props.onAutoUpdateProject();
             }
         }
         componentWillUnmount () {
@@ -75,13 +77,17 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 this.updateProjectToStorage();
             }
         }
+        isShowingCreatable (props) {
+            return props.canCreateNew && props.isShowingWithoutId;
+        }
         updateProjectToStorage () {
             this.props.onShowSavingAlert();
             return this.storeProject(this.props.reduxProjectId)
                 .then(() => {
-                    this.props.onShowSaveSuccessAlert();
-                    // there is nothing we expect to find in response that we need to check here
+                    // there's an http response object available here, but we don't need to examine
+                    // it, because there are no values contained in it that we care about
                     this.props.onUpdatedProject(this.props.loadingState);
+                    this.props.onShowSaveSuccessAlert();
                 })
                 .catch(err => {
                     // NOTE: should throw up a notice for user
@@ -95,11 +101,11 @@ const ProjectSaverHOC = function (WrappedComponent) {
             this.props.onShowCreatingAlert();
             return this.storeProject(null)
                 .then(response => {
-                    this.props.onShowCreateSuccessAlert();
                     this.props.onCreatedProject(response.id.toString(), this.props.loadingState);
+                    this.props.onShowCreateSuccessAlert();
                 })
                 .catch(err => {
-                    this.props.onShowAlert('savingError');
+                    this.props.onShowAlert('creatingError');
                     this.props.onProjectError(err);
                 });
         }
@@ -111,11 +117,11 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 title: this.props.reduxProjectTitle
             })
                 .then(response => {
-                    this.props.onShowCreateSuccessAlert();
                     this.props.onCreatedProject(response.id.toString(), this.props.loadingState);
+                    this.props.onShowCreateSuccessAlert();
                 })
                 .catch(err => {
-                    this.props.onShowAlert('savingError');
+                    this.props.onShowAlert('creatingError');
                     this.props.onProjectError(err);
                 });
         }
@@ -127,11 +133,11 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 title: this.props.reduxProjectTitle
             })
                 .then(response => {
-                    this.props.onShowCreateSuccessAlert();
                     this.props.onCreatedProject(response.id.toString(), this.props.loadingState);
+                    this.props.onShowCreateSuccessAlert();
                 })
                 .catch(err => {
-                    this.props.onShowAlert('savingError');
+                    this.props.onShowAlert('creatingError');
                     this.props.onProjectError(err);
                 });
         }
@@ -170,8 +176,8 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 );
             })
                 .catch(err => {
-                    // @todo do something here
                     log.error(err);
+                    throw err; // pass the error up the chain
                 });
         }
         render () {
@@ -188,7 +194,6 @@ const ProjectSaverHOC = function (WrappedComponent) {
                 onAutoUpdateProject,
                 onCreatedProject,
                 onCreateProject,
-                onManualUpdateProject,
                 onProjectError,
                 onShowAlert,
                 onShowCreateSuccessAlert,
@@ -224,7 +229,6 @@ const ProjectSaverHOC = function (WrappedComponent) {
         onAutoUpdateProject: PropTypes.func,
         onCreateProject: PropTypes.func,
         onCreatedProject: PropTypes.func,
-        onManualUpdateProject: PropTypes.func,
         onProjectError: PropTypes.func,
         onShowAlert: PropTypes.func,
         onShowCreateSuccessAlert: PropTypes.func,
@@ -256,7 +260,6 @@ const ProjectSaverHOC = function (WrappedComponent) {
         onAutoUpdateProject: () => dispatch(autoUpdateProject()),
         onCreatedProject: (projectId, loadingState) => dispatch(doneCreatingProject(projectId, loadingState)),
         onCreateProject: () => dispatch(createProject()),
-        onManualUpdateProject: () => dispatch(manualUpdateProject()),
         onProjectError: error => dispatch(projectError(error)),
         onShowAlert: alertType => dispatch(showStandardAlert(alertType)),
         onShowCreateSuccessAlert: () => showAlertWithTimeout(dispatch, 'createSuccess'),