fixed users reducer and seperate create state

scm-ui/src/users/containers/AddUser.js

@@ -50,10 +50,8 @@ const mapDispatchToProps = dispatch => {
 };
 
 const mapStateToProps = (state, ownProps) => {
-  if (state.users && state.users.users) {
-    return {
-      loading: state.users.users.loading
-    };
+  if (state.users && state.users.create) {
+    return state.users.create;
   }
   return {};
 };

scm-ui/src/users/containers/SingleUser.js

@@ -91,8 +91,8 @@ class SingleUser extends React.Component<Props> {
 const mapStateToProps = (state, ownProps) => {
   const name = ownProps.match.params.name;
   let userEntry;
-  if (state.users && state.users.usersByNames) {
-    userEntry = state.users.usersByNames[name];
+  if (state.users && state.users.byNames) {
+    userEntry = state.users.byNames[name];
   }
 
   return {

scm-ui/src/users/containers/Users.js

@@ -58,10 +58,10 @@ const mapStateToProps = state => {
   let error = null;
   let loading = false;
   let canAddUsers = false;
-  if (state.users && state.users.users) {
-    error = state.users.users.error;
-    canAddUsers = state.users.users.userCreatePermission;
-    loading = state.users.users.loading;
+  if (state.users && state.users.list) {
+    error = state.users.list.error;
+    canAddUsers = state.users.list.userCreatePermission;
+    loading = state.users.list.loading;
   }
   return {
     userEntries,

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

@@ -188,21 +188,17 @@ export function modifyUser(user: User) {
   };
 }
 
-function modifyUserPending(user: User): Action {
+export function modifyUserPending(user: User): Action {
   return {
     type: MODIFY_USER_PENDING,
-    payload: {
-      user
-    }
+    payload: user
   };
 }
 
-function modifyUserSuccess(user: User): Action {
+export function modifyUserSuccess(user: User): Action {
   return {
     type: MODIFY_USER_SUCCESS,
-    payload: {
-      user
-    }
+    payload: user
   };
 }
 
@@ -262,10 +258,10 @@ export function deleteUserFailure(user: User, error: Error): Action {
 //helper functions
 
 export function getUsersFromState(state: any) {
-  if (!state.users.users) {
+  if (!state.users.list) {
     return null;
   }
-  const userNames = state.users.users.entries;
+  const userNames = state.users.list.entries;
   if (!userNames) {
     return null;
   }
@@ -312,11 +308,7 @@ function deleteUserInEntries(users: [], userName: any) {
   return newUsers;
 }
 
-const reduceUsersByNames = (
-  state: any,
-  username: string,
-  newUserState: any
-) => {
+const reducerByName = (state: any, username: string, newUserState: any) => {
   const newUsersByNames = {
     ...state.byNames,
     [username]: newUserState
@@ -334,22 +326,21 @@ export default function reducer(state: any = {}, action: any = {}) {
     case FETCH_USERS_PENDING:
       return {
         ...state,
-        users: {
+        list: {
           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,
         list: {
           error: null,
           entries: userNames,
-          loading: false
+          loading: false,
+          userCreatePermission: action.payload._links.create ? true : false
         },
         byNames
       };
@@ -364,50 +355,50 @@ export default function reducer(state: any = {}, action: any = {}) {
       };
     // Fetch single user cases
     case FETCH_USER_PENDING:
-      return reduceUsersByNames(state, action.payload.name, {
+      return reducerByName(state, action.payload.name, {
         loading: true,
         error: null
       });
 
     case FETCH_USER_SUCCESS:
-      return reduceUsersByNames(state, action.payload.name, {
+      return reducerByName(state, action.payload.name, {
         loading: false,
         error: null,
         entry: action.payload
       });
 
     case FETCH_USER_FAILURE:
-      return reduceUsersByNames(state, action.payload.username, {
+      return reducerByName(state, action.payload.username, {
         loading: false,
         error: action.payload.error
       });
 
     // Delete single user cases
     case DELETE_USER:
-      return reduceUsersByNames(state, action.payload.name, {
+      return reducerByName(state, action.payload.name, {
         loading: true,
         error: null,
         entry: action.payload
       });
 
     case DELETE_USER_SUCCESS:
-      const newUserByNames = deleteUserInUsersByNames(state.usersByNames, [
+      const newUserByNames = deleteUserInUsersByNames(state.byNames, [
         action.payload.name
       ]);
-      const newUserEntries = deleteUserInEntries(state.users.entries, [
+      const newUserEntries = deleteUserInEntries(state.list.entries, [
         action.payload.name
       ]);
       return {
         ...state,
-        users: {
-          ...state.users,
+        list: {
+          ...state.list,
           entries: newUserEntries
         },
-        usersByNames: newUserByNames
+        byNames: newUserByNames
       };
 
     case DELETE_USER_FAILURE:
-      return reduceUsersByNames(state, action.payload.user.name, {
+      return reducerByName(state, action.payload.user.name, {
         loading: false,
         error: action.payload.error,
         entry: action.payload.user
@@ -417,55 +408,39 @@ export default function reducer(state: any = {}, action: any = {}) {
     case CREATE_USER_PENDING:
       return {
         ...state,
-        list: {
-          ...state.list,
-          loading: true,
-          error: null
+        create: {
+          loading: true
         }
       };
     case CREATE_USER_SUCCESS:
       return {
         ...state,
-        list: {
-          ...state.users,
-          loading: false,
-          error: null
+        create: {
+          loading: false
         }
       };
     case CREATE_USER_FAILURE:
       return {
         ...state,
-        list: {
-          ...state.users,
+        create: {
           loading: false,
           error: action.payload
         }
       };
+
     // Update single user cases
     case MODIFY_USER_PENDING:
-      return {
-        ...state,
-        usersByNames: {
-          ...state.usersByNames,
-          [action.user.name]: {
-            loading: true,
-            error: null,
-            entry: action.user
-          }
-        }
-      };
+      return reducerByName(state, action.payload.name, {
+        loading: true
+      });
     case MODIFY_USER_SUCCESS:
-      return {
-        ...state,
-        usersByNames: {
-          ...state.usersByNames,
-          [action.user.name]: {
-            loading: false,
-            error: null,
-            entry: action.user
-          }
-        }
-      };
+      return reducerByName(state, action.payload.name, {
+        entry: action.payload
+      });
+    case MODIFY_USER_FAILURE:
+      return reducerByName(state, action.payload.user.name, {
+        error: action.payload.error
+      });
     default:
       return state;
   }

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

@@ -31,6 +31,8 @@ import {
   createUser,
   createUserSuccess,
   createUserFailure,
+  modifyUserPending,
+  modifyUserSuccess,
   modifyUserFailure,
   fetchUserSuccess,
   deleteUserSuccess,
@@ -317,8 +319,8 @@ describe("users fetch()", () => {
 describe("users reducer", () => {
   it("should update state correctly according to FETCH_USERS_PENDING action", () => {
     const newState = reducer({}, fetchUsersPending());
-    expect(newState.users.loading).toBeTruthy();
-    expect(newState.users.error).toBeFalsy();
+    expect(newState.list.loading).toBeTruthy();
+    expect(newState.list.error).toBeFalsy();
   });
 
   it("should update state correctly according to FETCH_USERS_SUCCESS action", () => {
@@ -327,7 +329,8 @@ describe("users reducer", () => {
     expect(newState.list).toEqual({
       entries: ["zaphod", "ford"],
       error: null,
-      loading: false
+      loading: false,
+      userCreatePermission: true
     });
 
     expect(newState.byNames).toEqual({
@@ -339,7 +342,7 @@ describe("users reducer", () => {
       }
     });
 
-    expect(newState.userCreatePermission).toBeTruthy();
+    expect(newState.list.userCreatePermission).toBeTruthy();
   });
 
   test("should update state correctly according to DELETE_USER action", () => {
@@ -432,19 +435,19 @@ describe("users reducer", () => {
   it("should set userCreatePermission to true if update link is present", () => {
     const newState = reducer({}, fetchUsersSuccess(responseBody));
 
-    expect(newState.userCreatePermission).toBeTruthy();
+    expect(newState.list.userCreatePermission).toBeTruthy();
   });
 
   it("should update state correctly according to CREATE_USER_PENDING action", () => {
     const newState = reducer({}, createUserPending(userZaphod));
-    expect(newState.list.loading).toBeTruthy();
-    expect(newState.list.error).toBeNull();
+    expect(newState.create.loading).toBeTruthy();
+    expect(newState.create.error).toBeFalsy();
   });
 
   it("should update state correctly according to CREATE_USER_SUCCESS action", () => {
     const newState = reducer({ loading: true }, createUserSuccess());
-    expect(newState.list.loading).toBeFalsy();
-    expect(newState.list.error).toBeNull();
+    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", () => {
@@ -452,8 +455,8 @@ describe("users reducer", () => {
       { loading: true, error: null },
       createUserFailure(userFord, new Error("kaputt kaputt"))
     );
-    expect(newState.list.loading).toBeFalsy();
-    expect(newState.list.error).toEqual(new Error("kaputt kaputt"));
+    expect(newState.create.loading).toBeFalsy();
+    expect(newState.create.error).toEqual(new Error("kaputt kaputt"));
   });
 
   it("should update state according to FETCH_USER_PENDING action", () => {
@@ -500,4 +503,45 @@ describe("users reducer", () => {
     expect(newState.byNames["ford"].entry).toBe(userFord);
     expect(newState.list.entries).toEqual(["zaphod"]);
   });
+
+  it("should update state according to MODIFY_USER_PENDING action", () => {
+    const newState = reducer(
+      {
+        error: new Error("something"),
+        entry: {}
+      },
+      modifyUserPending(userFord)
+    );
+    expect(newState.byNames["ford"].loading).toBeTruthy();
+    expect(newState.byNames["ford"].error).toBeFalsy();
+    expect(newState.byNames["ford"].entry).toBeFalsy();
+  });
+
+  it("should update state according to MODIFY_USER_SUCCESS action", () => {
+    const newState = reducer(
+      {
+        loading: true,
+        error: new Error("something"),
+        entry: {}
+      },
+      modifyUserSuccess(userFord)
+    );
+    expect(newState.byNames["ford"].loading).toBeFalsy();
+    expect(newState.byNames["ford"].error).toBeFalsy();
+    expect(newState.byNames["ford"].entry).toBe(userFord);
+  });
+
+  it("should update state according to MODIFY_USER_SUCCESS action", () => {
+    const error = new Error("something went wrong");
+    const newState = reducer(
+      {
+        loading: true,
+        entry: {}
+      },
+      modifyUserFailure(userFord, error)
+    );
+    expect(newState.byNames["ford"].loading).toBeFalsy();
+    expect(newState.byNames["ford"].error).toBe(error);
+    expect(newState.byNames["ford"].entry).toBeFalsy();
+  });
 });