From 4a1bdcc51d5414c890fdeff7667c639b1d4bc07b Mon Sep 17 00:00:00 2001
From: Bob Silverberg <bob.silverberg@gmail.com>
Date: Thu, 3 May 2018 09:49:57 -0400
Subject: [PATCH] Address review comments

---
 src/amo/reducers/recommendations.js           | 21 ++++++++++++-------
 src/amo/sagas/recommendations.js              |  7 ++++---
 tests/unit/amo/api/test_recommendations.js    |  4 +---
 .../unit/amo/reducers/test_recommendations.js |  4 +---
 tests/unit/amo/sagas/test_recommendations.js  |  3 +--
 5 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/amo/reducers/recommendations.js b/src/amo/reducers/recommendations.js
index 0370331170c..0d23d621703 100644
--- a/src/amo/reducers/recommendations.js
+++ b/src/amo/reducers/recommendations.js
@@ -12,17 +12,17 @@ export const LOAD_RECOMMENDATIONS: 'LOAD_RECOMMENDATIONS'
   = 'LOAD_RECOMMENDATIONS';
 
 export type Recommendations = {|
-  addons: Array<ExternalAddonType>,
-  fallbackReason: string,
+  addons: Array<ExternalAddonType> | null,
+  fallbackReason: string | null,
   loading: boolean,
-  outcome: string,
+  outcome: string | null,
 |};
 
-export type RecommendationsState = {
+export type RecommendationsState = {|
   byGuid: {
     [guid: string]: Recommendations,
   },
-};
+|};
 
 export const initialState: RecommendationsState = {
   byGuid: {},
@@ -40,6 +40,7 @@ type AbortFetchRecommendationsAction = {|
 export const abortFetchRecommendations = ({
   guid,
 }: AbortFetchRecommendationsParams = {}): AbortFetchRecommendationsAction => {
+  invariant(guid, 'guid is required');
   return {
     type: ABORT_FETCH_RECOMMENDATIONS,
     payload: { guid },
@@ -110,6 +111,7 @@ export const getRecommendationsByGuid = (
   { guid, state }: GetRecommendationsByGuidParams
 ): Recommendations | null => {
   invariant(guid, 'guid is required');
+  invariant(state, 'state is required');
 
   return state.byGuid[guid] || null;
 };
@@ -117,8 +119,7 @@ export const getRecommendationsByGuid = (
 type Action =
   | AbortFetchRecommendationsAction
   | FetchRecommendationsAction
-  | LoadRecommendationsAction
-  ;
+  | LoadRecommendationsAction;
 
 const reducer = (
   state: RecommendationsState = initialState,
@@ -131,7 +132,10 @@ const reducer = (
         byGuid: {
           ...state.byGuid,
           [action.payload.guid]: {
+            addons: null,
+            fallbackReason: null,
             loading: false,
+            outcome: null,
           },
         },
       };
@@ -142,7 +146,10 @@ const reducer = (
         byGuid: {
           ...state.byGuid,
           [action.payload.guid]: {
+            addons: null,
+            fallbackReason: null,
             loading: true,
+            outcome: null,
           },
         },
       };
diff --git a/src/amo/sagas/recommendations.js b/src/amo/sagas/recommendations.js
index 3d184075832..adf459b1364 100644
--- a/src/amo/sagas/recommendations.js
+++ b/src/amo/sagas/recommendations.js
@@ -27,11 +27,12 @@ export function* fetchRecommendations({
       api: state.api, guid, recommended,
     };
     const recommendations = yield call(api.getRecommendations, params);
-    const { outcome } = recommendations;
+    const { fallback_reason: fallbackReason, outcome, results: addons }
+      = recommendations;
 
     yield put(loadRecommendations({
-      addons: recommendations.results,
-      fallbackReason: recommendations.fallback_reason,
+      addons,
+      fallbackReason,
       guid,
       outcome,
     }));
diff --git a/tests/unit/amo/api/test_recommendations.js b/tests/unit/amo/api/test_recommendations.js
index ad86d3b7be2..006344ba945 100644
--- a/tests/unit/amo/api/test_recommendations.js
+++ b/tests/unit/amo/api/test_recommendations.js
@@ -1,9 +1,7 @@
 import * as api from 'core/api';
 import { getRecommendations } from 'amo/api/recommendations';
 import { createApiResponse } from 'tests/unit/helpers';
-import {
-  dispatchClientMetadata,
-} from 'tests/unit/amo/helpers';
+import { dispatchClientMetadata } from 'tests/unit/amo/helpers';
 
 
 describe(__filename, () => {
diff --git a/tests/unit/amo/reducers/test_recommendations.js b/tests/unit/amo/reducers/test_recommendations.js
index 84c33ab5cf3..885f51b30b5 100644
--- a/tests/unit/amo/reducers/test_recommendations.js
+++ b/tests/unit/amo/reducers/test_recommendations.js
@@ -30,10 +30,9 @@ describe(__filename, () => {
     }));
 
     expect(state.byGuid[guid].loading).toEqual(true);
-    expect(state.byGuid[guid].addons).toEqual(undefined);
+    expect(state.byGuid[guid].addons).toEqual(null);
   });
 
-
   it('loads recommendations', () => {
     const addons = [fakeAddon, fakeAddon];
     const fallbackReason = 'timeout';
@@ -50,7 +49,6 @@ describe(__filename, () => {
 
     const loadedRecommendations = getRecommendationsByGuid({ guid, state });
 
-    expect(loadedRecommendations).not.toEqual(null);
     expect(loadedRecommendations).toEqual({
       addons: expectedAddons,
       fallbackReason,
diff --git a/tests/unit/amo/sagas/test_recommendations.js b/tests/unit/amo/sagas/test_recommendations.js
index 23c6941939d..292666db119 100644
--- a/tests/unit/amo/sagas/test_recommendations.js
+++ b/tests/unit/amo/sagas/test_recommendations.js
@@ -16,15 +16,14 @@ import {
 
 
 describe(__filename, () => {
-  let clientData;
   let errorHandler;
   let mockApi;
   let sagaTester;
 
   beforeEach(() => {
+    const clientData = dispatchClientMetadata();
     errorHandler = createStubErrorHandler();
     mockApi = sinon.mock(recommendationsApi);
-    clientData = dispatchClientMetadata();
     sagaTester = new SagaTester({
       initialState: clientData.state,
       reducers: {