From cba3fc38e6bf0da2e8461d790317a3fd4780f88e Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Date: Mon, 30 Jul 2018 15:51:45 +0200
Subject: [PATCH] fixed code smells and added some more tests

---
 scm-ui/src/modules/auth.js         |  5 ++++-
 scm-ui/src/modules/failure.js      |  5 ++++-
 scm-ui/src/modules/failure.test.js | 26 ++++++++++++++++++++++++++
 scm-ui/src/modules/pending.js      |  5 ++++-
 scm-ui/src/modules/pending.test.js |  6 ++++++
 scm-ui/src/users/modules/users.js  |  7 +------
 6 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/scm-ui/src/modules/auth.js b/scm-ui/src/modules/auth.js
index d0f8e0a246..74afd8259b 100644
--- a/scm-ui/src/modules/auth.js
+++ b/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:
diff --git a/scm-ui/src/modules/failure.js b/scm-ui/src/modules/failure.js
index 89fc33aa11..9328b59e78 100644
--- a/scm-ui/src/modules/failure.js
+++ b/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);
diff --git a/scm-ui/src/modules/failure.test.js b/scm-ui/src/modules/failure.test.js
index 866546de98..233537fa28 100644
--- a/scm-ui/src/modules/failure.test.js
+++ b/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", () => {
diff --git a/scm-ui/src/modules/pending.js b/scm-ui/src/modules/pending.js
index 04abf65c8c..3d34257f56 100644
--- a/scm-ui/src/modules/pending.js
+++ b/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);
diff --git a/scm-ui/src/modules/pending.test.js b/scm-ui/src/modules/pending.test.js
index a50ed37400..082695257e 100644
--- a/scm-ui/src/modules/pending.test.js
+++ b/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(
       {
diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js
index e1b8280115..afd77021af 100644
--- a/scm-ui/src/users/modules/users.js
+++ b/scm-ui/src/users/modules/users.js
@@ -98,12 +98,7 @@ export function fetchUser(name: string) {
     return apiClient
       .get(userUrl)
       .then(response => {
-        return response;
-      })
-      .then(response => {
-        if (response.ok) {
-          return response.json();
-        }
+        return response.json();
       })
       .then(data => {
         dispatch(fetchUserSuccess(data));