From 7e9880ce8d38ff4277568fb02d2c63bee1164bfd Mon Sep 17 00:00:00 2001 From: Jacob Gable Date: Sun, 27 Apr 2014 18:28:50 -0500 Subject: [PATCH] Settings API Primary Document refactor Closes #2606 - Refactor settings api responses to { settings: [ ] } format - Update all code using api.settings to handle new response format - Update test stubs to return new format - Update client site settings model to parse new format into one object of key/value pairs - Refactor to include all setting values - Remove unused settingsCollection method - Update settingsCache to store all attributes - Update settingsResult to send all attributes - Remove unnecessary when() wraps - Reject if editing a setting that doesn't exist - Reject earlier if setting key is empty - Update tests with new error messages - Use setting.add instead of edit that was incorrectly adding - Update importer to properly import activePlugins and installedPlugins - Update expected setting result fields - Fix a weird situation where hasOwnProperty didn't exist :shrug: --- core/client/models/settings.js | 14 +- core/server/api/db.js | 4 +- core/server/api/settings.js | 176 +++++++++--------- core/server/api/users.js | 9 +- core/server/apps/index.js | 8 +- core/server/config/theme.js | 8 +- core/server/config/url.js | 4 +- core/server/controllers/frontend.js | 36 ++-- core/server/data/import/000.js | 14 +- core/server/errorHandling.js | 3 +- core/server/helpers/index.js | 27 +-- core/server/index.js | 18 +- core/server/mail.js | 6 +- core/server/middleware/index.js | 4 +- core/server/middleware/middleware.js | 5 +- core/server/models/settings.js | 11 +- core/server/update-check.js | 21 ++- .../functional/routes/api/settings_test.js | 18 +- .../test/integration/api/api_settings_spec.js | 51 ++++- .../integration/model/model_settings_spec.js | 7 +- core/test/unit/config_spec.js | 8 +- core/test/unit/frontend_spec.js | 72 ++++--- core/test/unit/import_spec.js | 6 +- core/test/unit/server_helpers_index_spec.js | 14 +- core/test/unit/xmlrpc_spec.js | 2 +- core/test/utils/api.js | 10 +- 26 files changed, 346 insertions(+), 210 deletions(-) diff --git a/core/client/models/settings.js b/core/client/models/settings.js index f5b2c93710..8ffe3ec7b5 100644 --- a/core/client/models/settings.js +++ b/core/client/models/settings.js @@ -1,10 +1,20 @@ -/*global Ghost */ +/*global Ghost, _ */ (function () { 'use strict'; //id:0 is used to issue PUT requests Ghost.Models.Settings = Ghost.ProgressModel.extend({ url: Ghost.paths.apiRoot + '/settings/?type=blog,theme,app', - id: '0' + id: '0', + + parse: function (response) { + var result = _.reduce(response.settings, function (settings, setting) { + settings[setting.key] = setting.value; + + return settings; + }, {}); + + return result; + } }); }()); diff --git a/core/server/api/db.js b/core/server/api/db.js index e0ee749c46..835819aa0e 100644 --- a/core/server/api/db.js +++ b/core/server/api/db.js @@ -44,7 +44,9 @@ db = { return when.reject({code: 500, message: 'Please select a .json file to import.'}); } - return api.settings.read({ key: 'databaseVersion' }).then(function (setting) { + return api.settings.read({ key: 'databaseVersion' }).then(function (response) { + var setting = response.settings[0]; + return when(setting.value); }, function () { return when('002'); diff --git a/core/server/api/settings.js b/core/server/api/settings.js index b50c9f1333..5082eb651a 100644 --- a/core/server/api/settings.js +++ b/core/server/api/settings.js @@ -4,34 +4,16 @@ var _ = require('lodash'), errors = require('../errorHandling'), config = require('../config'), settings, - settingsObject, settingsCollection, settingsFilter, updateSettingsCache, readSettingsResult, filterPaths, + settingsResult, // Holds cached settings settingsCache = {}; // ### Helpers -// Turn a settings collection into a single object/hashmap -settingsObject = function (settings) { - if (_.isObject(settings)) { - return _.reduce(settings, function (res, item, key) { - if (_.isArray(item)) { - res[key] = item; - } else { - res[key] = item.value; - } - return res; - }, {}); - } - return (settings.toJSON ? settings.toJSON() : settings).reduce(function (res, item) { - if (item.toJSON) { item = item.toJSON(); } - if (item.key) { res[item.key] = item.value; } - return res; - }, {}); -}; // Turn an object into a collection settingsCollection = function (settings) { return _.map(settings, function (value, key) { @@ -57,45 +39,53 @@ updateSettingsCache = function (settings) { if (!_.isEmpty(settings)) { _.map(settings, function (setting, key) { - settingsCache[key].value = setting.value; - }); - } else { - return when(dataProvider.Settings.findAll()).then(function (result) { - return when(readSettingsResult(result)).then(function (s) { - settingsCache = s; - }); + settingsCache[key] = setting; }); + + return when(settingsCache); } + + return dataProvider.Settings.findAll() + .then(function (result) { + settingsCache = readSettingsResult(result.models); + + return settingsCache; + }); }; -readSettingsResult = function (result) { - var settings = {}; - return when(_.map(result.models, function (member) { - if (!settings.hasOwnProperty(member.attributes.key)) { - var val = {}; - val.value = member.attributes.value; - val.type = member.attributes.type; - settings[member.attributes.key] = val; - } - })).then(function () { - return when(config().paths.availableThemes).then(function (themes) { - var res = filterPaths(themes, settings.activeTheme.value); - settings.availableThemes = { - value: res, - type: 'theme' - }; - return settings; - }); - }).then(function () { - return when(config().paths.availableApps).then(function (apps) { - var res = filterPaths(apps, JSON.parse(settings.activeApps.value)); - settings.availableApps = { - value: res, - type: 'app' - }; - return settings; - }); - }); +readSettingsResult = function (settingsModels) { + var settings = _.reduce(settingsModels, function (memo, member) { + if (!memo.hasOwnProperty(member.attributes.key)) { + memo[member.attributes.key] = member.attributes; + } + + return memo; + }, {}), + themes = config().paths.availableThemes, + apps = config().paths.availableApps, + res; + + if (settings.activeTheme) { + res = filterPaths(themes, settings.activeTheme.value); + + settings.availableThemes = { + key: 'availableThemes', + value: res, + type: 'theme' + }; + } + + if (settings.activeApps) { + res = filterPaths(apps, JSON.parse(settings.activeApps.value)); + + settings.availableApps = { + key: 'availableApps', + value: res, + type: 'app' + }; + } + + return settings; }; @@ -119,9 +109,9 @@ filterPaths = function (paths, active) { _.each(pathKeys, function (key) { //do not include hidden files or _messages - if (key.indexOf('.') !== 0 - && key !== '_messages' - && key !== 'README.md' + if (key.indexOf('.') !== 0 && + key !== '_messages' && + key !== 'README.md' ) { item = { name: key @@ -141,18 +131,31 @@ filterPaths = function (paths, active) { return res; }; +settingsResult = function (settings, type) { + var filteredSettings = _.values(settingsFilter(settings, type)), + result = { + settings: filteredSettings + }; + + if (type) { + result.meta = { + filters: { + type: type + } + }; + } + + return result; +}; + settings = { // #### Browse // **takes:** options object browse: function browse(options) { + //TODO: omit where type==core // **returns:** a promise for a settings json object - if (settingsCache) { - return when(settingsCache).then(function (settings) { - //TODO: omit where type==core - return settingsObject(settingsFilter(settings, options.type)); - }, errors.logAndThrowError); - } + return when(settingsResult(settingsCache, options.type)); }, // #### Read @@ -163,17 +166,16 @@ settings = { options = { key: options }; } - if (settingsCache) { - return when(settingsCache[options.key]).then(function (setting) { - if (!setting) { - return when.reject({code: 404, message: 'Unable to find setting: ' + options.key}); - } - var res = {}; - res.key = options.key; - res.value = setting.value; - return res; - }, errors.logAndThrowError); + var setting = settingsCache[options.key], + result = {}; + + if (!setting) { + return when.reject({code: 404, message: 'Unable to find setting: ' + options.key}); } + + result[options.key] = setting; + + return when(settingsResult(result)); }, // #### Edit @@ -193,23 +195,23 @@ settings = { key = settingsCollection(key); return dataProvider.Settings.edit(key, {user: self.user}).then(function (result) { - result.models = result; - return when(readSettingsResult(result)).then(function (settings) { - updateSettingsCache(settings); + var readResult = readSettingsResult(result); + + return updateSettingsCache(readResult).then(function () { + return config.theme.update(settings, config().url); }).then(function () { - return config.theme.update(settings, config().url).then(function () { - return settingsObject(settingsFilter(settingsCache, type)); - }); + return settingsResult(readResult, type); }); }).otherwise(function (error) { return dataProvider.Settings.read(key.key).then(function (result) { if (!result) { return when.reject({code: 404, message: 'Unable to find setting: ' + key}); } - return when.reject({message: error.message}); + return when.reject({message: error.message, stack: error.stack}); }); }); } + return dataProvider.Settings.read(key).then(function (setting) { if (!setting) { return when.reject({code: 404, message: 'Unable to find setting: ' + key}); @@ -219,11 +221,19 @@ settings = { } setting.set('value', value); return dataProvider.Settings.edit(setting, {user: self.user}).then(function (result) { - settingsCache[_.first(result).attributes.key].value = _.first(result).attributes.value; - }).then(function () { + 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 settingsObject(settingsCache); + return updatedSetting; }); + }).then(function (updatedSetting) { + var result = {}; + result[updatedSetting.key] = updatedSetting; + + return settingsResult(result); }).otherwise(errors.logAndThrowError); }); } diff --git a/core/server/api/users.js b/core/server/api/users.js index adcd6c2370..1b9cd1606e 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -114,19 +114,22 @@ users = { generateResetToken: function generateResetToken(email) { var expires = Date.now() + ONE_DAY; - return settings.read('dbHash').then(function (dbHash) { + return settings.read('dbHash').then(function (response) { + var dbHash = response.settings[0].value; return dataProvider.User.generateResetToken(email, expires, dbHash); }); }, validateToken: function validateToken(token) { - return settings.read('dbHash').then(function (dbHash) { + return settings.read('dbHash').then(function (response) { + var dbHash = response.settings[0].value; return dataProvider.User.validateToken(token, dbHash); }); }, resetPassword: function resetPassword(token, newPassword, ne2Password) { - return settings.read('dbHash').then(function (dbHash) { + return settings.read('dbHash').then(function (response) { + var dbHash = response.settings[0].value; return dataProvider.User.resetPassword(token, newPassword, ne2Password, dbHash); }); }, diff --git a/core/server/apps/index.js b/core/server/apps/index.js index b4db32de2f..44626a7ada 100644 --- a/core/server/apps/index.js +++ b/core/server/apps/index.js @@ -9,7 +9,9 @@ var _ = require('lodash'), function getInstalledApps() { - return api.settings.read('installedApps').then(function (installed) { + return api.settings.read('installedApps').then(function (response) { + var installed = response.settings[0]; + installed.value = installed.value || '[]'; try { @@ -36,7 +38,9 @@ module.exports = { try { // We have to parse the value because it's a string - api.settings.read('activeApps').then(function (aApps) { + api.settings.read('activeApps').then(function (response) { + var aApps = response.settings[0]; + appsToLoad = JSON.parse(aApps.value) || []; }); } catch (e) { diff --git a/core/server/config/theme.js b/core/server/config/theme.js index a58ab925b7..bd1d75ec85 100644 --- a/core/server/config/theme.js +++ b/core/server/config/theme.js @@ -25,10 +25,10 @@ function update(settings, configUrl) { ]).then(function (globals) { // normalise the URL by removing any trailing slash themeConfig.url = configUrl.replace(/\/$/, ''); - themeConfig.title = globals[0].value; - themeConfig.description = globals[1].value; - themeConfig.logo = globals[2] ? globals[2].value : ''; - themeConfig.cover = globals[3] ? globals[3].value : ''; + themeConfig.title = globals[0].settings[0].value; + themeConfig.description = globals[1].settings[0].value; + themeConfig.logo = globals[2].settings[0] ? globals[2].settings[0].value : ''; + themeConfig.cover = globals[3].settings[0] ? globals[3].settings[0].value : ''; return; }); } diff --git a/core/server/config/url.js b/core/server/config/url.js index 58b92b85db..7e1f4453af 100644 --- a/core/server/config/url.js +++ b/core/server/config/url.js @@ -142,7 +142,9 @@ function urlFor(context, data, absolute) { // - post - a json object representing a post // - absolute (optional, default:false) - boolean whether or not the url should be absolute function urlForPost(settings, post, absolute) { - return settings.read('permalinks').then(function (permalinks) { + return settings.read('permalinks').then(function (response) { + var permalinks = response.settings[0]; + return urlFor('post', {post: post, permalinks: permalinks}, absolute); }); } diff --git a/core/server/controllers/frontend.js b/core/server/controllers/frontend.js index dce94d37b3..554982ac09 100644 --- a/core/server/controllers/frontend.js +++ b/core/server/controllers/frontend.js @@ -21,8 +21,9 @@ var moment = require('moment'), staticPostPermalink = new Route(null, '/:slug/:edit?'); function getPostPage(options) { - return api.settings.read('postsPerPage').then(function (postPP) { - var postsPerPage = parseInt(postPP.value, 10); + return api.settings.read('postsPerPage').then(function (response) { + var postPP = response.settings[0], + postsPerPage = parseInt(postPP.value, 10); // No negative posts per page, must be number if (!isNaN(postsPerPage) && postsPerPage > 0) { @@ -121,16 +122,17 @@ frontendControllers = { // Render the page of posts filters.doFilter('prePostsRender', page.posts).then(function (posts) { - api.settings.read('activeTheme').then(function (activeTheme) { - var paths = config().paths.availableThemes[activeTheme.value], + api.settings.read('activeTheme').then(function (response) { + var activeTheme = response.settings[0], + paths = config().paths.availableThemes[activeTheme.value], view = paths.hasOwnProperty('tag.hbs') ? 'tag' : 'index', // Format data for template - response = _.extend(formatPageResponse(posts, page), { + result = _.extend(formatPageResponse(posts, page), { tag: page.meta.filters.tags ? page.meta.filters.tags[0] : '' }); - res.render(view, response); + res.render(view, result); }); }); }).otherwise(handleError(next)); @@ -141,7 +143,10 @@ frontendControllers = { editFormat, usingStaticPermalink = false; - api.settings.read('permalinks').then(function (permalink) { + api.settings.read('permalinks').then(function (response) { + var permalink = response.settings[0], + postLookup; + editFormat = permalink.value[permalink.value.length - 1] === '/' ? ':edit?' : '/:edit?'; // Convert saved permalink into an express Route object @@ -167,7 +172,7 @@ frontendControllers = { params = permalink.params; // Sanitize params we're going to use to lookup the post. - var postLookup = _.pick(permalink.params, 'slug', 'id'); + postLookup = _.pick(permalink.params, 'slug', 'id'); // Add author, tag and fields postLookup.include = 'author,tags,fields'; @@ -194,8 +199,9 @@ frontendControllers = { setReqCtx(req, post); filters.doFilter('prePostsRender', post).then(function (post) { - api.settings.read('activeTheme').then(function (activeTheme) { - var paths = config().paths.availableThemes[activeTheme.value], + api.settings.read('activeTheme').then(function (response) { + var activeTheme = response.settings[0], + paths = config().paths.availableThemes[activeTheme.value], view = template.getThemeViewForPost(paths, post); res.render(view, {post: post}); @@ -285,11 +291,11 @@ frontendControllers = { return api.posts.browse(options).then(function (page) { - var title = result[0].value.value, - description = result[1].value.value, - permalinks = result[2].value, + var title = result[0].value.settings[0].value, + description = result[1].value.settings[0].value, + permalinks = result[2].value.settings[0], siteUrl = config.urlFor('home', {secure: req.secure}, true), - feedUrl = config.urlFor('rss', {secure: req.secure}, true), + feedUrl = config.urlFor('rss', {secure: req.secure}, true), maxPage = page.meta.pagination.pages, feedItems = [], feed; @@ -348,8 +354,8 @@ frontendControllers = { }); item.description = content; feed.item(item); - deferred.resolve(); feedItems.push(deferred.promise); + deferred.resolve(); }); }); diff --git a/core/server/data/import/000.js b/core/server/data/import/000.js index fbc57b9d34..fd509c404c 100644 --- a/core/server/data/import/000.js +++ b/core/server/data/import/000.js @@ -1,7 +1,13 @@ var when = require('when'), _ = require('lodash'), models = require('../../models'), - Importer000; + Importer000, + updatedSettingKeys; + +updatedSettingKeys = { + activePlugins: 'activeApps', + installedPlugins: 'installedApps' +}; Importer000 = function () { @@ -121,6 +127,12 @@ function importSettings(ops, tableData, transaction) { tableData = _.filter(tableData, function (data) { return blackList.indexOf(data.type) === -1; }); + + // Clean up legacy plugin setting references + _.each(tableData, function (datum) { + datum.key = updatedSettingKeys[datum.key] || datum.key; + }); + ops.push(models.Settings.edit(tableData, {user: 1, transacting: transaction}) // add pass-through error handling so that bluebird doesn't think we've dropped it .otherwise(function (error) { return when.reject(error); })); diff --git a/core/server/errorHandling.js b/core/server/errorHandling.js index 9452908e71..f7294605a8 100644 --- a/core/server/errorHandling.js +++ b/core/server/errorHandling.js @@ -125,7 +125,6 @@ errors = { renderErrorPage: function (code, err, req, res, next) { /*jshint unused:false*/ - var self = this; function parseStack(stack) { @@ -192,7 +191,7 @@ errors = { } // Are we admin? If so, don't worry about the user template - if ((res.isAdmin && req.session.user) || userErrorTemplateExists === true) { + if ((res.isAdmin && req.session && req.session.user) || userErrorTemplateExists === true) { return renderErrorInt(); } diff --git a/core/server/helpers/index.js b/core/server/helpers/index.js index 937d10ca75..b4d89ec52a 100644 --- a/core/server/helpers/index.js +++ b/core/server/helpers/index.js @@ -385,8 +385,9 @@ coreHelpers.body_class = function (options) { classes.push('page'); } - return api.settings.read('activeTheme').then(function (activeTheme) { - var paths = config().paths.availableThemes[activeTheme.value], + return api.settings.read('activeTheme').then(function (response) { + var activeTheme = response.settings[0], + paths = config().paths.availableThemes[activeTheme.value], view; if (post) { @@ -445,8 +446,8 @@ coreHelpers.ghost_head = function (options) { head.push(''); - head.push(''); + head.push(''); return coreHelpers.url.call(self, {hash: {absolute: true}}).then(function (url) { head.push(''); @@ -530,9 +531,9 @@ coreHelpers.e = function (key, defaultString, options) { api.settings.read('defaultLang'), api.settings.read('forceI18n') ]).then(function (values) { - if (values[0].value === 'en' - && _.isEmpty(options.hash) - && _.isEmpty(values[1].value)) { + if (values[0].settings.value === 'en' && + _.isEmpty(options.hash) && + _.isEmpty(values[1].settings.value)) { output = defaultString; } else { output = polyglot().t(key, options.hash); @@ -651,18 +652,18 @@ coreHelpers.pagination = function (options) { errors.logAndThrowError('pagination data is not an object or is a function'); return; } - if (_.isUndefined(this.pagination.page) || _.isUndefined(this.pagination.pages) - || _.isUndefined(this.pagination.total) || _.isUndefined(this.pagination.limit)) { + if (_.isUndefined(this.pagination.page) || _.isUndefined(this.pagination.pages) || + _.isUndefined(this.pagination.total) || _.isUndefined(this.pagination.limit)) { errors.logAndThrowError('All values must be defined for page, pages, limit and total'); return; } - if ((!_.isNull(this.pagination.next) && !_.isNumber(this.pagination.next)) - || (!_.isNull(this.pagination.prev) && !_.isNumber(this.pagination.prev))) { + if ((!_.isNull(this.pagination.next) && !_.isNumber(this.pagination.next)) || + (!_.isNull(this.pagination.prev) && !_.isNumber(this.pagination.prev))) { errors.logAndThrowError('Invalid value, Next/Prev must be a number'); return; } - if (!_.isNumber(this.pagination.page) || !_.isNumber(this.pagination.pages) - || !_.isNumber(this.pagination.total) || !_.isNumber(this.pagination.limit)) { + if (!_.isNumber(this.pagination.page) || !_.isNumber(this.pagination.pages) || + !_.isNumber(this.pagination.total) || !_.isNumber(this.pagination.limit)) { errors.logAndThrowError('Invalid value, check page, pages, limit and total are numbers'); return; } diff --git a/core/server/index.js b/core/server/index.js index b25226585c..d9888fda3f 100644 --- a/core/server/index.js +++ b/core/server/index.js @@ -53,19 +53,21 @@ function doFirstRun() { } function initDbHashAndFirstRun() { - return when(api.settings.read('dbHash')).then(function (hash) { - // we already ran this, chill - // Holds the dbhash (mainly used for cookie secret) - dbHash = hash.value; + return when(api.settings.read('dbHash')).then(function (response) { + var hash = response.settings[0].value, + initHash; + + dbHash = hash; if (dbHash === null) { - var initHash = uuid.v4(); - return when(api.settings.edit.call({user: 1}, 'dbHash', initHash)).then(function (settings) { - dbHash = settings.dbHash; + initHash = uuid.v4(); + return when(api.settings.edit.call({user: 1}, 'dbHash', initHash)).then(function (response) { + dbHash = response.settings[0].value; return dbHash; }).then(doFirstRun); } - return dbHash.value; + + return dbHash; }); } diff --git a/core/server/mail.js b/core/server/mail.js index 3aa37c7cb6..08453d3ec7 100644 --- a/core/server/mail.js +++ b/core/server/mail.js @@ -108,8 +108,10 @@ GhostMailer.prototype.send = function (message) { return when.reject(new Error('Email Error: Incomplete message data.')); } - return api.settings.read('email').then(function (email) { - var to = message.to || email.value; + return api.settings.read('email').then(function (response) { + + var email = response.settings[0], + to = message.to || email.value; message = _.extend(message, { from: self.fromAddress(), diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index 77e6404fb0..111a8a42bc 100644 --- a/core/server/middleware/index.js +++ b/core/server/middleware/index.js @@ -149,7 +149,9 @@ function manageAdminAndTheme(req, res, next) { expressServer.enable(expressServer.get('activeTheme')); expressServer.disable('admin'); } - api.settings.read('activeTheme').then(function (activeTheme) { + api.settings.read('activeTheme').then(function (response) { + var activeTheme = response.settings[0]; + // Check if the theme changed if (activeTheme.value !== expressServer.get('activeTheme')) { // Change theme diff --git a/core/server/middleware/middleware.js b/core/server/middleware/middleware.js index 0f67ffe178..e998c4bd3b 100644 --- a/core/server/middleware/middleware.js +++ b/core/server/middleware/middleware.js @@ -99,7 +99,7 @@ var middleware = { // Check if we're logged in, and if so, redirect people back to dashboard // Login and signup forms in particular redirectToDashboard: function (req, res, next) { - if (req.session.user) { + if (req.session && req.session.user) { return res.redirect(config().paths.subdir + '/ghost/'); } @@ -170,7 +170,8 @@ var middleware = { // to allow unit testing forwardToExpressStatic: function (req, res, next) { - api.settings.read('activeTheme').then(function (activeTheme) { + api.settings.read('activeTheme').then(function (response) { + var activeTheme = response.settings[0]; // For some reason send divides the max age number by 1000 express['static'](path.join(config().paths.themePath, activeTheme.value), {maxAge: ONE_HOUR_MS})(req, res, next); }); diff --git a/core/server/models/settings.js b/core/server/models/settings.js index 56ad6e54b6..35ea9bfdb3 100644 --- a/core/server/models/settings.js +++ b/core/server/models/settings.js @@ -15,7 +15,6 @@ function parseDefaultSettings() { var defaultSettingsInCategories = require('../data/default-settings.json'), defaultSettingsFlattened = {}; - _.each(defaultSettingsInCategories, function (settings, categoryName) { _.each(settings, function (setting, settingName) { setting.type = categoryName; @@ -46,7 +45,6 @@ Settings = ghostBookshelf.Model.extend({ validation.validateSettings(defaultSettings, this); }, - saving: function () { // disabling sanitization until we can implement a better version // All blog setting keys that need their values to be escaped. @@ -63,9 +61,7 @@ Settings = ghostBookshelf.Model.extend({ if (!_.isObject(_key)) { _key = { key: _key }; } - return when(ghostBookshelf.Model.read.call(this, _key)).then(function (element) { - return element; - }); + return when(ghostBookshelf.Model.read.call(this, _key)); }, edit: function (_data, options) { @@ -77,13 +73,16 @@ Settings = ghostBookshelf.Model.extend({ return when.map(_data, function (item) { // Accept an array of models as input if (item.toJSON) { item = item.toJSON(); } + if (!(_.isString(item.key) && item.key.length > 0)) { + return when.reject(new Error('Setting key cannot be empty.')); + } return Settings.forge({ key: item.key }).fetch(options).then(function (setting) { if (setting) { return setting.save({value: item.value}, options); } - return Settings.forge({ key: item.key, value: item.value }).save(null, options); + return when.reject(new Error('Unable to find setting to update: ' + item.key)); }, errors.logAndThrowError); }); diff --git a/core/server/update-check.js b/core/server/update-check.js index d595d02c1c..8e2631f8ee 100644 --- a/core/server/update-check.js +++ b/core/server/update-check.js @@ -53,7 +53,8 @@ function updateCheckData() { ops.push(api.settings.read('dbHash').otherwise(errors.rejectError)); ops.push(api.settings.read('activeTheme').otherwise(errors.rejectError)); ops.push(api.settings.read('activeApps') - .then(function (apps) { + .then(function (response) { + var apps = response.settings[0]; try { apps = JSON.parse(apps.value); } catch (e) { @@ -73,8 +74,8 @@ function updateCheckData() { data.email_transport = mailConfig && (mailConfig.options && mailConfig.options.service ? mailConfig.options.service : mailConfig.transport); return when.settle(ops).then(function (descriptors) { - var hash = descriptors[0].value, - theme = descriptors[1].value, + var hash = descriptors[0].value.settings[0], + theme = descriptors[1].value.settings[0], apps = descriptors[2].value, posts = descriptors[3].value, users = descriptors[4].value, @@ -85,9 +86,9 @@ function updateCheckData() { data.blog_id = crypto.createHash('md5').update(blogId).digest('hex'); data.theme = theme ? theme.value : ''; data.apps = apps || ''; - data.post_count = posts && posts.total ? posts.total : 0; - data.user_count = users && users.length ? users.length : 0; - data.blog_created_at = users && users[0] && users[0].created_at ? moment(users[0].created_at).unix() : ''; + data.post_count = posts && posts.posts && posts.posts.total ? posts.total : 0; + data.user_count = users && users.users && users.users.length ? users.length : 0; + data.blog_created_at = users && users.users && users.users[0] && users.users[0].created_at ? moment(users.users[0].created_at).unix() : ''; data.npm_version = _.isArray(npm) && npm[0] ? npm[0].toString().replace(/\n/, '') : ''; return data; @@ -170,7 +171,9 @@ function updateCheck() { // No update check deferred.resolve(); } else { - api.settings.read('nextUpdateCheck').then(function (nextUpdateCheck) { + api.settings.read('nextUpdateCheck').then(function (result) { + var nextUpdateCheck = result.settings[0]; + if (nextUpdateCheck && nextUpdateCheck.value && nextUpdateCheck.value > moment().unix()) { // It's not time to check yet deferred.resolve(); @@ -188,7 +191,9 @@ function updateCheck() { } function showUpdateNotification() { - return api.settings.read('displayUpdateNotification').then(function (display) { + return api.settings.read('displayUpdateNotification').then(function (response) { + var display = response.settings[0]; + // Version 0.4 used boolean to indicate the need for an update. This special case is // translated to the version string. // TODO: remove in future version. diff --git a/core/test/functional/routes/api/settings_test.js b/core/test/functional/routes/api/settings_test.js index c8047e1149..f4398c8e41 100644 --- a/core/test/functional/routes/api/settings_test.js +++ b/core/test/functional/routes/api/settings_test.js @@ -113,8 +113,10 @@ describe('Settings API', function () { var jsonResponse = res.body; jsonResponse.should.exist; - testUtils.API.checkResponseValue(jsonResponse, ['key', 'value']); - jsonResponse.key.should.eql('title'); + jsonResponse.settings.should.exist; + + testUtils.API.checkResponseValue(jsonResponse.settings[0], ['id','uuid','key','value','type','created_at','created_by','updated_at','updated_by']); + jsonResponse.settings[0].key.should.eql('title'); done(); }); }); @@ -144,13 +146,17 @@ describe('Settings API', function () { } var jsonResponse = res.body, - changedValue = 'Ghost changed'; + changedValue = 'Ghost changed', + settingToChange = { + title: changedValue + }; + jsonResponse.should.exist; - jsonResponse.title = changedValue; + jsonResponse.settings.should.exist; request.put(testUtils.API.getApiQuery('settings/')) .set('X-CSRF-Token', csrfToken) - .send(jsonResponse) + .send(settingToChange) .expect(200) .end(function (err, res) { if (err) { @@ -161,7 +167,7 @@ describe('Settings API', function () { res.headers['x-cache-invalidate'].should.eql('/*'); res.should.be.json; putBody.should.exist; - putBody.title.should.eql(changedValue); + putBody.settings[0].value.should.eql(changedValue); testUtils.API.checkResponse(putBody, 'settings'); done(); }); diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js index 7e302f4c0f..cbf3f26288 100644 --- a/core/test/integration/api/api_settings_spec.js +++ b/core/test/integration/api/api_settings_spec.js @@ -19,6 +19,9 @@ describe('Settings API', function () { .then(function () { return testUtils.insertDefaultFixtures(); }) + .then(function () { + return SettingsAPI.updateSettingsCache(); + }) .then(function () { done(); }, done); @@ -31,12 +34,46 @@ describe('Settings API', function () { }); it('can browse', function (done) { - SettingsAPI.updateSettingsCache().then(function () { - SettingsAPI.browse('blog').then(function (results) { - should.exist(results); - testUtils.API.checkResponse(results, 'settings'); - done(); - }); - }); + return SettingsAPI.browse('blog').then(function (results) { + should.exist(results); + testUtils.API.checkResponse(results, 'settings'); + results.settings.length.should.be.above(0); + testUtils.API.checkResponse(results.settings[0], 'setting'); + + done(); + }).catch(done); + }); + + it('can read by string', function (done) { + return SettingsAPI.read('title').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 read by object key', function (done) { + return SettingsAPI.read({ key: 'title' }).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', function (done) { + return SettingsAPI.edit('title', '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); }); }); \ No newline at end of file diff --git a/core/test/integration/model/model_settings_spec.js b/core/test/integration/model/model_settings_spec.js index c9c3c84c0c..fd08f9e2da 100644 --- a/core/test/integration/model/model_settings_spec.js +++ b/core/test/integration/model/model_settings_spec.js @@ -206,9 +206,8 @@ describe('Settings Model', function () { return SettingsModel.findAll(); }).then(function (allSettings) { allSettings.length.should.be.above(0); - return SettingsModel.read('description').then(function (descriptionSetting) { - return descriptionSetting; - }); + + 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 @@ -218,7 +217,7 @@ describe('Settings Model', function () { }); it('doesn\'t overwrite any existing settings', function (done) { - SettingsModel.edit({key: 'description', value: 'Adam\'s Blog'}, {user: 1}).then(function () { + SettingsModel.add({key: 'description', value: 'Adam\'s Blog'}, {user: 1}).then(function () { return SettingsModel.populateDefaults(); }).then(function () { return SettingsModel.read('description'); diff --git a/core/test/unit/config_spec.js b/core/test/unit/config_spec.js index e7eada4db8..2640137848 100644 --- a/core/test/unit/config_spec.js +++ b/core/test/unit/config_spec.js @@ -30,7 +30,7 @@ describe('Config', function () { settings = {'read': function read() {}}; settingsStub = sandbox.stub(settings, 'read', function () { - return when({value: 'casper'}); + return when({ settings: [{value: 'casper'}] }); }); theme.update(settings, 'http://my-ghost-blog.com') @@ -265,7 +265,7 @@ describe('Config', function () { it('should output correct url for post', function (done) { var settings = {'read': function read() {}}, settingsStub = sandbox.stub(settings, 'read', function () { - return when({value: '/:slug/'}); + return when({ settings: [{value: '/:slug/'}] }); }), testData = testUtils.DataGenerator.Content.posts[2], postLink = '/short-and-sweet/'; @@ -302,7 +302,7 @@ describe('Config', function () { it('should output correct url for post with date permalink', function (done) { var settings = {'read': function read() {}}, settingsStub = sandbox.stub(settings, 'read', function () { - return when({value: '/:year/:month/:day/:slug/'}); + return when({ settings: [{value: '/:year/:month/:day/:slug/'}] }); }), testData = testUtils.DataGenerator.Content.posts[2], today = new Date(), @@ -342,7 +342,7 @@ describe('Config', function () { it('should output correct url for page with date permalink', function (done) { var settings = {'read': function read() {}}, settingsStub = sandbox.stub(settings, 'read', function () { - return when({value: '/:year/:month/:day/:slug/'}); + return when({ settings: [{value: '/:year/:month/:day/:slug/'}] }); }), testData = testUtils.DataGenerator.Content.posts[5], postLink = '/static-page-test/'; diff --git a/core/test/unit/frontend_spec.js b/core/test/unit/frontend_spec.js index 787768f730..c517da59bc 100644 --- a/core/test/unit/frontend_spec.js +++ b/core/test/unit/frontend_spec.js @@ -44,9 +44,11 @@ describe('Frontend Controller', function () { }); apiSettingsStub = sandbox.stub(api.settings, 'read'); - apiSettingsStub.withArgs('postsPerPage').returns(when({ - 'key': 'postsPerPage', - 'value': 6 + apiSettingsStub.withArgs('postsPerPage').returns(when({ + settings: [{ + 'key': 'postsPerPage', + 'value': 6 + }] })); }); @@ -199,13 +201,17 @@ describe('Frontend Controller', function () { apiSettingsStub = sandbox.stub(api.settings, 'read'); apiSettingsStub.withArgs('activeTheme').returns(when({ - 'key': 'activeTheme', - 'value': 'casper' + settings: [{ + 'key': 'activeTheme', + 'value': 'casper' + }] })); apiSettingsStub.withArgs('postsPerPage').returns(when({ - 'key': 'postsPerPage', - 'value': '10' + settings: [{ + 'key': 'postsPerPage', + 'value': '10' + }] })); frontend.__set__('config', sandbox.stub().returns({ @@ -265,8 +271,10 @@ describe('Frontend Controller', function () { apiSettingsStub = sandbox.stub(api.settings, 'read'); apiSettingsStub.withArgs('postsPerPage').returns(when({ - 'key': 'postsPerPage', - 'value': 6 + settings: [{ + 'key': 'postsPerPage', + 'value': 6 + }] })); }); @@ -415,8 +423,10 @@ describe('Frontend Controller', function () { apiSettingsStub = sandbox.stub(api.settings, 'read'); apiSettingsStub.withArgs('activeTheme').returns(when({ - 'key': 'activeTheme', - 'value': 'casper' + settings: [{ + 'key': 'activeTheme', + 'value': 'casper' + }] })); frontend.__set__('config', sandbox.stub().returns({ @@ -441,7 +451,9 @@ describe('Frontend Controller', function () { describe('custom page templates', function () { beforeEach(function () { apiSettingsStub.withArgs('permalinks').returns(when({ - value: '/:slug/' + settings: [{ + value: '/:slug/' + }] })); }); @@ -463,7 +475,9 @@ describe('Frontend Controller', function () { describe('permalink set to slug', function () { beforeEach(function () { apiSettingsStub.withArgs('permalinks').returns(when({ - value: '/:slug/' + settings: [{ + value: '/:slug/' + }] })); }); @@ -532,7 +546,9 @@ describe('Frontend Controller', function () { describe('permalink set to date', function () { beforeEach(function () { apiSettingsStub.withArgs('permalinks').returns(when({ + settings: [{ value: '/:year/:month/:day/:slug/' + }] })); }); @@ -603,7 +619,9 @@ describe('Frontend Controller', function () { describe('permalink set to slug', function () { beforeEach(function () { apiSettingsStub.withArgs('permalinks').returns(when({ - value: '/:slug' + settings: [{ + value: '/:slug' + }] })); }); @@ -674,7 +692,9 @@ describe('Frontend Controller', function () { describe('permalink set to date', function () { beforeEach(function () { apiSettingsStub.withArgs('permalinks').returns(when({ - value: '/:year/:month/:day/:slug' + settings: [{ + value: '/:year/:month/:day/:slug' + }] })); }); @@ -762,7 +782,9 @@ describe('Frontend Controller', function () { describe('permalink set to custom format', function () { beforeEach(function () { apiSettingsStub.withArgs('permalinks').returns(when({ - value: '/:year/:slug' + settings: [{ + value: '/:year/:slug' + }] })); }); @@ -891,16 +913,22 @@ describe('Frontend Controller', function () { apiSettingsStub = sandbox.stub(api.settings, 'read'); apiSettingsStub.withArgs('title').returns(when({ - 'key': 'title', - 'value': 'Test' + settings: [{ + 'key': 'title', + 'value': 'Test' + }] })); apiSettingsStub.withArgs('description').returns(when({ - 'key': 'description', - 'value': 'Some Text' + settings: [{ + 'key': 'description', + 'value': 'Some Text' + }] })); apiSettingsStub.withArgs('permalinks').returns(when({ - 'key': 'permalinks', - 'value': '/:slug/' + settings: [{ + 'key': 'permalinks', + 'value': '/:slug/' + }] })); }); diff --git a/core/test/unit/import_spec.js b/core/test/unit/import_spec.js index eee562cffc..e0dcb71a82 100644 --- a/core/test/unit/import_spec.js +++ b/core/test/unit/import_spec.js @@ -303,7 +303,7 @@ describe("Import", function () { }).then(function () { (1).should.eql(0, 'Data import should not resolve promise.'); }, function (error) { - error.should.eql('Error importing data: Value in [settings.key] cannot be blank.'); + error.should.eql('Error importing data: Setting key cannot be empty.'); when.all([ knex("users").select(), @@ -423,7 +423,7 @@ describe("Import", function () { done(); }).otherwise(function (error) { done(new Error(error)); - }) + }); }); it("doesn't import invalid post data from 002", function (done) { @@ -486,7 +486,7 @@ describe("Import", function () { }).then(function () { (1).should.eql(0, 'Data import should not resolve promise.'); }, function (error) { - error.should.eql('Error importing data: Value in [settings.key] cannot be blank.'); + error.should.eql('Error importing data: Setting key cannot be empty.'); when.all([ knex("users").select(), diff --git a/core/test/unit/server_helpers_index_spec.js b/core/test/unit/server_helpers_index_spec.js index e7727a4dd5..3c632563ab 100644 --- a/core/test/unit/server_helpers_index_spec.js +++ b/core/test/unit/server_helpers_index_spec.js @@ -32,7 +32,9 @@ describe('Core Helpers', function () { helpers = rewire('../../server/helpers'); sandbox = sinon.sandbox.create(); apiStub = sandbox.stub(api.settings, 'read', function (arg) { - return when({value: 'casper'}); + return when({ + settings: [{value: 'casper'}] + }); }); config = helpers.__get__('config'); @@ -1186,7 +1188,9 @@ describe('Core Helpers', function () { apiStub = sandbox.stub(api.settings, 'read', function () { var futureversion = packageInfo.version.split('.'); futureversion[futureversion.length - 1] = parseInt(futureversion[futureversion.length - 1], 10) + 1; - return when({value: futureversion.join('.')}); + return when({ + settings: [{value: futureversion.join('.')}] + }); }); helpers.update_notification.call({currentUser: {name: 'bob'}}).then(function (rendered) { @@ -1208,7 +1212,7 @@ describe('Core Helpers', function () { it('does NOT output a correctly formatted notification when db version equals package version', function (done) { apiStub.restore(); apiStub = sandbox.stub(api.settings, 'read', function () { - return when({value: packageInfo.version}); + return when({ settings: [{value: packageInfo.version}] }); }); helpers.update_notification.call({currentUser: {name: 'bob'}}).then(function (rendered) { @@ -1223,7 +1227,7 @@ describe('Core Helpers', function () { apiStub.restore(); apiStub = sandbox.stub(api.settings, 'read', function () { - return when({value: 'true'}); + return when({ settings: [{value: 'true'}] }); }); helpers.update_notification.call({currentUser: {name: 'bob'}}).then(function (rendered) { @@ -1238,7 +1242,7 @@ describe('Core Helpers', function () { apiStub = sandbox.stub(api.settings, 'read', function () { var futureversion = packageInfo.version.split('.'); futureversion[futureversion.length-1] = parseInt(futureversion[futureversion.length-1], 10) + 1; - return when({value: futureversion.join('.')}); + return when({ settings: [{value: futureversion.join('.')}] }); }); helpers.update_notification.call().then(function (rendered) { diff --git a/core/test/unit/xmlrpc_spec.js b/core/test/unit/xmlrpc_spec.js index 312d885e65..47c7e9d75e 100644 --- a/core/test/unit/xmlrpc_spec.js +++ b/core/test/unit/xmlrpc_spec.js @@ -32,7 +32,7 @@ describe('XMLRPC', function () { ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200), testPost = testUtils.DataGenerator.Content.posts[2], settingsStub = sandbox.stub(settings, 'read', function () { - return when({value: '/:slug/'}); + return when({ settings: [{value: '/:slug/'}] }); }); xmlrpc.ping(testPost).then(function () { diff --git a/core/test/utils/api.js b/core/test/utils/api.js index 211c792e6d..eb87fdc6ef 100644 --- a/core/test/utils/api.js +++ b/core/test/utils/api.js @@ -10,10 +10,8 @@ var _ = require('lodash'), post: ['id', 'uuid', 'title', 'slug', 'markdown', 'html', 'meta_title', 'meta_description', 'featured', 'image', 'status', 'language', 'created_at', 'created_by', 'updated_at', 'updated_by', 'published_at', 'published_by', 'page', 'author', 'tags', 'fields'], - // TODO: remove databaseVersion, dbHash - settings: ['databaseVersion', 'dbHash', 'title', 'description', 'email', 'logo', 'cover', 'defaultLang', - "permalinks", 'postsPerPage', 'forceI18n', 'activeTheme', 'activeApps', 'installedApps', - 'availableThemes', 'availableApps', 'nextUpdateCheck', 'displayUpdateNotification'], + settings: ['settings'], + setting: ['id','uuid','key','value','type','created_at','created_by','updated_at','updated_by'], tag: ['id', 'uuid', 'name', 'slug', 'description', 'parent', 'meta_title', 'meta_description', 'created_at', 'created_by', 'updated_at', 'updated_by'], user: ['id', 'uuid', 'name', 'slug', 'email', 'image', 'cover', 'bio', 'website', @@ -44,6 +42,10 @@ function checkResponse (jsonResponse, objectType) { function checkResponseValue (jsonResponse, properties) { Object.keys(jsonResponse).length.should.eql(properties.length); for(var i=0; i