From 444072db13a1ffbc73e393fb0f6dc01caded70cb Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Mon, 16 Oct 2017 12:09:38 +0100 Subject: [PATCH] refactor(GUI): generalize the concept of a "pending" drive (#1777) This commit introduces a boolean `disabled` property rather than a `pending` flag. Making this distinction clearer means that we can now treat pending drives in different ways needed to improve the usbboot experience. Also, for usbboot, this commit removes the "pending" badge and uses a more descriptive drive description instead. Signed-off-by: Juan Cruz Viotti --- lib/shared/drive-constraints.js | 32 ++++++------------- lib/shared/sdk/standard/index.js | 1 - lib/shared/sdk/usbboot/index.js | 4 +-- tests/gui/modules/drive-scanner.spec.js | 4 --- tests/shared/drive-constraints.spec.js | 42 ++++++++++++------------- 5 files changed, 32 insertions(+), 51 deletions(-) diff --git a/lib/shared/drive-constraints.js b/lib/shared/drive-constraints.js index e32dacc2..1f7cb763 100644 --- a/lib/shared/drive-constraints.js +++ b/lib/shared/drive-constraints.js @@ -168,25 +168,25 @@ exports.isDriveLargeEnough = (drive, image) => { } /** - * @summary Check if a drive is pending (i.e. not ready for selection) + * @summary Check if a drive is disabled (i.e. not ready for selection) * @function * @public * * @param {Object} drive - drive - * @returns {Boolean} whether the drive is pending + * @returns {Boolean} whether the drive is disabled * * @example - * if (constraints.isDrivePending({ + * if (constraints.isDriveDisabled({ * device: '/dev/disk2', * name: 'My Drive', * size: 1000000000, - * pending: true + * disabled: true * })) { - * console.log('The drive is pending'); + * console.log('The drive is disabled'); * } */ -exports.isDrivePending = (drive) => { - return _.get(drive, [ 'pending' ], false) +exports.isDriveDisabled = (drive) => { + return _.get(drive, [ 'disabled' ], false) } /** @@ -217,7 +217,7 @@ exports.isDriveValid = (drive, image) => { !this.isDriveLocked(drive), this.isDriveLargeEnough(drive, image), !this.isSourceDrive(drive, image), - !this.isDrivePending(drive) + !this.isDriveDisabled(drive) ]) } @@ -301,15 +301,6 @@ exports.COMPATIBILITY_STATUS_MESSAGES = { */ SYSTEM: 'System Drive', - /** - * @property {String} PENDING - * @memberof COMPATIBILITY_STATUS_MESSAGES - * - * @description - * The drive is not fully loaded. - */ - PENDING: 'Pending', - /** * @property {String} CONTAINS_IMAGE * @memberof COMPATIBILITY_STATUS_MESSAGES @@ -374,12 +365,7 @@ exports.getDriveImageCompatibilityStatuses = (drive, image) => { const statusList = [] // Mind the order of the if-statements if you modify. - if (exports.isDrivePending(drive)) { - statusList.push({ - type: exports.COMPATIBILITY_STATUS_TYPES.ERROR, - message: exports.COMPATIBILITY_STATUS_MESSAGES.PENDING - }) - } else if (exports.isSourceDrive(drive, image)) { + if (exports.isSourceDrive(drive, image)) { statusList.push({ type: exports.COMPATIBILITY_STATUS_TYPES.ERROR, message: exports.COMPATIBILITY_STATUS_MESSAGES.CONTAINS_IMAGE diff --git a/lib/shared/sdk/standard/index.js b/lib/shared/sdk/standard/index.js index ad9f9b13..50118c4a 100644 --- a/lib/shared/sdk/standard/index.js +++ b/lib/shared/sdk/standard/index.js @@ -49,7 +49,6 @@ exports.scan = (options = {}) => { return drivelist.listAsync().filter((drive) => { return options.includeSystemDrives || !drive.system }).map((drive) => { - drive.pending = false drive.adaptor = exports.name // TODO: Find a better way to detect that a certain diff --git a/lib/shared/sdk/usbboot/index.js b/lib/shared/sdk/usbboot/index.js index 3f1708e6..8c243bff 100644 --- a/lib/shared/sdk/usbboot/index.js +++ b/lib/shared/sdk/usbboot/index.js @@ -327,13 +327,13 @@ exports.scan = (options) => { ], ':') device.device = idPair - device.displayName = idPair + device.displayName = 'Initializing device' device.raw = idPair device.size = null device.mountpoints = [] device.protected = false device.system = false - device.pending = true + device.disabled = true device.adaptor = exports.name // We need to open the device in order to access _configDescriptor diff --git a/tests/gui/modules/drive-scanner.spec.js b/tests/gui/modules/drive-scanner.spec.js index 65647a6b..f406d04f 100644 --- a/tests/gui/modules/drive-scanner.spec.js +++ b/tests/gui/modules/drive-scanner.spec.js @@ -160,7 +160,6 @@ describe('Browser: driveScanner', function () { path: '/mnt/foo' } ], - pending: false, adaptor: 'standard', system: false }, @@ -174,7 +173,6 @@ describe('Browser: driveScanner', function () { path: '/mnt/bar' } ], - pending: false, adaptor: 'standard', system: false } @@ -256,7 +254,6 @@ describe('Browser: driveScanner', function () { description: 'Foo', size: '14G', mountpoints: [], - pending: false, adaptor: 'standard', system: false }, @@ -270,7 +267,6 @@ describe('Browser: driveScanner', function () { path: 'F:' } ], - pending: false, adaptor: 'standard', system: false } diff --git a/tests/shared/drive-constraints.spec.js b/tests/shared/drive-constraints.spec.js index 6deeb884..d8f42c4a 100644 --- a/tests/shared/drive-constraints.spec.js +++ b/tests/shared/drive-constraints.spec.js @@ -527,33 +527,33 @@ describe('Shared: DriveConstraints', function () { }) }) - describe('.isDrivePending()', function () { - it('should return true if the drive is pending', function () { - const result = constraints.isDrivePending({ + describe('.isDriveDisabled()', function () { + it('should return true if the drive is disabled', function () { + const result = constraints.isDriveDisabled({ device: '/dev/disk1', name: 'USB Drive', size: 1000000000, protected: false, - pending: true + disabled: true }) m.chai.expect(result).to.be.true }) - it('should return false if the drive is not pending', function () { - const result = constraints.isDrivePending({ + it('should return false if the drive is not disabled', function () { + const result = constraints.isDriveDisabled({ device: '/dev/disk1', name: 'USB Drive', size: 1000000000, protected: false, - pending: false + disabled: false }) m.chai.expect(result).to.be.false }) - it('should return false if "pending" is undefined', function () { - const result = constraints.isDrivePending({ + it('should return false if "disabled" is undefined', function () { + const result = constraints.isDriveDisabled({ device: '/dev/disk1', name: 'USB Drive', size: 1000000000, @@ -676,9 +676,9 @@ describe('Shared: DriveConstraints', function () { this.drive.protected = true }) - describe('given the drive is pending', function () { + describe('given the drive is disabled', function () { beforeEach(function () { - this.drive.pending = true + this.drive.disabled = true }) it('should return false if the drive is not large enough and is a source drive', function () { @@ -734,9 +734,9 @@ describe('Shared: DriveConstraints', function () { }) }) - describe('given the drive is not pending', function () { + describe('given the drive is not disabled', function () { beforeEach(function () { - this.drive.pending = false + this.drive.disabled = false }) it('should return false if the drive is not large enough and is a source drive', function () { @@ -798,9 +798,9 @@ describe('Shared: DriveConstraints', function () { this.drive.protected = false }) - describe('given the drive is pending', function () { + describe('given the drive is disabled', function () { beforeEach(function () { - this.drive.pending = true + this.drive.disabled = true }) it('should return false if the drive is not large enough and is a source drive', function () { @@ -856,9 +856,9 @@ describe('Shared: DriveConstraints', function () { }) }) - describe('given the drive is not pending', function () { + describe('given the drive is not disabled', function () { beforeEach(function () { - this.drive.pending = false + this.drive.disabled = false }) it('should return false if the drive is not large enough and is a source drive', function () { @@ -931,7 +931,7 @@ describe('Shared: DriveConstraints', function () { name: 'My Drive', protected: false, system: false, - pending: false, + disabled: false, mountpoints: [ { path: this.mountpoint @@ -975,15 +975,15 @@ describe('Shared: DriveConstraints', function () { }) }) - describe('given the drive is pending', () => { + describe('given the drive is disabled', () => { it('should return an empty list', function () { - this.drive.pending = true + this.drive.disabled = true const result = constraints.getDriveImageCompatibilityStatuses(this.drive, { path: '/mnt/disk2/rpi.img', size: 1000000000 }) - const expectedTuples = [ [ 'ERROR', 'PENDING' ] ] + const expectedTuples = [] expectStatusTypesAndMessagesToBe(result, expectedTuples) }) })