From 14b04413b5866fb1387b8429676b37a37ddb8a41 Mon Sep 17 00:00:00 2001 From: Benedict Aas Date: Mon, 5 Jun 2017 15:30:58 +0100 Subject: [PATCH] refactor(GUI): remove angular from DrivesModel (#1265) We remove usage of Angular from DrivesModel. Depends: https://github.com/resin-io/etcher/pull/1261 Depends: https://github.com/resin-io/etcher/pull/1264 * remove angular injection from tests * move file * add empty array test --- lib/gui/app.js | 6 +- .../controllers/drive-selector.js | 6 +- .../drive-selector/drive-selector.js | 1 - lib/gui/models/available-drives.js | 70 ++++++++++++++++ lib/gui/models/drives.js | 83 ------------------- lib/gui/models/selection-state.js | 9 +- lib/gui/pages/main/controllers/main.js | 4 +- lib/gui/pages/main/main.js | 1 - tests/gui/components/drive-selector.spec.js | 5 +- tests/gui/models/drives.spec.js | 61 +++++++------- tests/gui/models/selection-state.spec.js | 31 +++---- tests/gui/pages/main.spec.js | 9 +- 12 files changed, 132 insertions(+), 154 deletions(-) create mode 100644 lib/gui/models/available-drives.js delete mode 100644 lib/gui/models/drives.js diff --git a/lib/gui/app.js b/lib/gui/app.js index 889b6135..674fc9e9 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -40,6 +40,7 @@ const settings = require('./models/settings'); const windowProgress = require('./os/window-progress'); const analytics = require('./modules/analytics'); const updateNotifier = require('./components/update-notifier'); +const availableDrives = require('./models/available-drives'); const Store = require('./models/store'); @@ -54,7 +55,6 @@ const app = angular.module('Etcher', [ // Models require('./models/selection-state'), - require('./models/drives'), // Components require('./components/svg-icon/svg-icon'), @@ -181,7 +181,7 @@ app.run(() => { }); }); -app.run(($timeout, DriveScannerService, DrivesModel, ErrorService) => { +app.run(($timeout, DriveScannerService, ErrorService) => { DriveScannerService.on('drives', (drives) => { // Safely trigger a digest cycle. @@ -189,7 +189,7 @@ app.run(($timeout, DriveScannerService, DrivesModel, ErrorService) => { // available drives list has changed, and incorrectly // keeps asking the user to "Connect a drive". $timeout(() => { - DrivesModel.setDrives(drives); + availableDrives.setDrives(drives); }); }); diff --git a/lib/gui/components/drive-selector/controllers/drive-selector.js b/lib/gui/components/drive-selector/controllers/drive-selector.js index 8ae0eb2a..7aecf825 100644 --- a/lib/gui/components/drive-selector/controllers/drive-selector.js +++ b/lib/gui/components/drive-selector/controllers/drive-selector.js @@ -21,11 +21,11 @@ const _ = require('lodash'); const messages = require('../../../../shared/messages'); const constraints = require('../../../../shared/drive-constraints'); const analytics = require('../../../modules/analytics'); +const availableDrives = require('../../../models/available-drives'); module.exports = function( $q, $uibModalInstance, - DrivesModel, SelectionStateModel, WarningModalService ) { @@ -53,9 +53,9 @@ module.exports = function( * We expose the whole service instead of the `.drives` * property, which is the one we're interested in since * this allows the property to be automatically updated - * when `DrivesModel` detects a change in the drives. + * when `availableDrives` detects a change in the drives. */ - this.drives = DrivesModel; + this.drives = availableDrives; /** * @summary Determine if we can change a drive's selection state diff --git a/lib/gui/components/drive-selector/drive-selector.js b/lib/gui/components/drive-selector/drive-selector.js index fe233181..658300d1 100644 --- a/lib/gui/components/drive-selector/drive-selector.js +++ b/lib/gui/components/drive-selector/drive-selector.js @@ -25,7 +25,6 @@ const MODULE_NAME = 'Etcher.Components.DriveSelector'; const DriveSelector = angular.module(MODULE_NAME, [ require('../modal/modal'), require('../warning-modal/warning-modal'), - require('../../models/drives'), require('../../models/selection-state'), require('../../utils/byte-size/byte-size') ]); diff --git a/lib/gui/models/available-drives.js b/lib/gui/models/available-drives.js new file mode 100644 index 00000000..d9f4241c --- /dev/null +++ b/lib/gui/models/available-drives.js @@ -0,0 +1,70 @@ +/* + * Copyright 2016 resin.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +const _ = require('lodash'); +const Store = require('./store'); + +/** + * @summary Check if there are available drives + * @function + * @public + * + * @returns {Boolean} whether there are available drives + * + * @example + * if (availableDrives.hasAvailableDrives()) { + * console.log('There are available drives!'); + * } + */ +exports.hasAvailableDrives = () => { + return !_.isEmpty(exports.getDrives()); +}; + +/** + * @summary Set a list of drives + * @function + * @private + * + * @param {Object[]} drives - drives + * + * @throws Will throw if no drives + * @throws Will throw if drives is not an array of objects + * + * @example + * availableDrives.setDrives([ ... ]); + */ +exports.setDrives = (drives) => { + Store.dispatch({ + type: Store.Actions.SET_AVAILABLE_DRIVES, + data: drives + }); +}; + +/** + * @summary Get detected drives + * @function + * @private + * + * @returns {Object[]} drives + * + * @example + * const drives = availableDrives.getDrives(); + */ +exports.getDrives = () => { + return Store.getState().toJS().availableDrives; +}; diff --git a/lib/gui/models/drives.js b/lib/gui/models/drives.js deleted file mode 100644 index 617d5c69..00000000 --- a/lib/gui/models/drives.js +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Copyright 2016 resin.io - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -/** - * @module Etcher.Models.Drives - */ - -const angular = require('angular'); -const _ = require('lodash'); -const Store = require('./store'); -const MODULE_NAME = 'Etcher.Models.Drives'; -const Drives = angular.module(MODULE_NAME, []); - -Drives.service('DrivesModel', function() { - - /** - * @summary Check if there are available drives - * @function - * @public - * - * @returns {Boolean} whether there are available drives - * - * @example - * if (DrivesModel.hasAvailableDrives()) { - * console.log('There are available drives!'); - * } - */ - this.hasAvailableDrives = () => { - return !_.isEmpty(this.getDrives()); - }; - - /** - * @summary Set a list of drives - * @function - * @private - * - * @param {Object[]} drives - drives - * - * @throws Will throw if no drives - * @throws Will throw if drives is not an array of objects - * - * @example - * DrivesModel.setDrives([ ... ]); - */ - this.setDrives = (drives) => { - Store.dispatch({ - type: Store.Actions.SET_AVAILABLE_DRIVES, - data: drives - }); - }; - - /** - * @summary Get detected drives - * @function - * @private - * - * @returns {Object[]} drives - * - * @example - * const drives = DrivesModel.getDrives(); - */ - this.getDrives = () => { - return Store.getState().toJS().availableDrives; - }; - -}); - -module.exports = MODULE_NAME; diff --git a/lib/gui/models/selection-state.js b/lib/gui/models/selection-state.js index 3c856f1f..04c1b10e 100644 --- a/lib/gui/models/selection-state.js +++ b/lib/gui/models/selection-state.js @@ -23,12 +23,11 @@ const _ = require('lodash'); const angular = require('angular'); const Store = require('./store'); +const availableDrives = require('./available-drives'); const MODULE_NAME = 'Etcher.Models.SelectionState'; -const SelectionStateModel = angular.module(MODULE_NAME, [ - require('./drives') -]); +const SelectionStateModel = angular.module(MODULE_NAME, []); -SelectionStateModel.service('SelectionStateModel', function(DrivesModel) { +SelectionStateModel.service('SelectionStateModel', function() { /** * @summary Set a drive @@ -95,7 +94,7 @@ SelectionStateModel.service('SelectionStateModel', function(DrivesModel) { * const drive = SelectionStateModel.getDrive(); */ this.getDrive = () => { - return _.find(DrivesModel.getDrives(), { + return _.find(availableDrives.getDrives(), { device: Store.getState().getIn([ 'selection', 'drive' ]) }); }; diff --git a/lib/gui/pages/main/controllers/main.js b/lib/gui/pages/main/controllers/main.js index d6b85d85..9072fe5f 100644 --- a/lib/gui/pages/main/controllers/main.js +++ b/lib/gui/pages/main/controllers/main.js @@ -19,10 +19,10 @@ const settings = require('../../../models/settings'); const flashState = require('../../../models/flash-state'); const analytics = require('../../../modules/analytics'); +const availableDrives = require('../../../models/available-drives'); module.exports = function( SelectionStateModel, - DrivesModel, TooltipModalService, ErrorService, OSOpenExternalService @@ -30,7 +30,7 @@ module.exports = function( // Expose several modules to the template for convenience this.selection = SelectionStateModel; - this.drives = DrivesModel; + this.drives = availableDrives; this.state = flashState; this.settings = settings; this.external = OSOpenExternalService; diff --git a/lib/gui/pages/main/main.js b/lib/gui/pages/main/main.js index fc8d9220..4896342b 100644 --- a/lib/gui/pages/main/main.js +++ b/lib/gui/pages/main/main.js @@ -47,7 +47,6 @@ const MainPage = angular.module(MODULE_NAME, [ require('../../modules/image-writer'), require('../../modules/error'), require('../../models/selection-state'), - require('../../models/drives'), require('../../utils/byte-size/byte-size') ]); diff --git a/tests/gui/components/drive-selector.spec.js b/tests/gui/components/drive-selector.spec.js index 55066185..d7d584cb 100644 --- a/tests/gui/components/drive-selector.spec.js +++ b/tests/gui/components/drive-selector.spec.js @@ -17,7 +17,6 @@ describe('Browser: DriveSelector', function() { let $rootScope; let $q; let $uibModalInstance; - let DrivesModel; let SelectionStateModel; let WarningModalService; @@ -27,14 +26,13 @@ describe('Browser: DriveSelector', function() { _$controller_, _$rootScope_, _$q_, - _DrivesModel_, _SelectionStateModel_, + _SelectionStateModel_, _WarningModalService_ ) { $controller = _$controller_; $rootScope = _$rootScope_; $q = _$q_; $uibModalInstance = {}; - DrivesModel = _DrivesModel_; SelectionStateModel = _SelectionStateModel_; WarningModalService = _WarningModalService_; })); @@ -44,7 +42,6 @@ describe('Browser: DriveSelector', function() { $scope: $rootScope.$new(), $q, $uibModalInstance, - DrivesModel, SelectionStateModel, WarningModalService }); diff --git a/tests/gui/models/drives.spec.js b/tests/gui/models/drives.spec.js index c103a91f..dff13f7e 100644 --- a/tests/gui/models/drives.spec.js +++ b/tests/gui/models/drives.spec.js @@ -3,46 +3,44 @@ const m = require('mochainon'); const path = require('path'); const angular = require('angular'); +const availableDrives = require('../../../lib/gui/models/available-drives'); require('angular-mocks'); -describe('Browser: DrivesModel', function() { +describe('Browser: availableDrives', function() { beforeEach(angular.mock.module( - require('../../../lib/gui/models/drives'), require('../../../lib/gui/models/selection-state') )); - describe('DrivesModel', function() { + describe('availableDrives', function() { - let DrivesModel; let SelectionStateModel; - beforeEach(angular.mock.inject(function(_DrivesModel_, _SelectionStateModel_) { - DrivesModel = _DrivesModel_; + beforeEach(angular.mock.inject(function(_SelectionStateModel_) { SelectionStateModel = _SelectionStateModel_; })); it('should have no drives by default', function() { - m.chai.expect(DrivesModel.getDrives()).to.deep.equal([]); + m.chai.expect(availableDrives.getDrives()).to.deep.equal([]); }); describe('.setDrives()', function() { it('should throw if no drives', function() { m.chai.expect(function() { - DrivesModel.setDrives(); + availableDrives.setDrives(); }).to.throw('Missing drives'); }); it('should throw if drives is not an array', function() { m.chai.expect(function() { - DrivesModel.setDrives(123); + availableDrives.setDrives(123); }).to.throw('Invalid drives: 123'); }); it('should throw if drives is not an array of objects', function() { m.chai.expect(function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ 123, 123, 123 @@ -57,7 +55,7 @@ describe('Browser: DrivesModel', function() { describe('.hasAvailableDrives()', function() { it('should return false', function() { - m.chai.expect(DrivesModel.hasAvailableDrives()).to.be.false; + m.chai.expect(availableDrives.hasAvailableDrives()).to.be.false; }); }); @@ -75,8 +73,8 @@ describe('Browser: DrivesModel', function() { } ]; - DrivesModel.setDrives(drives); - m.chai.expect(DrivesModel.getDrives()).to.deep.equal(drives); + availableDrives.setDrives(drives); + m.chai.expect(availableDrives.getDrives()).to.deep.equal(drives); }); describe('given no selected image and no selected drive', function() { @@ -89,7 +87,7 @@ describe('Browser: DrivesModel', function() { it('should auto-select a single valid available drive', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -137,7 +135,7 @@ describe('Browser: DrivesModel', function() { it('should not auto-select when there are multiple valid available drives', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -162,7 +160,7 @@ describe('Browser: DrivesModel', function() { it('should auto-select a single valid available drive', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -186,7 +184,7 @@ describe('Browser: DrivesModel', function() { it('should not auto-select a single too small drive', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -203,7 +201,7 @@ describe('Browser: DrivesModel', function() { it('should not auto-select a single drive that doesn\'t meet the recommended size', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -220,7 +218,7 @@ describe('Browser: DrivesModel', function() { it('should not auto-select a single protected drive', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -237,7 +235,7 @@ describe('Browser: DrivesModel', function() { it('should not auto-select a source drive', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -258,7 +256,7 @@ describe('Browser: DrivesModel', function() { it('should not auto-select a single system drive', function() { m.chai.expect(SelectionStateModel.hasDrive()).to.be.false; - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', name: 'Foo', @@ -300,13 +298,13 @@ describe('Browser: DrivesModel', function() { } ]; - DrivesModel.setDrives(this.drives); + availableDrives.setDrives(this.drives); }); describe('given one of the drives was selected', function() { beforeEach(function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdc', name: 'USB Drive', @@ -330,7 +328,7 @@ describe('Browser: DrivesModel', function() { // We have to provide at least two drives, otherwise, // if we only provide one, the single drive will be // auto-selected. - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sda', name: 'USB Drive', @@ -357,7 +355,7 @@ describe('Browser: DrivesModel', function() { describe('.hasAvailableDrives()', function() { it('should return true', function() { - const hasDrives = DrivesModel.hasAvailableDrives(); + const hasDrives = availableDrives.hasAvailableDrives(); m.chai.expect(hasDrives).to.be.true; }); @@ -366,14 +364,19 @@ describe('Browser: DrivesModel', function() { describe('.setDrives()', function() { it('should keep the same drives if equal', function() { - DrivesModel.setDrives(this.drives); - m.chai.expect(DrivesModel.getDrives()).to.deep.equal(this.drives); + availableDrives.setDrives(this.drives); + m.chai.expect(availableDrives.getDrives()).to.deep.equal(this.drives); + }); + + it('should return empty array given an empty array', function() { + availableDrives.setDrives([]); + m.chai.expect(availableDrives.getDrives()).to.deep.equal([]); }); it('should consider drives with different $$hashKey the same', function() { this.drives[0].$$haskey = 1234; - DrivesModel.setDrives(this.drives); - m.chai.expect(DrivesModel.getDrives()).to.deep.equal(this.drives); + availableDrives.setDrives(this.drives); + m.chai.expect(availableDrives.getDrives()).to.deep.equal(this.drives); }); }); diff --git a/tests/gui/models/selection-state.spec.js b/tests/gui/models/selection-state.spec.js index 012c7fcf..d9d9939a 100644 --- a/tests/gui/models/selection-state.spec.js +++ b/tests/gui/models/selection-state.spec.js @@ -4,6 +4,7 @@ const m = require('mochainon'); const _ = require('lodash'); const path = require('path'); const angular = require('angular'); +const availableDrives = require('../../../lib/gui/models/available-drives'); require('angular-mocks'); describe('Browser: SelectionState', function() { @@ -12,18 +13,12 @@ describe('Browser: SelectionState', function() { require('../../../lib/gui/models/selection-state') )); - beforeEach(angular.mock.module( - require('../../../lib/gui/models/drives') - )); - describe('SelectionStateModel', function() { let SelectionStateModel; - let DrivesModel; - beforeEach(angular.mock.inject(function(_SelectionStateModel_, _DrivesModel_) { + beforeEach(angular.mock.inject(function(_SelectionStateModel_) { SelectionStateModel = _SelectionStateModel_; - DrivesModel = _DrivesModel_; })); describe('given a clean state', function() { @@ -84,7 +79,7 @@ describe('Browser: SelectionState', function() { describe('given a drive', function() { beforeEach(function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk2', name: 'USB Drive', @@ -157,7 +152,7 @@ describe('Browser: SelectionState', function() { describe('.setDrive()', function() { it('should be able to set a drive', function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk5', name: 'USB Drive', @@ -177,7 +172,7 @@ describe('Browser: SelectionState', function() { }); it('should throw if drive is write protected', function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk1', name: 'USB Drive', @@ -192,7 +187,7 @@ describe('Browser: SelectionState', function() { }); it('should throw if the drive is not available', function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk1', name: 'USB Drive', @@ -242,7 +237,7 @@ describe('Browser: SelectionState', function() { describe('.setDrive()', function() { it('should throw if drive is not large enough', function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk2', name: 'USB Drive', @@ -762,7 +757,7 @@ describe('Browser: SelectionState', function() { }); it('should de-select a previously selected not-large-enough drive', function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk1', name: 'USB Drive', @@ -791,7 +786,7 @@ describe('Browser: SelectionState', function() { }); it('should de-select a previously selected not-recommended drive', function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk1', name: 'USB Drive', @@ -829,7 +824,7 @@ describe('Browser: SelectionState', function() { return '/mnt/bar/foo.img'; }); - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk1', name: 'USB Drive', @@ -869,7 +864,7 @@ describe('Browser: SelectionState', function() { describe('given a drive', function() { beforeEach(function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk1', name: 'USB Drive', @@ -949,7 +944,7 @@ describe('Browser: SelectionState', function() { describe('given a selected drive', function() { beforeEach(function() { - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/sdb', description: 'DataTraveler 2.0', @@ -1011,7 +1006,7 @@ describe('Browser: SelectionState', function() { protected: false }; - DrivesModel.setDrives([ + availableDrives.setDrives([ this.drive, { device: '/dev/disk2', diff --git a/tests/gui/pages/main.spec.js b/tests/gui/pages/main.spec.js index fa590338..c21df8cf 100644 --- a/tests/gui/pages/main.spec.js +++ b/tests/gui/pages/main.spec.js @@ -7,6 +7,7 @@ const supportedFormats = require('../../../lib/shared/supported-formats'); const angular = require('angular'); const settings = require('../../../lib/gui/models/settings'); const flashState = require('../../../lib/gui/models/flash-state'); +const availableDrives = require('../../../lib/gui/models/available-drives'); require('angular-mocks'); describe('Browser: MainPage', function() { @@ -19,12 +20,10 @@ describe('Browser: MainPage', function() { let $controller; let SelectionStateModel; - let DrivesModel; - beforeEach(angular.mock.inject(function(_$controller_, _SelectionStateModel_, _DrivesModel_) { + beforeEach(angular.mock.inject(function(_$controller_, _SelectionStateModel_) { $controller = _$controller_; SelectionStateModel = _SelectionStateModel_; - DrivesModel = _DrivesModel_; })); describe('.shouldDriveStepBeDisabled()', function() { @@ -99,7 +98,7 @@ describe('Browser: MainPage', function() { $scope: {} }); - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk2', description: 'Foo', @@ -120,7 +119,7 @@ describe('Browser: MainPage', function() { $scope: {} }); - DrivesModel.setDrives([ + availableDrives.setDrives([ { device: '/dev/disk2', description: 'Foo',