From 89ecddf6624a302f26da77635f84d2c22bcaba81 Mon Sep 17 00:00:00 2001
From: Keith Grant <kegrant@redhat.com>
Date: Fri, 5 Apr 2019 10:52:52 -0400
Subject: [PATCH] refactor OrganizationTeams/OrganizationTeamsList

---
 .../components/OrganizationTeamsList.jsx      | 160 +++++-------------
 .../screens/Organization/Organization.jsx     |   5 +-
 .../Organization/OrganizationTeams.jsx        |  85 ++++++++--
 3 files changed, 119 insertions(+), 131 deletions(-)

diff --git a/src/pages/Organizations/components/OrganizationTeamsList.jsx b/src/pages/Organizations/components/OrganizationTeamsList.jsx
index 83a5616198..0691d42a65 100644
--- a/src/pages/Organizations/components/OrganizationTeamsList.jsx
+++ b/src/pages/Organizations/components/OrganizationTeamsList.jsx
@@ -1,24 +1,17 @@
 import React, { Fragment } from 'react';
 import PropTypes from 'prop-types';
-
 import {
   DataList, DataListItem, DataListCell, Text,
   TextContent, TextVariants
 } from '@patternfly/react-core';
-
 import { I18n, i18nMark } from '@lingui/react';
 import { t } from '@lingui/macro';
-
-import {
-  Link
-} from 'react-router-dom';
+import { withRouter, Link } from 'react-router-dom';
 
 import Pagination from '../../../components/Pagination';
 import DataListToolbar from '../../../components/DataListToolbar';
 
-import {
-  parseQueryString,
-} from '../../../qs';
+import { encodeQueryString } from '../../../qs';
 
 const detailWrapperStyle = {
   display: 'grid',
@@ -45,125 +38,55 @@ class OrganizationTeamsList extends React.Component {
   constructor (props) {
     super(props);
 
-    const { page, page_size } = this.readQueryParams();
-
     this.state = {
-      page,
-      page_size,
-      count: 0,
-      sortOrder: 'ascending',
-      sortedColumnKey: 'name',
-      results: [],
+      error: null,
     };
 
-    this.readOrganizationTeamsList = this.readOrganizationTeamsList.bind(this);
     this.handleSetPage = this.handleSetPage.bind(this);
     this.handleSort = this.handleSort.bind(this);
-    this.readQueryParams = this.readQueryParams.bind(this);
   }
 
-  componentDidMount () {
-    const queryParams = this.readQueryParams();
-    try {
-      this.readOrganizationTeamsList(queryParams);
-    } catch (error) {
-      this.setState({ error });
+  getPageCount () {
+    const { itemCount, queryParams: { page_size } } = this.props;
+    return Math.ceil(itemCount / page_size);
+  }
+
+  getSortOrder () {
+    const { queryParams } = this.props;
+    if (queryParams.order_by && queryParams.order_by.startsWith('-')) {
+      return 'descending';
     }
+    return 'ascending';
   }
 
   handleSetPage (pageNumber, pageSize) {
-    const { sortOrder, sortedColumnKey } = this.state;
-    const page = parseInt(pageNumber, 10);
-    const page_size = parseInt(pageSize, 10);
-    let order_by = sortedColumnKey;
-
-    // Preserve sort order when paginating
-    if (sortOrder === 'descending') {
-      order_by = `-${order_by}`;
-    }
-
-    const queryParams = this.readQueryParams({ page, page_size, order_by });
-
-    this.readOrganizationTeamsList(queryParams);
+    this.pushHistoryState({
+      page: pageNumber,
+      page_size: pageSize,
+    });
   }
 
   handleSort (sortedColumnKey, sortOrder) {
-    const { page_size } = this.state;
-
-    let order_by = sortedColumnKey;
-
-    if (sortOrder === 'descending') {
-      order_by = `-${order_by}`;
-    }
-
-    const queryParams = this.readQueryParams({ order_by, page_size });
-
-    this.readOrganizationTeamsList(queryParams);
+    this.pushHistoryState({
+      order_by: sortOrder === 'ascending' ? sortedColumnKey : `-${sortedColumnKey}`,
+    });
   }
 
-  readQueryParams (overrides = {}) {
-    const { location } = this.props;
-    const { search } = location;
-
-    const searchParams = parseQueryString(search.substring(1));
-
-    return Object.assign({}, this.defaultParams, searchParams, overrides);
-  }
-
-  async readOrganizationTeamsList (queryParams) {
-    const { match, onReadTeamsList } = this.props;
-
-    const { page, page_size, order_by } = queryParams;
-
-    let sortOrder = 'ascending';
-    let sortedColumnKey = order_by;
-
-    if (order_by.startsWith('-')) {
-      sortOrder = 'descending';
-      sortedColumnKey = order_by.substring(1);
-    }
-
-    try {
-      const { data:
-        { count = 0, results = [] }
-      } = await onReadTeamsList(match.params.id, queryParams);
-      const pageCount = Math.ceil(count / page_size);
-
-      const stateToUpdate = {
-        count,
-        page,
-        pageCount,
-        page_size,
-        sortOrder,
-        sortedColumnKey,
-        results
-      };
-
-      this.setState(stateToUpdate);
-    } catch (error) {
-      this.setState({ error });
-    }
+  pushHistoryState (params) {
+    const { history } = this.props;
+    const { pathname } = history.location;
+    const qs = encodeQueryString(params);
+    history.push(`${pathname}?${qs}`);
   }
 
   render () {
-    const {
-      results,
-      error,
-      count,
-      page_size,
-      pageCount,
-      page,
-      sortedColumnKey,
-      sortOrder
-    } = this.state;
+    const { teams, itemCount, queryParams } = this.props;
+    const { error } = this.state;
     return (
       <I18n>
         {({ i18n }) => (
           <Fragment>
-            {!error && results.length <= 0 && (
-              <h1>Loading...</h1> // TODO: replace with proper loading state
-            )}
-            {error && results.length <= 0 && (
+            {error && (
               <Fragment>
                 <div>{error.message}</div>
                 {error.response && (
@@ -171,17 +94,17 @@ class OrganizationTeamsList extends React.Component {
                 )}
               </Fragment> // TODO: replace with proper error handling
             )}
-            {results.length > 0 && (
+            {teams.length > 0 && (
               <Fragment>
                 <DataListToolbar
-                  sortedColumnKey={sortedColumnKey}
-                  sortOrder={sortOrder}
+                  sortedColumnKey={queryParams.sort_by}
+                  sortOrder={this.getSortOrder()}
                   columns={this.columns}
                   onSearch={() => { }}
                   onSort={this.handleSort}
                 />
                 <DataList aria-label={i18n._(t`Teams List`)}>
-                  {results.map(({ url, id, name }) => (
+                  {teams.map(({ url, id, name }) => (
                     <DataListItem aria-labelledby={i18n._(t`teams-list-item`)} key={id}>
                       <DataListCell>
                         <TextContent style={detailWrapperStyle}>
@@ -194,10 +117,10 @@ class OrganizationTeamsList extends React.Component {
                   ))}
                 </DataList>
                 <Pagination
-                  count={count}
-                  page={page}
-                  pageCount={pageCount}
-                  page_size={page_size}
+                  count={itemCount}
+                  page={queryParams.page}
+                  pageCount={this.getPageCount()}
+                  page_size={queryParams.page_size}
                   onSetPage={this.handleSetPage}
                 />
               </Fragment>
@@ -210,7 +133,14 @@ class OrganizationTeamsList extends React.Component {
 }
 
 OrganizationTeamsList.propTypes = {
-  onReadTeamsList: PropTypes.func.isRequired,
+  teams: PropTypes.arrayOf({}).isRequired,
+  itemCount: PropTypes.number.isRequired,
+  queryParams: PropTypes.shape({
+    page: PropTypes.number,
+    page_size: PropTypes.number,
+    order_by: PropTypes.string,
+  }).isRequired
 };
 
-export default OrganizationTeamsList;
+export { OrganizationTeamsList as _OrganizationTeamsList };
+export default withRouter(OrganizationTeamsList);
diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx
index 33788d0890..e1bf14eabc 100644
--- a/src/pages/Organizations/screens/Organization/Organization.jsx
+++ b/src/pages/Organizations/screens/Organization/Organization.jsx
@@ -159,10 +159,9 @@ class Organization extends Component {
               path="/organizations/:id/teams"
               render={() => (
                 <OrganizationTeams
+                  id={Number(match.params.id)}
+                  searchString={location.search}
                   api={api}
-                  match={match}
-                  location={location}
-                  history={history}
                 />
               )}
             />
diff --git a/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx b/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx
index 0fd6f9e517..b6cf169e39 100644
--- a/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx
+++ b/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx
@@ -1,34 +1,93 @@
 import React from 'react';
+import PropTypes from 'prop-types';
+import { withRouter } from 'react-router-dom';
 import OrganizationTeamsList from '../../components/OrganizationTeamsList';
+import { parseQueryString } from '../../../../qs';
+
+const DEFAULT_QUERY_PARAMS = {
+  page: 1,
+  page_size: 5,
+  order_by: 'name',
+};
 
 class OrganizationTeams extends React.Component {
   constructor (props) {
     super(props);
 
     this.readOrganizationTeamsList = this.readOrganizationTeamsList.bind(this);
+
+    this.state = {
+      isLoading: false,
+      error: null,
+      itemCount: 0,
+      teams: [],
+    };
   }
 
-  readOrganizationTeamsList (id, params) {
-    const { api } = this.props;
-    return api.readOrganizationTeamsList(id, params);
+  componentDidMount () {
+    this.readOrganizationTeamsList();
+  }
+
+  componentDidUpdate (prevProps) {
+    const { location } = this.props;
+    if (location !== prevProps.location) {
+      this.readOrganizationTeamsList();
+    }
+  }
+
+  getQueryParams () {
+    const { searchString } = this.props;
+    const searchParams = parseQueryString(searchString.substring(1));
+
+    return {
+      ...DEFAULT_QUERY_PARAMS,
+      ...searchParams,
+    };
+  }
+
+  async readOrganizationTeamsList () {
+    const { api, id } = this.props;
+    const params = this.getQueryParams();
+    this.setState({ isLoading: true });
+    try {
+      const {
+        data: { count = 0, results = [] },
+      } = await api.readOrganizationTeamsList(id, params);
+      this.setState({
+        itemCount: count,
+        teams: results,
+        isLoading: false,
+      });
+    } catch (error) {
+      this.setState({
+        error,
+        isLoading: false
+      });
+    }
   }
 
   render () {
-    const {
-      location,
-      match,
-      history,
-    } = this.props;
+    const { teams, itemCount, isLoading } = this.state;
+
+    if (isLoading) {
+      return <div>Loading...</div>;
+    }
 
     return (
       <OrganizationTeamsList
-        onReadTeamsList={this.readOrganizationTeamsList}
-        match={match}
-        location={location}
-        history={history}
+        teams={teams}
+        itemCount={itemCount}
+        queryParams={this.getQueryParams()}
       />
     );
   }
 }
 
-export default OrganizationTeams;
+OrganizationTeams.propTypes = {
+  id: PropTypes.number.isRequired,
+  searchString: PropTypes.string.isRequired,
+  api: PropTypes.shape().isRequired, // TODO: remove?
+};
+
+export { OrganizationTeams as _OrganizationTeams };
+export default withRouter(OrganizationTeams);