Allow using cloud variables for projects uploaded from files and in the editor

src/components/menu-bar/menu-bar.jsx

@@ -776,31 +776,19 @@ class MenuBar extends React.Component {
                                             />
                                         </MenuItem>
                                     )}</ChangeUsername>
-                                    <CloudVariablesToggler>{(toggleCloudVariables, {enabled, canUseCloudVariables}) => (
-                                        <MenuItem
-                                            className={classNames({[styles.disabled]: !canUseCloudVariables})}
-                                            onClick={toggleCloudVariables}
-                                        >
-                                            {canUseCloudVariables ? (
-                                                enabled ? (
-                                                    <FormattedMessage
-                                                        defaultMessage="Disable Cloud Variables"
-                                                        description="Menu bar item for disabling cloud variables"
-                                                        id="tw.menuBar.cloudOff"
-                                                    />
-                                                ) : (
-                                                    <FormattedMessage
-                                                        defaultMessage="Enable Cloud Variables"
-                                                        description="Menu bar item for enabling cloud variables"
-                                                        id="tw.menuBar.cloudOn"
-                                                    />
-                                                )
+                                    <CloudVariablesToggler>{(toggleCloudVariables, enabled) => (
+                                        <MenuItem onClick={toggleCloudVariables}>
+                                            {enabled ? (
+                                                <FormattedMessage
+                                                    defaultMessage="Disable Cloud Variables"
+                                                    description="Menu bar item for disabling cloud variables"
+                                                    id="tw.menuBar.cloudOff"
+                                                />
                                             ) : (
                                                 <FormattedMessage
-                                                    defaultMessage="Cloud Variables are not Available"
-                                                    // eslint-disable-next-line max-len
-                                                    description="Menu bar item for when cloud variables are not available"
-                                                    id="tw.menuBar.cloudUnavailable"
+                                                    defaultMessage="Enable Cloud Variables"
+                                                    description="Menu bar item for enabling cloud variables"
+                                                    id="tw.menuBar.cloudOn"
                                                 />
                                             )}
                                         </MenuItem>

src/containers/tw-cloud-toggler.jsx

@@ -1,24 +1,8 @@
 import bindAll from 'lodash.bindall';
 import PropTypes from 'prop-types';
 import React from 'react';
-import {defineMessages, injectIntl, intlShape} from 'react-intl';
 import {connect} from 'react-redux';
 import {setCloud} from '../reducers/tw';
-import isScratchDesktop from '../lib/isScratchDesktop';
-
-const messages = defineMessages({
-    cloudUnavailableAlert: {
-        defaultMessage: 'Cannot use cloud variables, most likely because you opened the editor.',
-        // eslint-disable-next-line max-len
-        description: 'Message displayed when clicking on the option to toggle cloud variables when cloud variables are not available',
-        id: 'tw.menuBar.cloudUnavailableAlert'
-    },
-    cloudUnavailableDesktop: {
-        defaultMessage: 'Cannot use cloud variables in desktop app.',
-        description: 'Message displayed when clicking on the option to toggle cloud variables in desktop app',
-        id: 'tw.menuBar.cloudUnavailableDesktop'
-    }
-});
 
 class CloudVariablesToggler extends React.Component {
     constructor (props) {
@@ -28,47 +12,28 @@ class CloudVariablesToggler extends React.Component {
         ]);
     }
     toggleCloudVariables () {
-        if (!this.props.canUseCloudVariables) {
-            const message = this.props.intl.formatMessage(
-                isScratchDesktop() ? messages.cloudUnavailableDesktop : messages.cloudUnavailableAlert
-            );
-            // eslint-disable-next-line no-alert
-            alert(message);
-            return;
-        }
         this.props.onCloudChange(!this.props.enabled);
     }
     render () {
-        const {
-            /* eslint-disable no-unused-vars */
-            children,
-            /* eslint-enable no-unused-vars */
-            ...props
-        } = this.props;
-        return this.props.children(this.toggleCloudVariables, props);
+        return this.props.children(this.toggleCloudVariables, this.props.enabled);
     }
 }
 
 CloudVariablesToggler.propTypes = {
-    intl: intlShape,
     children: PropTypes.func,
     enabled: PropTypes.bool,
-    username: PropTypes.string,
-    onCloudChange: PropTypes.func,
-    canUseCloudVariables: PropTypes.bool
+    onCloudChange: PropTypes.func
 };
 
 const mapStateToProps = state => ({
-    username: state.scratchGui.tw.username,
-    enabled: state.scratchGui.tw.cloud,
-    canUseCloudVariables: !state.scratchGui.mode.hasEverEnteredEditor
+    enabled: state.scratchGui.tw.cloud
 });
 
 const mapDispatchToProps = dispatch => ({
     onCloudChange: enabled => dispatch(setCloud(enabled))
 });
 
-export default injectIntl(connect(
+export default connect(
     mapStateToProps,
     mapDispatchToProps
-)(CloudVariablesToggler));
+)(CloudVariablesToggler);

src/lib/cloud-manager-hoc.jsx

@@ -7,6 +7,7 @@ import VM from 'scratch-vm';
 import CloudProvider from '../lib/cloud-provider';
 
 import {
+    getIsShowingProject,
     getIsShowingWithId
 } from '../reducers/project-state';
 
@@ -54,7 +55,7 @@ const cloudManagerHOC = function (WrappedComponent) {
             // when loading a new project e.g. via file upload
             // (and eventually move it out of the vm.clear function)
 
-            if (this.shouldReconnect(this.props, prevProps)) {
+            if (this.shouldConsiderReconnecting(this.props, prevProps)) {
                 this.disconnectFromCloud();
                 if (this.shouldConnect(this.props)) {
                     this.connectToCloud();
@@ -67,6 +68,7 @@ const cloudManagerHOC = function (WrappedComponent) {
             }
 
             if (this.shouldDisconnect(this.props, prevProps)) {
+                this.shouldDisconnect(this.props, prevProps);
                 this.disconnectFromCloud();
             }
         }
@@ -86,24 +88,23 @@ const cloudManagerHOC = function (WrappedComponent) {
         }
         shouldConnect (props) {
             return !this.isConnected() && this.canUseCloud(props) &&
-                props.isShowingWithId && props.vm.runtime.hasCloudData() &&
+                props.isShowingProject && props.vm.runtime.hasCloudData() &&
                 props.canModifyCloudData;
         }
         shouldDisconnect (props, prevProps) {
             return this.isConnected() &&
                 ( // Can no longer use cloud or cloud provider info is now stale
                     !this.canUseCloud(props) ||
                     !props.vm.runtime.hasCloudData() ||
-                    (props.projectId !== prevProps.projectId) ||
-                    // tw: username changes are handled in "reconnect"
-                    // (props.username !== prevProps.username) ||
+                    props.projectId !== prevProps.projectId ||
                     // Editing someone else's project
                     !props.canModifyCloudData
                 );
         }
-        shouldReconnect (props, prevProps) {
+        shouldConsiderReconnecting (props, prevProps) {
             return this.isConnected() && (
                 props.username !== prevProps.username ||
+                props.projectId !== prevProps.projectId ||
                 props.reduxCloudHost !== prevProps.reduxCloudHost
             );
         }
@@ -151,7 +152,7 @@ const cloudManagerHOC = function (WrappedComponent) {
                 projectId,
                 username,
                 hasCloudPermission,
-                isShowingWithId,
+                isShowingProject,
                 onShowCloudInfo,
                 onInvalidUsername,
                 /* eslint-enable no-unused-vars */
@@ -174,7 +175,7 @@ const cloudManagerHOC = function (WrappedComponent) {
         reduxCloudHost: PropTypes.string,
         onSetReduxCloudHost: PropTypes.func,
         hasCloudPermission: PropTypes.bool,
-        isShowingWithId: PropTypes.bool.isRequired,
+        isShowingProject: PropTypes.bool.isRequired,
         onInvalidUsername: PropTypes.func,
         onShowCloudInfo: PropTypes.func,
         projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
@@ -190,14 +191,20 @@ const cloudManagerHOC = function (WrappedComponent) {
 
     const mapStateToProps = (state, ownProps) => {
         const loadingState = state.scratchGui.projectState.loadingState;
+        const baseProjectId = getIsShowingWithId(loadingState) || !getIsShowingProject(loadingState) ? (
+            state.scratchGui.projectState.projectId
+        ) : (
+            `@gui/${state.scratchGui.projectTitle}`
+        );
         return {
-            reduxCloudHost: state.scratchGui.tw.cloudHost,
-            isShowingWithId: getIsShowingWithId(loadingState),
-            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) &&
-                // possible security concern if the program attempts to encode webcam data over cloud variables
-                !(DISABLE_WITH_VIDEO_SENSING && ownProps.vm.extensionManager.isExtensionLoaded('videoSensing'))
+            isShowingProject: getIsShowingProject(loadingState),
+            projectId: state.scratchGui.mode.hasEverEnteredEditor ? `@editor/${baseProjectId}` : baseProjectId,
+            hasCloudPermission: state.scratchGui.tw ? state.scratchGui.tw.cloud : false,
+            username: state.scratchGui.tw ? state.scratchGui.tw.username : '',
+            canModifyCloudData: !(
+                DISABLE_WITH_VIDEO_SENSING && ownProps.vm.extensionManager.isExtensionLoaded('videoSensing')
+            ),
+            reduxCloudHost: state.scratchGui.tw.cloudHost
         };
     };
 

src/lib/vm-manager-hoc.jsx

@@ -122,7 +122,6 @@ const vmManagerHOC = function (WrappedComponent) {
         onLoadedProject: PropTypes.func,
         onSetProjectUnchanged: PropTypes.func,
         projectData: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
-        projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
         username: PropTypes.string,
         vm: PropTypes.instanceOf(VM).isRequired
     };
@@ -135,7 +134,6 @@ const vmManagerHOC = function (WrappedComponent) {
             locale: state.locales.locale,
             messages: state.locales.messages,
             projectData: state.scratchGui.projectState.projectData,
-            projectId: state.scratchGui.projectState.projectId,
             loadingState: loadingState,
             isPlayerOnly: state.scratchGui.mode.isPlayerOnly,
             isStarted: state.scratchGui.vmStatus.started

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

@@ -185,7 +185,7 @@ describe.skip('CloudManagerHOC', () => {
         vm.emit('HAS_CLOUD_DATA_UPDATE', true);
 
         mounted.setProps({
-            isShowingWithId: true,
+            isShowingProject: true,
             loadingState: LoadingState.SHOWING_WITH_ID
         });
         expect(vm.setCloudProvider.mock.calls.length).toBe(1);
@@ -194,7 +194,7 @@ describe.skip('CloudManagerHOC', () => {
         expect(onShowCloudInfo).not.toHaveBeenCalled();
     });
 
-    test('projectId change should not trigger cloudProvider connection unless isShowingWithId becomes true', () => {
+    test('projectId change should not trigger cloudProvider connection unless isShowingProject becomes true', () => {
         const Component = () => <div />;
         const WrappedComponent = cloudManagerHOC(Component);
         const mounted = mountWithIntl(
@@ -212,7 +212,7 @@ describe.skip('CloudManagerHOC', () => {
         expect(vm.setCloudProvider.mock.calls.length).toBe(0);
         expect(CloudProvider).not.toHaveBeenCalled();
         mounted.setProps({
-            isShowingWithId: true,
+            isShowingProject: true,
             loadingState: LoadingState.SHOWING_WITH_ID
         });
         expect(vm.setCloudProvider.mock.calls.length).toBe(1);
@@ -265,7 +265,8 @@ describe.skip('CloudManagerHOC', () => {
             projectId: 'a different id'
         });
 
-        expect(vm.setCloudProvider.mock.calls.length).toBe(2);
+        // should be 3 not 2 -- one to connect initially, one to disconnect, one to reconnect
+        expect(vm.setCloudProvider.mock.calls.length).toBe(3);
         expect(vm.setCloudProvider).toHaveBeenCalledWith(null);
         expect(requestCloseConnection).toHaveBeenCalledTimes(1);