simplify users by using selectors

scm-ui/src/types/Collection.js

@@ -10,3 +10,9 @@ export type PagedCollection = Collection & {
   page: number,
   pageTotal: number
 };
+
+export type PageCollectionStateSlice = {
+  entry?: PagedCollection,
+  error?: Error,
+  loading?: boolean
+};

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

@@ -8,33 +8,33 @@ import {
   fetchUsersByPage,
   fetchUsersByLink,
   getUsersFromState,
-  selectList
+  selectListAsCollection,
+  isPermittedToCreateUsers
 } from "../modules/users";
 
 import { Page } from "../../components/layout";
 import { UserTable } from "./../components/table";
-import type { User } from "../types/User";
-import { AddButton } from "../../components/buttons";
 import type { UserEntry } from "../types/UserEntry";
-import type { PagedCollection } from "../../types/Collection";
+import type { PageCollectionStateSlice } from "../../types/Collection";
 import Paginator from "../../components/Paginator";
 import CreateUserButton from "../components/buttons/CreateUserButton";
 
 type Props = {
-  loading?: boolean,
+  userEntries: UserEntry[],
-  error: Error,
-  t: string => string,
-  userEntries: Array<UserEntry>,
-  fetchUsersByPage: (page: number) => void,
-  fetchUsersByLink: (link: string) => void,
   canAddUsers: boolean,
-  list?: PagedCollection,
+  list: PageCollectionStateSlice,
+  page: number,
+
+  // context objects
+  t: string => string,
   history: History,
-  match: any,
+
-  page: number
+  // dispatch functions
+  fetchUsersByPage: (page: number) => void,
+  fetchUsersByLink: (link: string) => void
 };
 
-class Users extends React.Component<Props, User> {
+class Users extends React.Component<Props> {
   componentDidMount() {
     this.props.fetchUsersByPage(this.props.page);
   }
@@ -43,11 +43,14 @@ class Users extends React.Component<Props, User> {
     this.props.fetchUsersByLink(link);
   };
 
+  /**
+   * reflect page transitions in the uri
+   */
   componentDidUpdate = (prevProps: Props) => {
     const { page, list } = this.props;
-    if (list) {
+    if (list.entry) {
-      // backend starts with 0
+      // backend starts paging by 0
-      const statePage: number = list.page + 1;
+      const statePage: number = list.entry.page + 1;
       if (page !== statePage) {
         this.props.history.push(`/users/${statePage}`);
       }
@@ -55,30 +58,32 @@ class Users extends React.Component<Props, User> {
   };
 
   render() {
-    const { userEntries, list, loading, t, error } = this.props;
+    const { userEntries, list, t } = this.props;
     return (
       <Page
         title={t("users.title")}
         subtitle={t("users.subtitle")}
-        loading={loading || !userEntries}
+        loading={list.loading || !userEntries}
-        error={error}
+        error={list.error}
       >
         <UserTable entries={userEntries} />
         {this.renderPaginator()}
-        {this.renderAddButton()}
+        {this.renderCreateButton()}
       </Page>
     );
   }
 
   renderPaginator() {
     const { list } = this.props;
-    if (list) {
+    if (list.entry) {
-      return <Paginator collection={list} onPageChange={this.onPageChange} />;
+      return (
+        <Paginator collection={list.entry} onPageChange={this.onPageChange} />
+      );
     }
     return null;
   }
 
-  renderAddButton() {
+  renderCreateButton() {
     if (this.props.canAddUsers) {
       return <CreateUserButton />;
     } else {
@@ -87,27 +92,23 @@ class Users extends React.Component<Props, User> {
   }
 }
 
-const mapStateToProps = (state, ownProps) => {
+const getPageFromProps = props => {
-  let page = ownProps.match.params.page;
+  let page = props.match.params.page;
   if (page) {
     page = parseInt(page, 10);
   } else {
     page = 1;
   }
+  return page;
+};
+
+const mapStateToProps = (state, ownProps) => {
+  const page = getPageFromProps(ownProps);
   const userEntries = getUsersFromState(state);
-  let error = null;
+  const canAddUsers = isPermittedToCreateUsers(state);
-  let loading = false;
+  const list = selectListAsCollection(state);
-  let canAddUsers = false;
-  if (state.users && state.users.list) {
-    error = state.users.list.error;
-    canAddUsers = state.users.list.userCreatePermission;
-    loading = state.users.list.loading;
-  }
-  const list = selectList(state);
   return {
     userEntries,
-    error,
-    loading,
     canAddUsers,
     list,
     page

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

@@ -4,6 +4,7 @@ import type { User } from "../types/User";
 import type { UserEntry } from "../types/UserEntry";
 import { Dispatch } from "redux";
 import type { Action } from "../../types/Action";
+import type { PageCollectionStateSlice } from "../../types/Collection";
 
 export const FETCH_USERS_PENDING = "scm/users/FETCH_USERS_PENDING";
 export const FETCH_USERS_SUCCESS = "scm/users/FETCH_USERS_SUCCESS";
@@ -345,12 +346,14 @@ export default function reducer(state: any = {}, action: any = {}) {
         ...state,
         list: {
           error: null,
-          entries: userNames,
           loading: false,
-          userCreatePermission: action.payload._links.create ? true : false,
+          entries: userNames,
-          page: action.payload.page,
+          entry: {
-          pageTotal: action.payload.pageTotal,
+            userCreatePermission: action.payload._links.create ? true : false,
-          _links: action.payload._links
+            page: action.payload.page,
+            pageTotal: action.payload.pageTotal,
+            _links: action.payload._links
+          }
         },
         byNames
       };
@@ -458,8 +461,31 @@ export default function reducer(state: any = {}, action: any = {}) {
 
 // selectors
 
-export const selectList = (state: Object) => {
+const selectList = (state: Object) => {
-  if (state.users && state.users.list && state.users.list._links) {
+  if (state.users && state.users.list) {
     return state.users.list;
   }
+  return {};
+};
+
+const selectListEntry = (state: Object) => {
+  const list = selectList(state);
+  if (list.entry) {
+    return list.entry;
+  }
+  return {};
+};
+
+export const selectListAsCollection = (
+  state: Object
+): PageCollectionStateSlice => {
+  return selectList(state);
+};
+
+export const isPermittedToCreateUsers = (state: Object): boolean => {
+  const permission = selectListEntry(state).userCreatePermission;
+  if (permission) {
+    return true;
+  }
+  return false;
 };

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

@@ -38,7 +38,9 @@ import {
   deleteUserSuccess,
   fetchUsersPending,
   fetchUserPending,
-  fetchUserFailure
+  fetchUserFailure,
+  selectListAsCollection,
+  isPermittedToCreateUsers
 } from "./users";
 
 import reducer from "./users";
@@ -359,10 +361,12 @@ describe("users reducer", () => {
       entries: ["zaphod", "ford"],
       error: null,
       loading: false,
-      userCreatePermission: true,
+      entry: {
-      page: 0,
+        userCreatePermission: true,
-      pageTotal: 1,
+        page: 0,
-      _links: responseBody._links
+        pageTotal: 1,
+        _links: responseBody._links
+      }
     });
 
     expect(newState.byNames).toEqual({
@@ -374,7 +378,7 @@ describe("users reducer", () => {
       }
     });
 
-    expect(newState.list.userCreatePermission).toBeTruthy();
+    expect(newState.list.entry.userCreatePermission).toBeTruthy();
   });
 
   test("should update state correctly according to DELETE_USER action", () => {
@@ -464,10 +468,10 @@ describe("users reducer", () => {
     expect(newState.byNames["ford"]).toBeDefined();
   });
 
-  it("should set userCreatePermission to true if update link is present", () => {
+  it("should set userCreatePermission to true if create link is present", () => {
     const newState = reducer({}, fetchUsersSuccess(responseBody));
 
-    expect(newState.list.userCreatePermission).toBeTruthy();
+    expect(newState.list.entry.userCreatePermission).toBeTruthy();
   });
 
   it("should update state correctly according to CREATE_USER_PENDING action", () => {
@@ -577,3 +581,46 @@ describe("users reducer", () => {
     expect(newState.byNames["ford"].entry).toBeFalsy();
   });
 });
+
+describe("selector tests", () => {
+  it("should return an empty object", () => {
+    expect(selectListAsCollection({})).toEqual({});
+    expect(selectListAsCollection({ users: { a: "a" } })).toEqual({});
+  });
+
+  it("should return a state slice collection", () => {
+    const state = {
+      users: {
+        list: {
+          loading: false
+        }
+      }
+    };
+    expect(selectListAsCollection(state)).toEqual({ loading: false });
+  });
+
+  it("should return false", () => {
+    expect(isPermittedToCreateUsers({})).toBe(false);
+    expect(isPermittedToCreateUsers({ users: { list: { entry: {} } } })).toBe(
+      false
+    );
+    expect(
+      isPermittedToCreateUsers({
+        users: { list: { entry: { userCreatePermission: false } } }
+      })
+    ).toBe(false);
+  });
+
+  it("should return true", () => {
+    const state = {
+      users: {
+        list: {
+          entry: {
+            userCreatePermission: true
+          }
+        }
+      }
+    };
+    expect(isPermittedToCreateUsers(state)).toBe(true);
+  });
+});

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,