From b88a45aa797f02343b971e0b92a19cf8f3b14714 Mon Sep 17 00:00:00 2001 From: Benedict Aas Date: Tue, 24 Apr 2018 22:04:36 +0100 Subject: [PATCH] refactor(GUI): make the finish notification message concise (#2268) We make the finish notification message print the device name as usual when there's one target, and instead list quantity of successful and failed devices when there are multiple. Previously it would list all device names, and wouldn't specify how many were successful or failures. Change-Type: patch --- lib/gui/app/pages/main/controllers/flash.js | 5 +- lib/shared/messages.js | 38 +++++++----- tests/shared/messages.spec.js | 69 +++++++++++++++++++++ 3 files changed, 95 insertions(+), 17 deletions(-) diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js index 1d7a8eba..d52e68dc 100644 --- a/lib/gui/app/pages/main/controllers/flash.js +++ b/lib/gui/app/pages/main/controllers/flash.js @@ -151,8 +151,9 @@ module.exports = function ( return imageWriter.flash(image.path, devices) }).then(() => { if (!flashState.wasLastFlashCancelled()) { - notification.send('Success!', { - body: messages.info.flashComplete(path.basename(image.path), drives), + const flashResults = flashState.getFlashResults() + notification.send('Flash complete!', { + body: messages.info.flashComplete(path.basename(image.path), drives, flashResults.results.devices), icon: iconPath }) $state.go('success') diff --git a/lib/shared/messages.js b/lib/shared/messages.js index 1c336f7d..8d01ec46 100644 --- a/lib/shared/messages.js +++ b/lib/shared/messages.js @@ -52,14 +52,23 @@ module.exports = { */ info: { - flashComplete: (imageBasename, drives) => { - return [ - `${imageBasename} was successfully written to`, - // eslint-disable-next-line lodash/prefer-lodash-method - drives.map((drive) => { - return `${drive.description} (${drive.displayName})` - }).join(', ') - ].join(' ') + flashComplete: (imageBasename, [ drive ], { failed, succeeded: successful }) => { + /* eslint-disable no-magic-numbers */ + const targets = [] + if (failed + successful === 1) { + targets.push(`to ${drive.description} (${drive.displayName})`) + } else { + if (successful) { + const plural = successful === 1 ? '' : 's' + targets.push(`to ${successful} target${plural}`) + } + if (failed) { + const plural = failed === 1 ? '' : 's' + targets.push(`and failed to be flashed to ${failed} target${plural}`) + } + } + return `${imageBasename} was successfully flashed ${targets.join(' ')}` + /* eslint-enable no-magic-numbers */ } }, @@ -189,13 +198,12 @@ module.exports = { }, flashFailure: (imageBasename, drives) => { - return [ - `Something went wrong while writing ${imageBasename} to`, - // eslint-disable-next-line lodash/prefer-lodash-method - drives.map((drive) => { - return `${drive.description} (${drive.displayName})` - }).join(', ') - ].join(' ') + /* eslint-disable no-magic-numbers */ + const target = drives.length === 1 + ? `${drives[0].description} (${drives[0].displayName})` + : `${drives.length} targets` + return `Something went wrong while writing ${imageBasename} to ${target}.` + /* eslint-enable no-magic-numbers */ }, driveUnplugged: () => { diff --git a/tests/shared/messages.spec.js b/tests/shared/messages.spec.js index 3fe2618a..3733a493 100644 --- a/tests/shared/messages.spec.js +++ b/tests/shared/messages.spec.js @@ -21,6 +21,19 @@ const _ = require('lodash') const messages = require('../../lib/shared/messages') describe('Shared: Messages', function () { + beforeEach(function () { + this.drives = [ + { + description: 'My Drive', + displayName: '/dev/disk1' + }, + { + description: 'Other Drive', + displayName: '/dev/disk2' + } + ] + }) + it('should contain object properties', function () { m.chai.expect(_.every(_.map(messages, _.isPlainObject))).to.be.true }) @@ -30,4 +43,60 @@ describe('Shared: Messages', function () { m.chai.expect(_.every(_.map(category, _.isFunction))).to.be.true }) }) + + describe('.info', function () { + describe('.flashComplete()', function () { + it('should use singular when there are single results', function () { + const msg = messages.info.flashComplete('image.img', this.drives, { + failed: 1, + succeeded: 1 + }) + + m.chai.expect(msg).to.equal('image.img was successfully flashed to 1 target and failed to be flashed to 1 target') + }) + + it('should use plural when there are multiple results', function () { + const msg = messages.info.flashComplete('image.img', this.drives, { + failed: 2, + succeeded: 2 + }) + + m.chai.expect(msg).to.equal('image.img was successfully flashed to 2 targets and failed to be flashed to 2 targets') + }) + + it('should not contain failed target part when there are none', function () { + const msg = messages.info.flashComplete('image.img', this.drives, { + failed: 0, + succeeded: 2 + }) + + m.chai.expect(msg).to.equal('image.img was successfully flashed to 2 targets') + }) + + it('should show drive name and description when only target', function () { + const msg = messages.info.flashComplete('image.img', this.drives, { + failed: 0, + succeeded: 1 + }) + + m.chai.expect(msg).to.equal('image.img was successfully flashed to My Drive (/dev/disk1)') + }) + }) + }) + + describe('.error', function () { + describe('.flashFailure()', function () { + it('should use plural when there are multiple drives', function () { + const msg = messages.error.flashFailure('image.img', this.drives) + + m.chai.expect(msg).to.equal('Something went wrong while writing image.img to 2 targets.') + }) + + it('should use singular when there is one drive', function () { + const msg = messages.error.flashFailure('image.img', [ this.drives[0] ]) + + m.chai.expect(msg).to.equal('Something went wrong while writing image.img to My Drive (/dev/disk1).') + }) + }) + }) })