Merge pull request #5204 from AlexSCorey/5106-MissingOrDeletedFields

Adds `Deleted` text to missing resources in JT Details View

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/components/DetailList/Detail.jsx

@@ -1,4 +1,4 @@
-import React, { Fragment } from 'react';
+import React from 'react';
 import { node, bool } from 'prop-types';
 import { TextListItem, TextListItemVariants } from '@patternfly/react-core';
 import styled from 'styled-components';
@@ -25,19 +25,28 @@ const DetailValue = styled(({ fullWidth, ...props }) => (
   `}
 `;
 
-const Detail = ({ label, value, fullWidth }) => {
+const Detail = ({ label, value, fullWidth, className }) => {
   if (!value && typeof value !== 'number') {
     return null;
   }
+
   return (
-    <Fragment>
+    <>
-      <DetailName component={TextListItemVariants.dt} fullWidth={fullWidth}>
+      <DetailName
+        className={className}
+        component={TextListItemVariants.dt}
+        fullWidth={fullWidth}
+      >
         {label}
       </DetailName>
-      <DetailValue component={TextListItemVariants.dd} fullWidth={fullWidth}>
+      <DetailValue
+        className={className}
+        component={TextListItemVariants.dd}
+        fullWidth={fullWidth}
+      >
         {value}
       </DetailValue>
-    </Fragment>
+    </>
   );
 };
 Detail.propTypes = {

awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx

@@ -9,13 +9,20 @@ import mockJobData from '../shared/data.job.json';
 jest.mock('@api');
 
 describe('<JobDetail />', () => {
+  let wrapper;
+  beforeEach(() => {
+    wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
+  });
+  afterEach(() => {
+    wrapper.unmount();
+  });
   test('initially renders succesfully', () => {
-    mountWithContexts(<JobDetail job={mockJobData} />);
+    wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
+
+    expect(wrapper.length).toBe(1);
   });
 
   test('should display details', () => {
-    const wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
-
     function assertDetail(label, value) {
       expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
       expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value);
@@ -43,7 +50,6 @@ describe('<JobDetail />', () => {
   });
 
   test('should display credentials', () => {
-    const wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
     const credentialChip = wrapper.find('CredentialChip');
 
     expect(credentialChip.prop('credential')).toEqual(
@@ -52,21 +58,18 @@ describe('<JobDetail />', () => {
   });
 
   test('should display successful job status icon', () => {
-    const wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
     const statusDetail = wrapper.find('Detail[label="Status"]');
     expect(statusDetail.find('StatusIcon__SuccessfulTop')).toHaveLength(1);
     expect(statusDetail.find('StatusIcon__SuccessfulBottom')).toHaveLength(1);
   });
 
   test('should display successful project status icon', () => {
-    const wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
     const statusDetail = wrapper.find('Detail[label="Project"]');
     expect(statusDetail.find('StatusIcon__SuccessfulTop')).toHaveLength(1);
     expect(statusDetail.find('StatusIcon__SuccessfulBottom')).toHaveLength(1);
   });
 
   test('should properly delete job', async () => {
-    const wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
     wrapper.find('button[aria-label="Delete"]').simulate('click');
     await sleep(1);
     wrapper.update();
@@ -89,8 +92,6 @@ describe('<JobDetail />', () => {
         },
       })
     );
-    const wrapper = mountWithContexts(<JobDetail job={mockJobData} />);
-
     wrapper.find('button[aria-label="Delete"]').simulate('click');
     const modal = wrapper.find('Modal');
     expect(modal.length).toBe(1);
@@ -102,4 +103,23 @@ describe('<JobDetail />', () => {
     const errorModal = wrapper.find('ErrorDetail');
     expect(errorModal.length).toBe(1);
   });
+
+  test('DELETED is shown for required Job resources that have been deleted', () => {
+    const newMockJobData = { ...mockJobData };
+    newMockJobData.summary_fields.inventory = null;
+    newMockJobData.summary_fields.project = null;
+    const newWrapper = mountWithContexts(
+      <JobDetail job={newMockJobData} />
+    ).find('JobDetail');
+    async function assertMissingDetail(label) {
+      expect(newWrapper.length).toBe(1);
+      await sleep(0);
+      expect(newWrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label);
+      expect(newWrapper.find(`Detail[label="${label}"] dd`).text()).toBe(
+        'DELETED'
+      );
+    }
+    assertMissingDetail('Project');
+    assertMissingDetail('Inventory');
+  });
 });

awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx

@@ -1,4 +1,4 @@
-import React, { Component } from 'react';
+import React, { Component, Fragment } from 'react';
 import { Link, withRouter } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import {
@@ -28,6 +28,12 @@ const ButtonGroup = styled.div`
     margin-left: 20px;
   }
 `;
+
+const MissingDetail = styled(Detail)`
+  dd& {
+    color: red;
+  }
+`;
 class JobTemplateDetail extends Component {
   constructor(props) {
     super(props);
@@ -60,6 +66,7 @@ class JobTemplateDetail extends Component {
   render() {
     const {
       template: {
+        ask_inventory_on_launch,
         allow_simultaneous,
         become_enabled,
         created,
@@ -156,6 +163,28 @@ class JobTemplateDetail extends Component {
       </TextList>
     );
 
+    const renderMissingDataDetail = value => (
+      <MissingDetail label={value} value={i18n._(t`Deleted`)} />
+    );
+
+    const inventoryValue = (kind, id) => {
+      const inventorykind =
+        kind === 'smart' ? (kind = 'smart_inventory') : (kind = 'inventory');
+
+      return ask_inventory_on_launch ? (
+        <Fragment>
+          <Link to={`/inventories/${inventorykind}/${id}/details`}>
+            {summary_fields.inventory.name}
+          </Link>
+          <span> {i18n._(t`(Prompt on Launch)`)}</span>
+        </Fragment>
+      ) : (
+        <Link to={`/inventories/${inventorykind}/${id}/details`}>
+          {summary_fields.inventory.name}
+        </Link>
+      );
+    };
+
     if (contentError) {
       return <ContentError error={contentError} />;
     }
@@ -171,31 +200,32 @@ class JobTemplateDetail extends Component {
             <Detail label={i18n._(t`Name`)} value={name} />
             <Detail label={i18n._(t`Description`)} value={description} />
             <Detail label={i18n._(t`Job Type`)} value={job_type} />
-            {summary_fields.inventory && (
+
+            {summary_fields.inventory ? (
               <Detail
                 label={i18n._(t`Inventory`)}
-                value={
+                value={inventoryValue(
-                  <Link
+                  summary_fields.inventory.kind,
-                    to={`/inventories/${
+                  summary_fields.inventory.id
-                      summary_fields.inventory.kind === 'smart'
+                )}
-                        ? 'smart_inventory'
-                        : 'inventory'
-                    }/${summary_fields.inventory.id}/details`}
-                  >
-                    {summary_fields.inventory.name}
-                  </Link>
-                }
               />
+            ) : (
+              !ask_inventory_on_launch &&
+              renderMissingDataDetail(i18n._(t`Inventory`))
             )}
-            {summary_fields.project && (
+            {summary_fields.project ? (
               <Detail
                 label={i18n._(t`Project`)}
                 value={
                   <Link to={`/projects/${summary_fields.project.id}/details`}>
-                    {summary_fields.project.name}
+                    {summary_fields.project
+                      ? summary_fields.project.name
+                      : i18n._(t`Deleted`)}
                   </Link>
                 }
               />
+            ) : (
+              renderMissingDataDetail(i18n._(t`Project`))
             )}
             <Detail label={i18n._(t`Playbook`)} value={playbook} />
             <Detail label={i18n._(t`Forks`)} value={forks || '0'} />

awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx

@@ -9,6 +9,8 @@ import { JobTemplate } from '@types';
 import { getAddedAndRemoved } from '@util/lists';
 import JobTemplateForm from '../shared/JobTemplateForm';
 
+const loadRelatedProjectPlaybooks = async project =>
+  ProjectsAPI.readPlaybooks(project);
 class JobTemplateEdit extends Component {
   static propTypes = {
     template: JobTemplate.isRequired,
@@ -33,9 +35,6 @@ class JobTemplateEdit extends Component {
     this.handleCancel = this.handleCancel.bind(this);
     this.handleSubmit = this.handleSubmit.bind(this);
     this.loadRelatedCredentials = this.loadRelatedCredentials.bind(this);
-    this.loadRelatedProjectPlaybooks = this.loadRelatedProjectPlaybooks.bind(
-      this
-    );
     this.submitLabels = this.submitLabels.bind(this);
   }
 
@@ -44,15 +43,20 @@ class JobTemplateEdit extends Component {
   }
 
   async loadRelated() {
+    const {
+      template: { project },
+    } = this.props;
     this.setState({ contentError: null, hasContentLoading: true });
     try {
-      const [relatedCredentials, relatedProjectPlaybooks] = await Promise.all([
+      if (project) {
-        this.loadRelatedCredentials(),
+        const { data: playbook = [] } = await loadRelatedProjectPlaybooks(
-        this.loadRelatedProjectPlaybooks(),
+          project
-      ]);
+        );
+        this.setState({ relatedProjectPlaybooks: playbook });
+      }
+      const [relatedCredentials] = await this.loadRelatedCredentials();
       this.setState({
         relatedCredentials,
-        relatedProjectPlaybooks,
       });
     } catch (contentError) {
       this.setState({ contentError });
@@ -89,19 +93,6 @@ class JobTemplateEdit extends Component {
     }
   }
 
-  async loadRelatedProjectPlaybooks() {
-    const {
-      template: { project },
-    } = this.props;
-    try {
-      const { data: playbooks = [] } = await ProjectsAPI.readPlaybooks(project);
-      this.setState({ relatedProjectPlaybooks: playbooks });
-      return playbooks;
-    } catch (err) {
-      throw err;
-    }
-  }
-
   async handleSubmit(values) {
     const { template, history } = this.props;
     const {

awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx

@@ -44,6 +44,10 @@ const mockJobTemplate = {
       { id: 1, kind: 'cloud', name: 'Foo' },
       { id: 2, kind: 'ssh', name: 'Bar' },
     ],
+    project: {
+      id: 15,
+      name: 'Boo',
+    },
   },
 };
 
@@ -237,4 +241,50 @@ describe('<JobTemplateEdit />', () => {
       '/templates/job_template/1/details'
     );
   });
+  test('should not call ProjectsAPI.readPlaybooks if there is no project', async () => {
+    const history = createMemoryHistory({});
+    const noProjectTemplate = {
+      id: 1,
+      name: 'Foo',
+      description: 'Bar',
+      job_type: 'run',
+      inventory: 2,
+      playbook: 'Baz',
+      type: 'job_template',
+      forks: 0,
+      limit: '',
+      verbosity: '0',
+      job_slice_count: 1,
+      timeout: 0,
+      job_tags: '',
+      skip_tags: '',
+      diff_mode: false,
+      allow_callbacks: false,
+      allow_simultaneous: false,
+      use_fact_cache: false,
+      host_config_key: '',
+      summary_fields: {
+        user_capabilities: {
+          edit: true,
+        },
+        labels: {
+          results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }],
+        },
+        inventory: {
+          id: 2,
+          organization_id: 1,
+        },
+        credentials: [
+          { id: 1, kind: 'cloud', name: 'Foo' },
+          { id: 2, kind: 'ssh', name: 'Bar' },
+        ],
+      },
+    };
+    await act(async () =>
+      mountWithContexts(<JobTemplateEdit template={noProjectTemplate} />, {
+        context: { router: { history } },
+      })
+    );
+    expect(ProjectsAPI.readPlaybooks).not.toBeCalled();
+  });
 });

awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx

@@ -8,7 +8,11 @@ import {
 } from '@patternfly/react-core';
 import { t } from '@lingui/macro';
 import { withI18n } from '@lingui/react';
-import { PencilAltIcon, RocketIcon } from '@patternfly/react-icons';
+import {
+  ExclamationTriangleIcon,
+  PencilAltIcon,
+  RocketIcon,
+} from '@patternfly/react-icons';
 
 import ActionButtonCell from '@components/ActionButtonCell';
 import DataListCell from '@components/DataListCell';
@@ -58,7 +62,10 @@ class TemplateListItem extends Component {
   render() {
     const { i18n, template, isSelected, onSelect } = this.props;
     const canLaunch = template.summary_fields.user_capabilities.start;
-
+    const missingResourceIcon =
+      (!template.summary_fields.inventory &&
+        !template.ask_inventory_on_launch) ||
+      !template.summary_fields.project;
     return (
       <DataListItem
         aria-labelledby={`check-action-${template.id}`}
@@ -80,6 +87,16 @@ class TemplateListItem extends Component {
                     <b>{template.name}</b>
                   </Link>
                 </span>
+                {missingResourceIcon && (
+                  <Tooltip
+                    content={i18n._(
+                      t`Resources are missing from this template.`
+                    )}
+                    position="right"
+                  >
+                    <ExclamationTriangleIcon css="color: #c9190b; margin-left: 20px;" />
+                  </Tooltip>
+                )}
               </LeftDataListCell>,
               <RightDataListCell
                 css="padding-left: 40px;"

awx/ui_next/src/screens/Template/TemplateList/TemplatesListItem.test.jsx

@@ -81,4 +81,65 @@ describe('<TemplatesListItem />', () => {
     );
     expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy();
   });
+  test('missing resource icon is shown.', () => {
+    const wrapper = mountWithContexts(
+      <TemplatesListItem
+        isSelected={false}
+        template={{
+          id: 1,
+          name: 'Template 1',
+          url: '/templates/job_template/1',
+          type: 'job_template',
+          summary_fields: {
+            user_capabilities: {
+              edit: false,
+            },
+          },
+        }}
+      />
+    );
+    expect(wrapper.find('ExclamationTriangleIcon').exists()).toBe(true);
+  });
+  test('missing resource icon is not shown when there is a project and an inventory.', () => {
+    const wrapper = mountWithContexts(
+      <TemplatesListItem
+        isSelected={false}
+        template={{
+          id: 1,
+          name: 'Template 1',
+          url: '/templates/job_template/1',
+          type: 'job_template',
+          summary_fields: {
+            user_capabilities: {
+              edit: false,
+            },
+            project: { name: 'Foo', id: 2 },
+            inventory: { name: 'Bar', id: 2 },
+          },
+        }}
+      />
+    );
+    expect(wrapper.find('ExclamationTriangleIcon').exists()).toBe(false);
+  });
+  test('missing resource icon is not shown when inventory is prompt_on_launch, and a project', () => {
+    const wrapper = mountWithContexts(
+      <TemplatesListItem
+        isSelected={false}
+        template={{
+          id: 1,
+          name: 'Template 1',
+          url: '/templates/job_template/1',
+          type: 'job_template',
+          ask_inventory_on_launch: true,
+          summary_fields: {
+            user_capabilities: {
+              edit: false,
+            },
+            project: { name: 'Foo', id: 2 },
+          },
+        }}
+      />
+    );
+    expect(wrapper.find('ExclamationTriangleIcon').exists()).toBe(false);
+  });
 });

awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx

@@ -572,7 +572,9 @@ const FormikApp = withFormik({
         inventory: { organization: null },
       },
     } = template;
-
+    const hasInventory = summary_fields.inventory
+      ? summary_fields.inventory.organization_id
+      : null;
     return {
       name: template.name || '',
       description: template.description || '',
@@ -594,7 +596,7 @@ const FormikApp = withFormik({
       allow_simultaneous: template.allow_simultaneous || false,
       use_fact_cache: template.use_fact_cache || false,
       host_config_key: template.host_config_key || '',
-      organizationId: summary_fields.inventory.organization_id || null,
+      organizationId: hasInventory,
       initialInstanceGroups: [],
       instanceGroups: [],
       credentials: summary_fields.credentials || [],