Proper settings infrastructure, allowing new features without compromising old data.

On server load, check for settings which have not been set, and apply a default value to the settings table from a JSON file.

core/ghost.js

@@ -145,6 +145,8 @@ Ghost.prototype.init = function () {
         instance.getPaths(),
         instance.mail.init(self)
     ).then(function () {
+        return models.Settings.populateDefaults();
+    }).then(function () {
         return self.initPlugins();
     }).then(function () {
         // Initialize the settings cache

core/server/data/default-settings.json

@@ -0,0 +1,47 @@
+[
+    {
+        "key":          "title",
+        "value":        "Ghost",
+        "type":         "blog"
+    },
+    {
+        "key":          "description",
+        "value":        "Just a blogging platform.",
+        "type":         "blog"
+    },
+    {
+        "key":          "email",
+        "value":        "ghost@example.com",
+        "type":         "general"
+    },
+    {
+        "key":          "activePlugins",
+        "value":        "",
+        "type":         "general"
+    },
+    {
+        "key":          "activeTheme",
+        "value":        "content/themes/casper",
+        "type":         "general"
+    },
+    {
+        "key":          "currentVersion",
+        "value":        "002",
+        "type":         "core"
+    },
+    {
+        "key":          "installedPlugins",
+        "value":        "[]",
+        "type":         "core"
+    },
+    {
+        "key":          "logo",
+        "value":        "",
+        "type":         "blog"
+    },
+    {
+        "key":          "icon",
+        "value":        "",
+        "type":         "blog"
+    }
+]

core/server/data/fixtures/001.js

@@ -25,65 +25,6 @@ module.exports = {
         }
     ],
 
-    settings: [
-        {
-            "uuid":         uuid.v4(),
-            "key":          "url",
-            "value":        "http://localhost:2368",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "blog"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "title",
-            "value":        "Ghost",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "blog"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "description",
-            "value":        "Just a blogging platform.",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "blog"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "email",
-            "value":        "ghost@example.com",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "general"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "activePlugins",
-            "value":        "",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "general"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "activeTheme",
-            "value":        "content/themes/casper",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "general"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "currentVersion",
-            "value":        "001",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "core"
-        }
-    ],
-
     roles: [
         {
             "id": 1,

core/server/data/fixtures/002.js

@@ -3,34 +3,6 @@ var uuid = require('node-uuid');
 module.exports = {
     posts: [],
 
-    settings: [
-        {
-            "uuid":         uuid.v4(),
-            "key":          "installedPlugins",
-            "value":        "[]",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "core"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "logo",
-            "value":        "",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "blog"
-        },
-        {
-            "uuid":         uuid.v4(),
-            "key":          "icon",
-            "value":        "",
-            "created_by":    1,
-            "updated_by":    1,
-            "type":         "blog"
-        }
-
-    ],
-
     roles: [],
 
     permissions: [],

core/server/data/migration/001.js

@@ -101,7 +101,7 @@ up = function () {
             // knex('roles_users').insert(fixtures.roles_users),
             knex('permissions').insert(fixtures.permissions),
             knex('permissions_roles').insert(fixtures.permissions_roles),
-            knex('settings').insert(fixtures.settings)
+            knex('settings').insert({ key: 'currentVersion', 'value': '001', type: 'core' })
         ]);
 
     });

core/server/data/migration/002.js

@@ -58,14 +58,6 @@ up = function () {
         })
 
     ]).then(function () {
-
-        // Once we create all of the initial tables, bootstrap any of the data
-        return when.all([
-            knex('settings').insert(fixtures.settings)
-        ]);
-
-    }).then(function () {
-
         // Lastly, update the current version settings to reflect this version
         return knex('settings')
             .where('key', 'currentVersion')
@@ -84,6 +76,7 @@ down = function () {
             knex.Schema.dropTableIfExists("posts_custom_data")
         ]);
     });
+    // Should we also drop the currentVersion?
 };
 
 exports.up = up;

core/server/data/migration/index.js

@@ -7,6 +7,7 @@ var _ = require('underscore'),
     initialVersion = "001",
     // This currentVersion string should always be the current version of Ghost,
     // we could probably load it from the config file.
+    // - Will be possible after default-settings.json restructure
     currentVersion = "002";
 
 function getCurrentVersion() {

core/server/models/settings.js

@@ -3,7 +3,8 @@ var Settings,
     uuid = require('node-uuid'),
     _ = require('underscore'),
     errors = require('../errorHandling'),
-    when = require('when');
+    when = require('when'),
+    defaultSettings = require('../data/default-settings.json');
 
 // Each setting is saved as a separate row in the database,
 // but the overlying API treats them as a single key:value mapping
@@ -36,7 +37,7 @@ Settings = GhostBookshelf.Model.extend({
     saving: function () {
         // Deal with the related data here
 
-        // Remove any properties which don't belong on the post model
+        // Remove any properties which don't belong on the model
         this.attributes = this.pick(this.permittedAttributes);
     }
 }, {
@@ -57,10 +58,31 @@ Settings = GhostBookshelf.Model.extend({
             // Accept an array of models as input
             if (item.toJSON) { item = item.toJSON(); }
             return settings.forge({ key: item.key }).fetch().then(function (setting) {
-                return setting.set('value', item.value).save();
+                if (setting) {
+                    return setting.set('value', item.value).save();
+                }
+                return settings.forge({ key: item.key, value: item.value }).save();
+
             }, errors.logAndThrowError);
         });
+    },
+
+    populateDefaults: function () {
+        return this.findAll().then(function (allSettings) {
+            var usedKeys = allSettings.models.map(function (setting) { return setting.get('key'); }),
+                insertOperations = [];
+
+            defaultSettings.forEach(function (defaultSetting) {
+                var isMissingFromDB = usedKeys.indexOf(defaultSetting.key) === -1;
+                if (isMissingFromDB) {
+                    insertOperations.push(Settings.forge(defaultSetting).save());
+                }
+            });
+
+            return when.all(insertOperations);
+        });
     }
+
 });
 
 module.exports = {

core/test/unit/api_settings_spec.js

@@ -2,7 +2,8 @@
 var _ = require("underscore"),
     should = require('should'),
     helpers = require('./helpers'),
-    Models = require('../../server/models');
+    Models = require('../../server/models'),
+    knex = require('../../server/models/base').Knex;
 
 describe('Settings Model', function () {
 
@@ -27,165 +28,207 @@ describe('Settings Model', function () {
         }, done);
     });
 
-    it('can browse', function (done) {
+    describe('API', function () {
-        SettingsModel.browse().then(function (results) {
 
-            should.exist(results);
+        it('can browse', function (done) {
+            SettingsModel.browse().then(function (results) {
 
-            results.length.should.be.above(0);
+                should.exist(results);
 
-            done();
+                results.length.should.be.above(0);
-        }).then(null, done);
+
+                done();
+            }).then(null, done);
+        });
+
+        it('can read', function (done) {
+            var firstSetting;
+
+            SettingsModel.browse().then(function (results) {
+
+                should.exist(results);
+
+                results.length.should.be.above(0);
+
+                firstSetting = results.models[0];
+
+                return SettingsModel.read(firstSetting.attributes.key);
+
+            }).then(function (found) {
+
+                should.exist(found);
+
+                found.attributes.value.should.equal(firstSetting.attributes.value);
+
+                done();
+
+            }).then(null, done);
+        });
+
+        it('can edit single', function (done) {
+            var firstSetting;
+
+            SettingsModel.browse().then(function (results) {
+
+                should.exist(results);
+
+                results.length.should.be.above(0);
+
+                firstSetting = results.models[0];
+
+                // The edit method has been modified to take an object of
+                // key/value pairs
+                firstSetting.set('value', 'new value');
+
+                return SettingsModel.edit(firstSetting);
+
+            }).then(function (edited) {
+
+                should.exist(edited);
+
+                edited.length.should.equal(1);
+
+                edited = edited[0];
+
+                edited.attributes.key.should.equal(firstSetting.attributes.key);
+                edited.attributes.value.should.equal('new value');
+
+                done();
+
+            }).then(null, done);
+        });
+
+        it('can edit multiple', function (done) {
+            var model1,
+                model2,
+                editedModel;
+
+            SettingsModel.browse().then(function (results) {
+
+                should.exist(results);
+
+                results.length.should.be.above(0);
+
+                model1 = results.models[0];
+                model2 = results.models[1];
+
+                // The edit method has been modified to take an object of
+                // key/value pairs
+                model1.set('value', 'new value1');
+                model2.set('value', 'new value2');
+
+                return SettingsModel.edit([model1, model2]);
+
+            }).then(function (edited) {
+
+                should.exist(edited);
+
+                edited.length.should.equal(2);
+
+                editedModel = edited[0];
+
+                editedModel.attributes.key.should.equal(model1.attributes.key);
+                editedModel.attributes.value.should.equal('new value1');
+
+                editedModel = edited[1];
+
+                editedModel.attributes.key.should.equal(model2.attributes.key);
+                editedModel.attributes.value.should.equal('new value2');
+
+                done();
+
+            }).then(null, done);
+        });
+
+        it('can add', function (done) {
+            var newSetting = {
+                key: 'TestSetting1',
+                value: 'Test Content 1'
+            };
+
+            SettingsModel.add(newSetting).then(function (createdSetting) {
+
+                should.exist(createdSetting);
+                createdSetting.has('uuid').should.equal(true);
+                createdSetting.attributes.key.should.equal(newSetting.key, "key is correct");
+                createdSetting.attributes.value.should.equal(newSetting.value, "value is correct");
+                createdSetting.attributes.type.should.equal("general");
+
+                done();
+            }).then(null, done);
+        });
+
+        it('can delete', function (done) {
+            var firstSettingId;
+
+            SettingsModel.browse().then(function (results) {
+
+                should.exist(results);
+
+                results.length.should.be.above(0);
+
+                firstSettingId = results.models[0].id;
+
+                return SettingsModel.destroy(firstSettingId);
+
+            }).then(function () {
+
+                return SettingsModel.browse();
+
+            }).then(function (newResults) {
+
+                var ids, hasDeletedId;
+
+                ids = _.pluck(newResults.models, "id");
+
+                hasDeletedId = _.any(ids, function (id) {
+                    return id === firstSettingId;
+                });
+
+                hasDeletedId.should.equal(false);
+
+                done();
+
+            }).then(null, done);
+        });
     });
 
-    it('can read', function (done) {
+    describe('populating defaults from settings.json', function (done) {
-        var firstSetting;
 
-        SettingsModel.browse().then(function (results) {
+        beforeEach(function (done) {
-
+            knex('settings').truncate().then(function () {
-            should.exist(results);
+                done();
-
-            results.length.should.be.above(0);
-
-            firstSetting = results.models[0];
-
-            return SettingsModel.read(firstSetting.attributes.key);
-
-        }).then(function (found) {
-
-            should.exist(found);
-
-            found.attributes.value.should.equal(firstSetting.attributes.value);
-
-            done();
-
-        }).then(null, done);
-    });
-
-    it('can edit single', function (done) {
-        var firstSetting;
-
-        SettingsModel.browse().then(function (results) {
-
-            should.exist(results);
-
-            results.length.should.be.above(0);
-
-            firstSetting = results.models[0];
-
-            // The edit method has been modified to take an object of
-            // key/value pairs
-            firstSetting.set('value', 'new value');
-
-            return SettingsModel.edit(firstSetting);
-
-        }).then(function (edited) {
-
-            should.exist(edited);
-
-            edited.length.should.equal(1);
-
-            edited = edited[0];
-
-            edited.attributes.key.should.equal(firstSetting.attributes.key);
-            edited.attributes.value.should.equal('new value');
-
-            done();
-
-        }).then(null, done);
-    });
-
-    it('can edit multiple', function (done) {
-        var model1,
-            model2,
-            editedModel;
-
-        SettingsModel.browse().then(function (results) {
-
-            should.exist(results);
-
-            results.length.should.be.above(0);
-
-            model1 = results.models[0];
-            model2 = results.models[1];
-
-            // The edit method has been modified to take an object of
-            // key/value pairs
-            model1.set('value', 'new value1');
-            model2.set('value', 'new value2');
-
-            return SettingsModel.edit([model1, model2]);
-
-        }).then(function (edited) {
-
-            should.exist(edited);
-
-            edited.length.should.equal(2);
-
-            editedModel = edited[0];
-
-            editedModel.attributes.key.should.equal(model1.attributes.key);
-            editedModel.attributes.value.should.equal('new value1');
-
-            editedModel = edited[1];
-
-            editedModel.attributes.key.should.equal(model2.attributes.key);
-            editedModel.attributes.value.should.equal('new value2');
-
-            done();
-
-        }).then(null, done);
-    });
-
-    it('can add', function (done) {
-        var newSetting = {
-            key: 'TestSetting1',
-            value: 'Test Content 1'
-        };
-
-        SettingsModel.add(newSetting).then(function (createdSetting) {
-
-            should.exist(createdSetting);
-            createdSetting.has('uuid').should.equal(true);
-            createdSetting.attributes.key.should.equal(newSetting.key, "key is correct");
-            createdSetting.attributes.value.should.equal(newSetting.value, "value is correct");
-            createdSetting.attributes.type.should.equal("general");
-
-            done();
-        }).then(null, done);
-    });
-
-    it('can delete', function (done) {
-        var firstSettingId;
-
-        SettingsModel.browse().then(function (results) {
-
-            should.exist(results);
-
-            results.length.should.be.above(0);
-
-            firstSettingId = results.models[0].id;
-
-            return SettingsModel.destroy(firstSettingId);
-
-        }).then(function () {
-
-            return SettingsModel.browse();
-
-        }).then(function (newResults) {
-
-            var ids, hasDeletedId;
-
-            ids = _.pluck(newResults.models, "id");
-
-            hasDeletedId = _.any(ids, function (id) {
-                return id === firstSettingId;
             });
+        });
 
-            hasDeletedId.should.equal(false);
+        it('populates any unset settings from the JSON defaults', function (done) {
+            SettingsModel.findAll().then(function (allSettings) {
+                console.log(allSettings.models)
+                allSettings.length.should.equal(0);
+                return SettingsModel.populateDefaults();
+            }).then(function () {
+                return SettingsModel.findAll();
+            }).then(function (allSettings) {
+                allSettings.length.should.be.above(0);
+                return SettingsModel.read('description');
+            }).then(function (descriptionSetting) {
+                // Testing against the actual value in default-settings.json feels icky,
+                // but it's easier to fix the test if that ever changes than to mock out that behaviour
+                descriptionSetting.get('value').should.equal('Just a blogging platform.');
+                done();
+            }).then(null, done);
+        });
 
-            done();
+        it('doesn\'t overwrite any existing settings', function (done) {
-
+            SettingsModel.edit({key: 'description', value: 'Adam\'s Blog'}).then(function () {
-        }).then(null, done);
+                return SettingsModel.populateDefaults();
+            }).then(function () {
+                return SettingsModel.read('description');
+            }).then(function (descriptionSetting) {
+                descriptionSetting.get('value').should.equal('Adam\'s Blog');
+                done();
+            }).then(null, done);
+        });
     });
+
 });

core/test/unit/export_spec.js

@@ -8,7 +8,8 @@ var _ = require("underscore"),
     exporter = require('../../server/data/export'),
     Exporter001 = require('../../server/data/export/001'),
     Exporter002 = require('../../server/data/export/002'),
-    errors = require('../../server/errorHandling');
+    errors = require('../../server/errorHandling'),
+    Settings = require('../../server/models/settings').Settings;
 
 describe("Export", function () {
 
@@ -43,6 +44,8 @@ describe("Export", function () {
             // initialise database to version 001 - confusingly we have to set the max version to be one higher
             // than the migration version we want
             migration.migrateUpFromVersion('001', '002').then(function () {
+                return Settings.populateDefaults();
+            }).then(function () {
                 return exporter("001");
             }).then(function (exportData) {
                 var tables = ['posts', 'users', 'roles', 'roles_users', 'permissions', 'permissions_roles', 'settings'];
@@ -88,6 +91,8 @@ describe("Export", function () {
             // initialise database to version 001 - confusingly we have to set the max version to be one higher
             // than the migration version we want
             migration.migrateUpFromVersion('001', '003').then(function () {
+                return Settings.populateDefaults();
+            }).then(function () {
                 return exporter("002");
             }).then(function (exportData) {
                 var tables = [

core/test/unit/helpers.js

@@ -4,6 +4,7 @@ process.env.NODE_ENV = process.env.TRAVIS ? 'travis' : 'testing';
 var knex = require('../../server/models/base').Knex,
     when = require('when'),
     migration = require("../../server/data/migration/"),
+    Settings = require('../../server/models/settings').Settings,
     helpers,
     samplePost,
     sampleUser,
@@ -44,7 +45,9 @@ sampleUserRole = function (i) {
 helpers = {
 
     initData: function (done) {
-        return migration.init();
+        return migration.init().then(function () {
+           return Settings.populateDefaults();
+        });
     },
 
     clearData: function () {

core/test/unit/import_spec.js

@@ -10,7 +10,8 @@ var _ = require("underscore"),
     importer = require('../../server/data/import'),
     Importer001 = require('../../server/data/import/001'),
     Importer002 = require('../../server/data/import/002'),
-    errors = require('../../server/errorHandling');
+    errors = require('../../server/errorHandling'),
+    Settings = require('../../server/models/settings').Settings;
 
 describe("Import", function () {
 
@@ -50,6 +51,8 @@ describe("Import", function () {
             // initialise database to version 001 - confusingly we have to set the max version to be one higher
             // than the migration version we want
             migration.migrateUpFromVersion('001', '002').then(function () {
+                return Settings.populateDefaults();
+            }).then(function () {
                 // export the version 001 data ready to import
                 // TODO: Should have static test data here?
                 return exporter("001");
@@ -83,8 +86,7 @@ describe("Import", function () {
                 // we always have 0 users as there isn't one in fixtures
                 importedData[0].length.should.equal(0);
                 importedData[1].length.should.equal(exportData.data.posts.length);
-                // version 001 settings have 7 fields
+                importedData[2].length.should.be.above(0);
-                importedData[2].length.should.equal(7);
 
                 _.findWhere(exportData.data.settings, {key: "currentVersion"}).value.should.equal("001");
 
@@ -119,6 +121,8 @@ describe("Import", function () {
             // initialise database to version 001 - confusingly we have to set the max version to be one higher
             // than the migration version we want
             migration.migrateUpFromVersion('001', '002').then(function () {
+                return Settings.populateDefaults();
+            }).then(function () {
                 // export the version 001 data ready to import
                 // TODO: Should have static test data here?
                 return exporter("001");
@@ -146,8 +150,7 @@ describe("Import", function () {
                 importedData[0].length.should.equal(0);
                 // import no longer requires all data to be dropped, and adds posts
                 importedData[1].length.should.equal(exportData.data.posts.length + 1);
-                // version 002 settings have 10 fields, and settings get updated not inserted
+                importedData[2].length.should.be.above(0);
-                importedData[2].length.should.equal(10);
 
                 _.findWhere(importedData[2], {key: "currentVersion"}).value.should.equal("002");
 
@@ -161,6 +164,8 @@ describe("Import", function () {
             // initialise database to version 001 - confusingly we have to set the max version to be one higher
             // than the migration version we want
             migration.migrateUpFromVersion('001', '003').then(function () {
+                return Settings.populateDefaults();
+            }).then(function () {
                 // export the version 002 data ready to import
                 // TODO: Should have static test data here?
                 return exporter("002");
@@ -184,8 +189,7 @@ describe("Import", function () {
                 importedData[0].length.should.equal(0);
                 // import no longer requires all data to be dropped, and adds posts
                 importedData[1].length.should.equal(exportData.data.posts.length + 1);
-                // version 002 settings have 10 fields, and settings get updated not inserted
+                importedData[2].length.should.be.above(0);
-                importedData[2].length.should.equal(10);
 
                 _.findWhere(importedData[2], {key: "currentVersion"}).value.should.equal("002");