From 2876178dcfe281cdbad8456554161e7ef7586c45 Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Fri, 8 May 2020 17:43:40 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20logic=20error=20in=20nav?=
 =?UTF-8?q?igation=20for=20isSecondary?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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       |  4 ++-
 test/unit/helpers/navigation_spec.js      | 44 +++++++++++++++++++++++
 test/unit/helpers/test_tpl/navigation.hbs |  2 +-
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/core/frontend/helpers/navigation.js b/core/frontend/helpers/navigation.js
index 8111c6e844..deab5be387 100644
--- a/core/frontend/helpers/navigation.js
+++ b/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];
diff --git a/test/unit/helpers/navigation_spec.js b/test/unit/helpers/navigation_spec.js
index 1d41b5a37c..8c64dfb868 100644
--- a/test/unit/helpers/navigation_spec.js
+++ b/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');
+        });
+    });
 });
diff --git a/test/unit/helpers/test_tpl/navigation.hbs b/test/unit/helpers/test_tpl/navigation.hbs
index c52a74f38d..d1f0209acf 100644
--- a/test/unit/helpers/test_tpl/navigation.hbs
+++ b/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>