From d0c8b6645984e42bd56cd4a538fa7f3d15c2184e Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Wed, 25 Jul 2018 16:42:52 +0200 Subject: [PATCH 01/11] Added unit tests --- scm-ui/src/users/modules/users.js | 17 +++--- scm-ui/src/users/modules/users.test.js | 74 ++++++++++++++++---------- 2 files changed, 55 insertions(+), 36 deletions(-) diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index 9cefdaec37..da3bfc0df6 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -180,7 +180,6 @@ export function modifyUser(user: User) { .putWithContentType(user._links.update.href, user, CONTENT_TYPE_USER) .then(() => { dispatch(modifyUserSuccess(user)); - dispatch(fetchUsers()); }) .catch(err => { dispatch(modifyUserFailure(user, err)); @@ -292,7 +291,7 @@ function extractUsersByNames( } return usersByNames; } -function deleteUserInUsersByNames(users: {}, userName: any) { +function deleteUserInUsersByNames(users: {}, userName: string) { let newUsers = {}; for (let username in users) { if (username !== userName) newUsers[username] = users[username]; @@ -300,7 +299,7 @@ function deleteUserInUsersByNames(users: {}, userName: any) { return newUsers; } -function deleteUserInEntries(users: [], userName: any) { +function deleteUserInEntries(users: [], userName: string) { let newUsers = []; for (let user of users) { if (user !== userName) newUsers.push(user); @@ -331,11 +330,13 @@ export default function reducer(state: any = {}, action: any = {}) { } }; 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, @@ -382,12 +383,14 @@ export default function reducer(state: any = {}, action: any = {}) { }); case DELETE_USER_SUCCESS: - const newUserByNames = deleteUserInUsersByNames(state.byNames, [ + const newUserByNames = deleteUserInUsersByNames( + state.byNames, action.payload.name - ]); - const newUserEntries = deleteUserInEntries(state.list.entries, [ + ); + const newUserEntries = deleteUserInEntries( + state.list.entries, action.payload.name - ]); + ); return { ...state, list: { diff --git a/scm-ui/src/users/modules/users.test.js b/scm-ui/src/users/modules/users.test.js index 0dcca7e465..ce4daaadfc 100644 --- a/scm-ui/src/users/modules/users.test.js +++ b/scm-ui/src/users/modules/users.test.js @@ -89,28 +89,6 @@ const userFord = { } }; -const responseBodyZaphod = { - page: 0, - pageTotal: 1, - _links: { - self: { - href: "http://localhost:3000/scm/api/rest/v2/users/?page=0&pageSize=10" - }, - first: { - href: "http://localhost:3000/scm/api/rest/v2/users/?page=0&pageSize=10" - }, - last: { - href: "http://localhost:3000/scm/api/rest/v2/users/?page=0&pageSize=10" - }, - create: { - href: "http://localhost:3000/scm/api/rest/v2/users/" - } - }, - _embedded: { - users: [userZaphod] - } -}; - const responseBody = { page: 0, pageTotal: 1, @@ -259,14 +237,12 @@ describe("users fetch()", () => { status: 204 }); // after update, the users are fetched again - fetchMock.getOnce(USERS_URL, response); const store = mockStore({}); return store.dispatch(modifyUser(userZaphod)).then(() => { const actions = store.getActions(); expect(actions[0].type).toEqual(MODIFY_USER_PENDING); expect(actions[1].type).toEqual(MODIFY_USER_SUCCESS); - expect(actions[2].type).toEqual(FETCH_USERS_PENDING); }); }); @@ -345,7 +321,7 @@ describe("users reducer", () => { expect(newState.list.userCreatePermission).toBeTruthy(); }); - test("should update state correctly according to DELETE_USER action", () => { + test("should update state correctly according to DELETE_USER_PENDING action", () => { const state = { usersByNames: { zaphod: { @@ -364,7 +340,7 @@ describe("users reducer", () => { it("should not effect other users if one user will be deleted", () => { const state = { - usersByNames: { + byNames: { zaphod: { loading: false, error: null, @@ -377,7 +353,7 @@ describe("users reducer", () => { }; const newState = reducer(state, deleteUserPending(userZaphod)); - const ford = newState.usersByNames["ford"]; + const ford = newState.byNames["ford"]; expect(ford.loading).toBeFalsy(); }); @@ -400,7 +376,7 @@ describe("users reducer", () => { it("should not effect other users if one user could not be deleted", () => { const state = { - usersByNames: { + byNames: { zaphod: { loading: false, error: null, @@ -414,10 +390,35 @@ describe("users reducer", () => { const error = new Error("error"); const newState = reducer(state, deleteUserFailure(userZaphod, error)); - const ford = newState.usersByNames["ford"]; + const ford = newState.byNames["ford"]; expect(ford.loading).toBeFalsy(); }); + it("should remove user from state when delete succeeds", () => { + const state = { + list: { + entries: ["ford", "zaphod"] + }, + byNames: { + zaphod: { + loading: true, + error: null, + entry: userZaphod + }, + ford: { + loading: true, + error: null, + entry: userFord + } + } + }; + + const newState = reducer(state, deleteUserSuccess(userFord)); + expect(newState.byNames["zaphod"]).toBeDefined(); + expect(newState.list.entries).toEqual(["zaphod"]); + expect(newState.byNames["ford"]).toBeFalsy(); + }); + it("should not replace whole byNames map when fetching users", () => { const oldState = { byNames: { @@ -432,6 +433,21 @@ describe("users reducer", () => { 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)); From 3ac532e605084e9edba7aa993f12e4ae220adc17 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 26 Jul 2018 09:25:42 +0200 Subject: [PATCH 02/11] 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) ); From 11588024929860013add46b5a169765eb6179862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maren=20S=C3=BCwer?= Date: Thu, 26 Jul 2018 09:49:44 +0200 Subject: [PATCH 03/11] refactoring of buttons to navLinks --- .../components/buttons/EditUserButton.js | 34 ------------------- .../components/buttons/EditUserButton.test.js | 31 ----------------- scm-ui/src/users/components/buttons/index.js | 2 -- .../DeleteUserNavLink.js} | 4 +-- .../DeleteUserNavLink.test.js} | 32 ++++++++--------- .../components/navLinks/EditUserNavLink.js | 28 +++++++++++++++ .../navLinks/EditUserNavLink.test.js | 27 +++++++++++++++ scm-ui/src/users/components/navLinks/index.js | 2 ++ scm-ui/src/users/containers/SingleUser.js | 6 ++-- 9 files changed, 78 insertions(+), 88 deletions(-) delete mode 100644 scm-ui/src/users/components/buttons/EditUserButton.js delete mode 100644 scm-ui/src/users/components/buttons/EditUserButton.test.js delete mode 100644 scm-ui/src/users/components/buttons/index.js rename scm-ui/src/users/components/{buttons/DeleteUserButton.js => navLinks/DeleteUserNavLink.js} (92%) rename scm-ui/src/users/components/{buttons/DeleteUserButton.test.js => navLinks/DeleteUserNavLink.test.js} (61%) create mode 100644 scm-ui/src/users/components/navLinks/EditUserNavLink.js create mode 100644 scm-ui/src/users/components/navLinks/EditUserNavLink.test.js create mode 100644 scm-ui/src/users/components/navLinks/index.js diff --git a/scm-ui/src/users/components/buttons/EditUserButton.js b/scm-ui/src/users/components/buttons/EditUserButton.js deleted file mode 100644 index dc7b8d9351..0000000000 --- a/scm-ui/src/users/components/buttons/EditUserButton.js +++ /dev/null @@ -1,34 +0,0 @@ -//@flow -import React from "react"; -import { translate } from "react-i18next"; -import { EditButton } from "../../../components/buttons"; -import type { UserEntry } from "../../types/UserEntry"; - -type Props = { - t: string => string, - entry: UserEntry -}; - -class EditUserButton extends React.Component { - render() { - const { entry, t } = this.props; - const link = "/users/edit/" + entry.entry.name; - - if (!this.isEditable()) { - return ""; - } - return ( - - ); - } - - isEditable = () => { - return this.props.entry.entry._links.update; - }; -} - -export default translate("users")(EditUserButton); diff --git a/scm-ui/src/users/components/buttons/EditUserButton.test.js b/scm-ui/src/users/components/buttons/EditUserButton.test.js deleted file mode 100644 index c708284f0f..0000000000 --- a/scm-ui/src/users/components/buttons/EditUserButton.test.js +++ /dev/null @@ -1,31 +0,0 @@ -import React from "react"; -import { shallow } from "enzyme"; -import "../../../tests/enzyme"; -import "../../../tests/i18n"; -import EditUserButton from "./EditUserButton"; - -it("should render nothing, if the edit link is missing", () => { - const entry = { - entry: { - _links: {} - } - }; - - const button = shallow(); - expect(button.text()).toBe(""); -}); - -it("should render the button", () => { - const entry = { - entry: { - _links: { - update: { - href: "/users" - } - } - } - }; - - const button = shallow(); - expect(button.text()).not.toBe(""); -}); diff --git a/scm-ui/src/users/components/buttons/index.js b/scm-ui/src/users/components/buttons/index.js deleted file mode 100644 index b5baba92d5..0000000000 --- a/scm-ui/src/users/components/buttons/index.js +++ /dev/null @@ -1,2 +0,0 @@ -export { default as DeleteUserButton } from "./DeleteUserButton"; -export { default as EditUserButton } from "./EditUserButton"; diff --git a/scm-ui/src/users/components/buttons/DeleteUserButton.js b/scm-ui/src/users/components/navLinks/DeleteUserNavLink.js similarity index 92% rename from scm-ui/src/users/components/buttons/DeleteUserButton.js rename to scm-ui/src/users/components/navLinks/DeleteUserNavLink.js index 39a33d32ac..96334e4560 100644 --- a/scm-ui/src/users/components/buttons/DeleteUserButton.js +++ b/scm-ui/src/users/components/navLinks/DeleteUserNavLink.js @@ -12,7 +12,7 @@ type Props = { deleteUser: (user: User) => void }; -class DeleteUserButton extends React.Component { +class DeleteUserNavLink extends React.Component { static defaultProps = { confirmDialog: true }; @@ -54,4 +54,4 @@ class DeleteUserButton extends React.Component { } } -export default translate("users")(DeleteUserButton); +export default translate("users")(DeleteUserNavLink); diff --git a/scm-ui/src/users/components/buttons/DeleteUserButton.test.js b/scm-ui/src/users/components/navLinks/DeleteUserNavLink.test.js similarity index 61% rename from scm-ui/src/users/components/buttons/DeleteUserButton.test.js rename to scm-ui/src/users/components/navLinks/DeleteUserNavLink.test.js index 09e1d87cb9..dddfd0f595 100644 --- a/scm-ui/src/users/components/buttons/DeleteUserButton.test.js +++ b/scm-ui/src/users/components/navLinks/DeleteUserNavLink.test.js @@ -2,24 +2,24 @@ import React from "react"; import { mount, shallow } from "enzyme"; import "../../../tests/enzyme"; import "../../../tests/i18n"; -import DeleteUserButton from "./DeleteUserButton"; +import DeleteUserNavLink from "./DeleteUserNavLink"; import { confirmAlert } from "../../../components/modals/ConfirmAlert"; jest.mock("../../../components/modals/ConfirmAlert"); -describe("DeleteUserButton", () => { +describe("DeleteUserNavLink", () => { it("should render nothing, if the delete link is missing", () => { const user = { _links: {} }; - const button = shallow( - {}} /> + const navLink = shallow( + {}} /> ); - expect(button.text()).toBe(""); + expect(navLink.text()).toBe(""); }); - it("should render the button", () => { + it("should render the navLink", () => { const user = { _links: { delete: { @@ -28,13 +28,13 @@ describe("DeleteUserButton", () => { } }; - const button = mount( - {}} /> + const navLink = mount( + {}} /> ); - expect(button.text()).not.toBe(""); + expect(navLink.text()).not.toBe(""); }); - it("should open the confirm dialog on button click", () => { + it("should open the confirm dialog on navLink click", () => { const user = { _links: { delete: { @@ -43,10 +43,10 @@ describe("DeleteUserButton", () => { } }; - const button = mount( - {}} /> + const navLink = mount( + {}} /> ); - button.find("a").simulate("click"); + navLink.find("a").simulate("click"); expect(confirmAlert.mock.calls.length).toBe(1); }); @@ -65,14 +65,14 @@ describe("DeleteUserButton", () => { calledUrl = user._links.delete.href; } - const button = mount( - ); - button.find("a").simulate("click"); + navLink.find("a").simulate("click"); expect(calledUrl).toBe("/users"); }); diff --git a/scm-ui/src/users/components/navLinks/EditUserNavLink.js b/scm-ui/src/users/components/navLinks/EditUserNavLink.js new file mode 100644 index 0000000000..246d7fc536 --- /dev/null +++ b/scm-ui/src/users/components/navLinks/EditUserNavLink.js @@ -0,0 +1,28 @@ +//@flow +import React from "react"; +import { translate } from "react-i18next"; +import type { UserEntry } from "../../types/UserEntry"; +import { NavLink } from "../../../components/navigation"; + +type Props = { + t: string => string, + user: UserEntry, + editUrl: String +}; + +class EditUserNavLink extends React.Component { + render() { + const { t, editUrl } = this.props; + + if (!this.isEditable()) { + return null; + } + return ; + } + + isEditable = () => { + return this.props.user._links.update; + }; +} + +export default translate("users")(EditUserNavLink); diff --git a/scm-ui/src/users/components/navLinks/EditUserNavLink.test.js b/scm-ui/src/users/components/navLinks/EditUserNavLink.test.js new file mode 100644 index 0000000000..ec46c531a1 --- /dev/null +++ b/scm-ui/src/users/components/navLinks/EditUserNavLink.test.js @@ -0,0 +1,27 @@ +import React from "react"; +import { shallow } from "enzyme"; +import "../../../tests/enzyme"; +import "../../../tests/i18n"; +import EditUserNavLink from "./EditUserNavLink"; + +it("should render nothing, if the edit link is missing", () => { + const user = { + _links: {} + }; + + const navLink = shallow(); + expect(navLink.text()).toBe(""); +}); + +it("should render the navLink", () => { + const user = { + _links: { + update: { + href: "/users" + } + } + }; + + const navLink = shallow(); + expect(navLink.text()).not.toBe(""); +}); diff --git a/scm-ui/src/users/components/navLinks/index.js b/scm-ui/src/users/components/navLinks/index.js new file mode 100644 index 0000000000..a3ccd16a32 --- /dev/null +++ b/scm-ui/src/users/components/navLinks/index.js @@ -0,0 +1,2 @@ +export { default as DeleteUserNavLink } from "./DeleteUserNavLink"; +export { default as EditUserNavLink } from "./EditUserNavLink"; diff --git a/scm-ui/src/users/containers/SingleUser.js b/scm-ui/src/users/containers/SingleUser.js index 215f78ae6e..3ba758c503 100644 --- a/scm-ui/src/users/containers/SingleUser.js +++ b/scm-ui/src/users/containers/SingleUser.js @@ -12,7 +12,7 @@ import { fetchUser, deleteUser } from "../modules/users"; import Loading from "../../components/Loading"; import { Navigation, Section, NavLink } from "../../components/navigation"; -import { DeleteUserButton } from "./../components/buttons"; +import { DeleteUserNavLink, EditUserNavLink } from "./../components/navLinks"; import ErrorPage from "../../components/ErrorPage"; type Props = { @@ -84,10 +84,10 @@ class SingleUser extends React.Component {
- +
- +
From c4249f97d1a1b15bb30caa7d0be0b88c6ca13a1c Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 26 Jul 2018 10:00:48 +0200 Subject: [PATCH 04/11] Prettified users.js --- scm-ui/src/users/modules/users.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index 0792bfa616..af24a02132 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -1,9 +1,9 @@ // @flow -import {apiClient} from "../../apiclient"; -import type {User} from "../types/User"; -import type {UserEntry} from "../types/UserEntry"; -import {combineReducers, Dispatch} from "redux"; -import type {Action} from "../../types/Action"; +import { apiClient } from "../../apiclient"; +import type { User } from "../types/User"; +import type { UserEntry } from "../types/UserEntry"; +import { combineReducers, Dispatch } from "redux"; +import type { Action } from "../../types/Action"; export const FETCH_USERS_PENDING = "scm/users/FETCH_USERS_PENDING"; export const FETCH_USERS_SUCCESS = "scm/users/FETCH_USERS_SUCCESS"; From 82760ffb8946cc8d88ff2094dc6a7f598b8d96eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maren=20S=C3=BCwer?= Date: Thu, 26 Jul 2018 10:19:26 +0200 Subject: [PATCH 05/11] update translation --- scm-ui/public/locales/en/users.json | 12 ++++++++++++ scm-ui/src/users/containers/AddUser.js | 11 ++++++----- scm-ui/src/users/containers/SingleUser.js | 21 +++++++++++---------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/scm-ui/public/locales/en/users.json b/scm-ui/public/locales/en/users.json index 7f8e026fb9..2949b4fd2b 100644 --- a/scm-ui/public/locales/en/users.json +++ b/scm-ui/public/locales/en/users.json @@ -26,5 +26,17 @@ }, "user-form": { "submit": "Submit" + }, + "add-user": { + "title": "Create User", + "subtitle": "Create a new user" + }, + "single-user": { + "error-title": "Error", + "error-subtitle": "Unknown user error", + "navigation-label": "Navigation", + "actions-label": "Actions", + "information-label": "Information", + "back-label": "Back" } } diff --git a/scm-ui/src/users/containers/AddUser.js b/scm-ui/src/users/containers/AddUser.js index 54a5f7b419..df7baccde9 100644 --- a/scm-ui/src/users/containers/AddUser.js +++ b/scm-ui/src/users/containers/AddUser.js @@ -6,8 +6,10 @@ import type { User } from "../types/User"; import type { History } from "history"; import { createUser } from "../modules/users"; import { Page } from "../../components/layout"; +import { translate } from "react-i18next"; type Props = { + t: string => string, addUser: (user: User, callback?: () => void) => void, loading?: boolean, error?: Error, @@ -25,13 +27,12 @@ class AddUser extends React.Component { }; render() { - const { loading, error } = this.props; + const { t, loading, error } = this.props; - // TODO i18n return ( @@ -59,4 +60,4 @@ const mapStateToProps = (state, ownProps) => { export default connect( mapStateToProps, mapDispatchToProps -)(AddUser); +)(translate("users")(AddUser)); diff --git a/scm-ui/src/users/containers/SingleUser.js b/scm-ui/src/users/containers/SingleUser.js index 3ba758c503..d4897b4fd7 100644 --- a/scm-ui/src/users/containers/SingleUser.js +++ b/scm-ui/src/users/containers/SingleUser.js @@ -14,8 +14,11 @@ import Loading from "../../components/Loading"; import { Navigation, Section, NavLink } from "../../components/navigation"; import { DeleteUserNavLink, EditUserNavLink } from "./../components/navLinks"; import ErrorPage from "../../components/ErrorPage"; +import { translate } from "react-i18next"; + type Props = { + t: string => string, name: string, userEntry?: UserEntry, match: any, @@ -49,7 +52,7 @@ class SingleUser extends React.Component { }; render() { - const { userEntry } = this.props; + const { t, userEntry } = this.props; if (!userEntry || userEntry.loading) { return ; @@ -58,8 +61,8 @@ class SingleUser extends React.Component { if (userEntry.error) { return ( ); @@ -68,8 +71,6 @@ class SingleUser extends React.Component { const user = userEntry.entry; const url = this.matchedUrl(); - // TODO i18n - return (
@@ -82,13 +83,13 @@ class SingleUser extends React.Component {
-
- +
+
-
+
- +
@@ -125,4 +126,4 @@ const mapDispatchToProps = dispatch => { export default connect( mapStateToProps, mapDispatchToProps -)(SingleUser); +)(translate("users")(SingleUser)); From 0ab0950bb33a1901e9b7263e0ad2afaba15fa178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maren=20S=C3=BCwer?= Date: Thu, 26 Jul 2018 10:20:09 +0200 Subject: [PATCH 06/11] remove unused import --- scm-ui/src/users/types/User.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-ui/src/users/types/User.js b/scm-ui/src/users/types/User.js index 0d85aa179c..b113f7f795 100644 --- a/scm-ui/src/users/types/User.js +++ b/scm-ui/src/users/types/User.js @@ -1,5 +1,5 @@ //@flow -import type { Link, Links } from "../../types/hal"; +import type { Links } from "../../types/hal"; export type User = { displayName: string, From a7ac14616130a66242bc14ad262ea0f1a3e8c754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maren=20S=C3=BCwer?= Date: Thu, 26 Jul 2018 10:26:37 +0200 Subject: [PATCH 07/11] add remark to possible translation of error messages --- scm-ui/src/users/modules/users.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index af24a02132..6434452d11 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -29,6 +29,8 @@ const USERS_URL = "users"; const CONTENT_TYPE_USER = "application/vnd.scmm-user+json;v=2"; +//TODO i18n + //fetch users export function fetchUsers() { From e1b3efb5ff33b1f888018c3d8139e5c3e27a1000 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 26 Jul 2018 13:14:08 +0200 Subject: [PATCH 08/11] Fixed unit test --- .../sonia/scm/api/v2/resources/UserToUserDtoMapper.java | 6 ++++++ .../java/sonia/scm/api/v2/resources/MeResourceTest.java | 7 ++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java index 85973bab75..00aba5a700 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java @@ -1,5 +1,6 @@ package sonia.scm.api.v2.resources; +import com.google.common.annotations.VisibleForTesting; import de.otto.edison.hal.Links; import org.mapstruct.AfterMapping; import org.mapstruct.Mapper; @@ -21,6 +22,11 @@ public abstract class UserToUserDtoMapper extends BaseMapper { @Inject private ResourceLinks resourceLinks; + @VisibleForTesting + void setResourceLinks(ResourceLinks resourceLinks) { + this.resourceLinks = resourceLinks; + } + @AfterMapping void removePassword(@MappingTarget UserDto target) { target.setPassword(UserResource.DUMMY_PASSWORT); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java index 8c08a433f5..56c60098fb 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java @@ -39,8 +39,6 @@ import static org.mockito.Mockito.*; import static org.mockito.MockitoAnnotations.initMocks; @SubjectAware( -// username = "trillian", -// password = "secret", configuration = "classpath:sonia/scm/repository/shiro.ini" ) public class MeResourceTest { @@ -50,6 +48,8 @@ public class MeResourceTest { private Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); + + private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(URI.create("/")); @Mock private UriInfo uriInfo; @Mock @@ -67,9 +67,10 @@ public class MeResourceTest { public void prepareEnvironment() throws IOException, UserException { initMocks(this); createDummyUser("trillian"); - doNothing().when(userManager).create(userCaptor.capture()); + when(userManager.create(userCaptor.capture())).thenAnswer(invocation -> invocation.getArguments()[0]); doNothing().when(userManager).modify(userCaptor.capture()); doNothing().when(userManager).delete(userCaptor.capture()); + userToDtoMapper.setResourceLinks(resourceLinks); MeResource meResource = new MeResource(userToDtoMapper, userManager); dispatcher.getRegistry().addSingletonResource(meResource); when(uriInfo.getBaseUri()).thenReturn(URI.create("/")); From 66bc8b75c50b2aa4b6630a22b4e4f2fae25ad175 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 26 Jul 2018 15:04:50 +0200 Subject: [PATCH 09/11] Added unit test for authentication resource --- .../resources/AuthenticationResourceTest.java | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java new file mode 100644 index 0000000000..d84a117a8a --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java @@ -0,0 +1,142 @@ +package sonia.scm.api.v2.resources; + +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import org.jboss.resteasy.core.Dispatcher; +import org.jboss.resteasy.mock.MockDispatcherFactory; +import org.jboss.resteasy.mock.MockHttpRequest; +import org.jboss.resteasy.mock.MockHttpResponse; +import org.jboss.resteasy.spi.ResteasyProviderFactory; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import sonia.scm.config.ScmConfiguration; +import sonia.scm.security.AccessToken; +import sonia.scm.security.AccessTokenBuilder; +import sonia.scm.security.AccessTokenBuilderFactory; +import sonia.scm.security.AccessTokenCookieIssuer; +import sonia.scm.user.User; +import sonia.scm.user.UserException; +import sonia.scm.user.UserManager; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.UriInfo; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Date; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.*; + +@SubjectAware( + configuration = "classpath:sonia/scm/repository/shiro.ini" +) +@RunWith(MockitoJUnitRunner.class) +public class AuthenticationResourceTest { + + @Rule + public ShiroRule shiro = new ShiroRule(); + + private Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); + + @Mock + private AccessTokenBuilderFactory accessTokenBuilderFactory; + + @Mock + private AccessTokenBuilder accessTokenBuilder; + + private AccessTokenCookieIssuer cookieIssuer = new AccessTokenCookieIssuer(mock(ScmConfiguration.class)); + + private static final String AUTH_JSON_TRILLIAN = "{\n" + + "\t\"cookie\": true,\n" + + "\t\"grant_type\": \"password\",\n" + + "\t\"username\": \"trillian\",\n" + + "\t\"password\": \"secret\"\n" + + "}"; + + private static final String AUTH_JSON_TRILLIAN_WRONG_PW = "{\n" + + "\t\"cookie\": true,\n" + + "\t\"grant_type\": \"password\",\n" + + "\t\"username\": \"trillian\",\n" + + "\t\"password\": \"justWrong\"\n" + + "}"; + + private static final String AUTH_JSON_NOT_EXISTING_USER = "{\n" + + "\t\"cookie\": true,\n" + + "\t\"grant_type\": \"password\",\n" + + "\t\"username\": \"iDoNotExist\",\n" + + "\t\"password\": \"doesNotMatter\"\n" + + "}"; + + @Before + public void prepareEnvironment() { + AuthenticationResource authenticationResource = new AuthenticationResource(accessTokenBuilderFactory, cookieIssuer); + dispatcher.getRegistry().addSingletonResource(authenticationResource); + + AccessToken accessToken = mock(AccessToken.class); + when(accessToken.getExpiration()).thenReturn(new Date(Long.MAX_VALUE)); + when(accessTokenBuilder.build()).thenReturn(accessToken); + + when(accessTokenBuilderFactory.create()).thenReturn(accessTokenBuilder); + + HttpServletRequest servletRequest = mock(HttpServletRequest.class); + ResteasyProviderFactory.getContextDataMap().put(HttpServletRequest.class, servletRequest); + + HttpServletResponse servletResponse = mock(HttpServletResponse.class); + ResteasyProviderFactory.getContextDataMap().put(HttpServletResponse.class, servletResponse); + } + + @Test + public void shouldAuthCorrectly() throws URISyntaxException { + + MockHttpRequest request = getMockHttpRequest(AUTH_JSON_TRILLIAN); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); + } + + @Test + public void shouldNotAuthUserWithWrongPassword() throws URISyntaxException { + + MockHttpRequest request = getMockHttpRequest(AUTH_JSON_TRILLIAN_WRONG_PW); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + } + + @Test + public void shouldNotAuthNonexistingUser() throws URISyntaxException { + + MockHttpRequest request = getMockHttpRequest(AUTH_JSON_NOT_EXISTING_USER); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + } + + private MockHttpRequest getMockHttpRequest(String jsonPayload) throws URISyntaxException { + MockHttpRequest request = MockHttpRequest.post("/" + AuthenticationResource.PATH + "/access_token"); + + request.content(jsonPayload.getBytes()); + request.contentType(MediaType.APPLICATION_JSON_TYPE); + return request; + } + +} From 52b98c196d53d283e4e74b62bb12bc19828e8101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maren=20S=C3=BCwer?= Date: Thu, 26 Jul 2018 15:29:21 +0200 Subject: [PATCH 10/11] add validation --- scm-ui/public/locales/en/users.json | 8 ++ scm-ui/src/components/forms/InputField.js | 16 +++- scm-ui/src/users/components/UserForm.js | 91 ++++++++++++++++++----- 3 files changed, 91 insertions(+), 24 deletions(-) diff --git a/scm-ui/public/locales/en/users.json b/scm-ui/public/locales/en/users.json index 2949b4fd2b..c07b9afc9f 100644 --- a/scm-ui/public/locales/en/users.json +++ b/scm-ui/public/locales/en/users.json @@ -38,5 +38,13 @@ "actions-label": "Actions", "information-label": "Information", "back-label": "Back" + }, + "validation": { + "mail-invalid": "This email is invalid", + "name-invalid": "This name is invalid", + "displayname-invalid": "This displayname is invalid", + "password-invalid": "Password has to be at least six characters", + "passwordValidation-invalid": "Passwords have to be the same", + "validatePassword": "Please validate password here" } } diff --git a/scm-ui/src/components/forms/InputField.js b/scm-ui/src/components/forms/InputField.js index 0bf44e1bdc..8d7e60e23f 100644 --- a/scm-ui/src/components/forms/InputField.js +++ b/scm-ui/src/components/forms/InputField.js @@ -1,5 +1,6 @@ //@flow import React from "react"; +import classNames from "classnames"; type Props = { label?: string, @@ -7,7 +8,9 @@ type Props = { value?: string, type?: string, autofocus?: boolean, - onChange: string => void + onChange: string => void, + validationError: boolean, + errorMessage: string }; class InputField extends React.Component { @@ -37,8 +40,9 @@ class InputField extends React.Component { }; render() { - const { type, placeholder, value } = this.props; - + const { type, placeholder, value, validationError, errorMessage } = this.props; + const errorView = validationError ? "is-danger" : ""; + const helper = validationError ?

{errorMessage}

: ""; return (
{this.renderLabel()} @@ -47,13 +51,17 @@ class InputField extends React.Component { ref={input => { this.field = input; }} - className="input" + className={ classNames( + "input", + errorView + )} type={type} placeholder={placeholder} value={value} onChange={this.handleInput} />
+ {helper} ); } diff --git a/scm-ui/src/users/components/UserForm.js b/scm-ui/src/users/components/UserForm.js index 54c5e04dde..15553a2b3a 100644 --- a/scm-ui/src/users/components/UserForm.js +++ b/scm-ui/src/users/components/UserForm.js @@ -11,33 +11,54 @@ type Props = { t: string => string }; -class UserForm extends React.Component { +type State = { + user: User, + mailValidationError: boolean, + nameValidationError: boolean, + displayNameValidationError: boolean, + passwordValidationError: boolean, + validatePasswordError: boolean, + validatePassword: string +}; + + +class UserForm extends React.Component { constructor(props: Props) { super(props); + this.state = { - name: "", - displayName: "", - mail: "", - password: "", - admin: false, - active: false, - _links: {} + user: { + name: "", + displayName: "", + mail: "", + password: "", + admin: false, + active: false, + _links: {} + }, + mailValidationError: false, + displayNameValidationError: false, + nameValidationError: false, + passwordValidationError: false, + validatePasswordError: false, + validatePassword: "" }; } componentDidMount() { - this.setState({ ...this.props.user }); + this.setState({ user: {...this.props.user} }); } submit = (event: Event) => { event.preventDefault(); - this.props.submitForm(this.state); + this.props.submitForm(this.state.user); }; render() { const { t } = this.props; - const user = this.state; - + const user = this.state.user; + const ButtonClickable = (this.state.validatePasswordError || this.state.nameValidationError || this.state.mailValidationError || this.state.validatePasswordError + || this.state.displayNameValidationError || user.name === undefined|| user.displayName === undefined); let nameField = null; if (!this.props.user) { nameField = ( @@ -45,6 +66,8 @@ class UserForm extends React.Component { label={t("user.name")} onChange={this.handleUsernameChange} value={user ? user.name : ""} + validationError= {this.state.nameValidationError} + errorMessage= {t("validation.name-invalid")} /> ); } @@ -55,17 +78,31 @@ class UserForm extends React.Component { label={t("user.displayName")} onChange={this.handleDisplayNameChange} value={user ? user.displayName : ""} + validationError={this.state.displayNameValidationError} + errorMessage={t("validation.displayname-invalid")} /> + { onChange={this.handleActiveChange} checked={user ? user.active : false} /> - + ); } + handleUsernameChange = (name: string) => { - this.setState({ name }); + const REGEX_NAME = /^[^ ][A-z0-9\\.\-_@ ]*[^ ]$/; + this.setState( {nameValidationError: !REGEX_NAME.test(name), user : {...this.state.user, name} } ); }; - handleDisplayNameChange = (displayName: string) => { - this.setState({ displayName }); + handleDisplayNameChange = (displayName: string) => { + const REGEX_NAME = /^[^ ][A-z0-9\\.\-_@ ]*[^ ]$/; + this.setState({displayNameValidationError: !REGEX_NAME.test(displayName), user : {...this.state.user, displayName} } ); }; handleEmailChange = (mail: string) => { - this.setState({ mail }); + const REGEX_MAIL = /^[A-z0-9][\w.-]*@[A-z0-9][\w\-\\.]*\.[A-z0-9][A-z0-9-]+$/; + this.setState( {mailValidationError: !REGEX_MAIL.test(mail), user : {...this.state.user, mail} } ); }; handlePasswordChange = (password: string) => { - this.setState({ password }); + const validatePasswordError = !this.checkPasswords(password, this.state.validatePassword); + this.setState( {validatePasswordError: (password.length < 6) || (password.length > 32), passwordValidationError: validatePasswordError, user : {...this.state.user, password} } ); }; + handlePasswordValidationChange = (validatePassword: string) => { + const validatePasswordError = this.checkPasswords(this.state.user.password, validatePassword) + this.setState({ validatePassword, passwordValidationError: !validatePasswordError }); + }; + + checkPasswords = (password1: string, password2: string) => { + return (password1 === password2); + } + handleAdminChange = (admin: boolean) => { - this.setState({ admin }); + this.setState({ user : {...this.state.user, admin} }); }; handleActiveChange = (active: boolean) => { - this.setState({ active }); + this.setState({ user : {...this.state.user, active} }); }; } From f5033290e2929dc78c81841376b5202288b402da Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 26 Jul 2018 17:17:53 +0200 Subject: [PATCH 11/11] Extracted AuthenticationRequestDto into own class --- .../resources/AuthenticationRequestDto.java | 53 +++++++++++++++++ .../v2/resources/AuthenticationResource.java | 58 +------------------ 2 files changed, 56 insertions(+), 55 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationRequestDto.java diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationRequestDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationRequestDto.java new file mode 100644 index 0000000000..951863590d --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationRequestDto.java @@ -0,0 +1,53 @@ +package sonia.scm.api.v2.resources; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; + +import javax.ws.rs.FormParam; +import java.util.List; + +public class AuthenticationRequestDto { + + @FormParam("grant_type") + @JsonProperty("grant_type") + private String grantType; + + @FormParam("username") + private String username; + + @FormParam("password") + private String password; + + @FormParam("cookie") + private boolean cookie; + + @FormParam("scope") + private List scope; + + public String getGrantType() { + return grantType; + } + + public String getUsername() { + return username; + } + + public String getPassword() { + return password; + } + + public boolean isCookie() { + return cookie; + } + + public List getScope() { + return scope; + } + + public void validate() { + Preconditions.checkArgument(!Strings.isNullOrEmpty(grantType), "grant_type parameter is required"); + Preconditions.checkArgument(!Strings.isNullOrEmpty(username), "username parameter is required"); + Preconditions.checkArgument(!Strings.isNullOrEmpty(password), "password parameter is required"); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationResource.java index ed9733fdef..bf6bc5d76b 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationResource.java @@ -1,8 +1,5 @@ package sonia.scm.api.v2.resources; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; import com.google.inject.Inject; import com.webcohesion.enunciate.metadata.rs.ResponseCode; import com.webcohesion.enunciate.metadata.rs.StatusCodes; @@ -26,11 +23,7 @@ import javax.ws.rs.core.Response; import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlRootElement; -import java.util.List; -/** - * Created by masuewer on 04.07.18. - */ @Path(AuthenticationResource.PATH) public class AuthenticationResource { @@ -61,7 +54,7 @@ public class AuthenticationResource { public Response authenticateViaForm( @Context HttpServletRequest request, @Context HttpServletResponse response, - @BeanParam AuthenticationRequest authentication + @BeanParam AuthenticationRequestDto authentication ) { return authenticate(request, response, authentication); } @@ -78,7 +71,7 @@ public class AuthenticationResource { public Response authenticateViaJSONBody( @Context HttpServletRequest request, @Context HttpServletResponse response, - AuthenticationRequest authentication + AuthenticationRequestDto authentication ) { return authenticate(request, response, authentication); } @@ -86,7 +79,7 @@ public class AuthenticationResource { private Response authenticate( HttpServletRequest request, HttpServletResponse response, - AuthenticationRequest authentication + AuthenticationRequestDto authentication ) { authentication.validate(); @@ -180,51 +173,6 @@ public class AuthenticationResource { return Response.noContent().build(); } - public static class AuthenticationRequest { - - @FormParam("grant_type") - @JsonProperty("grant_type") - private String grantType; - - @FormParam("username") - private String username; - - @FormParam("password") - private String password; - - @FormParam("cookie") - private boolean cookie; - - @FormParam("scope") - private List scope; - - public String getGrantType() { - return grantType; - } - - public String getUsername() { - return username; - } - - public String getPassword() { - return password; - } - - public boolean isCookie() { - return cookie; - } - - public List getScope() { - return scope; - } - - public void validate() { - Preconditions.checkArgument(!Strings.isNullOrEmpty(grantType), "grant_type parameter is required"); - Preconditions.checkArgument(!Strings.isNullOrEmpty(username), "username parameter is required"); - Preconditions.checkArgument(!Strings.isNullOrEmpty(password), "password parameter is required"); - } - } - private Response handleFailedAuthentication(HttpServletRequest request, AuthenticationException ex, Response.Status status,