From 7b921da1745bfb2a8794f85493480384dda128ab Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Wed, 25 Jul 2018 13:57:24 +0200 Subject: [PATCH] use list/byNames in user state, merged heads --- scm-ui/src/modules/auth.test.js | 7 -- .../users/containers/EditUserButton.test.js | 2 +- scm-ui/src/users/modules/users.js | 28 +++---- scm-ui/src/users/modules/users.test.js | 77 +++++++++---------- 4 files changed, 50 insertions(+), 64 deletions(-) diff --git a/scm-ui/src/modules/auth.test.js b/scm-ui/src/modules/auth.test.js index 6701aace33..831bed8d4b 100644 --- a/scm-ui/src/modules/auth.test.js +++ b/scm-ui/src/modules/auth.test.js @@ -153,8 +153,6 @@ describe("auth actions", () => { status: 400 }); - const expectedActions = [{ type: LOGIN_REQUEST }, { type: LOGIN_FAILURE }]; - const store = mockStore({}); return store.dispatch(login("tricia", "secret123")).then(() => { const actions = store.getActions(); @@ -244,11 +242,6 @@ describe("auth actions", () => { status: 500 }); - const expectedActions = [ - { type: LOGOUT_REQUEST }, - { type: LOGOUT_FAILURE } - ]; - const store = mockStore({}); return store.dispatch(logout()).then(() => { const actions = store.getActions(); diff --git a/scm-ui/src/users/containers/EditUserButton.test.js b/scm-ui/src/users/containers/EditUserButton.test.js index 9c79e0ca33..68f2cc7d35 100644 --- a/scm-ui/src/users/containers/EditUserButton.test.js +++ b/scm-ui/src/users/containers/EditUserButton.test.js @@ -1,5 +1,5 @@ import React from "react"; -import { configure, shallow } from "enzyme"; +import { shallow } from "enzyme"; import "../../tests/enzyme"; import "../../tests/i18n"; import EditUserButton from "./EditUserButton"; diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index dc5022a3bd..ba4f919297 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -272,7 +272,7 @@ export function getUsersFromState(state: any) { const userEntries: Array = []; for (let userName of userNames) { - userEntries.push(state.users.usersByNames[userName]); + userEntries.push(state.users.byNames[userName]); } return userEntries; @@ -318,13 +318,13 @@ const reduceUsersByNames = ( newUserState: any ) => { const newUsersByNames = { - ...state.usersByNames, + ...state.byNames, [username]: newUserState }; return { ...state, - usersByNames: newUsersByNames + byNames: newUsersByNames }; }; @@ -342,25 +342,21 @@ export default function reducer(state: any = {}, action: any = {}) { // return red(state, action.payload._embedded.users); const users = action.payload._embedded.users; const userNames = users.map(user => user.name); - const usersByNames = extractUsersByNames( - users, - userNames, - state.usersByNames - ); + const byNames = extractUsersByNames(users, userNames, state.byNames); return { ...state, - users: { - userCreatePermission: action.payload._links.create ? true : false, + userCreatePermission: action.payload._links.create ? true : false, + list: { error: null, entries: userNames, loading: false }, - usersByNames + byNames }; case FETCH_USERS_FAILURE: return { ...state, - users: { + list: { ...state.users, loading: false, error: action.payload.error @@ -421,8 +417,8 @@ export default function reducer(state: any = {}, action: any = {}) { case CREATE_USER_PENDING: return { ...state, - users: { - ...state.users, + list: { + ...state.list, loading: true, error: null } @@ -430,7 +426,7 @@ export default function reducer(state: any = {}, action: any = {}) { case CREATE_USER_SUCCESS: return { ...state, - users: { + list: { ...state.users, loading: false, error: null @@ -439,7 +435,7 @@ export default function reducer(state: any = {}, action: any = {}) { case CREATE_USER_FAILURE: return { ...state, - users: { + list: { ...state.users, loading: false, error: action.payload diff --git a/scm-ui/src/users/modules/users.test.js b/scm-ui/src/users/modules/users.test.js index d217a38deb..adc5ff60dd 100644 --- a/scm-ui/src/users/modules/users.test.js +++ b/scm-ui/src/users/modules/users.test.js @@ -32,11 +32,11 @@ import { createUserSuccess, createUserFailure, modifyUserFailure, + fetchUserSuccess, deleteUserSuccess, fetchUsersPending, fetchUserPending, - fetchUserFailure, - fetchUserSuccess + fetchUserFailure } from "./users"; import reducer from "./users"; @@ -209,6 +209,9 @@ describe("users fetch()", () => { status: 204 }); + // after create, the users are fetched again + fetchMock.getOnce(USERS_URL, response); + const store = mockStore({}); return store.dispatch(createUser(userZaphod)).then(() => { const actions = store.getActions(); @@ -283,6 +286,8 @@ describe("users fetch()", () => { fetchMock.deleteOnce("http://localhost:8081/scm/api/rest/v2/users/zaphod", { status: 204 }); + // after update, the users are fetched again + fetchMock.getOnce(USERS_URL, response); const store = mockStore({}); return store.dispatch(deleteUser(userZaphod)).then(() => { @@ -319,14 +324,13 @@ describe("users reducer", () => { it("should update state correctly according to FETCH_USERS_SUCCESS action", () => { const newState = reducer({}, fetchUsersSuccess(responseBody)); - expect(newState.users).toEqual({ + expect(newState.list).toEqual({ entries: ["zaphod", "ford"], error: null, - loading: false, - userCreatePermission: true + loading: false }); - expect(newState.usersByNames).toEqual({ + expect(newState.byNames).toEqual({ zaphod: { entry: userZaphod }, @@ -334,6 +338,8 @@ describe("users reducer", () => { entry: userFord } }); + + expect(newState.userCreatePermission).toBeTruthy(); }); test("should update state correctly according to DELETE_USER action", () => { @@ -348,7 +354,7 @@ describe("users reducer", () => { }; const newState = reducer(state, deleteUserPending(userZaphod)); - const zaphod = newState.usersByNames["zaphod"]; + const zaphod = newState.byNames["zaphod"]; expect(zaphod.loading).toBeTruthy(); expect(zaphod.entry).toBe(userZaphod); }); @@ -384,7 +390,7 @@ describe("users reducer", () => { const error = new Error("error"); const newState = reducer(state, deleteUserFailure(userZaphod, error)); - const zaphod = newState.usersByNames["zaphod"]; + const zaphod = newState.byNames["zaphod"]; expect(zaphod.loading).toBeFalsy(); expect(zaphod.error).toBe(error); }); @@ -409,9 +415,9 @@ describe("users reducer", () => { expect(ford.loading).toBeFalsy(); }); - it("should not replace whole usersByNames map when fetching users", () => { + it("should not replace whole byNames map when fetching users", () => { const oldState = { - usersByNames: { + byNames: { ford: { entry: userFord } @@ -419,40 +425,40 @@ describe("users reducer", () => { }; const newState = reducer(oldState, fetchUsersSuccess(responseBody)); - expect(newState.usersByNames["zaphod"]).toBeDefined(); - expect(newState.usersByNames["ford"]).toBeDefined(); + expect(newState.byNames["zaphod"]).toBeDefined(); + expect(newState.byNames["ford"]).toBeDefined(); }); it("should set userCreatePermission to true if update link is present", () => { const newState = reducer({}, fetchUsersSuccess(responseBody)); - expect(newState.users.userCreatePermission).toBeTruthy(); + expect(newState.userCreatePermission).toBeTruthy(); }); 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(); + expect(newState.list.loading).toBeTruthy(); + expect(newState.list.error).toBeNull(); }); 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(); + expect(newState.list.loading).toBeFalsy(); + expect(newState.list.error).toBeNull(); }); - it("should set the loading to false and the error if user could not be added", () => { + it("should set the loading to false and the error if user could not be created", () => { const newState = reducer( { loading: true, error: null }, createUserFailure(userFord, new Error("kaputt kaputt")) ); - expect(newState.users.loading).toBeFalsy(); - expect(newState.users.error).toEqual(new Error("kaputt kaputt")); + expect(newState.list.loading).toBeFalsy(); + expect(newState.list.error).toEqual(new Error("kaputt kaputt")); }); it("should update state according to FETCH_USER_PENDING action", () => { const newState = reducer({}, fetchUserPending("zaphod")); - expect(newState.usersByNames["zaphod"].loading).toBeTruthy(); + expect(newState.byNames["zaphod"].loading).toBeTruthy(); }); it("should not affect users state", () => { @@ -464,43 +470,34 @@ describe("users reducer", () => { }, fetchUserPending("zaphod") ); - expect(newState.usersByNames["zaphod"].loading).toBeTruthy(); + expect(newState.byNames["zaphod"].loading).toBeTruthy(); expect(newState.users.entries).toEqual(["ford"]); }); it("should update state according to FETCH_USER_FAILURE action", () => { const error = new Error("kaputt!"); - const newState = reducer( - { - usersByNames: { - ford: { - loading: true - } - } - }, - fetchUserFailure(userFord.name, error) - ); - expect(newState.usersByNames["ford"].error).toBe(error); - expect(newState.usersByNames["ford"].loading).toBeFalsy(); + const newState = reducer({}, fetchUserFailure(userFord.name, error)); + expect(newState.byNames["ford"].error).toBe(error); + expect(newState.byNames["ford"].loading).toBeFalsy(); }); it("should update state according to FETCH_USER_SUCCESS action", () => { const newState = reducer({}, fetchUserSuccess(userFord)); - expect(newState.usersByNames["ford"].loading).toBeFalsy(); - expect(newState.usersByNames["ford"].entry).toBe(userFord); + expect(newState.byNames["ford"].loading).toBeFalsy(); + expect(newState.byNames["ford"].entry).toBe(userFord); }); it("should affect users state nor the state of other users", () => { const newState = reducer( { - users: { + list: { entries: ["zaphod"] } }, fetchUserSuccess(userFord) ); - expect(newState.usersByNames["ford"].loading).toBeFalsy(); - expect(newState.usersByNames["ford"].entry).toBe(userFord); - expect(newState.users.entries).toEqual(["zaphod"]); + expect(newState.byNames["ford"].loading).toBeFalsy(); + expect(newState.byNames["ford"].entry).toBe(userFord); + expect(newState.list.entries).toEqual(["zaphod"]); }); });