From 765de94ca34a3f72189d65fa9d116ae9b91542df Mon Sep 17 00:00:00 2001 From: Benedict Aas Date: Tue, 20 Feb 2018 13:13:29 +0000 Subject: [PATCH] refactor: consolidate store-state nil-checking (#2062) We make the nil-checking of store state fields generic through a `verifyNoNilFields` function that throws an error if any fields are nil. Change-Type: patch Changelog-Entry: Consolidate store state nil-checking with helper function. --- lib/shared/store.js | 88 +++++++++++---------- tests/shared/models/flash-state.spec.js | 14 ++-- tests/shared/models/selection-state.spec.js | 6 +- 3 files changed, 58 insertions(+), 50 deletions(-) diff --git a/lib/shared/store.js b/lib/shared/store.js index 6b4d9e1e..fdc5265f 100644 --- a/lib/shared/store.js +++ b/lib/shared/store.js @@ -28,6 +28,52 @@ const fileExtensions = require('./file-extensions') const utils = require('./utils') const packageJSON = require('../../package.json') +/** + * @summary Verify and throw if any state fields are nil + * @function + * @public + * + * @param {Object} object - state object + * @param {Array> | Array} fields - array of object field paths + * @param {String} name - name of the state we're dealing with + * @throws + * + * @example + * const fields = [ 'type', 'percentage' ] + * verifyNoNilFields(action.data, fields) + */ +const verifyNoNilFields = (object, fields, name) => { + const nilFields = _.filter(fields, (field) => { + return _.isNil(_.get(object, field)) + }) + if (nilFields.length) { + throw new Error(`Missing ${name} fields: ${nilFields.join(', ')}`) + } +} + +/** + * @summary FLASH_STATE fields that can't be nil + * @constant + * @private + */ +const flashStateNoNilFields = [ + 'type', + 'percentage', + 'eta', + 'speed' +] + +/** + * @summary SELECT_IMAGE fields that can't be nil + * @constant + * @private + */ +const selectImageNoNilFields = [ + 'path', + 'extension', + 'size' +] + /** * @summary Application default state * @type {Object} @@ -179,11 +225,7 @@ const storeReducer = (state = DEFAULT_STATE, action) => { }) } - if (!action.data.type) { - throw errors.createError({ - title: 'Missing state type' - }) - } + verifyNoNilFields(action.data, flashStateNoNilFields, 'flash') if (!_.isString(action.data.type)) { throw errors.createError({ @@ -191,36 +233,18 @@ const storeReducer = (state = DEFAULT_STATE, action) => { }) } - if (_.isNil(action.data.percentage)) { - throw errors.createError({ - title: 'Missing state percentage' - }) - } - if (!utils.isValidPercentage(action.data.percentage)) { throw errors.createError({ title: `Invalid state percentage: ${action.data.percentage}` }) } - if (_.isNil(action.data.eta)) { - throw errors.createError({ - title: 'Missing state eta' - }) - } - if (!_.isNumber(action.data.eta)) { throw errors.createError({ title: `Invalid state eta: ${action.data.eta}` }) } - if (_.isNil(action.data.speed)) { - throw errors.createError({ - title: 'Missing state speed' - }) - } - return state.set('flashState', Immutable.fromJS(action.data)) } @@ -322,11 +346,7 @@ const storeReducer = (state = DEFAULT_STATE, action) => { // place where all the image extension / format handling // takes place, to avoid having to check 2+ locations with different logic case ACTIONS.SELECT_IMAGE: { - if (!action.data.path) { - throw errors.createError({ - title: 'Missing image path' - }) - } + verifyNoNilFields(action.data, selectImageNoNilFields, 'image') if (!_.isString(action.data.path)) { throw errors.createError({ @@ -334,12 +354,6 @@ const storeReducer = (state = DEFAULT_STATE, action) => { }) } - if (!action.data.extension) { - throw errors.createError({ - title: 'Missing image extension' - }) - } - if (_.some([ !_.isString(action.data.extension), !_.includes(supportedFormats.getAllExtensions(), action.data.extension) @@ -374,12 +388,6 @@ const storeReducer = (state = DEFAULT_STATE, action) => { } } - if (!action.data.size) { - throw errors.createError({ - title: 'Missing image size' - }) - } - if (!_.isPlainObject(action.data.size)) { throw errors.createError({ title: `Invalid image size: ${action.data.size}` diff --git a/tests/shared/models/flash-state.spec.js b/tests/shared/models/flash-state.spec.js index 1ba752c2..d5529a82 100644 --- a/tests/shared/models/flash-state.spec.js +++ b/tests/shared/models/flash-state.spec.js @@ -102,7 +102,7 @@ describe('Model: flashState', function () { eta: 15, speed: 100000000000 }) - }).to.throw('Missing state type') + }).to.throw('Missing flash fields: type') }) it('should throw if type is not a string', function () { @@ -126,7 +126,7 @@ describe('Model: flashState', function () { eta: 15, speed: 100000000000 }) - }).to.not.throw('Missing state percentage') + }).to.not.throw('Missing flash fields: percentage') }) it('should throw if percentage is missing', function () { @@ -137,7 +137,7 @@ describe('Model: flashState', function () { eta: 15, speed: 100000000000 }) - }).to.throw('Missing state percentage') + }).to.throw('Missing flash fields: percentage') }) it('should throw if percentage is not a number', function () { @@ -184,7 +184,7 @@ describe('Model: flashState', function () { percentage: 50, speed: 100000000000 }) - }).to.throw('Missing state eta') + }).to.throw('Missing flash fields: eta') }) it('should not throw if eta is equal to zero', function () { @@ -196,7 +196,7 @@ describe('Model: flashState', function () { eta: 0, speed: 100000000000 }) - }).to.not.throw('Missing state eta') + }).to.not.throw('Missing flash field eta') }) it('should throw if eta is not a number', function () { @@ -219,7 +219,7 @@ describe('Model: flashState', function () { percentage: 50, eta: 15 }) - }).to.throw('Missing state speed') + }).to.throw('Missing flash fields: speed') }) it('should not throw if speed is 0', function () { @@ -231,7 +231,7 @@ describe('Model: flashState', function () { eta: 15, speed: 0 }) - }).to.not.throw('Missing state speed') + }).to.not.throw('Missing flash fields: speed') }) it('should floor the percentage number', function () { diff --git a/tests/shared/models/selection-state.spec.js b/tests/shared/models/selection-state.spec.js index cb165fb8..56f947b6 100644 --- a/tests/shared/models/selection-state.spec.js +++ b/tests/shared/models/selection-state.spec.js @@ -418,7 +418,7 @@ describe('Model: selectionState', function () { } } }) - }).to.throw('Missing image path') + }).to.throw('Missing image fields: path') }) it('should throw if path is not a string', function () { @@ -449,7 +449,7 @@ describe('Model: selectionState', function () { } } }) - }).to.throw('Missing image extension') + }).to.throw('Missing image fields: extension') }) it('should throw if extension is not a string', function () { @@ -574,7 +574,7 @@ describe('Model: selectionState', function () { path: 'foo.img', extension: 'img' }) - }).to.throw('Missing image size') + }).to.throw('Missing image fields: size') }) it('should throw if size is not a plain object', function () {