From 34c85eb150926961a9f6bd760d352429ead858df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C8=98tefan=20Daniel=20Mih=C4=83il=C4=83?= Date: Fri, 10 Mar 2017 21:18:49 +0200 Subject: [PATCH] feat(GUI): improve analytics events (#1111) * feat(GUI): improve analytics events This commit adds more events to our current analytics. Will further improve in a future commit. Change-Type: patch See: https://github.com/resin-io/etcher/issues/1100 * refactor(gui): use single function to set normal and dangerous settings Change-Type: patch Signed-off-by: Juan Cruz Viotti --- lib/gui/app.js | 80 ++++++++--- .../controllers/drive-selector.js | 14 +- .../drive-selector/drive-selector.js | 3 +- lib/gui/components/modal/modal.js | 3 +- lib/gui/components/modal/services/modal.js | 38 ++++-- .../controllers/update-notifier.js | 7 +- .../update-notifier/update-notifier.js | 3 +- lib/gui/modules/analytics.js | 53 +++++--- lib/gui/os/open-external/open-external.js | 4 +- .../open-external/services/open-external.js | 6 +- .../pages/main/controllers/drive-selection.js | 5 +- lib/gui/pages/main/controllers/flash.js | 16 ++- .../pages/main/controllers/image-selection.js | 8 +- lib/gui/pages/main/controllers/main.js | 7 +- lib/gui/pages/main/templates/main.tpl.html | 2 +- .../pages/settings/controllers/settings.js | 41 ++++-- lib/gui/pages/settings/settings.js | 3 +- .../settings/templates/settings.tpl.html | 8 +- lib/shared/utils.js | 83 ++++++++++++ npm-shrinkwrap.json | 45 +++++++ package.json | 3 + tests/shared/utils.spec.js | 127 ++++++++++++++++++ 22 files changed, 477 insertions(+), 82 deletions(-) create mode 100644 lib/shared/utils.js create mode 100644 tests/shared/utils.spec.js diff --git a/lib/gui/app.js b/lib/gui/app.js index 1aa4d9a8..4e13eb6c 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -30,6 +30,7 @@ const electron = require('electron'); const Bluebird = require('bluebird'); const EXIT_CODES = require('../shared/exit-codes'); const messages = require('../shared/messages'); +const packageJSON = require('../../package.json'); const Store = require('./models/store'); @@ -85,26 +86,47 @@ app.run(() => { app.run((AnalyticsService, ErrorService, UpdateNotifierService, SelectionStateModel) => { AnalyticsService.logEvent('Application start'); - if (UpdateNotifierService.shouldCheckForUpdates() && !process.env.ETCHER_DISABLE_UPDATES) { - AnalyticsService.logEvent('Checking for updates'); + const shouldCheckForUpdates = UpdateNotifierService.shouldCheckForUpdates(); - UpdateNotifierService.isLatestVersion().then((isLatestVersion) => { - - // In case the internet connection is not good and checking the - // latest published version takes too long, only show notify - // the user about the new version if he didn't start the flash - // process (e.g: selected an image), otherwise such interruption - // might be annoying. - if (!isLatestVersion && !SelectionStateModel.hasImage()) { - - AnalyticsService.logEvent('Notifying update'); - return UpdateNotifierService.notify(); - } - - return Bluebird.resolve(); - }).catch(ErrorService.reportException); + if (!shouldCheckForUpdates || process.env.ETCHER_DISABLE_UPDATES) { + AnalyticsService.logEvent('Not checking for updates', { + shouldCheckForUpdates, + disableUpdatesEnvironmentVariable: process.env.ETCHER_DISABLE_UPDATES + }); + return; } + AnalyticsService.logEvent('Checking for updates', { + currentVersion: packageJSON.version + }); + + UpdateNotifierService.isLatestVersion().then((isLatestVersion) => { + + if (isLatestVersion) { + AnalyticsService.logEvent('Update notification skipped', { + reason: 'Latest version' + }); + return Bluebird.resolve(); + } + + // In case the internet connection is not good and checking the + // latest published version takes too long, only show notify + // the user about the new version if he didn't start the flash + // process (e.g: selected an image), otherwise such interruption + // might be annoying. + if (SelectionStateModel.hasImage()) { + AnalyticsService.logEvent('Update notification skipped', { + reason: 'Image selected' + }); + return Bluebird.resolve(); + } + + AnalyticsService.logEvent('Notifying update'); + + return UpdateNotifierService.notify(); + + }).catch(ErrorService.reportException); + }); app.run((AnalyticsService, OSWindowProgressService, FlashStateModel) => { @@ -158,11 +180,14 @@ app.run(($timeout, DriveScannerService, DrivesModel, ErrorService) => { DriveScannerService.start(); }); -app.run(($window, WarningModalService, ErrorService, FlashStateModel, OSDialogService) => { +app.run(($window, AnalyticsService, WarningModalService, ErrorService, FlashStateModel, OSDialogService) => { let popupExists = false; $window.addEventListener('beforeunload', (event) => { if (!FlashStateModel.isFlashing() || popupExists) { + AnalyticsService.logEvent('Close application', { + isFlashing: FlashStateModel.isFlashing() + }); return; } @@ -172,6 +197,8 @@ app.run(($window, WarningModalService, ErrorService, FlashStateModel, OSDialogSe // Don't open any more popups popupExists = true; + AnalyticsService.logEvent('Close attempt while flashing'); + OSDialogService.showWarning({ confirmationLabel: 'Yes, quit', rejectionLabel: 'Cancel', @@ -179,6 +206,7 @@ app.run(($window, WarningModalService, ErrorService, FlashStateModel, OSDialogSe description: messages.warning.exitWhileFlashing() }).then((confirmed) => { if (confirmed) { + AnalyticsService.logEvent('Close confirmed while flashing'); // This circumvents the 'beforeunload' event unlike // electron.remote.app.quit() which does not. @@ -186,11 +214,27 @@ app.run(($window, WarningModalService, ErrorService, FlashStateModel, OSDialogSe } + AnalyticsService.logEvent('Close rejected while flashing'); popupExists = false; }).catch(ErrorService.reportException); }); }); +app.run(($rootScope, AnalyticsService) => { + $rootScope.$on('$stateChangeSuccess', (event, toState, toParams, fromState) => { + + // Ignore first navigation + if (!fromState.name) { + return; + } + + AnalyticsService.logEvent('Navigate', { + to: toState.name, + from: fromState.name + }); + }); +}); + app.config(($urlRouterProvider) => { $urlRouterProvider.otherwise('/main'); }); diff --git a/lib/gui/components/drive-selector/controllers/drive-selector.js b/lib/gui/components/drive-selector/controllers/drive-selector.js index 42b3cf91..79ad5ec9 100644 --- a/lib/gui/components/drive-selector/controllers/drive-selector.js +++ b/lib/gui/components/drive-selector/controllers/drive-selector.js @@ -25,7 +25,9 @@ module.exports = function( DrivesModel, SelectionStateModel, WarningModalService, - DriveConstraintsModel) { + DriveConstraintsModel, + AnalyticsService +) { /** * @summary The drive selector state @@ -105,10 +107,17 @@ module.exports = function( * }); */ this.toggleDrive = (drive) => { + + AnalyticsService.logEvent('Toggle drive', { + drive, + previouslySelected: SelectionStateModel.isCurrentDrive(drive.device) + }); + return shouldChangeDriveSelectionState(drive).then((canChangeDriveSelectionState) => { if (canChangeDriveSelectionState) { SelectionStateModel.toggleSetDrive(drive.device); } + }); }; @@ -153,6 +162,9 @@ module.exports = function( return shouldChangeDriveSelectionState(drive).then((canChangeDriveSelectionState) => { if (canChangeDriveSelectionState) { SelectionStateModel.setDrive(drive.device); + + AnalyticsService.logEvent('Drive selected (double click)'); + this.closeModal(); } }); diff --git a/lib/gui/components/drive-selector/drive-selector.js b/lib/gui/components/drive-selector/drive-selector.js index 14a7c3ad..ad6f500e 100644 --- a/lib/gui/components/drive-selector/drive-selector.js +++ b/lib/gui/components/drive-selector/drive-selector.js @@ -28,7 +28,8 @@ const DriveSelector = angular.module(MODULE_NAME, [ require('../../models/drives'), require('../../models/selection-state'), require('../../models/drive-constraints'), - require('../../utils/byte-size/byte-size') + require('../../utils/byte-size/byte-size'), + require('../../modules/analytics') ]); DriveSelector.controller('DriveSelectorController', require('./controllers/drive-selector')); diff --git a/lib/gui/components/modal/modal.js b/lib/gui/components/modal/modal.js index b72c0417..c12d21d4 100644 --- a/lib/gui/components/modal/modal.js +++ b/lib/gui/components/modal/modal.js @@ -23,7 +23,8 @@ const angular = require('angular'); const MODULE_NAME = 'Etcher.Components.Modal'; const Modal = angular.module(MODULE_NAME, [ - require('angular-ui-bootstrap') + require('angular-ui-bootstrap'), + require('../../modules/analytics') ]); Modal.service('ModalService', require('./services/modal')); diff --git a/lib/gui/components/modal/services/modal.js b/lib/gui/components/modal/services/modal.js index 31ceeabc..32171fd7 100644 --- a/lib/gui/components/modal/services/modal.js +++ b/lib/gui/components/modal/services/modal.js @@ -18,7 +18,7 @@ const _ = require('lodash'); -module.exports = function($uibModal, $q) { +module.exports = function($uibModal, $q, AnalyticsService) { /** * @summary Open a modal @@ -44,6 +44,10 @@ module.exports = function($uibModal, $q) { size: 'sm' }); + AnalyticsService.logEvent('Open modal', { + template: options.template + }); + const modal = $uibModal.open({ animation: true, templateUrl: options.template, @@ -55,17 +59,29 @@ module.exports = function($uibModal, $q) { return { close: modal.close, result: $q((resolve, reject) => { - modal.result - .then(resolve) - .catch((error) => { - - // Bootstrap doesn't 'resolve' these but cancels the dialog - if (error === 'escape key press' || error === 'backdrop click') { - return resolve(); - } - - return reject(error); + modal.result.then((value) => { + AnalyticsService.logEvent('Modal accepted', { + value }); + + resolve(value); + }).catch((error) => { + + // Bootstrap doesn't 'resolve' these but cancels the dialog + if (error === 'escape key press' || error === 'backdrop click') { + AnalyticsService.logEvent('Modal rejected', { + method: error + }); + + return resolve(); + } + + AnalyticsService.logEvent('Modal rejected', { + value: error + }); + + return reject(error); + }); }) }; }; diff --git a/lib/gui/components/update-notifier/controllers/update-notifier.js b/lib/gui/components/update-notifier/controllers/update-notifier.js index 80cb26d2..845d457e 100644 --- a/lib/gui/components/update-notifier/controllers/update-notifier.js +++ b/lib/gui/components/update-notifier/controllers/update-notifier.js @@ -16,7 +16,7 @@ 'use strict'; -module.exports = function($uibModalInstance, SettingsModel, UPDATE_NOTIFIER_SLEEP_DAYS, options) { +module.exports = function($uibModalInstance, SettingsModel, AnalyticsService, UPDATE_NOTIFIER_SLEEP_DAYS, options) { // We update this value in this controller since its the only place // where we can be sure the modal was really presented to the user. @@ -56,6 +56,11 @@ module.exports = function($uibModalInstance, SettingsModel, UPDATE_NOTIFIER_SLEE * UpdateNotifierController.closeModal(); */ this.closeModal = () => { + AnalyticsService.logEvent('Close update modal', { + sleepUpdateCheck: this.sleepUpdateCheck, + notifyVersion: options.version + }); + $uibModalInstance.dismiss(); }; diff --git a/lib/gui/components/update-notifier/update-notifier.js b/lib/gui/components/update-notifier/update-notifier.js index f11b4856..b063bee8 100644 --- a/lib/gui/components/update-notifier/update-notifier.js +++ b/lib/gui/components/update-notifier/update-notifier.js @@ -26,7 +26,8 @@ const UpdateNotifier = angular.module(MODULE_NAME, [ require('../modal/modal'), require('../../models/settings'), require('../../utils/manifest-bind/manifest-bind'), - require('../../os/open-external/open-external') + require('../../os/open-external/open-external'), + require('../../modules/analytics') ]); /** diff --git a/lib/gui/modules/analytics.js b/lib/gui/modules/analytics.js index 0cc3b0dd..130b649b 100644 --- a/lib/gui/modules/analytics.js +++ b/lib/gui/modules/analytics.js @@ -24,9 +24,11 @@ const _ = require('lodash'); const angular = require('angular'); const username = require('username'); const isRunningInAsar = require('electron-is-running-in-asar'); -const app = require('electron').remote.app; const errors = require('../../shared/errors'); +const os = require('os'); const packageJSON = require('../../../package.json'); +const arch = require('arch'); +const utils = require('../../shared/utils'); // Force Mixpanel snippet to load Mixpanel locally // instead of using a CDN for performance reasons @@ -40,6 +42,26 @@ const analytics = angular.module(MODULE_NAME, [ require('../models/settings') ]); +/** + * @summary Get host architecture + * @function + * @private + * + * @description + * We need this because node's os.arch() returns the process architecture + * See: https://github.com/nodejs/node-v0.x-archive/issues/2862 + * + * @returns {String} Host architecture + * + * @example + * if (getHostArchitecture() === 'x64') { + * console.log('Host architecture is x64'); + * } + */ +const getHostArchitecture = () => { + return _.includes([ 'ia32', 'x64' ], process.arch) ? arch().replace('x86', 'ia32') : process.arch; +}; + // Mixpanel integration // https://github.com/kuhnza/angular-mixpanel @@ -47,17 +69,16 @@ analytics.config(($mixpanelProvider) => { $mixpanelProvider.apiKey('63e5fc4563e00928da67d1226364dd4c'); $mixpanelProvider.superProperties({ - - /* eslint-disable camelcase */ - - distinct_id: username.sync(), - - /* eslint-enable camelcase */ - - electron: app.getVersion(), + electron: process.versions.electron, node: process.version, arch: process.arch, - version: packageJSON.version + version: packageJSON.version, + osPlatform: os.platform(), + hostArch: getHostArchitecture(), + osRelease: os.release(), + cpuCores: os.cpus().length, + totalMemory: os.totalmem(), + startFreeMemory: os.freemem() }); }); @@ -120,18 +141,14 @@ analytics.service('AnalyticsService', function($log, $window, $mixpanel, Setting * }); */ this.logEvent = (message, data) => { + const flatStartCaseData = utils.makeFlatStartCaseObject(data); if (SettingsModel.get('errorReporting') && isRunningInAsar()) { - - // Clone data before passing it to `mixpanel.track` - // since this function mutates the object adding - // some custom private Mixpanel properties. - $mixpanel.track(message, _.clone(data)); - + $mixpanel.track(message, flatStartCaseData); } const debugMessage = _.attempt(() => { - if (data) { - return `${message} (${JSON.stringify(data)})`; + if (flatStartCaseData) { + return `${message} (${JSON.stringify(flatStartCaseData)})`; } return message; diff --git a/lib/gui/os/open-external/open-external.js b/lib/gui/os/open-external/open-external.js index 023882c3..7eb3752c 100644 --- a/lib/gui/os/open-external/open-external.js +++ b/lib/gui/os/open-external/open-external.js @@ -24,7 +24,9 @@ const angular = require('angular'); const url = require('url'); const MODULE_NAME = 'Etcher.OS.OpenExternal'; -const OSOpenExternal = angular.module(MODULE_NAME, []); +const OSOpenExternal = angular.module(MODULE_NAME, [ + require('../../modules/analytics') +]); OSOpenExternal.service('OSOpenExternalService', require('./services/open-external')); OSOpenExternal.directive('osOpenExternal', require('./directives/open-external')); diff --git a/lib/gui/os/open-external/services/open-external.js b/lib/gui/os/open-external/services/open-external.js index c73dc9bf..42dcdb9a 100644 --- a/lib/gui/os/open-external/services/open-external.js +++ b/lib/gui/os/open-external/services/open-external.js @@ -18,7 +18,7 @@ const electron = require('electron'); -module.exports = function() { +module.exports = function(AnalyticsService) { /** * @summary Open an external resource @@ -31,6 +31,10 @@ module.exports = function() { * OSOpenExternalService.open('https://www.google.com'); */ this.open = (url) => { + AnalyticsService.logEvent('Open external link', { + url + }); + if (url) { electron.shell.openExternal(url); } diff --git a/lib/gui/pages/main/controllers/drive-selection.js b/lib/gui/pages/main/controllers/drive-selection.js index 18dedc34..d9d7dcd6 100644 --- a/lib/gui/pages/main/controllers/drive-selection.js +++ b/lib/gui/pages/main/controllers/drive-selection.js @@ -16,7 +16,7 @@ 'use strict'; -module.exports = function(SelectionStateModel, AnalyticsService, ErrorService, DriveSelectorService) { +module.exports = function(SelectionStateModel, AnalyticsService, ErrorService, DriveSelectorService, SettingsModel) { /** * @summary Open drive selector @@ -35,7 +35,8 @@ module.exports = function(SelectionStateModel, AnalyticsService, ErrorService, D SelectionStateModel.setDrive(drive.device); AnalyticsService.logEvent('Select drive', { - device: drive.device + device: drive.device, + unsafeMode: SettingsModel.get('unsafeMode') }); }).catch(ErrorService.reportException); }; diff --git a/lib/gui/pages/main/controllers/flash.js b/lib/gui/pages/main/controllers/flash.js index ba8fc7c9..2beff46c 100644 --- a/lib/gui/pages/main/controllers/flash.js +++ b/lib/gui/pages/main/controllers/flash.js @@ -36,11 +36,14 @@ module.exports = function( * @function * @public * - * @param {String} image - image path + * @param {Object} image - image * @param {Object} drive - drive * * @example - * FlashController.flashImageToDrive('rpi.img', { + * FlashController.flashImageToDrive({ + * path: 'rpi.img', + * size: 1000000000 + * }, { * device: '/dev/disk2', * description: 'Foo', * size: 99999, @@ -59,10 +62,11 @@ module.exports = function( AnalyticsService.logEvent('Flash', { image, - device: drive.device + drive, + unmountOnSuccess: SettingsModel.get('unmountOnSuccess') }); - ImageWriterService.flash(image, drive).then(() => { + ImageWriterService.flash(image.path, drive).then(() => { if (FlashStateModel.wasLastFlashCancelled()) { return; } @@ -83,7 +87,9 @@ module.exports = function( } else { FlashErrorModalService.show(messages.error.genericFlashError()); ErrorService.reportException(error); - AnalyticsService.logEvent('Flash error'); + AnalyticsService.logEvent('Flash error', { + error + }); } }) diff --git a/lib/gui/pages/main/controllers/image-selection.js b/lib/gui/pages/main/controllers/image-selection.js index 0f16c59c..a04418fa 100644 --- a/lib/gui/pages/main/controllers/image-selection.js +++ b/lib/gui/pages/main/controllers/image-selection.js @@ -115,11 +115,14 @@ module.exports = function( * ImageSelectionController.openImageSelector(); */ this.openImageSelector = () => { + AnalyticsService.logEvent('Open image selector'); + OSDialogService.selectImage().then((image) => { // Avoid analytics and selection state changes // if no file was resolved from the dialog. if (!image) { + AnalyticsService.logEvent('Image selector closed'); return; } @@ -136,8 +139,11 @@ module.exports = function( * ImageSelectionController.reselectImage(); */ this.reselectImage = () => { + AnalyticsService.logEvent('Reselect image', { + previousImage: SelectionStateModel.getImage() + }); + this.openImageSelector(); - AnalyticsService.logEvent('Reselect image'); }; }; diff --git a/lib/gui/pages/main/controllers/main.js b/lib/gui/pages/main/controllers/main.js index 0aa8fcfc..a09162b1 100644 --- a/lib/gui/pages/main/controllers/main.js +++ b/lib/gui/pages/main/controllers/main.js @@ -23,7 +23,8 @@ module.exports = function( SettingsModel, TooltipModalService, ErrorService, - OSOpenExternalService + OSOpenExternalService, + AnalyticsService ) { // Expose several modules to the template for convenience @@ -76,6 +77,10 @@ module.exports = function( * MainController.showSelectedImageDetails() */ this.showSelectedImageDetails = () => { + AnalyticsService.logEvent('Show selected image tooltip', { + imagePath: SelectionStateModel.getImagePath() + }); + return TooltipModalService.show({ title: 'Image File Name', message: SelectionStateModel.getImagePath() diff --git a/lib/gui/pages/main/templates/main.tpl.html b/lib/gui/pages/main/templates/main.tpl.html index 3bbdf010..196a4672 100644 --- a/lib/gui/pages/main/templates/main.tpl.html +++ b/lib/gui/pages/main/templates/main.tpl.html @@ -83,7 +83,7 @@ percentage="main.state.getFlashState().percentage" striped="{{ main.state.getFlashState().type == 'check' }}" ng-attr-active="{{ main.state.isFlashing() }}" - ng-click="flash.flashImageToDrive(main.selection.getImagePath(), main.selection.getDrive())" + ng-click="flash.flashImageToDrive(main.selection.getImage(), main.selection.getDrive())" ng-disabled="main.shouldFlashStepBeDisabled() || main.state.getLastFlashErrorCode()"> diff --git a/lib/gui/pages/settings/controllers/settings.js b/lib/gui/pages/settings/controllers/settings.js index ba2ce0d2..c78b9cb6 100644 --- a/lib/gui/pages/settings/controllers/settings.js +++ b/lib/gui/pages/settings/controllers/settings.js @@ -17,8 +17,9 @@ 'use strict'; const os = require('os'); +const _ = require('lodash'); -module.exports = function(WarningModalService, SettingsModel, ErrorService) { +module.exports = function(WarningModalService, SettingsModel, ErrorService, AnalyticsService) { /** * @summary Client platform @@ -53,34 +54,48 @@ module.exports = function(WarningModalService, SettingsModel, ErrorService) { this.model = SettingsModel; /** - * @summary Enable a dangerous setting + * @summary Toggle setting * @function * @public * - * @param {String} name - setting name - * @param {Object} options - options - * @param {String} options.description - modal description - * @param {String} options.confirmationLabel - modal confirmation label + * @description + * If warningOptions is given, it should be an object having `description` and `confirmationLabel`; + * these will be used to present a user confirmation modal before enabling the setting. + * If warningOptions is missing, no confirmation modal is displayed. + * + * @param {String} setting - setting key + * @param {Object} [options] - options + * @param {String} [options.description] - warning modal description + * @param {String} [options.confirmationLabel] - warning modal confirmation label * @returns {Undefined} * * @example - * SettingsController.enableDangerousSetting('unsafeMode', { + * SettingsController.toggle('unsafeMode', { * description: 'Don\'t do this!', * confirmationLabel: 'Do it!' * }); */ - this.enableDangerousSetting = (name, options) => { - if (!this.currentData[name]) { - this.model.set(name, false); - return this.refreshSettings(); + this.toggle = (setting, options) => { + + const value = this.currentData[setting]; + const dangerous = !_.isUndefined(options); + + AnalyticsService.logEvent('Toggle setting', { + setting, + value, + dangerous + }); + + if (!value || !dangerous) { + return this.model.set(setting, value); } // Keep the checkbox unchecked until the user confirms - this.currentData[name] = false; + this.currentData[setting] = false; return WarningModalService.display(options).then((userAccepted) => { if (userAccepted) { - this.model.set(name, true); + this.model.set(setting, true); this.refreshSettings(); } }).catch(ErrorService.reportException); diff --git a/lib/gui/pages/settings/settings.js b/lib/gui/pages/settings/settings.js index 58ae9346..dc29f9a5 100644 --- a/lib/gui/pages/settings/settings.js +++ b/lib/gui/pages/settings/settings.js @@ -26,7 +26,8 @@ const SettingsPage = angular.module(MODULE_NAME, [ require('angular-ui-router'), require('../../components/warning-modal/warning-modal'), require('../../models/settings'), - require('../../modules/error') + require('../../modules/error'), + require('../../modules/analytics') ]); SettingsPage.controller('SettingsController', require('./controllers/settings')); diff --git a/lib/gui/pages/settings/templates/settings.tpl.html b/lib/gui/pages/settings/templates/settings.tpl.html index 509ea924..51e8e67a 100644 --- a/lib/gui/pages/settings/templates/settings.tpl.html +++ b/lib/gui/pages/settings/templates/settings.tpl.html @@ -5,7 +5,7 @@ @@ -15,7 +15,7 @@