diff --git a/lib/gui/app.js b/lib/gui/app.js index d470dd80..67623c66 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -148,7 +148,6 @@ app.controller('AppController', function( this.writer = ImageWriterService; this.settings = SettingsModel.data; this.tooltipModal = TooltipModalService; - this.success = true; this.handleError = (error) => { OSDialogService.showError(error); @@ -249,10 +248,14 @@ app.controller('AppController', function( preserveImage: true }); - this.success = true; + this.writer.resetState(); AnalyticsService.logEvent('Restart after failure'); }; + this.wasLastFlashSuccessful = () => { + return _.get(this.writer.getFlashResults(), 'passedValidation', true); + }; + this.flash = (image, drive) => { if (this.writer.isFlashing()) { @@ -268,22 +271,17 @@ app.controller('AppController', function( device: drive.device }); - return this.writer.flash(image, drive).then((results) => { + return this.writer.flash(image, drive).then(() => { + const results = ImageWriterService.getFlashResults(); if (results.cancelled) { return; } - // TODO: Find a better way to manage flash/check - // success/error state than a global boolean flag. - this.success = results.passedValidation; - - if (this.success) { + if (results.passedValidation) { OSNotificationService.send('Success!', 'Your flash is complete'); AnalyticsService.logEvent('Done'); - $state.go('success', { - checksum: results.sourceChecksum - }); + $state.go('success'); } else { OSNotificationService.send('Oops!', 'Looks like your flash has failed'); AnalyticsService.logEvent('Validation error'); diff --git a/lib/gui/models/store.js b/lib/gui/models/store.js index b6811809..e9a1bf0e 100644 --- a/lib/gui/models/store.js +++ b/lib/gui/models/store.js @@ -30,6 +30,7 @@ const DEFAULT_STATE = Immutable.fromJS({ availableDrives: [], selection: {}, isFlashing: false, + flashResults: {}, flashState: { percentage: 0, speed: 0 @@ -139,17 +140,58 @@ const storeReducer = (state, action) => { } case ACTIONS.RESET_FLASH_STATE: { - return state.set('flashState', DEFAULT_STATE.get('flashState')); + return state + .set('flashState', DEFAULT_STATE.get('flashState')) + .set('flashResults', DEFAULT_STATE.get('flashResults')); } case ACTIONS.SET_FLASHING_FLAG: { - return state.set('isFlashing', true); + return state + .set('isFlashing', true) + .set('flashResults', DEFAULT_STATE.get('flashResults')); } case ACTIONS.UNSET_FLASHING_FLAG: { - return storeReducer(state.set('isFlashing', false), { - type: ACTIONS.RESET_FLASH_STATE - }); + if (!action.data) { + throw new Error('Missing results'); + } + + if (_.isUndefined(action.data.passedValidation) || _.isNull(action.data.passedValidation)) { + throw new Error('Missing results passedValidation'); + } + + if (!_.isBoolean(action.data.passedValidation)) { + throw new Error(`Invalid results passedValidation: ${action.data.passedValidation}`); + } + + if (_.isUndefined(action.data.cancelled) || _.isNull(action.data.cancelled)) { + throw new Error('Missing results cancelled'); + } + + if (!_.isBoolean(action.data.cancelled)) { + throw new Error(`Invalid results cancelled: ${action.data.cancelled}`); + } + + if (action.data.cancelled && action.data.sourceChecksum) { + throw new Error('The sourceChecksum value can\'t exist if the flashing was cancelled'); + } + + if (action.data.cancelled && action.data.passedValidation) { + throw new Error('The passedValidation value can\'t be true if the flashing was cancelled'); + } + + if (action.data.passedValidation && !action.data.sourceChecksum) { + throw new Error('Missing results sourceChecksum'); + } + + if (action.data.passedValidation && !_.isString(action.data.sourceChecksum)) { + throw new Error(`Invalid results sourceChecksum: ${action.data.sourceChecksum}`); + } + + return state + .set('isFlashing', false) + .set('flashResults', Immutable.fromJS(action.data)) + .set('flashState', DEFAULT_STATE.get('flashState')); } case ACTIONS.SELECT_DRIVE: { diff --git a/lib/gui/modules/image-writer.js b/lib/gui/modules/image-writer.js index 4ea2fd8d..992c4b5b 100644 --- a/lib/gui/modules/image-writer.js +++ b/lib/gui/modules/image-writer.js @@ -21,6 +21,7 @@ */ const angular = require('angular'); +const _ = require('lodash'); const Store = require('../models/store'); const childWriter = require('../../src/child-writer'); @@ -81,28 +82,49 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) }; /** - * @summary Set the flashing status + * @summary Set the flashing flag * @function * @private * * @description * This function is extracted for testing purposes. * - * @param {Boolean} status - flashing status + * The flag is used to signify that we're going to + * start a flash process. * * @example - * ImageWriterService.setFlashing(true); + * ImageWriterService.setFlashingFlag(); */ - this.setFlashing = (status) => { - if (Boolean(status)) { - Store.dispatch({ - type: Store.Actions.SET_FLASHING_FLAG - }); - } else { - Store.dispatch({ - type: Store.Actions.UNSET_FLASHING_FLAG - }); - } + this.setFlashingFlag = () => { + Store.dispatch({ + type: Store.Actions.SET_FLASHING_FLAG + }); + }; + + /** + * @summary Unset the flashing flag + * @function + * @private + * + * @description + * This function is extracted for testing purposes. + * + * The flag is used to signify that the write process ended. + * + * @param {Object} results - flash results + * + * @example + * ImageWriterService.unsetFlashingFlag({ + * passedValidation: true, + * cancelled: false, + * sourceChecksum: 'a1b45d' + * }); + */ + this.unsetFlashingFlag = (results) => { + Store.dispatch({ + type: Store.Actions.UNSET_FLASHING_FLAG, + data: results + }); }; /** @@ -137,6 +159,20 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) }); }; + /** + * @summary Get the flash results + * @function + * @public + * + * @returns {Object} flash results + * + * @example + * const results = ImageWriterService.getFlashResults(); + */ + this.getFlashResults = () => { + return Store.getState().toJS().flashResults; + }; + /** * @summary Perform write operation * @function @@ -178,8 +214,6 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) * * @param {String} image - image path * @param {Object} drive - drive - * - * @fulfil {Object} flash results * @returns {Promise} * * @example @@ -194,10 +228,24 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) return $q.reject(new Error('There is already a flash in progress')); } - this.setFlashing(true); + this.setFlashingFlag(); - return this.performWrite(image, drive, this.setProgressState).finally(() => { - this.setFlashing(false); + return this.performWrite(image, drive, this.setProgressState).then((results) => { + + // TODO: Make sure `etcher-image-write` always + // sends a `cancelled` property. + _.defaults(results, { + cancelled: false + }); + + this.unsetFlashingFlag(results); + }).catch((error) => { + this.unsetFlashingFlag({ + cancelled: false, + passedValidation: false + }); + + return $q.reject(error); }); }; diff --git a/lib/gui/pages/finish/controllers/finish.js b/lib/gui/pages/finish/controllers/finish.js index 79a30e68..43500c06 100644 --- a/lib/gui/pages/finish/controllers/finish.js +++ b/lib/gui/pages/finish/controllers/finish.js @@ -16,7 +16,7 @@ 'use strict'; -module.exports = function($state, $stateParams, SelectionStateModel, AnalyticsService, SettingsModel) { +module.exports = function($state, ImageWriterService, SelectionStateModel, AnalyticsService, SettingsModel) { /** * @summary Settings data @@ -26,11 +26,11 @@ module.exports = function($state, $stateParams, SelectionStateModel, AnalyticsSe this.settings = SettingsModel.data; /** - * @summary Route params - * @type Object + * @summary Source checksum + * @type String * @public */ - this.params = $stateParams; + this.checksum = ImageWriterService.getFlashResults().sourceChecksum; /** * @summary Restart the flashing process diff --git a/lib/gui/pages/finish/finish.js b/lib/gui/pages/finish/finish.js index 54c27b5f..6c5e0484 100644 --- a/lib/gui/pages/finish/finish.js +++ b/lib/gui/pages/finish/finish.js @@ -31,6 +31,7 @@ const MODULE_NAME = 'Etcher.Pages.Finish'; const FinishPage = angular.module(MODULE_NAME, [ require('angular-ui-router'), require('../../modules/analytics'), + require('../../modules/image-writer'), require('../../models/selection-state'), require('../../models/settings') ]); @@ -40,7 +41,7 @@ FinishPage.controller('FinishController', require('./controllers/finish')); FinishPage.config(($stateProvider) => { $stateProvider .state('success', { - url: '/success/:checksum', + url: '/success', controller: 'FinishController as finish', templateUrl: './pages/finish/templates/success.tpl.html' }); diff --git a/lib/gui/pages/finish/templates/success.tpl.html b/lib/gui/pages/finish/templates/success.tpl.html index bd16af38..b10ce993 100644 --- a/lib/gui/pages/finish/templates/success.tpl.html +++ b/lib/gui/pages/finish/templates/success.tpl.html @@ -28,7 +28,7 @@ - CRC32 CHECKSUM : {{ ::finish.params.checksum }} + CRC32 CHECKSUM : {{ ::finish.checksum }} diff --git a/lib/gui/partials/main.html b/lib/gui/partials/main.html index a0e3c2dc..ad7db37d 100644 --- a/lib/gui/partials/main.html +++ b/lib/gui/partials/main.html @@ -84,7 +84,7 @@ percentage="app.writer.state.percentage" striped="{{ app.writer.state.type == 'check' }}" ng-attr-active="{{ app.writer.isFlashing() }}" - ng-show="app.success" + ng-show="app.wasLastFlashSuccessful()" ng-click="app.flash(app.selection.getImagePath(), app.selection.getDrive())" ng-disabled="!app.selection.hasImage() || !app.selection.hasDrive()"> Finishing... @@ -96,7 +96,7 @@ ng-bind="app.writer.state.percentage + '% Validating...'"> -
+
Your removable drive did not pass validation check.
Please insert another one and @@ -106,7 +106,7 @@
- diff --git a/tests/gui/modules/image-writer.spec.js b/tests/gui/modules/image-writer.spec.js index 3fbb4c97..7e96fdd1 100644 --- a/tests/gui/modules/image-writer.spec.js +++ b/tests/gui/modules/image-writer.spec.js @@ -37,7 +37,7 @@ describe('Browser: ImageWriter', function() { describe('.resetState()', function() { - it('should be able to reset the state', function() { + it('should be able to reset the progress state', function() { ImageWriterService.state = { percentage: 50, speed: 3 @@ -52,6 +52,17 @@ describe('Browser: ImageWriter', function() { }); }); + it('should be able to reset the progress state', function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); + + ImageWriterService.resetState(); + m.chai.expect(ImageWriterService.getFlashResults()).to.deep.equal({}); + }); + }); describe('.isFlashing()', function() { @@ -61,7 +72,7 @@ describe('Browser: ImageWriter', function() { }); it('should return true if flashing', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(ImageWriterService.isFlashing()).to.be.true; }); @@ -70,7 +81,12 @@ describe('Browser: ImageWriter', function() { describe('.setProgressState()', function() { it('should not allow setting the state if flashing is false', function() { - ImageWriterService.setFlashing(false); + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); + m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -82,7 +98,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if type is missing', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ percentage: 50, @@ -93,7 +109,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if type is not a string', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 1234, @@ -105,7 +121,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if percentage is missing', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -116,7 +132,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if percentage is not a number', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -128,7 +144,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if eta is missing', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -139,7 +155,7 @@ describe('Browser: ImageWriter', function() { }); it('should not throw if eta is equal to zero', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -151,7 +167,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if eta is not a number', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -163,7 +179,7 @@ describe('Browser: ImageWriter', function() { }); it('should throw if speed is missing', function() { - ImageWriterService.setFlashing(true); + ImageWriterService.setFlashingFlag(); m.chai.expect(function() { ImageWriterService.setProgressState({ type: 'write', @@ -175,28 +191,120 @@ describe('Browser: ImageWriter', function() { }); - describe('.setFlashing()', function() { + describe('.getFlashResults()', function() { - it('should be able to set flashing to true', function() { - ImageWriterService.setFlashing(true); - m.chai.expect(ImageWriterService.isFlashing()).to.be.true; + it('should get the flash results', function() { + ImageWriterService.setFlashingFlag(); + + const expectedResults = { + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }; + + ImageWriterService.unsetFlashingFlag(expectedResults); + const results = ImageWriterService.getFlashResults(); + m.chai.expect(results).to.deep.equal(expectedResults); + }); + + }); + + describe('.unsetFlashingFlag()', function() { + + it('should throw if no flashing results', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag(); + }).to.throw('Missing results'); + }); + + it('should throw if no passedValidation', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + cancelled: false, + sourceChecksum: '1234' + }); + }).to.throw('Missing results passedValidation'); + }); + + it('should throw if passedValidation is not boolean', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: 'true', + cancelled: false, + sourceChecksum: '1234' + }); + }).to.throw('Invalid results passedValidation: true'); + }); + + it('should throw if no cancelled', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + sourceChecksum: '1234' + }); + }).to.throw('Missing results cancelled'); + }); + + it('should throw if cancelled is not boolean', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: 'false', + sourceChecksum: '1234' + }); + }).to.throw('Invalid results cancelled: false'); + }); + + it('should throw if passedValidation is true and sourceChecksum does not exist', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false + }); + }).to.throw('Missing results sourceChecksum'); + }); + + it('should throw if passedValidation is true and sourceChecksum is not a string', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: 12345 + }); + }).to.throw('Invalid results sourceChecksum: 12345'); + }); + + it('should throw if cancelled is true and sourceChecksum exists', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: false, + cancelled: true, + sourceChecksum: '1234' + }); + }).to.throw('The sourceChecksum value can\'t exist if the flashing was cancelled'); + }); + + it('should throw if cancelled is true and passedValidation is true', function() { + m.chai.expect(function() { + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: true + }); + }).to.throw('The passedValidation value can\'t be true if the flashing was cancelled'); }); it('should be able to set flashing to false', function() { - ImageWriterService.setFlashing(false); + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); + m.chai.expect(ImageWriterService.isFlashing()).to.be.false; }); - it('should cast to boolean by default', function() { - ImageWriterService.setFlashing('hello'); - m.chai.expect(ImageWriterService.isFlashing()).to.be.true; - - ImageWriterService.setFlashing(''); - m.chai.expect(ImageWriterService.isFlashing()).to.be.false; - }); - - it('should reset the flashing state if set to false', function() { - ImageWriterService.setFlashing(true); + it('should reset the flashing state', function() { + ImageWriterService.setFlashingFlag(); ImageWriterService.setProgressState({ type: 'write', @@ -212,7 +320,11 @@ describe('Browser: ImageWriter', function() { speed: 0 }); - ImageWriterService.setFlashing(false); + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); $timeout.flush(); @@ -224,13 +336,40 @@ describe('Browser: ImageWriter', function() { }); + describe('.setFlashingFlag()', function() { + + it('should be able to set flashing to true', function() { + ImageWriterService.setFlashingFlag(); + m.chai.expect(ImageWriterService.isFlashing()).to.be.true; + }); + + it('should reset the flash results', function() { + const expectedResults = { + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }; + + ImageWriterService.unsetFlashingFlag(expectedResults); + const results = ImageWriterService.getFlashResults(); + m.chai.expect(results).to.deep.equal(expectedResults); + ImageWriterService.setFlashingFlag(); + m.chai.expect(ImageWriterService.getFlashResults()).to.deep.equal({}); + }); + + }); + describe('.flash()', function() { describe('given a succesful write', function() { beforeEach(function() { this.performWriteStub = m.sinon.stub(ImageWriterService, 'performWrite'); - this.performWriteStub.returns($q.resolve()); + this.performWriteStub.returns($q.resolve({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + })); }); afterEach(function() { @@ -238,14 +377,24 @@ describe('Browser: ImageWriter', function() { }); it('should set flashing to false when done', function() { - ImageWriterService.setFlashing(false); + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); + ImageWriterService.flash('foo.img', '/dev/disk2'); $rootScope.$apply(); m.chai.expect(ImageWriterService.isFlashing()).to.be.false; }); it('should prevent writing more than once', function() { - ImageWriterService.setFlashing(false); + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); + ImageWriterService.flash('foo.img', '/dev/disk2'); ImageWriterService.flash('foo.img', '/dev/disk2'); $rootScope.$apply(); @@ -286,7 +435,11 @@ describe('Browser: ImageWriter', function() { }); it('should be rejected with the error', function() { - ImageWriterService.setFlashing(false); + ImageWriterService.unsetFlashingFlag({ + passedValidation: true, + cancelled: false, + sourceChecksum: '1234' + }); let rejection; ImageWriterService.flash('foo.img', '/dev/disk2').catch(function(error) {