Make the {{navigation}} helper global

refs #4535

- Rather than storing navigation data as a top level key, store it as @blog.navigation
- Reference the global data from the helper

core/server/api/settings.js

@@ -40,7 +40,8 @@ updateConfigTheme = function () {
             title: (settingsCache.title && settingsCache.title.value) || '',
             description: (settingsCache.description && settingsCache.description.value) || '',
             logo: (settingsCache.logo && settingsCache.logo.value) || '',
-            cover: (settingsCache.cover && settingsCache.cover.value) || ''
+            cover: (settingsCache.cover && settingsCache.cover.value) || '',
+            navigation: (settingsCache.navigation && JSON.parse(settingsCache.navigation.value)) || []
         }
     });
 };

core/server/config/index.js

@@ -118,6 +118,11 @@ ConfigManager.prototype.set = function (config) {
     // local copy with properties that have been explicitly set.
     _.merge(this._config, config);
 
+    // Special case for the them.navigation JSON object, which should be overridden not merged
+    if (config && config.theme && config.theme.navigation) {
+        this._config.theme.navigation = config.theme.navigation;
+    }
+
     // Protect against accessing a non-existant object.
     // This ensures there's always at least a paths object
     // because it's referenced in multiple places.

core/server/controllers/frontend.js

@@ -37,25 +37,10 @@ function getPostPage(options) {
     });
 }
 
-/**
- * returns a promise with an array of values used in {{navigation}}
- * TODO(nsfmc): should this be in the 'prePostsRender' pipeline?
- * @return {Promise} containing an array of navigation items
- */
-function getSiteNavigation() {
-    return Promise.resolve(api.settings.read('navigation')).then(function (result) {
-        if (result && result.settings && result.settings.length) {
-            return JSON.parse(result.settings[0].value) || [];
-        }
-        return [];
-    });
-}
-
 /**
  * formats variables for handlebars in multi-post contexts.
  * If extraValues are available, they are merged in the final value
- * TODO(nsfmc): should this be in the 'prePostsRender' pipeline?
+ * @return {Object} containing page variables
- * @return {Promise} containing page variables
  */
 function formatPageResponse(posts, page, extraValues) {
     // Delete email from author for frontend output
@@ -68,20 +53,16 @@ function formatPageResponse(posts, page, extraValues) {
     });
     extraValues = extraValues || {};
 
-    return getSiteNavigation().then(function (navigation) {
+    var resp = {
-        var resp = {
+        posts: posts,
-            posts: posts,
+        pagination: page.meta.pagination
-            pagination: page.meta.pagination,
+    };
-            navigation: navigation || {}
+    return _.extend(resp, extraValues);
-        };
-        return _.extend(resp, extraValues);
-    });
 }
 
 /**
  * similar to formatPageResponse, but for single post pages
- * TODO(nsfmc): should this be in the 'prePostsRender' pipeline?
+ * @return {Object} containing page variables
- * @return {Promise} containing page variables
  */
 function formatResponse(post) {
     // Delete email from author for frontend output
@@ -90,12 +71,9 @@ function formatResponse(post) {
         delete post.author.email;
     }
 
-    return getSiteNavigation().then(function (navigation) {
+    return {
-        return {
+        post: post
-            post: post,
+    };
-            navigation: navigation
-        };
-    });
 }
 
 function handleError(next) {
@@ -192,9 +170,7 @@ frontendControllers = {
                     }
 
                     setResponseContext(req, res);
-                    formatPageResponse(posts, page).then(function (result) {
+                    res.render(view, formatPageResponse(posts, page));
-                        res.render(view, result);
-                    });
                 });
             });
         }).catch(handleError(next));
@@ -237,19 +213,18 @@ frontendControllers = {
             // Render the page of posts
             filters.doFilter('prePostsRender', page.posts).then(function (posts) {
                 getActiveThemePaths().then(function (paths) {
-                    var view = template.getThemeViewForTag(paths, options.tag);
+                    var view = template.getThemeViewForTag(paths, options.tag),
+                    // Format data for template
+                        result = formatPageResponse(posts, page, {
+                            tag: page.meta.filters.tags ? page.meta.filters.tags[0] : ''
+                        });
 
-                        // Format data for template
+                    // If the resulting tag is '' then 404.
-                    formatPageResponse(posts, page, {
+                    if (!result.tag) {
-                        tag: page.meta.filters.tags ? page.meta.filters.tags[0] : ''
+                        return next();
-                    }).then(function (result) {
+                    }
-                        // If the resulting tag is '' then 404.
+                    setResponseContext(req, res);
-                        if (!result.tag) {
+                    res.render(view, result);
-                            return next();
-                        }
-                        setResponseContext(req, res);
-                        res.render(view, result);
-                    });
                 });
             });
         }).catch(handleError(next));
@@ -292,20 +267,19 @@ frontendControllers = {
             // Render the page of posts
             filters.doFilter('prePostsRender', page.posts).then(function (posts) {
                 getActiveThemePaths().then(function (paths) {
-                    var view = paths.hasOwnProperty('author.hbs') ? 'author' : 'index';
+                    var view = paths.hasOwnProperty('author.hbs') ? 'author' : 'index',
-
                         // Format data for template
-                    formatPageResponse(posts, page, {
+                        result = formatPageResponse(posts, page, {
-                        author: page.meta.filters.author ? page.meta.filters.author : ''
+                            author: page.meta.filters.author ? page.meta.filters.author : ''
-                    }).then(function (result) {
+                        });
-                        // If the resulting author is '' then 404.
-                        if (!result.author) {
-                            return next();
-                        }
 
-                        setResponseContext(req, res);
+                    // If the resulting author is '' then 404.
-                        res.render(view, result);
+                    if (!result.author) {
-                    });
+                        return next();
+                    }
+
+                    setResponseContext(req, res);
+                    res.render(view, result);
                 });
             });
         }).catch(handleError(next));
@@ -378,13 +352,12 @@ frontendControllers = {
 
                 filters.doFilter('prePostsRender', post).then(function (post) {
                     getActiveThemePaths().then(function (paths) {
-                        var view = template.getThemeViewForPost(paths, post);
+                        var view = template.getThemeViewForPost(paths, post),
+                            response = formatResponse(post);
 
-                        return formatResponse(post).then(function (response) {
+                        setResponseContext(req, res, response);
-                            setResponseContext(req, res, response);
 
-                            res.render(view, response);
+                        res.render(view, response);
-                        });
                     });
                 });
             }

core/server/helpers/navigation.js

@@ -4,28 +4,30 @@
 
 var _               = require('lodash'),
     hbs             = require('express-hbs'),
+
     errors          = require('../errors'),
     template        = require('./template'),
     navigation;
 
 navigation = function (options) {
     /*jshint unused:false*/
-    var navigation,
+    var navigationData = options.data.blog.navigation,
-        context,
+        currentUrl = options.data.root.relativeUrl,
-        currentUrl = this.relativeUrl;
+        output,
+        context;
 
-    if (!_.isObject(this.navigation) || _.isFunction(this.navigation)) {
+    if (!_.isObject(navigationData) || _.isFunction(navigationData)) {
         return errors.logAndThrowError('navigation data is not an object or is a function');
     }
 
-    if (this.navigation.filter(function (e) {
+    if (navigationData.filter(function (e) {
         return (_.isUndefined(e.label) || _.isUndefined(e.url));
     }).length > 0) {
         return errors.logAndThrowError('All values must be defined for label, url and current');
     }
 
     // check for non-null string values
-    if (this.navigation.filter(function (e) {
+    if (navigationData.filter(function (e) {
         return ((!_.isNull(e.label) && !_.isString(e.label)) ||
             (!_.isNull(e.url) && !_.isString(e.url)));
     }).length > 0) {
@@ -37,11 +39,11 @@ navigation = function (options) {
     }
 
     // {{navigation}} should no-op if no data passed in
-    if (this.navigation.length === 0) {
+    if (navigationData.length === 0) {
         return new hbs.SafeString('');
     }
 
-    navigation = this.navigation.map(function (e) {
+    output = navigationData.map(function (e) {
         var out = {};
         out.current = e.url === currentUrl;
         out.label = e.label;
@@ -50,7 +52,7 @@ navigation = function (options) {
         return out;
     });
 
-    context = _.merge({}, {navigation: navigation});
+    context = _.merge({}, {navigation: output});
 
     return template.execute('navigation', context);
 };

core/test/unit/server_helpers/navigation_spec.js

@@ -1,4 +1,4 @@
-/*globals describe, before, it*/
+/*globals describe, before, beforeEach, it*/
 /*jshint expr:true*/
 var should         = require('should'),
     hbs            = require('express-hbs'),
@@ -9,6 +9,13 @@ var should         = require('should'),
     helpers        = require('../../../server/helpers');
 
 describe('{{navigation}} helper', function () {
+    var runHelper = function (data) {
+            return function () {
+                helpers.navigation(data);
+            };
+        },
+        optionsData;
+
     before(function (done) {
         utils.loadHelpers();
         hbs.express3({partialsDir: [utils.config.paths.helperTemplates]});
@@ -17,27 +24,43 @@ describe('{{navigation}} helper', function () {
         });
     });
 
+    beforeEach(function () {
+        optionsData = {
+            data: {
+                blog: {
+                    navigation: []
+                },
+                root: {
+                    relativeUrl: ''
+                }
+            }
+        };
+    });
+
     it('has loaded navigation helper', function () {
         should.exist(handlebars.helpers.navigation);
     });
 
     it('should throw errors on invalid data', function () {
-        var runHelper = function (data) {
+        // Test 1: navigation = string
-            return function () {
+        optionsData.data.blog.navigation = 'not an object';
-                helpers.navigation.call(data);
+        runHelper(optionsData).should.throwError('navigation data is not an object or is a function');
-            };
-        };
 
-        runHelper('not an object').should.throwError('navigation data is not an object or is a function');
+        // Test 2: navigation = function
-        runHelper(function () {}).should.throwError('navigation data is not an object or is a function');
+        optionsData.data.blog.navigation = function () {};
+        runHelper(optionsData).should.throwError('navigation data is not an object or is a function');
 
-        runHelper({navigation: [{label: 1, url: 'bar'}]}).should.throwError('Invalid value, Url and Label must be strings');
+        // Test 3: invalid label
-        runHelper({navigation: [{label: 'foo', url: 1}]}).should.throwError('Invalid value, Url and Label must be strings');
+        optionsData.data.blog.navigation = [{label: 1, url: 'bar'}];
+        runHelper(optionsData).should.throwError('Invalid value, Url and Label must be strings');
+
+        // Test 4: invalid url
+        optionsData.data.blog.navigation = [{label: 'foo', url: 1}];
+        runHelper(optionsData).should.throwError('Invalid value, Url and Label must be strings');
     });
 
     it('can render empty nav', function () {
-        var navigation = {navigation:[]},
+        var rendered = helpers.navigation(optionsData);
-            rendered = helpers.navigation.call(navigation);
 
         should.exist(rendered);
         rendered.string.should.be.equal('');
@@ -45,9 +68,11 @@ describe('{{navigation}} helper', function () {
 
     it('can render one item', function () {
         var singleItem = {label: 'Foo', url: '/foo'},
-            navigation = {navigation: [singleItem]},
+            testUrl = 'href="' + utils.config.url + '/foo"',
-            rendered = helpers.navigation.call(navigation),
+            rendered;
-            testUrl = 'href="' + utils.config.url + '/foo"';
+
+        optionsData.data.blog.navigation = [singleItem];
+        rendered = helpers.navigation(optionsData);
 
         should.exist(rendered);
         rendered.string.should.containEql('li');
@@ -58,10 +83,12 @@ describe('{{navigation}} helper', function () {
     it('can render multiple items', function () {
         var firstItem = {label: 'Foo', url: '/foo'},
             secondItem = {label: 'Bar Baz Qux', url: '/qux'},
-            navigation = {navigation: [firstItem, secondItem]},
-            rendered = helpers.navigation.call(navigation),
             testUrl = 'href="' + utils.config.url + '/foo"',
-            testUrl2 = 'href="' + utils.config.url + '/qux"';
+            testUrl2 = 'href="' + utils.config.url + '/qux"',
+            rendered;
+
+        optionsData.data.blog.navigation = [firstItem, secondItem];
+        rendered = helpers.navigation(optionsData);
 
         should.exist(rendered);
         rendered.string.should.containEql('nav-foo');
@@ -73,11 +100,11 @@ describe('{{navigation}} helper', function () {
     it('can annotate the current url', function () {
         var firstItem = {label: 'Foo', url: '/foo'},
             secondItem = {label: 'Bar', url: '/qux'},
-            navigation = {
+            rendered;
-                relativeUrl: '/foo',
+
-                navigation: [firstItem, secondItem]
+        optionsData.data.blog.navigation = [firstItem, secondItem];
-            },
+        optionsData.data.root.relativeUrl = '/foo';
-            rendered = helpers.navigation.call(navigation);
+        rendered = helpers.navigation(optionsData);
 
         should.exist(rendered);
         rendered.string.should.containEql('nav-foo');