From a8159273eb00e4d5e89d94c6e066c95736792990 Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Tue, 1 Dec 2020 13:59:05 -0800
Subject: [PATCH] convert InventoryList to use PaginatedTable

---
 .../DataListToolbar/DataListToolbar.jsx       |   8 +-
 .../components/PaginatedTable/HeaderRow.jsx   |  17 +-
 .../PaginatedTable/PaginatedTable.jsx         |  19 +-
 .../Inventory/InventoryList/InventoryList.jsx |  25 ++-
 .../InventoryList/InventoryListItem.jsx       | 212 +++++++-----------
 .../OrganizationList/OrganizationList.jsx     |  23 +-
 .../OrganizationList/OrganizationListItem.jsx |   2 +-
 7 files changed, 114 insertions(+), 192 deletions(-)

diff --git a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx
index 3fbf41679c..bcecf8a2bc 100644
--- a/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx
+++ b/awx/ui_next/src/components/DataListToolbar/DataListToolbar.jsx
@@ -93,9 +93,11 @@ function DataListToolbar({
               onRemove={onRemove}
             />
           </ToolbarItem>
-          <ToolbarItem>
-            <Sort qsConfig={qsConfig} columns={sortColumns} onSort={onSort} />
-          </ToolbarItem>
+          {sortColumns && (
+            <ToolbarItem>
+              <Sort qsConfig={qsConfig} columns={sortColumns} onSort={onSort} />
+            </ToolbarItem>
+          )}
         </ToolbarToggleGroup>
         {showExpandCollapse && (
           <ToolbarGroup>
diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx
index 51c92e6de8..906370c3b9 100644
--- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx
+++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx
@@ -7,21 +7,13 @@ import {
   replaceParams,
 } from '../../util/qs';
 
-export default function HeaderRow({
-  handleSelectAll,
-  isAllSelected,
-  qsConfig,
-  defaultSortKey,
-  children,
-}) {
+export default function HeaderRow({ qsConfig, defaultSortKey, children }) {
   const location = useLocation();
   const history = useHistory();
 
   const params = parseQueryString(qsConfig, location.search);
 
-  // TODO: asc vs desc -- correct for both alpha & numeric sorting?
   const onSort = (key, order) => {
-    console.log({ key, order });
     const newParams = replaceParams(params, {
       order_by: order === 'asc' ? key : `-${key}`,
       page: null,
@@ -43,12 +35,7 @@ export default function HeaderRow({
   return (
     <Thead>
       <Tr>
-        <Th
-          select={{
-            onSelect: handleSelectAll,
-            isSelected: isAllSelected,
-          }}
-        />
+        <Th />
         {React.Children.map(children, child =>
           React.cloneElement(child, {
             onSort,
diff --git a/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx b/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx
index b78f1afb1d..912773e573 100644
--- a/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx
+++ b/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx
@@ -18,7 +18,7 @@ import {
   replaceParams,
 } from '../../util/qs';
 import PaginatedTableRow from './PaginatedTableRow';
-import { QSConfig, SearchColumns, SortColumns } from '../../types';
+import { QSConfig, SearchColumns } from '../../types';
 
 function PaginatedTable({
   contentError,
@@ -32,7 +32,6 @@ function PaginatedTable({
   toolbarSearchColumns,
   toolbarSearchableKeys,
   toolbarRelatedSearchableKeys,
-  toolbarSortColumns,
   pluralizedItemName,
   showPageSizeOptions,
   i18n,
@@ -71,14 +70,6 @@ function PaginatedTable({
           isDefault: true,
         },
       ];
-  const sortColumns = toolbarSortColumns.length
-    ? toolbarSortColumns
-    : [
-        {
-          name: i18n._(t`Name`),
-          key: 'name',
-        },
-      ];
   const queryParams = parseQueryString(qsConfig, history.location.search);
 
   const dataListLabel = i18n._(t`${pluralizedItemName} List`);
@@ -98,10 +89,7 @@ function PaginatedTable({
     );
   } else {
     Content = (
-      <TableComposable
-        aria-label={dataListLabel}
-        // onSelectDataListItem={handleListItemSelect}
-      >
+      <TableComposable aria-label={dataListLabel}>
         {headerRow}
         <Tbody>{items.map(renderRow)}</Tbody>
       </TableComposable>
@@ -137,7 +125,6 @@ function PaginatedTable({
         renderToolbar={renderToolbar}
         emptyStateControls={emptyStateControls}
         searchColumns={searchColumns}
-        sortColumns={sortColumns}
         searchableKeys={toolbarSearchableKeys}
         relatedSearchableKeys={toolbarRelatedSearchableKeys}
         qsConfig={qsConfig}
@@ -183,7 +170,6 @@ PaginatedTable.propTypes = {
   toolbarSearchColumns: SearchColumns,
   toolbarSearchableKeys: PropTypes.arrayOf(PropTypes.string),
   toolbarRelatedSearchableKeys: PropTypes.arrayOf(PropTypes.string),
-  toolbarSortColumns: SortColumns,
   showPageSizeOptions: PropTypes.bool,
   renderToolbar: PropTypes.func,
   hasContentLoading: PropTypes.bool,
@@ -197,7 +183,6 @@ PaginatedTable.defaultProps = {
   toolbarSearchColumns: [],
   toolbarSearchableKeys: [],
   toolbarRelatedSearchableKeys: [],
-  toolbarSortColumns: [],
   pluralizedItemName: 'Items',
   showPageSizeOptions: true,
   renderRow: item => <PaginatedTableRow key={item.id} item={item} />,
diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx
index 6bb721a427..32e8ddd8ea 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx
@@ -8,9 +8,11 @@ import useRequest, { useDeleteItems } from '../../../util/useRequest';
 import AlertModal from '../../../components/AlertModal';
 import DatalistToolbar from '../../../components/DataListToolbar';
 import ErrorDetail from '../../../components/ErrorDetail';
-import PaginatedDataList, {
-  ToolbarDeleteButton,
-} from '../../../components/PaginatedDataList';
+import { ToolbarDeleteButton } from '../../../components/PaginatedDataList';
+import PaginatedTable, {
+  HeaderRow,
+  HeaderCell,
+} from '../../../components/PaginatedTable';
 import { getQSConfig, parseQueryString } from '../../../util/qs';
 import useWsInventories from './useWsInventories';
 import AddDropDownButton from '../../../components/AddDropDownButton';
@@ -149,10 +151,11 @@ function InventoryList({ i18n }) {
       ]}
     />
   );
+
   return (
     <PageSection>
       <Card>
-        <PaginatedDataList
+        <PaginatedTable
           contentError={contentError}
           hasContentLoading={hasContentLoading}
           items={inventories}
@@ -187,6 +190,17 @@ function InventoryList({ i18n }) {
           ]}
           toolbarSearchableKeys={searchableKeys}
           toolbarRelatedSearchableKeys={relatedSearchableKeys}
+          headerRow={
+            <HeaderRow defaultSortKey="name" qsConfig={QS_CONFIG}>
+              <HeaderCell sortKey="name">{i18n._(t`Name`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Status`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Type`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Organization`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Groups`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Hosts`)}</HeaderCell>
+              <HeaderCell>{i18n._(t`Sources`)}</HeaderCell>
+            </HeaderRow>
+          }
           renderToolbar={props => (
             <DatalistToolbar
               {...props}
@@ -209,11 +223,12 @@ function InventoryList({ i18n }) {
               ]}
             />
           )}
-          renderItem={inventory => (
+          renderRow={(inventory, index) => (
             <InventoryListItem
               key={inventory.id}
               value={inventory.name}
               inventory={inventory}
+              rowIndex={index}
               fetchInventories={fetchInventories}
               detailUrl={
                 inventory.kind === 'smart'
diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx
index 45408f88a2..190443aeae 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx
@@ -1,50 +1,21 @@
 import React, { useState, useCallback } from 'react';
 import { string, bool, func } from 'prop-types';
 import { withI18n } from '@lingui/react';
-import {
-  Button,
-  DataListAction as _DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemCells,
-  DataListItemRow,
-  Label,
-  Tooltip,
-  Badge as PFBadge,
-} from '@patternfly/react-core';
+import { Button, Label, Tooltip } from '@patternfly/react-core';
+import { Tr, Td } from '@patternfly/react-table';
 import { PencilAltIcon } from '@patternfly/react-icons';
 import { t } from '@lingui/macro';
 import { Link } from 'react-router-dom';
-import styled from 'styled-components';
 import { timeOfDay } from '../../../util/dates';
 import { InventoriesAPI } from '../../../api';
 import { Inventory } from '../../../types';
-import DataListCell from '../../../components/DataListCell';
+import { ActionsTd } from '../../../components/PaginatedTable';
 import CopyButton from '../../../components/CopyButton';
 import SyncStatusIndicator from '../../../components/SyncStatusIndicator';
 
-const DataListAction = styled(_DataListAction)`
-  align-items: center;
-  display: grid;
-  grid-gap: 16px;
-  grid-template-columns: repeat(2, 40px);
-`;
-
-const Badge = styled(PFBadge)`
-  margin-left: 8px;
-`;
-
-const ListGroup = styled.div`
-  margin-left: 8px;
-  display: inline-block;
-`;
-
-const OrgLabel = styled.b`
-  margin-right: 20px;
-`;
-
 function InventoryListItem({
   inventory,
+  rowIndex,
   isSelected,
   onSelect,
   detailUrl,
@@ -85,108 +56,81 @@ function InventoryListItem({
   }
 
   return (
-    <DataListItem
-      key={inventory.id}
-      aria-labelledby={labelId}
-      id={`${inventory.id}`}
-    >
-      <DataListItemRow>
-        <DataListCheck
-          id={`select-inventory-${inventory.id}`}
-          isDisabled={inventory.pending_deletion}
-          checked={isSelected}
-          onChange={onSelect}
-          aria-labelledby={labelId}
-        />
-        <DataListItemCells
-          dataListCells={[
-            <DataListCell key="sync-status" isIcon>
-              {inventory.kind !== 'smart' && (
-                <SyncStatusIndicator status={syncStatus} />
-              )}
-            </DataListCell>,
-            <DataListCell key="name">
-              {inventory.pending_deletion ? (
-                <b>{inventory.name}</b>
-              ) : (
-                <Link to={`${detailUrl}`}>
-                  <b>{inventory.name}</b>
-                </Link>
-              )}
-            </DataListCell>,
-            <DataListCell key="kind">
-              {inventory.kind === 'smart'
-                ? i18n._(t`Smart Inventory`)
-                : i18n._(t`Inventory`)}
-            </DataListCell>,
-            <DataListCell key="organization">
-              <OrgLabel>{i18n._(t`Organization`)}</OrgLabel>
-              <Link
-                to={`/organizations/${inventory.summary_fields.organization.id}/details`}
-              >
-                {inventory.summary_fields.organization.name}
-              </Link>
-            </DataListCell>,
-            <DataListCell key="groups-hosts-sources-counts">
-              <ListGroup>
-                {i18n._(t`Groups`)}
-                <Badge isRead>{inventory.total_groups}</Badge>
-              </ListGroup>
-              <ListGroup>
-                {i18n._(t`Hosts`)}
-                <Badge isRead>{inventory.total_hosts}</Badge>
-              </ListGroup>
-              <ListGroup>
-                {i18n._(t`Sources`)}
-                <Badge isRead>{inventory.total_inventory_sources}</Badge>
-              </ListGroup>
-            </DataListCell>,
-            inventory.pending_deletion && (
-              <DataListCell alignRight isFilled={false} key="pending-delete">
-                <Label color="red">{i18n._(t`Pending delete`)}</Label>
-              </DataListCell>
-            ),
-          ]}
-        />
-        {!inventory.pending_deletion && (
-          <DataListAction
-            aria-label="actions"
-            aria-labelledby={labelId}
-            id={labelId}
-          >
-            {inventory.summary_fields.user_capabilities.edit ? (
-              <Tooltip content={i18n._(t`Edit Inventory`)} position="top">
-                <Button
-                  isDisabled={isDisabled}
-                  aria-label={i18n._(t`Edit Inventory`)}
-                  variant="plain"
-                  component={Link}
-                  to={`/inventories/${
-                    inventory.kind === 'smart' ? 'smart_inventory' : 'inventory'
-                  }/${inventory.id}/edit`}
-                >
-                  <PencilAltIcon />
-                </Button>
-              </Tooltip>
-            ) : (
-              ''
-            )}
-            {inventory.summary_fields.user_capabilities.copy && (
-              <CopyButton
-                copyItem={copyInventory}
-                isDisabled={isDisabled}
-                onCopyStart={handleCopyStart}
-                onCopyFinish={handleCopyFinish}
-                helperText={{
-                  tooltip: i18n._(t`Copy Inventory`),
-                  errorMessage: i18n._(t`Failed to copy inventory.`),
-                }}
-              />
-            )}
-          </DataListAction>
+    <Tr id={inventory.id}>
+      <Td
+        select={{
+          rowIndex,
+          isSelected,
+          onSelect,
+        }}
+      />
+      <Td id={labelId}>
+        {inventory.pending_deletion ? (
+          <b>{inventory.name}</b>
+        ) : (
+          <Link to={`${detailUrl}`}>
+            <b>{inventory.name}</b>
+          </Link>
         )}
-      </DataListItemRow>
-    </DataListItem>
+      </Td>
+      <Td>
+        {inventory.kind !== 'smart' && (
+          <SyncStatusIndicator status={syncStatus} />
+        )}
+      </Td>
+      <Td>
+        {inventory.kind === 'smart'
+          ? i18n._(t`Smart Inventory`)
+          : i18n._(t`Inventory`)}
+      </Td>
+      <Td key="organization">
+        <Link
+          to={`/organizations/${inventory.summary_fields.organization.id}/details`}
+        >
+          {inventory.summary_fields.organization.name}
+        </Link>
+      </Td>
+      <Td>{inventory.total_groups}</Td>
+      <Td>{inventory.total_hosts}</Td>
+      <Td>{inventory.total_inventory_sources}</Td>
+      {inventory.pending_deletion ? (
+        <Td>
+          <Label color="red">{i18n._(t`Pending delete`)}</Label>
+        </Td>
+      ) : (
+        <ActionsTd numActions={2}>
+          {inventory.summary_fields.user_capabilities.edit ? (
+            <Tooltip content={i18n._(t`Edit Inventory`)} position="top">
+              <Button
+                isDisabled={isDisabled}
+                aria-label={i18n._(t`Edit Inventory`)}
+                variant="plain"
+                component={Link}
+                to={`/inventories/${
+                  inventory.kind === 'smart' ? 'smart_inventory' : 'inventory'
+                }/${inventory.id}/edit`}
+              >
+                <PencilAltIcon />
+              </Button>
+            </Tooltip>
+          ) : (
+            <div />
+          )}
+          {inventory.summary_fields.user_capabilities.copy && (
+            <CopyButton
+              copyItem={copyInventory}
+              isDisabled={isDisabled}
+              onCopyStart={handleCopyStart}
+              onCopyFinish={handleCopyFinish}
+              helperText={{
+                tooltip: i18n._(t`Copy Inventory`),
+                errorMessage: i18n._(t`Failed to copy inventory.`),
+              }}
+            />
+          )}
+        </ActionsTd>
+      )}
+    </Tr>
   );
 }
 export default withI18n()(InventoryListItem);
diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx
index 05512fc04e..2d542fe3fe 100644
--- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx
+++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx
@@ -122,12 +122,13 @@ function OrganizationsList({ i18n }) {
       <PageSection>
         <Card>
           <PaginatedTable
+            // TODO: audit if any of these props are no longer in use
             contentError={contentError}
             hasContentLoading={hasContentLoading}
             items={organizations}
             itemCount={organizationCount}
             pluralizedItemName={i18n._(t`Organizations`)}
-            qsConfig={QS_CONFIG} // TODO: still used?
+            qsConfig={QS_CONFIG}
             onRowClick={handleSelect}
             toolbarSearchColumns={[
               {
@@ -148,22 +149,10 @@ function OrganizationsList({ i18n }) {
                 key: 'modified_by__username__icontains',
               },
             ]}
-            toolbarSortColumns={[
-              {
-                name: i18n._(t`Name`),
-                key: 'name',
-              },
-            ]}
             toolbarSearchableKeys={searchableKeys}
             toolbarRelatedSearchableKeys={relatedSearchableKeys}
             headerRow={
-              // TODO: move selectAll logic into HeaderRow?
-              <HeaderRow
-                handleSelectAll={handleSelectAll}
-                isAllSelected={isAllSelected}
-                defaultSortKey="name"
-                qsConfig={QS_CONFIG}
-              >
+              <HeaderRow defaultSortKey="name" qsConfig={QS_CONFIG}>
                 <HeaderCell sortKey="name">{i18n._(t`Name`)}</HeaderCell>
                 <HeaderCell>{i18n._(t`Members`)}</HeaderCell>
                 <HeaderCell>{i18n._(t`Teams`)}</HeaderCell>
@@ -172,9 +161,9 @@ function OrganizationsList({ i18n }) {
             renderToolbar={props => (
               <DataListToolbar
                 {...props}
-                // showSelectAll
-                // isAllSelected={isAllSelected}
-                // onSelectAll={handleSelectAll}
+                showSelectAll
+                isAllSelected={isAllSelected}
+                onSelectAll={handleSelectAll}
                 qsConfig={QS_CONFIG}
                 additionalControls={[
                   ...(canAdd
diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx
index c251bdebc5..d029632598 100644
--- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx
+++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx
@@ -36,7 +36,7 @@ function OrganizationListItem({
       </Td>
       <Td>{organization.summary_fields.related_field_counts.users}</Td>
       <Td>{organization.summary_fields.related_field_counts.teams}</Td>
-      <ActionsTd numActions={2}>
+      <ActionsTd numActions={1}>
         {organization.summary_fields.user_capabilities.edit ? (
           <Tooltip content={i18n._(t`Edit Organization`)} position="top">
             <Button