🐛 Fixed logic error in navigation for isSecondary

closes #11772

- Ensures that isSecondary is a boolean true or false
- Added tests that cover the bug, switching to using compile because the helpers have to be run together
- TODO: all tests for helpers should be switched to compile, it's SO MUCH easier

core/frontend/helpers/navigation.js

@@ -13,7 +13,9 @@ module.exports = function navigation(options) {
     options.data = options.data || {};
 
     const key = options.hash.type && options.hash.type === 'secondary' ? 'secondary_navigation' : 'navigation';
-    options.hash.isSecondary = options.hash.type && options.hash.type === 'secondary';
+    // Set isSecondary so we can compare in the template
+    options.hash.isSecondary = !!(options.hash.type && options.hash.type === 'secondary');
+    // Remove type, so it's not accessible
     delete options.hash.type;
 
     const navigationData = options.data.site[key];

test/unit/helpers/navigation_spec.js

@@ -3,6 +3,7 @@ const hbs = require('../../../core/frontend/services/themes/engine');
 const configUtils = require('../../utils/configUtils');
 const path = require('path');
 const helpers = require('../../../core/frontend/helpers');
+const handlebars = require('../../../core/frontend/services/themes/engine').handlebars;
 
 const runHelper = data => helpers.navigation.call({}, data);
 const runHelperThunk = data => () => runHelper(data);
@@ -290,4 +291,47 @@ describe('{{navigation}} helper with custom template', function () {
         rendered.string.should.containEql(testUrl);
         rendered.string.should.containEql('Fighters');
     });
+
+    describe('using compile', function () {
+        let defaultGlobals;
+        function compile(templateString) {
+            const template = handlebars.compile(templateString);
+            template.with = (locals = {}, globals) => {
+                globals = globals || defaultGlobals;
+
+                return template(locals, globals);
+            };
+
+            return template;
+        }
+
+        before(function () {
+            handlebars.registerHelper('link_class', helpers.link_class);
+            handlebars.registerHelper('concat', helpers.concat);
+            handlebars.registerHelper('url', helpers.concat);
+            handlebars.registerHelper('navigation', helpers.navigation);
+            configUtils.config.set('url', 'https://siteurl.com');
+            defaultGlobals = {
+                data: {
+                    site: {
+                        url: configUtils.config.get('url'),
+                        navigation: [{label: 'Foo', url: '/foo'}],
+                        secondary_navigation: [{label: 'Fighters', url: '/foo'}]
+                    }
+                }
+            };
+        });
+
+        it('can render both primary and secondary nav in order', function () {
+            compile('{{navigation}}{{navigation type="secondary"}}')
+                .with({})
+                .should.eql('\n\n\n\nPrime time!\n\n    <a href="">Foo</a>\n\n\n\n\nJeremy Bearimy baby!\n\n    <a href="">Fighters</a>\n');
+        });
+
+        it('can render both primary and secondary nav in reverse order', function () {
+            compile('{{navigation type="secondary"}}{{navigation}}')
+                .with({})
+                .should.eql('\n\n\n\nJeremy Bearimy baby!\n\n    <a href="">Fighters</a>\n\n\n\n\nPrime time!\n\n    <a href="">Foo</a>\n');
+        });
+    });
 });

test/unit/helpers/test_tpl/navigation.hbs

@@ -2,7 +2,7 @@
 
 {{#if isHeader}}isHeader is set{{/if}}
 
-{{#if isSecondary}}Jeremy Bearimy baby!{{/if}}
+{{#if isSecondary}}Jeremy Bearimy baby!{{else}}Prime time!{{/if}}
 
 {{#foreach navigation}}
     <a href="{{url absolute="true"}}">{{label}}</a>