Skip to content
Snippets Groups Projects
Unverified Commit ede6d2fe authored by Karishma Chadha's avatar Karishma Chadha Committed by GitHub
Browse files

Merge pull request #3890 from kchadha/editor-cloud-disconnect

Disconnect from cloud when entering editor mode on someone else's project
parents c27a328c 2ca51904
No related branches found
No related tags found
No related merge requests found
......@@ -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
};
......
......@@ -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
}}
);
};
......
......@@ -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;
......
......@@ -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);
});
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment