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';
@@ -11,6 +11,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';
@@ -28,6 +29,7 @@ type AnnotationEditorProps = {
 
   // Injected
   annotationsService: AnnotationsService;
+  groups: GroupsService;
   settings: SidebarSettings;
   toastMessenger: ToastMessengerService;
   tags: TagsService;
@@ -40,6 +42,7 @@ function AnnotationEditor({
   annotation,
   draft,
   annotationsService,
+  groups: groupsService,
   settings,
   tags: tagsService,
   toastMessenger,
@@ -174,6 +177,15 @@ function AnnotationEditor({
 
   const mentionsEnabled = store.isFeatureEnabled('at_mentions');
   const usersWhoAnnotated = store.usersWhoAnnotated();
+  const focusedGroupMembers = store.getFocusedGroupMembers();
+  const focusedGroupId = store.focusedGroupId();
+
+  useEffect(() => {
+    // Load members for focused group only if not yet loaded
+    if (mentionsEnabled && focusedGroupId && focusedGroupMembers === null) {
+      groupsService.loadFocusedGroupMembers(focusedGroupId);
+    }
+  }, [focusedGroupId, focusedGroupMembers, groupsService, mentionsEnabled]);
 
   return (
     /* eslint-disable-next-line jsx-a11y/no-static-element-interactions */
@@ -214,6 +226,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().resolves(undefined),
+    };
 
     fakeStore = {
       createDraft: sinon.stub(),
@@ -63,6 +69,8 @@ describe('AnnotationEditor', () => {
       removeAnnotations: sinon.stub(),
       isFeatureEnabled: sinon.stub().returns(false),
       usersWhoAnnotated: sinon.stub().returns([]),
+      getFocusedGroupMembers: sinon.stub(),
+      focusedGroupId: sinon.stub().returns(null),
     };
 
     $imports.$mock(mockImportedComponents());
@@ -405,6 +413,61 @@ describe('AnnotationEditor', () => {
     });
   });
 
+  describe('loading focused group members', () => {
+    [
+      {
+        atMentionsEnabled: true,
+        focusedGroupMembers: null,
+        focusedGroupId: 'group_id',
+        shouldLoadMembers: true,
+      },
+      {
+        atMentionsEnabled: true,
+        focusedGroupMembers: null,
+        focusedGroupId: null,
+        shouldLoadMembers: false,
+      },
+      {
+        atMentionsEnabled: false,
+        focusedGroupMembers: null,
+        focusedGroupId: 'group_id',
+        shouldLoadMembers: false,
+      },
+      {
+        atMentionsEnabled: true,
+        focusedGroupMembers: [],
+        focusedGroupId: 'group_id',
+        shouldLoadMembers: false,
+      },
+      {
+        atMentionsEnabled: false,
+        focusedGroupMembers: [],
+        focusedGroupId: 'group_id',
+        shouldLoadMembers: false,
+      },
+    ].forEach(
+      ({
+        atMentionsEnabled,
+        focusedGroupMembers,
+        focusedGroupId,
+        shouldLoadMembers,
+      }) => {
+        it('loads focused group members when mounted', () => {
+          fakeStore.isFeatureEnabled.returns(atMentionsEnabled);
+          fakeStore.getFocusedGroupMembers.returns(focusedGroupMembers);
+          fakeStore.focusedGroupId.returns(focusedGroupId);
+
+          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),
 };
 
+export const MEMBERS_PAGE_SIZE = 100;
+
 /**
  * For any group that does not have an associated organization, populate with
  * the default Hypothesis organization.
@@ -61,6 +63,12 @@ export class GroupsService {
   private _serviceConfig: Service | null;
   private _reloadSetUp: boolean;
 
+  /**
+   * Tracks the loading of the focused group members.
+   * If this is null it means we are not loading members yet.
+   */
+  private _focusedMembersController: AbortController | null = null;
+
   constructor(
     store: SidebarStore,
     api: APIService,
@@ -450,6 +458,10 @@ export class GroupsService {
 
     const groupHasChanged = prevGroupId !== newGroupId && prevGroupId !== null;
     if (groupHasChanged && newGroupId) {
+      // Abort previous in-flight members loading if the group was different
+      // from the focused one
+      this._focusedMembersController?.abort();
+
       // Move any top-level new annotations to the newly-focused group.
       // Leave replies where they are.
       const updatedAnnotations = this._store
@@ -478,4 +490,71 @@ 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(groupId: string): Promise<void> {
+    // Abort previous loading, if any
+    this._focusedMembersController?.abort();
+
+    this._focusedMembersController = new AbortController();
+    const { signal } = this._focusedMembersController;
+
+    const members = await this._fetchAllMembers(groupId, signal);
+    if (!signal?.aborted) {
+      this._store.loadFocusedGroupMembers(members);
+    }
+  }
+
+  private async _fetchAllMembers(
+    groupId: string,
+    signal?: AbortSignal,
+  ): Promise<GroupMember[]> {
+    // Fetch first page of members, to determine how many more pages there are
+    const firstPage = await this._fetchMembers({ groupId, signal });
+    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.
+    // TODO Consider parallelizing requests
+    for (let page = 2; page <= pages; page++) {
+      // Do not attempt any further request once the signal has been aborted
+      if (signal?.aborted) {
+        break;
+      }
+
+      const membersPage = await this._fetchMembers({ groupId, page, signal });
+      members = members.concat(membersPage.data);
+    }
+
+    return members;
+  }
+
+  private _fetchMembers({
+    groupId,
+    page = 1,
+    signal,
+  }: {
+    groupId: string;
+    page?: number;
+    signal?: AbortSignal;
+  }): Promise<GroupMembers> {
+    return this._api.group.members.read(
+      {
+        pubid: groupId,
+        'page[number]': page,
+        'page[size]': MEMBERS_PAGE_SIZE,
+      },
+      undefined,
+      signal,
+    );
+  }
 }

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);