From e3caa93aa72895f66ccf0873563d090aeda0b1c7 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Wed, 11 Jul 2018 12:02:53 +0200 Subject: [PATCH] Improved flow coverage, fixed bugs and enabled deleting users --- scm-ui/package.json | 3 +- scm-ui/src/apiclient.js | 16 +++++-- scm-ui/src/containers/App.js | 16 ++++--- scm-ui/src/modules/login.js | 25 ++++++++--- scm-ui/src/types/hal.js | 6 +++ .../src/users/containers/DeleteUserButton.js | 9 +--- .../users/containers/DeleteUserButton.test.js | 31 ++++++------- scm-ui/src/users/containers/UserRow.js | 14 +++--- scm-ui/src/users/containers/Users.js | 34 +++++++------- scm-ui/src/users/modules/users.js | 45 +++++++------------ scm-ui/src/users/types/User.js | 10 +++++ 11 files changed, 118 insertions(+), 91 deletions(-) create mode 100644 scm-ui/src/types/hal.js create mode 100644 scm-ui/src/users/types/User.js diff --git a/scm-ui/package.json b/scm-ui/package.json index 84776009b5..5c11dc9813 100644 --- a/scm-ui/package.json +++ b/scm-ui/package.json @@ -5,7 +5,6 @@ "private": true, "dependencies": { "classnames": "^2.2.5", - "flow-bin": "^0.75.0", "history": "^4.7.2", "react": "^16.4.1", "react-dom": "^16.4.1", @@ -34,6 +33,8 @@ "devDependencies": { "enzyme": "^3.3.0", "enzyme-adapter-react-16": "^1.1.1", + "flow-bin": "^0.75.0", + "flow-typed": "^2.5.1", "prettier": "^1.13.7", "react-test-renderer": "^16.4.1" }, diff --git a/scm-ui/src/apiclient.js b/scm-ui/src/apiclient.js index 02d3263fec..0b945d1fbd 100644 --- a/scm-ui/src/apiclient.js +++ b/scm-ui/src/apiclient.js @@ -4,6 +4,7 @@ const apiUrl = process.env.API_URL || process.env.PUBLIC_URL || "/scm"; export const PAGE_NOT_FOUND_ERROR = Error("page not found"); +export const NOT_AUTHENTICATED_ERROR = Error("not authenticated"); const fetchOptions: RequestOptions = { credentials: "same-origin", @@ -15,7 +16,7 @@ const fetchOptions: RequestOptions = { function handleStatusCode(response: Response) { if (!response.ok) { if (response.status === 401) { - return response; + throw NOT_AUTHENTICATED_ERROR; } if (response.status === 404) { throw PAGE_NOT_FOUND_ERROR; @@ -26,11 +27,14 @@ function handleStatusCode(response: Response) { } function createUrl(url: string) { + if (url.indexOf("://") > 0) { + return url; + } return `${apiUrl}/api/rest/v2/${url}`; } class ApiClient { - get(url: string) { + get(url: string): Promise { return fetch(createUrl(url), fetchOptions).then(handleStatusCode); } @@ -38,7 +42,7 @@ class ApiClient { return this.httpRequestWithJSONBody(url, payload, "POST"); } - delete(url: string) { + delete(url: string): Promise { let options: RequestOptions = { method: "DELETE" }; @@ -46,7 +50,11 @@ class ApiClient { return fetch(createUrl(url), options).then(handleStatusCode); } - httpRequestWithJSONBody(url: string, payload: any, method: string) { + httpRequestWithJSONBody( + url: string, + payload: any, + method: string + ): Promise { let options: RequestOptions = { method: method, body: JSON.stringify(payload) diff --git a/scm-ui/src/containers/App.js b/scm-ui/src/containers/App.js index 1ee8df1c98..aa40fdc4a9 100644 --- a/scm-ui/src/containers/App.js +++ b/scm-ui/src/containers/App.js @@ -9,17 +9,19 @@ import { withRouter } from "react-router-dom"; type Props = { login: boolean, username: string, - getAuthState: any + getAuthState: () => void, + loading: boolean }; -class App extends Component { - componentWillMount() { +class App extends Component { + componentDidMount() { this.props.getAuthState(); } render() { - const { login, username } = this.props.login; - - if (!login) { + const { login, username, loading } = this.props; + if (loading) { + return
Loading...
; + } else if (!login) { return (
@@ -44,7 +46,7 @@ const mapDispatchToProps = dispatch => { }; const mapStateToProps = state => { - return { login: state.login }; + return state.login || {}; }; export default withRouter( diff --git a/scm-ui/src/modules/login.js b/scm-ui/src/modules/login.js index b4f505296d..c2bced3891 100644 --- a/scm-ui/src/modules/login.js +++ b/scm-ui/src/modules/login.js @@ -1,6 +1,6 @@ //@flow -import { apiClient } from "../apiclient"; +import { apiClient, NOT_AUTHENTICATED_ERROR } from "../apiclient"; const LOGIN_URL = "/auth/access_token"; const AUTHENTICATION_INFO_URL = "/me"; @@ -21,7 +21,7 @@ export function getIsAuthenticatedRequest() { } export function getIsAuthenticated() { - return function(dispatch: (any) => void) { + return function(dispatch: any => void) { dispatch(getIsAuthenticatedRequest()); return apiClient .get(AUTHENTICATION_INFO_URL) @@ -32,6 +32,13 @@ export function getIsAuthenticated() { if (data) { dispatch(isAuthenticated(data.username)); } + }) + .catch((error: Error) => { + if (error === NOT_AUTHENTICATED_ERROR) { + dispatch(isNotAuthenticated()); + } else { + // TODO: Handle errors other than not_authenticated + } }); }; } @@ -60,9 +67,9 @@ export function login(username: string, password: string) { cookie: true, grant_type: "password", username, - password, + password }; - return function(dispatch: (any) => void) { + return function(dispatch: any => void) { dispatch(loginRequest()); return apiClient.post(LOGIN_URL, login_data).then(response => { if (response.ok) { @@ -79,23 +86,29 @@ export function loginSuccessful() { }; } -export default function reducer(state: any = {}, action: any = {}) { +export default function reducer( + state: any = { loading: true }, + action: any = {} +) { switch (action.type) { case LOGIN: return { ...state, + loading: true, login: false, error: null }; case LOGIN_SUCCESSFUL: return { ...state, + loading: false, login: true, error: null }; case LOGIN_FAILED: return { ...state, + loading: false, login: false, error: action.payload }; @@ -103,12 +116,14 @@ export default function reducer(state: any = {}, action: any = {}) { return { ...state, login: true, + loading: false, username: action.username }; case IS_NOT_AUTHENTICATED: return { ...state, login: false, + loading: false, username: null, error: null }; diff --git a/scm-ui/src/types/hal.js b/scm-ui/src/types/hal.js new file mode 100644 index 0000000000..4c68bbab93 --- /dev/null +++ b/scm-ui/src/types/hal.js @@ -0,0 +1,6 @@ +// @flow +export type Link = { + href: string +}; + +export type Links = { [string]: Link }; diff --git a/scm-ui/src/users/containers/DeleteUserButton.js b/scm-ui/src/users/containers/DeleteUserButton.js index c26dc56fb6..15e14bc7f6 100644 --- a/scm-ui/src/users/containers/DeleteUserButton.js +++ b/scm-ui/src/users/containers/DeleteUserButton.js @@ -1,21 +1,17 @@ // @flow import React from "react"; +import type { User } from "../types/User"; type Props = { - user: any, + user: User, deleteUser: (link: string) => void }; class DeleteUser extends React.Component { - deleteUser = () => { this.props.deleteUser(this.props.user._links.delete.href); }; - if(deleteButtonClicked) { - let deleteButtonAsk =
You really want to remove this user?
- } - isDeletable = () => { return this.props.user._links.delete; }; @@ -28,7 +24,6 @@ class DeleteUser extends React.Component { - ); } } diff --git a/scm-ui/src/users/containers/DeleteUserButton.test.js b/scm-ui/src/users/containers/DeleteUserButton.test.js index efda995c86..4e1a1af663 100644 --- a/scm-ui/src/users/containers/DeleteUserButton.test.js +++ b/scm-ui/src/users/containers/DeleteUserButton.test.js @@ -1,42 +1,39 @@ -import React from 'react'; -import {configure, shallow} from 'enzyme'; +import React from "react"; +import { configure, shallow } from "enzyme"; import DeleteUserButton from "./DeleteUserButton"; -import Adapter from 'enzyme-adapter-react-16'; +import Adapter from "enzyme-adapter-react-16"; -import 'raf/polyfill'; +import "raf/polyfill"; configure({ adapter: new Adapter() }); -it('should render nothing, if the delete link is missing', () => { - +it("should render nothing, if the delete link is missing", () => { const user = { _links: {} }; - const button = shallow(); + const button = shallow(); expect(button.text()).toBe(""); }); -it('should render the button', () => { - +it("should render the button", () => { const user = { _links: { - "delete": { - "href": "/users" + delete: { + href: "/users" } } }; - const button = shallow(); + const button = shallow(); expect(button.text()).not.toBe(""); }); -it('should call the delete user function with delete url', () => { - +it("should call the delete user function with delete url", () => { const user = { _links: { - "delete": { - "href": "/users" + delete: { + href: "/users" } } }; @@ -47,7 +44,7 @@ it('should call the delete user function with delete url', () => { calledUrl = url; } - const button = shallow(); + const button = shallow(); button.simulate("click"); expect(calledUrl).toBe("/users"); diff --git a/scm-ui/src/users/containers/UserRow.js b/scm-ui/src/users/containers/UserRow.js index 5431178ba9..2f2dba001b 100644 --- a/scm-ui/src/users/containers/UserRow.js +++ b/scm-ui/src/users/containers/UserRow.js @@ -1,22 +1,26 @@ // @flow import React from "react"; import DeleteUserButton from "./DeleteUserButton"; +import type { User } from "../types/User"; type Props = { - user: any + user: User, + deleteUser: string => void }; export default class UserRow extends React.Component { render() { + const { user, deleteUser } = this.props; return ( - {this.props.user.displayName} - {this.props.user.mail} + {user.name} + {user.displayName} + {user.mail} - + - + ); diff --git a/scm-ui/src/users/containers/Users.js b/scm-ui/src/users/containers/Users.js index 0702a56245..9c9914e6dd 100644 --- a/scm-ui/src/users/containers/Users.js +++ b/scm-ui/src/users/containers/Users.js @@ -2,27 +2,22 @@ import React from "react"; import { connect } from "react-redux"; -import { fetchUsersIfNeeded, fetchUsers } from "../modules/users"; +import { fetchUsers, deleteUser } from "../modules/users"; import Login from "../../containers/Login"; import UserRow from "./UserRow"; +import type { User } from "../types/User"; type Props = { login: boolean, - error: any, - users: any, - fetchUsersIfNeeded: () => void, + error: Error, + users: Array, fetchUsers: () => void, - fetchUsersIfNeeded: (url: string) => void, - + deleteUser: string => void }; class Users extends React.Component { - componentWillMount() { - this.props.fetchUsersIfNeeded(); - } - - componentDidUpdate() { - this.props.fetchUsersIfNeeded(); + componentDidMount() { + this.props.fetchUsers(); } render() { @@ -35,13 +30,20 @@ class Users extends React.Component { Name + Display Name E-Mail Admin {this.props.users.map((user, index) => { - return ; + return ( + + ); })} @@ -61,11 +63,11 @@ const mapStateToProps = state => { const mapDispatchToProps = dispatch => { return { - fetchUsersIfNeeded: () => { - dispatch(fetchUsersIfNeeded()); - }, fetchUsers: () => { dispatch(fetchUsers()); + }, + deleteUser: (link: string) => { + dispatch(deleteUser(link)); } }; }; diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index 1fdc499535..14c4db1851 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -1,5 +1,6 @@ // @flow import { apiClient, PAGE_NOT_FOUND_ERROR } from "../../apiclient"; +import { ThunkDispatch } from "redux-thunk"; const FETCH_USERS = "scm/users/FETCH"; const FETCH_USERS_SUCCESS = "scm/users/FETCH_SUCCESS"; @@ -34,7 +35,7 @@ function usersNotFound(url: string) { } export function fetchUsers() { - return function(dispatch: any => void) { + return function(dispatch: ThunkDispatch) { dispatch(requestUsers()); return apiClient .get(USERS_URL) @@ -66,21 +67,6 @@ function fetchUsersSuccess(users: any) { }; } -export function shouldFetchUsers(state: any): boolean { - if (state.users.users == null) { - return true; - } - return false; -} - -export function fetchUsersIfNeeded() { - return (dispatch, getState) => { - if (shouldFetchUsers(getState())) { - dispatch(fetchUsers()); - } - }; -} - function requestDeleteUser(url: string) { return { type: DELETE_USER, @@ -102,41 +88,42 @@ function deleteUserFailure(url: string, err: Error) { }; } -export function deleteUser(username: string) { - return function(dispatch) { - dispatch(requestDeleteUser(username)); +export function deleteUser(link: string) { + return function(dispatch: ThunkDispatch) { + dispatch(requestDeleteUser(link)); return apiClient - .delete(USERS_URL + "/" + username) + .delete(link) .then(() => { dispatch(deleteUserSuccess()); + dispatch(fetchUsers()); }) - .catch(err => dispatch(deleteUserFailure(username, err))); + .catch(err => dispatch(deleteUserFailure(link, err))); }; } export default function reducer(state: any = {}, action: any = {}) { switch (action.type) { case FETCH_USERS: + case DELETE_USER: return { ...state, - users: null + users: null, + loading: true }; case FETCH_USERS_SUCCESS: return { ...state, error: null, - users: action.payload._embedded.users + users: action.payload._embedded.users, + loading: false }; case FETCH_USERS_FAILURE: + case DELETE_USER_FAILURE: return { ...state, login: false, - error: action.payload - }; - case DELETE_USER_SUCCESS: - return { - ...state, - users: null + error: action.payload, + loading: false }; default: diff --git a/scm-ui/src/users/types/User.js b/scm-ui/src/users/types/User.js new file mode 100644 index 0000000000..9cf4e79728 --- /dev/null +++ b/scm-ui/src/users/types/User.js @@ -0,0 +1,10 @@ +//@flow +import type { Link, Links } from "../../types/hal"; + +export type User = { + displayName: string, + name: string, + mail: string, + admin: boolean, + _links: Links +};