Combine groups from multiple endpoints when feature flag is set (#901)

* Add profile.groups.read api endpoint

* Remove metadata from list groups api call

Rather than relying on the token of the logged in user being returned
as extra metadata from the list groups api call, simple use the profile
userid from the data store to determine whether a use is logged in or not.

* Add util/groups module with combineGroups helper

This will be used when the community-groups feature flag is set, to combine
groups from the groups list and profile groups endpoints.

* Add tests for util groups module

* Add tests for community-groups feature flag

src/sidebar/services/api.js

@@ -225,7 +225,9 @@ function api($http, $q, apiRoutes, auth) {
       list: apiCall('groups.read'),
     },
     profile: {
-      groups: apiCall('profile.groups'),
+      groups: {
+        read: apiCall('profile.groups.read'),
+      },
       read: apiCall('profile.read'),
       update: apiCall('profile.update'),
     },

src/sidebar/services/groups.js

@@ -14,6 +14,7 @@ const DEFAULT_ORGANIZATION = {
 
 const events = require('../events');
 const { awaitStateChange } = require('../util/state-util');
+const { combineGroups } = require('../util/groups');
 const serviceConfig = require('../service-config');
 
 // @ngInject
@@ -25,7 +26,8 @@ function groups(
   localStorage,
   serviceUrl,
   session,
-  settings
+  settings,
+  features
 ) {
   const svc = serviceConfig(settings);
   const authority = svc ? svc.authority : null;
@@ -150,13 +152,25 @@ function groups(
         }
         documentUri = uri;
 
-        // Fetch groups from the API.
-        return api.groups.list(params, null, { includeMetadata: true });
+        if (features.flagEnabled('community_groups')) {
+          const profileParams = {
+            expand: ['organization'],
+          };
+          const profileGroupsApi = api.profile.groups.read(profileParams);
+          const listGroupsApi = api.groups.list(params);
+          return Promise.all([profileGroupsApi, listGroupsApi]).then(
+            ([myGroups, featuredGroups]) =>
+              combineGroups(myGroups, featuredGroups)
+          );
+        } else {
+          // Fetch groups from the API.
+          return api.groups.list(params);
+        }
       })
-      .then(({ data, token }) => {
-        const isLoggedIn = token !== null;
+      .then(groups => {
+        const isLoggedIn = store.profile().userid !== null;
         const directLinkedAnnotation = settings.annotations;
-        return filterGroups(data, isLoggedIn, directLinkedAnnotation);
+        return filterGroups(groups, isLoggedIn, directLinkedAnnotation);
       })
       .then(groups => {
         injectOrganizations(groups);

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

@@ -36,6 +36,7 @@ const dummyGroups = [
 ];
 
 describe('groups', function() {
+  let fakeFeatures;
   let fakeStore;
   let fakeIsSidebar;
   let fakeSession;
@@ -47,6 +48,9 @@ describe('groups', function() {
   let sandbox;
 
   beforeEach(function() {
+    fakeFeatures = {
+      flagEnabled: sinon.stub().returns(false),
+    };
     sandbox = sinon.sandbox.create();
 
     fakeStore = fakeReduxStore(
@@ -54,6 +58,7 @@ describe('groups', function() {
         searchUris: ['http://example.org'],
         focusedGroup: null,
         groups: [],
+        profile: sinon.stub().returns({ userid: 'userid' }),
       },
       {
         focusGroup: sinon.stub(),
@@ -74,6 +79,9 @@ describe('groups', function() {
         },
       }
     );
+    fakeStore.profile = () => {
+      return { userid: 'userid' };
+    };
     fakeSession = sessionWithThreeGroups();
     fakeIsSidebar = true;
     fakeLocalStorage = {
@@ -106,12 +114,12 @@ describe('groups', function() {
         },
       },
       groups: {
-        list: sandbox.stub().returns(
-          Promise.resolve({
-            data: dummyGroups,
-            token: '1234',
-          })
-        ),
+        list: sandbox.stub().returns(Promise.resolve(dummyGroups)),
+      },
+      profile: {
+        groups: {
+          read: sandbox.stub().returns(Promise.resolve([dummyGroups[0]])),
+        },
       },
     };
     fakeServiceUrl = sandbox.stub();
@@ -131,7 +139,8 @@ describe('groups', function() {
       fakeLocalStorage,
       fakeServiceUrl,
       fakeSession,
-      fakeSettings
+      fakeSettings,
+      fakeFeatures
     );
   }
 
@@ -144,6 +153,23 @@ describe('groups', function() {
   });
 
   describe('#load', function() {
+    it('combines groups from both endpoints if community-groups feature flag is set', function() {
+      const svc = service();
+
+      const groups = [
+        { id: 'groupa', name: 'GroupA' },
+        { id: 'groupb', name: 'GroupB' },
+      ];
+
+      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);
+      });
+    });
+
     it('loads all available groups', function() {
       const svc = service();
 
@@ -161,6 +187,22 @@ describe('groups', function() {
       });
     });
 
+    it('sends `expand` parameter when community-groups feature flag is enabled', function() {
+      const svc = service();
+      fakeFeatures.flagEnabled.withArgs('community_groups').returns(true);
+
+      return svc.load().then(() => {
+        assert.calledWith(
+          fakeApi.profile.groups.read,
+          sinon.match({ expand: ['organization'] })
+        );
+        assert.calledWith(
+          fakeApi.groups.list,
+          sinon.match({ expand: 'organization' })
+        );
+      });
+    });
+
     it('sets the focused group from the value saved in local storage', () => {
       const svc = service();
       fakeLocalStorage.getItem.returns(dummyGroups[1].id);
@@ -226,12 +268,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']);
@@ -266,12 +303,10 @@ describe('groups', function() {
             groups.push({ name: 'BioPub', id: 'biopub' });
           }
 
-          fakeApi.groups.list.returns(
-            Promise.resolve({
-              token: loggedIn ? '1234' : null,
-              data: groups,
-            })
-          );
+          fakeStore.profile = () => {
+            return { userid: loggedIn ? 'userid' : null };
+          };
+          fakeApi.groups.list.returns(Promise.resolve(groups));
 
           return svc.load().then(groups => {
             const publicGroupShown = groups.some(g => g.id === '__world__');
@@ -324,12 +359,7 @@ 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));
 
         return svc.load().then(groups => {
           let displayedGroups = groups.map(g => g.id);

src/sidebar/util/groups.js

+'use strict';
+
+/**
+ * Combine groups from multiple api calls together to form a unique list of groups.
+ * Add an isMember property to each group indicating whether the logged in user is a member.
+ *
+ * @param {Group[]} userGroups - groups the user is a member of
+ * @param {Group[]} featuredGroups - groups scoped to the particular uri, authority, and user
+ */
+function combineGroups(userGroups, featuredGroups) {
+  const worldGroup = featuredGroups.find(g => g.id === '__world__');
+  if (worldGroup) {
+    userGroups.unshift(worldGroup);
+  }
+
+  const myGroupIds = userGroups.map(g => g.id);
+  featuredGroups = featuredGroups.filter(g => !myGroupIds.includes(g.id));
+
+  // Set the isMember property indicating user membership.
+  featuredGroups.forEach(group => (group.isMember = false));
+  userGroups.forEach(group => (group.isMember = true));
+
+  return userGroups.concat(featuredGroups);
+}
+
+module.exports = {
+  combineGroups,
+};

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

+'use strict';
+
+const { combineGroups } = require('../groups');
+
+describe('sidebar.util.groups', () => {
+  describe('combineGroups', () => {
+    it('labels groups in both lists as isMember true', () => {
+      const userGroups = [{ id: 'groupa', name: 'GroupA' }];
+      const featuredGroups = [{ id: 'groupa', name: 'GroupA' }];
+      const groups = combineGroups(userGroups, featuredGroups);
+      const groupA = groups.find(g => g.id === 'groupa');
+      assert.equal(groupA.isMember, true);
+    });
+
+    it('combines groups from both lists uniquely', () => {
+      const userGroups = [
+        { id: 'groupa', name: 'GroupA' },
+        { id: 'groupb', name: 'GroupB' },
+      ];
+      const featuredGroups = [
+        { id: 'groupa', name: 'GroupA' },
+        { id: '__world__', name: 'Public' },
+      ];
+      const groups = combineGroups(userGroups, featuredGroups);
+      const ids = groups.map(g => g.id);
+      assert.deepEqual(ids, ['__world__', 'groupa', 'groupb']);
+    });
+
+    it('adds isMember attribute to each group', () => {
+      const userGroups = [{ id: 'groupa', name: 'GroupA' }];
+      const featuredGroups = [
+        { id: 'groupb', name: 'GroupB' },
+        { id: '__world__', name: 'Public' },
+      ];
+
+      const expectedMembership = {
+        __world__: true,
+        groupa: true,
+        groupb: false,
+      };
+
+      const groups = combineGroups(userGroups, featuredGroups);
+      groups.forEach(g => assert.equal(g.isMember, expectedMembership[g.id]));
+    });
+
+    it('maintains the original ordering', () => {
+      const userGroups = [
+        { id: 'one', name: 'GroupA' },
+        { id: 'two', name: 'GroupB' },
+      ];
+      const featuredGroups = [
+        { id: 'one', name: 'GroupA' },
+        { id: 'three', name: 'GroupC' },
+      ];
+
+      const groups = combineGroups(userGroups, featuredGroups);
+      const ids = groups.map(g => g.id);
+      assert.deepEqual(ids, ['one', 'two', 'three']);
+    });
+
+    it('lists the Public group first', () => {
+      const userGroups = [{ id: 'one', name: 'GroupA' }];
+      const featuredGroups = [{ id: '__world__', name: 'Public' }];
+
+      const groups = combineGroups(userGroups, featuredGroups);
+      assert.equal(groups[0].id, '__world__');
+    });
+
+    it('handles case where there is no Public group', () => {
+      const userGroups = [];
+      const featuredGroups = [];
+
+      const groups = combineGroups(userGroups, featuredGroups);
+      assert.deepEqual(groups, []);
+    });
+  });
+});