fixed code smells and added some more tests

scm-ui/src/modules/auth.js

@@ -28,7 +28,10 @@ export const LOGOUT_FAILURE = `${LOGOUT}_${types.FAILURE_SUFFIX}`;
 
 const initialState = {};
 
-export default function reducer(state: Object = initialState, action: Object) {
+export default function reducer(
+  state: Object = initialState,
+  action: Object = { type: "UNKNOWN" }
+) {
   switch (action.type) {
     case LOGIN_SUCCESS:
     case FETCH_ME_SUCCESS:

scm-ui/src/modules/failure.js

@@ -23,7 +23,10 @@ function removeFromState(state: Object, identifier: string) {
   return newState;
 }
 
-export default function reducer(state: Object = {}, action: Action): Object {
+export default function reducer(
+  state: Object = {},
+  action: Action = { type: "UNKNOWN" }
+): Object {
   const type = action.type;
   if (type.endsWith(FAILURE_SUFFIX)) {
     const identifier = extractIdentifierFromFailure(action);

scm-ui/src/modules/failure.test.js

@@ -10,6 +10,12 @@ describe("failure reducer", () => {
     expect(newState["FETCH_ITEMS"]).toBe(err);
   });
 
+  it("should do nothing for unknown action types", () => {
+    const state = {};
+    const newState = reducer(state, { type: "UNKNOWN" });
+    expect(newState).toBe(state);
+  });
+
   it("should set the error for FETCH_ITEMS, if payload has multiple values", () => {
     const newState = reducer(
       {},
@@ -52,6 +58,18 @@ describe("failure reducer", () => {
     expect(newState["FETCH_ITEMS"]).toBeFalsy();
   });
 
+  it("should reset FETCH_ITEMS after FETCH_ITEMS_RESET, but should not affect others", () => {
+    const newState = reducer(
+      {
+        FETCH_ITEMS: err,
+        FETCH_USERS: err
+      },
+      { type: "FETCH_ITEMS_RESET" }
+    );
+    expect(newState["FETCH_ITEMS"]).toBeFalsy();
+    expect(newState["FETCH_USERS"]).toBe(err);
+  });
+
   it("should set the error for a single item of FETCH_ITEM", () => {
     const newState = reducer(
       {},
@@ -59,6 +77,14 @@ describe("failure reducer", () => {
     );
     expect(newState["FETCH_ITEM/42"]).toBe(err);
   });
+
+  it("should reset error for a single item of FETCH_ITEM", () => {
+    const newState = reducer(
+      { "FETCH_ITEM/42": err },
+      { type: "FETCH_ITEM_SUCCESS", payload: err, itemId: 42 }
+    );
+    expect(newState["FETCH_ITEM/42"]).toBeUndefined();
+  });
 });
 
 describe("failure selector", () => {

scm-ui/src/modules/pending.js

@@ -28,7 +28,10 @@ function extractIdentifierFromPending(action: Action) {
   return identifier;
 }
 
-export default function reducer(state: Object = {}, action: Action): Object {
+export default function reducer(
+  state: Object = {},
+  action: Action = { type: "UNKNOWN" }
+): Object {
   const type = action.type;
   if (type.endsWith(PENDING_SUFFIX)) {
     const identifier = extractIdentifierFromPending(action);

scm-ui/src/modules/pending.test.js

@@ -6,6 +6,12 @@ describe("pending reducer", () => {
     expect(newState["FETCH_ITEMS"]).toBe(true);
   });
 
+  it("should do nothing for unknown action types", () => {
+    const state = {};
+    const newState = reducer(state, { type: "UNKNOWN" });
+    expect(newState).toBe(state);
+  });
+
   it("should set pending for FETCH_ITEMS to true, but should not affect others", () => {
     const newState = reducer(
       {

scm-ui/src/users/modules/users.js

@@ -98,12 +98,7 @@ export function fetchUser(name: string) {
     return apiClient
       .get(userUrl)
       .then(response => {
-        return response;
+        return response.json();
-      })
-      .then(response => {
-        if (response.ok) {
-          return response.json();
-        }
       })
       .then(data => {
         dispatch(fetchUserSuccess(data));