Remove an unnecessary `/api/groups` call on page load

On startup an initial `FRAME_CONNECTED` event is fired when the main
frame connects to the sidebar. This triggered an extra `/api/groups`
fetch. Avoid this by recording the main frame URI used in the last
groups fetch in  `load()`. The existing test in the `FRAME_CONNECTED`
handler then skips the `load()` call if this URI has not changed.

src/sidebar/services/groups.js

@@ -20,6 +20,9 @@ var serviceConfig = require('../service-config');
 // @ngInject
 function groups($rootScope, store, api, isSidebar, localStorage, serviceUrl, session,
                 settings) {
+
+  // The document URI passed to the most recent `GET /api/groups` call in order
+  // to include groups associated with this page.
   var documentUri;
 
   var svc = serviceConfig(settings);
@@ -61,6 +64,7 @@ function groups($rootScope, store, api, isSidebar, localStorage, serviceUrl, ses
       if (uri) {
         params.document_uri = uri;
       }
+      documentUri = uri;
       return api.groups.list(params);
     }).then(groups => {
       var isFirstLoad = store.allGroups().length === 0;
@@ -141,8 +145,6 @@ function groups($rootScope, store, api, isSidebar, localStorage, serviceUrl, ses
 
   // refetch the list of groups when document url changes
   $rootScope.$on(events.FRAME_CONNECTED, () => {
-    // FIXME Makes a third api call on page load. better way?
-    // return for use in test
     return getDocumentUriForGroupSearch().then(uri => {
       if (documentUri !== uri) {
         documentUri = uri;

src/sidebar/services/test/groups-test.js

@@ -253,7 +253,6 @@ describe('groups', function() {
     var changeEvents = [
       {event: events.GROUPS_CHANGED},
       {event: events.USER_CHANGED},
-      {event: events.FRAME_CONNECTED},
     ];
 
     unroll('should fetch the list of groups from the server when #event occurs', function (testCase) {
@@ -263,5 +262,34 @@ describe('groups', function() {
         assert.calledOnce(fakeApi.groups.list);
       });
     }, changeEvents);
+
+    context('when a new frame connects', () => {
+      it('should refetch groups if main frame URL has changed', () => {
+        var svc = service();
+
+        fakeStore.setState({ searchUris: ['https://domain.com/page-a'] });
+        return svc.load().then(() => {
+          // Simulate main frame URL change, eg. due to client-side navigation in
+          // a single page application.
+          fakeApi.groups.list.resetHistory();
+          fakeStore.setState({searchUris: ['https://domain.com/page-b']});
+
+          return fakeRootScope.eventCallbacks[events.FRAME_CONNECTED]();
+        }).then(() => {
+          assert.calledOnce(fakeApi.groups.list);
+        });
+      });
+
+      it('should not refetch groups if main frame URL has not changed', () => {
+        var svc = service();
+
+        fakeStore.setState({ searchUris: ['https://domain.com/page-a'] });
+        return svc.load().then(() => {
+          return fakeRootScope.eventCallbacks[events.FRAME_CONNECTED]();
+        }).then(() => {
+          assert.calledOnce(fakeApi.groups.list);
+        });
+      });
+    });
   });
 });