From b0c4bee84b3d38b267c5c8052d57829ecf1ac978 Mon Sep 17 00:00:00 2001 From: Ben Wheeler <wheeler.benjamin@gmail.com> Date: Thu, 18 Oct 2018 16:00:12 -0400 Subject: [PATCH] handle many modes of new project creation responding to canSave without needing canSaveNew prop removed canSaveNew added default for onUpdateProjectId removed conflict files erroneously added don't pass down onError got tests passing better creating and saving logic and names --- src/components/menu-bar/menu-bar.jsx | 1 - src/containers/gui.jsx | 87 ++++-- src/containers/sb-file-uploader.jsx | 5 +- src/lib/project-fetcher-hoc.jsx | 3 +- src/lib/project-saver-hoc.jsx | 43 ++- src/lib/vm-manager-hoc.jsx | 12 +- src/reducers/project-state.js | 111 ++++---- .../reducers/project-state-reducer.test.js | 53 ++-- test/unit/util/project-saver-hoc.test.jsx | 257 ++++++++++++++++++ test/unit/util/vm-manager-hoc.test.jsx | 5 +- 10 files changed, 458 insertions(+), 119 deletions(-) create mode 100644 test/unit/util/project-saver-hoc.test.jsx diff --git a/src/components/menu-bar/menu-bar.jsx b/src/components/menu-bar/menu-bar.jsx index f046710e0..b0c831a05 100644 --- a/src/components/menu-bar/menu-bar.jsx +++ b/src/components/menu-bar/menu-bar.jsx @@ -656,7 +656,6 @@ MenuBar.propTypes = { onUpdateProjectTitle: PropTypes.func, renderLogin: PropTypes.func, sessionExists: PropTypes.bool, - startSaving: PropTypes.func, username: PropTypes.string }; diff --git a/src/containers/gui.jsx b/src/containers/gui.jsx index 49b29672d..8a89fb132 100644 --- a/src/containers/gui.jsx +++ b/src/containers/gui.jsx @@ -4,9 +4,13 @@ import {compose} from 'redux'; import {connect} from 'react-redux'; import ReactModal from 'react-modal'; import VM from 'scratch-vm'; +import {injectIntl, intlShape} from 'react-intl'; import ErrorBoundaryHOC from '../lib/error-boundary-hoc.jsx'; import {openExtensionLibrary} from '../reducers/modals'; +import { + getIsShowingProject +} from '../reducers/project-state'; import {setProjectTitle} from '../reducers/project-title'; import { activateTab, @@ -25,18 +29,29 @@ import ProjectFetcherHOC from '../lib/project-fetcher-hoc.jsx'; import ProjectSaverHOC from '../lib/project-saver-hoc.jsx'; import vmListenerHOC from '../lib/vm-listener-hoc.jsx'; import vmManagerHOC from '../lib/vm-manager-hoc.jsx'; +import {defaultProjectTitleMessages} from '../reducers/project-title'; import GUIComponent from '../components/gui/gui.jsx'; class GUI extends React.Component { componentDidMount () { - if (this.props.projectTitle) { - this.props.onUpdateReduxProjectTitle(this.props.projectTitle); + this.setReduxTitle(this.props.projectTitle); + } + componentDidUpdate (prevProps) { + if (this.props.projectId !== prevProps.projectId && this.props.projectId !== null) { + this.props.onUpdateProjectId(this.props.projectId); + } + if (this.props.projectTitle !== prevProps.projectTitle) { + this.setReduxTitle(this.props.projectTitle); } } - componentWillReceiveProps (nextProps) { - if (this.props.projectTitle !== nextProps.projectTitle) { - this.props.onUpdateReduxProjectTitle(nextProps.projectTitle); + setReduxTitle (newTitle) { + if (newTitle === null || typeof newTitle === 'undefined') { + this.props.onUpdateReduxProjectTitle( + this.props.intl.formatMessage(defaultProjectTitleMessages.defaultProjectTitle) + ); + } else { + this.props.onUpdateReduxProjectTitle(newTitle); } } render () { @@ -50,8 +65,11 @@ class GUI extends React.Component { errorMessage, hideIntro, loadingError, + isShowingProject, + onUpdateProjectId, onUpdateReduxProjectTitle, projectHost, + projectId, projectTitle, /* eslint-enable no-unused-vars */ children, @@ -78,40 +96,53 @@ GUI.propTypes = { fetchingProject: PropTypes.bool, hideIntro: PropTypes.bool, importInfoVisible: PropTypes.bool, + intl: intlShape, isLoading: PropTypes.bool, + isShowingProject: PropTypes.bool, loadingError: PropTypes.bool, loadingStateVisible: PropTypes.bool, onChangeProjectInfo: PropTypes.func, onSeeCommunity: PropTypes.func, + onUpdateProjectId: PropTypes.func, onUpdateProjectTitle: PropTypes.func, onUpdateReduxProjectTitle: PropTypes.func, previewInfoVisible: PropTypes.bool, projectHost: PropTypes.string, + projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), projectTitle: PropTypes.string, vm: PropTypes.instanceOf(VM).isRequired }; -const mapStateToProps = (state, ownProps) => ({ - activeTabIndex: state.scratchGui.editorTab.activeTabIndex, - alertsVisible: state.scratchGui.alerts.visible, - backdropLibraryVisible: state.scratchGui.modals.backdropLibrary, - blocksTabVisible: state.scratchGui.editorTab.activeTabIndex === BLOCKS_TAB_INDEX, - cardsVisible: state.scratchGui.cards.visible, - costumeLibraryVisible: state.scratchGui.modals.costumeLibrary, - costumesTabVisible: state.scratchGui.editorTab.activeTabIndex === COSTUMES_TAB_INDEX, - importInfoVisible: state.scratchGui.modals.importInfo, - isPlayerOnly: state.scratchGui.mode.isPlayerOnly, - isRtl: state.locales.isRtl, - loadingStateVisible: state.scratchGui.modals.loadingProject, - previewInfoVisible: state.scratchGui.modals.previewInfo && !ownProps.hideIntro, - targetIsStage: ( - state.scratchGui.targets.stage && - state.scratchGui.targets.stage.id === state.scratchGui.targets.editingTarget - ), - soundsTabVisible: state.scratchGui.editorTab.activeTabIndex === SOUNDS_TAB_INDEX, - tipsLibraryVisible: state.scratchGui.modals.tipsLibrary, - vm: state.scratchGui.vm -}); +GUI.defaultProps = { + onUpdateProjectId: () => {} +}; + +const mapStateToProps = (state, ownProps) => { + const loadingState = state.scratchGui.projectState.loadingState; + return { + activeTabIndex: state.scratchGui.editorTab.activeTabIndex, + alertsVisible: state.scratchGui.alerts.visible, + backdropLibraryVisible: state.scratchGui.modals.backdropLibrary, + blocksTabVisible: state.scratchGui.editorTab.activeTabIndex === BLOCKS_TAB_INDEX, + cardsVisible: state.scratchGui.cards.visible, + costumeLibraryVisible: state.scratchGui.modals.costumeLibrary, + costumesTabVisible: state.scratchGui.editorTab.activeTabIndex === COSTUMES_TAB_INDEX, + importInfoVisible: state.scratchGui.modals.importInfo, + isPlayerOnly: state.scratchGui.mode.isPlayerOnly, + isRtl: state.locales.isRtl, + isShowingProject: getIsShowingProject(loadingState), + loadingStateVisible: state.scratchGui.modals.loadingProject, + previewInfoVisible: state.scratchGui.modals.previewInfo && !ownProps.hideIntro, + projectId: state.scratchGui.projectState.projectId, + targetIsStage: ( + state.scratchGui.targets.stage && + state.scratchGui.targets.stage.id === state.scratchGui.targets.editingTarget + ), + soundsTabVisible: state.scratchGui.editorTab.activeTabIndex === SOUNDS_TAB_INDEX, + tipsLibraryVisible: state.scratchGui.modals.tipsLibrary, + vm: state.scratchGui.vm + }; +}; const mapDispatchToProps = dispatch => ({ onExtensionButtonClick: () => dispatch(openExtensionLibrary()), @@ -123,10 +154,10 @@ const mapDispatchToProps = dispatch => ({ onUpdateReduxProjectTitle: title => dispatch(setProjectTitle(title)) }); -const ConnectedGUI = connect( +const ConnectedGUI = injectIntl(connect( mapStateToProps, mapDispatchToProps, -)(GUI); +)(GUI)); // note that redux's 'compose' function is just being used as a general utility to make // the hierarchy of HOC constructor calls clearer here; it has nothing to do with redux's diff --git a/src/containers/sb-file-uploader.jsx b/src/containers/sb-file-uploader.jsx index 46be81724..ffa4e0122 100644 --- a/src/containers/sb-file-uploader.jsx +++ b/src/containers/sb-file-uploader.jsx @@ -112,6 +112,7 @@ class SBFileUploader extends React.Component { } SBFileUploader.propTypes = { + canSave: PropTypes.bool, // eslint-disable-line react/no-unused-prop-types children: PropTypes.func, intl: intlShape.isRequired, loadingState: PropTypes.oneOf(LoadingStates), @@ -127,9 +128,9 @@ const mapStateToProps = state => ({ vm: state.scratchGui.vm }); -const mapDispatchToProps = dispatch => ({ +const mapDispatchToProps = (dispatch, ownProps) => ({ onLoadingFinished: loadingState => { - dispatch(onLoadedProject(loadingState)); + dispatch(onLoadedProject(loadingState, ownProps.canSave)); dispatch(closeLoadingProject()); }, onLoadingStarted: () => { diff --git a/src/lib/project-fetcher-hoc.jsx b/src/lib/project-fetcher-hoc.jsx index 884a77cfc..6a29cf75d 100644 --- a/src/lib/project-fetcher-hoc.jsx +++ b/src/lib/project-fetcher-hoc.jsx @@ -7,8 +7,8 @@ import {connect} from 'react-redux'; import { LoadingStates, defaultProjectId, - onFetchedProjectData, getIsFetchingWithId, + onFetchedProjectData, setProjectId } from '../reducers/project-state'; @@ -102,6 +102,7 @@ const ProjectFetcherHOC = function (WrappedComponent) { } ProjectFetcherComponent.propTypes = { assetHost: PropTypes.string, + canSave: PropTypes.bool, intl: intlShape.isRequired, isFetchingWithId: PropTypes.bool, loadingState: PropTypes.oneOf(LoadingStates), diff --git a/src/lib/project-saver-hoc.jsx b/src/lib/project-saver-hoc.jsx index 83ccd12c1..5bc15f1d5 100644 --- a/src/lib/project-saver-hoc.jsx +++ b/src/lib/project-saver-hoc.jsx @@ -6,11 +6,15 @@ import VM from 'scratch-vm'; import storage from '../lib/storage'; import { LoadingStates, + createProject, getIsCreating, + getIsShowingProject, + getIsShowingWithoutId, getIsUpdating, onCreated, onUpdated, - onError + onError, + saveProject } from '../reducers/project-state'; /** @@ -27,7 +31,7 @@ const ProjectSaverHOC = function (WrappedComponent) { componentDidUpdate (prevProps) { if (this.props.isUpdating && !prevProps.isUpdating) { this.storeProject(this.props.reduxProjectId) - .then(() => { // eslint-disable-line no-unused-vars + .then(() => { // there is nothing we expect to find in response that we need to check here this.props.onUpdated(this.props.loadingState); }) @@ -46,6 +50,19 @@ const ProjectSaverHOC = function (WrappedComponent) { this.props.onError(`Creating a new project failed with error: ${err}`); }); } + // if this is the first time we're able to create this project on the server, create it! + const showingCreateable = this.props.canSave && this.props.isShowingWithoutId; + const prevShowingCreateable = prevProps.canSave && prevProps.isShowingWithoutId; + if (showingCreateable && !prevShowingCreateable) { + this.props.createProject(); + } 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; + if (showingSaveable && becameAbleToSave) { + this.props.saveProject(); + } + } } /** * storeProject: @@ -72,13 +89,17 @@ const ProjectSaverHOC = function (WrappedComponent) { render () { const { /* eslint-disable no-unused-vars */ + createProject: createProjectProp, onCreated: onCreatedProp, onUpdated: onUpdatedProp, onError: onErrorProp, isCreating: isCreatingProp, + isShowingWithId: isShowingWithIdProp, + isShowingWithoutId: isShowingWithoutIdProp, isUpdating: isUpdatingProp, loadingState, reduxProjectId, + saveProject: saveProjectProp, /* eslint-enable no-unused-vars */ ...componentProps } = this.props; @@ -90,19 +111,26 @@ const ProjectSaverHOC = function (WrappedComponent) { } } ProjectSaverComponent.propTypes = { + canSave: PropTypes.bool, + createProject: PropTypes.func, isCreating: PropTypes.bool, + isShowingWithId: PropTypes.bool, + isShowingWithoutId: PropTypes.bool, isUpdating: PropTypes.bool, loadingState: PropTypes.oneOf(LoadingStates), onCreated: PropTypes.func, onError: PropTypes.func, onUpdated: PropTypes.func, reduxProjectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), + saveProject: PropTypes.func, vm: PropTypes.instanceOf(VM).isRequired }; const mapStateToProps = state => { const loadingState = state.scratchGui.projectState.loadingState; return { isCreating: getIsCreating(loadingState), + isShowingWithId: getIsShowingProject(loadingState), + isShowingWithoutId: getIsShowingWithoutId(loadingState), isUpdating: getIsUpdating(loadingState), loadingState: loadingState, reduxProjectId: state.scratchGui.projectState.projectId, @@ -110,13 +138,20 @@ const ProjectSaverHOC = function (WrappedComponent) { }; }; const mapDispatchToProps = dispatch => ({ + createProject: () => dispatch(createProject()), onCreated: projectId => dispatch(onCreated(projectId)), onUpdated: (projectId, loadingState) => dispatch(onUpdated(projectId, loadingState)), - onError: errStr => dispatch(onError(errStr)) + onError: errStr => dispatch(onError(errStr)), + saveProject: () => dispatch(saveProject()) }); + // Allow incoming props to override redux-provided props. Used to mock in tests. + const mergeProps = (stateProps, dispatchProps, ownProps) => Object.assign( + {}, stateProps, dispatchProps, ownProps + ); return connect( mapStateToProps, - mapDispatchToProps + mapDispatchToProps, + mergeProps )(ProjectSaverComponent); }; diff --git a/src/lib/vm-manager-hoc.jsx b/src/lib/vm-manager-hoc.jsx index 56bd7eabf..cfc80bc97 100644 --- a/src/lib/vm-manager-hoc.jsx +++ b/src/lib/vm-manager-hoc.jsx @@ -42,13 +42,13 @@ const vmManagerHOC = function (WrappedComponent) { // and they weren't both that way until now... load project! if (this.props.isLoadingWithId && this.props.fontsLoaded && (!prevProps.isLoadingWithId || !prevProps.fontsLoaded)) { - this.loadProject(this.props.projectData, this.props.loadingState); + this.loadProject(); } } - loadProject (projectData, loadingState) { - return this.props.vm.loadProject(projectData) + loadProject () { + return this.props.vm.loadProject(this.props.projectData) .then(() => { - this.props.onLoadedProject(loadingState); + this.props.onLoadedProject(this.props.loadingState, this.props.canSave); }) .catch(e => { // Need to catch this error and update component state so that @@ -82,6 +82,7 @@ const vmManagerHOC = function (WrappedComponent) { } VMManager.propTypes = { + canSave: PropTypes.bool, fontsLoaded: PropTypes.bool, isLoadingWithId: PropTypes.bool, loadingState: PropTypes.oneOf(LoadingStates), @@ -102,7 +103,8 @@ const vmManagerHOC = function (WrappedComponent) { }; const mapDispatchToProps = dispatch => ({ - onLoadedProject: loadingState => dispatch(onLoadedProject(loadingState)) + onLoadedProject: (loadingState, canSave) => + dispatch(onLoadedProject(loadingState, canSave)) }); // Allow incoming props to override redux-provided props. Used to mock in tests. diff --git a/src/reducers/project-state.js b/src/reducers/project-state.js index 56e5bb128..de171edc5 100644 --- a/src/reducers/project-state.js +++ b/src/reducers/project-state.js @@ -1,18 +1,17 @@ import keyMirror from 'keymirror'; +const START_CREATING_NEW = 'scratch-gui/project-state/START_CREATING_NEW'; const DONE_CREATING_NEW = 'scratch-gui/project-state/DONE_CREATING_NEW'; const DONE_FETCHING_WITH_ID = 'scratch-gui/project-state/DONE_FETCHING_WITH_ID'; const DONE_FETCHING_DEFAULT = 'scratch-gui/project-state/DONE_FETCHING_DEFAULT'; -const DONE_FETCHING_DEFAULT_TO_SAVE = 'scratch-gui/project-state/DONE_FETCHING_DEFAULT_TO_SAVE'; const DONE_LOADING_VM_WITH_ID = 'scratch-gui/project-state/DONE_LOADING_VM_WITH_ID'; -const DONE_LOADING_VM_NEW_DEFAULT = 'scratch-gui/project-state/DONE_LOADING_VM_NEW_DEFAULT'; -const DONE_LOADING_VM_NEW_DEFAULT_TO_SAVE = 'scratch-gui/project-state/DONE_LOADING_VM_NEW_DEFAULT_TO_SAVE'; -const DONE_LOADING_VM_FILE_UPLOAD = 'scratch-gui/project-state/DONE_LOADING_VM_FILE_UPLOAD'; +const DONE_LOADING_VM_WITHOUT_ID = 'scratch-gui/project-state/DONE_LOADING_VM_WITHOUT_ID'; +const DONE_LOADING_VM_TO_SAVE = 'scratch-gui/project-state/DONE_LOADING_VM_TO_SAVE'; const DONE_SAVING_WITH_ID = 'scratch-gui/project-state/DONE_SAVING_WITH_ID'; const DONE_SAVING_WITH_ID_BEFORE_NEW = 'scratch-gui/project-state/DONE_SAVING_WITH_ID_BEFORE_NEW'; const GO_TO_ERROR_STATE = 'scratch-gui/project-state/GO_TO_ERROR_STATE'; const SET_PROJECT_ID = 'scratch-gui/project-state/SET_PROJECT_ID'; -const START_FETCHING_NEW_WITHOUT_SAVING = 'scratch-gui/project-state/START_FETCHING_NEW_WITHOUT_SAVING'; +const START_FETCHING_NEW = 'scratch-gui/project-state/START_FETCHING_NEW'; const START_LOADING_VM_FILE_UPLOAD = 'scratch-gui/project-state/START_LOADING_FILE_UPLOAD'; const START_SAVING = 'scratch-gui/project-state/START_SAVING'; const START_SAVING_BEFORE_CREATING_NEW = 'scratch-gui/project-state/START_SAVING_BEFORE_CREATING_NEW'; @@ -24,11 +23,9 @@ const LoadingState = keyMirror({ ERROR: null, FETCHING_WITH_ID: null, FETCHING_NEW_DEFAULT: null, - FETCHING_NEW_DEFAULT_TO_SAVE: null, LOADING_VM_WITH_ID: null, LOADING_VM_FILE_UPLOAD: null, LOADING_VM_NEW_DEFAULT: null, - LOADING_VM_NEW_DEFAULT_TO_SAVE: null, SHOWING_WITH_ID: null, SHOWING_WITHOUT_ID: null, SAVING_WITH_ID: null, @@ -41,18 +38,15 @@ const LoadingStates = Object.keys(LoadingState); const getIsFetchingWithoutId = loadingState => ( // LOADING_VM_FILE_UPLOAD is an honorary fetch, since there is no fetching step for file uploads loadingState === LoadingState.LOADING_VM_FILE_UPLOAD || - loadingState === LoadingState.FETCHING_NEW_DEFAULT || - loadingState === LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE + loadingState === LoadingState.FETCHING_NEW_DEFAULT ); const getIsFetchingWithId = loadingState => ( loadingState === LoadingState.FETCHING_WITH_ID || - loadingState === LoadingState.FETCHING_NEW_DEFAULT || - loadingState === LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE + loadingState === LoadingState.FETCHING_NEW_DEFAULT ); const getIsLoadingWithId = loadingState => ( loadingState === LoadingState.LOADING_VM_WITH_ID || - loadingState === LoadingState.LOADING_VM_NEW_DEFAULT || - loadingState === LoadingState.LOADING_VM_NEW_DEFAULT_TO_SAVE + loadingState === LoadingState.LOADING_VM_NEW_DEFAULT ); const getIsCreating = loadingState => ( loadingState === LoadingState.CREATING_NEW @@ -65,6 +59,12 @@ const getIsShowingProject = loadingState => ( loadingState === LoadingState.SHOWING_WITH_ID || loadingState === LoadingState.SHOWING_WITHOUT_ID ); +const getIsShowingWithId = loadingState => ( + loadingState === LoadingState.SHOWING_WITH_ID +); +const getIsShowingWithoutId = loadingState => ( + loadingState === LoadingState.SHOWING_WITHOUT_ID +); const initialState = { errStr: null, @@ -103,18 +103,9 @@ const reducer = function (state, action) { }); } return state; - case DONE_FETCHING_DEFAULT_TO_SAVE: - if (state.loadingState === LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE) { - return Object.assign({}, state, { - loadingState: LoadingState.LOADING_VM_NEW_DEFAULT_TO_SAVE, - projectData: action.projectData - }); - } - return state; - case DONE_LOADING_VM_FILE_UPLOAD: - // note that we don't need to explicitly set projectData, because it is loaded - // into the vm directly in file-loader-from-local - if (state.loadingState === LoadingState.LOADING_VM_FILE_UPLOAD) { + case DONE_LOADING_VM_WITHOUT_ID: + if (state.loadingState === LoadingState.LOADING_VM_FILE_UPLOAD || + state.loadingState === LoadingState.LOADING_VM_NEW_DEFAULT) { return Object.assign({}, state, { loadingState: LoadingState.SHOWING_WITHOUT_ID }); @@ -127,20 +118,10 @@ const reducer = function (state, action) { }); } return state; - case DONE_LOADING_VM_NEW_DEFAULT: - if (state.loadingState === LoadingState.LOADING_VM_NEW_DEFAULT) { - return Object.assign({}, state, { - loadingState: LoadingState.SHOWING_WITHOUT_ID - }); - } - return state; - case DONE_LOADING_VM_NEW_DEFAULT_TO_SAVE: - if (state.loadingState === LoadingState.LOADING_VM_NEW_DEFAULT_TO_SAVE) { + case DONE_LOADING_VM_TO_SAVE: + if (state.loadingState === LoadingState.LOADING_VM_FILE_UPLOAD) { return Object.assign({}, state, { - // NOTE: this is set to skip over sending a POST to create the new project - // on the server, until we can get that working on the backend. - // loadingState: LoadingState.CREATING_NEW - loadingState: LoadingState.SHOWING_WITH_ID + loadingState: LoadingState.SAVING_WITH_ID }); } return state; @@ -154,12 +135,19 @@ const reducer = function (state, action) { case DONE_SAVING_WITH_ID_BEFORE_NEW: if (state.loadingState === LoadingState.SAVING_WITH_ID_BEFORE_NEW) { return Object.assign({}, state, { - loadingState: LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE, + loadingState: LoadingState.FETCHING_NEW_DEFAULT, projectId: defaultProjectId }); } return state; case SET_PROJECT_ID: + // if setting the default project id, specifically fetch that project + if (action.id === defaultProjectId) { + return Object.assign({}, state, { + loadingState: LoadingState.FETCHING_NEW_DEFAULT, + projectId: defaultProjectId + }); + } // if we were already showing a project, and a different projectId is set, only fetch that project if // projectId has changed. This prevents re-fetching projects unnecessarily. if (state.loadingState === LoadingState.SHOWING_WITH_ID) { @@ -176,7 +164,14 @@ const reducer = function (state, action) { }); } return state; - case START_FETCHING_NEW_WITHOUT_SAVING: + case START_CREATING_NEW: + if (state.loadingState === LoadingState.SHOWING_WITHOUT_ID) { + return Object.assign({}, state, { + loadingState: LoadingState.CREATING_NEW + }); + } + return state; + case START_FETCHING_NEW: if ([ LoadingState.SHOWING_WITH_ID, LoadingState.SHOWING_WITHOUT_ID @@ -216,10 +211,13 @@ const reducer = function (state, action) { case GO_TO_ERROR_STATE: // NOTE: we should introduce handling in components for showing ERROR state if ([ - LoadingState.NOT_LOADED, + LoadingState.LOADING_VM_NEW_DEFAULT, + LoadingState.LOADING_VM_WITH_ID, LoadingState.FETCHING_WITH_ID, LoadingState.FETCHING_NEW_DEFAULT, - LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE + LoadingState.SAVING_WITH_ID, + LoadingState.SAVING_WITH_ID_BEFORE_NEW, + LoadingState.CREATING_NEW ].includes(state.loadingState)) { return Object.assign({}, state, { loadingState: LoadingState.ERROR, @@ -232,6 +230,10 @@ const reducer = function (state, action) { } }; +const createProject = () => ({ + type: START_CREATING_NEW +}); + const onCreated = id => ({ type: DONE_CREATING_NEW, id: id @@ -249,33 +251,29 @@ const onFetchedProjectData = (projectData, loadingState) => { type: DONE_FETCHING_DEFAULT, projectData: projectData }; - case LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE: - return { - type: DONE_FETCHING_DEFAULT_TO_SAVE, - projectData: projectData - }; default: break; } }; -const onLoadedProject = 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) { + return { + type: DONE_LOADING_VM_TO_SAVE + }; + } return { - type: DONE_LOADING_VM_FILE_UPLOAD + type: DONE_LOADING_VM_WITHOUT_ID }; case LoadingState.LOADING_VM_NEW_DEFAULT: return { - type: DONE_LOADING_VM_NEW_DEFAULT - }; - case LoadingState.LOADING_VM_NEW_DEFAULT_TO_SAVE: - return { - type: DONE_LOADING_VM_NEW_DEFAULT_TO_SAVE + type: DONE_LOADING_VM_WITHOUT_ID }; default: break; @@ -309,7 +307,7 @@ const setProjectId = id => ({ const requestNewProject = canSave => { if (canSave) return {type: START_SAVING_BEFORE_CREATING_NEW}; - return {type: START_FETCHING_NEW_WITHOUT_SAVING}; + return {type: START_FETCHING_NEW}; }; const onProjectUploadStarted = () => ({ @@ -325,6 +323,7 @@ export { initialState as projectStateInitialState, LoadingState, LoadingStates, + createProject, defaultProjectId, getIsCreating, getIsFetchingWithoutId, @@ -332,6 +331,8 @@ export { getIsLoadingWithId, getIsUpdating, getIsShowingProject, + getIsShowingWithId, + getIsShowingWithoutId, onCreated, onError, onFetchedProjectData, diff --git a/test/unit/reducers/project-state-reducer.test.js b/test/unit/reducers/project-state-reducer.test.js index 920999ffc..c91d1909c 100644 --- a/test/unit/reducers/project-state-reducer.test.js +++ b/test/unit/reducers/project-state-reducer.test.js @@ -67,24 +67,22 @@ test('onFetchedProjectData new loads project data into vm', () => { expect(resultState.projectData).toBe('1010101'); }); -test('onFetchedProjectData new, to save loads project data into vm, prepares to save next', () => { +test('onLoadedProject upload, with canSave false, shows without id', () => { const initialState = { - projectData: null, - loadingState: LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE + loadingState: LoadingState.LOADING_VM_FILE_UPLOAD }; - const action = onFetchedProjectData('1010101', initialState.loadingState); + const action = onLoadedProject(initialState.loadingState, false); const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.LOADING_VM_NEW_DEFAULT_TO_SAVE); - expect(resultState.projectData).toBe('1010101'); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -test('onLoadedProject upload shows without id', () => { +test('onLoadedProject upload, with canSave true, prepares to save', () => { const initialState = { loadingState: LoadingState.LOADING_VM_FILE_UPLOAD }; - const action = onLoadedProject(initialState.loadingState); + const action = onLoadedProject(initialState.loadingState, true); const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); + expect(resultState.loadingState).toBe(LoadingState.SAVING_WITH_ID); }); test('onLoadedProject with id shows with id', () => { @@ -105,13 +103,13 @@ test('onLoadedProject new shows without id', () => { expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); -test('onLoadedProject new, to save shows with id', () => { +test('onLoadedProject new, to save shows without id', () => { const initialState = { - loadingState: LoadingState.LOADING_VM_NEW_DEFAULT_TO_SAVE + loadingState: LoadingState.LOADING_VM_NEW_DEFAULT }; const action = onLoadedProject(initialState.loadingState); const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITH_ID); + expect(resultState.loadingState).toBe(LoadingState.SHOWING_WITHOUT_ID); }); test('onUpdated with id shows with id', () => { @@ -129,7 +127,7 @@ test('onUpdated with id, before new, fetches default project', () => { }; const action = onUpdated(initialState.loadingState); const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.FETCHING_NEW_DEFAULT_TO_SAVE); + expect(resultState.loadingState).toBe(LoadingState.FETCHING_NEW_DEFAULT); }); test('setProjectId, with same id as before, should show with id, not fetch', () => { @@ -219,15 +217,26 @@ test('saveProject should prepare to save', () => { expect(resultState.loadingState).toBe(LoadingState.SAVING_WITH_ID); }); -test('onError from unloaded state should show error', () => { - const initialState = { - errStr: null, - loadingState: LoadingState.NOT_LOADED - }; - const action = onError('Error string'); - const resultState = projectStateReducer(initialState, action); - expect(resultState.loadingState).toBe(LoadingState.ERROR); - expect(resultState.errStr).toBe('Error string'); +test('onError from various states should show error', () => { + const startStates = [ + LoadingState.LOADING_VM_NEW_DEFAULT, + LoadingState.LOADING_VM_WITH_ID, + LoadingState.FETCHING_WITH_ID, + LoadingState.FETCHING_NEW_DEFAULT, + LoadingState.SAVING_WITH_ID, + LoadingState.SAVING_WITH_ID_BEFORE_NEW, + LoadingState.CREATING_NEW + ]; + for (const startState of startStates) { + const initialState = { + errStr: null, + loadingState: startState + }; + const action = onError('Error string'); + const resultState = projectStateReducer(initialState, action); + expect(resultState.loadingState).toBe(LoadingState.ERROR); + expect(resultState.errStr).toBe('Error string'); + } }); test('onError from showing project should show error', () => { diff --git a/test/unit/util/project-saver-hoc.test.jsx b/test/unit/util/project-saver-hoc.test.jsx new file mode 100644 index 000000000..ca7874c7a --- /dev/null +++ b/test/unit/util/project-saver-hoc.test.jsx @@ -0,0 +1,257 @@ +import 'web-audio-test-api'; + +import React from 'react'; +import configureStore from 'redux-mock-store'; +import {mount} from 'enzyme'; +import {LoadingState} from '../../../src/reducers/project-state'; +import VM from 'scratch-vm'; + +import projectSaverHOC from '../../../src/lib/project-saver-hoc.jsx'; + +describe('projectSaverHOC', () => { + const mockStore = configureStore(); + let store; + let vm; + + beforeEach(() => { + store = mockStore({ + scratchGui: { + projectState: {} + } + }); + vm = new VM(); + }); + + test('if canSave becomes true when showing a project with an id, project will be saved', () => { + const mockedSaveProject = jest.fn(); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + isShowingWithId + canSave={false} + isCreating={false} + isShowingWithoutId={false} + isUpdating={false} + loadingState={LoadingState.SHOWING_WITH_ID} + saveProject={mockedSaveProject} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + canSave: true + }); + expect(mockedSaveProject).toHaveBeenCalled(); + }); + + test('if canSave is alreatdy true and we show a project with an id, project will NOT be saved', () => { + const mockedSaveProject = jest.fn(); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + canSave + isCreating={false} + isShowingWithId={false} + isShowingWithoutId={false} + isUpdating={false} + loadingState={LoadingState.LOADING_VM_WITH_ID} + saveProject={mockedSaveProject} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + canSave: true, + isShowingWithId: true, + loadingState: LoadingState.SHOWING_WITH_ID + }); + expect(mockedSaveProject).not.toHaveBeenCalled(); + }); + + test('if canSave is false when showing a project without an id, project will NOT be created', () => { + const mockedCreateProject = jest.fn(); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + isShowingWithoutId + canSave={false} + createProject={mockedCreateProject} + isCreating={false} + isShowingWithId={false} + isUpdating={false} + loadingState={LoadingState.LOADING_VM_NEW_DEFAULT} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + isShowingWithoutId: true, + loadingState: LoadingState.SHOWING_WITHOUT_ID + }); + expect(mockedCreateProject).not.toHaveBeenCalled(); + }); + + test('if canSave becomes true when showing a project without an id, project will be created', () => { + const mockedCreateProject = jest.fn(); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + isShowingWithoutId + canSave={false} + createProject={mockedCreateProject} + isCreating={false} + isShowingWithId={false} + isUpdating={false} + loadingState={LoadingState.SHOWING_WITHOUT_ID} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + canSave: true + }); + expect(mockedCreateProject).toHaveBeenCalled(); + }); + + test('if canSave is true and we transition to showing new project, project will be created', () => { + const mockedCreateProject = jest.fn(); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + canSave + createProject={mockedCreateProject} + isCreating={false} + isShowingWithId={false} + isShowingWithoutId={false} + isUpdating={false} + loadingState={LoadingState.LOADING_VM_NEW_DEFAULT} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + isShowingWithoutId: true, + loadingState: LoadingState.SHOWING_WITHOUT_ID + }); + expect(mockedCreateProject).toHaveBeenCalled(); + }); + + test('if we enter creating state, vm project should be requested', () => { + vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + canSave + isCreating={false} + isShowingWithId={false} + isShowingWithoutId={false} + isUpdating={false} + loadingState={LoadingState.LOADING_VM_NEW_DEFAULT} + reduxProjectId={'100'} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + isCreating: true, + loadingState: LoadingState.CREATING_NEW + }); + expect(vm.saveProjectSb3).toHaveBeenCalled(); + }); + + test('if we enter updating/saving state, vm project shold be requested', () => { + vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + canSave + isCreating={false} + isShowingWithId={false} + isShowingWithoutId={false} + isUpdating={false} + loadingState={LoadingState.LOADING_VM_WITH_ID} + reduxProjectId={'100'} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + isUpdating: true, + loadingState: LoadingState.SAVING_WITH_ID + }); + expect(vm.saveProjectSb3).toHaveBeenCalled(); + }); + + test('if we are already in updating/saving state, vm project shold be NOT requested', () => { + vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); + const Component = () => <div />; + const WrappedComponent = projectSaverHOC(Component); + const mounted = mount( + <WrappedComponent + canSave + isUpdating + isCreating={false} + isShowingWithId={false} + isShowingWithoutId={false} + loadingState={LoadingState.SAVING_WITH_ID} + reduxProjectId={'100'} + store={store} + vm={vm} + /> + ); + mounted.setProps({ + isUpdating: true, + loadingState: LoadingState.SAVING_WITH_ID, + reduxProjectId: '99' // random change to force a re-render and componentDidUpdate + }); + expect(vm.saveProjectSb3).not.toHaveBeenCalled(); + }); + + // test('template', () => { + // vm.saveProjectSb3 = jest.fn(() => Promise.resolve()); + // const mockedCreateProject = jest.fn(); + // const mockedOnCreated = jest.fn(); + // const mockedOnError = jest.fn(); + // const mockedOnUpdated = jest.fn(); + // const mockedSaveProject = jest.fn(); + // + // const Component = () => <div />; + // const WrappedComponent = projectSaverHOC(Component); + // const mounted = mount( + // <WrappedComponent + // canSave={false} + // createProject={mockedCreateProject} + // isCreating={false} + // isShowingWithId={false} + // isShowingWithoutId={false} + // isUpdating={false} + // loadingState={LoadingState.NOT_LOADED} + // reduxProjectId={'100'} + // saveProject={mockedSaveProject} + // store={store} + // vm={vm} + // onCreated={mockedOnCreated} + // onError={mockedOnError} + // onUpdated={mockedOnUpdated} + // /> + // ); + // mounted.setProps({ + // canSave: true, + // isShowingWithId: true, + // loadingState: LoadingState.SHOWING_WITH_ID, + // projectData: '100' + // }); + // expect(vm.loadProject).toHaveBeenLastCalledWith('100'); + // // nextTick needed since vm.loadProject is async, and we have to wait for it :/ + // process.nextTick(() => ( + // expect(mockedCreateProject).toHaveBeenCalled() + // )); + // }); +}); diff --git a/test/unit/util/vm-manager-hoc.test.jsx b/test/unit/util/vm-manager-hoc.test.jsx index 39015a4de..10b1cfda4 100644 --- a/test/unit/util/vm-manager-hoc.test.jsx +++ b/test/unit/util/vm-manager-hoc.test.jsx @@ -68,13 +68,16 @@ describe('VMManagerHOC', () => { /> ); mounted.setProps({ + canSave: true, isLoadingWithId: true, loadingState: LoadingState.LOADING_VM_WITH_ID, projectData: '100' }); expect(vm.loadProject).toHaveBeenLastCalledWith('100'); // nextTick needed since vm.loadProject is async, and we have to wait for it :/ - process.nextTick(() => expect(mockedOnLoadedProject).toHaveBeenLastCalledWith(LoadingState.LOADING_VM_WITH_ID)); + process.nextTick(() => ( + expect(mockedOnLoadedProject).toHaveBeenLastCalledWith(LoadingState.LOADING_VM_WITH_ID, true) + )); }); test('if the fontsLoaded prop becomes true, it loads project data into the vm', () => { vm.loadProject = jest.fn(() => Promise.resolve()); -- GitLab