Merge pull request #6820 from marshmalien/6530-wf-node-detail-cleanup

Show missing detail fields in workflow node view modal Reviewed-by: https://github.com/apps/softwarefactory-project-zuul
2026-01-29 07:14:43 -03:30 · 2020-04-23 20:34:36 +00:00 · 2020-04-23 20:34:36 +00:00 · 9e87ddde69
commit 9e87ddde69
parent 5d40cf7635 75b54d2c14
11 changed files with 209 additions and 92 deletions
--- a/awx/ui_next/src/components/DetailList/DeletedDetail.jsx
+++ b/awx/ui_next/src/components/DetailList/DeletedDetail.jsx
@ -0,0 +1,22 @@
+import React from 'react';
+import { withI18n } from '@lingui/react';
+import { t } from '@lingui/macro';
+import { node } from 'prop-types';
+import styled from 'styled-components';
+import _Detail from './Detail';
+
+const Detail = styled(_Detail)`
+  dd& {
+    color: red;
+  }
+`;
+
+function DeletedDetail({ i18n, label }) {
+  return <Detail label={label} value={i18n._(t`Deleted`)} />;
+}
+
+DeletedDetail.propTypes = {
+  label: node.isRequired,
+};
+
+export default withI18n()(DeletedDetail);
--- a/awx/ui_next/src/components/DetailList/index.js
+++ b/awx/ui_next/src/components/DetailList/index.js
@ -1,3 +1,4 @@
 export { default as DetailList } from './DetailList';
 export { default as Detail, DetailName, DetailValue } from './Detail';
+export { default as DeletedDetail } from './DeletedDetail';
 export { default as UserDateDetail } from './UserDateDetail';
--- a/awx/ui_next/src/components/PromptDetail/PromptDetail.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptDetail.jsx
@ -34,6 +34,21 @@ function formatTimeout(timeout) {
  );
 }

+function buildResourceLink(resource) {
+  const link = {
+    job_template: `/templates/job_template/${resource.id}/details`,
+    project: `/projects/${resource.id}/details`,
+    inventory_source: `/inventories/inventory/${resource.inventory}/sources/${resource.id}/details`,
+    workflow_job_template: `/templates/workflow_job_template/${resource.id}/details`,
+  };
+
+  return link[(resource?.type)] ? (
+    <Link to={link[resource.type]}>{resource.name}</Link>
+  ) : (
+    resource.name
+  );
+}
+
 function hasPromptData(launchData) {
  return (
    launchData.ask_credential_on_launch ||
@ -150,7 +165,7 @@ function PromptDetail({ i18n, resource, launchConfig = {} }) {
  return (
    <>
      <DetailList gutter="sm">
-        <Detail label={i18n._(t`Name`)} value={details.name} />
+        <Detail label={i18n._(t`Name`)} value={buildResourceLink(resource)} />
        <Detail label={i18n._(t`Description`)} value={details.description} />
        <Detail
          label={i18n._(t`Type`)}
@ -160,19 +175,6 @@ function PromptDetail({ i18n, resource, launchConfig = {} }) {
          label={i18n._(t`Timeout`)}
          value={formatTimeout(details?.timeout)}
        />
-        {details?.summary_fields?.organization && (
-          <Detail
-            label={i18n._(t`Organization`)}
-            value={
-              <Link
-                to={`/organizations/${details?.summary_fields.organization.id}/details`}
-              >
-                {details?.summary_fields?.organization.name}
-              </Link>
-            }
-          />
-        )}
-
        {details?.type === 'project' && (
          <PromptProjectDetail resource={details} />
        )}
@ -185,17 +187,20 @@ function PromptDetail({ i18n, resource, launchConfig = {} }) {
        {details?.type === 'workflow_job_template' && (
          <PromptWFJobTemplateDetail resource={details} />
        )}
-
-        <UserDateDetail
-          label={i18n._(t`Created`)}
-          date={details?.created}
-          user={details?.summary_fields?.created_by}
-        />
-        <UserDateDetail
-          label={i18n._(t`Last Modified`)}
-          date={details?.modified}
-          user={details?.summary_fields?.modified_by}
-        />
+        {details?.created && (
+          <UserDateDetail
+            label={i18n._(t`Created`)}
+            date={details.created}
+            user={details?.summary_fields?.created_by}
+          />
+        )}
+        {details?.modified && (
+          <UserDateDetail
+            label={i18n._(t`Last Modified`)}
+            date={details?.modified}
+            user={details?.summary_fields?.modified_by}
+          />
+        )}
      </DetailList>

      {hasPromptData(launchConfig) && hasOverrides && (
--- a/awx/ui_next/src/components/PromptDetail/PromptInventorySourceDetail.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptInventorySourceDetail.jsx
@ -4,7 +4,7 @@ import { t } from '@lingui/macro';
 import { Link } from 'react-router-dom';

 import { Chip, ChipGroup, List, ListItem } from '@patternfly/react-core';
-import { Detail } from '@components/DetailList';
+import { Detail, DeletedDetail } from '@components/DetailList';
 import { VariablesDetail } from '@components/CodeMirrorInput';
 import CredentialChip from '@components/CredentialChip';

@ -57,6 +57,20 @@ function PromptInventorySourceDetail({ i18n, resource }) {

  return (
    <>
+      {summary_fields?.organization ? (
+        <Detail
+          label={i18n._(t`Organization`)}
+          value={
+            <Link
+              to={`/organizations/${summary_fields.organization.id}/details`}
+            >
+              {summary_fields?.organization.name}
+            </Link>
+          }
+        />
+      ) : (
+        <DeletedDetail label={i18n._(t`Organization`)} />
+      )}
      {summary_fields?.inventory && (
        <Detail
          label={i18n._(t`Inventory`)}
--- a/awx/ui_next/src/components/PromptDetail/PromptInventorySourceDetail.test.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptInventorySourceDetail.test.jsx
@ -3,6 +3,11 @@ import { mountWithContexts } from '@testUtils/enzymeHelpers';
 import PromptInventorySourceDetail from './PromptInventorySourceDetail';
 import mockInvSource from './data.inventory_source.json';

+function assertDetail(wrapper, label, value) {
+  expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
+  expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
+}
+
 describe('PromptInventorySourceDetail', () => {
  let wrapper;

@ -21,18 +26,13 @@ describe('PromptInventorySourceDetail', () => {
  });

  test('should render expected details', () => {
-    function assertDetail(label, value) {
-      expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
-      expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
-    }
-
-    assertDetail('Inventory', 'Demo Inventory');
-    assertDetail('Source', 'scm');
-    assertDetail('Project', 'Mock Project');
-    assertDetail('Inventory File', 'foo');
-    assertDetail('Custom Inventory Script', 'Mock Script');
-    assertDetail('Verbosity', '2 (More Verbose)');
-    assertDetail('Cache Timeout', '2 Seconds');
+    assertDetail(wrapper, 'Inventory', 'Demo Inventory');
+    assertDetail(wrapper, 'Source', 'scm');
+    assertDetail(wrapper, 'Project', 'Mock Project');
+    assertDetail(wrapper, 'Inventory File', 'foo');
+    assertDetail(wrapper, 'Custom Inventory Script', 'Mock Script');
+    assertDetail(wrapper, 'Verbosity', '2 (More Verbose)');
+    assertDetail(wrapper, 'Cache Timeout', '2 Seconds');
    expect(
      wrapper
        .find('Detail[label="Regions"]')
@ -74,4 +74,12 @@ describe('PromptInventorySourceDetail', () => {
        ])
    ).toEqual(true);
  });
+
+  test('should render "Deleted" details', () => {
+    delete mockInvSource.summary_fields.organization;
+    wrapper = mountWithContexts(
+      <PromptInventorySourceDetail resource={mockInvSource} />
+    );
+    assertDetail(wrapper, 'Organization', 'Deleted');
+  });
 });
--- a/awx/ui_next/src/components/PromptDetail/PromptJobTemplateDetail.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptJobTemplateDetail.jsx
@ -4,7 +4,7 @@ import { t } from '@lingui/macro';
 import { Link } from 'react-router-dom';

 import { Chip, ChipGroup, List, ListItem } from '@patternfly/react-core';
-import { Detail } from '@components/DetailList';
+import { Detail, DeletedDetail } from '@components/DetailList';
 import { VariablesDetail } from '@components/CodeMirrorInput';
 import CredentialChip from '@components/CredentialChip';
 import Sparkline from '@components/Sparkline';
@ -13,6 +13,7 @@ import { toTitleCase } from '@util/strings';
 function PromptJobTemplateDetail({ i18n, resource }) {
  const {
    allow_simultaneous,
+    ask_inventory_on_launch,
    become_enabled,
    diff_mode,
    extra_vars,
@ -84,7 +85,21 @@ function PromptJobTemplateDetail({ i18n, resource }) {
        />
      )}
      <Detail label={i18n._(t`Job Type`)} value={toTitleCase(job_type)} />
-      {summary_fields?.inventory && (
+      {summary_fields?.organization ? (
+        <Detail
+          label={i18n._(t`Organization`)}
+          value={
+            <Link
+              to={`/organizations/${summary_fields.organization.id}/details`}
+            >
+              {summary_fields?.organization.name}
+            </Link>
+          }
+        />
+      ) : (
+        <DeletedDetail label={i18n._(t`Organization`)} />
+      )}
+      {summary_fields?.inventory ? (
        <Detail
          label={i18n._(t`Inventory`)}
          value={
@ -95,8 +110,12 @@ function PromptJobTemplateDetail({ i18n, resource }) {
            </Link>
          }
        />
+      ) : (
+        !ask_inventory_on_launch && (
+          <DeletedDetail label={i18n._(t`Inventory`)} />
+        )
      )}
-      {summary_fields?.project && (
+      {summary_fields?.project ? (
        <Detail
          label={i18n._(t`Project`)}
          value={
@ -105,6 +124,8 @@ function PromptJobTemplateDetail({ i18n, resource }) {
            </Link>
          }
        />
+      ) : (
+        <DeletedDetail label={i18n._(t`Project`)} />
      )}
      <Detail label={i18n._(t`Source Control Branch`)} value={scm_branch} />
      <Detail label={i18n._(t`Playbook`)} value={playbook} />
--- a/awx/ui_next/src/components/PromptDetail/PromptJobTemplateDetail.test.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptJobTemplateDetail.test.jsx
@ -18,6 +18,11 @@ const mockJT = {
  ],
 };

+function assertDetail(wrapper, label, value) {
+  expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
+  expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
+}
+
 describe('PromptJobTemplateDetail', () => {
  let wrapper;

@ -34,24 +39,19 @@ describe('PromptJobTemplateDetail', () => {
  });

  test('should render expected details', () => {
-    function assertDetail(label, value) {
-      expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
-      expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
-    }
-
-    assertDetail('Job Type', 'Run');
-    assertDetail('Inventory', 'Demo Inventory');
-    assertDetail('Project', 'Mock Project');
-    assertDetail('Source Control Branch', 'Foo branch');
-    assertDetail('Playbook', 'ping.yml');
-    assertDetail('Forks', '2');
-    assertDetail('Limit', 'alpha:beta');
-    assertDetail('Verbosity', '3 (Debug)');
-    assertDetail('Show Changes', 'Off');
-    assertDetail('Job Slicing', '1');
-    assertDetail('Host Config Key', 'a1b2c3');
-    assertDetail('Webhook Service', 'Github');
-    assertDetail('Webhook Key', 'PiM3n2');
+    assertDetail(wrapper, 'Job Type', 'Run');
+    assertDetail(wrapper, 'Inventory', 'Demo Inventory');
+    assertDetail(wrapper, 'Project', 'Mock Project');
+    assertDetail(wrapper, 'Source Control Branch', 'Foo branch');
+    assertDetail(wrapper, 'Playbook', 'ping.yml');
+    assertDetail(wrapper, 'Forks', '2');
+    assertDetail(wrapper, 'Limit', 'alpha:beta');
+    assertDetail(wrapper, 'Verbosity', '3 (Debug)');
+    assertDetail(wrapper, 'Show Changes', 'Off');
+    assertDetail(wrapper, 'Job Slicing', '1');
+    assertDetail(wrapper, 'Host Config Key', 'a1b2c3');
+    assertDetail(wrapper, 'Webhook Service', 'Github');
+    assertDetail(wrapper, 'Webhook Key', 'PiM3n2');
    expect(wrapper.find('StatusIcon')).toHaveLength(2);
    expect(wrapper.find('Detail[label="Webhook URL"] dd').text()).toEqual(
      expect.stringContaining('/api/v2/job_templates/7/github/')
@ -112,4 +112,16 @@ describe('PromptJobTemplateDetail', () => {
      '---foo: bar'
    );
  });
+
+  test('should render "Deleted" details', () => {
+    delete mockJT.summary_fields.inventory;
+    delete mockJT.summary_fields.organization;
+    delete mockJT.summary_fields.project;
+
+    wrapper = mountWithContexts(<PromptJobTemplateDetail resource={mockJT} />);
+
+    assertDetail(wrapper, 'Inventory', 'Deleted');
+    assertDetail(wrapper, 'Organization', 'Deleted');
+    assertDetail(wrapper, 'Project', 'Deleted');
+  });
 });
--- a/awx/ui_next/src/components/PromptDetail/PromptProjectDetail.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptProjectDetail.jsx
@ -3,8 +3,9 @@ import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Config } from '@contexts/Config';
 import { List, ListItem } from '@patternfly/react-core';
+import { Link } from 'react-router-dom';

-import { Detail } from '@components/DetailList';
+import { Detail, DeletedDetail } from '@components/DetailList';
 import CredentialChip from '@components/CredentialChip';
 import { toTitleCase } from '@util/strings';

@ -49,6 +50,20 @@ function PromptProjectDetail({ i18n, resource }) {

  return (
    <>
+      {summary_fields?.organization ? (
+        <Detail
+          label={i18n._(t`Organization`)}
+          value={
+            <Link
+              to={`/organizations/${summary_fields.organization.id}/details`}
+            >
+              {summary_fields?.organization.name}
+            </Link>
+          }
+        />
+      ) : (
+        <DeletedDetail label={i18n._(t`Organization`)} />
+      )}
      <Detail
        label={i18n._(t`Source Control Type`)}
        value={scm_type === '' ? i18n._(t`Manual`) : toTitleCase(scm_type)}
--- a/awx/ui_next/src/components/PromptDetail/PromptProjectDetail.test.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptProjectDetail.test.jsx
@ -3,13 +3,18 @@ import { mountWithContexts } from '@testUtils/enzymeHelpers';
 import PromptProjectDetail from './PromptProjectDetail';
 import mockProject from './data.project.json';

+function assertDetail(wrapper, label, value) {
+  expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
+  expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
+}
+
 describe('PromptProjectDetail', () => {
  let wrapper;
+  const config = {
+    project_base_dir: 'dir/foo/bar',
+  };

  beforeAll(() => {
-    const config = {
-      project_base_dir: 'dir/foo/bar',
-    };
    wrapper = mountWithContexts(
      <PromptProjectDetail resource={mockProject} />,
      {
@ -27,23 +32,19 @@ describe('PromptProjectDetail', () => {
  });

  test('should render expected details', () => {
-    function assertDetail(label, value) {
-      expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
-      expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
-    }
-
-    assertDetail('Source Control Type', 'Git');
+    assertDetail(wrapper, 'Source Control Type', 'Git');
    assertDetail(
+      wrapper,
      'Source Control URL',
      'https://github.com/ansible/ansible-tower-samples'
    );
-    assertDetail('Source Control Branch', 'foo');
-    assertDetail('Source Control Refspec', 'refs/');
-    assertDetail('Cache Timeout', '3 Seconds');
-    assertDetail('Ansible Environment', 'mock virtual env');
-    assertDetail('Project Base Path', 'dir/foo/bar');
-    assertDetail('Playbook Directory', '_6__demo_project');
-    assertDetail('Source Control Credential', 'Scm: mock scm');
+    assertDetail(wrapper, 'Source Control Branch', 'foo');
+    assertDetail(wrapper, 'Source Control Refspec', 'refs/');
+    assertDetail(wrapper, 'Cache Timeout', '3 Seconds');
+    assertDetail(wrapper, 'Ansible Environment', 'mock virtual env');
+    assertDetail(wrapper, 'Project Base Path', 'dir/foo/bar');
+    assertDetail(wrapper, 'Playbook Directory', '_6__demo_project');
+    assertDetail(wrapper, 'Source Control Credential', 'Scm: mock scm');
    expect(
      wrapper
        .find('Detail[label="Options"]')
@ -55,4 +56,15 @@ describe('PromptProjectDetail', () => {
        ])
    ).toEqual(true);
  });
+
+  test('should render "Deleted" details', () => {
+    delete mockProject.summary_fields.organization;
+    wrapper = mountWithContexts(
+      <PromptProjectDetail resource={mockProject} />,
+      {
+        context: { config },
+      }
+    );
+    assertDetail(wrapper, 'Organization', 'Deleted');
+  });
 });
--- a/awx/ui_next/src/components/PromptDetail/PromptWFJobTemplateDetail.jsx
+++ b/awx/ui_next/src/components/PromptDetail/PromptWFJobTemplateDetail.jsx
@ -52,6 +52,18 @@ function PromptWFJobTemplateDetail({ i18n, resource }) {
          label={i18n._(t`Activity`)}
        />
      )}
+      {summary_fields?.organization && (
+        <Detail
+          label={i18n._(t`Organization`)}
+          value={
+            <Link
+              to={`/organizations/${summary_fields.organization.id}/details`}
+            >
+              {summary_fields?.organization.name}
+            </Link>
+          }
+        />
+      )}
      {summary_fields?.inventory && (
        <Detail
          label={i18n._(t`Inventory`)}
--- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx
+++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx
@ -10,7 +10,6 @@ import {
  TextListItemVariants,
  TextListVariants,
 } from '@patternfly/react-core';
-import styled from 'styled-components';
 import { t } from '@lingui/macro';

 import AlertModal from '@components/AlertModal';
@ -18,19 +17,18 @@ import { CardBody, CardActionsRow } from '@components/Card';
 import ContentError from '@components/ContentError';
 import ContentLoading from '@components/ContentLoading';
 import CredentialChip from '@components/CredentialChip';
-import { DetailList, Detail, UserDateDetail } from '@components/DetailList';
+import {
+  Detail,
+  DetailList,
+  DeletedDetail,
+  UserDateDetail,
+} from '@components/DetailList';
 import DeleteButton from '@components/DeleteButton';
 import ErrorDetail from '@components/ErrorDetail';
 import LaunchButton from '@components/LaunchButton';
 import { VariablesDetail } from '@components/CodeMirrorInput';
 import { JobTemplatesAPI } from '@api';

-const MissingDetail = styled(Detail)`
-  dd& {
-    color: red;
-  }
-`;
-
 function JobTemplateDetail({ i18n, template }) {
  const {
    ask_inventory_on_launch,
@ -133,10 +131,6 @@ function JobTemplateDetail({ i18n, template }) {
    </TextList>
  );

-  const renderMissingDataDetail = value => (
-    <MissingDetail label={value} value={i18n._(t`Deleted`)} />
-  );
-
  const inventoryValue = (kind, id) => {
    const inventorykind = kind === 'smart' ? 'smart_inventory' : 'inventory';

@ -180,7 +174,7 @@ function JobTemplateDetail({ i18n, template }) {
            }
          />
        ) : (
-          renderMissingDataDetail(i18n._(t`Project`))
+          <DeletedDetail label={i18n._(t`Organization`)} />
        )}
        {summary_fields.inventory ? (
          <Detail
@ -191,8 +185,9 @@ function JobTemplateDetail({ i18n, template }) {
            )}
          />
        ) : (
-          !ask_inventory_on_launch &&
-          renderMissingDataDetail(i18n._(t`Inventory`))
+          !ask_inventory_on_launch && (
+            <DeletedDetail label={i18n._(t`Inventory`)} />
+          )
        )}
        {summary_fields.project ? (
          <Detail
@ -204,7 +199,7 @@ function JobTemplateDetail({ i18n, template }) {
            }
          />
        ) : (
-          renderMissingDataDetail(i18n._(t`Project`))
+          <DeletedDetail label={i18n._(t`Project`)} />
        )}
        <Detail
          label={i18n._(t`Source Control Branch`)}