From 2795e723e151b018e3b1d6bdc31aebb96994c8a2 Mon Sep 17 00:00:00 2001 From: Fabian Becker Date: Tue, 6 May 2014 18:52:11 +0000 Subject: [PATCH] Move to new API format for Settings. refs #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 | 15 +++- core/server/api/settings.js | 77 ++++++------------- .../functional/routes/api/settings_test.js | 13 ++-- .../test/integration/api/api_settings_spec.js | 11 +++ 4 files changed, 56 insertions(+), 60 deletions(-) diff --git a/core/client/models/settings.js b/core/client/models/settings.js index 8ffe3ec7b5..0993b1c369 100644 --- a/core/client/models/settings.js +++ b/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); } }); diff --git a/core/server/api/settings.js b/core/server/api/settings.js index 3178782ff5..3378cf4c11 100644 --- a/core/server/api/settings.js +++ b/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; + //clean data + type = _.find(key.settings, function (setting) { return setting.key === 'type'; }); + if (_.isObject(type)) { + type = type.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; + key = _.reject(key.settings, function (setting) { + return setting.key === 'type' || setting.key === 'availableThemes' || setting.key === 'availableApps'; + }); - return settingsResult(result); - }).otherwise(errors.logAndThrowError); + 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}); }); } }; diff --git a/core/test/functional/routes/api/settings_test.js b/core/test/functional/routes/api/settings_test.js index 770eecf560..16944a9a5e 100644 --- a/core/test/functional/routes/api/settings_test.js +++ b/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); diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js index 90c8d6b35b..91936f2e90 100644 --- a/core/test/integration/api/api_settings_spec.js +++ b/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');