refactoring

2025-11-12 00:15:44 +01:00 · 2018-07-23 17:00:33 +02:00
parent 5851b7e0f4
commit bb87780ab9
7 changed files with 153 additions and 148 deletions
--- a/scm-ui/src/users/containers/AddUser.js
+++ b/scm-ui/src/users/containers/AddUser.js
@@ -4,7 +4,7 @@ import { connect } from "react-redux";
 import UserForm from "./UserForm";
 import type { User } from "../types/User";

-import { addUser } from "../modules/users";
+import { createUser } from "../modules/users";

 type Props = {
  addUser: User => void,
@@ -29,7 +29,7 @@ class AddUser extends React.Component<Props> {
 const mapDispatchToProps = dispatch => {
  return {
    addUser: (user: User) => {
-      dispatch(addUser(user));
+      dispatch(createUser(user));
    }
  };
 };
--- a/scm-ui/src/users/containers/EditUser.js
+++ b/scm-ui/src/users/containers/EditUser.js
@@ -6,7 +6,7 @@ import type { User } from "../types/User";
 import type { UserEntry } from "../types/UserEntry";
 import Loading from "../../components/Loading";

-import { updateUser, fetchUser } from "../modules/users";
+import { modifyUser, fetchUser } from "../modules/users";

 type Props = {
  name: string,
@@ -48,7 +48,7 @@ const mapDispatchToProps = dispatch => {
      dispatch(fetchUser(name));
    },
    updateUser: (user: User) => {
-      dispatch(updateUser(user));
+      dispatch(modifyUser(user));
    }
  };
 };
--- a/scm-ui/src/users/containers/EditUserButton.js
+++ b/scm-ui/src/users/containers/EditUserButton.js
@@ -2,7 +2,6 @@
 import React from "react";
 import EditButton from "../../components/EditButton";
 import type { UserEntry } from "../types/UserEntry";
-import { Link } from "react-router-dom";

 type Props = {
  entry: UserEntry
--- a/scm-ui/src/users/containers/UserForm.js
+++ b/scm-ui/src/users/containers/UserForm.js
@@ -4,7 +4,6 @@ import type { User } from "../types/User";
 import InputField from "../../components/InputField";
 import Checkbox from "../../components/Checkbox";
 import SubmitButton from "../../components/SubmitButton";
-import { connect } from "react-redux";

 type Props = {
  submitForm: User => void,
--- a/scm-ui/src/users/containers/Users.js
+++ b/scm-ui/src/users/containers/Users.js
@@ -4,7 +4,6 @@ import { connect } from "react-redux";

 import { fetchUsers, deleteUser, getUsersFromState } from "../modules/users";
 import Page from "../../components/Page";
-import ErrorNotification from "../../components/ErrorNotification";
 import UserTable from "./UserTable";
 import type { User } from "../types/User";
 import AddButton from "../../components/AddButton";
--- a/scm-ui/src/users/modules/users.js
+++ b/scm-ui/src/users/modules/users.js
@@ -1,56 +1,38 @@
 // @flow
-import { apiClient, NOT_FOUND_ERROR } from "../../apiclient";
+import { apiClient } from "../../apiclient";
 import type { User } from "../types/User";
 import type { UserEntry } from "../types/UserEntry";
 import { Dispatch } from "redux";

-export const FETCH_USERS = "scm/users/FETCH";
-export const FETCH_USERS_SUCCESS = "scm/users/FETCH_SUCCESS";
-export const FETCH_USERS_FAILURE = "scm/users/FETCH_FAILURE";
-export const FETCH_USERS_NOTFOUND = "scm/users/FETCH_NOTFOUND";
+export const FETCH_USERS_PENDING = "scm/users/FETCH_USERS_PENDING";
+export const FETCH_USERS_SUCCESS = "scm/users/FETCH_USERS_SUCCESS";
+export const FETCH_USERS_FAILURE = "scm/users/FETCH_USERS_FAILURE";

-export const FETCH_USER = "scm/users/FETCH_USER";
+export const FETCH_USER_PENDING = "scm/users/FETCH_USER_PENDING";
 export const FETCH_USER_SUCCESS = "scm/users/FETCH_USER_SUCCESS";
 export const FETCH_USER_FAILURE = "scm/users/FETCH_USER_FAILURE";

-export const ADD_USER = "scm/users/ADD";
-export const ADD_USER_SUCCESS = "scm/users/ADD_SUCCESS";
-export const ADD_USER_FAILURE = "scm/users/ADD_FAILURE";
+export const CREATE_USER_PENDING = "scm/users/CREATE_USER_PENDING";
+export const CREATE_USER_SUCCESS = "scm/users/CREATE_USER_SUCCESS";
+export const CREATE_USER_FAILURE = "scm/users/CREATE_USER_FAILURE";

-export const EDIT_USER = "scm/users/EDIT";
-
-export const UPDATE_USER = "scm/users/UPDATE";
-export const UPDATE_USER_SUCCESS = "scm/users/UPDATE_SUCCESS";
-export const UPDATE_USER_FAILURE = "scm/users/UPDATE_FAILURE";
+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_SUCCESS = "scm/users/DELETE_SUCCESS";
 export const DELETE_USER_FAILURE = "scm/users/DELETE_FAILURE";

 const USERS_URL = "users";
-const USER_URL = "users/";

 const CONTENT_TYPE_USER = "application/vnd.scmm-user+json;v=2";

-export function requestUsers() {
-  return {
-    type: FETCH_USERS
-  };
-}
-
-export function failedToFetchUsers(url: string, error: Error) {
-  return {
-    type: FETCH_USERS_FAILURE,
-    payload: {
-      error,
-      url
-    }
-  };
-}
+//fetch users

 export function fetchUsers() {
  return function(dispatch: any) {
-    dispatch(requestUsers());
+    dispatch(fetchUsersPending());
    return apiClient
      .get(USERS_URL)
      .then(response => {
@@ -66,11 +48,17 @@ export function fetchUsers() {
      })
      .catch(cause => {
        const error = new Error(`could not fetch users: ${cause.message}`);
-        dispatch(failedToFetchUsers(USERS_URL, error));
+        dispatch(fetchUsersFailure(USERS_URL, error));
      });
  };
 }

+export function fetchUsersPending() {
+  return {
+    type: FETCH_USERS_PENDING
+  };
+}
+
 export function fetchUsersSuccess(users: any) {
  return {
    type: FETCH_USERS_SUCCESS,
@@ -78,17 +66,22 @@ export function fetchUsersSuccess(users: any) {
  };
 }

-export function requestUser(name: string) {
+export function fetchUsersFailure(url: string, error: Error) {
  return {
-    type: FETCH_USER,
-    payload: { name }
+    type: FETCH_USERS_FAILURE,
+    payload: {
+      error,
+      url
+    }
  };
 }

+//fetch user
+//TODO: fetchUsersPending and FetchUsersFailure are the wrong functions here!
 export function fetchUser(name: string) {
-  const userUrl = USER_URL + name;
+  const userUrl = USERS_URL + "/" + name;
  return function(dispatch: any) {
-    dispatch(requestUsers());
+    dispatch(fetchUsersPending());
    return apiClient
      .get(userUrl)
      .then(response => {
@@ -104,12 +97,19 @@ export function fetchUser(name: string) {
      })
      .catch(cause => {
        const error = new Error(`could not fetch user: ${cause.message}`);
-        dispatch(failedToFetchUsers(USERS_URL, error));
+        dispatch(fetchUsersFailure(USERS_URL, error));
      });
  };
 }

-export function fetchUserSuccess(user: User) {
+export function fetchUserPending(name: string) {
+  return {
+    type: FETCH_USER_PENDING,
+    payload: { name }
+  };
+}
+
+export function fetchUserSuccess(user: any) {
  return {
    type: FETCH_USER_SUCCESS,
    payload: user
@@ -124,25 +124,20 @@ export function fetchUserFailure(user: User, error: Error) {
  };
 }

-export function requestAddUser(user: User) {
-  return {
-    type: ADD_USER,
-    user
-  };
-}
+//create user

-export function addUser(user: User) {
+export function createUser(user: User) {
  return function(dispatch: Dispatch) {
-    dispatch(requestAddUser(user));
+    dispatch(createUserPending(user));
    return apiClient
      .postWithContentType(USERS_URL, user, CONTENT_TYPE_USER)
      .then(() => {
-        dispatch(addUserSuccess());
+        dispatch(createUserSuccess());
        dispatch(fetchUsers());
      })
      .catch(err =>
        dispatch(
-          addUserFailure(
+          createUserFailure(
            user,
            new Error(`failed to add user ${user.name}: ${err.message}`)
          )
@@ -151,59 +146,89 @@ export function addUser(user: User) {
  };
 }

-export function addUserSuccess() {
+export function createUserPending(user: User) {
  return {
-    type: ADD_USER_SUCCESS
+    type: CREATE_USER_PENDING,
+    user
  };
 }

-export function addUserFailure(user: User, err: Error) {
+
+export function createUserSuccess() {
  return {
-    type: ADD_USER_FAILURE,
+    type: CREATE_USER_SUCCESS
+  };
+}
+
+export function createUserFailure(user: User, err: Error) {
+  return {
+    type: CREATE_USER_FAILURE,
    payload: err,
    user
  };
 }

-function requestUpdateUser(user: User) {
-  return {
-    type: UPDATE_USER,
-    user
-  };
-}
+//modify user

-export function updateUser(user: User) {
+export function modifyUser(user: User) {
  return function(dispatch: Dispatch) {
-    dispatch(requestUpdateUser(user));
+    dispatch(modifyUserPending(user));
    return apiClient
      .putWithContentType(user._links.update.href, user, CONTENT_TYPE_USER)
      .then(() => {
-        dispatch(updateUserSuccess(user));
+        dispatch(modifyUserSuccess(user));
        dispatch(fetchUsers());
      })
      .catch(err => {
        console.log(err);
-        dispatch(updateUserFailure(user, err));
+        dispatch(modifyUserFailure(user, err));
      });
  };
 }

-function updateUserSuccess(user: User) {
+function modifyUserPending(user: User) {
  return {
-    type: UPDATE_USER_SUCCESS,
+    type: MODIFY_USER_PENDING,
    user
  };
 }

-export function updateUserFailure(user: User, error: Error) {
+function modifyUserSuccess(user: User) {
  return {
-    type: UPDATE_USER_FAILURE,
+    type: MODIFY_USER_SUCCESS,
+    user
+  };
+}
+
+export function modifyUserFailure(user: User, error: Error) {
+  return {
+    type: MODIFY_USER_FAILURE,
    payload: error,
    user
  };
 }

-export function requestDeleteUser(user: User) {
+//delete user
+
+export function deleteUser(user: User) {
+  return function(dispatch: any) {
+    dispatch(deleteUserPending(user));
+    return apiClient
+      .delete(user._links.delete.href)
+      .then(() => {
+        dispatch(deleteUserSuccess(user));
+        dispatch(fetchUsers());
+      })
+      .catch(cause => {
+        const error = new Error(
+          `could not delete user ${user.name}: ${cause.message}`
+        );
+        dispatch(deleteUserFailure(user, error));
+      });
+  };
+}
+
+export function deleteUserPending(user: User) {
  return {
    type: DELETE_USER,
    payload: user
@@ -227,25 +252,9 @@ export function deleteUserFailure(user: User, error: Error) {
  };
 }

-export function deleteUser(user: User) {
-  return function(dispatch: any) {
-    dispatch(requestDeleteUser(user));
-    return apiClient
-      .delete(user._links.delete.href)
-      .then(() => {
-        dispatch(deleteUserSuccess(user));
-        dispatch(fetchUsers());
-      })
-      .catch(cause => {
-        const error = new Error(
-          `could not delete user ${user.name}: ${cause.message}`
-        );
-        dispatch(deleteUserFailure(user, error));
-      });
-  };
-}
+//helper functions

-export function getUsersFromState(state) {
+export function getUsersFromState(state: any) {
  if (!state.users.users) {
    return null;
  }
@@ -283,7 +292,7 @@ function extractUsersByNames(
 function deleteUserInUsersByNames(users: {}, userName: any) {
  let newUsers = {};
  for (let username in users) {
-    if (username != userName) newUsers[username] = users[username];
+    if (username !== userName) newUsers[username] = users[username];
  }
  return newUsers;
 }
@@ -291,7 +300,7 @@ function deleteUserInUsersByNames(users: {}, userName: any) {
 function deleteUserInEntries(users: [], userName: any) {
  let newUsers = [];
  for (let user of users) {
-    if (user != userName) newUsers.push(user);
+    if (user !== userName) newUsers.push(user);
  }
  return newUsers;
 }
@@ -315,7 +324,7 @@ const reduceUsersByNames = (
 export default function reducer(state: any = {}, action: any = {}) {
  switch (action.type) {
    // fetch user list cases
-    case FETCH_USERS:
+    case FETCH_USERS_PENDING:
      return {
        ...state,
        users: {
@@ -351,7 +360,7 @@ export default function reducer(state: any = {}, action: any = {}) {
        }
      };
    // Fetch single user cases
-    case FETCH_USER:
+    case FETCH_USER_PENDING:
      return reduceUsersByNames(state, action.payload.name, {
        loading: true,
        error: null
@@ -415,7 +424,7 @@ export default function reducer(state: any = {}, action: any = {}) {
        }
      };
    // Add single user cases
-    case ADD_USER:
+    case CREATE_USER_PENDING:
      return {
        ...state,
        users: {
@@ -424,7 +433,7 @@ export default function reducer(state: any = {}, action: any = {}) {
          error: null
        }
      };
-    case ADD_USER_SUCCESS:
+    case CREATE_USER_SUCCESS:
      return {
        ...state,
        users: {
@@ -433,7 +442,7 @@ export default function reducer(state: any = {}, action: any = {}) {
          error: null
        }
      };
-    case ADD_USER_FAILURE:
+    case CREATE_USER_FAILURE:
      return {
        ...state,
        users: {
@@ -443,7 +452,7 @@ export default function reducer(state: any = {}, action: any = {}) {
        }
      };
    // Update single user cases
-    case UPDATE_USER:
+    case MODIFY_USER_PENDING:
      return {
        ...state,
        usersByNames: {
@@ -455,7 +464,7 @@ export default function reducer(state: any = {}, action: any = {}) {
          }
        }
      };
-    case UPDATE_USER_SUCCESS:
+    case MODIFY_USER_SUCCESS:
      return {
        ...state,
        usersByNames: {
--- a/scm-ui/src/users/modules/users.test.js
+++ b/scm-ui/src/users/modules/users.test.js
@@ -4,34 +4,33 @@ import thunk from "redux-thunk";
 import fetchMock from "fetch-mock";

 import {
-  FETCH_USERS,
+  FETCH_USERS_PENDING,
  FETCH_USERS_SUCCESS,
  fetchUsers,
  FETCH_USERS_FAILURE,
-  addUser,
-  ADD_USER,
-  ADD_USER_SUCCESS,
-  ADD_USER_FAILURE,
-  updateUser,
-  UPDATE_USER,
-  UPDATE_USER_FAILURE,
-  UPDATE_USER_SUCCESS,
-  EDIT_USER,
-  requestDeleteUser,
+  createUserPending,
+  CREATE_USER_PENDING,
+  CREATE_USER_SUCCESS,
+  CREATE_USER_FAILURE,
+  modifyUser,
+  MODIFY_USER_PENDING,
+  MODIFY_USER_FAILURE,
+  MODIFY_USER_SUCCESS,
+  deleteUserPending,
  deleteUserFailure,
  DELETE_USER,
  DELETE_USER_SUCCESS,
  DELETE_USER_FAILURE,
  deleteUser,
-  requestUsers,
+  fetchUsersFailure,
  fetchUsersSuccess,
-  requestAddUser,
-  addUserSuccess,
-  addUserFailure,
-  updateUserFailure,
+  createUser,
+  createUserSuccess,
+  createUserFailure,
+  modifyUserFailure,
  deleteUserSuccess,
-  failedToFetchUsers,
-  requestUser,
+  fetchUsersPending,
+  fetchUserPending,
  fetchUserFailure
 } from "./users";

@@ -143,7 +142,7 @@ describe("users fetch()", () => {
    fetchMock.getOnce("/scm/api/rest/v2/users", response);

    const expectedActions = [
-      { type: FETCH_USERS },
+      { type: FETCH_USERS_PENDING },
      {
        type: FETCH_USERS_SUCCESS,
        payload: response
@@ -165,7 +164,7 @@ describe("users fetch()", () => {
    const store = mockStore({});
    return store.dispatch(fetchUsers()).then(() => {
      const actions = store.getActions();
-      expect(actions[0].type).toEqual(FETCH_USERS);
+      expect(actions[0].type).toEqual(FETCH_USERS_PENDING);
      expect(actions[1].type).toEqual(FETCH_USERS_FAILURE);
      expect(actions[1].payload).toBeDefined();
    });
@@ -181,11 +180,11 @@ describe("users fetch()", () => {
    fetchMock.getOnce("/scm/api/rest/v2/users", response);

    const store = mockStore({});
-    return store.dispatch(addUser(userZaphod)).then(() => {
+    return store.dispatch(createUser(userZaphod)).then(() => {
      const actions = store.getActions();
-      expect(actions[0].type).toEqual(ADD_USER);
-      expect(actions[1].type).toEqual(ADD_USER_SUCCESS);
-      expect(actions[2].type).toEqual(FETCH_USERS);
+      expect(actions[0].type).toEqual(CREATE_USER_PENDING);
+      expect(actions[1].type).toEqual(CREATE_USER_SUCCESS);
+      expect(actions[2].type).toEqual(FETCH_USERS_PENDING);
    });
  });

@@ -195,10 +194,10 @@ describe("users fetch()", () => {
    });

    const store = mockStore({});
-    return store.dispatch(addUser(userZaphod)).then(() => {
+    return store.dispatch(createUser(userZaphod)).then(() => {
      const actions = store.getActions();
-      expect(actions[0].type).toEqual(ADD_USER);
-      expect(actions[1].type).toEqual(ADD_USER_FAILURE);
+      expect(actions[0].type).toEqual(CREATE_USER_PENDING);
+      expect(actions[1].type).toEqual(CREATE_USER_FAILURE);
      expect(actions[1].payload).toBeDefined();
    });
  });
@@ -211,11 +210,11 @@ describe("users fetch()", () => {
    fetchMock.getOnce("/scm/api/rest/v2/users", response);

    const store = mockStore({});
-    return store.dispatch(updateUser(userZaphod)).then(() => {
+    return store.dispatch(modifyUser(userZaphod)).then(() => {
      const actions = store.getActions();
-      expect(actions[0].type).toEqual(UPDATE_USER);
-      expect(actions[1].type).toEqual(UPDATE_USER_SUCCESS);
-      expect(actions[2].type).toEqual(FETCH_USERS);
+      expect(actions[0].type).toEqual(MODIFY_USER_PENDING);
+      expect(actions[1].type).toEqual(MODIFY_USER_SUCCESS);
+      expect(actions[2].type).toEqual(FETCH_USERS_PENDING);
    });
  });

@@ -225,10 +224,10 @@ describe("users fetch()", () => {
    });

    const store = mockStore({});
-    return store.dispatch(updateUser(userZaphod)).then(() => {
+    return store.dispatch(modifyUser(userZaphod)).then(() => {
      const actions = store.getActions();
-      expect(actions[0].type).toEqual(UPDATE_USER);
-      expect(actions[1].type).toEqual(UPDATE_USER_FAILURE);
+      expect(actions[0].type).toEqual(MODIFY_USER_PENDING);
+      expect(actions[1].type).toEqual(MODIFY_USER_FAILURE);
      expect(actions[1].payload).toBeDefined();
    });
  });
@@ -246,7 +245,7 @@ describe("users fetch()", () => {
      expect(actions[0].type).toEqual(DELETE_USER);
      expect(actions[0].payload).toBe(userZaphod);
      expect(actions[1].type).toEqual(DELETE_USER_SUCCESS);
-      expect(actions[2].type).toEqual(FETCH_USERS);
+      expect(actions[2].type).toEqual(FETCH_USERS_PENDING);
    });
  });

@@ -267,8 +266,8 @@ describe("users fetch()", () => {
 });

 describe("users reducer", () => {
-  it("should update state correctly according to FETCH_USERS action", () => {
-    const newState = reducer({}, requestUsers());
+  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();
  });
@@ -304,7 +303,7 @@ describe("users reducer", () => {
      }
    };

-    const newState = reducer(state, requestDeleteUser(userZaphod));
+    const newState = reducer(state, deleteUserPending(userZaphod));
    const zaphod = newState.usersByNames["zaphod"];
    expect(zaphod.loading).toBeTruthy();
    expect(zaphod.entry).toBe(userZaphod);
@@ -324,7 +323,7 @@ describe("users reducer", () => {
      }
    };

-    const newState = reducer(state, requestDeleteUser(userZaphod));
+    const newState = reducer(state, deleteUserPending(userZaphod));
    const ford = newState.usersByNames["ford"];
    expect(ford.loading).toBeFalsy();
  });
@@ -386,14 +385,14 @@ describe("users reducer", () => {
    expect(newState.users.userCreatePermission).toBeTruthy();
  });

-  it("should update state correctly according to ADD_USER action", () => {
-    const newState = reducer({}, requestAddUser(userZaphod));
+  it("should update state correctly according to CREATE_USER_PENDING action", () => {
+    const newState = reducer({}, createUserPending(userZaphod));
    expect(newState.users.loading).toBeTruthy();
    expect(newState.users.error).toBeNull();
  });

-  it("should update state correctly according to ADD_USER_SUCCESS action", () => {
-    const newState = reducer({ loading: true }, addUserSuccess());
+  it("should update state correctly according to CREATE_USER_SUCCESS action", () => {
+    const newState = reducer({ loading: true }, createUserSuccess());
    expect(newState.users.loading).toBeFalsy();
    expect(newState.users.error).toBeNull();
  });
@@ -401,14 +400,14 @@ describe("users reducer", () => {
  it("should set the loading to false and the error if user could not be added", () => {
    const newState = reducer(
      { loading: true, error: null },
-      addUserFailure(userFord, new Error("kaputt kaputt"))
+      createUserFailure(userFord, new Error("kaputt kaputt"))
    );
    expect(newState.users.loading).toBeFalsy();
    expect(newState.users.error).toEqual(new Error("kaputt kaputt"));
  });

-  it("should update state according to FETCH_USER action", () => {
-    const newState = reducer({}, requestUser("zaphod"));
+  it("should update state according to FETCH_USER_PENDING action", () => {
+    const newState = reducer({}, fetchUserPending("zaphod"));
    expect(newState.usersByNames["zaphod"].loading).toBeTruthy();
  });