From 2f7607a080e1e591c7ffbe2b373e927fafbbd3b0 Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Mon, 16 Dec 2019 16:11:24 -0800
Subject: [PATCH] use VariablesDetail for displaying variables field in details
 views

---
 awx/ui_next/src/components/Card/index.js      |   1 +
 .../CodeMirrorInput/VariablesDetail.jsx       | 105 ++++++++++--------
 .../CodeMirrorInput/VariablesField.jsx        |   1 -
 .../screens/Host/HostDetail/HostDetail.jsx    |  58 +++-------
 .../Host/HostDetail/HostDetail.test.jsx       |  15 ++-
 .../InventoryDetail/InventoryDetail.jsx       |  17 ++-
 6 files changed, 90 insertions(+), 107 deletions(-)

diff --git a/awx/ui_next/src/components/Card/index.js b/awx/ui_next/src/components/Card/index.js
index 6d197d5600..55c70b8e14 100644
--- a/awx/ui_next/src/components/Card/index.js
+++ b/awx/ui_next/src/components/Card/index.js
@@ -1 +1,2 @@
+// eslint-disable-next-line import/prefer-default-export
 export { default as TabbedCardHeader } from './TabbedCardHeader';
diff --git a/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx b/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx
index 04c7a1ed4b..2ea8e15ff9 100644
--- a/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx
+++ b/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx
@@ -1,6 +1,7 @@
 import React, { useState } from 'react';
 import { string, number } from 'prop-types';
-import { Split, SplitItem } from '@patternfly/react-core';
+import { Split, SplitItem, TextListItemVariants } from '@patternfly/react-core';
+import { DetailName, DetailValue } from '@components/DetailList';
 import CodeMirrorInput from './CodeMirrorInput';
 import YamlJsonToggle from './YamlJsonToggle';
 import { yamlToJson, jsonToYaml, isJson } from '../../util/yaml';
@@ -10,54 +11,68 @@ const JSON_MODE = 'javascript';
 
 function VariablesDetail({ value, label, rows }) {
   const [mode, setMode] = useState(isJson(value) ? JSON_MODE : YAML_MODE);
-  const [val, setVal] = useState(value);
+  const [currentValue, setCurrentValue] = useState(value);
   const [error, setError] = useState(null);
 
   return (
-    <div css="margin: 20px 0">
-      <Split gutter="sm">
-        <SplitItem>
-          <div className="pf-c-form__label">
-            <span
-              className="pf-c-form__label-text"
-              css="font-weight: var(--pf-global--FontWeight--bold)"
-            >
-              {label}
-            </span>
-          </div>
-        </SplitItem>
-        <SplitItem>
-          <YamlJsonToggle
-            mode={mode}
-            onChange={newMode => {
-              try {
-                const newVal =
-                  newMode === YAML_MODE ? jsonToYaml(val) : yamlToJson(val);
-                setVal(newVal);
-                setMode(newMode);
-              } catch (err) {
-                setError(err);
-              }
-            }}
-          />
-        </SplitItem>
-      </Split>
-      <CodeMirrorInput
-        mode={mode}
-        value={val}
-        readOnly
-        rows={rows}
-        css="margin-top: 10px"
-      />
-      {error && (
-        <div
-          css="color: var(--pf-global--danger-color--100);
+    <>
+      <DetailName
+        component={TextListItemVariants.dt}
+        fullWidth
+        css="grid-column: 1 / -1"
+      >
+        <Split gutter="sm">
+          <SplitItem>
+            <div className="pf-c-form__label">
+              <span
+                className="pf-c-form__label-text"
+                css="font-weight: var(--pf-global--FontWeight--bold)"
+              >
+                {label}
+              </span>
+            </div>
+          </SplitItem>
+          <SplitItem>
+            <YamlJsonToggle
+              mode={mode}
+              onChange={newMode => {
+                try {
+                  const newVal =
+                    newMode === YAML_MODE
+                      ? jsonToYaml(currentValue)
+                      : yamlToJson(currentValue);
+                  setCurrentValue(newVal);
+                  setMode(newMode);
+                } catch (err) {
+                  setError(err);
+                }
+              }}
+            />
+          </SplitItem>
+        </Split>
+      </DetailName>
+      <DetailValue
+        component={TextListItemVariants.dd}
+        fullWidth
+        css="grid-column: 1 / -1; margin-top: -20px"
+      >
+        <CodeMirrorInput
+          mode={mode}
+          value={currentValue}
+          readOnly
+          rows={rows}
+          css="margin-top: 10px"
+        />
+        {error && (
+          <div
+            css="color: var(--pf-global--danger-color--100);
             font-size: var(--pf-global--FontSize--sm"
-        >
-          Error: {error.message}
-        </div>
-      )}
-    </div>
+          >
+            Error: {error.message}
+          </div>
+        )}
+      </DetailValue>
+    </>
   );
 }
 VariablesDetail.propTypes = {
diff --git a/awx/ui_next/src/components/CodeMirrorInput/VariablesField.jsx b/awx/ui_next/src/components/CodeMirrorInput/VariablesField.jsx
index 96b516946e..97c35c1871 100644
--- a/awx/ui_next/src/components/CodeMirrorInput/VariablesField.jsx
+++ b/awx/ui_next/src/components/CodeMirrorInput/VariablesField.jsx
@@ -7,7 +7,6 @@ import YamlJsonToggle from './YamlJsonToggle';
 import { yamlToJson, jsonToYaml } from '../../util/yaml';
 
 const YAML_MODE = 'yaml';
-const JSON_MODE = 'javascript';
 
 function VariablesField({ id, name, label, readOnly }) {
   // TODO: detect initial mode
diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx
index e9e18c3c4f..cf026f40f7 100644
--- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx
+++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.jsx
@@ -4,10 +4,9 @@ import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import styled from 'styled-components';
 import { Host } from '@types';
-import { formatDateString } from '@util/dates';
 import { Button, CardBody } from '@patternfly/react-core';
-import { DetailList, Detail } from '@components/DetailList';
-import CodeMirrorInput from '@components/CodeMirrorInput';
+import { DetailList, Detail, UserDateDetail } from '@components/DetailList';
+import { VariablesDetail } from '@components/CodeMirrorInput';
 
 const ActionButtonWrapper = styled.div`
   display: flex;
@@ -21,30 +20,6 @@ const ActionButtonWrapper = styled.div`
 function HostDetail({ host, i18n }) {
   const { created, description, id, modified, name, summary_fields } = host;
 
-  let createdBy = '';
-  if (created) {
-    if (summary_fields.created_by && summary_fields.created_by.username) {
-      createdBy = i18n._(
-        t`${formatDateString(created)} by ${summary_fields.created_by.username}`
-      );
-    } else {
-      createdBy = formatDateString(created);
-    }
-  }
-
-  let modifiedBy = '';
-  if (modified) {
-    if (summary_fields.modified_by && summary_fields.modified_by.username) {
-      modifiedBy = i18n._(
-        t`${formatDateString(modified)} by ${
-          summary_fields.modified_by.username
-        }`
-      );
-    } else {
-      modifiedBy = formatDateString(modified);
-    }
-  }
-
   return (
     <CardBody>
       <DetailList gutter="sm">
@@ -66,23 +41,20 @@ function HostDetail({ host, i18n }) {
             }
           />
         )}
-        {/* TODO: Link to user in users */}
-        <Detail label={i18n._(t`Created`)} value={createdBy} />
-        {/* TODO: Link to user in users */}
-        <Detail label={i18n._(t`Last Modified`)} value={modifiedBy} />
-        <Detail
-          fullWidth
+        <UserDateDetail
+          label={i18n._(t`Created`)}
+          date={created}
+          user={summary_fields.created_by}
+        />
+        <UserDateDetail
+          label={i18n._(t`Last Modified`)}
+          date={modified}
+          user={summary_fields.modified_by}
+        />
+        <VariablesDetail
           label={i18n._(t`Variables`)}
-          value={
-            <CodeMirrorInput
-              mode="yaml"
-              readOnly
-              value={host.variables}
-              onChange={() => {}}
-              rows={6}
-              hasErrors={false}
-            />
-          }
+          value={host.variables}
+          rows={6}
         />
       </DetailList>
       <ActionButtonWrapper>
diff --git a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx
index a748f9596d..749c512171 100644
--- a/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx
+++ b/awx/ui_next/src/screens/Host/HostDetail/HostDetail.test.jsx
@@ -30,7 +30,7 @@ describe('<HostDetail />', () => {
     mountWithContexts(<HostDetail host={mockHost} />);
   });
 
-  test('should render Details', async done => {
+  test('should render Details', async () => {
     const wrapper = mountWithContexts(<HostDetail host={mockHost} />);
     const testParams = [
       { label: 'Name', value: 'Foo' },
@@ -46,23 +46,22 @@ describe('<HostDetail />', () => {
       expect(detail.find('dt').text()).toBe(label);
       expect(detail.find('dd').text()).toBe(value);
     }
-    done();
   });
 
-  test('should show edit button for users with edit permission', async done => {
+  test('should show edit button for users with edit permission', async () => {
     const wrapper = mountWithContexts(<HostDetail host={mockHost} />);
-    const editButton = await waitForElement(wrapper, 'HostDetail Button');
+    // VariablesDetail has two buttons
+    const editButton = wrapper.find('Button').at(2);
     expect(editButton.text()).toEqual('Edit');
     expect(editButton.prop('to')).toBe('/hosts/1/edit');
-    done();
   });
 
-  test('should hide edit button for users without edit permission', async done => {
+  test('should hide edit button for users without edit permission', async () => {
     const readOnlyHost = { ...mockHost };
     readOnlyHost.summary_fields.user_capabilities.edit = false;
     const wrapper = mountWithContexts(<HostDetail host={readOnlyHost} />);
     await waitForElement(wrapper, 'HostDetail');
-    expect(wrapper.find('HostDetail Button').length).toBe(0);
-    done();
+    // VariablesDetail has two buttons
+    expect(wrapper.find('Button').length).toBe(2);
   });
 });
diff --git a/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx b/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx
index dffff1881c..933b8794f3 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx
@@ -56,16 +56,13 @@ function InventoryDetail({ inventory, i18n }) {
             )
           }
         />
-      </DetailList>
-      {inventory.variables && (
-        <VariablesDetail
-          id="job-artifacts"
-          label={i18n._(t`Variables`)}
-          value={inventory.variables}
-          rows={4}
-        />
-      )}
-      <DetailList>
+        {inventory.variables && (
+          <VariablesDetail
+            label={i18n._(t`Variables`)}
+            value={inventory.variables}
+            rows={4}
+          />
+        )}
         <UserDateDetail
           label={i18n._(t`Created`)}
           date={inventory.created}