Add helper for fetching all API pages

src/amo/api/collections.js

@@ -1,9 +1,5 @@
 /* @flow */
-/* eslint-disable no-await-in-loop */
-import { oneLine } from 'common-tags';
-
-import { callApi } from 'core/api';
-import log from 'core/logger';
+import { callApi, allPages } from 'core/api';
 import type {
   ExternalCollectionAddon, ExternalCollectionDetail,
 } from 'amo/reducers/collections';
@@ -64,30 +60,25 @@ export const getCollectionAddons = (
   return callApi(request);
 };
 
+type GetAllCollectionAddonsParams = {|
+  ...GetCollectionParams,
+  _allPages?: typeof allPages,
+  _getCollectionAddons?: typeof getCollectionAddons,
+|};
+
 export const getAllCollectionAddons = async (
-  { api, slug, user }: GetCollectionParams
+  {
+    api,
+    slug,
+    user,
+    _allPages = allPages,
+    _getCollectionAddons = getCollectionAddons,
+  }: GetAllCollectionAddonsParams
 ): Promise<Array<ExternalCollectionAddon>> => {
-  let allResults = [];
-  let done = false;
-  let nextURL;
-
-  // TODO: make a helper function for this pattern.
-  while (!done) {
-    const response = await getCollectionAddons({
-      api, nextURL, slug, user,
-    });
-    allResults = allResults.concat(response.results);
-
-    if (response.next) {
-      nextURL = response.next;
-      log.debug(oneLine`Fetching next page "${nextURL}" of
-        getCollectionAddons for "${user}/${slug}"`);
-    } else {
-      done = true;
-    }
-  }
-
-  return allResults;
+  const { results } = await _allPages(
+    (nextURL) => _getCollectionAddons({ api, nextURL, slug, user })
+  );
+  return results;
 };
 
 type ListCollectionsParams = {|
@@ -107,27 +98,24 @@ export const listCollections = (
   return callApi({ auth: true, endpoint, state: api });
 };
 
+type GetAllUserCollectionsParams = {|
+  ...ListCollectionsParams,
+  _allPages?: typeof allPages,
+  _listCollections?: typeof listCollections,
+|};
+
 export const getAllUserCollections = async (
-  { api, user }: ListCollectionsParams
+  {
+    api,
+    user,
+    _allPages = allPages,
+    _listCollections = listCollections,
+  }: GetAllUserCollectionsParams
 ): Promise<Array<ExternalCollectionDetail>> => {
-  let allResults = [];
-  let done = false;
-  let nextURL;
-
-  while (!done) {
-    const response = await listCollections({ api, user, nextURL });
-    allResults = allResults.concat(response.results);
-
-    if (response.next) {
-      nextURL = response.next;
-      log.debug(oneLine`Fetching next page "${nextURL}" of
-        listCollections for user "${user}"`);
-    } else {
-      done = true;
-    }
-  }
-
-  return allResults;
+  const { results } = await _allPages(
+    (nextURL) => _listCollections({ api, nextURL, user })
+  );
+  return results;
 };
 
 type GetAllUserAddonCollections = {|
@@ -147,10 +135,6 @@ export const getAllUserAddonCollections = async (
     _getAllUserCollections = getAllUserCollections,
   }: GetAllUserAddonCollections
 ): Promise<Array<ExternalCollectionDetail>> => {
-  // TODO: ultimately, we should query a single API endpoint to
-  // fetch all user collections that an add-on belongs to.
-  // https://github.com/mozilla/addons-server/issues/7167
-
   // Fetch all collections belonging to the user.
   const collectionResults = await _getAllUserCollections({ api, user });
 

src/amo/reducers/collections.js

@@ -179,13 +179,15 @@ export const fetchCurrentCollectionPage = ({
   };
 };
 
-type ExternalCollectionAddons = Array<{|
+export type ExternalCollectionAddon = {|
   addon: ExternalAddonType,
   downloads: number,
   notes: string | null,
-|}>;
+|};
+
+type ExternalCollectionAddons = Array<ExternalCollectionAddon>;
 
-type ExternalCollectionDetail = {|
+export type ExternalCollectionDetail = {|
   addon_count: number,
   author: {|
     id: number,

src/core/api/index.js

@@ -16,6 +16,7 @@ import {
 } from 'core/searchUtils';
 import type { ErrorHandlerType } from 'core/errorHandler';
 import type { ApiStateType } from 'core/reducers/api';
+import type { PaginatedApiResponse } from 'core/types/api';
 import type { ReactRouterLocation } from 'core/types/router';
 
 
@@ -265,3 +266,42 @@ export function autocomplete({ api, filters }: AutocompleteParams) {
     state: api,
   });
 }
+
+type GetNextResponseType =
+  (nextURL?: string) => Promise<PaginatedApiResponse<any>>;
+
+type AllPagesOptions = {| pageLimit: number |};
+
+export const allPages = async (
+  getNextResponse: GetNextResponseType,
+  { pageLimit = 100 }: AllPagesOptions = {},
+): Promise<PaginatedApiResponse<any>> => {
+  let results = [];
+  let nextURL;
+  let count = 0;
+  let pageSize = 0;
+
+  for (let page = 1; page <= pageLimit; page++) {
+    // eslint-disable-next-line no-await-in-loop
+    const response = await getNextResponse(nextURL);
+    if (!count) {
+      // Every response page returns a count for all results.
+      count = response.count;
+    }
+    if (!pageSize) {
+      pageSize = response.page_size;
+    }
+    results = results.concat(response.results);
+
+    if (response.next) {
+      nextURL = response.next;
+      log.debug(oneLine`Fetching next page "${nextURL}" of
+        ${getNextResponse}`);
+    } else {
+      return { count, page_size: pageSize, results };
+    }
+  }
+
+  // If we get this far the callback may not be advancing pages correctly.
+  throw new Error(`Fetched too many pages (the limit is ${pageLimit})`);
+};

src/core/client/base.js

@@ -1,7 +1,6 @@
 /* global document */
 
-import 'babel-polyfill';
-import 'raf/polyfill';
+import 'core/polyfill';
 import { oneLine } from 'common-tags';
 import config from 'config';
 import FastClick from 'fastclick';

src/core/polyfill.js

@@ -0,0 +1,2 @@
+import 'babel-polyfill';
+import 'raf/polyfill';

src/core/server/base.js

@@ -2,8 +2,7 @@ import fs from 'fs';
 import path from 'path';
 import https from 'https';
 
-import 'babel-polyfill';
-import 'raf/polyfill';
+import 'core/polyfill';
 import { oneLine } from 'common-tags';
 import defaultConfig from 'config';
 import Express from 'express';

tests/unit/amo/api/test_collections.js

@@ -119,48 +119,28 @@ describe(__filename, () => {
     it('gets all pages of the collection add-ons list API', async () => {
       const user = 'example-username';
       const slug = 'example-collection-slug';
+      const addonResults = createFakeCollectionAddons().results;
 
-      const firstAddonSet = createFakeCollectionAddons({
-        addons: [{ ...fakeAddon, id: 1 }],
-      });
-      const secondAddonSet = createFakeCollectionAddons({
-        addons: [{ ...fakeAddon, id: 2 }],
-      });
+      const _getCollectionAddons = sinon.spy(
+        () => Promise.resolve(apiResponsePage({ results: addonResults }))
+      );
 
-      let page = 0;
-      const endpoint =
-        `accounts/account/${user}/collections/${slug}/addons`;
-      mockApi
-        .expects('callApi')
-        .withArgs({
-          auth: true,
-          endpoint: sinon.match(endpoint),
-          params: sinon.match.any,
-          state: apiState,
-        })
-        .twice()
-        .callsFake((request) => {
-          page += 1;
-          let next;
-          let results = [];
-          if (page === 1) {
-            next = `${endpoint}?page=2`;
-            results = firstAddonSet.results;
-          } else {
-            results = secondAddonSet.results;
-            // When we pass a next URL, it will include ?page=... so it
-            // is important that the page parameter is not sent.
-            expect(request.params).toEqual(undefined);
-          }
-          return Promise.resolve(apiResponsePage({ next, results }));
-        });
+      const nextURL = 'the-endpoint?page=2';
+      const _allPages = sinon.spy((nextPage) => nextPage(nextURL));
 
       const addons = await getAllCollectionAddons({
-        api: apiState, user, slug,
+        api: apiState,
+        user,
+        slug,
+        _allPages,
+        _getCollectionAddons,
+      });
+
+      expect(addons).toEqual(addonResults);
+      sinon.assert.called(_getCollectionAddons);
+      expect(_getCollectionAddons.firstCall.args[0]).toEqual({
+        api: apiState, user, slug, nextURL,
       });
-      expect(addons)
-        .toEqual(firstAddonSet.results.concat(secondAddonSet.results));
-      mockApi.verify();
     });
   });
 
@@ -285,39 +265,28 @@ describe(__filename, () => {
     it('returns collections from multiple pages', async () => {
       const user = 'some-user-id';
 
-      const firstCollection = createFakeCollectionDetail({
-        slug: 'first',
-      });
-      const secondCollection = createFakeCollectionDetail({
-        slug: 'second',
-      });
+      const collectionResults = [
+        createFakeCollectionDetail({ slug: 'first' }),
+      ];
 
-      let page = 0;
-      const endpoint = `accounts/account/${user}/collections`;
-      mockApi
-        .expects('callApi')
-        .withArgs({
-          auth: true, endpoint: sinon.match(endpoint), state: apiState,
-        })
-        .twice()
-        .callsFake(() => {
-          page += 1;
-          let next;
-          const results = [];
-          if (page === 1) {
-            next = `${endpoint}?page=2`;
-            results.push(firstCollection);
-          } else {
-            results.push(secondCollection);
-          }
-          return Promise.resolve(apiResponsePage({ next, results }));
-        });
+      const _listCollections = sinon.spy(
+        () => Promise.resolve(
+          apiResponsePage({ results: collectionResults })
+        )
+      );
+
+      const nextURL = 'the-endpoint?page=2';
+      const _allPages = sinon.spy((nextPage) => nextPage(nextURL));
 
-      const results = await getAllUserCollections({
-        user, api: apiState,
+      const collections = await getAllUserCollections({
+        api: apiState, user, _allPages, _listCollections,
+      });
+
+      expect(collections).toEqual(collectionResults);
+      sinon.assert.called(_listCollections);
+      expect(_listCollections.firstCall.args[0]).toEqual({
+        api: apiState, user, nextURL,
       });
-      expect(results).toEqual([firstCollection, secondCollection]);
-      mockApi.verify();
     });
   });