From c3bab52a61fbe865055fbe221172a5cec5204ae0 Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Fri, 29 Jan 2021 15:04:11 -0800
Subject: [PATCH] convert notification template list to tables

---
 .../NotificationTemplateList.jsx              |  31 ++--
 .../NotificationTemplateList.test.jsx         |  28 +++-
 .../NotificationTemplateListItem.jsx          | 146 +++++++-----------
 .../NotificationTemplateListItem.test.jsx     |  90 ++++++-----
 4 files changed, 152 insertions(+), 143 deletions(-)

diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx
index a377e73e79..c1b277b730 100644
--- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx
+++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx
@@ -4,7 +4,11 @@ import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Card, PageSection } from '@patternfly/react-core';
 import { NotificationTemplatesAPI } from '../../../api';
-import PaginatedDataList, {
+import PaginatedTable, {
+  HeaderRow,
+  HeaderCell,
+} from '../../../components/PaginatedTable';
+import {
   ToolbarAddButton,
   ToolbarDeleteButton,
 } from '../../../components/PaginatedDataList';
@@ -104,7 +108,7 @@ function NotificationTemplatesList({ i18n }) {
     <>
       <PageSection>
         <Card>
-          <PaginatedDataList
+          <PaginatedTable
             contentError={contentError}
             hasContentLoading={isTemplatesLoading || isDeleteLoading}
             items={templates}
@@ -149,16 +153,6 @@ function NotificationTemplatesList({ i18n }) {
             ]}
             toolbarSearchableKeys={searchableKeys}
             toolbarRelatedSearchableKeys={relatedSearchableKeys}
-            toolbarSortColumns={[
-              {
-                name: i18n._(t`Name`),
-                key: 'name',
-              },
-              {
-                name: i18n._(t`Type`),
-                key: 'notification_type',
-              },
-            ]}
             renderToolbar={props => (
               <DataListToolbar
                 {...props}
@@ -179,7 +173,17 @@ function NotificationTemplatesList({ i18n }) {
                 ]}
               />
             )}
-            renderItem={template => (
+            headerRow={
+              <HeaderRow qsConfig={QS_CONFIG}>
+                <HeaderCell sortKey="name">{i18n._(t`Name`)}</HeaderCell>
+                <HeaderCell>{i18n._(t`Status`)}</HeaderCell>
+                <HeaderCell sortKey="notification_type">
+                  {i18n._(t`Type`)}
+                </HeaderCell>
+                <HeaderCell>{i18n._(t`Actions`)}</HeaderCell>
+              </HeaderRow>
+            }
+            renderRow={(template, index) => (
               <NotificationTemplateListItem
                 key={template.id}
                 fetchTemplates={fetchTemplates}
@@ -187,6 +191,7 @@ function NotificationTemplatesList({ i18n }) {
                 detailUrl={`${match.url}/${template.id}`}
                 isSelected={selected.some(row => row.id === template.id)}
                 onSelect={() => handleSelect(template)}
+                rowIndex={index}
               />
             )}
             emptyStateControls={
diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx
index d39bffe087..345d2d0a42 100644
--- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx
+++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx
@@ -89,21 +89,33 @@ describe('<NotificationTemplateList />', () => {
   });
 
   test('should select item', async () => {
-    const itemCheckboxInput = 'input#select-template-1';
     await act(async () => {
       wrapper = mountWithContexts(<NotificationTemplateList />);
     });
     wrapper.update();
-    expect(wrapper.find(itemCheckboxInput).prop('checked')).toEqual(false);
+    expect(
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .prop('checked')
+    ).toEqual(false);
     await act(async () => {
       wrapper
-        .find(itemCheckboxInput)
-        .closest('DataListCheck')
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
         .props()
         .onChange();
     });
     wrapper.update();
-    expect(wrapper.find(itemCheckboxInput).prop('checked')).toEqual(true);
+    expect(
+      wrapper
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
+        .prop('checked')
+    ).toEqual(true);
   });
 
   test('should delete notifications', async () => {
@@ -135,7 +147,6 @@ describe('<NotificationTemplateList />', () => {
   });
 
   test('should show error dialog shown for failed deletion', async () => {
-    const itemCheckboxInput = 'input#select-template-1';
     OrganizationsAPI.destroy.mockRejectedValue(
       new Error({
         response: {
@@ -153,8 +164,9 @@ describe('<NotificationTemplateList />', () => {
     wrapper.update();
     await act(async () => {
       wrapper
-        .find(itemCheckboxInput)
-        .closest('DataListCheck')
+        .find('.pf-c-table__check')
+        .first()
+        .find('input')
         .props()
         .onChange();
     });
diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx
index f3386f12a9..f3d99b76db 100644
--- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx
+++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx
@@ -3,32 +3,17 @@ import React, { useState, useEffect, useCallback } from 'react';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Link } from 'react-router-dom';
-import styled from 'styled-components';
-import {
-  Button,
-  DataListAction as _DataListAction,
-  DataListCheck,
-  DataListItem,
-  DataListItemCells,
-  DataListItemRow,
-  Tooltip,
-} from '@patternfly/react-core';
+import { Button } from '@patternfly/react-core';
+import { Tr, Td } from '@patternfly/react-table';
 import { PencilAltIcon, BellIcon } from '@patternfly/react-icons';
+import { ActionsTd, ActionItem } from '../../../components/PaginatedTable';
 import { timeOfDay } from '../../../util/dates';
 import { NotificationTemplatesAPI, NotificationsAPI } from '../../../api';
-import DataListCell from '../../../components/DataListCell';
 import StatusLabel from '../../../components/StatusLabel';
 import CopyButton from '../../../components/CopyButton';
 import useRequest from '../../../util/useRequest';
 import { NOTIFICATION_TYPES } from '../constants';
 
-const DataListAction = styled(_DataListAction)`
-  align-items: center;
-  display: grid;
-  grid-gap: 16px;
-  grid-template-columns: repeat(3, 40px);
-`;
-
 const NUM_RETRIES = 25;
 const RETRY_TIMEOUT = 5000;
 
@@ -38,6 +23,7 @@ function NotificationTemplateListItem({
   fetchTemplates,
   isSelected,
   onSelect,
+  rowIndex,
   i18n,
 }) {
   const recentNotifications = template.summary_fields?.recent_notifications;
@@ -102,76 +88,62 @@ function NotificationTemplateListItem({
   const labelId = `template-name-${template.id}`;
 
   return (
-    <DataListItem key={template.id} aria-labelledby={labelId} id={template.id}>
-      <DataListItemRow>
-        <DataListCheck
-          id={`select-template-${template.id}`}
-          checked={isSelected}
-          onChange={onSelect}
-          aria-labelledby={labelId}
-        />
-        <DataListItemCells
-          dataListCells={[
-            <DataListCell key="name" id={labelId}>
-              <Link to={detailUrl}>
-                <b>{template.name}</b>
-              </Link>
-            </DataListCell>,
-            <DataListCell key="status">
-              {status && <StatusLabel status={status} />}
-            </DataListCell>,
-            <DataListCell key="type">
-              <strong>{i18n._(t`Type:`)}</strong>{' '}
-              {NOTIFICATION_TYPES[template.notification_type] ||
-                template.notification_type}
-            </DataListCell>,
-          ]}
-        />
-        <DataListAction
-          aria-label={i18n._(t`actions`)}
-          aria-labelledby={labelId}
+    <Tr id={`notification-template-row-${template.id}`}>
+      <Td
+        select={{
+          rowIndex,
+          isSelected,
+          onSelect,
+        }}
+        dataLabel={i18n._(t`Selected`)}
+      />
+      <Td id={labelId} dataLabel={i18n._(t`Name`)}>
+        <Link to={`${detailUrl}`}>
+          <b>{template.name}</b>
+        </Link>
+      </Td>
+      <Td dataLabel={i18n._(t`Status`)}>
+        {status && <StatusLabel status={status} />}
+      </Td>
+      <Td dataLabel={i18n._(t`Type`)}>
+        {NOTIFICATION_TYPES[template.notification_type] ||
+          template.notification_type}
+      </Td>
+      <ActionsTd dataLabel={i18n._(t`Actions`)}>
+        <ActionItem visible tooltip={i18n._(t`Test notification`)}>
+          <Button
+            aria-label={i18n._(t`Test Notification`)}
+            variant="plain"
+            onClick={sendTestNotification}
+            isDisabled={isLoading || status === 'running'}
+          >
+            <BellIcon />
+          </Button>
+        </ActionItem>
+        <ActionItem
+          visible={template.summary_fields.user_capabilities.edit}
+          tooltip={i18n._(t`Edit`)}
         >
-          <Tooltip content={i18n._(t`Test Notification`)} position="top">
-            <Button
-              aria-label={i18n._(t`Test Notification`)}
-              variant="plain"
-              onClick={sendTestNotification}
-              isDisabled={isLoading || status === 'running'}
-            >
-              <BellIcon />
-            </Button>
-          </Tooltip>
-          {template.summary_fields.user_capabilities.edit ? (
-            <Tooltip
-              content={i18n._(t`Edit Notification Template`)}
-              position="top"
-            >
-              <Button
-                aria-label={i18n._(t`Edit Notification Template`)}
-                variant="plain"
-                component={Link}
-                to={`/notification_templates/${template.id}/edit`}
-              >
-                <PencilAltIcon />
-              </Button>
-            </Tooltip>
-          ) : (
-            <div />
-          )}
-          {template.summary_fields.user_capabilities.copy && (
-            <Tooltip content={i18n._(t`Copy Notification Template`)}>
-              <CopyButton
-                copyItem={copyTemplate}
-                isCopyDisabled={isCopyDisabled}
-                onCopyStart={handleCopyStart}
-                onCopyFinish={handleCopyFinish}
-                errorMessage={i18n._(t`Failed to copy template.`)}
-              />
-            </Tooltip>
-          )}
-        </DataListAction>
-      </DataListItemRow>
-    </DataListItem>
+          <Button
+            aria-label={i18n._(t`Edit Notification Template`)}
+            variant="plain"
+            component={Link}
+            to={`/notification_templates/${template.id}/edit`}
+          >
+            <PencilAltIcon />
+          </Button>
+        </ActionItem>
+        <ActionItem visible={template.summary_fields.user_capabilities.copy}>
+          <CopyButton
+            copyItem={copyTemplate}
+            isCopyDisabled={isCopyDisabled}
+            onCopyStart={handleCopyStart}
+            onCopyFinish={handleCopyFinish}
+            errorMessage={i18n._(t`Failed to copy template.`)}
+          />
+        </ActionItem>
+      </ActionsTd>
+    </Tr>
   );
 }
 
diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.test.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.test.jsx
index 65f959522c..2bdcde637f 100644
--- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.test.jsx
+++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.test.jsx
@@ -26,17 +26,21 @@ const template = {
 describe('<NotificationTemplateListItem />', () => {
   test('should render template row', () => {
     const wrapper = mountWithContexts(
-      <NotificationTemplateListItem
-        template={template}
-        detailUrl="/notification_templates/3/detail"
-      />
+      <table>
+        <tbody>
+          <NotificationTemplateListItem
+            template={template}
+            detailUrl="/notification_templates/3/detail"
+          />
+        </tbody>
+      </table>
     );
 
-    const cells = wrapper.find('DataListCell');
-    expect(cells).toHaveLength(3);
-    expect(cells.at(0).text()).toEqual('Test Notification');
-    expect(cells.at(1).text()).toEqual('Success');
-    expect(cells.at(2).text()).toEqual('Type: Slack');
+    const cells = wrapper.find('Td');
+    expect(cells).toHaveLength(5);
+    expect(cells.at(1).text()).toEqual('Test Notification');
+    expect(cells.at(2).text()).toEqual('Success');
+    expect(cells.at(3).text()).toEqual('Slack');
   });
 
   test('should send test notification', async () => {
@@ -45,10 +49,14 @@ describe('<NotificationTemplateListItem />', () => {
     });
 
     const wrapper = mountWithContexts(
-      <NotificationTemplateListItem
-        template={template}
-        detailUrl="/notification_templates/3/detail"
-      />
+      <table>
+        <tbody>
+          <NotificationTemplateListItem
+            template={template}
+            detailUrl="/notification_templates/3/detail"
+          />
+        </tbody>
+      </table>
     );
     await act(async () => {
       wrapper
@@ -59,8 +67,8 @@ describe('<NotificationTemplateListItem />', () => {
     expect(NotificationTemplatesAPI.test).toHaveBeenCalledTimes(1);
     expect(
       wrapper
-        .find('DataListCell')
-        .at(1)
+        .find('Td')
+        .at(2)
         .text()
     ).toEqual('Running');
   });
@@ -69,10 +77,14 @@ describe('<NotificationTemplateListItem />', () => {
     NotificationTemplatesAPI.copy.mockResolvedValue();
 
     const wrapper = mountWithContexts(
-      <NotificationTemplateListItem
-        template={template}
-        detailUrl="/notification_templates/3/detail"
-      />
+      <table>
+        <tbody>
+          <NotificationTemplateListItem
+            template={template}
+            detailUrl="/notification_templates/3/detail"
+          />
+        </tbody>
+      </table>
     );
 
     await act(async () =>
@@ -86,10 +98,14 @@ describe('<NotificationTemplateListItem />', () => {
     NotificationTemplatesAPI.copy.mockRejectedValue(new Error());
 
     const wrapper = mountWithContexts(
-      <NotificationTemplateListItem
-        template={template}
-        detailUrl="/notification_templates/3/detail"
-      />
+      <table>
+        <tbody>
+          <NotificationTemplateListItem
+            template={template}
+            detailUrl="/notification_templates/3/detail"
+          />
+        </tbody>
+      </table>
     );
     await act(async () =>
       wrapper.find('Button[aria-label="Copy"]').prop('onClick')()
@@ -101,18 +117,22 @@ describe('<NotificationTemplateListItem />', () => {
 
   test('should not render copy button', async () => {
     const wrapper = mountWithContexts(
-      <NotificationTemplateListItem
-        template={{
-          ...template,
-          summary_fields: {
-            user_capabilities: {
-              copy: false,
-              edit: false,
-            },
-          },
-        }}
-        detailUrl="/notification_templates/3/detail"
-      />
+      <table>
+        <tbody>
+          <NotificationTemplateListItem
+            template={{
+              ...template,
+              summary_fields: {
+                user_capabilities: {
+                  copy: false,
+                  edit: false,
+                },
+              },
+            }}
+            detailUrl="/notification_templates/3/detail"
+          />
+        </tbody>
+      </table>
     );
     expect(wrapper.find('CopyButton').length).toBe(0);
   });