From 369fd2c6bdf792f87f45f0521b312a138d06e013 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 2 Dec 2015 00:13:05 +0800 Subject: [PATCH 1/2] Initial work on internal tags feature refs #6165 --- core/server/api/configuration.js | 1 + core/server/api/tags.js | 2 +- .../controllers/frontend/channel-config.js | 5 +- core/server/helpers/tags.js | 5 ++ core/server/models/base/utils.js | 3 +- core/server/models/tag.js | 3 +- core/server/utils/labs.js | 2 +- .../frontend/channel-config_spec.js | 54 +++++++++++++++++++ core/test/unit/server_helpers/tags_spec.js | 38 ++++++++++++- 9 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 core/test/unit/controllers/frontend/channel-config_spec.js diff --git a/core/server/api/configuration.js b/core/server/api/configuration.js index 22073687ad..40d12b42fe 100644 --- a/core/server/api/configuration.js +++ b/core/server/api/configuration.js @@ -33,6 +33,7 @@ function getBaseConfig() { useGoogleFonts: {value: !config.isPrivacyDisabled('useGoogleFonts'), type: 'bool'}, useGravatar: {value: !config.isPrivacyDisabled('useGravatar'), type: 'bool'}, publicAPI: labsFlag('publicAPI'), + hiddenTags: labsFlag('hiddenTags'), blogUrl: {value: config.url.replace(/\/$/, ''), type: 'string'}, blogTitle: {value: config.theme.title, type: 'string'}, routeKeywords: {value: JSON.stringify(config.routeKeywords), type: 'json'} diff --git a/core/server/api/tags.js b/core/server/api/tags.js index 2310d63e0c..077466d08c 100644 --- a/core/server/api/tags.js +++ b/core/server/api/tags.js @@ -54,7 +54,7 @@ tags = { * @return {Promise} Tag */ read: function read(options) { - var attrs = ['id', 'slug'], + var attrs = ['id', 'slug', 'hidden'], tasks; /** diff --git a/core/server/controllers/frontend/channel-config.js b/core/server/controllers/frontend/channel-config.js index ac0f98e3bd..b3bc39e909 100644 --- a/core/server/controllers/frontend/channel-config.js +++ b/core/server/controllers/frontend/channel-config.js @@ -1,5 +1,6 @@ var _ = require('lodash'), config = require('../../config'), + labs = require('../../utils/labs'), channelConfig; channelConfig = function channelConfig() { @@ -13,13 +14,13 @@ channelConfig = function channelConfig() { name: 'tag', route: '/' + config.routeKeywords.tag + '/:slug/', postOptions: { - filter: 'tags:\'%s\'' + filter: labs.isSet('hiddenTags') ? 'tags:\'%s\'+tags.hidden:false' : 'tags:\'%s\'' }, data: { tag: { type: 'read', resource: 'tags', - options: {slug: '%s'} + options: labs.isSet('hiddenTags') ? {slug: '%s', hidden: false} : {slug: '%s'} } }, slugTemplate: true, diff --git a/core/server/helpers/tags.js b/core/server/helpers/tags.js index de56c0012e..2ea324be12 100644 --- a/core/server/helpers/tags.js +++ b/core/server/helpers/tags.js @@ -9,6 +9,7 @@ var hbs = require('express-hbs'), _ = require('lodash'), config = require('../config'), + labs = require('../utils/labs'), utils = require('./utils'), tags; @@ -26,6 +27,10 @@ tags = function (options) { output = ''; function createTagList(tags) { + if (labs.isSet('hiddenTags')) { + tags = _.filter(tags, 'hidden', false); + } + if (autolink) { return _.map(tags, function (tag) { return utils.linkTemplate({ diff --git a/core/server/models/base/utils.js b/core/server/models/base/utils.js index 94401d57ba..55ade2f9d4 100644 --- a/core/server/models/base/utils.js +++ b/core/server/models/base/utils.js @@ -33,8 +33,9 @@ tagUpdate = { }, createTagThenAttachTagToPost: function createTagThenAttachTagToPost(TagModel, post, tag, index, options) { + var fields = ['name', 'slug', 'description', 'image', 'hidden', 'parent_id', 'meta_title', 'meta_description']; return function () { - return TagModel.add({name: tag.name}, options).then(function then(createdTag) { + return TagModel.add(_.pick(tag, fields), options).then(function then(createdTag) { return tagUpdate.attachTagToPost(post, createdTag, index, options)(); }); }; diff --git a/core/server/models/tag.js b/core/server/models/tag.js index 227b464997..4f54eaa9dd 100644 --- a/core/server/models/tag.js +++ b/core/server/models/tag.js @@ -76,7 +76,8 @@ Tag = ghostBookshelf.Model.extend({ // these are the only options that can be passed to Bookshelf / Knex. validOptions = { findPage: ['page', 'limit', 'columns', 'filter', 'order'], - findAll: ['columns'] + findAll: ['columns'], + findOne: ['hidden'] }; if (validOptions[methodName]) { diff --git a/core/server/utils/labs.js b/core/server/utils/labs.js index ba3856ddb9..ea799a849b 100644 --- a/core/server/utils/labs.js +++ b/core/server/utils/labs.js @@ -4,7 +4,7 @@ var config = require('../config'), flagIsSet = function flagIsSet(flag) { var labsConfig = config.labs; - return !!labsConfig[flag] && labsConfig[flag] === true; + return labsConfig && labsConfig[flag] && labsConfig[flag] === true; }; module.exports.isSet = flagIsSet; diff --git a/core/test/unit/controllers/frontend/channel-config_spec.js b/core/test/unit/controllers/frontend/channel-config_spec.js new file mode 100644 index 0000000000..dc4dfb2cb3 --- /dev/null +++ b/core/test/unit/controllers/frontend/channel-config_spec.js @@ -0,0 +1,54 @@ +/*globals describe, afterEach, it*/ +/*jshint expr:true*/ +var should = require('should'), + sinon = require('sinon'), + +// Stuff we are testing + labs = require('../../../../server/utils/labs'), + channelConfig = require('../../../../server/controllers/frontend/channel-config'), + + sandbox = sinon.sandbox.create(); + +describe('Channel Config', function () { + afterEach(function () { + sandbox.restore(); + }); + + it('should get the index config', function () { + var result = channelConfig('index'); + should.exist(result); + result.name.should.eql('index'); + }); + + it('should get the author config', function () { + var result = channelConfig('author'); + should.exist(result); + result.name.should.eql('author'); + }); + + it('should get the tag config', function () { + var result = channelConfig('tag'); + should.exist(result); + result.name.should.eql('tag'); + }); + + describe('hidden tags labs flag', function () { + it('should return old tag config if labs flag is not set', function () { + sandbox.stub(labs, 'isSet').returns(false); + var result = channelConfig('tag'); + should.exist(result); + result.name.should.eql('tag'); + result.postOptions.filter.should.eql('tags:%s'); + result.data.tag.options.should.eql({slug: '%s'}); + }); + + it('should return new tag config if labs flag is not set', function () { + sandbox.stub(labs, 'isSet').returns(true); + var result = channelConfig('tag'); + should.exist(result); + result.name.should.eql('tag'); + result.postOptions.filter.should.eql('tags:%s+tags.hidden:false'); + result.data.tag.options.should.eql({slug: '%s', hidden: false}); + }); + }); +}); diff --git a/core/test/unit/server_helpers/tags_spec.js b/core/test/unit/server_helpers/tags_spec.js index 63515945f2..bd8485af5f 100644 --- a/core/test/unit/server_helpers/tags_spec.js +++ b/core/test/unit/server_helpers/tags_spec.js @@ -1,18 +1,27 @@ -/*globals describe, before, it*/ +/*globals describe, afterEach, before, it*/ + var should = require('should'), + sinon = require('sinon'), hbs = require('express-hbs'), utils = require('./utils'), rewire = require('rewire'), // Stuff we are testing handlebars = hbs.handlebars, - helpers = rewire('../../../server/helpers'); + labs = require('../../../server/utils/labs'), + helpers = rewire('../../../server/helpers'), + + sandbox = sinon.sandbox.create(); describe('{{tags}} helper', function () { before(function () { utils.loadHelpers(); }); + afterEach(function () { + sandbox.restore(); + }); + it('has loaded tags helper', function () { should.exist(handlebars.helpers.tags); }); @@ -174,4 +183,29 @@ describe('{{tags}} helper', function () { String(rendered).should.equal('foo, bar, baz'); }); + + describe('Hidden/Internal tags', function () { + it('Should output internal tags when the labs flag IS NOT set', function () { + sandbox.stub(labs, 'isSet').returns(false); + var tags = [{name: 'foo', slug: 'foo-bar', hidden: false}, {name: 'bar', slug: 'bar', hidden: true}], + rendered = helpers.tags.call( + {tags: tags} + ); + should.exist(rendered); + + String(rendered).should.equal('foo, bar'); + }); + + it('Should NOT output internal tags when the labs flag IS set', function () { + sandbox.stub(labs, 'isSet').returns(true); + + var tags = [{name: 'foo', slug: 'foo-bar', hidden: false}, {name: 'bar', slug: 'bar', hidden: true}], + rendered = helpers.tags.call( + {tags: tags} + ); + should.exist(rendered); + + String(rendered).should.equal('foo'); + }); + }); }); From aed8c0800eb708421e03f6e15955d7a89a977688 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Sat, 11 Jun 2016 09:12:04 -0600 Subject: [PATCH 2/2] internal tags feature refs #6165 - change behavior to use 'visibility' property - finish out client & server-side behavior - add tests --- core/server/api/configuration.js | 2 +- core/server/api/tags.js | 2 +- .../controllers/frontend/channel-config.js | 5 +- .../controllers/frontend/render-channel.js | 10 +++ core/server/helpers/tags.js | 4 +- core/server/models/base/index.js | 10 +++ core/server/models/base/utils.js | 2 +- core/server/models/tag.js | 2 +- .../frontend/channel-config_spec.js | 34 +--------- .../frontend/render-channel_spec.js | 68 +++++++++++++++++++ core/test/unit/server_helpers/tags_spec.js | 5 +- 11 files changed, 101 insertions(+), 43 deletions(-) create mode 100644 core/test/unit/controllers/frontend/render-channel_spec.js diff --git a/core/server/api/configuration.js b/core/server/api/configuration.js index 40d12b42fe..c36695b9d2 100644 --- a/core/server/api/configuration.js +++ b/core/server/api/configuration.js @@ -33,7 +33,7 @@ function getBaseConfig() { useGoogleFonts: {value: !config.isPrivacyDisabled('useGoogleFonts'), type: 'bool'}, useGravatar: {value: !config.isPrivacyDisabled('useGravatar'), type: 'bool'}, publicAPI: labsFlag('publicAPI'), - hiddenTags: labsFlag('hiddenTags'), + internalTags: labsFlag('internalTags'), blogUrl: {value: config.url.replace(/\/$/, ''), type: 'string'}, blogTitle: {value: config.theme.title, type: 'string'}, routeKeywords: {value: JSON.stringify(config.routeKeywords), type: 'json'} diff --git a/core/server/api/tags.js b/core/server/api/tags.js index 077466d08c..c47b7d919e 100644 --- a/core/server/api/tags.js +++ b/core/server/api/tags.js @@ -54,7 +54,7 @@ tags = { * @return {Promise} Tag */ read: function read(options) { - var attrs = ['id', 'slug', 'hidden'], + var attrs = ['id', 'slug', 'visibility'], tasks; /** diff --git a/core/server/controllers/frontend/channel-config.js b/core/server/controllers/frontend/channel-config.js index b3bc39e909..ac0f98e3bd 100644 --- a/core/server/controllers/frontend/channel-config.js +++ b/core/server/controllers/frontend/channel-config.js @@ -1,6 +1,5 @@ var _ = require('lodash'), config = require('../../config'), - labs = require('../../utils/labs'), channelConfig; channelConfig = function channelConfig() { @@ -14,13 +13,13 @@ channelConfig = function channelConfig() { name: 'tag', route: '/' + config.routeKeywords.tag + '/:slug/', postOptions: { - filter: labs.isSet('hiddenTags') ? 'tags:\'%s\'+tags.hidden:false' : 'tags:\'%s\'' + filter: 'tags:\'%s\'' }, data: { tag: { type: 'read', resource: 'tags', - options: labs.isSet('hiddenTags') ? {slug: '%s', hidden: false} : {slug: '%s'} + options: {slug: '%s'} } }, slugTemplate: true, diff --git a/core/server/controllers/frontend/render-channel.js b/core/server/controllers/frontend/render-channel.js index 10cc660b86..66d01419fd 100644 --- a/core/server/controllers/frontend/render-channel.js +++ b/core/server/controllers/frontend/render-channel.js @@ -2,6 +2,7 @@ var _ = require('lodash'), errors = require('../../errors'), filters = require('../../filters'), safeString = require('../../utils/index').safeString, + labs = require('../../utils/labs'), handleError = require('./error'), fetchData = require('./fetch-data'), formatResponse = require('./format-response'), @@ -21,6 +22,15 @@ function renderChannel(req, res, next) { channelOpts.postOptions.page = pageParam; channelOpts.slugParam = slugParam; + // this is needed here because the channel config is cloned, + // and thus changes to labs flags don't update the config + // Once internal tags is moved out of labs the functionality can be + // moved back into the channel config + if (labs.isSet('internalTags') && channelOpts.name === 'tag') { + channelOpts.postOptions.filter = 'tags:\'%s\'+tags.visibility:\'public\''; + channelOpts.data.tag.options = {slug: '%s', visibility: 'public'}; + } + // Call fetchData to get everything we need from the API return fetchData(channelOpts).then(function handleResult(result) { // If page is greater than number of pages we have, go straight to 404 diff --git a/core/server/helpers/tags.js b/core/server/helpers/tags.js index 2ea324be12..1c0871c154 100644 --- a/core/server/helpers/tags.js +++ b/core/server/helpers/tags.js @@ -27,8 +27,8 @@ tags = function (options) { output = ''; function createTagList(tags) { - if (labs.isSet('hiddenTags')) { - tags = _.filter(tags, 'hidden', false); + if (labs.isSet('internalTags')) { + tags = _.filter(tags, 'visibility', 'public'); } if (autolink) { diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js index 217b4e9a6a..75dedb31d7 100644 --- a/core/server/models/base/index.js +++ b/core/server/models/base/index.js @@ -15,6 +15,7 @@ var _ = require('lodash'), Promise = require('bluebird'), schema = require('../../data/schema'), utils = require('../../utils'), + labs = require('../../utils/labs'), uuid = require('node-uuid'), validation = require('../../data/validation'), plugins = require('../plugins'), @@ -494,6 +495,15 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ slug = (slug.indexOf('-') > -1) ? slug.substr(0, slug.indexOf('-')) : slug; } + if (!_.has(options, 'importing') || !options.importing) { + // TODO: remove the labs requirement when internal tags is out of beta + // This checks if the first character of a tag name is a #. If it is, this + // is an internal tag, and as such we should add 'hash' to the beginning of the slug + if (labs.isSet('internalTags') && baseName === 'tag' && /^#/.test(base)) { + slug = 'hash-' + slug; + } + } + // Check the filtered slug doesn't match any of the reserved keywords return filters.doFilter('slug.reservedSlugs', config.slugs.reserved).then(function then(slugList) { // Some keywords cannot be changed diff --git a/core/server/models/base/utils.js b/core/server/models/base/utils.js index 55ade2f9d4..68170dfb5d 100644 --- a/core/server/models/base/utils.js +++ b/core/server/models/base/utils.js @@ -33,7 +33,7 @@ tagUpdate = { }, createTagThenAttachTagToPost: function createTagThenAttachTagToPost(TagModel, post, tag, index, options) { - var fields = ['name', 'slug', 'description', 'image', 'hidden', 'parent_id', 'meta_title', 'meta_description']; + var fields = ['name', 'slug', 'description', 'image', 'visibility', 'parent_id', 'meta_title', 'meta_description']; return function () { return TagModel.add(_.pick(tag, fields), options).then(function then(createdTag) { return tagUpdate.attachTagToPost(post, createdTag, index, options)(); diff --git a/core/server/models/tag.js b/core/server/models/tag.js index 4f54eaa9dd..1b845cfe8f 100644 --- a/core/server/models/tag.js +++ b/core/server/models/tag.js @@ -77,7 +77,7 @@ Tag = ghostBookshelf.Model.extend({ validOptions = { findPage: ['page', 'limit', 'columns', 'filter', 'order'], findAll: ['columns'], - findOne: ['hidden'] + findOne: ['visibility'] }; if (validOptions[methodName]) { diff --git a/core/test/unit/controllers/frontend/channel-config_spec.js b/core/test/unit/controllers/frontend/channel-config_spec.js index dc4dfb2cb3..793390e388 100644 --- a/core/test/unit/controllers/frontend/channel-config_spec.js +++ b/core/test/unit/controllers/frontend/channel-config_spec.js @@ -1,19 +1,9 @@ -/*globals describe, afterEach, it*/ +/*globals describe, it*/ /*jshint expr:true*/ var should = require('should'), - sinon = require('sinon'), - -// Stuff we are testing - labs = require('../../../../server/utils/labs'), - channelConfig = require('../../../../server/controllers/frontend/channel-config'), - - sandbox = sinon.sandbox.create(); + channelConfig = require('../../../../server/controllers/frontend/channel-config').get; describe('Channel Config', function () { - afterEach(function () { - sandbox.restore(); - }); - it('should get the index config', function () { var result = channelConfig('index'); should.exist(result); @@ -31,24 +21,4 @@ describe('Channel Config', function () { should.exist(result); result.name.should.eql('tag'); }); - - describe('hidden tags labs flag', function () { - it('should return old tag config if labs flag is not set', function () { - sandbox.stub(labs, 'isSet').returns(false); - var result = channelConfig('tag'); - should.exist(result); - result.name.should.eql('tag'); - result.postOptions.filter.should.eql('tags:%s'); - result.data.tag.options.should.eql({slug: '%s'}); - }); - - it('should return new tag config if labs flag is not set', function () { - sandbox.stub(labs, 'isSet').returns(true); - var result = channelConfig('tag'); - should.exist(result); - result.name.should.eql('tag'); - result.postOptions.filter.should.eql('tags:%s+tags.hidden:false'); - result.data.tag.options.should.eql({slug: '%s', hidden: false}); - }); - }); }); diff --git a/core/test/unit/controllers/frontend/render-channel_spec.js b/core/test/unit/controllers/frontend/render-channel_spec.js new file mode 100644 index 0000000000..3c3c56823f --- /dev/null +++ b/core/test/unit/controllers/frontend/render-channel_spec.js @@ -0,0 +1,68 @@ +/*globals describe, it, beforeEach, afterEach*/ +/*jshint expr:true*/ +var should = require('should'), + rewire = require('rewire'), + sinon = require('sinon'), + channelConfig = require('../../../../server/controllers/frontend/channel-config').get, + + // stuff being tested + labs = require('../../../../server/utils/labs'), + renderChannel = rewire('../../../../server/controllers/frontend/render-channel'), + + sandbox = sinon.sandbox.create(), + originalFetchData; + +// stop jshint complaining +should.equal(true, true); + +describe('Render Channel', function () { + beforeEach(function () { + originalFetchData = renderChannel.__get__('fetchData'); + }); + + afterEach(function () { + sandbox.restore(); + + renderChannel.__set__('fetchData', originalFetchData); + }); + + describe('internal tags labs flag', function () { + var req = { + channelConfig: channelConfig('tag'), + params: {} + }, + promise = { + then: function () { + return {catch: function () {}}; + } + }; + + it('should return normal tag config if labs flag is not set', function () { + sandbox.stub(labs, 'isSet').returns(false); + + renderChannel.__set__('fetchData', function (channelOpts) { + channelOpts.name.should.eql('tag'); + channelOpts.postOptions.filter.should.eql('tags:\'%s\''); + channelOpts.data.tag.options.should.eql({slug: '%s'}); + + return promise; + }); + + renderChannel(req); + }); + + it('should return new tag config if labs flag is set', function () { + sandbox.stub(labs, 'isSet').returns(true); + + renderChannel.__set__('fetchData', function (channelOpts) { + channelOpts.name.should.eql('tag'); + channelOpts.postOptions.filter.should.eql('tags:\'%s\'+tags.visibility:\'public\''); + channelOpts.data.tag.options.should.eql({slug: '%s', visibility: 'public'}); + + return promise; + }); + + renderChannel(req); + }); + }); +}); diff --git a/core/test/unit/server_helpers/tags_spec.js b/core/test/unit/server_helpers/tags_spec.js index bd8485af5f..ebbebfeb97 100644 --- a/core/test/unit/server_helpers/tags_spec.js +++ b/core/test/unit/server_helpers/tags_spec.js @@ -185,9 +185,10 @@ describe('{{tags}} helper', function () { }); describe('Hidden/Internal tags', function () { + // @TODO: remove these once internal tags are out of beta it('Should output internal tags when the labs flag IS NOT set', function () { sandbox.stub(labs, 'isSet').returns(false); - var tags = [{name: 'foo', slug: 'foo-bar', hidden: false}, {name: 'bar', slug: 'bar', hidden: true}], + var tags = [{name: 'foo', slug: 'foo-bar', visibility: 'public'}, {name: 'bar', slug: 'bar', visibility: 'public'}], rendered = helpers.tags.call( {tags: tags} ); @@ -199,7 +200,7 @@ describe('{{tags}} helper', function () { it('Should NOT output internal tags when the labs flag IS set', function () { sandbox.stub(labs, 'isSet').returns(true); - var tags = [{name: 'foo', slug: 'foo-bar', hidden: false}, {name: 'bar', slug: 'bar', hidden: true}], + var tags = [{name: 'foo', slug: 'foo-bar', visibility: 'public'}, {name: 'bar', slug: 'bar', visibility: 'internal'}], rendered = helpers.tags.call( {tags: tags} );