From 09a22e061f194b1807b3dcbec019c4849bca57d6 Mon Sep 17 00:00:00 2001
From: Paul Kaplan <pkaplan@media.mit.edu>
Date: Tue, 4 Dec 2018 15:12:23 -0500
Subject: [PATCH] Add an alert that is shown when you first add a cloud
 variable

---
 src/lib/alerts/index.jsx                  | 30 +++++++++++++++++++++++
 src/lib/cloud-manager-hoc.jsx             | 13 ++++++++--
 test/unit/util/cloud-manager-hoc.test.jsx | 18 ++++++++++++++
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/src/lib/alerts/index.jsx b/src/lib/alerts/index.jsx
index dda178e1f..ffe84da5d 100644
--- a/src/lib/alerts/index.jsx
+++ b/src/lib/alerts/index.jsx
@@ -102,6 +102,36 @@ const alerts = [
         ),
         iconSpinner: true,
         level: AlertLevels.INFO
+    },
+    {
+        alertId: 'cloudInfo',
+        alertType: AlertTypes.STANDARD,
+        clearList: ['cloudInfo'],
+        content: (
+            <FormattedMessage
+                defaultMessage="Please note, cloud variables only support numbers, not letters or symbols. {learnMoreLink}" // eslint-disable-line max-len
+                description="Info about cloud variable limitations"
+                id="gui.alerts.cloudInfo"
+                values={{
+                    learnMoreLink: (
+                        <a
+                            href="https://scratch.mit.edu/info/faq/#clouddata"
+                            rel="noopener noreferrer"
+                            target="_blank"
+                        >
+                            <FormattedMessage
+                                defaultMessage="Learn more."
+                                description="Link text to cloud var faq"
+                                id="gui.alerts.cloudInfoLearnMore"
+                            />
+                        </a>
+                    )
+                }}
+            />
+        ),
+        closeButton: true,
+        level: AlertLevels.SUCCESS,
+        maxDisplaySecs: 15
     }
 ];
 
diff --git a/src/lib/cloud-manager-hoc.jsx b/src/lib/cloud-manager-hoc.jsx
index a78239b80..05047b435 100644
--- a/src/lib/cloud-manager-hoc.jsx
+++ b/src/lib/cloud-manager-hoc.jsx
@@ -10,6 +10,10 @@ import {
     getIsShowingWithId
 } from '../reducers/project-state';
 
+import {
+    showAlertWithTimeout
+} from '../reducers/alerts';
+
 /*
  * Higher Order Component to manage the connection to the cloud dserver.
  * @param {React.Component} WrappedComponent component to manage VM events for
@@ -90,7 +94,8 @@ const cloudManagerHOC = function (WrappedComponent) {
         handleCloudDataUpdate (projectHasCloudData) {
             if (this.isConnected() && !projectHasCloudData) {
                 this.disconnectFromCloud();
-            } else if (!this.isConnected() && this.canUseCloud(this.props) && projectHasCloudData) {
+            } else if (this.shouldConnect(this.props)) {
+                this.props.onShowCloudInfo();
                 this.connectToCloud();
             }
         }
@@ -103,6 +108,7 @@ const cloudManagerHOC = function (WrappedComponent) {
                 hasCloudPermission,
                 hasEverEnteredEditor,
                 isShowingWithId,
+                onShowCloudInfo,
                 /* eslint-enable no-unused-vars */
                 vm,
                 ...componentProps
@@ -123,6 +129,7 @@ const cloudManagerHOC = function (WrappedComponent) {
         hasCloudPermission: PropTypes.bool,
         hasEverEnteredEditor: PropTypes.bool,
         isShowingWithId: PropTypes.bool,
+        onShowCloudInfo: PropTypes.func,
         projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
         username: PropTypes.string,
         vm: PropTypes.instanceOf(VM).isRequired
@@ -137,7 +144,9 @@ const cloudManagerHOC = function (WrappedComponent) {
         };
     };
 
-    const mapDispatchToProps = () => ({});
+    const mapDispatchToProps = dispatch => ({
+        onShowCloudInfo: () => showAlertWithTimeout(dispatch, 'cloudInfo')
+    });
 
     // Allow incoming props to override redux-provided props. Used to mock in tests.
     const mergeProps = (stateProps, dispatchProps, ownProps) => Object.assign(
diff --git a/test/unit/util/cloud-manager-hoc.test.jsx b/test/unit/util/cloud-manager-hoc.test.jsx
index 977b5045f..d372e1ff5 100644
--- a/test/unit/util/cloud-manager-hoc.test.jsx
+++ b/test/unit/util/cloud-manager-hoc.test.jsx
@@ -56,6 +56,8 @@ describe('CloudManagerHOC', () => {
     test('when it mounts, the cloud provider is set on the vm', () => {
         const Component = () => (<div />);
         const WrappedComponent = cloudManagerHOC(Component);
+        const onShowCloudInfo = jest.fn();
+
         mount(
             <WrappedComponent
                 canSave
@@ -64,11 +66,13 @@ describe('CloudManagerHOC', () => {
                 store={store}
                 username="user"
                 vm={vm}
+                onShowCloudInfo={onShowCloudInfo}
             />
         );
         expect(vm.setCloudProvider.mock.calls.length).toBe(1);
         expect(CloudProvider).toHaveBeenCalledTimes(1);
         expect(vm.setCloudProvider).toHaveBeenCalledWith(mockCloudProviderInstance);
+        expect(onShowCloudInfo).not.toHaveBeenCalled();
     });
 
     test('when cloudHost is missing, the cloud provider is not set on the vm', () => {
@@ -144,6 +148,9 @@ describe('CloudManagerHOC', () => {
     test('if the isShowingWithId prop becomes true, it sets the cloud provider on the vm', () => {
         const Component = () => <div />;
         const WrappedComponent = cloudManagerHOC(Component);
+        const onShowCloudInfo = jest.fn();
+        vm.runtime.hasCloudData = jest.fn(() => false);
+
         const mounted = mount(
             <WrappedComponent
                 canSave
@@ -152,8 +159,14 @@ describe('CloudManagerHOC', () => {
                 store={stillLoadingStore}
                 username="user"
                 vm={vm}
+                onShowCloudInfo={onShowCloudInfo}
             />
         );
+        expect(onShowCloudInfo).not.toHaveBeenCalled();
+
+        vm.runtime.hasCloudData = jest.fn(() => true);
+        vm.emit('HAS_CLOUD_DATA_UPDATE', true);
+
         mounted.setProps({
             isShowingWithId: true,
             loadingState: LoadingState.SHOWING_WITH_ID
@@ -161,6 +174,7 @@ describe('CloudManagerHOC', () => {
         expect(vm.setCloudProvider.mock.calls.length).toBe(1);
         expect(CloudProvider).toHaveBeenCalledTimes(1);
         expect(vm.setCloudProvider).toHaveBeenCalledWith(mockCloudProviderInstance);
+        expect(onShowCloudInfo).not.toHaveBeenCalled();
     });
 
     test('projectId change should not trigger cloudProvider connection unless isShowingWithId becomes true', () => {
@@ -295,6 +309,7 @@ describe('CloudManagerHOC', () => {
         // Mock the vm runtime function so that has cloud data is not
         // initially true
         vm.runtime.hasCloudData = jest.fn(() => false);
+        const onShowCloudInfo = jest.fn();
 
         const Component = () => <div />;
         const WrappedComponent = cloudManagerHOC(Component);
@@ -306,10 +321,12 @@ describe('CloudManagerHOC', () => {
                 store={store}
                 username="user"
                 vm={vm}
+                onShowCloudInfo={onShowCloudInfo}
             />
         );
         expect(vm.setCloudProvider.mock.calls.length).toBe(0);
         expect(CloudProvider).not.toHaveBeenCalled();
+        expect(onShowCloudInfo).not.toHaveBeenCalled();
 
         // Mock VM hasCloudData becoming true and emitting an update
         vm.runtime.hasCloudData = jest.fn(() => true);
@@ -318,6 +335,7 @@ describe('CloudManagerHOC', () => {
         expect(vm.setCloudProvider.mock.calls.length).toBe(1);
         expect(CloudProvider).toHaveBeenCalledTimes(1);
         expect(vm.setCloudProvider).toHaveBeenCalledWith(mockCloudProviderInstance);
+        expect(onShowCloudInfo).toHaveBeenCalled();
     });
 
     test('projectHasCloudDataUpdate becoming false should trigger cloudProvider disconnection', () => {
-- 
GitLab