diff --git a/src/lib/cloud-manager-hoc.jsx b/src/lib/cloud-manager-hoc.jsx index 8f2cc6d3371258bfc18befe61e854c2395d004b9..407b50c9934b31a24bf971135c5120ec35b47631 100644 --- a/src/lib/cloud-manager-hoc.jsx +++ b/src/lib/cloud-manager-hoc.jsx @@ -52,9 +52,13 @@ const cloudManagerHOC = function (WrappedComponent) { // This should cover info about the website specifically, like scrather status return !!(props.cloudHost && props.username && props.vm && props.projectId); } + shouldNotModifyCloudData (props) { + return (props.hasEverEnteredEditor && !props.canSave); + } shouldConnect (props) { return !this.isConnected() && this.canUseCloud(props) && - props.isShowingWithId && props.vm.runtime.hasCloudData(); + props.isShowingWithId && props.vm.runtime.hasCloudData() && + !this.shouldNotModifyCloudData(props); } shouldDisconnect (props, prevProps) { return this.isConnected() && @@ -62,10 +66,10 @@ const cloudManagerHOC = function (WrappedComponent) { !this.canUseCloud(props) || !props.vm.runtime.hasCloudData() || (props.projectId !== prevProps.projectId) || - (props.username !== prevProps.username) + (props.username !== prevProps.username) || + // Editing someone else's project + this.shouldNotModifyCloudData(props) ); - // TODO need to add provisions for viewing someone - // else's project in editor mode } isConnected () { return this.cloudProvider && !!this.cloudProvider.connection; @@ -98,6 +102,7 @@ const cloudManagerHOC = function (WrappedComponent) { cloudHost, projectId, username, + hasEverEnteredEditor, isShowingWithId, /* eslint-enable no-unused-vars */ vm, @@ -114,7 +119,9 @@ const cloudManagerHOC = function (WrappedComponent) { } CloudManager.propTypes = { + canSave: PropTypes.bool.isRequired, cloudHost: PropTypes.string, + hasEverEnteredEditor: PropTypes.bool, isShowingWithId: PropTypes.bool, projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), username: PropTypes.string, @@ -124,6 +131,7 @@ const cloudManagerHOC = function (WrappedComponent) { const mapStateToProps = state => { const loadingState = state.scratchGui.projectState.loadingState; return { + hasEverEnteredEditor: state.scratchGui.mode.hasEverEnteredEditor, isShowingWithId: getIsShowingWithId(loadingState), projectId: state.scratchGui.projectState.projectId }; diff --git a/src/reducers/gui.js b/src/reducers/gui.js index edffa1cf363316d0a38a705eb20edc2f064b9566..8fb65a2112e9fac70f4be41fe7297903421e4088 100644 --- a/src/reducers/gui.js +++ b/src/reducers/gui.js @@ -62,7 +62,10 @@ const initPlayer = function (currentState) { currentState, {mode: { isFullScreen: currentState.mode.isFullScreen, - isPlayerOnly: true + isPlayerOnly: true, + // When initializing in player mode, make sure to reset + // hasEverEnteredEditorMode + hasEverEnteredEditor: false }} ); }; @@ -72,7 +75,8 @@ const initFullScreen = function (currentState) { currentState, {mode: { isFullScreen: true, - isPlayerOnly: currentState.mode.isPlayerOnly + isPlayerOnly: currentState.mode.isPlayerOnly, + hasEverEnteredEditor: currentState.mode.hasEverEnteredEditor }} ); }; diff --git a/src/reducers/mode.js b/src/reducers/mode.js index d95cb1f9905c95befaa7673484e0d6d26235da2b..739ae163d93f7221c59387b4c2b57da0524e2230 100644 --- a/src/reducers/mode.js +++ b/src/reducers/mode.js @@ -3,7 +3,8 @@ const SET_PLAYER = 'scratch-gui/mode/SET_PLAYER'; const initialState = { isFullScreen: false, - isPlayerOnly: false + isPlayerOnly: false, + hasEverEnteredEditor: true }; const reducer = function (state, action) { @@ -17,7 +18,8 @@ const reducer = function (state, action) { case SET_PLAYER: return { isFullScreen: state.isFullScreen, - isPlayerOnly: action.isPlayerOnly + isPlayerOnly: action.isPlayerOnly, + hasEverEnteredEditor: state.hasEverEnteredEditor || !action.isPlayerOnly }; default: return state; diff --git a/test/unit/util/cloud-manager-hoc.test.jsx b/test/unit/util/cloud-manager-hoc.test.jsx index c49f335cb4bf3668d1f7393f3fab2ab6b15eb431..84e4687cc7974294c47db31fbd7d321f3eb084dc 100644 --- a/test/unit/util/cloud-manager-hoc.test.jsx +++ b/test/unit/util/cloud-manager-hoc.test.jsx @@ -28,6 +28,9 @@ describe('CloudManagerHOC', () => { projectState: { projectId: '1234', loadingState: LoadingState.SHOWING_WITH_ID + }, + mode: { + hasEverEnteredEditor: false } } }); @@ -36,6 +39,9 @@ describe('CloudManagerHOC', () => { projectState: { projectId: '1234', loadingState: LoadingState.LOADING_WITH_ID + }, + mode: { + hasEverEnteredEditor: false } } }); @@ -52,6 +58,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -68,6 +75,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave store={store} username="user" vm={vm} @@ -84,6 +92,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} vm={vm} @@ -99,6 +108,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={stillLoadingStore} username="user" @@ -114,6 +124,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); const mounted = mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={stillLoadingStore} username="user" @@ -134,6 +145,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); const mounted = mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={stillLoadingStore} username="user" @@ -159,6 +171,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); const mounted = mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -183,6 +196,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); const mounted = mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -208,6 +222,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); const mounted = mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -237,6 +252,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -256,6 +272,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -279,6 +296,7 @@ describe('CloudManagerHOC', () => { const WrappedComponent = cloudManagerHOC(Component); mount( <WrappedComponent + canSave cloudHost="nonEmpty" store={store} username="user" @@ -295,6 +313,57 @@ describe('CloudManagerHOC', () => { expect(vm.setCloudProvider.mock.calls.length).toBe(2); expect(vm.setCloudProvider).toHaveBeenCalledWith(null); expect(requestCloseConnection).toHaveBeenCalledTimes(1); + }); + + // Editor Mode Connection/Disconnection Tests + test('Entering editor mode and can\'t save project should disconnect cloud provider # 1', () => { + const Component = () => <div />; + const WrappedComponent = cloudManagerHOC(Component); + const mounted = mount( + <WrappedComponent + canSave + cloudHost="nonEmpty" + store={store} + username="user" + vm={vm} + /> + ); + + expect(CloudProvider).toHaveBeenCalled(); + const requestCloseConnection = mockCloudProviderInstance.requestCloseConnection; + mounted.setProps({ + canSave: false, + hasEverEnteredEditor: true + }); + + expect(vm.setCloudProvider.mock.calls.length).toBe(2); + expect(vm.setCloudProvider).toHaveBeenCalledWith(null); + expect(requestCloseConnection).toHaveBeenCalledTimes(1); + }); + + test('Entering editor mode and can\'t save project should disconnect cloud provider # 2', () => { + const Component = () => <div />; + const WrappedComponent = cloudManagerHOC(Component); + const mounted = mount( + <WrappedComponent + canSave={false} + cloudHost="nonEmpty" + store={store} + username="user" + vm={vm} + /> + ); + + expect(CloudProvider).toHaveBeenCalled(); + const requestCloseConnection = mockCloudProviderInstance.requestCloseConnection; + + mounted.setProps({ + hasEverEnteredEditor: true + }); + + expect(vm.setCloudProvider.mock.calls.length).toBe(2); + expect(vm.setCloudProvider).toHaveBeenCalledWith(null); + expect(requestCloseConnection).toHaveBeenCalledTimes(1); }); });