Do not render full navbar until user has left /login

As part of this, I also removed the loggedIn and loggedOut Vuex getters,
which weren't doing a lot of work and which I think would now be
slightly confusing.

src/components/account/login.vue

@@ -108,8 +108,8 @@ export default {
             (location) => {
               // We only set this.disabled to `false` before redirecting within
               // Frontend. If we also set this.disabled before redirecting
-              // outside Frontend, there might be a moment before the user is
-              // redirected when buttons are re-enabled.
+              // outside Frontend, the buttons might be re-enabled before the
+              // external page is loaded.
               this.disabled = false;
               this.$router.replace(location);
             },

src/components/navbar.vue

@@ -24,10 +24,10 @@ except according to the terms contained in the LICENSE file.
         <router-link to="/" class="navbar-brand">ODK Central</router-link>
       </div>
       <div id="navbar-collapse" class="collapse navbar-collapse">
-        <navbar-links v-if="currentUser != null"/>
+        <navbar-links v-if="loggedIn"/>
         <ul class="nav navbar-nav navbar-right">
           <navbar-locale-dropdown v-show="false"/>
-          <navbar-actions/>
+          <navbar-actions :logged-in="loggedIn"/>
         </ul>
       </div>
     </div>
@@ -43,9 +43,20 @@ import { requestData } from '../store/modules/request';
 export default {
   name: 'Navbar',
   components: { NavbarActions, NavbarLinks, NavbarLocaleDropdown },
-  // The component does not assume that this data will exist when the component
-  // is created.
-  computed: requestData(['currentUser'])
+  computed: {
+    // The component does not assume that this data will exist when the
+    // component is created.
+    ...requestData(['session']),
+    /* Usually once the user has a session (either after their session has been
+    restored or after they have logged in), we render a fuller navbar. However,
+    if after logging in, the user is redirected to a lazy-loaded route or
+    outside Frontend, they will remain on /login until they are redirected. In
+    that case, we do not render the fuller navbar until the user is
+    redirected. */
+    loggedIn() {
+      return this.session != null && this.$route.path !== '/login';
+    }
+  }
 };
 </script>
 

src/components/navbar/actions.vue

@@ -10,7 +10,7 @@ including this file, may be copied, modified, propagated, or distributed
 except according to the terms contained in the LICENSE file.
 -->
 <template>
-  <li v-if="$store.getters.loggedOut" id="navbar-actions">
+  <li v-if="!loggedIn" id="navbar-actions">
     <a href="#" @click.prevent>
       <span class="icon-user-circle-o"></span>{{ $t('notLoggedIn') }}
     </a>
@@ -46,6 +46,12 @@ import { requestData } from '../../store/modules/request';
 export default {
   name: 'NavbarActions',
   mixins: [request()],
+  props: {
+    loggedIn: {
+      type: Boolean,
+      default: false
+    }
+  },
   // The component does not assume that this data will exist when the component
   // is created.
   computed: requestData(['currentUser', 'session']),

src/components/submission/decrypt.vue

@@ -70,6 +70,8 @@ export default {
       passphrase: ''
     };
   },
+  // The component assumes that this data will exist when the component is
+  // created.
   computed: requestData(['session']),
   watch: {
     state() {

src/mixins/request.js

@@ -93,9 +93,8 @@ function request({
 
   if (this.awaitingResponse != null) this.awaitingResponse = true;
 
-  const token = this.$store.getters.loggedIn
-    ? this.$store.state.request.data.session.token
-    : null;
+  const { session } = this.$store.state.request.data;
+  const token = session != null ? session.token : null;
   const { currentRoute } = this.$store.state.router;
   return this.$http.request(configForPossibleBackendRequest(axiosConfig, token))
     .catch(error => {

src/router.js

@@ -93,13 +93,14 @@ router.beforeEach((to, from, next) => {
 // Implements the requireLogin and requireAnonymity meta fields.
 router.beforeEach((to, from, next) => {
   const { meta } = to.matched[to.matched.length - 1];
+  const { session } = store.state.request.data;
   if (meta.requireLogin) {
-    if (store.getters.loggedIn)
+    if (session != null)
       next();
     else
       next({ path: '/login', query: { next: to.fullPath } });
   } else if (meta.requireAnonymity) { // eslint-disable-line no-lonely-if
-    if (store.getters.loggedIn)
+    if (session != null)
       next('/');
     else
       next();

src/store/modules/request.js

@@ -248,7 +248,7 @@ export default {
     will react to the changes to the store state, for example, running watchers
     and updating the DOM.
     */
-    get({ state, getters, commit }, configs) {
+    get({ state, commit }, configs) {
       let alerted = false;
       return Promise.all(configs.map(config => {
         const {
@@ -310,7 +310,8 @@ export default {
         baseConfig.headers = extended
           ? { ...headers, 'X-Extended-Metadata': 'true' }
           : headers;
-        const token = getters.loggedIn ? data.session.token : null;
+        const { session } = data;
+        const token = session != null ? session.token : null;
         const axiosConfig = configForPossibleBackendRequest(baseConfig, token);
 
         const promise = Vue.prototype.$http.request(axiosConfig)

src/store/modules/request/keys.js

@@ -94,9 +94,6 @@ export const transforms = {
 // GETTERS
 
 const dataGetters = {
-  loggedIn: ({ data: { session } }) => session != null && session.token != null,
-  loggedOut: (state, getters) => !getters.loggedIn,
-
   projectRoles: ({ data: { roles } }) => {
     if (roles == null) return null;
     // If you add a new role, make sure to also add a new i18n message.

test/components/account/login.spec.js

@@ -157,24 +157,27 @@ describe('AccountLogin', () => {
         });
     });
 
-    it('does not re-enable buttons before an external redirect', () => {
+    it('does not update Frontend before an external redirect', () => {
       testData.extendedUsers.createPast(1, { email: '[email protected]' });
       return load('/login?next=%2Fenketo%2Fxyz')
         .restoreSession(false)
         .complete()
         .request(app => {
           const accountLogin = app.first(AccountLogin);
-          sinon.replace(accountLogin.vm, 'navigateToNext', sinon.fake(() => {
-            accountLogin.first('.btn-primary').should.be.disabled();
-            accountLogin.first('.btn-link').should.be.disabled();
-          }));
+          sinon.replace(accountLogin.vm, 'navigateToNext', sinon.fake());
           return submitForm(accountLogin, 'form', [
             ['input[type="email"]', '[email protected]'],
             ['input[type="password"]', 'password']
           ]);
         })
         .respondWithData(() => testData.sessions.createNew())
-        .respondWithData(() => testData.extendedUsers.first());
+        .respondWithData(() => testData.extendedUsers.first())
+        .afterResponses(app => {
+          app.find('#navbar-links').length.should.equal(0);
+          app.first('#navbar-actions a').text().trim().should.equal('Not logged in');
+          app.first('#account-login .btn-primary').should.be.disabled();
+          app.first('#account-login .btn-link').should.be.disabled();
+        });
     });
   });
 });

test/components/navbar/actions.spec.js

@@ -1,12 +1,12 @@
-import { load, mockRoute } from '../../util/http';
+import { load } from '../../util/http';
 import { mockLogin } from '../../util/session';
 import { trigger } from '../../util/event';
 
 describe('NavbarActions', () => {
-  it('indicates that the user is logged out', () =>
-    mockRoute('/login')
+  it('indicates if the user is not logged in', () =>
+    load('/login')
       .restoreSession(false)
-      .afterResponse(app => {
+      .afterResponses(app => {
         const text = app.first('#navbar-actions > a').text().trim();
         text.should.equal('Not logged in');
       }));