Move to new API format for Settings.

refs TryGhost#2606
- Use new API format when updating settings from the client side
- Add additional test to test new API format
- Adjust functional tests to work with the new format

core/client/models/settings.js

@@ -1,4 +1,4 @@
-/*global Ghost, _ */
+/*global Backbone, Ghost, _ */
 (function () {
     'use strict';
     //id:0 is used to issue PUT requests
@@ -14,6 +14,19 @@
             }, {});
 
             return result;
+        },
+
+        sync: function (method, model, options) {
+            var settings = _.map(this.attributes, function (value, key) {
+                return { key: key, value: value };
+            });
+            //wrap settings in {settings: [{...}]}
+            if (method === 'update') {
+                options.data = JSON.stringify({settings: settings});
+                options.contentType = 'application/json';
+            }
+
+            return Backbone.Model.prototype.sync.apply(this, arguments);
         }
     });
 

core/server/api/settings.js

@@ -1,10 +1,8 @@
 var _            = require('lodash'),
     dataProvider = require('../models'),
     when         = require('when'),
-    errors       = require('../errorHandling'),
     config       = require('../config'),
     settings,
-    settingsCollection,
     settingsFilter,
     updateSettingsCache,
     readSettingsResult,
@@ -14,12 +12,6 @@ var _            = require('lodash'),
     settingsCache = {};
 
 // ### Helpers
-// Turn an object into a collection
-settingsCollection = function (settings) {
-    return _.map(settings, function (value, key) {
-        return { key: key, value: value };
-    });
-};
 
 // Filters an object based on a given filter object
 settingsFilter = function (settings, filter) {
@@ -185,56 +177,33 @@ settings = {
         var self = this,
             type;
 
-        // Check for passing a collection of settings first
-        if (_.isObject(key)) {
-            //clean data
-            type = key.type;
-            delete key.type;
-            delete key.availableThemes;
-            delete key.availableApps;
-
-            key = settingsCollection(key);
-            return dataProvider.Settings.edit(key, {user: self.user}).then(function (result) {
-                var readResult = readSettingsResult(result);
-
-                return updateSettingsCache(readResult).then(function () {
-                    return config.theme.update(settings, config().url);
-                }).then(function () {
-                    return settingsResult(readResult, type);
-                });
-            }).otherwise(function (error) {
-                return dataProvider.Settings.findOne(key.key).then(function (result) {
-                    if (!result) {
-                        return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + key + '.'});
-                    }
-                    return when.reject({type: 'InternalServerError', message: error.message});
-                });
-            });
+        // Allow shorthand syntax
+        if (_.isString(key)) {
+            key = { settings: [{ key: key, value: value }]};
         }
 
-        return dataProvider.Settings.findOne(key).then(function (setting) {
-            if (!setting) {
-                return when.reject({type: 'NotFound', message: 'Unable to find setting: ' + key + '.'});
-            }
-            if (!_.isString(value)) {
-                value = JSON.stringify(value);
-            }
-            setting.set('value', value);
-            return dataProvider.Settings.edit(setting, {user: self.user}).then(function (result) {
-                var updatedSetting = _.first(result).attributes;
-                settingsCache[updatedSetting.key].value = updatedSetting.value;
-
-                return updatedSetting;
-            }).then(function (updatedSetting) {
-                return config.theme.update(settings, config().url).then(function () {
-                    return updatedSetting;
-                });
-            }).then(function (updatedSetting) {
-                var result = {};
-                result[updatedSetting.key] = updatedSetting;
-
-                return settingsResult(result);
-            }).otherwise(errors.logAndThrowError);
+        //clean data
+        type = _.find(key.settings, function (setting) { return setting.key === 'type'; });
+        if (_.isObject(type)) {
+            type = type.value;
+        }
+
+        key = _.reject(key.settings, function (setting) {
+            return setting.key === 'type' || setting.key === 'availableThemes' || setting.key === 'availableApps';
+        });
+
+        return dataProvider.Settings.edit(key, {user: self.user}).then(function (result) {
+            var readResult = readSettingsResult(result);
+
+            return updateSettingsCache(readResult).then(function () {
+                return config.theme.update(settings, config().url);
+            }).then(function () {
+                return settingsResult(readResult, type);
+            });
+        }).otherwise(function (error) {
+            // In case something goes wrong it is most likely because of an invalid key
+            // or because of a badly formatted request.
+            return when.reject({type: 'BadRequest', message: error.message});
         });
     }
 };

core/test/functional/routes/api/settings_test.js

@@ -148,7 +148,9 @@ describe('Settings API', function () {
                 var jsonResponse = res.body,
                     changedValue = 'Ghost changed',
                     settingToChange = {
-                        title: changedValue
+                        settings: [
+                            { key: 'title', value: changedValue }
+                        ]
                     };
 
                 jsonResponse.should.exist;
@@ -175,7 +177,7 @@ describe('Settings API', function () {
     });
 
     it('can\'t edit settings with invalid CSRF token', function (done) {
-        request.get(testUtils.API.getApiQuery('settings'))
+        request.get(testUtils.API.getApiQuery('settings/'))
             .end(function (err, res) {
                 if (err) {
                     return done(err);
@@ -202,7 +204,7 @@ describe('Settings API', function () {
 
 
     it('can\'t edit non existent setting', function (done) {
-        request.get(testUtils.API.getApiQuery('settings'))
+        request.get(testUtils.API.getApiQuery('settings/'))
             .end(function (err, res) {
                 if (err) {
                     return done(err);
@@ -211,12 +213,13 @@ describe('Settings API', function () {
                 var jsonResponse = res.body,
                     newValue = 'new value';
                 jsonResponse.should.exist;
-                jsonResponse.testvalue = newValue;
+                should.exist(jsonResponse.settings);
+                jsonResponse.settings.push({ key: 'testvalue', value: newValue });
 
                 request.put(testUtils.API.getApiQuery('settings/'))
                     .set('X-CSRF-Token', csrfToken)
                     .send(jsonResponse)
-                    .expect(404)
+                    .expect(400)
                     .end(function (err, res) {
                         if (err) {
                             return done(err);

core/test/integration/api/api_settings_spec.js

@@ -67,6 +67,17 @@ describe('Settings API', function () {
     });
 
     it('can edit', function (done) {
+        return SettingsAPI.edit({ settings: [{ key: 'title', value: 'UpdatedGhost'}]}).then(function (response) {
+            should.exist(response);
+            testUtils.API.checkResponse(response, 'settings');
+            response.settings.length.should.equal(1);
+            testUtils.API.checkResponse(response.settings[0], 'setting');
+
+            done();
+        }).catch(done);
+    })
+
+    it('can edit, by key/value', function (done) {
         return SettingsAPI.edit('title', 'UpdatedGhost').then(function (response) {
             should.exist(response);
             testUtils.API.checkResponse(response, 'settings');