From e2c8519b777991f1ec20d1df2f5b2727748b3349 Mon Sep 17 00:00:00 2001
From: "Keith J. Grant" <barkeep@keithjgrant.com>
Date: Mon, 3 May 2021 16:00:09 -0700
Subject: [PATCH] convert InventoryHostGroups list to table

---
 .../InventoryHostGroupItem.jsx                | 87 ++++++++-----------
 .../InventoryHostGroupItem.test.jsx           | 32 ++++---
 .../InventoryHostGroupsList.jsx               | 25 +++---
 .../InventoryHostGroupsList.test.jsx          | 47 ++++++----
 4 files changed, 104 insertions(+), 87 deletions(-)

diff --git a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.jsx
index b87ab23da0..58b24b7dac 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.jsx
@@ -1,66 +1,55 @@
 import React from 'react';
 import { bool, func, number, oneOfType, string } from 'prop-types';
-
 import { t } from '@lingui/macro';
-
-import {
-  Button,
-  DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemCells,
-  DataListItemRow,
-  Tooltip,
-} from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
+import { Tr, Td } from '@patternfly/react-table';
 
 import { Link } from 'react-router-dom';
 import { PencilAltIcon } from '@patternfly/react-icons';
-import DataListCell from '../../../components/DataListCell';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
 import { Group } from '../../../types';
 
-function InventoryHostGroupItem({ group, inventoryId, isSelected, onSelect }) {
+function InventoryHostGroupItem({
+  group,
+  inventoryId,
+  isSelected,
+  onSelect,
+  rowIndex,
+}) {
   const labelId = `check-action-${group.id}`;
   const detailUrl = `/inventories/inventory/${inventoryId}/groups/${group.id}/details`;
   const editUrl = `/inventories/inventory/${inventoryId}/groups/${group.id}/edit`;
 
   return (
-    <DataListItem key={group.id} aria-labelledby={labelId} id={`${group.id}`}>
-      <DataListItemRow>
-        <DataListCheck
-          aria-labelledby={labelId}
-          id={`select-group-${group.id}`}
-          checked={isSelected}
-          onChange={onSelect}
-        />
-        <DataListItemCells
-          dataListCells={[
-            <DataListCell key="name">
-              <Link to={`${detailUrl}`} id={labelId}>
-                <b>{group.name}</b>
-              </Link>
-            </DataListCell>,
-          ]}
-        />
-        <DataListAction
-          aria-label={t`actions`}
-          aria-labelledby={labelId}
-          id={labelId}
+    <Tr id={`inventory-host-group-row-${group.id}`}>
+      <Td
+        select={{
+          rowIndex,
+          isSelected,
+          onSelect,
+        }}
+      />
+      <Td id={labelId} dataLabel={t`Name`}>
+        <Link to={`${detailUrl}`} id={labelId}>
+          {group.name}
+        </Link>
+      </Td>
+      <ActionsTd dataLabel={t`Actions`} gridColumns="auto 40px">
+        <ActionItem
+          visible={group.summary_fields.user_capabilities.edit}
+          tooltip={t`Edit group`}
         >
-          {group.summary_fields.user_capabilities.edit && (
-            <Tooltip content={t`Edit Group`} position="top">
-              <Button
-                ouiaId={`${group.id}-edit-button`}
-                variant="plain"
-                component={Link}
-                to={editUrl}
-              >
-                <PencilAltIcon />
-              </Button>
-            </Tooltip>
-          )}
-        </DataListAction>
-      </DataListItemRow>
-    </DataListItem>
+          <Button
+            ouiaId={`${group.id}-edit-button`}
+            variant="plain"
+            component={Link}
+            to={editUrl}
+          >
+            <PencilAltIcon />
+          </Button>
+        </ActionItem>
+      </ActionsTd>
+    </Tr>
   );
 }
 
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.test.jsx
index 6092de88c7..2da25d1cde 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.test.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupItem.test.jsx
@@ -18,12 +18,16 @@ describe('<InventoryHostGroupItem />', () => {
 
   beforeEach(() => {
     wrapper = mountWithContexts(
-      <InventoryHostGroupItem
-        group={mockGroup}
-        inventoryId={1}
-        isSelected={false}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <InventoryHostGroupItem
+            group={mockGroup}
+            inventoryId={1}
+            isSelected={false}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
   });
 
@@ -40,12 +44,16 @@ describe('<InventoryHostGroupItem />', () => {
     copyMockGroup.summary_fields.user_capabilities.edit = false;
 
     wrapper = mountWithContexts(
-      <InventoryHostGroupItem
-        group={copyMockGroup}
-        inventoryId={1}
-        isSelected={false}
-        onSelect={() => {}}
-      />
+      <table>
+        <tbody>
+          <InventoryHostGroupItem
+            group={copyMockGroup}
+            inventoryId={1}
+            isSelected={false}
+            onSelect={() => {}}
+          />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy();
   });
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.jsx b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.jsx
index 4454be3417..49868207c5 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.jsx
@@ -12,9 +12,11 @@ import { HostsAPI, InventoriesAPI } from '../../../api';
 import DataListToolbar from '../../../components/DataListToolbar';
 import AlertModal from '../../../components/AlertModal';
 import ErrorDetail from '../../../components/ErrorDetail';
-import PaginatedDataList, {
-  ToolbarAddButton,
-} from '../../../components/PaginatedDataList';
+import PaginatedTable, {
+  HeaderRow,
+  HeaderCell,
+} from '../../../components/PaginatedTable';
+import { ToolbarAddButton } from '../../../components/PaginatedDataList';
 import AssociateModal from '../../../components/AssociateModal';
 import DisassociateButton from '../../../components/DisassociateButton';
 import AdHocCommands from '../../../components/AdHocCommands/AdHocCommands';
@@ -146,7 +148,7 @@ function InventoryHostGroupsList() {
 
   return (
     <>
-      <PaginatedDataList
+      <PaginatedTable
         contentError={contentError}
         hasContentLoading={
           isLoading || isDisassociateLoading || isAdHocLaunchLoading
@@ -170,21 +172,22 @@ function InventoryHostGroupsList() {
             key: 'modified_by__username__icontains',
           },
         ]}
-        toolbarSortColumns={[
-          {
-            name: t`Name`,
-            key: 'name',
-          },
-        ]}
         toolbarSearchableKeys={searchableKeys}
         toolbarRelatedSearchableKeys={relatedSearchableKeys}
-        renderItem={item => (
+        headerRow={
+          <HeaderRow qsConfig={QS_CONFIG}>
+            <HeaderCell sortKey="name">{t`Name`}</HeaderCell>
+            <HeaderCell>{t`Actions`}</HeaderCell>
+          </HeaderRow>
+        }
+        renderRow={(item, index) => (
           <InventoryHostGroupItem
             key={item.id}
             group={item}
             inventoryId={item.summary_fields.inventory.id}
             isSelected={selected.some(row => row.id === item.id)}
             onSelect={() => handleSelect(item)}
+            rowIndex={index}
           />
         )}
         renderToolbar={props => (
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.test.jsx
index 493e9dc65a..043618c46a 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.test.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHostGroups/InventoryHostGroupsList.test.jsx
@@ -114,46 +114,63 @@ describe('<InventoryHostGroupsList />', () => {
 
   test('should check and uncheck the row item', async () => {
     expect(
-      wrapper.find('DataListCheck[id="select-group-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(false);
 
     await act(async () => {
-      wrapper.find('DataListCheck[id="select-group-1"]').invoke('onChange')(
-        true
-      );
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .invoke('onChange')(true);
     });
     wrapper.update();
     expect(
-      wrapper.find('DataListCheck[id="select-group-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(true);
 
     await act(async () => {
-      wrapper.find('DataListCheck[id="select-group-1"]').invoke('onChange')(
-        false
-      );
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .invoke('onChange')(false);
     });
     wrapper.update();
     expect(
-      wrapper.find('DataListCheck[id="select-group-1"]').props().checked
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .props().checked
     ).toBe(false);
   });
 
   test('should check all row items when select all is checked', async () => {
-    wrapper.find('DataListCheck').forEach(el => {
+    expect.assertions(9);
+    wrapper.find('.pf-c-table__check input').forEach(el => {
       expect(el.props().checked).toBe(false);
     });
     await act(async () => {
       wrapper.find('Checkbox#select-all').invoke('onChange')(true);
     });
     wrapper.update();
-    wrapper.find('DataListCheck').forEach(el => {
+    wrapper.find('.pf-c-table__check input').forEach(el => {
       expect(el.props().checked).toBe(true);
     });
     await act(async () => {
       wrapper.find('Checkbox#select-all').invoke('onChange')(false);
     });
     wrapper.update();
-    wrapper.find('DataListCheck').forEach(el => {
+    wrapper.find('.pf-c-table__check input').forEach(el => {
       expect(el.props().checked).toBe(false);
     });
   });
@@ -230,15 +247,15 @@ describe('<InventoryHostGroupsList />', () => {
   test('expected api calls are made for multi-disassociation', async () => {
     expect(HostsAPI.disassociateGroup).toHaveBeenCalledTimes(0);
     expect(HostsAPI.readAllGroups).toHaveBeenCalledTimes(1);
-    expect(wrapper.find('DataListCheck').length).toBe(3);
-    wrapper.find('DataListCheck').forEach(el => {
+    expect(wrapper.find('.pf-c-table__check').length).toBe(3);
+    wrapper.find('.pf-c-table__check input').forEach(el => {
       expect(el.props().checked).toBe(false);
     });
     await act(async () => {
       wrapper.find('Checkbox#select-all').invoke('onChange')(true);
     });
     wrapper.update();
-    wrapper.find('DataListCheck').forEach(el => {
+    wrapper.find('.pf-c-table__check input').forEach(el => {
       expect(el.props().checked).toBe(true);
     });
     wrapper.find('button[aria-label="Disassociate"]').simulate('click');