diff --git a/lib/gui/app/models/local-settings.js b/lib/gui/app/models/local-settings.js index b7096c34..ae9eceaa 100644 --- a/lib/gui/app/models/local-settings.js +++ b/lib/gui/app/models/local-settings.js @@ -21,6 +21,13 @@ const Bluebird = require('bluebird') const fs = require('fs') const path = require('path') +/** + * @summary Number of spaces to indent JSON output with + * @type {Number} + * @constant + */ +const JSON_INDENT = 2 + /** * @summary Userdata directory path * @description @@ -86,6 +93,7 @@ const readConfigFile = (filename) => { * @private * * @param {String} filename - file path + * @param {Object} data - data * @fulfil {Object} data - data * @returns {Promise} * @@ -97,7 +105,7 @@ const readConfigFile = (filename) => { */ const writeConfigFile = (filename, data) => { return new Bluebird((resolve, reject) => { - const contents = JSON.stringify(data, null, 2) + const contents = JSON.stringify(data, null, JSON_INDENT) fs.writeFile(filename, contents, (error) => { if (error) { reject(error) @@ -161,8 +169,13 @@ exports.writeAll = (settings) => { * }); */ exports.clear = () => { - // TODO: Unlink config file - return Bluebird.try(() => { - // settingsStorage.clearAll() + return new Bluebird((resolve, reject) => { + fs.unlink(HOME_CONFIG_PATH, (error) => { + if (error) { + reject(error) + } else { + resolve() + } + }) }) } diff --git a/lib/gui/app/models/settings.js b/lib/gui/app/models/settings.js index 0ad76ecb..3364d69c 100644 --- a/lib/gui/app/models/settings.js +++ b/lib/gui/app/models/settings.js @@ -66,6 +66,7 @@ let settings = _.cloneDeep(DEFAULT_SETTINGS) */ exports.reset = () => { debug('reset') + // TODO: Remove default settings from config file (?) settings = _.cloneDeep(DEFAULT_SETTINGS) return localSettings.writeAll(settings) @@ -76,7 +77,7 @@ exports.reset = () => { * @function * @public * - * @param {Object} settings - settings + * @param {Object} value - value * @returns {Promise} * * @example @@ -103,9 +104,9 @@ exports.assign = (value) => { const newSettings = _.assign({}, settings, value) return localSettings.writeAll(newSettings) - .then((localSettings) => { + .then((updatedSettings) => { // NOTE: Only update in memory settings when successfully written - settings = localSettings + settings = updatedSettings }) } @@ -156,9 +157,16 @@ exports.set = (key, value) => { })) } + const previousValue = settings[key] + settings[key] = value return localSettings.writeAll(settings) + .catch((error) => { + // Revert to previous value if persisting settings failed + settings[key] = previousValue + throw error + }) } /** @@ -173,7 +181,6 @@ exports.set = (key, value) => { * const value = settings.get('unmountOnSuccess'); */ exports.get = (key) => { - // debug('get', key) return _.cloneDeep(_.get(settings, [ key ])) } diff --git a/lib/gui/app/models/store.js b/lib/gui/app/models/store.js index 0492b455..e3334093 100644 --- a/lib/gui/app/models/store.js +++ b/lib/gui/app/models/store.js @@ -509,10 +509,25 @@ module.exports = _.merge(redux.createStore(storeReducer, DEFAULT_STATE), { Defaults: DEFAULT_STATE }) +/** + * @summary Observe the store for changes + * @param {Function} onChange - change handler + * @returns {Function} unsubscribe + * @example + * store.observe((newState) => { + * // ... + * }) + */ module.exports.observe = (onChange) => { - let currentState + let currentState = null - function changeHandler() { + /** + * @summary Internal change detection handler + * @private + * @example + * store.subscribe(changeHandler) + */ + const changeHandler = () => { const nextState = module.exports.getState() if (!_.isEqual(nextState, currentState)) { currentState = nextState @@ -524,4 +539,3 @@ module.exports.observe = (onChange) => { changeHandler() return unsubscribe } - diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js index 86474ab6..4188f019 100644 --- a/lib/gui/app/pages/main/controllers/flash.js +++ b/lib/gui/app/pages/main/controllers/flash.js @@ -28,7 +28,6 @@ const path = require('path') const store = require('../../../models/store') const constraints = require('../../../../../shared/drive-constraints') const availableDrives = require('../../../models/available-drives') -const debug = require('debug')('etcher:controller:flash') module.exports = function ( $q, diff --git a/tests/gui/models/settings.spec.js b/tests/gui/models/settings.spec.js index 7f10da11..bbc672bd 100644 --- a/tests/gui/models/settings.spec.js +++ b/tests/gui/models/settings.spec.js @@ -176,20 +176,10 @@ describe('Browser: settings', function () { }) }) - it('should throw if setting an object', function (done) { - settings.set('foo', { - setting: 1 - }).asCallback((error) => { - m.chai.expect(error).to.be.an.instanceof(Error) - m.chai.expect(error.message).to.equal('Invalid setting value: [object Object] for foo') - done() - }) - }) - it('should throw if setting an array', function (done) { settings.assign([ 1, 2, 3 ]).asCallback((error) => { m.chai.expect(error).to.be.an.instanceof(Error) - m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo') + m.chai.expect(error.message).to.equal('Settings must be an object') done() }) }) @@ -212,22 +202,6 @@ describe('Browser: settings', function () { }) }) - it('should not store invalid settings to the local machine', function () { - return localSettings.readAll().then((data) => { - m.chai.expect(data.foo).to.be.undefined - - return new Bluebird((resolve) => { - settings.set('foo', [ 1, 2, 3 ]).asCallback((error) => { - m.chai.expect(error).to.be.an.instanceof(Error) - m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo') - return resolve() - }) - }) - }).then(localSettings.readAll).then((data) => { - m.chai.expect(data.foo).to.be.undefined - }) - }) - it('should not change the application state if storing to the local machine results in an error', function (done) { settings.set('foo', 'bar').then(() => { m.chai.expect(settings.get('foo')).to.equal('bar') diff --git a/tests/shared/models/available-drives.spec.js b/tests/shared/models/available-drives.spec.js index 82447c8e..28a8a194 100644 --- a/tests/shared/models/available-drives.spec.js +++ b/tests/shared/models/available-drives.spec.js @@ -18,8 +18,8 @@ const m = require('mochainon') const path = require('path') -const availableDrives = require('../../../lib/gui/app/available-drives') -const selectionState = require('../../../lib/gui/app/selection-state') +const availableDrives = require('../../../lib/gui/app/models/available-drives') +const selectionState = require('../../../lib/gui/app/models/selection-state') const constraints = require('../../../lib/shared/drive-constraints') describe('Model: availableDrives', function () { diff --git a/tests/shared/models/selection-state.spec.js b/tests/shared/models/selection-state.spec.js index 49531883..cf276589 100644 --- a/tests/shared/models/selection-state.spec.js +++ b/tests/shared/models/selection-state.spec.js @@ -19,8 +19,8 @@ const m = require('mochainon') const _ = require('lodash') const path = require('path') -const availableDrives = require('../../../lib/gui/app/available-drives') -const selectionState = require('../../../lib/gui/app/selection-state') +const availableDrives = require('../../../lib/gui/app/models/available-drives') +const selectionState = require('../../../lib/gui/app/models/selection-state') describe('Model: selectionState', function () { describe('given a clean state', function () {