Add feedback button

src/components/app.vue

@@ -45,20 +45,19 @@ export default {
   components: { Alert, Navbar, FeedbackButton: defineAsyncComponent(loadAsync('FeedbackButton')) },
   inject: ['alert', 'config'],
   setup() {
-    useSessions();
+    const { visiblyLoggedIn } = useSessions();
     useDisabled();
 
-    const { centralVersion, currentUser } = useRequestData();
+    const { centralVersion } = useRequestData();
     const { callWait } = useCallWait();
-    return { centralVersion, currentUser, callWait };
+    return { visiblyLoggedIn, centralVersion, callWait };
   },
   computed: {
     routerReady() {
       return this.$route !== START_LOCATION;
     },
-
     showsFeedbackButton() {
-      return this.config.showsFeedbackButton && this.currentUser.dataExists && this.$route.path !== '/login'
+      return this.config.showsFeedbackButton && this.visiblyLoggedIn;
     },
   },
   created() {

src/components/navbar.vue

@@ -25,7 +25,7 @@ except according to the terms contained in the LICENSE file.
           <router-link to="/" class="navbar-brand">ODK Central</router-link>
         </div>
         <div class="collapse navbar-collapse">
-          <navbar-links v-if="loggedIn"/>
+          <navbar-links v-if="visiblyLoggedIn"/>
           <div class="navbar-right">
             <a v-show="showsAnalyticsNotice" id="navbar-analytics-notice"
               href="#" @click.prevent="analyticsIntroduction.show()">
@@ -34,7 +34,7 @@ except according to the terms contained in the LICENSE file.
             <ul class="nav navbar-nav">
               <navbar-help-dropdown/>
               <navbar-locale-dropdown/>
-              <navbar-actions :logged-in="loggedIn"/>
+              <navbar-actions/>
             </ul>
           </div>
         </div>
@@ -67,7 +67,7 @@ export default {
     NavbarLinks,
     NavbarLocaleDropdown
   },
-  inject: ['config'],
+  inject: ['config', 'visiblyLoggedIn'],
   setup() {
     // The component does not assume that this data will exist when the
     // component is created.
@@ -81,16 +81,8 @@ export default {
     };
   },
   computed: {
-    // Usually once the user is logged in (either after their session has been
-    // restored or after they have submitted the login form), we render a fuller
-    // navbar. However, if after submitting the login form, the user is
-    // redirected to outside Frontend, they will remain on /login until they are
-    // redirected. In that case, we do not render the fuller navbar.
-    loggedIn() {
-      return this.currentUser.dataExists && this.$route.path !== '/login';
-    },
     showsAnalyticsNotice() {
-      return this.config.showsAnalytics && this.loggedIn &&
+      return this.config.showsAnalytics && this.visiblyLoggedIn &&
         this.canRoute('/system/analytics') && this.analyticsConfig.dataExists &&
         this.analyticsConfig.isEmpty() &&
         Date.now() - Date.parse(this.currentUser.createdAt) >= /* 14 days */ 1209600000;

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="!loggedIn" id="navbar-actions">
+  <li v-if="!visiblyLoggedIn" id="navbar-actions">
     <a href="#" @click.prevent>
       <span class="icon-user-circle-o"></span>{{ $t('notLoggedIn') }}
     </a>
@@ -44,13 +44,7 @@ import { useRequestData } from '../../request-data';
 
 export default {
   name: 'NavbarActions',
-  inject: ['container', 'alert', 'unsavedChanges'],
-  props: {
-    loggedIn: {
-      type: Boolean,
-      default: false
-    }
-  },
+  inject: ['container', 'alert', 'unsavedChanges', 'visiblyLoggedIn'],
   setup() {
     // The component does not assume that this data will exist when the
     // component is created.

src/util/session.js

@@ -65,7 +65,7 @@ hand-in-hand.
 */
 
 import { START_LOCATION } from 'vue-router';
-import { inject, onBeforeUnmount } from 'vue';
+import { computed, inject, onBeforeUnmount, provide } from 'vue';
 
 import { afterNextNavigation, forceReplace } from './router';
 import { apiPaths, isProblem, requestAlertMessage } from './request';
@@ -194,13 +194,28 @@ const logOutAfterStorageChange = (container) => (event) => {
 
 export const useSessions = () => {
   const container = inject('container');
-  const id = setInterval(logOutBeforeSessionExpires(container), 15000);
-  const handler = logOutAfterStorageChange(container);
-  window.addEventListener('storage', handler);
+  const intervalId = setInterval(logOutBeforeSessionExpires(container), 15000);
+  const storageHandler = logOutAfterStorageChange(container);
+  window.addEventListener('storage', storageHandler);
   onBeforeUnmount(() => {
-    clearInterval(id);
-    window.removeEventListener('storage', handler);
+    clearInterval(intervalId);
+    window.removeEventListener('storage', storageHandler);
   });
+
+  /* visiblyLoggedIn.value is `true` if the user not only has all the data from
+  login, but is also visibly logged in. An example of when the user has data,
+  but isn't visibly logged in is if the user has submitted the login form and is
+  being redirected to outside Frontend (which isn't instant). In that case, they
+  will remain on /login until the redirect is complete, and the navbar will not
+  change to reflect their login. */
+  const { router, requestData } = container;
+  const { currentUser } = requestData;
+  const visiblyLoggedIn = computed(() => currentUser.dataExists &&
+    router.currentRoute.value !== START_LOCATION &&
+    router.currentRoute.value.path !== '/login');
+  provide('visiblyLoggedIn', visiblyLoggedIn);
+
+  return { visiblyLoggedIn };
 };
 
 export const restoreSession = (session) =>

test/components/navbar/actions.spec.js

@@ -1,35 +1,23 @@
 import sinon from 'sinon';
 
-import Navbar from '../../../src/components/navbar.vue';
 import NavbarActions from '../../../src/components/navbar/actions.vue';
 
 import { load } from '../../util/http';
 import { mockLogin } from '../../util/session';
-import { mockRouter } from '../../util/router';
-import { mount } from '../../util/lifecycle';
 
 describe('NavbarActions', () => {
-  it('indicates if the user is not logged in', () => {
-    const navbar = mount(Navbar, {
-      container: { router: mockRouter('/login') },
-      global: {
-        // Stubbing AnalyticsIntroduction because of its custom <router-link>
-        stubs: { AnalyticsIntroduction: true }
-      }
-    });
-    const text = navbar.getComponent(NavbarActions).get('a').text();
-    text.should.equal('Not logged in');
-  });
+  it('indicates if the user is not logged in', () =>
+    load('/login')
+      .restoreSession(false)
+      .afterResponses(app => {
+        const text = app.getComponent(NavbarActions).get('a').text();
+        text.should.equal('Not logged in');
+      }));
 
   it("shows the user's display name", async () => {
     mockLogin({ displayName: 'Alice Allison' });
-    const navbar = mount(Navbar, {
-      container: { router: mockRouter('/') },
-      global: {
-        stubs: { AnalyticsIntroduction: true }
-      }
-    });
-    const a = navbar.getComponent(NavbarActions).get('a');
+    const app = await load('/');
+    const a = app.getComponent(NavbarActions).get('a');
     a.text().should.equal('Alice Allison');
     await a.get('span:nth-child(2)').should.have.textTooltip();
   });

test/components/navbar/links.spec.js

@@ -1,23 +1,19 @@
 import { RouterLinkStub } from '@vue/test-utils';
 
-import Navbar from '../../../src/components/navbar.vue';
 import NavbarLinks from '../../../src/components/navbar/links.vue';
 
+import { load } from '../../util/http';
 import { mockLogin } from '../../util/session';
 import { mockRouter } from '../../util/router';
 import { mount } from '../../util/lifecycle';
 
 describe('NavbarLinks', () => {
-  it('does not render the links before login', () => {
-    const navbar = mount(Navbar, {
-      container: { router: mockRouter('/login') },
-      global: {
-        // Stubbing AnalyticsIntroduction because of its custom <router-link>
-        stubs: { AnalyticsIntroduction: true }
-      }
-    });
-    navbar.findComponent(NavbarLinks).exists().should.be.false();
-  });
+  it('does not render the links before login', () =>
+    load('/login')
+      .restoreSession(false)
+      .afterResponses(app => {
+        app.findComponent(NavbarLinks).exists().should.be.false();
+      }));
 
   it('renders the correct links for a sitewide administrator', () => {
     mockLogin();

test/unit/session.spec.js

@@ -7,7 +7,7 @@ import createTestContainer from '../util/container';
 import testData from '../data';
 import { load, mockHttp } from '../util/http';
 import { mockLogin } from '../util/session';
-import { mockRouter } from '../util/router';
+import { mockRouter, testRouter } from '../util/router';
 import { setRequestData } from '../util/request-data';
 import { withSetup } from '../util/lifecycle';
 
@@ -872,4 +872,53 @@ describe('util/session', () => {
         });
     });
   });
+
+  describe('visiblyLoggedIn', () => {
+    it('equals true after navigation from /login', () => {
+      const user = testData.extendedUsers.createPast(1).last();
+      const container = {
+        // Prevent a request for the analytics config.
+        config: { showsAnalytics: false }
+      };
+      let correctBeforeNavigation = false;
+      return load('/login', { container })
+        .restoreSession(false)
+        .afterResponses(app => {
+          app.vm.visiblyLoggedIn.should.be.false();
+        })
+        .request(async (app) => {
+          const form = app.get('#account-login form');
+          await form.get('input[type="email"]').setValue('alice@getodk.org');
+          await form.get('input[type="password"]').setValue('foo');
+          app.vm.$router.beforeEach(() => {
+            const { currentUser } = app.vm.$container.requestData;
+            // Even though data exists from login, the user shouldn't be visibly
+            // logged in until after the navigation.
+            correctBeforeNavigation = currentUser.dataExists && !app.vm.visiblyLoggedIn;
+          });
+          return form.trigger('submit');
+        })
+        .respondWithData(() => testData.sessions.createNew())
+        .respondWithData(() => user)
+        .respondFor('/')
+        .afterResponses(app => {
+          correctBeforeNavigation.should.be.true();
+          app.vm.visiblyLoggedIn.should.be.true();
+        });
+    });
+
+    it('equals false during the initial navigation', async () => {
+      mockLogin();
+      const container = createTestContainer({ router: testRouter() });
+      const { visiblyLoggedIn } = withSetup(useSessions, { container });
+      const { router, requestData: { currentUser } } = container;
+      let correctBeforeNavigation = false;
+      router.beforeEach(() => {
+        correctBeforeNavigation = currentUser.dataExists && !visiblyLoggedIn.value;
+      });
+      await router.push('/');
+      correctBeforeNavigation.should.be.true();
+      visiblyLoggedIn.value.should.be.true();
+    });
+  });
 });