From 9c6882ee6e046791ca4c783962e7e7cd4e1be72c Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 25 Jul 2018 14:34:00 +0200 Subject: [PATCH] fixed users reducer and seperate create state --- scm-ui/src/users/containers/AddUser.js | 6 +- scm-ui/src/users/containers/SingleUser.js | 4 +- scm-ui/src/users/containers/Users.js | 8 +- scm-ui/src/users/modules/users.js | 97 +++++++++-------------- scm-ui/src/users/modules/users.test.js | 66 ++++++++++++--- 5 files changed, 99 insertions(+), 82 deletions(-) diff --git a/scm-ui/src/users/containers/AddUser.js b/scm-ui/src/users/containers/AddUser.js index 71d9bf58be..a80a3fc039 100644 --- a/scm-ui/src/users/containers/AddUser.js +++ b/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 {}; }; diff --git a/scm-ui/src/users/containers/SingleUser.js b/scm-ui/src/users/containers/SingleUser.js index d00af2a4c2..d0c018ee3a 100644 --- a/scm-ui/src/users/containers/SingleUser.js +++ b/scm-ui/src/users/containers/SingleUser.js @@ -91,8 +91,8 @@ class SingleUser extends React.Component { 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 { diff --git a/scm-ui/src/users/containers/Users.js b/scm-ui/src/users/containers/Users.js index 8382852cb0..4639cf9100 100644 --- a/scm-ui/src/users/containers/Users.js +++ b/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, diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index ba4f919297..9cefdaec37 100644 --- a/scm-ui/src/users/modules/users.js +++ b/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; } diff --git a/scm-ui/src/users/modules/users.test.js b/scm-ui/src/users/modules/users.test.js index adc5ff60dd..0dcca7e465 100644 --- a/scm-ui/src/users/modules/users.test.js +++ b/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(); + }); });