Split user reducer

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

@@ -2,7 +2,7 @@
 import { apiClient } from "../../apiclient";
 import type { User } from "../types/User";
 import type { UserEntry } from "../types/UserEntry";
-import { Dispatch } from "redux";
+import { Dispatch, combineReducers } from "redux";
 import type { Action } from "../../types/Action";
 
 export const FETCH_USERS_PENDING = "scm/users/FETCH_USERS_PENDING";
@@ -21,7 +21,7 @@ export const MODIFY_USER_PENDING = "scm/users/MODIFY_USER_PENDING";
 export const MODIFY_USER_SUCCESS = "scm/users/MODIFY_USER_SUCCESS";
 export const MODIFY_USER_FAILURE = "scm/users/MODIFY_USER_FAILURE";
 
-export const DELETE_USER = "scm/users/DELETE";
+export const DELETE_USER_PENDING = "scm/users/DELETE_PENDING";
 export const DELETE_USER_SUCCESS = "scm/users/DELETE_SUCCESS";
 export const DELETE_USER_FAILURE = "scm/users/DELETE_FAILURE";
 
@@ -232,7 +232,7 @@ export function deleteUser(user: User) {
 
 export function deleteUserPending(user: User): Action {
   return {
-    type: DELETE_USER,
+    type: DELETE_USER_PENDING,
     payload: user
   };
 }
@@ -291,6 +291,7 @@ function extractUsersByNames(
   }
   return usersByNames;
 }
+
 function deleteUserInUsersByNames(users: {}, userName: string) {
   let newUsers = {};
   for (let username in users) {
@@ -309,129 +310,82 @@ function deleteUserInEntries(users: [], userName: string) {
 
 const reducerByName = (state: any, username: string, newUserState: any) => {
   const newUsersByNames = {
-    ...state.byNames,
+    ...state,
     [username]: newUserState
   };
 
-  return {
+  return newUsersByNames;
-    ...state,
-    byNames: newUsersByNames
-  };
 };
 
-export default function reducer(state: any = {}, action: any = {}) {
+function listReducer(state: any = {}, action: any = {}) {
   switch (action.type) {
-    // fetch user list cases
+    // Fetch all users actions
     case FETCH_USERS_PENDING:
       return {
         ...state,
-        list: {
+        loading: true
-          loading: true
-        }
       };
     case FETCH_USERS_SUCCESS:
-      // return red(state, action.payload._embedded.users);
       const users = action.payload._embedded.users;
       const userNames = users.map(user => user.name);
-      const byNames = extractUsersByNames(users, userNames, state.byNames);
       return {
         ...state,
-        userCreatePermission: action.payload._links.create ? true : false,
+        error: null,
-        list: {
+        entries: userNames,
-          error: null,
+        loading: false,
-          entries: userNames,
+        userCreatePermission: action.payload._links.create ? true : false
-          loading: false,
-          userCreatePermission: action.payload._links.create ? true : false
-        },
-        byNames
       };
     case FETCH_USERS_FAILURE:
       return {
         ...state,
-        list: {
+        loading: false,
-          ...state.users,
+        error: action.payload.error
-          loading: false,
-          error: action.payload.error
-        }
       };
-    // Fetch single user cases
+    // Delete single user actions
+    case DELETE_USER_SUCCESS:
+      const newUserEntries = deleteUserInEntries(
+        state.entries,
+        action.payload.name
+      );
+      return {
+        ...state,
+        entries: newUserEntries
+      };
+    default:
+      return state;
+  }
+}
+
+function byNamesReducer(state: any = {}, action: any = {}) {
+  switch (action.type) {
+    // Fetch all users actions
+    case FETCH_USERS_SUCCESS:
+      const users = action.payload._embedded.users;
+      const userNames = users.map(user => user.name);
+      const byNames = extractUsersByNames(users, userNames, state.byNames);
+      return {
+        ...byNames
+      };
+
+    // Fetch single user actions
     case FETCH_USER_PENDING:
       return reducerByName(state, action.payload.name, {
         loading: true,
         error: null
       });
-
     case FETCH_USER_SUCCESS:
       return reducerByName(state, action.payload.name, {
         loading: false,
         error: null,
         entry: action.payload
       });
-
     case FETCH_USER_FAILURE:
       return reducerByName(state, action.payload.username, {
         loading: false,
         error: action.payload.error
       });
 
-    // Delete single user cases
+    // Update single user actions
-    case DELETE_USER:
-      return reducerByName(state, action.payload.name, {
-        loading: true,
-        error: null,
-        entry: action.payload
-      });
-
-    case DELETE_USER_SUCCESS:
-      const newUserByNames = deleteUserInUsersByNames(
-        state.byNames,
-        action.payload.name
-      );
-      const newUserEntries = deleteUserInEntries(
-        state.list.entries,
-        action.payload.name
-      );
-      return {
-        ...state,
-        list: {
-          ...state.list,
-          entries: newUserEntries
-        },
-        byNames: newUserByNames
-      };
-
-    case DELETE_USER_FAILURE:
-      return reducerByName(state, action.payload.user.name, {
-        loading: false,
-        error: action.payload.error,
-        entry: action.payload.user
-      });
-
-    // Add single user cases
-    case CREATE_USER_PENDING:
-      return {
-        ...state,
-        create: {
-          loading: true
-        }
-      };
-    case CREATE_USER_SUCCESS:
-      return {
-        ...state,
-        create: {
-          loading: false
-        }
-      };
-    case CREATE_USER_FAILURE:
-      return {
-        ...state,
-        create: {
-          loading: false,
-          error: action.payload
-        }
-      };
-
-    // Update single user cases
     case MODIFY_USER_PENDING:
       return reducerByName(state, action.payload.name, {
         loading: true
@@ -444,7 +398,57 @@ export default function reducer(state: any = {}, action: any = {}) {
       return reducerByName(state, action.payload.user.name, {
         error: action.payload.error
       });
+
+    // Delete single user actions
+    case DELETE_USER_PENDING:
+      return reducerByName(state, action.payload.name, {
+        loading: true,
+        error: null,
+        entry: action.payload
+      });
+    case DELETE_USER_SUCCESS:
+      const newUserByNames = deleteUserInUsersByNames(
+        state,
+        action.payload.name
+      );
+      return newUserByNames;
+
+    case DELETE_USER_FAILURE:
+      return reducerByName(state, action.payload.user.name, {
+        loading: false,
+        error: action.payload.error,
+        entry: action.payload.user
+      });
     default:
       return state;
   }
 }
+
+function createReducer(state: any = {}, action: any = {}) {
+  switch (action.type) {
+    case CREATE_USER_PENDING:
+      return {
+        ...state,
+        loading: true
+      };
+    case CREATE_USER_SUCCESS:
+      return {
+        ...state,
+        loading: false
+      };
+    case CREATE_USER_FAILURE:
+      return {
+        ...state,
+        loading: false,
+        error: action.payload
+      };
+    default:
+      return state;
+  }
+}
+
+export default combineReducers({
+  list: listReducer,
+  byNames: byNamesReducer,
+  create: createReducer
+});

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

@@ -18,7 +18,7 @@ import {
   MODIFY_USER_SUCCESS,
   deleteUserPending,
   deleteUserFailure,
-  DELETE_USER,
+  DELETE_USER_PENDING,
   DELETE_USER_SUCCESS,
   DELETE_USER_FAILURE,
   deleteUser,
@@ -270,7 +270,7 @@ describe("users fetch()", () => {
     const store = mockStore({});
     return store.dispatch(deleteUser(userZaphod)).then(() => {
       const actions = store.getActions();
-      expect(actions[0].type).toEqual(DELETE_USER);
+      expect(actions[0].type).toEqual(DELETE_USER_PENDING);
       expect(actions[0].payload).toBe(userZaphod);
       expect(actions[1].type).toEqual(DELETE_USER_SUCCESS);
     });
@@ -284,7 +284,7 @@ describe("users fetch()", () => {
     const store = mockStore({});
     return store.dispatch(deleteUser(userZaphod)).then(() => {
       const actions = store.getActions();
-      expect(actions[0].type).toEqual(DELETE_USER);
+      expect(actions[0].type).toEqual(DELETE_USER_PENDING);
       expect(actions[0].payload).toBe(userZaphod);
       expect(actions[1].type).toEqual(DELETE_USER_FAILURE);
       expect(actions[1].payload).toBeDefined();
@@ -318,12 +318,46 @@ describe("users reducer", () => {
       }
     });
 
+    it("should set error when fetching users failed", () => {
+      const oldState = {
+        list: {
+          loading: true
+        }
+      };
+
+      const error = new Error("kaputt");
+
+      const newState = reducer(oldState, fetchUsersFailure("url.com", error));
+      expect(newState.list.loading).toBeFalsy();
+      expect(newState.list.error).toEqual(error);
+    });
+
+    it("should set userCreatePermission to true if update link is present", () => {
+      const newState = reducer({}, fetchUsersSuccess(responseBody));
+
+      expect(newState.list.userCreatePermission).toBeTruthy();
+    });
+
     expect(newState.list.userCreatePermission).toBeTruthy();
   });
 
+  it("should not replace whole byNames map when fetching users", () => {
+    const oldState = {
+      byNames: {
+        ford: {
+          entry: userFord
+        }
+      }
+    };
+
+    const newState = reducer(oldState, fetchUsersSuccess(responseBody));
+    expect(newState.byNames["zaphod"]).toBeDefined();
+    expect(newState.byNames["ford"]).toBeDefined();
+  });
+
   test("should update state correctly according to DELETE_USER_PENDING action", () => {
     const state = {
-      usersByNames: {
+      byNames: {
         zaphod: {
           loading: false,
           error: null,
@@ -359,7 +393,7 @@ describe("users reducer", () => {
 
   it("should set the error of user which could not be deleted", () => {
     const state = {
-      usersByNames: {
+      byNames: {
         zaphod: {
           loading: true,
           entry: userZaphod
@@ -419,41 +453,6 @@ describe("users reducer", () => {
     expect(newState.byNames["ford"]).toBeFalsy();
   });
 
-  it("should not replace whole byNames map when fetching users", () => {
-    const oldState = {
-      byNames: {
-        ford: {
-          entry: userFord
-        }
-      }
-    };
-
-    const newState = reducer(oldState, fetchUsersSuccess(responseBody));
-    expect(newState.byNames["zaphod"]).toBeDefined();
-    expect(newState.byNames["ford"]).toBeDefined();
-  });
-
-  it("should set error when fetching users failed", () => {
-    const oldState = {
-      list: {
-        loading: true
-      }
-    };
-
-    const error = new Error("kaputt");
-
-    const newState = reducer(oldState, fetchUsersFailure("url.com", error));
-
-    expect(newState.list.loading).toBeFalsy();
-    expect(newState.list.error).toEqual(error);
-  });
-
-  it("should set userCreatePermission to true if update link is present", () => {
-    const newState = reducer({}, fetchUsersSuccess(responseBody));
-
-    expect(newState.list.userCreatePermission).toBeTruthy();
-  });
-
   it("should update state correctly according to CREATE_USER_PENDING action", () => {
     const newState = reducer({}, createUserPending(userZaphod));
     expect(newState.create.loading).toBeTruthy();
@@ -461,14 +460,17 @@ describe("users reducer", () => {
   });
 
   it("should update state correctly according to CREATE_USER_SUCCESS action", () => {
-    const newState = reducer({ loading: true }, createUserSuccess());
+    const newState = reducer(
+      { create: { loading: true } },
+      createUserSuccess()
+    );
     expect(newState.create.loading).toBeFalsy();
     expect(newState.create.error).toBeFalsy();
   });
 
   it("should set the loading to false and the error if user could not be created", () => {
     const newState = reducer(
-      { loading: true, error: null },
+      { create: { loading: true, error: null } },
       createUserFailure(userFord, new Error("kaputt kaputt"))
     );
     expect(newState.create.loading).toBeFalsy();
@@ -480,17 +482,17 @@ describe("users reducer", () => {
     expect(newState.byNames["zaphod"].loading).toBeTruthy();
   });
 
-  it("should not affect users state", () => {
+  it("should not affect list state", () => {
     const newState = reducer(
       {
-        users: {
+        list: {
           entries: ["ford"]
         }
       },
       fetchUserPending("zaphod")
     );
     expect(newState.byNames["zaphod"].loading).toBeTruthy();
-    expect(newState.users.entries).toEqual(["ford"]);
+    expect(newState.list.entries).toEqual(["ford"]);
   });
 
   it("should update state according to FETCH_USER_FAILURE action", () => {
@@ -523,8 +525,12 @@ describe("users reducer", () => {
   it("should update state according to MODIFY_USER_PENDING action", () => {
     const newState = reducer(
       {
-        error: new Error("something"),
+        byNames: {
-        entry: {}
+          ford: {
+            error: new Error("something"),
+            entry: {}
+          }
+        }
       },
       modifyUserPending(userFord)
     );
@@ -536,9 +542,13 @@ describe("users reducer", () => {
   it("should update state according to MODIFY_USER_SUCCESS action", () => {
     const newState = reducer(
       {
-        loading: true,
+        byNames: {
-        error: new Error("something"),
+          ford: {
-        entry: {}
+            loading: true,
+            error: new Error("something"),
+            entry: {}
+          }
+        }
       },
       modifyUserSuccess(userFord)
     );
@@ -551,8 +561,12 @@ describe("users reducer", () => {
     const error = new Error("something went wrong");
     const newState = reducer(
       {
-        loading: true,
+        byNames: {
-        entry: {}
+          ford: {
+            loading: true,
+            entry: {}
+          }
+        }
       },
       modifyUserFailure(userFord, error)
     );