From 397400b4f89a7c95fae9cd2239b69ed9030d65bc Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Wed, 13 Dec 2017 13:19:51 +0100 Subject: [PATCH] Moved visibility utility to static model fn (#9327) refs #9178 - this logic belongs to a static model helper - the visibility property is a model property, the knowledge about the visibility values belongs to the model - rename the functions, so they make more sense --- core/server/data/meta/keywords.js | 6 ++-- core/server/helpers/foreach.js | 6 ++-- core/server/helpers/proxy.js | 5 +-- core/server/helpers/tags.js | 23 ++++++------ core/server/models/base/index.js | 46 +++++++++++++++++++++++- core/server/utils/visibility.js | 37 ------------------- core/test/unit/metadata/keywords_spec.js | 6 ++++ 7 files changed, 72 insertions(+), 57 deletions(-) delete mode 100644 core/server/utils/visibility.js diff --git a/core/server/data/meta/keywords.js b/core/server/data/meta/keywords.js index 96eb180344..4b6d43ae3b 100644 --- a/core/server/data/meta/keywords.js +++ b/core/server/data/meta/keywords.js @@ -1,8 +1,10 @@ -var visibilityFilter = require('../../utils/visibility').filter; +'use strict'; + +const models = require('../../models'); function getKeywords(data) { if (data.post && data.post.tags && data.post.tags.length > 0) { - return visibilityFilter(data.post.tags, ['public'], false, function processItem(item) { + return models.Base.Model.filterByVisibility(data.post.tags, ['public'], false, function processItem(item) { return item.name; }); } diff --git a/core/server/helpers/foreach.js b/core/server/helpers/foreach.js index 39b3b3dcb0..637c01d321 100644 --- a/core/server/helpers/foreach.js +++ b/core/server/helpers/foreach.js @@ -6,14 +6,14 @@ var proxy = require('./proxy'), _ = require('lodash'), logging = proxy.logging, i18n = proxy.i18n, - visibilityUtils = proxy.visibility, + models = proxy.models, hbsUtils = proxy.hbs.Utils, createFrame = proxy.hbs.handlebars.createFrame; function filterItemsByVisibility(items, options) { - var visibility = visibilityUtils.parser(options); + var visibilityArr = models.Base.Model.parseVisibilityString(options.hash.visibility); - return visibilityUtils.filter(items, visibility, !!options.hash.visibility); + return models.Base.Model.filterByVisibility(items, visibilityArr, !!options.hash.visibility); } module.exports = function foreach(items, options) { diff --git a/core/server/helpers/proxy.js b/core/server/helpers/proxy.js index 65263856c0..67627aa45c 100644 --- a/core/server/helpers/proxy.js +++ b/core/server/helpers/proxy.js @@ -20,6 +20,8 @@ module.exports = { // TODO: Expose less of the API to make this safe api: require('../api'), + models: require('../models'), + // TODO: Only expose "get" settingsCache: settingsCache, @@ -76,6 +78,5 @@ module.exports = { return result; }, null); } - }, - visibility: require('../utils/visibility') + } }; diff --git a/core/server/helpers/tags.js b/core/server/helpers/tags.js index 5b0f052b79..411473af89 100644 --- a/core/server/helpers/tags.js +++ b/core/server/helpers/tags.js @@ -8,25 +8,24 @@ var proxy = require('./proxy'), _ = require('lodash'), - SafeString = proxy.SafeString, templates = proxy.templates, url = proxy.url, - visibilityUtils = proxy.visibility; + models = proxy.models; module.exports = function tags(options) { options = options || {}; options.hash = options.hash || {}; - var autolink = !(_.isString(options.hash.autolink) && options.hash.autolink === 'false'), - separator = _.isString(options.hash.separator) ? options.hash.separator : ', ', - prefix = _.isString(options.hash.prefix) ? options.hash.prefix : '', - suffix = _.isString(options.hash.suffix) ? options.hash.suffix : '', - limit = options.hash.limit ? parseInt(options.hash.limit, 10) : undefined, - from = options.hash.from ? parseInt(options.hash.from, 10) : 1, - to = options.hash.to ? parseInt(options.hash.to, 10) : undefined, - visibility = visibilityUtils.parser(options), - output = ''; + var autolink = !(_.isString(options.hash.autolink) && options.hash.autolink === 'false'), + separator = _.isString(options.hash.separator) ? options.hash.separator : ', ', + prefix = _.isString(options.hash.prefix) ? options.hash.prefix : '', + suffix = _.isString(options.hash.suffix) ? options.hash.suffix : '', + limit = options.hash.limit ? parseInt(options.hash.limit, 10) : undefined, + from = options.hash.from ? parseInt(options.hash.from, 10) : 1, + to = options.hash.to ? parseInt(options.hash.to, 10) : undefined, + visibilityArr = models.Base.Model.parseVisibilityString(options.hash.visibility), + output = ''; function createTagList(tags) { function processTag(tag) { @@ -36,7 +35,7 @@ module.exports = function tags(options) { }) : _.escape(tag.name); } - return visibilityUtils.filter(tags, visibility, !!options.hash.visibility, processTag); + return models.Base.Model.filterByVisibility(tags, visibilityArr, !!options.hash.visibility, processTag); } if (this.tags && this.tags.length) { diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index cf4e2866b8..37b2e2a228 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -809,8 +809,52 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ }); return result; - } + }, + /** + * All models which have a visibility property, can use this static helper function. + * Filter models by visibility. + * + * @param {Array|Object} items + * @param {Array} visibility + * @param {Boolean} [explicit] + * @param {Function} [fn] + * @returns {Array|Object} filtered items + */ + filterByVisibility: function filterByVisibility(items, visibility, explicit, fn) { + var memo = _.isArray(items) ? [] : {}; + + if (_.includes(visibility, 'all')) { + return fn ? _.map(items, fn) : items; + } + + // We don't want to change the structure of what is returned + return _.reduce(items, function (items, item, key) { + if (!item.visibility && !explicit || _.includes(visibility, item.visibility)) { + var newItem = fn ? fn(item) : item; + if (_.isArray(items)) { + memo.push(newItem); + } else { + memo[key] = newItem; + } + } + return memo; + }, memo); + }, + + /** + * Returns an Array of visibility values. + * e.g. public,all => ['public, 'all'] + * @param visibility + * @returns {*} + */ + parseVisibilityString: function parseVisibilityString(visibility) { + if (!visibility) { + return ['public']; + } + + return _.map(visibility.split(','), _.trim); + } }); // Export ghostBookshelf for use elsewhere diff --git a/core/server/utils/visibility.js b/core/server/utils/visibility.js deleted file mode 100644 index a85814ed5d..0000000000 --- a/core/server/utils/visibility.js +++ /dev/null @@ -1,37 +0,0 @@ -var _ = require('lodash'); -/** - * - * @param {Array|Object} items - * @param {Array} visibility - * @param {Boolean} [explicit] - * @param {Function} [fn] - * @returns {Array|Object} filtered items - */ -module.exports.filter = function visibilityFilter(items, visibility, explicit, fn) { - var memo = _.isArray(items) ? [] : {}; - - if (_.includes(visibility, 'all')) { - return fn ? _.map(items, fn) : items; - } - - // We don't want to change the structure of what is returned - return _.reduce(items, function (items, item, key) { - if (!item.visibility && !explicit || _.includes(visibility, item.visibility)) { - var newItem = fn ? fn(item) : item; - if (_.isArray(items)) { - memo.push(newItem); - } else { - memo[key] = newItem; - } - } - return memo; - }, memo); -}; - -module.exports.parser = function visibilityParser(options) { - if (!options.hash.visibility) { - return ['public']; - } - - return _.map(options.hash.visibility.split(','), _.trim); -}; diff --git a/core/test/unit/metadata/keywords_spec.js b/core/test/unit/metadata/keywords_spec.js index 1021bfacc1..b95eeeeb61 100644 --- a/core/test/unit/metadata/keywords_spec.js +++ b/core/test/unit/metadata/keywords_spec.js @@ -1,12 +1,18 @@ var should = require('should'), sinon = require('sinon'), + models = require('../../../server/models'), getKeywords = require('../../../server/data/meta/keywords'), sandbox = sinon.sandbox.create(); describe('getKeywords', function () { + before(function () { + models.init(); + }); + afterEach(function () { sandbox.restore(); }); + it('should return tags as keywords if post has tags', function () { var keywords = getKeywords({ post: {