Improved flow coverage, fixed bugs and enabled deleting users

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"
   },

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<Response> {
     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<Response> {
     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<Response> {
     let options: RequestOptions = {
       method: method,
       body: JSON.stringify(payload)

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<Props> {
+  componentDidMount() {
     this.props.getAuthState();
   }
   render() {
-    const { login, username } = this.props.login;
-
-    if (!login) {
+    const { login, username, loading } = this.props;
+    if (loading) {
+      return <div>Loading...</div>;
+    } else if (!login) {
       return (
         <div>
           <Login />
@@ -44,7 +46,7 @@ const mapDispatchToProps = dispatch => {
 };
 
 const mapStateToProps = state => {
-  return { login: state.login };
+  return state.login || {};
 };
 
 export default withRouter(

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
       };

scm-ui/src/types/hal.js

@@ -0,0 +1,6 @@
+// @flow
+export type Link = {
+  href: string
+};
+
+export type Links = { [string]: Link };

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<Props> {
-
   deleteUser = () => {
     this.props.deleteUser(this.props.user._links.delete.href);
   };
 
-  if(deleteButtonClicked) {
-    let deleteButtonAsk = <div>You really want to remove this user?</div>
-  }
-
   isDeletable = () => {
     return this.props.user._links.delete;
   };
@@ -28,7 +24,6 @@ class DeleteUser extends React.Component<Props> {
       <button type="button" onClick={this.deleteUser}>
         Delete User
       </button>
-
     );
   }
 }

scm-ui/src/users/containers/DeleteUserButton.test.js

@@ -1,14 +1,13 @@
-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: {}
   };
@@ -17,12 +16,11 @@ it('should render nothing, if the delete link is missing', () => {
   expect(button.text()).toBe("");
 });
 
-it('should render the button', () => {
-
+it("should render the button", () => {
   const user = {
     _links: {
-      "delete": {
-        "href": "/users"
+      delete: {
+        href: "/users"
       }
     }
   };
@@ -31,12 +29,11 @@ it('should render the button', () => {
   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"
       }
     }
   };

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<Props> {
   render() {
+    const { user, deleteUser } = this.props;
     return (
       <tr>
-        <td>{this.props.user.displayName}</td>
-        <td>{this.props.user.mail}</td>
+        <td>{user.name}</td>
+        <td>{user.displayName}</td>
+        <td>{user.mail}</td>
         <td>
-          <input type="checkbox" id="admin" checked={this.props.user.admin} />
+          <input type="checkbox" id="admin" checked={user.admin} readOnly />
         </td>
         <td>
-          <DeleteUserButton user={this.props.user} />
+          <DeleteUserButton user={user} deleteUser={deleteUser} />
         </td>
       </tr>
     );

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<User>,
   fetchUsers: () => void,
-  fetchUsersIfNeeded: (url: string) => void,
-
+  deleteUser: string => void
 };
 
 class Users extends React.Component<Props> {
-  componentWillMount() {
-    this.props.fetchUsersIfNeeded();
-  }
-
-  componentDidUpdate() {
-    this.props.fetchUsersIfNeeded();
+  componentDidMount() {
+    this.props.fetchUsers();
   }
 
   render() {
@@ -35,13 +30,20 @@ class Users extends React.Component<Props> {
             <thead>
               <tr>
                 <th>Name</th>
+                <th>Display Name</th>
                 <th>E-Mail</th>
                 <th>Admin</th>
               </tr>
             </thead>
             <tbody>
               {this.props.users.map((user, index) => {
-                return <UserRow key={index} user={user} />;
+                return (
+                  <UserRow
+                    key={index}
+                    user={user}
+                    deleteUser={this.props.deleteUser}
+                  />
+                );
               })}
             </tbody>
           </table>
@@ -61,11 +63,11 @@ const mapStateToProps = state => {
 
 const mapDispatchToProps = dispatch => {
   return {
-    fetchUsersIfNeeded: () => {
-      dispatch(fetchUsersIfNeeded());
-    },
     fetchUsers: () => {
       dispatch(fetchUsers());
+    },
+    deleteUser: (link: string) => {
+      dispatch(deleteUser(link));
     }
   };
 };

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:

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
+};