restructure ui for users

scm-ui/src/components/Checkbox.js

@@ -2,13 +2,15 @@
 import React from "react";
 
 type Props = {
-  label: string,
+  label?: string,
   checked: boolean,
-  onChange: boolean => void
+  onChange?: boolean => void
 };
 class Checkbox extends React.Component<Props> {
   onCheckboxChange = (event: SyntheticInputEvent<HTMLInputElement>) => {
+    if (this.props.onChange) {
       this.props.onChange(event.target.checked);
+    }
   };
 
   render() {

scm-ui/src/components/SecondaryNavigation/NavAction.js

@@ -0,0 +1,20 @@
+//@flow
+import React from "react";
+
+type Props = {
+  label: string,
+  action: () => void
+};
+
+class NavAction extends React.Component<Props> {
+  render() {
+    const { label, action } = this.props;
+    return (
+      <li>
+        <a onClick={action}>{label}</a>
+      </li>
+    );
+  }
+}
+
+export default NavAction;

scm-ui/src/components/SecondaryNavigation/NavLink.js

@@ -0,0 +1,37 @@
+//@flow
+import * as React from "react";
+import { Route, Link } from "react-router-dom";
+
+// TODO mostly copy of PrimaryNavigationLink
+
+type Props = {
+  to: string,
+  label: string,
+  activeOnlyWhenExact?: boolean
+};
+
+class NavLink extends React.Component<Props> {
+  static defaultProps = {
+    activeOnlyWhenExact: true
+  };
+
+  renderLink = (route: any) => {
+    const { to, label } = this.props;
+    return (
+      <li>
+        <Link className={route.match ? "is-active" : ""} to={to}>
+          {label}
+        </Link>
+      </li>
+    );
+  };
+
+  render() {
+    const { to, activeOnlyWhenExact } = this.props;
+    return (
+      <Route path={to} exact={activeOnlyWhenExact} children={this.renderLink} />
+    );
+  }
+}
+
+export default NavLink;

scm-ui/src/components/SecondaryNavigation/Navigation.js

@@ -0,0 +1,14 @@
+//@flow
+import * as React from "react";
+
+type Props = {
+  children?: React.Node
+};
+
+class Navigation extends React.Component<Props> {
+  render() {
+    return <aside className="menu">{this.props.children}</aside>;
+  }
+}
+
+export default Navigation;

scm-ui/src/components/SecondaryNavigation/Section.js

@@ -0,0 +1,21 @@
+//@flow
+import * as React from "react";
+
+type Props = {
+  label: string,
+  children?: React.Node
+};
+
+class Section extends React.Component<Props> {
+  render() {
+    const { label, children } = this.props;
+    return (
+      <div>
+        <p className="menu-label">{label}</p>
+        <ul className="menu-list">{children}</ul>
+      </div>
+    );
+  }
+}
+
+export default Section;

scm-ui/src/components/SecondaryNavigation/index.js

@@ -0,0 +1,4 @@
+export { default as Navigation } from "./Navigation";
+export { default as Section } from "./Section";
+export { default as NavLink } from "./NavLink";
+export { default as NavAction } from "./NavAction";

scm-ui/src/containers/Main.js

@@ -10,8 +10,8 @@ import Logout from "../containers/Logout";
 
 import { Switch } from "react-router-dom";
 import ProtectedRoute from "../components/ProtectedRoute";
-import EditUser from "../users/containers/EditUser";
 import AddUser from "../users/containers/AddUser";
+import SingleUser from "../users/containers/SingleUser";
 
 type Props = {
   authenticated?: boolean
@@ -39,13 +39,13 @@ class Main extends React.Component<Props> {
           />
           <ProtectedRoute
             authenticated={authenticated}
-            path="/users/edit/:name"
-            component={EditUser}
+            path="/users/add"
+            component={AddUser}
           />
           <ProtectedRoute
             authenticated={authenticated}
-            path="/users/add"
-            component={AddUser}
+            path="/user/:name"
+            component={SingleUser}
           />
         </Switch>
       </div>

scm-ui/src/users/containers/AddUser.js

@@ -3,33 +3,48 @@ import React from "react";
 import { connect } from "react-redux";
 import UserForm from "./UserForm";
 import type { User } from "../types/User";
-
+import type { History } from "history";
 import { createUser } from "../modules/users";
+import Page from "../../components/Page";
 
 type Props = {
-  addUser: User => void,
-  loading?: boolean
+  addUser: (user: User, callback?: () => void) => void,
+  loading?: boolean,
+  error?: Error,
+  history: History
 };
 
 class AddUser extends React.Component<Props> {
-  render() {
-    const addUser = this.props.addUser;
+  userCreated = () => {
+    const { history } = this.props;
+    history.push("/users");
+  };
 
+  createUser = (user: User) => {
+    this.props.addUser(user, this.userCreated);
+  };
+
+  render() {
+    const { loading, error } = this.props;
+
+    // TODO i18n
     return (
-      <div>
-        <UserForm
-          submitForm={user => addUser(user)}
-          loading={this.props.loading}
-        />
-      </div>
+      <Page
+        title="Create User"
+        subtitle="Create a new user"
+        loading={loading}
+        error={error}
+      >
+        <UserForm submitForm={user => this.createUser(user)} />
+      </Page>
     );
   }
 }
 
 const mapDispatchToProps = dispatch => {
   return {
-    addUser: (user: User) => {
-      dispatch(createUser(user));
+    addUser: (user: User, callback?: () => void) => {
+      dispatch(createUser(user, callback));
     }
   };
 };

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

@@ -2,12 +2,11 @@
 import React from "react";
 import { translate } from "react-i18next";
 import type { User } from "../types/User";
-import type { UserEntry } from "../types/UserEntry";
 import { confirmAlert } from "../../components/ConfirmAlert";
-import DeleteButton from "../../components/DeleteButton";
+import { NavAction } from "../../components/SecondaryNavigation";
 
 type Props = {
-  entry: UserEntry,
+  user: User,
   confirmDialog?: boolean,
   t: string => string,
   deleteUser: (user: User) => void
@@ -19,7 +18,7 @@ class DeleteUserButton extends React.Component<Props> {
   };
 
   deleteUser = () => {
-    this.props.deleteUser(this.props.entry.entry);
+    this.props.deleteUser(this.props.user);
   };
 
   confirmDelete = () => {
@@ -41,23 +40,17 @@ class DeleteUserButton extends React.Component<Props> {
   };
 
   isDeletable = () => {
-    return this.props.entry.entry._links.delete;
+    return this.props.user._links.delete;
   };
 
   render() {
-    const { confirmDialog, entry, t } = this.props;
+    const { confirmDialog, t } = this.props;
     const action = confirmDialog ? this.confirmDelete : this.deleteUser;
 
     if (!this.isDeletable()) {
       return null;
     }
-    return (
-      <DeleteButton
-        label={t("delete-user-button.label")}
-        action={action}
-        loading={entry.loading}
-      />
-    );
+    return <NavAction label={t("delete-user-button.label")} action={action} />;
   }
 }
 

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

@@ -9,63 +9,55 @@ jest.mock("../../components/ConfirmAlert");
 
 describe("DeleteUserButton", () => {
   it("should render nothing, if the delete link is missing", () => {
-    const entry = {
-      entry: {
+    const user = {
       _links: {}
-      }
     };
 
     const button = shallow(
-      <DeleteUserButton entry={entry} deleteUser={() => {}} />
+      <DeleteUserButton user={user} deleteUser={() => {}} />
     );
     expect(button.text()).toBe("");
   });
 
   it("should render the button", () => {
-    const entry = {
-      entry: {
+    const user = {
       _links: {
         delete: {
           href: "/users"
         }
       }
-      }
     };
 
     const button = mount(
-      <DeleteUserButton entry={entry} deleteUser={() => {}} />
+      <DeleteUserButton user={user} deleteUser={() => {}} />
     );
     expect(button.text()).not.toBe("");
   });
 
   it("should open the confirm dialog on button click", () => {
-    const entry = {
-      entry: {
+    const user = {
       _links: {
         delete: {
           href: "/users"
         }
       }
-      }
     };
 
     const button = mount(
-      <DeleteUserButton entry={entry} deleteUser={() => {}} />
+      <DeleteUserButton user={user} deleteUser={() => {}} />
     );
-    button.simulate("click");
+    button.find("a").simulate("click");
 
     expect(confirmAlert.mock.calls.length).toBe(1);
   });
 
   it("should call the delete user function with delete url", () => {
-    const entry = {
-      entry: {
+    const user = {
       _links: {
         delete: {
           href: "/users"
         }
       }
-      }
     };
 
     let calledUrl = null;
@@ -75,12 +67,12 @@ describe("DeleteUserButton", () => {
 
     const button = mount(
       <DeleteUserButton
-        entry={entry}
+        user={user}
         confirmDialog={false}
         deleteUser={capture}
       />
     );
-    button.simulate("click");
+    button.find("a").simulate("click");
 
     expect(calledUrl).toBe("/users");
   });

scm-ui/src/users/containers/Details.js

@@ -0,0 +1,48 @@
+//@flow
+import React from "react";
+import type { User } from "../types/User";
+import { translate } from "react-i18next";
+import Checkbox from "../../components/Checkbox";
+
+type Props = {
+  user: User,
+  t: string => string
+};
+
+class Details extends React.Component<Props> {
+  render() {
+    const { user, t } = this.props;
+    return (
+      <table className="table">
+        <tbody>
+          <tr>
+            <td>{t("user.name")}</td>
+            <td>{user.name}</td>
+          </tr>
+          <tr>
+            <td>{t("user.displayName")}</td>
+            <td>{user.displayName}</td>
+          </tr>
+          <tr>
+            <td>{t("user.mail")}</td>
+            <td>{user.mail}</td>
+          </tr>
+          <tr>
+            <td>{t("user.admin")}</td>
+            <td>
+              <Checkbox checked={user.admin} />
+            </td>
+          </tr>
+          <tr>
+            <td>{t("user.active")}</td>
+            <td>
+              <Checkbox checked={user.active} />
+            </td>
+          </tr>
+        </tbody>
+      </table>
+    );
+  }
+}
+
+export default translate("users")(Details);

scm-ui/src/users/containers/EditUser.js

@@ -3,50 +3,23 @@ import React from "react";
 import { connect } from "react-redux";
 import UserForm from "./UserForm";
 import type { User } from "../types/User";
-import type { UserEntry } from "../types/UserEntry";
-import Loading from "../../components/Loading";
-
-import { modifyUser, fetchUser } from "../modules/users";
+import { modifyUser } from "../modules/users";
 
 type Props = {
-  name: string,
-  fetchUser: string => void,
-  userEntry?: UserEntry,
+  user: User,
   updateUser: User => void,
   loading: boolean
 };
 
 class EditUser extends React.Component<Props> {
-  componentDidMount() {
-    this.props.fetchUser(this.props.name);
-  }
-
   render() {
-    const submitUser = this.props.updateUser;
-
-    const { userEntry } = this.props;
-
-    if (!userEntry || userEntry.loading) {
-      return <Loading />;
-    } else {
-      return (
-        <div>
-          <UserForm
-            submitForm={user => submitUser(user)}
-            user={userEntry.entry}
-            loading={userEntry.loading}
-          />
-        </div>
-      );
-    }
+    const { user, updateUser } = this.props;
+    return <UserForm submitForm={user => updateUser(user)} user={user} />;
   }
 }
 
 const mapDispatchToProps = dispatch => {
   return {
-    fetchUser: (name: string) => {
-      dispatch(fetchUser(name));
-    },
     updateUser: (user: User) => {
       dispatch(modifyUser(user));
     }
@@ -54,16 +27,7 @@ const mapDispatchToProps = dispatch => {
 };
 
 const mapStateToProps = (state, ownProps) => {
-  const name = ownProps.match.params.name;
-  let userEntry;
-  if (state.users && state.users.usersByNames) {
-    userEntry = state.users.usersByNames[name];
-  }
-
-  return {
-    name,
-    userEntry
-  };
+  return {};
 };
 
 export default connect(

scm-ui/src/users/containers/SingleUser.js

@@ -0,0 +1,118 @@
+//@flow
+import React from "react";
+import { connect } from "react-redux";
+import Page from "../../components/Page";
+import { Route } from "react-router";
+import Details from "./Details";
+import EditUser from "./EditUser";
+import type { User } from "../types/User";
+import type { UserEntry } from "../types/UserEntry";
+import { fetchUser, deleteUser } from "../modules/users";
+import Loading from "../../components/Loading";
+
+import {
+  Navigation,
+  Section,
+  NavLink
+} from "../../components/SecondaryNavigation";
+import DeleteUserButton from "./DeleteUserButton";
+import ErrorPage from "../../components/ErrorPage";
+
+type Props = {
+  name: string,
+  userEntry?: UserEntry,
+  match: any,
+  deleteUser: (user: User) => void,
+  fetchUser: string => void
+};
+
+class SingleUser extends React.Component<Props> {
+  componentDidMount() {
+    this.props.fetchUser(this.props.name);
+  }
+
+  stripEndingSlash = (url: string) => {
+    if (url.endsWith("/")) {
+      return url.substring(0, url.length - 2);
+    }
+    return url;
+  };
+
+  render() {
+    const { userEntry, match, deleteUser } = this.props;
+
+    if (!userEntry || userEntry.loading) {
+      return <Loading />;
+    }
+
+    if (userEntry.error) {
+      return (
+        <ErrorPage
+          title="Error"
+          subtitle="Unknown user error"
+          error={userEntry.error}
+        />
+      );
+    }
+
+    const user = userEntry.entry;
+    const url = this.stripEndingSlash(match.url);
+
+    // TODO i18n
+
+    return (
+      <Page title={user.displayName}>
+        <div className="columns">
+          <div className="column is-three-quarters">
+            <Route path={url} exact component={() => <Details user={user} />} />
+            <Route
+              path={`${url}/edit`}
+              component={() => <EditUser user={user} />}
+            />
+          </div>
+          <div className="column">
+            <Navigation>
+              <Section label="Navigation">
+                <NavLink to={`${url}`} label="Information" />
+                <NavLink to={`${url}/edit`} label="Edit" />
+              </Section>
+              <Section label="Actions">
+                <DeleteUserButton user={user} deleteUser={deleteUser} />
+                <NavLink to="/users" label="Back" />
+              </Section>
+            </Navigation>
+          </div>
+        </div>
+      </Page>
+    );
+  }
+}
+
+const mapStateToProps = (state, ownProps) => {
+  const name = ownProps.match.params.name;
+  let userEntry;
+  if (state.users && state.users.usersByNames) {
+    userEntry = state.users.usersByNames[name];
+  }
+
+  return {
+    name,
+    userEntry
+  };
+};
+
+const mapDispatchToProps = dispatch => {
+  return {
+    fetchUser: (name: string) => {
+      dispatch(fetchUser(name));
+    },
+    deleteUser: (user: User) => {
+      dispatch(deleteUser(user));
+    }
+  };
+};
+
+export default connect(
+  mapStateToProps,
+  mapDispatchToProps
+)(SingleUser);

scm-ui/src/users/containers/UserForm.js

@@ -42,7 +42,6 @@ class UserForm extends React.Component<Props, User> {
     const user = this.state;
     if (user) {
       return (
-        <div className="container">
         <form onSubmit={this.submit}>
           <InputField
             label={t("user.name")}
@@ -80,7 +79,6 @@ class UserForm extends React.Component<Props, User> {
             loading={this.props.loading}
           />
         </form>
-        </div>
       );
     } else {
       return <Loading />;

scm-ui/src/users/containers/UserRow.js

@@ -1,33 +1,30 @@
 // @flow
 import React from "react";
-import DeleteUserButton from "./DeleteUserButton";
-import EditUserButton from "./EditUserButton";
+import { Link } from "react-router-dom";
 import type { User } from "../types/User";
-import type { UserEntry } from "../types/UserEntry";
 
 type Props = {
-  entry: UserEntry,
-  deleteUser: User => void
+  user: User
 };
 
 export default class UserRow extends React.Component<Props> {
+  renderLink(to: string, label: string) {
+    return <Link to={to}>{label}</Link>;
+  }
+
   render() {
-    const { entry, deleteUser } = this.props;
-    const user = entry.entry;
+    const { user } = this.props;
+    const to = `/user/${user.name}`;
     return (
       <tr>
-        <td>{user.name}</td>
-        <td>{user.displayName}</td>
-        <td>{user.mail}</td>
+        <td className="is-hidden-mobile">{this.renderLink(to, user.name)}</td>
+        <td>{this.renderLink(to, user.displayName)}</td>
         <td>
+          <a href={`mailto: ${user.mail}`}>{user.mail}</a>
+        </td>
+        <td className="is-hidden-mobile">
           <input type="checkbox" id="admin" checked={user.admin} readOnly />
         </td>
-        <td>
-          <DeleteUserButton entry={entry} deleteUser={deleteUser} />
-        </td>
-        <td>
-          <EditUserButton entry={entry} />
-        </td>
       </tr>
     );
   }

scm-ui/src/users/containers/UserTable.js

@@ -2,36 +2,29 @@
 import React from "react";
 import { translate } from "react-i18next";
 import UserRow from "./UserRow";
-import type { User } from "../types/User";
 import type { UserEntry } from "../types/UserEntry";
 
 type Props = {
   t: string => string,
-  entries: Array<UserEntry>,
-  deleteUser: User => void
+  entries: Array<UserEntry>
 };
 
 class UserTable extends React.Component<Props> {
   render() {
-    const { deleteUser, t } = this.props;
-    const entries = this.props.entries;
+    const { entries, t } = this.props;
     return (
       <table className="table is-hoverable is-fullwidth">
         <thead>
           <tr>
-            <th>{t("user.name")}</th>
+            <th className="is-hidden-mobile">{t("user.name")}</th>
             <th>{t("user.displayName")}</th>
             <th>{t("user.mail")}</th>
-            <th>{t("user.admin")}</th>
-            <th />
-            <th />
+            <th className="is-hidden-mobile">{t("user.admin")}</th>
           </tr>
         </thead>
         <tbody>
           {entries.map((entry, index) => {
-            return (
-              <UserRow key={index} entry={entry} deleteUser={deleteUser} />
-            );
+            return <UserRow key={index} user={entry.entry} />;
           })}
         </tbody>
       </table>

scm-ui/src/users/containers/Users.js

@@ -3,7 +3,7 @@ import React from "react";
 import { connect } from "react-redux";
 import { translate } from "react-i18next";
 
-import { fetchUsers, deleteUser, getUsersFromState } from "../modules/users";
+import { fetchUsers, getUsersFromState } from "../modules/users";
 import Page from "../../components/Page";
 import UserTable from "./UserTable";
 import type { User } from "../types/User";
@@ -16,7 +16,6 @@ type Props = {
   t: string => string,
   userEntries: Array<UserEntry>,
   fetchUsers: () => void,
-  deleteUser: User => void,
   canAddUsers: boolean
 };
 
@@ -26,7 +25,7 @@ class Users extends React.Component<Props, User> {
   }
 
   render() {
-    const { userEntries, deleteUser, loading, t, error } = this.props;
+    const { userEntries, loading, t, error } = this.props;
     return (
       <Page
         title={t("users.title")}
@@ -34,7 +33,7 @@ class Users extends React.Component<Props, User> {
         loading={loading || !userEntries}
         error={error}
       >
-        <UserTable entries={userEntries} deleteUser={deleteUser} />
+        <UserTable entries={userEntries} />
         {this.renderAddButton()}
       </Page>
     );
@@ -76,9 +75,6 @@ const mapDispatchToProps = dispatch => {
   return {
     fetchUsers: () => {
       dispatch(fetchUsers());
-    },
-    deleteUser: (user: User) => {
-      dispatch(deleteUser(user));
     }
   };
 };

scm-ui/src/users/modules/users.js

@@ -78,7 +78,6 @@ export function fetchUsersFailure(url: string, error: Error): Action {
 }
 
 //fetch user
-//TODO: fetchUsersPending and FetchUsersFailure are the wrong functions here!
 export function fetchUser(name: string) {
   const userUrl = USERS_URL + "/" + name;
   return function(dispatch: any) {
@@ -98,7 +97,7 @@ export function fetchUser(name: string) {
       })
       .catch(cause => {
         const error = new Error(`could not fetch user: ${cause.message}`);
-        dispatch(fetchUserFailure(USERS_URL, error));
+        dispatch(fetchUserFailure(name, error));
       });
   };
 }
@@ -129,14 +128,16 @@ export function fetchUserFailure(username: string, error: Error): Action {
 
 //create user
 
-export function createUser(user: User) {
+export function createUser(user: User, callback?: () => void) {
   return function(dispatch: Dispatch) {
     dispatch(createUserPending(user));
     return apiClient
       .postWithContentType(USERS_URL, user, CONTENT_TYPE_USER)
       .then(() => {
         dispatch(createUserSuccess());
-        dispatch(fetchUsers());
+        if (callback) {
+          callback();
+        }
       })
       .catch(err =>
         dispatch(
@@ -224,7 +225,6 @@ export function deleteUser(user: User) {
       .delete(user._links.delete.href)
       .then(() => {
         dispatch(deleteUserSuccess(user));
-        dispatch(fetchUsers());
       })
       .catch(cause => {
         const error = new Error(
@@ -382,7 +382,7 @@ export default function reducer(state: any = {}, action: any = {}) {
 
     case FETCH_USER_FAILURE:
       return reduceUsersByNames(state, action.payload.username, {
-        loading: true,
+        loading: false,
         error: action.payload.error
       });
 
@@ -411,18 +411,12 @@ export default function reducer(state: any = {}, action: any = {}) {
       };
 
     case DELETE_USER_FAILURE:
-      const newState = reduceUsersByNames(state, action.payload.user.name, {
+      return reduceUsersByNames(state, action.payload.user.name, {
         loading: false,
         error: action.payload.error,
         entry: action.payload.user
       });
-      return {
-        ...newState,
-        users: {
-          ...newState.users,
-          error: action.payload.error
-        }
-      };
+
     // Add single user cases
     case CREATE_USER_PENDING:
       return {

scm-ui/src/users/modules/users.test.js

@@ -209,15 +209,11 @@ 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();
       expect(actions[0].type).toEqual(CREATE_USER_PENDING);
       expect(actions[1].type).toEqual(CREATE_USER_SUCCESS);
-      expect(actions[2].type).toEqual(FETCH_USERS_PENDING);
     });
   });
 
@@ -235,6 +231,24 @@ describe("users fetch()", () => {
     });
   });
 
+  it("should call the callback after user successfully created", () => {
+    // unmatched
+    fetchMock.postOnce(USERS_URL, {
+      status: 204
+    });
+
+    let callMe = "not yet";
+
+    const callback = () => {
+      callMe = "yeah";
+    };
+
+    const store = mockStore({});
+    return store.dispatch(createUser(userZaphod, callback)).then(() => {
+      expect(callMe).toBe("yeah");
+    });
+  });
+
   it("successfully update user", () => {
     fetchMock.putOnce("http://localhost:8081/scm/api/rest/v2/users/zaphod", {
       status: 204
@@ -269,8 +283,6 @@ 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(() => {
@@ -278,7 +290,6 @@ describe("users fetch()", () => {
       expect(actions[0].type).toEqual(DELETE_USER);
       expect(actions[0].payload).toBe(userZaphod);
       expect(actions[1].type).toEqual(DELETE_USER_SUCCESS);
-      expect(actions[2].type).toEqual(FETCH_USERS_PENDING);
     });
   });
 
@@ -458,11 +469,19 @@ describe("users reducer", () => {
   });
 
   it("should update state according to FETCH_USER_FAILURE action", () => {
+    const error = new Error("kaputt!");
     const newState = reducer(
-      {},
-      fetchUserFailure(userFord.name, new Error("kaputt!"))
+      {
+        usersByNames: {
+          ford: {
+            loading: true
+          }
+        }
+      },
+      fetchUserFailure(userFord.name, error)
     );
-    expect(newState.usersByNames["ford"].error).toBeTruthy();
+    expect(newState.usersByNames["ford"].error).toBe(error);
+    expect(newState.usersByNames["ford"].loading).toBeFalsy();
   });
 
   it("should update state according to FETCH_USER_SUCCESS action", () => {