✨ Move internal tags out of labs (#7519)

closes #6165

- internal tags has been in labs for a couple of months, we've fixed some bugs & are ready to ship
- removes all code that tests for the labs flag
- also refactors the various usage of the visibility filter into a single util
- all the tests still pass!!!
- this marks #6165 as closed because I think the remaining UI tasks will be handled as part of a larger piece of work

core/server/api/configuration.js

@@ -32,7 +32,6 @@ function getBaseConfig() {
         fileStorage:    {value: (config.fileStorage !== false), type: 'bool'},
         useGravatar:    {value: !config.isPrivacyDisabled('useGravatar'), type: 'bool'},
         publicAPI:      labsFlag('publicAPI'),
-        internalTags:   labsFlag('internalTags'),
         blogUrl:        {value: config.get('url').replace(/\/$/, ''), type: 'string'},
         blogTitle:      {value: config.get('theme').title, type: 'string'},
         routeKeywords:  {value: JSON.stringify(config.get('routeKeywords')), type: 'json'}

core/server/controllers/frontend/channel-config.js

@@ -13,13 +13,13 @@ channelConfig = function channelConfig() {
             name: 'tag',
             route: '/' + config.get('routeKeywords').tag + '/:slug/',
             postOptions: {
-                filter: 'tags:\'%s\''
+                filter: 'tags:\'%s\'+tags.visibility:\'public\''
             },
             data: {
                 tag: {
                     type: 'read',
                     resource: 'tags',
-                    options: {slug: '%s'}
+                    options: {slug: '%s', visibility: 'public'}
                 }
             },
             slugTemplate: true,

core/server/controllers/frontend/render-channel.js

@@ -4,7 +4,6 @@ var debug = require('debug')('ghost:channels:render'),
     i18n        = require('../../i18n'),
     filters     = require('../../filters'),
     safeString  = require('../../utils/index').safeString,
-    labs        = require('../../utils/labs'),
     handleError        = require('./error'),
     fetchData          = require('./fetch-data'),
     formatResponse     = require('./format-response'),
@@ -25,15 +24,6 @@ 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

core/server/data/meta/keywords.js

@@ -1,16 +1,12 @@
-var labs            = require('../../utils/labs');
+var visibilityFilter = require('../../utils/visibility-filter');
 
 function getKeywords(data) {
     if (data.post && data.post.tags && data.post.tags.length > 0) {
-        return data.post.tags.reduce(function (tags, tag) {
-            if (tag.visibility !== 'internal' || !labs.isSet('internalTags')) {
-                tags.push(tag.name);
-            }
-            return tags;
-        }, []);
+        return visibilityFilter(data.post.tags, ['public'], false, function processItem(item) {
+            return item.name;
+        });
     }
     return null;
 }
 
 module.exports = getKeywords;
-

core/server/data/xml/rss/index.js

@@ -7,7 +7,6 @@ var crypto      = require('crypto'),
     i18n        = require('../../../i18n'),
     filters     = require('../../../filters'),
     processUrls = require('../../../utils/make-absolute-urls'),
-    labs        = require('../../../utils/labs'),
 
     // Really ugly temporary hack for location of things
     fetchData   = require('../../../controllers/frontend/fetch-data'),
@@ -83,7 +82,7 @@ getFeedXml = function getFeedXml(path, data) {
 generateTags = function generateTags(data) {
     if (data.tags) {
         return data.tags.reduce(function (tags, tag) {
-            if (tag.visibility !== 'internal' || !labs.isSet('internalTags')) {
+            if (tag.visibility !== 'internal') {
                 tags.push(tag.name);
             }
             return tags;

core/server/helpers/foreach.js

@@ -6,7 +6,7 @@ var hbs             = require('express-hbs'),
     _               = require('lodash'),
     logging         = require('../logging'),
     i18n            = require('../i18n'),
-    labs            = require('../utils/labs'),
+    visibilityFilter = require('../utils/visibility-filter'),
     utils           = require('./utils'),
 
     hbsUtils        = hbs.handlebars.Utils,
@@ -15,20 +15,7 @@ var hbs             = require('express-hbs'),
 function filterItemsByVisibility(items, options) {
     var visibility = utils.parseVisibility(options);
 
-    if (!labs.isSet('internalTags') || _.includes(visibility, 'all')) {
-        return items;
-    }
-
-    function visibilityFilter(item) {
-        // If the item doesn't have a visibility property && options.hash.visibility wasn't set
-        // We return the item, else we need to be sure that this item has the property
-        if (!item.visibility && !options.hash.visibility || _.includes(visibility, item.visibility)) {
-            return item;
-        }
-    }
-
-    // We don't want to change the structure of what is returned
-    return _.isArray(items) ? _.filter(items, visibilityFilter) : _.pickBy(items, visibilityFilter);
+    return visibilityFilter(items, visibility, !!options.hash.visibility);
 }
 
 foreach = function (items, options) {

core/server/helpers/tags.js

@@ -9,8 +9,8 @@
 var hbs             = require('express-hbs'),
     _               = require('lodash'),
     utils           = require('../utils'),
-    labs            = require('../utils/labs'),
     localUtils      = require('./utils'),
+    visibilityFilter = require('../utils/visibility-filter'),
     tags;
 
 tags = function (options) {
@@ -28,29 +28,14 @@ tags = function (options) {
         output     = '';
 
     function createTagList(tags) {
-        return _.reduce(tags, function (tagArray, tag) {
-            // If labs.internalTags is set && visibility is not set to all
-            // Then, if tag has a visibility property, and that visibility property is also not explicitly allowed, skip tag
-            // or if there is no visibility property, and options.hash.visibility was set, skip tag
-            if (labs.isSet('internalTags') && !_.includes(visibility, 'all')) {
-                if (
-                    (tag.visibility && !_.includes(visibility, tag.visibility) && !_.includes(visibility, 'all')) ||
-                    (!!options.hash.visibility && !_.includes(visibility, 'all') && !tag.visibility)
-                ) {
-                    // Skip this tag
-                    return tagArray;
-                }
-            }
-
-            var tagOutput = autolink ? localUtils.linkTemplate({
+        function processTag(tag) {
+            return autolink ? localUtils.linkTemplate({
                 url: utils.url.urlFor('tag', {tag: tag}),
                 text: _.escape(tag.name)
             }) : _.escape(tag.name);
+        }
 
-            tagArray.push(tagOutput);
-
-            return tagArray;
-        }, []);
+        return visibilityFilter(tags, visibility, !!options.hash.visibility, processTag);
     }
 
     if (this.tags && this.tags.length) {

core/server/models/base/index.js

@@ -16,7 +16,6 @@ var _          = require('lodash'),
     filters    = require('../../filters'),
     schema     = require('../../data/schema'),
     utils      = require('../../utils'),
-    labs       = require('../../utils/labs'),
     validation = require('../../data/validation'),
     plugins    = require('../plugins'),
     i18n       = require('../../i18n'),
@@ -520,10 +519,9 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
         }
 
         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)) {
+            if (baseName === 'tag' && /^#/.test(base)) {
                 slug = 'hash-' + slug;
             }
         }

core/server/utils/visibility-filter.js

@@ -0,0 +1,29 @@
+var _ = require('lodash');
+/**
+ *
+ * @param {Array|Object} items
+ * @param {Array} visibility
+ * @param {Boolean} [explicit]
+ * @param {Function} [fn]
+ * @returns {Array|Object} filtered items
+ */
+module.exports = 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);
+};

core/test/unit/controllers/frontend/render-channel_spec.js

@@ -5,7 +5,6 @@ var should  = require('should'),
     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(),
@@ -25,7 +24,7 @@ describe('Render Channel', function () {
         renderChannel.__set__('fetchData', originalFetchData);
     });
 
-    describe('internal tags labs flag', function () {
+    describe('Tag config', function () {
         var req = {
                 channelConfig: channelConfig('tag'),
                 params: {}
@@ -36,23 +35,7 @@ describe('Render Channel', 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);
-
+        it('should return correct tag config', function () {
             renderChannel.__set__('fetchData', function (channelOpts) {
                 channelOpts.name.should.eql('tag');
                 channelOpts.postOptions.filter.should.eql('tags:\'%s\'+tags.visibility:\'public\'');

core/test/unit/metadata/keywords_spec.js

@@ -1,6 +1,5 @@
 var getKeywords = require('../../../server/data/meta/keywords'),
     sinon   = require('sinon'),
-    labs    = require('../../../server/utils/labs'),
     should = require('should'),
     sandbox = sinon.sandbox.create();
 
@@ -21,8 +20,7 @@ describe('getKeywords', function () {
         should.deepEqual(keywords, ['one', 'two', 'three']);
     });
 
-    it('should only return visible tags if internal tags are enabled in labs', function () {
-        sandbox.stub(labs, 'isSet').returns(true);
+    it('should only return visible tags', function () {
         var keywords = getKeywords({
             post: {
                 tags: [
@@ -36,21 +34,6 @@ describe('getKeywords', function () {
         should.deepEqual(keywords, ['one', 'three']);
     });
 
-    it('should return all tags if internal tags are disabled in labs', function () {
-        sandbox.stub(labs, 'isSet').returns(false);
-        var keywords = getKeywords({
-            post: {
-                tags: [
-                    {name: 'one', visibility: 'public'},
-                    {name: 'two', visibility: 'internal'},
-                    {name: 'three'},
-                    {name: 'four', visibility: 'internal'}
-                ]
-            }
-        });
-        should.deepEqual(keywords, ['one', 'two', 'three', 'four']);
-    });
-
     it('should return null if post has tags is empty array', function () {
         var keywords = getKeywords({
             post: {

core/test/unit/rss_spec.js

@@ -4,7 +4,6 @@ var should          = require('should'),
     _               = require('lodash'),
     Promise         = require('bluebird'),
     testUtils       = require('../utils'),
-    labs            = require('../../server/utils/labs'),
 
     channelConfig   = require('../../server/controllers/frontend/channel-config'),
 
@@ -148,8 +147,7 @@ describe('RSS', function () {
             rss(req, res, failTest(done));
         });
 
-        it('should only return visible tags if internal tags are enabled in labs', function (done) {
-            sandbox.stub(labs, 'isSet').returns(true);
+        it('should only return visible tags', function (done) {
             var postWithTags = posts[2];
             postWithTags.tags = [
                 {name: 'public', visibility: 'public'},
@@ -185,43 +183,6 @@ describe('RSS', function () {
             rss(req, res, failTest(done));
         });
 
-        it('should return all tags if internal tags are not enabled in labs', function (done) {
-            sandbox.stub(labs, 'isSet').returns(false);
-            var postWithTags = posts[2];
-            postWithTags.tags = [
-                {name: 'public', visibility: 'public'},
-                {name: 'internal', visibility: 'internal'},
-                {name: 'visibility'}
-            ];
-
-            rss.__set__('getData', function () {
-                return Promise.resolve({
-                    title: 'Test Title',
-                    description: 'Testing Desc',
-                    permalinks: '/:slug/',
-                    results: {posts: [postWithTags], meta: {pagination: {pages: 1}}}
-                });
-            });
-
-            res.send = function send(xmlData) {
-                should.exist(xmlData);
-                // item tags
-                xmlData.should.match(/<title><!\[CDATA\[Short and Sweet\]\]>/);
-                xmlData.should.match(/<description><!\[CDATA\[test stuff/);
-                xmlData.should.match(/<content:encoded><!\[CDATA\[<h2 id="testing">testing<\/h2>\n\n/);
-                xmlData.should.match(/<img src="http:\/\/placekitten.com\/500\/200"/);
-                xmlData.should.match(/<media:content url="http:\/\/placekitten.com\/500\/200" medium="image"\/>/);
-                xmlData.should.match(/<category><!\[CDATA\[public\]\]/);
-                xmlData.should.match(/<category><!\[CDATA\[visibility\]\]/);
-                xmlData.should.match(/<category><!\[CDATA\[internal\]\]/);
-                done();
-            };
-
-            req.channelConfig = channelConfig.get('index');
-            req.channelConfig.isRSS = true;
-            rss(req, res, failTest(done));
-        });
-
         it('should use meta_description and image where available', function (done) {
             rss.__set__('getData', function () {
                 return Promise.resolve({
@@ -374,7 +335,7 @@ describe('RSS', function () {
             // test
             res.send = function send(xmlData) {
                 apiBrowseStub.calledOnce.should.be.true();
-                apiBrowseStub.calledWith({page: 1, filter: 'tags:\'magic\'', include: 'author,tags'}).should.be.true();
+                apiBrowseStub.calledWith({page: 1, filter: 'tags:\'magic\'+tags.visibility:\'public\'', include: 'author,tags'}).should.be.true();
                 apiTagStub.calledOnce.should.be.true();
                 xmlData.should.match(/<channel><title><!\[CDATA\[Magic - Test\]\]><\/title>/);
                 done();