Add logic to load focused group members

src/sidebar/components/Annotation/AnnotationEditor.tsx

@@ -1,4 +1,4 @@
-import { useCallback, useMemo, useState } from 'preact/hooks';
+import { useCallback, useEffect, useMemo, useState } from 'preact/hooks';
 
 import type { Annotation } from '../../../types/api';
 import type { SidebarSettings } from '../../../types/config';
@@ -10,6 +10,7 @@ import {
 import { applyTheme } from '../../helpers/theme';
 import { withServices } from '../../service-context';
 import type { AnnotationsService } from '../../services/annotations';
+import type { GroupsService } from '../../services/groups';
 import type { TagsService } from '../../services/tags';
 import type { ToastMessengerService } from '../../services/toast-messenger';
 import { useSidebarStore } from '../../store';
@@ -27,6 +28,7 @@ type AnnotationEditorProps = {
 
   // Injected
   annotationsService: AnnotationsService;
+  groups: GroupsService;
   settings: SidebarSettings;
   toastMessenger: ToastMessengerService;
   tags: TagsService;
@@ -39,6 +41,7 @@ function AnnotationEditor({
   annotation,
   draft,
   annotationsService,
+  groups: groupsService,
   settings,
   tags: tagsService,
   toastMessenger,
@@ -169,6 +172,14 @@ function AnnotationEditor({
 
   const mentionsEnabled = store.isFeatureEnabled('at_mentions');
   const usersWhoAnnotated = store.usersWhoAnnotated();
+  const focusedGroupMembers = store.getFocusedGroupMembers();
+
+  useEffect(() => {
+    // When entering edit mode, load focused group members if not yet loaded
+    if (mentionsEnabled && focusedGroupMembers === null) {
+      groupsService.loadFocusedGroupMembers();
+    }
+  }, [focusedGroupMembers, groupsService, mentionsEnabled]);
 
   return (
     /* eslint-disable-next-line jsx-a11y/no-static-element-interactions */
@@ -208,6 +219,7 @@ function AnnotationEditor({
 
 export default withServices(AnnotationEditor, [
   'annotationsService',
+  'groups',
   'settings',
   'tags',
   'toastMessenger',

src/sidebar/components/Annotation/test/AnnotationEditor-test.js

@@ -5,6 +5,7 @@ import {
 } from '@hypothesis/frontend-testing';
 import { mount } from '@hypothesis/frontend-testing';
 import { act } from 'preact/test-utils';
+import sinon from 'sinon';
 
 import * as fixtures from '../../../test/annotation-fixtures';
 import AnnotationEditor, { $imports } from '../AnnotationEditor';
@@ -16,6 +17,7 @@ describe('AnnotationEditor', () => {
   let fakeTagsService;
   let fakeSettings;
   let fakeToastMessenger;
+  let fakeGroupsService;
 
   let fakeStore;
 
@@ -30,6 +32,7 @@ describe('AnnotationEditor', () => {
         settings={fakeSettings}
         tags={fakeTagsService}
         toastMessenger={fakeToastMessenger}
+        groups={fakeGroupsService}
         {...props}
       />,
     );
@@ -54,6 +57,9 @@ describe('AnnotationEditor', () => {
       error: sinon.stub(),
       success: sinon.stub(),
     };
+    fakeGroupsService = {
+      loadFocusedGroupMembers: sinon.stub(),
+    };
 
     fakeStore = {
       createDraft: sinon.stub(),
@@ -63,6 +69,7 @@ describe('AnnotationEditor', () => {
       removeAnnotations: sinon.stub(),
       isFeatureEnabled: sinon.stub().returns(false),
       usersWhoAnnotated: sinon.stub().returns([]),
+      getFocusedGroupMembers: sinon.stub(),
     };
 
     $imports.$mock(mockImportedComponents());
@@ -386,6 +393,45 @@ describe('AnnotationEditor', () => {
     assert.isFalse(wrapper.find('AnnotationLicense').exists());
   });
 
+  describe('loading focused group members', () => {
+    [
+      {
+        atMentionsEnabled: true,
+        focusedGroupMembers: null,
+        shouldLoadMembers: true,
+      },
+      {
+        atMentionsEnabled: false,
+        focusedGroupMembers: null,
+        shouldLoadMembers: false,
+      },
+      {
+        atMentionsEnabled: true,
+        focusedGroupMembers: [],
+        shouldLoadMembers: false,
+      },
+      {
+        atMentionsEnabled: false,
+        focusedGroupMembers: [],
+        shouldLoadMembers: false,
+      },
+    ].forEach(
+      ({ atMentionsEnabled, focusedGroupMembers, shouldLoadMembers }) => {
+        it('loads focused group members when mounted', () => {
+          fakeStore.isFeatureEnabled.returns(atMentionsEnabled);
+          fakeStore.getFocusedGroupMembers.returns(focusedGroupMembers);
+
+          createComponent();
+
+          assert.equal(
+            fakeGroupsService.loadFocusedGroupMembers.called,
+            shouldLoadMembers,
+          );
+        });
+      },
+    );
+  });
+
   it(
     'should pass a11y checks',
     checkAccessibility([

src/sidebar/services/api.ts

@@ -4,6 +4,7 @@ import type {
   RouteMap,
   RouteMetadata,
   Profile,
+  GroupMembers,
 } from '../../types/api';
 import { stripInternalProperties } from '../helpers/strip-internal-properties';
 import type { SidebarStore } from '../store';
@@ -218,6 +219,17 @@ export class APIService {
     member: {
       delete: APICall<{ pubid: string; userid: string }>;
     };
+    members: {
+      read: APICall<
+        {
+          pubid: string;
+          'page[number]'?: number;
+          'page[size]'?: number;
+        },
+        void,
+        GroupMembers
+      >;
+    };
     read: APICall<{ id: string; expand: string[] }, void, Group>;
   };
   groups: {
@@ -287,6 +299,17 @@ export class APIService {
           userid: string;
         }>,
       },
+      members: {
+        read: apiCall('group.members.read') as APICall<
+          {
+            pubid: string;
+            'page[number]'?: number;
+            'page[size]'?: number;
+          },
+          void,
+          GroupMembers
+        >,
+      },
       read: apiCall('group.read') as APICall<
         { id: string; expand: string[] },
         void,

src/sidebar/services/groups.ts

@@ -2,7 +2,7 @@ import shallowEqual from 'shallowequal';
 
 // @ts-ignore - TS doesn't know about SVG files.
 import { default as logo } from '../../images/icons/logo.svg';
-import type { Group } from '../../types/api';
+import type { Group, GroupMember, GroupMembers } from '../../types/api';
 import type { SidebarSettings } from '../../types/config';
 import type { Service } from '../../types/config';
 import { serviceConfig } from '../config/service-config';
@@ -23,6 +23,8 @@ const DEFAULT_ORGANIZATION = {
   logo: 'data:image/svg+xml;utf8,' + encodeURIComponent(logo),
 };
 
+const MEMBERS_PAGE_SIZE = 100;
+
 /**
  * For any group that does not have an associated organization, populate with
  * the default Hypothesis organization.
@@ -478,4 +480,48 @@ export class GroupsService {
       userid: 'me',
     });
   }
+
+  /**
+   * Fetch members for a group from the API and load them into the store as the
+   * members of the focused group.
+   */
+  async loadFocusedGroupMembers(): Promise<void> {
+    const groupId = this._store.focusedGroupId();
+    if (!groupId) {
+      return; // TODO Throw error?
+    }
+
+    const members = await this._fetchAllMembers(groupId);
+    this._store.loadFocusedGroupMembers(members);
+  }
+
+  private async _fetchAllMembers(groupId: string): Promise<GroupMember[]> {
+    // Fetch first page of members, to determine how many more pages there are
+    const firstPage = await this._fetchMembers(groupId);
+    const pages = Math.min(
+      Math.ceil(firstPage.meta.page.total / MEMBERS_PAGE_SIZE),
+      // Do not try to load more than 10 pages, to avoid long loading times and
+      // hitting the server too much
+      10,
+    );
+    let members = firstPage.data;
+
+    // Fetch remaining pages. Start at 2 to skip first one which was already
+    // loaded.
+    for (let page = 2; page <= pages; page++) {
+      // TODO Consider parallelizing requests
+      const membersPage = await this._fetchMembers(groupId, page);
+      members = members.concat(membersPage.data);
+    }
+
+    return members;
+  }
+
+  private _fetchMembers(groupId: string, page = 1): Promise<GroupMembers> {
+    return this._api.group.members.read({
+      pubid: groupId,
+      'page[number]': page,
+      'page[size]': MEMBERS_PAGE_SIZE,
+    });
+  }
 }

src/sidebar/services/test/api-index.json

@@ -30,6 +30,13 @@
           "desc": "Remove the current user from a group."
         }
       },
+      "members": {
+        "read": {
+          "url": "https://example.com/api/groups/:pubid/members",
+          "method": "GET",
+          "desc": "Fetch a list of all members of a group"
+        }
+      },
       "read": {
         "url": "https://example.com/api/groups/:id",
         "method": "GET",

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

@@ -124,6 +124,26 @@ describe('APIService', () => {
     return api.group.member.delete({ pubid: 'an-id', userid: 'me' });
   });
 
+  it('gets group members', () => {
+    const groupMembers = {
+      meta: {
+        page: { total: 0 },
+      },
+      data: [],
+    };
+    expectCall(
+      'get',
+      `groups/an-id/members?${encodeURIComponent('page[number]')}=1&${encodeURIComponent('page[size]')}=100`,
+      200,
+      groupMembers,
+    );
+    return api.group.members.read({
+      pubid: 'an-id',
+      'page[number]': 1,
+      'page[size]': 100,
+    });
+  });
+
   it('gets a group by provided group id', () => {
     const group = { id: 'group-id', name: 'Group' };
     expectCall('get', 'groups/group-id', 200, group);

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

@@ -1,4 +1,5 @@
 import { delay, waitFor } from '@hypothesis/frontend-testing';
+import sinon from 'sinon';
 
 import { fakeReduxStore } from '../../test/fake-redux-store';
 import { GroupsService, $imports } from '../groups';
@@ -89,6 +90,9 @@ describe('GroupsService', () => {
         clearDirectLinkedGroupFetchFailed: sinon.stub(),
         profile: sinon.stub().returns({ userid: null }),
         route: sinon.stub().returns('sidebar'),
+        isFeatureEnabled: sinon.stub().returns(false),
+        getFocusedGroupMembers: sinon.stub().returns(null),
+        loadFocusedGroupMembers: sinon.stub(),
       },
     );
     fakeApi = {
@@ -100,6 +104,9 @@ describe('GroupsService', () => {
         member: {
           delete: sinon.stub().returns(Promise.resolve()),
         },
+        members: {
+          read: sinon.stub().resolves({}),
+        },
         read: sinon.stub().returns(Promise.resolve(new Error('404 Error'))),
       },
       groups: {
@@ -205,6 +212,46 @@ describe('GroupsService', () => {
 
       assert.notCalled(fakeStore.setDefault);
     });
+
+    describe('#loadFocusedGroupMembers', () => {
+      it('skips loading members if focused group is not set', async () => {
+        const svc = createService();
+        await svc.loadFocusedGroupMembers();
+
+        assert.notCalled(fakeStore.loadFocusedGroupMembers);
+        assert.notCalled(fakeApi.group.members.read);
+      });
+
+      [
+        { totalMembers: 48, expectedApiCalls: 1 },
+        { totalMembers: 100, expectedApiCalls: 1 },
+        { totalMembers: 125, expectedApiCalls: 2 },
+        { totalMembers: 236, expectedApiCalls: 3 },
+        { totalMembers: 650, expectedApiCalls: 7 },
+        { totalMembers: 936, expectedApiCalls: 10 },
+
+        // We'll never load more than 10 pages
+        { totalMembers: 1_000, expectedApiCalls: 10 },
+        { totalMembers: 1_200, expectedApiCalls: 10 },
+        { totalMembers: 10_000, expectedApiCalls: 10 },
+      ].forEach(({ totalMembers, expectedApiCalls }) => {
+        it('calls members API as many times as needed, until all members are loaded', async () => {
+          fakeStore.focusedGroupId.returns('group');
+          fakeApi.group.members.read.resolves({
+            meta: {
+              page: { total: totalMembers },
+            },
+            data: [],
+          });
+
+          const svc = createService();
+          await svc.loadFocusedGroupMembers();
+
+          assert.called(fakeStore.loadFocusedGroupMembers);
+          assert.callCount(fakeApi.group.members.read, expectedApiCalls);
+        });
+      });
+    });
   });
 
   describe('#load', () => {