API helpers for creating a collection (#4506)

This adds helpers for creating a collection, and refactors the code for updating a collection to be shared with that for creating a collection. It also introduces the use of [`invariant`](https://github.com/zertosh/invariant) for checking for required parameters.
2018-03-07 15:15:09 -05:00 · 2018-03-07 15:15:09 -05:00 · 045cce65c1
--- a/package.json
+++ b/package.json
@ -169,6 +169,7 @@
    "hot-shots": "5.2.0",
    "html-entities": "1.2.1",
    "humps": "2.0.1",
+    "invariant": "2.2.3",
    "isomorphic-fetch": "2.2.1",
    "jed": "1.1.1",
    "join-url": "2.0.0",
--- a/src/amo/api/collections.js
+++ b/src/amo/api/collections.js
@ -1,4 +1,6 @@
 /* @flow */
+import invariant from 'invariant';
+
 import { callApi, allPages, validateLocalizedString } from 'core/api';
 import type {
  ExternalCollectionAddon,
@ -154,43 +156,67 @@ export const addAddonToCollection = (
  });
 };

-export type UpdateCollectionParams = {|
+type ModifyCollectionParams = {|
  api: ApiStateType,
+  defaultLocale: ?string,
+  description: ?LocalizedString,
+  // Even though the API accepts string|number, we need to always use
+  // string usernames. This helps keep public-facing URLs consistent.
+  user: string,
+  // eslint-disable-next-line no-use-before-define
+  _modifyCollection?: typeof modifyCollection,
+  _validateLocalizedString?: typeof validateLocalizedString,
+|};
+
+export type UpdateCollectionParams = {|
+  ...ModifyCollectionParams,
  // We identify the collection by its slug. This is confusing because the
  // slug can also be edited.
  // TODO: use the actual ID instead.
  // See https://github.com/mozilla/addons-server/issues/7529
  collectionSlug: string,
-  defaultLocale: ?string,
-  description: ?LocalizedString,
  name: ?LocalizedString,
  // This is a value for a new slug, if defined.
  slug: ?string,
-  // Even though the API accepts string|number, we need to always use
-  // string usernames. This helps keep public-facing URLs consistent.
-  user: string,
-  _validateLocalizedString?: typeof validateLocalizedString,
 |};

-export const updateCollection = ({
-  api,
-  collectionSlug,
-  defaultLocale,
-  description,
-  name,
-  slug,
-  user,
-  _validateLocalizedString = validateLocalizedString,
-}: UpdateCollectionParams): Promise<void> => {
-  if (!api) {
-    throw new Error('The api parameter cannot be empty');
+export type CreateCollectionParams = {|
+  ...ModifyCollectionParams,
+  name: LocalizedString,
+  slug: string,
+|};
+
+export const modifyCollection = (
+  action: 'create' | 'update',
+  params: {
+    ...ModifyCollectionParams,
+    collectionSlug?: string,
+    name?: ?LocalizedString,
+    slug?: ?string,
  }
-  if (!collectionSlug) {
-    throw new Error('The collectionSlug parameter cannot be empty');
-  }
-  if (!user) {
-    throw new Error('The user parameter cannot be empty');
+): Promise<void> => {
+  const {
+    api,
+    collectionSlug = '',
+    defaultLocale,
+    description,
+    name,
+    slug,
+    user,
+    _validateLocalizedString = validateLocalizedString,
+  } = params;
+
+  const creating = action === 'create';
+
+  invariant(api, 'The api parameter is required');
+  invariant(user, 'The user parameter is required');
+  if (creating) {
+    invariant(slug, 'The slug parameter is required when creating');
+  } else {
+    invariant(collectionSlug,
+      'The collectionSlug parameter is required when updating');
  }
+
  if (description) {
    _validateLocalizedString(description);
  }
@ -209,8 +235,53 @@ export const updateCollection = ({
      // because collections are always public. Omitting this parameter
      // should cut down on unexpected bugs.
    },
-    endpoint: `accounts/account/${user}/collections/${collectionSlug}`,
-    method: 'PATCH',
+    endpoint:
+      `accounts/account/${user}/collections/${creating ? '' : collectionSlug}`,
+    method: creating ? 'POST' : 'PATCH',
    state: api,
  });
 };
+
+export const updateCollection = ({
+  api,
+  collectionSlug,
+  defaultLocale,
+  description,
+  name,
+  slug,
+  user,
+  _modifyCollection = modifyCollection,
+  _validateLocalizedString = validateLocalizedString,
+}: UpdateCollectionParams): Promise<void> => {
+  return _modifyCollection('update', {
+    api,
+    collectionSlug,
+    defaultLocale,
+    description,
+    name,
+    slug,
+    user,
+    _validateLocalizedString,
+  });
+};
+
+export const createCollection = ({
+  api,
+  defaultLocale,
+  description,
+  name,
+  slug,
+  user,
+  _modifyCollection = modifyCollection,
+  _validateLocalizedString = validateLocalizedString,
+}: CreateCollectionParams): Promise<void> => {
+  return _modifyCollection('create', {
+    api,
+    defaultLocale,
+    description,
+    name,
+    slug,
+    user,
+    _validateLocalizedString,
+  });
+};
--- a/tests/unit/amo/api/test_collections.js
+++ b/tests/unit/amo/api/test_collections.js
@ -1,11 +1,13 @@
 import * as api from 'core/api';
 import {
  addAddonToCollection,
+  createCollection,
  getAllCollectionAddons,
  getAllUserCollections,
  getCollectionAddons,
  getCollectionDetail,
  listCollections,
+  modifyCollection,
  updateCollection,
 } from 'amo/api/collections';
 import { apiResponsePage, createApiResponse } from 'tests/unit/helpers';
@ -288,49 +290,30 @@ describe(__filename, () => {
    });
  });

-  describe('updateCollection', () => {
+  describe('modifyCollection', () => {
+    const slug = 'collection-slug';
+    const name = { fr: 'nomme' };
+
    const defaultParams = (params = {}) => {
      return {
        api: apiState,
-        collectionSlug: 'collection-slug',
+        name,
        user: 'user-id-or-username',
        ...params,
      };
    };

-    it('requires an api parameter', () => {
-      const params = defaultParams();
-      delete params.api;
-
-      expect(() => updateCollection(params))
-        .toThrow(/api parameter cannot be empty/);
-    });
-
-    it('requires a collectionSlug parameter', () => {
-      const params = defaultParams();
-      delete params.collectionSlug;
-
-      expect(() => updateCollection(params))
-        .toThrow(/collectionSlug parameter cannot be empty/);
-    });
-
-    it('requires a user parameter', () => {
-      const params = defaultParams();
-      delete params.user;
-
-      expect(() => updateCollection(params))
-        .toThrow(/user parameter cannot be empty/);
-    });
-
    it('validates description value', async () => {
      const validator = sinon.stub();
      const description = { fr: 'la description' };
      const params = defaultParams({
-        description, _validateLocalizedString: validator,
+        description,
+        slug,
+        _validateLocalizedString: validator,
      });

      mockApi.expects('callApi');
-      await updateCollection(params);
+      await modifyCollection('create', params);

      sinon.assert.calledWith(validator, description);
      mockApi.verify();
@ -338,23 +321,23 @@ describe(__filename, () => {

    it('validates name value', async () => {
      const validator = sinon.stub();
-      const name = { fr: 'nomme' };
      const params = defaultParams({
-        name, _validateLocalizedString: validator,
+        slug,
+        _validateLocalizedString: validator,
      });

      mockApi.expects('callApi');
-      await updateCollection(params);
+      await modifyCollection('create', params);

      sinon.assert.calledWith(validator, name);
      mockApi.verify();
    });

-    it('makes a patch request to the API', async () => {
-      const params = defaultParams({ name: { fr: 'nomme' } });
+    it('makes a POST request to the API for create', async () => {
+      const params = defaultParams({ slug });

      const endpoint =
-        `accounts/account/${params.user}/collections/${params.collectionSlug}`;
+        `accounts/account/${params.user}/collections/`;
      mockApi
        .expects('callApi')
        .withArgs({
@ -362,7 +345,34 @@ describe(__filename, () => {
          body: {
            default_locale: undefined,
            description: undefined,
-            name: params.name,
+            name,
+            slug,
+          },
+          endpoint,
+          method: 'POST',
+          state: params.api,
+        })
+        .once()
+        .returns(Promise.resolve());
+
+      await modifyCollection('create', params);
+
+      mockApi.verify();
+    });
+
+    it('makes a PATCH request to the API for update', async () => {
+      const params = defaultParams({ collectionSlug: slug });
+
+      const endpoint =
+        `accounts/account/${params.user}/collections/${slug}`;
+      mockApi
+        .expects('callApi')
+        .withArgs({
+          auth: true,
+          body: {
+            default_locale: undefined,
+            description: undefined,
+            name,
            slug: undefined,
          },
          endpoint,
@ -372,9 +382,58 @@ describe(__filename, () => {
        .once()
        .returns(Promise.resolve());

-      await updateCollection(params);
+      await modifyCollection('update', params);

      mockApi.verify();
    });
  });
+
+  describe('updateCollection', () => {
+    it('calls modifyCollection with the expected params', async () => {
+      const validator = sinon.stub();
+      const modifier = sinon.spy(() => Promise.resolve());
+      const modifyParams = {
+        api: apiState,
+        collectionSlug: 'collection-slug',
+        defaultLocale: undefined,
+        description: undefined,
+        name: undefined,
+        slug: undefined,
+        user: 'user-id-or-username',
+        _validateLocalizedString: validator,
+      };
+      const updateParams = {
+        ...modifyParams,
+        _modifyCollection: modifier,
+      };
+
+      await updateCollection(updateParams);
+
+      sinon.assert.calledWith(modifier, 'update', modifyParams);
+    });
+  });
+
+  describe('createCollection', () => {
+    it('calls modifyCollection with the expected params', async () => {
+      const validator = sinon.stub();
+      const modifier = sinon.spy(() => Promise.resolve());
+      const modifyParams = {
+        api: apiState,
+        defaultLocale: undefined,
+        description: undefined,
+        name: undefined,
+        slug: 'collection-slug',
+        user: 'user-id-or-username',
+        _validateLocalizedString: validator,
+      };
+      const createParams = {
+        ...modifyParams,
+        _modifyCollection: modifier,
+      };
+
+      await createCollection(createParams);
+
+      sinon.assert.calledWith(modifier, 'create', modifyParams);
+    });
+  });
 });
--- a/yarn.lock
+++ b/yarn.lock
@ -4471,6 +4471,12 @@ intl@^1.2.5:
  version "1.2.5"
  resolved "https://registry.yarnpkg.com/intl/-/intl-1.2.5.tgz#82244a2190c4e419f8371f5aa34daa3420e2abde"

+invariant@2.2.3:
+  version "2.2.3"
+  resolved "https://registry.yarnpkg.com/invariant/-/invariant-2.2.3.tgz#1a827dfde7dcbd7c323f0ca826be8fa7c5e9d688"
+  dependencies:
+    loose-envify "^1.0.0"
+
 invariant@^2.0.0, invariant@^2.2.0, invariant@^2.2.1, invariant@^2.2.2:
  version "2.2.2"
  resolved "https://registry.yarnpkg.com/invariant/-/invariant-2.2.2.tgz#9e1f56ac0acdb6bf303306f338be3b204ae60360"