From 3ac532e605084e9edba7aa993f12e4ae220adc17 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 26 Jul 2018 09:25:42 +0200 Subject: [PATCH] Split user reducer --- scm-ui/src/users/modules/users.js | 182 +++++++++++++------------ scm-ui/src/users/modules/users.test.js | 118 +++++++++------- 2 files changed, 159 insertions(+), 141 deletions(-) diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index da3bfc0df6..850aaebf65 100644 --- a/scm-ui/src/users/modules/users.js +++ b/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 { - ...state, - byNames: newUsersByNames - }; + return 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, - list: { - error: null, - entries: userNames, - loading: false, - userCreatePermission: action.payload._links.create ? true : false - }, - byNames + error: null, + entries: userNames, + loading: false, + userCreatePermission: action.payload._links.create ? true : false }; case FETCH_USERS_FAILURE: return { ...state, - list: { - ...state.users, - loading: false, - 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 - 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 + // Update single user actions 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 +}); diff --git a/scm-ui/src/users/modules/users.test.js b/scm-ui/src/users/modules/users.test.js index ce4daaadfc..65e03bf60d 100644 --- a/scm-ui/src/users/modules/users.test.js +++ b/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"), - entry: {} + byNames: { + 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, - error: new Error("something"), - entry: {} + byNames: { + ford: { + 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, - entry: {} + byNames: { + ford: { + loading: true, + entry: {} + } + } }, modifyUserFailure(userFord, error) );