Merge pull request #4134 from benjiwheeler/save-before-upload

When user uploads project file, project will auto-save before loading

src/containers/sb-file-uploader.jsx

@@ -6,7 +6,12 @@ import {defineMessages, injectIntl, intlShape} from 'react-intl';
 
 import analytics from '../lib/analytics';
 import log from '../lib/log';
-import {LoadingStates, onLoadedProject, onProjectUploadStarted} from '../reducers/project-state';
+import {
+    LoadingStates,
+    getIsLoadingUpload,
+    onLoadedProject,
+    requestProjectUpload
+} from '../reducers/project-state';
 
 import {
     openLoadingProject,
@@ -48,9 +53,31 @@ class SBFileUploader extends React.Component {
             'renderFileInput',
             'setFileInput',
             'handleChange',
-            'handleClick'
+            'handleClick',
+            'onload',
+            'resetFileInput'
         ]);
     }
+    componentWillMount () {
+        this.reader = new FileReader();
+        this.reader.onload = this.onload;
+        this.resetFileInput();
+    }
+    componentDidUpdate (prevProps) {
+        if (this.props.isLoadingUpload && !prevProps.isLoadingUpload && this.fileToUpload && this.reader) {
+            this.reader.readAsArrayBuffer(this.fileToUpload);
+        }
+    }
+    componentWillUnmount () {
+        this.reader = null;
+        this.resetFileInput();
+    }
+    resetFileInput () {
+        this.fileToUpload = null;
+        if (this.fileInput) {
+            this.fileInput.value = null;
+        }
+    }
     getProjectTitleFromFilename (fileInputFilename) {
         if (!fileInputFilename) return '';
         // only parse title from files like "filename.sb2" or "filename.sb3"
@@ -60,35 +87,43 @@ class SBFileUploader extends React.Component {
     }
     // called when user has finished selecting a file to upload
     handleChange (e) {
-        // Remove the hash if any (without triggering a hash change event or a reload)
-        history.replaceState({}, document.title, '.');
-        const reader = new FileReader();
         const thisFileInput = e.target;
-        reader.onload = () => this.props.vm.loadProject(reader.result)
-            .then(() => {
-                analytics.event({
-                    category: 'project',
-                    action: 'Import Project File',
-                    nonInteraction: true
-                });
-                this.props.onLoadingFinished(this.props.loadingState);
-                // Reset the file input after project is loaded
-                // This is necessary in case the user wants to reload a project
-                thisFileInput.value = null;
-            })
-            .catch(error => {
-                log.warn(error);
-                alert(this.props.intl.formatMessage(messages.loadError)); // eslint-disable-line no-alert
-                this.props.onLoadingFinished(this.props.loadingState);
-                // Reset the file input after project is loaded
-                // This is necessary in case the user wants to reload a project
-                thisFileInput.value = null;
-            });
         if (thisFileInput.files) { // Don't attempt to load if no file was selected
+            this.fileToUpload = thisFileInput.files[0];
+            this.props.requestProjectUpload(this.props.loadingState);
+        }
+    }
+    // called when file upload raw data is available in the reader
+    onload () {
+        if (this.reader) {
             this.props.onLoadingStarted();
-            reader.readAsArrayBuffer(thisFileInput.files[0]);
-            const uploadedProjectTitle = this.getProjectTitleFromFilename(thisFileInput.files[0].name);
-            this.props.onUpdateProjectTitle(uploadedProjectTitle);
+            const filename = this.fileToUpload && this.fileToUpload.name;
+            this.props.vm.loadProject(this.reader.result)
+                .then(() => {
+                    analytics.event({
+                        category: 'project',
+                        action: 'Import Project File',
+                        nonInteraction: true
+                    });
+                    // Remove the hash if any (without triggering a hash change event or a reload)
+                    history.replaceState({}, document.title, '.');
+                    this.props.onLoadingFinished(this.props.loadingState, true);
+                    // Reset the file input after project is loaded
+                    // This is necessary in case the user wants to reload a project
+                    if (filename) {
+                        const uploadedProjectTitle = this.getProjectTitleFromFilename(filename);
+                        this.props.onUpdateProjectTitle(uploadedProjectTitle);
+                    }
+                    this.resetFileInput();
+                })
+                .catch(error => {
+                    log.warn(error);
+                    alert(this.props.intl.formatMessage(messages.loadError)); // eslint-disable-line no-alert
+                    this.props.onLoadingFinished(this.props.loadingState, false);
+                    // Reset the file input after project is loaded
+                    // This is necessary in case the user wants to reload a project
+                    this.resetFileInput();
+                });
         }
     }
     handleClick () {
@@ -119,10 +154,12 @@ SBFileUploader.propTypes = {
     children: PropTypes.func,
     className: PropTypes.string,
     intl: intlShape.isRequired,
+    isLoadingUpload: PropTypes.bool,
     loadingState: PropTypes.oneOf(LoadingStates),
     onLoadingFinished: PropTypes.func,
     onLoadingStarted: PropTypes.func,
     onUpdateProjectTitle: PropTypes.func,
+    requestProjectUpload: PropTypes.func,
     vm: PropTypes.shape({
         loadProject: PropTypes.func
     })
@@ -130,21 +167,23 @@ SBFileUploader.propTypes = {
 SBFileUploader.defaultProps = {
     className: ''
 };
-const mapStateToProps = state => ({
-    loadingState: state.scratchGui.projectState.loadingState,
-    vm: state.scratchGui.vm
-});
+const mapStateToProps = state => {
+    const loadingState = state.scratchGui.projectState.loadingState;
+    return {
+        isLoadingUpload: getIsLoadingUpload(loadingState),
+        loadingState: loadingState,
+        vm: state.scratchGui.vm
+    };
+};
 
 const mapDispatchToProps = (dispatch, ownProps) => ({
-    onLoadingFinished: loadingState => {
-        dispatch(onLoadedProject(loadingState, ownProps.canSave));
+    onLoadingFinished: (loadingState, success) => {
+        dispatch(onLoadedProject(loadingState, ownProps.canSave, success));
         dispatch(closeLoadingProject());
         dispatch(closeFileMenu());
     },
-    onLoadingStarted: () => {
-        dispatch(openLoadingProject());
-        dispatch(onProjectUploadStarted());
-    }
+    requestProjectUpload: loadingState => dispatch(requestProjectUpload(loadingState)),
+    onLoadingStarted: () => dispatch(openLoadingProject())
 });
 
 // Allow incoming props to override redux-provided props. Used to mock in tests.

src/lib/vm-manager-hoc.jsx

@@ -129,7 +129,7 @@ const vmManagerHOC = function (WrappedComponent) {
     const mapDispatchToProps = dispatch => ({
         onError: error => dispatch(projectError(error)),
         onLoadedProject: (loadingState, canSave) =>
-            dispatch(onLoadedProject(loadingState, canSave)),
+            dispatch(onLoadedProject(loadingState, canSave, true)),
         onSetProjectUnchanged: () => dispatch(setProjectUnchanged())
     });
 

src/reducers/project-state.js

@@ -10,7 +10,9 @@ const DONE_LOADING_VM_WITHOUT_ID = 'scratch-gui/project-state/DONE_LOADING_VM_WI
 const DONE_REMIXING = 'scratch-gui/project-state/DONE_REMIXING';
 const DONE_UPDATING = 'scratch-gui/project-state/DONE_UPDATING';
 const DONE_UPDATING_BEFORE_COPY = 'scratch-gui/project-state/DONE_UPDATING_BEFORE_COPY';
+const DONE_UPDATING_BEFORE_FILE_UPLOAD = 'scratch-gui/project-state/DONE_UPDATING_BEFORE_FILE_UPLOAD';
 const DONE_UPDATING_BEFORE_NEW = 'scratch-gui/project-state/DONE_UPDATING_BEFORE_NEW';
+const RETURN_TO_SHOWING = 'scratch-gui/project-state/RETURN_TO_SHOWING';
 const SET_PROJECT_ID = 'scratch-gui/project-state/SET_PROJECT_ID';
 const START_AUTO_UPDATING = 'scratch-gui/project-state/START_AUTO_UPDATING';
 const START_CREATING_NEW = 'scratch-gui/project-state/START_CREATING_NEW';
@@ -21,6 +23,7 @@ const START_MANUAL_UPDATING = 'scratch-gui/project-state/START_MANUAL_UPDATING';
 const START_REMIXING = 'scratch-gui/project-state/START_REMIXING';
 const START_UPDATING_BEFORE_CREATING_COPY = 'scratch-gui/project-state/START_UPDATING_BEFORE_CREATING_COPY';
 const START_UPDATING_BEFORE_CREATING_NEW = 'scratch-gui/project-state/START_UPDATING_BEFORE_CREATING_NEW';
+const START_UPDATING_BEFORE_FILE_UPLOAD = 'scratch-gui/project-state/START_UPDATING_BEFORE_FILE_UPLOAD';
 
 const defaultProjectId = '0'; // hardcoded id of default project
 
@@ -40,6 +43,7 @@ const LoadingState = keyMirror({
     SHOWING_WITH_ID: null,
     SHOWING_WITHOUT_ID: null,
     UPDATING_BEFORE_COPY: null,
+    UPDATING_BEFORE_FILE_UPLOAD: null,
     UPDATING_BEFORE_NEW: null
 });
 
@@ -63,6 +67,9 @@ const getIsLoading = loadingState => (
     loadingState === LoadingState.LOADING_VM_WITH_ID ||
     loadingState === LoadingState.LOADING_VM_NEW_DEFAULT
 );
+const getIsLoadingUpload = loadingState => (
+    loadingState === LoadingState.LOADING_VM_FILE_UPLOAD
+);
 const getIsCreatingNew = loadingState => (
     loadingState === LoadingState.CREATING_NEW
 );
@@ -84,6 +91,7 @@ const getIsUpdating = loadingState => (
     loadingState === LoadingState.AUTO_UPDATING ||
     loadingState === LoadingState.MANUAL_UPDATING ||
     loadingState === LoadingState.UPDATING_BEFORE_COPY ||
+    loadingState === LoadingState.UPDATING_BEFORE_FILE_UPLOAD ||
     loadingState === LoadingState.UPDATING_BEFORE_NEW
 );
 const getIsShowingProject = loadingState => (
@@ -141,7 +149,8 @@ const reducer = function (state, action) {
         if (state.loadingState === LoadingState.LOADING_VM_FILE_UPLOAD ||
             state.loadingState === LoadingState.LOADING_VM_NEW_DEFAULT) {
             return Object.assign({}, state, {
-                loadingState: LoadingState.SHOWING_WITHOUT_ID
+                loadingState: LoadingState.SHOWING_WITHOUT_ID,
+                projectId: defaultProjectId
             });
         }
         return state;
@@ -194,6 +203,13 @@ const reducer = function (state, action) {
             });
         }
         return state;
+    case DONE_UPDATING_BEFORE_FILE_UPLOAD:
+        if (state.loadingState === LoadingState.UPDATING_BEFORE_FILE_UPLOAD) {
+            return Object.assign({}, state, {
+                loadingState: LoadingState.LOADING_VM_FILE_UPLOAD
+            });
+        }
+        return state;
     case DONE_UPDATING_BEFORE_NEW:
         if (state.loadingState === LoadingState.UPDATING_BEFORE_NEW) {
             return Object.assign({}, state, {
@@ -202,6 +218,16 @@ const reducer = function (state, action) {
             });
         }
         return state;
+    case RETURN_TO_SHOWING:
+        if (state.projectId === null || state.projectId === defaultProjectId) {
+            return Object.assign({}, state, {
+                loadingState: LoadingState.SHOWING_WITHOUT_ID,
+                projectId: defaultProjectId
+            });
+        }
+        return Object.assign({}, state, {
+            loadingState: LoadingState.SHOWING_WITH_ID
+        });
     case SET_PROJECT_ID:
         // if the projectId hasn't actually changed do nothing
         if (state.projectId === action.projectId) {
@@ -275,8 +301,7 @@ const reducer = function (state, action) {
             LoadingState.SHOWING_WITHOUT_ID
         ].includes(state.loadingState)) {
             return Object.assign({}, state, {
-                loadingState: LoadingState.LOADING_VM_FILE_UPLOAD,
-                projectId: null // clear any current projectId
+                loadingState: LoadingState.LOADING_VM_FILE_UPLOAD
             });
         }
         return state;
@@ -308,6 +333,13 @@ const reducer = function (state, action) {
             });
         }
         return state;
+    case START_UPDATING_BEFORE_FILE_UPLOAD:
+        if (state.loadingState === LoadingState.SHOWING_WITH_ID) {
+            return Object.assign({}, state, {
+                loadingState: LoadingState.UPDATING_BEFORE_FILE_UPLOAD
+            });
+        }
+        return state;
     case START_ERROR:
         // fatal errors: there's no correct editor state for us to show
         if ([
@@ -328,6 +360,7 @@ const reducer = function (state, action) {
             LoadingState.MANUAL_UPDATING,
             LoadingState.REMIXING,
             LoadingState.UPDATING_BEFORE_COPY,
+            LoadingState.UPDATING_BEFORE_FILE_UPLOAD,
             LoadingState.UPDATING_BEFORE_NEW
         ].includes(state.loadingState)) {
             return Object.assign({}, state, {
@@ -398,28 +431,33 @@ const onFetchedProjectData = (projectData, loadingState) => {
     }
 };
 
-const onLoadedProject = (loadingState, canSave) => {
-    switch (loadingState) {
-    case LoadingState.LOADING_VM_WITH_ID:
-        return {
-            type: DONE_LOADING_VM_WITH_ID
-        };
-    case LoadingState.LOADING_VM_FILE_UPLOAD:
-        if (canSave) {
+const onLoadedProject = (loadingState, canSave, success) => {
+    if (success) {
+        switch (loadingState) {
+        case LoadingState.LOADING_VM_WITH_ID:
+            return {
+                type: DONE_LOADING_VM_WITH_ID
+            };
+        case LoadingState.LOADING_VM_FILE_UPLOAD:
+            if (canSave) {
+                return {
+                    type: DONE_LOADING_VM_TO_SAVE
+                };
+            }
             return {
-                type: DONE_LOADING_VM_TO_SAVE
+                type: DONE_LOADING_VM_WITHOUT_ID
             };
+        case LoadingState.LOADING_VM_NEW_DEFAULT:
+            return {
+                type: DONE_LOADING_VM_WITHOUT_ID
+            };
+        default:
+            break;
         }
-        return {
-            type: DONE_LOADING_VM_WITHOUT_ID
-        };
-    case LoadingState.LOADING_VM_NEW_DEFAULT:
-        return {
-            type: DONE_LOADING_VM_WITHOUT_ID
-        };
-    default:
-        break;
     }
+    return {
+        type: RETURN_TO_SHOWING
+    };
 };
 
 const doneUpdatingProject = loadingState => {
@@ -433,6 +471,10 @@ const doneUpdatingProject = loadingState => {
         return {
             type: DONE_UPDATING_BEFORE_COPY
         };
+    case LoadingState.UPDATING_BEFORE_FILE_UPLOAD:
+        return {
+            type: DONE_UPDATING_BEFORE_FILE_UPLOAD
+        };
     case LoadingState.UPDATING_BEFORE_NEW:
         return {
             type: DONE_UPDATING_BEFORE_NEW
@@ -457,9 +499,21 @@ const requestNewProject = needSave => {
     return {type: START_FETCHING_NEW};
 };
 
-const onProjectUploadStarted = () => ({
-    type: START_LOADING_VM_FILE_UPLOAD
-});
+const requestProjectUpload = loadingState => {
+    switch (loadingState) {
+    case LoadingState.SHOWING_WITH_ID:
+        return {
+            type: START_UPDATING_BEFORE_FILE_UPLOAD
+        };
+    case LoadingState.NOT_LOADED:
+    case LoadingState.SHOWING_WITHOUT_ID:
+        return {
+            type: START_LOADING_VM_FILE_UPLOAD
+        };
+    default:
+        break;
+    }
+};
 
 const autoUpdateProject = () => ({
     type: START_AUTO_UPDATING
@@ -495,6 +549,7 @@ export {
     getIsFetchingWithoutId,
     getIsLoading,
     getIsLoadingWithId,
+    getIsLoadingUpload,
     getIsManualUpdating,
     getIsRemixing,
     getIsShowingProject,
@@ -504,10 +559,10 @@ export {
     manualUpdateProject,
     onFetchedProjectData,
     onLoadedProject,
-    onProjectUploadStarted,
     projectError,
     remixProject,
     requestNewProject,
+    requestProjectUpload,
     saveProjectAsCopy,
     setProjectId
 };

test/unit/reducers/project-state-reducer.test.js

@@ -8,10 +8,10 @@ import {
     manualUpdateProject,
     onFetchedProjectData,
     onLoadedProject,
-    onProjectUploadStarted,
     projectError,
     remixProject,
     requestNewProject,
+    requestProjectUpload,
     saveProjectAsCopy,
     setProjectId
 } from '../../../src/reducers/project-state';
@@ -96,7 +96,7 @@ test('onLoadedProject upload, with canSave false, shows without id', () => {
     const initialState = {
         loadingState: LoadingState.LOADING_VM_FILE_UPLOAD
     };
-    const action = onLoadedProject(initialState.loadingState, false);
+    const action = onLoadedProject(initialState.loadingState, false, true);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID);
 });
@@ -105,7 +105,7 @@ test('onLoadedProject upload, with canSave true, prepares to save', () => {
     const initialState = {
         loadingState: LoadingState.LOADING_VM_FILE_UPLOAD
     };
-    const action = onLoadedProject(initialState.loadingState, true);
+    const action = onLoadedProject(initialState.loadingState, true, true);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.AUTO_UPDATING);
 });
@@ -114,7 +114,7 @@ test('onLoadedProject with id shows with id', () => {
     const initialState = {
         loadingState: LoadingState.LOADING_VM_WITH_ID
     };
-    const action = onLoadedProject(initialState.loadingState);
+    const action = onLoadedProject(initialState.loadingState, true, true);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID);
 });
@@ -123,7 +123,7 @@ test('onLoadedProject new shows without id', () => {
     const initialState = {
         loadingState: LoadingState.LOADING_VM_NEW_DEFAULT
     };
-    const action = onLoadedProject(initialState.loadingState);
+    const action = onLoadedProject(initialState.loadingState, false, true);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID);
 });
@@ -132,11 +132,31 @@ test('onLoadedProject new, to save shows without id', () => {
     const initialState = {
         loadingState: LoadingState.LOADING_VM_NEW_DEFAULT
     };
-    const action = onLoadedProject(initialState.loadingState);
+    const action = onLoadedProject(initialState.loadingState, true, true);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID);
 });
 
+test('onLoadedProject with success false, no project id, shows without id', () => {
+    const initialState = {
+        loadingState: LoadingState.LOADING_VM_WITH_ID,
+        projectId: null
+    };
+    const action = onLoadedProject(initialState.loadingState, false, false);
+    const resultState = projectStateReducer(initialState, action);
+    expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID);
+});
+
+test('onLoadedProject with success false, valid project id, shows with id', () => {
+    const initialState = {
+        loadingState: LoadingState.LOADING_VM_WITH_ID,
+        projectId: '12345'
+    };
+    const action = onLoadedProject(initialState.loadingState, false, false);
+    const resultState = projectStateReducer(initialState, action);
+    expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID);
+});
+
 test('doneUpdatingProject with id shows with id', () => {
     const initialState = {
         loadingState: LoadingState.MANUAL_UPDATING
@@ -215,29 +235,29 @@ test('requestNewProject, when can create new, should save and prepare to fetch d
     expect(resultState.loadingState).toBe(LoadingState.UPDATING_BEFORE_NEW);
 });
 
-test('onProjectUploadStarted when project not loaded should load', () => {
+test('requestProjectUpload when project not loaded should load', () => {
     const initialState = {
         loadingState: LoadingState.NOT_LOADED
     };
-    const action = onProjectUploadStarted();
+    const action = requestProjectUpload(initialState.loadingState);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.LOADING_VM_FILE_UPLOAD);
 });
 
-test('onProjectUploadStarted when showing project with id should load', () => {
+test('requestProjectUpload when showing project with id should load', () => {
     const initialState = {
         loadingState: LoadingState.SHOWING_WITH_ID
     };
-    const action = onProjectUploadStarted();
+    const action = requestProjectUpload(initialState.loadingState);
     const resultState = projectStateReducer(initialState, action);
-    expect(resultState.loadingState).toBe(LoadingState.LOADING_VM_FILE_UPLOAD);
+    expect(resultState.loadingState).toBe(LoadingState.UPDATING_BEFORE_FILE_UPLOAD);
 });
 
-test('onProjectUploadStarted when showing project without id should load', () => {
+test('requestProjectUpload when showing project without id should load', () => {
     const initialState = {
         loadingState: LoadingState.SHOWING_WITHOUT_ID
     };
-    const action = onProjectUploadStarted();
+    const action = requestProjectUpload(initialState.loadingState);
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.LOADING_VM_FILE_UPLOAD);
 });