Merge pull request #1124 from hypothesis/simplify-groups-load-part-2

Simplify groups service `load` method using async/await

src/sidebar/services/groups.js

@@ -138,7 +138,7 @@ function groups(
   // to include groups associated with this page. This is retained to determine
   // whether we need to re-fetch groups if the URLs of frames connected to the
   // sidebar app changes.
-  let documentUri;
+  let documentUri = null;
 
   /*
    * Fetch an individual group.
@@ -158,54 +158,35 @@ function groups(
    * group.
    *
    * The groups that are fetched depend on the current user, the URI of
-   * the current document, and whether any direct-links were followed (either
-   * to an annotation or group).
+   * the current document, and the direct-linked group and/or annotation.
    *
    * @return {Promise<Group[]>}
    */
-  function load() {
-    let uri = Promise.resolve(null);
+  async function load() {
+    // Step 1: Get the URI of the active document, so we can fetch groups
+    // associated with that document.
     if (isSidebar) {
-      uri = getDocumentUriForGroupSearch();
+      documentUri = await getDocumentUriForGroupSearch();
     }
-    const directLinkedGroupId = store.getState().directLinkedGroupId;
-    const directLinkedAnnId = store.getState().directLinkedAnnotationId;
-    const params = {
-      expand: ['organization', 'scopes'],
-    };
-
-    let directLinkedAnnotationGroupId = null;
 
-    // Step 1: Get the URI of the active document, so we can fetch groups
-    // associated with that document.
-    return uri
-      .then(uri => {
     // Step 2: Concurrently fetch the groups the user is a member of,
     // the groups associated with the current document and the annotation
-        // or group that was direct-linked (if any).
+    // and/or group that was direct-linked (if any).
+    const params = {
+      expand: ['organization', 'scopes'],
+    };
     if (authority) {
       params.authority = authority;
     }
-        if (uri) {
-          params.document_uri = uri;
+    if (documentUri) {
+      params.document_uri = documentUri;
     }
-        documentUri = uri;
-
-        const profileGroupsApi = api.profile.groups.read({
-          expand: params.expand,
-        });
-        const listGroupsApi = api.groups.list(params);
-        let groupApiRequests = [
-          profileGroupsApi,
-          listGroupsApi,
-          auth.tokenGetter(),
-        ];
 
-        // If there is a direct-linked annotation, fetch the annotation to see
-        // if there needs to be a second API request to fetch its group since
-        // the group may not be in the results returned by group.list,
-        // profile.groups, or the direct-linked group.
-        let directLinkedAnnApi = Promise.resolve(null);
+    // If there is a direct-linked annotation, fetch the annotation in case
+    // the associated group has not already been fetched and we need to make
+    // an additional request for it.
+    const directLinkedAnnId = store.getState().directLinkedAnnotationId;
+    let directLinkedAnnApi = null;
     if (directLinkedAnnId) {
       directLinkedAnnApi = api.annotation
         .get({ id: directLinkedAnnId })
@@ -214,12 +195,12 @@ function groups(
           return null;
         });
     }
-        groupApiRequests = groupApiRequests.concat(directLinkedAnnApi);
 
     // If there is a direct-linked group, add an API request to get that
-        // particular group since it may not be in the results returned by
-        // group.list or profile.groups.
-        let directLinkedGroupApi = Promise.resolve(null);
+    // particular group since it may not be in the set of groups that are
+    // fetched by other requests.
+    const directLinkedGroupId = store.getState().directLinkedGroupId;
+    let directLinkedGroupApi = null;
     if (directLinkedGroupId) {
       directLinkedGroupApi = fetchGroup({
         id: directLinkedGroupId,
@@ -234,58 +215,58 @@ function groups(
         return group;
       });
     }
-        groupApiRequests = groupApiRequests.concat(directLinkedGroupApi);
-        return Promise.all(groupApiRequests);
-      })
-      .then(
-        ([
+
+    const [
       myGroups,
       featuredGroups,
       token,
       directLinkedAnn,
       directLinkedGroup,
-        ]) => {
+    ] = await Promise.all([
+      api.profile.groups.read({ expand: params.expand }),
+      api.groups.list(params),
+      auth.tokenGetter(),
+      directLinkedAnnApi,
+      directLinkedGroupApi,
+    ]);
+
     // Step 3. Add the direct-linked group to the list of featured groups,
     // and if there was a direct-linked annotation, fetch its group if we
     // don't already have it.
 
     // If there is a direct-linked group, add it to the featured groups list.
-          let allFeaturedGroups =
-            directLinkedGroup !== null &&
+    if (
+      directLinkedGroup &&
       !featuredGroups.some(g => g.id === directLinkedGroup.id)
-              ? featuredGroups.concat([directLinkedGroup])
-              : featuredGroups;
+    ) {
+      featuredGroups.push(directLinkedGroup);
+    }
 
     // If there's a direct-linked annotation it may require an extra API call
     // to fetch its group.
+    let directLinkedAnnotationGroupId = null;
     if (directLinkedAnn) {
       // Set the directLinkedAnnotationGroupId to be used later in
       // the filterGroups method.
       directLinkedAnnotationGroupId = directLinkedAnn.group;
 
-            const directLinkedAnnGroup = myGroups
-              .concat(allFeaturedGroups)
-              .some(g => g.id === directLinkedAnn.group);
-
       // If the direct-linked annotation's group has not already been fetched,
       // fetch it.
+      const directLinkedAnnGroup = myGroups
+        .concat(featuredGroups)
+        .find(g => g.id === directLinkedAnn.group);
+
       if (!directLinkedAnnGroup) {
-              const initialFeaturedGroups = allFeaturedGroups;
-              allFeaturedGroups = fetchGroup({
+        const directLinkedAnnGroup = await fetchGroup({
           id: directLinkedAnn.group,
           expand: params.expand,
-              }).then(directLinkedAnnGroup => {
-                if (!directLinkedAnnGroup) {
-                  return initialFeaturedGroups;
-                }
-                return initialFeaturedGroups.concat(directLinkedAnnGroup);
         });
+        if (directLinkedAnnGroup) {
+          featuredGroups.push(directLinkedAnnGroup);
         }
       }
-          return Promise.all([myGroups, allFeaturedGroups, documentUri, token]);
     }
-      )
-      .then(([myGroups, featuredGroups, documentUri, token]) => {
+
     // Step 4. Combine all the groups into a single list and set additional
     // metadata on them that will be used elsewhere in the app.
     const isLoggedIn = token !== null;
@@ -305,22 +286,17 @@ function groups(
 
     store.loadGroups(groups);
 
-        if (
-          isFirstLoad &&
-          groups.some(g => g.id === directLinkedAnnotationGroupId)
-        ) {
+    if (isFirstLoad) {
+      if (groups.some(g => g.id === directLinkedAnnotationGroupId)) {
         store.focusGroup(directLinkedAnnotationGroupId);
-        } else if (
-          isFirstLoad &&
-          groups.some(g => g.id === directLinkedGroupId)
-        ) {
+      } else if (groups.some(g => g.id === directLinkedGroupId)) {
         store.focusGroup(directLinkedGroupId);
-        } else if (isFirstLoad && groups.some(g => g.id === prevFocusedGroup)) {
+      } else if (groups.some(g => g.id === prevFocusedGroup)) {
         store.focusGroup(prevFocusedGroup);
       }
+    }
 
     return groups;
-      });
   }
 
   const sortGroups = memoize(groups => {