From 9be3b7c849a43a01d44712cd2641470c5eb2f483 Mon Sep 17 00:00:00 2001
From: Austin Burdine <acburdine@gmail.com>
Date: Fri, 13 May 2016 19:02:55 -0600
Subject: [PATCH] don't show the nav menu when on a 404 route and not signed in

no issue
- fixes problem when the nav menu would be shown on an error404 route when the user is not logged in
- adds failing test that passes with this change
---
 core/client/app/controllers/application.js    |  6 +++-
 core/client/app/templates/application.hbs     |  4 +--
 .../tests/acceptance/authentication-test.js   | 32 ++++++++++++++++++-
 core/client/tests/acceptance/signin-test.js   |  2 +-
 4 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/core/client/app/controllers/application.js b/core/client/app/controllers/application.js
index afcb626501..4a26ce9466 100644
--- a/core/client/app/controllers/application.js
+++ b/core/client/app/controllers/application.js
@@ -8,8 +8,12 @@ const {
 
 export default Controller.extend({
     dropdown: service(),
+    session: service(),
 
-    signedOut: computed.match('currentPath', /(signin|signup|setup|reset)/),
+    showNavMenu: computed('currentPath', 'session.isAuthenticated', function () {
+        return (this.get('currentPath') !== 'error404' || this.get('session.isAuthenticated')) &&
+            !this.get('currentPath').match(/(signin|signup|setup|reset)/);
+    }),
 
     topNotificationCount: 0,
     showMobileMenu: false,
diff --git a/core/client/app/templates/application.hbs b/core/client/app/templates/application.hbs
index 362798234f..eea9080b04 100644
--- a/core/client/app/templates/application.hbs
+++ b/core/client/app/templates/application.hbs
@@ -4,9 +4,9 @@
     {{gh-alerts notify="topNotificationChange"}}
 
     <div class="gh-viewport {{if autoNav 'gh-autonav'}} {{if showSettingsMenu 'settings-menu-expanded'}} {{if showMobileMenu 'mobile-menu-expanded'}}">
-        {{#unless signedOut}}
+        {{#if showNavMenu}}
             {{gh-nav-menu open=autoNavOpen toggleMaximise="toggleAutoNav" openAutoNav="openAutoNav" showMarkdownHelp="toggleMarkdownHelpModal" closeMobileMenu="closeMobileMenu"}}
-        {{/unless}}
+        {{/if}}
 
         {{#gh-main onMouseEnter="closeAutoNav" data-notification-count=topNotificationCount}}
             {{outlet}}
diff --git a/core/client/tests/acceptance/authentication-test.js b/core/client/tests/acceptance/authentication-test.js
index 592a4c3cce..f6b7ba87a6 100644
--- a/core/client/tests/acceptance/authentication-test.js
+++ b/core/client/tests/acceptance/authentication-test.js
@@ -45,7 +45,7 @@ describe('Acceptance: Authentication', function () {
         });
 
         it('invalidates session on 401 API response', function () {
-            // return a 401 when attempting to retrieve tags
+            // return a 401 when attempting to retrieve users
             server.get('/users/', (db, request) => {
                 return new Mirage.Response(401, {}, {
                     errors: [
@@ -61,6 +61,36 @@ describe('Acceptance: Authentication', function () {
                 expect(currentURL(), 'url after 401').to.equal('/signin');
             });
         });
+
+        it('doesn\'t show navigation menu on invalid url when not authenticated', function () {
+            invalidateSession(application);
+
+            visit('/');
+
+            andThen(() => {
+                expect(currentURL(), 'current url').to.equal('/signin');
+                expect(find('nav.gh-nav').length, 'nav menu presence').to.equal(0);
+            });
+
+            visit('/signin/invalidurl/');
+
+            andThen(() => {
+                expect(currentURL(), 'url after invalid url').to.equal('/signin/invalidurl/');
+                expect(currentPath(), 'path after invalid url').to.equal('error404');
+                expect(find('nav.gh-nav').length, 'nav menu presence').to.equal(0);
+            });
+        });
+
+        it('shows nav menu on invalid url when authenticated', function () {
+            authenticateSession(application);
+            visit('/signin/invalidurl/');
+
+            andThen(() => {
+                expect(currentURL(), 'url after invalid url').to.equal('/signin/invalidurl/');
+                expect(currentPath(), 'path after invalid url').to.equal('error404');
+                expect(find('nav.gh-nav').length, 'nav menu presence').to.equal(1);
+            });
+        });
     });
 
     describe('editor', function () {
diff --git a/core/client/tests/acceptance/signin-test.js b/core/client/tests/acceptance/signin-test.js
index 88324a2f2e..511f90a752 100644
--- a/core/client/tests/acceptance/signin-test.js
+++ b/core/client/tests/acceptance/signin-test.js
@@ -34,7 +34,7 @@ describe('Acceptance: Signin', function() {
         });
     });
 
-    describe('when attempting to sigin', function () {
+    describe('when attempting to signin', function () {
         beforeEach(function () {
             let role = server.create('role', {name: 'Administrator'});
             let user = server.create('user', {roles: [role], slug: 'test-user'});