Merge pull request #64 from cwillisf/fix-telemetry-more

More telemetry fixes (plus project title editing)

package-lock.json

@@ -7640,6 +7640,18 @@
       "integrity": "sha512-DHb1ub+rMjjrxqlB3H56/6MXtm1lSksDp2rA2cNWjG8mlDUYFhUj3Di2Zn5IwSU87xLv8tNIQ7sSwE/YOX/D/Q==",
       "dev": true
     },
+    "lodash.bindall": {
+      "version": "4.4.0",
+      "resolved": "https://registry.npmjs.org/lodash.bindall/-/lodash.bindall-4.4.0.tgz",
+      "integrity": "sha1-p7/Ugro9LnBxad/NyZPrv2w9eZg=",
+      "dev": true
+    },
+    "lodash.defaultsdeep": {
+      "version": "4.6.1",
+      "resolved": "https://registry.npmjs.org/lodash.defaultsdeep/-/lodash.defaultsdeep-4.6.1.tgz",
+      "integrity": "sha512-3j8wdDzYuWO3lM3Reg03MuQR957t287Rpcxp1njpEa8oDrikb+FwGdW3n+FELh/A6qib6yPit0j/pv9G/yeAqA==",
+      "dev": true
+    },
     "lodash.memoize": {
       "version": "4.1.2",
       "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz",

package.json

@@ -45,6 +45,8 @@
     "eslint-plugin-import": "^2.18.0",
     "eslint-plugin-react": "^7.14.2",
     "intl": "1.2.5",
+    "lodash.bindall": "^4.4.0",
+    "lodash.defaultsdeep": "^4.6.1",
     "mkdirp": "^0.5.1",
     "nets": "^3.2.0",
     "react": "16.2.0",

src/main/ScratchDesktopTelemetry.js

@@ -1,4 +1,5 @@
 import {app, ipcMain} from 'electron';
+import defaultsDeep from 'lodash.defaultsdeep';
 
 import TelemetryClient from './telemetry/TelemetryClient';
 
@@ -6,13 +7,15 @@ const EVENT_TEMPLATE = {
     version: '3.0.0',
     projectName: '',
     language: '',
-    scriptCount: -1,
-    spriteCount: -1,
-    variablesCount: -1,
-    blocksCount: -1,
-    costumesCount: -1,
-    listsCount: -1,
-    soundsCount: -1
+    metadata: {
+        scriptCount: -1,
+        spriteCount: -1,
+        variablesCount: -1,
+        blocksCount: -1,
+        costumesCount: -1,
+        listsCount: -1,
+        soundsCount: -1
+    }
 };
 
 const APP_ID = 'scratch-desktop';
@@ -42,19 +45,44 @@ class ScratchDesktopTelemetry {
     }
 
     projectDidLoad (metadata = {}) {
-        this._telemetryClient.addEvent('project::load', {...EVENT_TEMPLATE, ...metadata});
+        this._telemetryClient.addEvent('project::load', this._buildMetadata(metadata));
     }
 
     projectDidSave (metadata = {}) {
-        this._telemetryClient.addEvent('project::save', {...EVENT_TEMPLATE, ...metadata});
+        // Since the save dialog appears on the main process the GUI does not wait for the actual save to complete.
+        // That means the GUI sends this event before we know the file name used for the save, which is where the new
+        // project title comes from. Instead, just hold on to this metadata pending a `projectSaveCompleted` event
+        // from the save code on the main process. If the user cancels the save this data will be cleared.
+        this._pendingProjectSave = metadata;
+    }
+
+    projectSaveCompleted (newProjectTitle) {
+        const metadata = this._pendingProjectSave;
+        this._pendingProjectSave = null;
+
+        metadata.projectName = newProjectTitle;
+        this._telemetryClient.addEvent('project::save', this._buildMetadata(metadata));
+    }
+
+    projectSaveCanceled () {
+        this._pendingProjectSave = null;
     }
 
     projectWasCreated (metadata = {}) {
-        this._telemetryClient.addEvent('project::create', {...EVENT_TEMPLATE, ...metadata});
+        this._telemetryClient.addEvent('project::create', this._buildMetadata(metadata));
     }
 
     projectWasUploaded (metadata = {}) {
-        this._telemetryClient.addEvent('project::upload', {...EVENT_TEMPLATE, ...metadata});
+        this._telemetryClient.addEvent('project::upload', this._buildMetadata(metadata));
+    }
+
+    _buildMetadata (metadata) {
+        const { projectName, language, ...codeMetadata } = metadata;
+        return defaultsDeep({
+            projectName,
+            language,
+            metadata: codeMetadata
+        }, EVENT_TEMPLATE);
     }
 }
 

src/main/index.js

@@ -62,13 +62,22 @@ const createMainWindow = () => {
         if (extName) {
             const extNameNoDot = extName.replace(/^\./, '');
             const options = {
+                defaultPath: baseName,
                 filters: [getFilterForExtension(extNameNoDot)]
             };
             const userChosenPath = dialog.showSaveDialog(window, options);
             if (userChosenPath) {
                 item.setSavePath(userChosenPath);
+                const newProjectTitle = path.basename(userChosenPath, extName);
+                webContents.send('setTitleFromSave', {title: newProjectTitle});
+
+                // "setTitleFromSave" will set the project title but GUI has already reported the telemetry event
+                // using the old title. This call lets the telemetry client know that the save was actually completed
+                // and the event should be committed to the event queue with this new title.
+                telemetry.projectSaveCompleted(newProjectTitle);
             } else {
                 item.cancel();
+                telemetry.projectSaveCanceled();
             }
         }
     });

src/renderer/app.jsx

@@ -1,7 +1,10 @@
 import {ipcRenderer, shell} from 'electron';
+import bindAll from 'lodash.bindall';
+import PropTypes from 'prop-types';
 import React from 'react';
 import ReactDOM from 'react-dom';
-import GUI, {AppStateHOC} from 'scratch-gui';
+import {compose} from 'redux';
+import GUI, {AppStateHOC, TitledHOC} from 'scratch-gui';
 
 import ElectronStorageHelper from '../common/ElectronStorageHelper';
 
@@ -23,27 +26,69 @@ appTarget.className = styles.app || 'app'; // TODO
 document.body.appendChild(appTarget);
 
 GUI.setAppElement(appTarget);
-const WrappedGui = AppStateHOC(GUI);
 
-const onStorageInit = storageInstance => {
-    storageInstance.addHelper(new ElectronStorageHelper(storageInstance));
-    // storageInstance.addOfficialScratchWebStores(); // TODO: do we want this?
-};
-
-const guiProps = {
-    onStorageInit,
-    isScratchDesktop: true,
-    projectId: defaultProjectId,
-    showTelemetryModal: (typeof ipcRenderer.sendSync('getTelemetryDidOptIn')) !== 'boolean',
-    onTelemetryModalOptIn: () => {
-        ipcRenderer.send('setTelemetryDidOptIn', true);
-    },
-    onTelemetryModalOptOut: () => {
-        ipcRenderer.send('setTelemetryDidOptIn', false);
-    },
-    onProjectTelemetryEvent: (event, metadata) => {
-        ipcRenderer.send(event, metadata);
+const ScratchDesktopHOC = function (WrappedComponent) {
+    class ScratchDesktopComponent extends React.Component {
+        constructor (props) {
+            super(props);
+            bindAll(this, [
+                'handleProjectTelemetryEvent',
+                'handleSetTitleFromSave',
+                'handleStorageInit',
+                'handleTelemetryModalOptIn',
+                'handleTelemetryModalOptOut'
+            ]);
+        }
+        componentDidMount () {
+            ipcRenderer.on('setTitleFromSave', this.handleSetTitleFromSave);
+        }
+        componentWillUnmount () {
+            ipcRenderer.removeListener('setTitleFromSave', this.handleSetTitleFromSave);
+        }
+        handleProjectTelemetryEvent (event, metadata) {
+            ipcRenderer.send(event, metadata);
+        }
+        handleSetTitleFromSave (event, args) {
+            this.props.onUpdateProjectTitle(args.title);
+        }
+        handleStorageInit (storageInstance) {
+            storageInstance.addHelper(new ElectronStorageHelper(storageInstance));
+        }
+        handleTelemetryModalOptIn () {
+            ipcRenderer.send('setTelemetryDidOptIn', true);
+        }
+        handleTelemetryModalOptOut () {
+            ipcRenderer.send('setTelemetryDidOptIn', false);
+        }
+        render () {
+            const shouldShowTelemetryModal = (typeof ipcRenderer.sendSync('getTelemetryDidOptIn') !== 'boolean');
+            return (<WrappedComponent
+                isScratchDesktop
+                projectId={defaultProjectId}
+                showTelemetryModal={shouldShowTelemetryModal}
+                onProjectTelemetryEvent={this.handleProjectTelemetryEvent}
+                onStorageInit={this.handleStorageInit}
+                onTelemetryModalOptIn={this.handleTelemetryModalOptIn}
+                onTelemetryModalOptOut={this.handleTelemetryModalOptOut}
+                {...this.props}
+            />);
+        }
     }
+
+    ScratchDesktopComponent.propTypes = {
+        onUpdateProjectTitle: PropTypes.func
+    };
+
+    return ScratchDesktopComponent;
 };
-const wrappedGui = React.createElement(WrappedGui, guiProps);
-ReactDOM.render(wrappedGui, appTarget);
+
+// note that redux's 'compose' function is just being used as a general utility to make
+// the hierarchy of HOC constructor calls clearer here; it has nothing to do with redux's
+// ability to compose reducers.
+const WrappedGui = compose(
+    AppStateHOC,
+    TitledHOC,
+    ScratchDesktopHOC // must come after `TitledHOC` so it has access to `onUpdateProjectTitle`
+)(GUI);
+
+ReactDOM.render(<WrappedGui />, appTarget);