Merge pull request scratchfoundation#5198 from benjiwheeler/block-cloud-on-embed

simplify props of cloud-manager-hoc

Author: benjiwheeler

src/lib/cloud-manager-hoc.jsx

@@ -54,13 +54,10 @@ const cloudManagerHOC = function (WrappedComponent) {
         canUseCloud (props) {
             return !!(props.cloudHost && props.username && props.vm && props.projectId && props.hasCloudPermission);
         }
-        shouldNotModifyCloudData (props) {
-            return (props.hasEverEnteredEditor && !props.canSave);
-        }
         shouldConnect (props) {
             return !this.isConnected() && this.canUseCloud(props) &&
                 props.isShowingWithId && props.vm.runtime.hasCloudData() &&
-                !this.shouldNotModifyCloudData(props);
+                props.canModifyCloudData;
         }
         shouldDisconnect (props, prevProps) {
             return this.isConnected() &&
@@ -70,7 +67,7 @@ const cloudManagerHOC = function (WrappedComponent) {
                     (props.projectId !== prevProps.projectId) ||
                     (props.username !== prevProps.username) ||
                     // Editing someone else's project
-                    this.shouldNotModifyCloudData(props)
+                    !props.canModifyCloudData
                 );
         }
         isConnected () {
@@ -102,11 +99,11 @@ const cloudManagerHOC = function (WrappedComponent) {
         render () {
             const {
                 /* eslint-disable no-unused-vars */
+                canModifyCloudData,
                 cloudHost,
                 projectId,
                 username,
                 hasCloudPermission,
-                hasEverEnteredEditor,
                 isShowingWithId,
                 onShowCloudInfo,
                 /* eslint-enable no-unused-vars */
@@ -124,23 +121,30 @@ const cloudManagerHOC = function (WrappedComponent) {
     }
 
     CloudManager.propTypes = {
-        canSave: PropTypes.bool.isRequired,
+        canModifyCloudData: PropTypes.bool.isRequired,
         cloudHost: PropTypes.string,
         hasCloudPermission: PropTypes.bool,
-        hasEverEnteredEditor: PropTypes.bool,
-        isShowingWithId: PropTypes.bool,
+        isShowingWithId: PropTypes.bool.isRequired,
         onShowCloudInfo: PropTypes.func,
         projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
         username: PropTypes.string,
         vm: PropTypes.instanceOf(VM).isRequired
     };
 
-    const mapStateToProps = state => {
+    CloudManager.defaultProps = {
+        cloudHost: null,
+        hasCloudPermission: false,
+        onShowCloudInfo: () => {},
+        username: null
+    };
+
+    const mapStateToProps = (state, ownProps) => {
         const loadingState = state.scratchGui.projectState.loadingState;
         return {
-            hasEverEnteredEditor: state.scratchGui.mode.hasEverEnteredEditor,
             isShowingWithId: getIsShowingWithId(loadingState),
-            projectId: state.scratchGui.projectState.projectId
+            projectId: state.scratchGui.projectState.projectId,
+            // if you're editing someone else's project, you can't modify cloud data
+            canModifyCloudData: (!state.scratchGui.mode.hasEverEnteredEditor || ownProps.canSave)
         };
     };
 

test/unit/util/cloud-manager-hoc.test.jsx

@@ -60,7 +60,6 @@ describe('CloudManagerHOC', () => {
 
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -80,7 +79,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 store={store}
                 username="user"
@@ -98,7 +96,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -115,7 +112,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={stillLoadingStore}
@@ -132,7 +128,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 cloudHost="nonEmpty"
                 hasCloudPermission={false}
                 store={store}
@@ -153,7 +148,6 @@ describe('CloudManagerHOC', () => {
 
         const mounted = mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={stillLoadingStore}
@@ -182,7 +176,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         const mounted = mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={stillLoadingStore}
@@ -209,7 +202,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         const mounted = mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -235,7 +227,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         const mounted = mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -262,7 +253,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         const mounted = mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -293,7 +283,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -315,7 +304,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -343,7 +331,6 @@ describe('CloudManagerHOC', () => {
         const WrappedComponent = cloudManagerHOC(Component);
         mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -364,12 +351,11 @@ describe('CloudManagerHOC', () => {
     });
 
     // Editor Mode Connection/Disconnection Tests
-    test('Entering editor mode and can\'t save project should disconnect cloud provider # 1', () => {
+    test('Entering editor mode and can\'t save project should disconnect cloud provider', () => {
         const Component = () => <div />;
         const WrappedComponent = cloudManagerHOC(Component);
         const mounted = mount(
             <WrappedComponent
-                canSave
                 hasCloudPermission
                 cloudHost="nonEmpty"
                 store={store}
@@ -382,34 +368,7 @@ describe('CloudManagerHOC', () => {
         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
-                hasCloudPermission
-                canSave={false}
-                cloudHost="nonEmpty"
-                store={store}
-                username="user"
-                vm={vm}
-            />
-        );
-
-        expect(CloudProvider).toHaveBeenCalled();
-        const requestCloseConnection = mockCloudProviderInstance.requestCloseConnection;
-
-        mounted.setProps({
-            hasEverEnteredEditor: true
+            canModifyCloudData: false
         });
 
         expect(vm.setCloudProvider.mock.calls.length).toBe(2);