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
This commit is contained in:
Benedict Aas 2018-04-24 22:04:36 +01:00 committed by GitHub
parent 3c20a056e6
commit b88a45aa79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 95 additions and 17 deletions

View File

@ -151,8 +151,9 @@ module.exports = function (
return imageWriter.flash(image.path, devices) return imageWriter.flash(image.path, devices)
}).then(() => { }).then(() => {
if (!flashState.wasLastFlashCancelled()) { if (!flashState.wasLastFlashCancelled()) {
notification.send('Success!', { const flashResults = flashState.getFlashResults()
body: messages.info.flashComplete(path.basename(image.path), drives), notification.send('Flash complete!', {
body: messages.info.flashComplete(path.basename(image.path), drives, flashResults.results.devices),
icon: iconPath icon: iconPath
}) })
$state.go('success') $state.go('success')

View File

@ -52,14 +52,23 @@ module.exports = {
*/ */
info: { info: {
flashComplete: (imageBasename, drives) => { flashComplete: (imageBasename, [ drive ], { failed, succeeded: successful }) => {
return [ /* eslint-disable no-magic-numbers */
`${imageBasename} was successfully written to`, const targets = []
// eslint-disable-next-line lodash/prefer-lodash-method if (failed + successful === 1) {
drives.map((drive) => { targets.push(`to ${drive.description} (${drive.displayName})`)
return `${drive.description} (${drive.displayName})` } else {
}).join(', ') if (successful) {
].join(' ') 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) => { flashFailure: (imageBasename, drives) => {
return [ /* eslint-disable no-magic-numbers */
`Something went wrong while writing ${imageBasename} to`, const target = drives.length === 1
// eslint-disable-next-line lodash/prefer-lodash-method ? `${drives[0].description} (${drives[0].displayName})`
drives.map((drive) => { : `${drives.length} targets`
return `${drive.description} (${drive.displayName})` return `Something went wrong while writing ${imageBasename} to ${target}.`
}).join(', ') /* eslint-enable no-magic-numbers */
].join(' ')
}, },
driveUnplugged: () => { driveUnplugged: () => {

View File

@ -21,6 +21,19 @@ const _ = require('lodash')
const messages = require('../../lib/shared/messages') const messages = require('../../lib/shared/messages')
describe('Shared: Messages', function () { 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 () { it('should contain object properties', function () {
m.chai.expect(_.every(_.map(messages, _.isPlainObject))).to.be.true 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 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).')
})
})
})
}) })