From 700700eb706f449642685bed9098e1de0b1df158 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Mon, 19 Nov 2018 13:26:03 -0500 Subject: [PATCH 1/2] avoid create/save/error loops --- src/lib/alerts/index.jsx | 2 ++ src/lib/project-saver-hoc.jsx | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/lib/alerts/index.jsx b/src/lib/alerts/index.jsx index 9fb1800f8..96d16a04e 100644 --- a/src/lib/alerts/index.jsx +++ b/src/lib/alerts/index.jsx @@ -48,6 +48,7 @@ const alerts = [ }, { alertId: 'creatingError', + clearList: ['creating', 'createSuccess'], content: ( { - this.props.onShowSaveSuccessAlert(); // there is nothing we expect to find in response that we need to check here this.props.onUpdatedProject(this.props.loadingState); + this.props.onShowSaveSuccessAlert(); }) .catch(err => { // NOTE: should throw up a notice for user @@ -95,11 +97,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 +113,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 +129,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 +172,8 @@ const ProjectSaverHOC = function (WrappedComponent) { ); }) .catch(err => { - // @todo do something here log.error(err); + throw err; // pass the error up the chain }); } render () { From b012a2f34e9247605e37b8ed1a7ee81f55d44e2d Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Mon, 19 Nov 2018 18:16:48 -0500 Subject: [PATCH 2/2] cleaned up create and save logic --- src/lib/project-saver-hoc.jsx | 43 ++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/lib/project-saver-hoc.jsx b/src/lib/project-saver-hoc.jsx index 90a42b7db..11df60b1a 100644 --- a/src/lib/project-saver-hoc.jsx +++ b/src/lib/project-saver-hoc.jsx @@ -22,7 +22,6 @@ import { getIsShowingWithId, getIsShowingWithoutId, getIsUpdating, - manualUpdateProject, projectError } from '../reducers/project-state'; @@ -51,24 +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; - const justTriedCreatingNew = prevProps.isCreatingNew; // don't immediately keep trying - if (showingCreateable && !prevShowingCreateable && !justTriedCreatingNew) { + // 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; - const justTriedUpdating = prevProps.isUpdating; // don't immediately keep trying - if (showingSaveable && !justTriedUpdating && (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 () { @@ -77,11 +77,15 @@ const ProjectSaverHOC = function (WrappedComponent) { this.updateProjectToStorage(); } } + isShowingCreatable (props) { + return props.canCreateNew && props.isShowingWithoutId; + } updateProjectToStorage () { this.props.onShowSavingAlert(); return this.storeProject(this.props.reduxProjectId) .then(() => { - // 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(); }) @@ -190,7 +194,6 @@ const ProjectSaverHOC = function (WrappedComponent) { onAutoUpdateProject, onCreatedProject, onCreateProject, - onManualUpdateProject, onProjectError, onShowAlert, onShowCreateSuccessAlert, @@ -226,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, @@ -258,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'),