Remove loading based on conditional feature flag

Since the feature flag isn't available until the profile api request
completes, and the profile api request is run asynchronously with the
group api requests, it doesn't make sense to load conditionaly. Instead,
call all the group api requests and filter the groups for the old groups
menu later. In this case, filtering is done in the groups.all
conditionally based on the feature flag.

src/sidebar/services/groups.js

@@ -77,7 +77,9 @@ function groups(
 
     // If the main document URL has no groups associated with it, always show
     // the "Public" group.
-    const pageHasAssociatedGroups = groups.some(g => g.id !== '__world__');
+    const pageHasAssociatedGroups = groups.some(
+      g => g.id !== '__world__' && g.isScopedToUri
+    );
     if (!pageHasAssociatedGroups) {
       return Promise.resolve(groups);
     }
@@ -144,7 +146,7 @@ function groups(
     return uri
       .then(uri => {
         const params = {
-          expand: 'organization',
+          expand: ['organization', 'scopes'],
         };
         if (authority) {
           params.authority = authority;
@@ -154,13 +156,11 @@ function groups(
         }
         documentUri = uri;
 
-        if (features.flagEnabled('community_groups')) {
-          params.expand = ['organization', 'scopes'];
-          const profileParams = {
-            expand: ['organization', 'scopes'],
-          };
-          const profileGroupsApi = api.profile.groups.read(profileParams);
+        const profileGroupsApi = api.profile.groups.read({
+          expand: params.expand,
+        });
         const listGroupsApi = api.groups.list(params);
+
         return Promise.all([
           profileGroupsApi,
           listGroupsApi,
@@ -169,12 +169,6 @@ function groups(
           combineGroups(myGroups, featuredGroups, documentUri),
           token,
         ]);
-        } else {
-          // Fetch groups from the API.
-          return api.groups
-            .list(params, null, { includeMetadata: true })
-            .then(({ data, token }) => [data, token]);
-        }
       })
       .then(([groups, token]) => {
         const isLoggedIn = token !== null;
@@ -197,8 +191,11 @@ function groups(
   }
 
   function all() {
+    if (features.flagEnabled('community_groups')) {
       return store.allGroups();
     }
+    return store.getInScopeGroups();
+  }
 
   // Return the full object for the group with the given id.
   function get(id) {

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

@@ -70,6 +70,9 @@ describe('groups', function() {
         allGroups() {
           return this.getState().groups;
         },
+        getInScopeGroups() {
+          return this.getState().groups;
+        },
         focusedGroup() {
           return this.getState().focusedGroup;
         },
@@ -114,12 +117,7 @@ describe('groups', function() {
         },
       },
       groups: {
-        list: sandbox.stub().returns(
-          Promise.resolve({
-            data: dummyGroups,
-            token: '1234',
-          })
-        ),
+        list: sandbox.stub().returns(dummyGroups),
       },
       profile: {
         groups: {
@@ -151,15 +149,24 @@ describe('groups', function() {
   }
 
   describe('#all', function() {
-    it('returns all groups', function() {
+    it('returns all groups from store.allGroups when community-groups feature flag is enabled', () => {
+      const svc = service();
+      fakeStore.allGroups = sinon.stub().returns(dummyGroups);
+      fakeFeatures.flagEnabled.withArgs('community_groups').returns(true);
+      assert.deepEqual(svc.all(), dummyGroups);
+      assert.called(fakeStore.allGroups);
+    });
+
+    it('returns all groups from store.getInScopeGroups when community-groups feature flag is disabled', () => {
       const svc = service();
-      fakeStore.setState({ groups: dummyGroups });
+      fakeStore.getInScopeGroups = sinon.stub().returns(dummyGroups);
       assert.deepEqual(svc.all(), dummyGroups);
+      assert.called(fakeStore.getInScopeGroups);
     });
   });
 
   describe('#load', function() {
-    it('combines groups from both endpoints if community-groups feature flag is set', function() {
+    it('combines groups from both endpoints', function() {
       const svc = service();
 
       const groups = [
@@ -169,7 +176,6 @@ describe('groups', function() {
 
       fakeApi.profile.groups.read.returns(Promise.resolve(groups));
       fakeApi.groups.list.returns(Promise.resolve([groups[0]]));
-      fakeFeatures.flagEnabled.withArgs('community_groups').returns(true);
 
       return svc.load().then(() => {
         assert.calledWith(fakeStore.loadGroups, groups);
@@ -184,21 +190,11 @@ describe('groups', function() {
       });
     });
 
-    it('always sends the `expand` parameter', function() {
-      const svc = service();
-      return svc.load().then(() => {
-        const call = fakeApi.groups.list.getCall(0);
-        assert.isObject(call.args[0]);
-        assert.equal(call.args[0].expand, 'organization');
-      });
-    });
-
-    it('sends `expand` parameter when community-groups feature flag is enabled', function() {
+    it('sends `expand` parameter', function() {
       const svc = service();
       fakeApi.groups.list.returns(
         Promise.resolve([{ id: 'groupa', name: 'GroupA' }])
       );
-      fakeFeatures.flagEnabled.withArgs('community_groups').returns(true);
 
       return svc.load().then(() => {
         assert.calledWith(
@@ -241,7 +237,7 @@ describe('groups', function() {
         return loaded.then(() => {
           assert.calledWith(fakeApi.groups.list, {
             document_uri: 'https://asite.com',
-            expand: 'organization',
+            expand: ['organization', 'scopes'],
           });
         });
       });
@@ -257,7 +253,7 @@ describe('groups', function() {
         const svc = service();
         return svc.load().then(() => {
           assert.calledWith(fakeApi.groups.list, {
-            expand: 'organization',
+            expand: ['organization', 'scopes'],
           });
         });
       });
@@ -277,12 +273,7 @@ describe('groups', function() {
     it('injects a defalt organization if group is missing an organization', function() {
       const svc = service();
       const groups = [{ id: '39r39f', name: 'Ding Dong!' }];
-      fakeApi.groups.list.returns(
-        Promise.resolve({
-          token: '1234',
-          data: groups,
-        })
-      );
+      fakeApi.groups.list.returns(Promise.resolve(groups));
       return svc.load().then(groups => {
         assert.isObject(groups[0].organization);
         assert.hasAllKeys(groups[0].organization, ['id', 'logo']);
@@ -317,12 +308,9 @@ describe('groups', function() {
             groups.push({ name: 'BioPub', id: 'biopub' });
           }
 
-          fakeApi.groups.list.returns(
-            Promise.resolve({
-              token: loggedIn ? '1234' : null,
-              data: groups,
-            })
-          );
+          fakeAuth.tokenGetter.returns(loggedIn ? '1234' : null);
+          fakeApi.groups.list.returns(Promise.resolve(groups));
+          fakeApi.profile.groups.read.returns(Promise.resolve([]));
 
           return svc.load().then(groups => {
             const publicGroupShown = groups.some(g => g.id === '__world__');
@@ -375,12 +363,8 @@ describe('groups', function() {
           { name: 'DEF', id: 'def456', groupid: null },
         ];
 
-        fakeApi.groups.list.returns(
-          Promise.resolve({
-            token: '1234',
-            data: groups,
-          })
-        );
+        fakeApi.groups.list.returns(Promise.resolve(groups));
+        fakeApi.profile.groups.read.returns(Promise.resolve([]));
 
         return svc.load().then(groups => {
           let displayedGroups = groups.map(g => g.id);