From db84db990f47bc4f496f100f980111d4b8a78218 Mon Sep 17 00:00:00 2001
From: Matthew Riley MacPherson <matt@lonelyvegan.com>
Date: Thu, 10 Nov 2016 00:33:33 +0000
Subject: [PATCH] Improve reducers and actions

---
 config/default.js                             |   7 +-
 src/amo/components/Categories.js              |  43 +++---
 src/amo/containers/CategoriesPage.js          |  12 +-
 src/amo/containers/CategoryAddons.js          |  25 +++
 src/core/actions/categories.js                |   4 +-
 src/core/reducers/categories.js               |  46 +++++-
 .../client/amo/components/TestCategeories.js  |  21 +--
 .../amo/containers/TestCategoriesPage.js      |  12 +-
 tests/client/core/actions/test_categories.js  |   4 +-
 tests/client/core/reducers/test_categories.js | 145 ++++++++++++++++--
 10 files changed, 248 insertions(+), 71 deletions(-)
 create mode 100644 src/amo/containers/CategoryAddons.js

diff --git a/config/default.js b/config/default.js
index 8c8c158c92f..b979c5516e6 100644
--- a/config/default.js
+++ b/config/default.js
@@ -81,6 +81,7 @@ module.exports = {
     'trackingEnabled',
     'trackingId',
     'trackingSendInitPageView',
+    'validClientApplications',
   ],
 
   // Content Security Policy.
@@ -158,11 +159,11 @@ module.exports = {
 
   enablePostCssLoader: true,
 
-  // The list of valid client application names. These are derived from UA strings when
-  // not supplied in the URL.
+  // The list of valid client application names.
+  // These are derived from UA strings when not supplied in the URL.
   validClientApplications: [
-    'firefox',
     'android',
+    'firefox',
   ],
 
   // The default app used in the URL.
diff --git a/src/amo/components/Categories.js b/src/amo/components/Categories.js
index e947198e74b..827827069eb 100644
--- a/src/amo/components/Categories.js
+++ b/src/amo/components/Categories.js
@@ -7,54 +7,51 @@ import translate from 'core/i18n/translate';
 import './Categories.scss';
 
 
-export function filterAndSortCategories(categories, addonType, clientApp) {
-  return categories.filter((category) => {
-    return category.type === addonType && category.application === clientApp;
-  }).sort((a, b) => {
-    return a.name > b.name;
-  });
-}
-
 export class CategoriesBase extends React.Component {
   static propTypes = {
     addonType: PropTypes.string.isRequired,
     categories: PropTypes.arrayOf(PropTypes.object),
-    clientApp: PropTypes.string.isRequired,
     error: PropTypes.bool,
     loading: PropTypes.bool.isRequired,
     i18n: PropTypes.object.isRequired,
   }
 
   render() {
-    const {
-      addonType, categories, clientApp, error, loading, i18n,
-    } = this.props;
+    const { addonType, categories, error, loading, i18n } = this.props;
 
-    if (loading && !categories.length) {
-      return <div>{i18n.gettext('Loading...')}</div>;
+    if (loading) {
+      return (
+        <div className="Categories">
+          <p>{i18n.gettext('Loading...')}</p>
+        </div>
+      );
     }
 
     if (error) {
-      return <div>{i18n.gettext('Failed to load categories.')}</div>;
+      return (
+        <div className="Categories">
+          <p>{i18n.gettext('Failed to load categories.')}</p>
+        </div>
+      );
     }
 
-    const categoriesToShow = filterAndSortCategories(
-      categories, addonType, clientApp);
-
-    if (!loading && !categoriesToShow.length) {
-      return <div>{i18n.gettext('No categories found.')}</div>;
+    if (!Object.values(categories).length) {
+      return (
+        <div className="Categories">
+          <p>{i18n.gettext('No categories found.')}</p>
+        </div>
+      );
     }
 
     return (
       <div className="Categories">
         <ul className="Categories-list"
           ref={(ref) => { this.categories = ref; }}>
-          {categoriesToShow.map((category) => {
-            const queryParams = { category: category.slug, type: addonType };
+          {Object.values(categories).map((category) => {
             return (
               <li className="Categories-list-item">
                 <Link className="Categories-link"
-                  to={{ pathname: '/search/', query: queryParams }}>
+                  to={`/${addonType}s/${category.slug}/`}>
                   {category.name}
                 </Link>
               </li>
diff --git a/src/amo/containers/CategoriesPage.js b/src/amo/containers/CategoriesPage.js
index f4703365203..2e2362f363f 100644
--- a/src/amo/containers/CategoriesPage.js
+++ b/src/amo/containers/CategoriesPage.js
@@ -7,10 +7,16 @@ import { loadCategoriesIfNeeded } from 'core/utils';
 
 
 export function mapStateToProps(state, ownProps) {
+  const addonType = ownProps.params.addonType.replace(/s$/, '');
+  const clientApp = state.api.clientApp;
+  const categories = state.categories.categories[clientApp][addonType] ?
+    state.categories.categories[clientApp][addonType] : {};
+
   return {
-    addonType: ownProps.params.addonType.replace(/s$/, ''),
-    clientApp: state.api.clientApp,
-    ...state.categories,
+    addonType,
+    categories,
+    error: state.categories.error,
+    loading: state.categories.loading,
   };
 }
 
diff --git a/src/amo/containers/CategoryAddons.js b/src/amo/containers/CategoryAddons.js
new file mode 100644
index 00000000000..0f1e115a743
--- /dev/null
+++ b/src/amo/containers/CategoryAddons.js
@@ -0,0 +1,25 @@
+import { connect } from 'react-redux';
+import { asyncConnect } from 'redux-connect';
+import { compose } from 'redux';
+
+import SearchPage from 'amo/components/SearchPage';
+import {
+  loadSearchResultsIfNeeded,
+  mapStateToProps,
+} from 'core/containers/SearchPage';
+import { loadCategoriesIfNeeded } from 'core/utils';
+
+
+export default compose(
+  asyncConnect([
+    {
+      deferred: true,
+      promise: loadSearchResultsIfNeeded,
+    },
+    {
+      deferred: true,
+      promise: loadCategoriesIfNeeded,
+    },
+  ]),
+  connect(mapStateToProps)
+)(SearchPage);
diff --git a/src/core/actions/categories.js b/src/core/actions/categories.js
index 33ef47a77bd..45ebd06a0d9 100644
--- a/src/core/actions/categories.js
+++ b/src/core/actions/categories.js
@@ -16,9 +16,7 @@ export function categoriesLoad({ result }) {
     type: CATEGORIES_LOAD,
     payload: {
       loading: false,
-      results: Object.keys(result).map((key) => {
-        return result[key];
-      }),
+      result,
     },
   };
 }
diff --git a/src/core/reducers/categories.js b/src/core/reducers/categories.js
index bc8c749c43d..1f713bee2ac 100644
--- a/src/core/reducers/categories.js
+++ b/src/core/reducers/categories.js
@@ -1,3 +1,4 @@
+import config from 'config';
 import {
   CATEGORIES_GET,
   CATEGORIES_LOAD,
@@ -5,8 +6,16 @@ import {
 } from 'core/constants';
 
 
+export function emptyCategoryList() {
+  let categories = {};
+  config.get('validClientApplications').forEach((appName) => {
+    categories[appName] = {};
+  });
+  return categories;
+}
+
 const initialState = {
-  categories: [],
+  categories: emptyCategoryList(),
   error: false,
   loading: false,
 };
@@ -16,13 +25,44 @@ export default function categories(state = initialState, action) {
 
   switch (action.type) {
     case CATEGORIES_GET:
-      return { ...state, ...payload, loading: true, categories: [] };
+      return { ...state, ...payload, loading: true };
     case CATEGORIES_LOAD:
+      let categories = emptyCategoryList();
+      payload.result.forEach((result) => {
+        // If the API returns data for an application we don't support,
+        // we'll ignore it for now.
+        if (!categories[result.application]) {
+          return;
+        }
+
+        if (!categories[result.application][result.type]) {
+          categories[result.application][result.type] = [];
+        }
+
+        categories[result.application][result.type].push({
+          ...result,
+          // count: 0,
+          // page: null,
+          // results: [],
+        });
+      });
+
+      config.get('validClientApplications').forEach((appName) => {
+        Object.keys(categories[appName]).forEach((addonType) => {
+          categories[appName][addonType] = categories[appName][addonType]
+            .sort((a, b) => a.name > b.name)
+            .reduce((object, value, index) => {
+              object[value.slug] = value;
+              return object;
+            }, {});
+        });
+      });
+
       return {
         ...state,
         ...payload,
         loading: false,
-        categories: payload.results,
+        categories,
       };
     case CATEGORIES_FAILED:
       return { ...initialState, ...payload, error: true };
diff --git a/tests/client/amo/components/TestCategeories.js b/tests/client/amo/components/TestCategeories.js
index 6353d4225c1..e8900b1736c 100644
--- a/tests/client/amo/components/TestCategeories.js
+++ b/tests/client/amo/components/TestCategeories.js
@@ -7,7 +7,7 @@ import {
 import { Provider } from 'react-redux';
 
 import createStore from 'amo/store';
-import Categories, { filterAndSortCategories } from 'amo/components/Categories';
+import Categories from 'amo/components/Categories';
 import { getFakeI18nInst } from 'tests/client/helpers';
 
 
@@ -99,22 +99,3 @@ describe('Categories', () => {
     assert.equal(root.textContent, 'Failed to load categories.');
   });
 });
-
-describe('filterAndSortCategories', () => {
-  it('filters out categories by addonType and clientApp', () => {
-    assert.lengthOf(
-      filterAndSortCategories(categories, 'extension', 'firefox'), 1);
-    assert.lengthOf(
-      filterAndSortCategories(categories, 'extension', 'android'), 2);
-    assert.lengthOf(
-      filterAndSortCategories(categories, 'theme', 'android'), 1);
-  });
-
-  it('sorts categories by name and weight', () => {
-    const sortedCategories = filterAndSortCategories(
-      categories, 'extension', 'android');
-
-    assert.equal(sortedCategories[0].slug, 'Games');
-    assert.equal(sortedCategories[1].slug, 'travel');
-  });
-});
diff --git a/tests/client/amo/containers/TestCategoriesPage.js b/tests/client/amo/containers/TestCategoriesPage.js
index 8721c8515d6..299498f93e4 100644
--- a/tests/client/amo/containers/TestCategoriesPage.js
+++ b/tests/client/amo/containers/TestCategoriesPage.js
@@ -5,15 +5,19 @@ describe('<CategoriesPage />', () => {
   it('maps state to props', () => {
     const props = mapStateToProps({
       api: { clientApp: 'android', lang: 'pt' },
-      categories: { categories: [], loading: true },
+      categories: {
+        categories: { android: {}, firefox: {} },
+        error: false,
+        loading: true,
+      },
     }, {
-      params: { addonType: 'theme' },
+      params: { addonType: 'themes' },
     });
 
     assert.deepEqual(props, {
       addonType: 'theme',
-      categories: [],
-      clientApp: 'android',
+      categories: {},
+      error: false,
       loading: true,
     });
   });
diff --git a/tests/client/core/actions/test_categories.js b/tests/client/core/actions/test_categories.js
index 8df28f8e959..c9119f0f02c 100644
--- a/tests/client/core/actions/test_categories.js
+++ b/tests/client/core/actions/test_categories.js
@@ -17,7 +17,7 @@ describe('CATEGORIES_GET', () => {
 
 describe('CATEGORIES_LOAD', () => {
   const params = {
-    result: { 0: 'foo', 1: 'bar' },
+    result: ['foo', 'bar'],
     loading: false,
   };
   const action = actions.categoriesLoad(params);
@@ -28,7 +28,7 @@ describe('CATEGORIES_LOAD', () => {
 
   it('sets the payload', () => {
     assert.deepEqual(action.payload.loading, false);
-    assert.deepEqual(action.payload.results, ['foo', 'bar']);
+    assert.deepEqual(action.payload.result, ['foo', 'bar']);
   });
 });
 
diff --git a/tests/client/core/reducers/test_categories.js b/tests/client/core/reducers/test_categories.js
index 48943771506..058df397eee 100644
--- a/tests/client/core/reducers/test_categories.js
+++ b/tests/client/core/reducers/test_categories.js
@@ -1,11 +1,15 @@
-import categories from 'core/reducers/categories';
+import categories, { emptyCategoryList } from 'core/reducers/categories';
 
 describe('categories reducer', () => {
-  const initialState = { categories: [], error: false, loading: true };
+  const initialState = {
+    categories: emptyCategoryList(), error: false, loading: true,
+  };
 
   it('defaults to an empty set of categories', () => {
-    const state = categories(undefined, { type: 'unrelated' });
-    assert.deepEqual(state.categories, []);
+    const state = categories(initialState, { type: 'unrelated' });
+    assert.deepEqual(state.categories, {
+      android: {}, firefox: {},
+    });
   });
 
   it('defaults to not loading', () => {
@@ -22,21 +26,139 @@ describe('categories reducer', () => {
     it('sets loading', () => {
       const state = categories(initialState,
         { type: 'CATEGORIES_GET', payload: { loading: true } });
-      assert.deepEqual(state.categories, []);
+      assert.deepEqual(state.categories, {
+        android: {}, firefox: {},
+      });
       assert.equal(state.error, false);
       assert.equal(state.loading, true);
     });
   });
 
   describe('CATEGORIES_LOAD', () => {
-    const results = ['foo', 'bar'];
+    const result = [
+      {
+        application: 'android',
+        name: 'Alerts & Update',
+        slug: 'alert-update',
+        type: 'extension',
+      },
+      {
+        application: 'android',
+        name: 'Games',
+        slug: 'Games',
+        type: 'extension',
+      },
+      {
+        application: 'android',
+        name: 'Blogging',
+        slug: 'blogging',
+        type: 'extension',
+      },
+      {
+        application: 'android',
+        name: 'Naturé',
+        slug: 'naturé',
+        type: 'theme',
+      },
+      {
+        application: 'android',
+        name: 'Painting',
+        slug: 'painting',
+        type: 'theme',
+      },
+      {
+        application: 'firefox',
+        name: 'Anime',
+        slug: 'anime',
+        type: 'theme',
+      },
+      {
+        application: 'firefox',
+        name: 'Alerts & Update',
+        slug: 'alert-update',
+        type: 'extension',
+      },
+      {
+        application: 'firefox',
+        name: 'Security',
+        slug: 'security',
+        type: 'extension',
+      },
+      {
+        application: 'netscape',
+        name: 'I should not appear',
+        slug: 'i-should-not-appear',
+        type: 'extension',
+      },
+    ];
     const state = categories(initialState, {
       type: 'CATEGORIES_LOAD',
-      payload: { results },
+      payload: { result },
     });
 
     it('sets the categories', () => {
-      assert.deepEqual(state.categories, ['foo', 'bar']);
+      assert.deepEqual(state.categories, {
+        firefox: {
+          extension: {
+            'alert-update': {
+              application: 'firefox',
+              name: 'Alerts & Update',
+              slug: 'alert-update',
+              type: 'extension',
+            },
+            security: {
+              application: 'firefox',
+              name: 'Security',
+              slug: 'security',
+              type: 'extension',
+            },
+          },
+          theme: {
+            anime: {
+              application: 'firefox',
+              name: 'Anime',
+              slug: 'anime',
+              type: 'theme',
+            },
+          },
+        },
+        android: {
+          extension: {
+            'alert-update': {
+              application: 'android',
+              name: 'Alerts & Update',
+              slug: 'alert-update',
+              type: 'extension',
+            },
+            blogging: {
+              application: 'android',
+              name: 'Blogging',
+              slug: 'blogging',
+              type: 'extension',
+            },
+            Games: {
+              application: 'android',
+              name: 'Games',
+              slug: 'Games',
+              type: 'extension',
+            },
+          },
+          theme: {
+            'naturé': {
+              application: 'android',
+              name: 'Naturé',
+              slug: 'naturé',
+              type: 'theme',
+            },
+            painting: {
+              application: 'android',
+              name: 'Painting',
+              slug: 'painting',
+              type: 'theme',
+            },
+          },
+        },
+      });
     });
 
     it('sets loading', () => {
@@ -56,8 +178,11 @@ describe('categories reducer', () => {
       const loading = false;
 
       const state = categories(initialState, {
-        type: 'CATEGORIES_FAILED', payload: { error, loading } });
-      assert.deepEqual(state, { categories: [], error, loading });
+        type: 'CATEGORIES_FAILED', payload: { error, loading },
+      });
+      assert.deepEqual(state, {
+        categories: emptyCategoryList(), error, loading,
+      });
     });
   });
 });