Merge pull request #5752 from marshmalien/details-delete-job-template

Add delete button to Job Template details

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

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

@@ -1,5 +1,5 @@
-import React, { Component, Fragment } from 'react';
+import React, { Fragment, useState, useEffect } from 'react';
-import { Link, withRouter } from 'react-router-dom';
+import { Link, useHistory, useParams } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import {
   Button,
@@ -11,12 +11,15 @@ import {
 import styled from 'styled-components';
 import { t } from '@lingui/macro';
 
+import AlertModal from '@components/AlertModal';
 import { CardBody, CardActionsRow } from '@components/Card';
 import ContentError from '@components/ContentError';
-import LaunchButton from '@components/LaunchButton';
 import ContentLoading from '@components/ContentLoading';
 import { ChipGroup, Chip, CredentialChip } from '@components/Chip';
 import { DetailList, Detail, UserDateDetail } from '@components/DetailList';
+import DeleteButton from '@components/DeleteButton';
+import ErrorDetail from '@components/ErrorDetail';
+import LaunchButton from '@components/LaunchButton';
 import { JobTemplatesAPI } from '@api';
 
 const MissingDetail = styled(Detail)`
@@ -25,38 +28,8 @@ const MissingDetail = styled(Detail)`
   }
 `;
 
-class JobTemplateDetail extends Component {
+function JobTemplateDetail({ i18n, template }) {
-  constructor(props) {
-    super(props);
-    this.state = {
-      contentError: null,
-      hasContentLoading: true,
-      instanceGroups: [],
-    };
-    this.readInstanceGroups = this.readInstanceGroups.bind(this);
-  }
-
-  componentDidMount() {
-    this.readInstanceGroups();
-  }
-
-  async readInstanceGroups() {
-    const { match } = this.props;
-    try {
-      const { data } = await JobTemplatesAPI.readInstanceGroups(
-        match.params.id
-      );
-      this.setState({ instanceGroups: [...data.results] });
-    } catch (err) {
-      this.setState({ contentError: err });
-    } finally {
-      this.setState({ hasContentLoading: false });
-    }
-  }
-
-  render() {
   const {
-      template: {
     ask_inventory_on_launch,
     allow_simultaneous,
     become_enabled,
@@ -78,14 +51,44 @@ class JobTemplateDetail extends Component {
     use_fact_cache,
     url,
     verbosity,
-      },
+  } = template;
-      hasTemplateLoading,
+  const [contentError, setContentError] = useState(null);
-      template,
+  const [deletionError, setDeletionError] = useState(null);
-      i18n,
+  const [hasContentLoading, setHasContentLoading] = useState(false);
-      match,
+  const [instanceGroups, setInstanceGroups] = useState([]);
-    } = this.props;
+  const { id: templateId } = useParams();
-    const canLaunch = summary_fields.user_capabilities.start;
+  const history = useHistory();
-    const { instanceGroups, hasContentLoading, contentError } = this.state;
+
+  useEffect(() => {
+    (async () => {
+      setContentError(null);
+      setHasContentLoading(true);
+      try {
+        const {
+          data: { results = [] },
+        } = await JobTemplatesAPI.readInstanceGroups(templateId);
+        setInstanceGroups(results);
+      } catch (error) {
+        setContentError(error);
+      } finally {
+        setHasContentLoading(false);
+      }
+    })();
+  }, [templateId]);
+
+  const handleDelete = async () => {
+    setHasContentLoading(true);
+    try {
+      await JobTemplatesAPI.destroy(templateId);
+      history.push(`/templates`);
+    } catch (error) {
+      setDeletionError(error);
+    }
+    setHasContentLoading(false);
+  };
+
+  const canLaunch =
+    summary_fields.user_capabilities && summary_fields.user_capabilities.start;
   const verbosityOptions = [
     { verbosity: 0, details: i18n._(t`0 (Normal)`) },
     { verbosity: 1, details: i18n._(t`1 (Verbose)`) },
@@ -98,8 +101,6 @@ class JobTemplateDetail extends Component {
     option => option.verbosity === verbosity
   );
   const generateCallBackUrl = `${window.location.origin + url}callback/`;
-    const isInitialized = !hasTemplateLoading && !hasContentLoading;
-
   const renderOptionsField =
     become_enabled || host_config_key || allow_simultaneous || use_fact_cache;
 
@@ -133,8 +134,7 @@ class JobTemplateDetail extends Component {
   );
 
   const inventoryValue = (kind, id) => {
-      const inventorykind =
+    const inventorykind = kind === 'smart' ? 'smart_inventory' : 'inventory';
-        kind === 'smart' ? (kind = 'smart_inventory') : (kind = 'inventory');
 
     return ask_inventory_on_launch ? (
       <Fragment>
@@ -159,17 +159,11 @@ class JobTemplateDetail extends Component {
   }
 
   return (
-      isInitialized && (
     <CardBody>
       <DetailList gutter="sm">
-            <Detail
+        <Detail label={i18n._(t`Name`)} value={name} dataCy="jt-detail-name" />
-              label={i18n._(t`Name`)}
-              value={name}
-              dataCy="jt-detail-name"
-            />
         <Detail label={i18n._(t`Description`)} value={description} />
         <Detail label={i18n._(t`Job Type`)} value={job_type} />
-
         {summary_fields.inventory ? (
           <Detail
             label={i18n._(t`Inventory`)}
@@ -187,9 +181,7 @@ class JobTemplateDetail extends Component {
             label={i18n._(t`Project`)}
             value={
               <Link to={`/projects/${summary_fields.project.id}/details`}>
-                    {summary_fields.project
+                {summary_fields.project.name}
-                      ? summary_fields.project.name
-                      : i18n._(t`Deleted`)}
               </Link>
             }
           />
@@ -235,8 +227,7 @@ class JobTemplateDetail extends Component {
         {renderOptionsField && (
           <Detail label={i18n._(t`Options`)} value={renderOptions} />
         )}
-            {summary_fields.credentials &&
+        {summary_fields.credentials && summary_fields.credentials.length > 0 && (
-              summary_fields.credentials.length > 0 && (
           <Detail
             fullWidth
             label={i18n._(t`Credentials`)}
@@ -311,10 +302,11 @@ class JobTemplateDetail extends Component {
         )}
       </DetailList>
       <CardActionsRow>
-            {summary_fields.user_capabilities.edit && (
+        {summary_fields.user_capabilities &&
+          summary_fields.user_capabilities.edit && (
             <Button
               component={Link}
-                to={`/templates/job_template/${match.params.id}/edit`}
+              to={`/templates/job_template/${templateId}/edit`}
               aria-label={i18n._(t`Edit`)}
             >
               {i18n._(t`Edit`)}
@@ -323,21 +315,38 @@ class JobTemplateDetail extends Component {
         {canLaunch && (
           <LaunchButton resource={template} aria-label={i18n._(t`Launch`)}>
             {({ handleLaunch }) => (
-                  <Button
+              <Button variant="secondary" type="submit" onClick={handleLaunch}>
-                    variant="secondary"
-                    type="submit"
-                    onClick={handleLaunch}
-                  >
                 {i18n._(t`Launch`)}
               </Button>
             )}
           </LaunchButton>
         )}
+        {summary_fields.user_capabilities &&
+          summary_fields.user_capabilities.delete && (
+            <DeleteButton
+              name={name}
+              modalTitle={i18n._(t`Delete Job Template`)}
+              onConfirm={handleDelete}
+            >
+              {i18n._(t`Delete`)}
+            </DeleteButton>
+          )}
       </CardActionsRow>
+      {/* Update delete modal to show dependencies https://github.com/ansible/awx/issues/5546 */}
+      {deletionError && (
+        <AlertModal
+          isOpen={deletionError}
+          variant="danger"
+          title={i18n._(t`Error!`)}
+          onClose={() => setDeletionError(null)}
+        >
+          {i18n._(t`Failed to delete job template.`)}
+          <ErrorDetail error={deletionError} />
+        </AlertModal>
+      )}
     </CardBody>
-      )
   );
 }
-}
+
 export { JobTemplateDetail as _JobTemplateDetail };
-export default withI18n()(withRouter(JobTemplateDetail));
+export default withI18n()(JobTemplateDetail);

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

@@ -1,37 +1,12 @@
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
-import JobTemplateDetail, { _JobTemplateDetail } from './JobTemplateDetail';
+import JobTemplateDetail from './JobTemplateDetail';
 import { JobTemplatesAPI } from '@api';
+import mockTemplate from '../shared/data.job_template.json';
 
 jest.mock('@api');
 
-describe('<JobTemplateDetail />', () => {
-  const template = {
-    forks: 1,
-    host_config_key: 'ssh',
-    name: 'Temp 1',
-    job_type: 'run',
-    inventory: 1,
-    limit: '1',
-    project: 7,
-    playbook: '',
-    id: 1,
-    verbosity: 1,
-    summary_fields: {
-      user_capabilities: { edit: true },
-      created_by: { id: 1, username: 'Joe' },
-      modified_by: { id: 1, username: 'Joe' },
-      credentials: [
-        { id: 1, kind: 'ssh', name: 'Credential 1' },
-        { id: 2, kind: 'awx', name: 'Credential 2' },
-      ],
-      inventory: { name: 'Inventory' },
-      project: { name: 'Project' },
-    },
-    created: '2020-04-25T01:23:45.678901Z',
-    modified: '2020-04-25T01:23:45.678901Z',
-  };
-
 const mockInstanceGroups = {
   count: 5,
   data: {
@@ -39,123 +14,128 @@ describe('<JobTemplateDetail />', () => {
   },
 };
 
-  const readInstanceGroups = jest.spyOn(
+describe('<JobTemplateDetail />', () => {
-    _JobTemplateDetail.prototype,
+  let wrapper;
-    'readInstanceGroups'
-  );
 
-  beforeEach(() => {
+  beforeEach(async () => {
     JobTemplatesAPI.readInstanceGroups.mockResolvedValue(mockInstanceGroups);
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <JobTemplateDetail template={mockTemplate} />
+      );
+    });
+    await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
   });
 
   afterEach(() => {
     jest.clearAllMocks();
   });
 
-  test('Can load with missing summary fields', async () => {
+  test('should render successfully with missing summary fields', async () => {
-    const mockTemplate = { ...template };
+    await act(async () => {
-    mockTemplate.summary_fields = { user_capabilities: {} };
+      wrapper = mountWithContexts(
-
+        <JobTemplateDetail
-    const wrapper = mountWithContexts(
+          template={{
-      <JobTemplateDetail template={mockTemplate} />
+            ...mockTemplate,
+            become_enabled: true,
+            summary_fields: { user_capabilities: {} },
+          }}
+        />
       );
+    });
+    await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
     await waitForElement(
       wrapper,
-      'Detail[label="Description"]',
+      'Detail[label="Name"]',
       el => el.length === 1
     );
   });
 
-  test('When component mounts API is called to get instance groups', async done => {
+  test('should request instance groups from api', async () => {
-    const wrapper = mountWithContexts(
-      <JobTemplateDetail template={template} />
-    );
-    await waitForElement(
-      wrapper,
-      'JobTemplateDetail',
-      el => el.state('hasContentLoading') === true
-    );
-    expect(readInstanceGroups).toHaveBeenCalled();
-    await waitForElement(
-      wrapper,
-      'JobTemplateDetail',
-      el => el.state('hasContentLoading') === false
-    );
     expect(JobTemplatesAPI.readInstanceGroups).toHaveBeenCalledTimes(1);
-    done();
   });
 
-  test('Edit button is absent when user does not have edit privilege', async done => {
+  test('should hide edit button for users without edit permission', async () => {
-    const regularUser = {
+    JobTemplatesAPI.readInstanceGroups.mockResolvedValue({ data: {} });
-      forks: 1,
+    await act(async () => {
-      host_config_key: 'ssh',
+      wrapper = mountWithContexts(
-      name: 'Temp 1',
+        <JobTemplateDetail
-      job_tags: 'cookies,pizza',
+          template={{
-      job_type: 'run',
+            ...mockTemplate,
-      inventory: 1,
+            diff_mode: true,
-      limit: '1',
+            host_config_key: 'key',
-      project: 7,
+            summary_fields: { user_capabilities: { edit: false } },
-      playbook: '',
+          }}
-      id: 1,
+        />
-      verbosity: 0,
-      created_by: 'Alex',
-      skip_tags: 'coffe,tea',
-      summary_fields: {
-        user_capabilities: { edit: false },
-        created_by: { id: 1, username: 'Joe' },
-        modified_by: { id: 1, username: 'Joe' },
-        inventory: { name: 'Inventory' },
-        project: { name: 'Project' },
-        labels: { count: 1, results: [{ name: 'Label', id: 1 }] },
-      },
-      created: '2020-04-25T01:23:45.678901Z',
-      modified: '2020-04-25T01:23:45.678901Z',
-    };
-    const wrapper = mountWithContexts(
-      <JobTemplateDetail template={regularUser} />
       );
-    const jobTemplateDetail = wrapper.find('JobTemplateDetail');
-    const editButton = jobTemplateDetail.find('button[aria-label="Edit"]');
-
-    jobTemplateDetail.setState({
-      instanceGroups: mockInstanceGroups,
-      hasContentLoading: false,
-      contentError: false,
     });
-    expect(editButton.length).toBe(0);
+    expect(wrapper.find('button[aria-label="Edit"]').length).toBe(0);
-    done();
   });
 
-  test('should render CredentialChip', () => {
+  test('should render credential chips', () => {
-    template.summary_fields.credentials = [{ id: 1, name: 'cred', kind: null }];
+    const chips = wrapper.find('CredentialChip');
-    const wrapper = mountWithContexts(
+    expect(chips).toHaveLength(2);
-      <JobTemplateDetail template={template} />
+    chips.forEach((chip, id) => {
-    );
-    wrapper.find('JobTemplateDetail').setState({
-      instanceGroups: mockInstanceGroups,
-      hasContentLoading: false,
-      contentError: false,
-    });
-
-    const chip = wrapper.find('CredentialChip');
-    expect(chip).toHaveLength(1);
       expect(chip.prop('credential')).toEqual(
-      template.summary_fields.credentials[0]
+        mockTemplate.summary_fields.credentials[id]
       );
     });
-  test('should render SCM_Branch', async () => {
+  });
-    const mockTemplate = { ...template };
-    mockTemplate.scm_branch = 'Foo branch';
 
-    const wrapper = mountWithContexts(
+  test('should render SCM_Branch', async () => {
-      <JobTemplateDetail template={mockTemplate} />
-    );
-    await waitForElement(
-      wrapper,
-      'JobTemplateDetail',
-      el => el.state('hasContentLoading') === false
-    );
     const SCMBranch = wrapper.find('Detail[label="SCM Branch"]');
     expect(SCMBranch.prop('value')).toBe('Foo branch');
   });
+
+  test('should show content error for failed instance group fetch', async () => {
+    JobTemplatesAPI.readInstanceGroups.mockImplementationOnce(() =>
+      Promise.reject(new Error())
+    );
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <JobTemplateDetail
+          template={{
+            ...mockTemplate,
+            allow_simultaneous: true,
+            ask_inventory_on_launch: true,
+            summary_fields: {
+              inventory: {
+                kind: 'smart',
+              },
+            },
+          }}
+        />
+      );
+    });
+    await waitForElement(wrapper, 'ContentError', el => el.length === 1);
+  });
+
+  test('expected api calls are made for delete', async () => {
+    await act(async () => {
+      wrapper.find('DeleteButton').invoke('onConfirm')();
+    });
+    expect(JobTemplatesAPI.destroy).toHaveBeenCalledTimes(1);
+  });
+
+  test('Error dialog shown for failed deletion', async () => {
+    JobTemplatesAPI.destroy.mockImplementationOnce(() =>
+      Promise.reject(new Error())
+    );
+    await act(async () => {
+      wrapper.find('DeleteButton').invoke('onConfirm')();
+    });
+    await waitForElement(
+      wrapper,
+      'Modal[title="Error!"]',
+      el => el.length === 1
+    );
+    await act(async () => {
+      wrapper.find('Modal[title="Error!"]').invoke('onClose')();
+    });
+    await waitForElement(
+      wrapper,
+      'Modal[title="Error!"]',
+      el => el.length === 0
+    );
+  });
 });

awx/ui_next/src/screens/Template/JobTemplateDetail/index.js

@@ -1,4 +1 @@
-import JobTemplateDetail from './JobTemplateDetail';
+export { default } from './JobTemplateDetail';
-
-export { JobTemplateDetail as _JobTemplateDetail };
-export default JobTemplateDetail;

awx/ui_next/src/screens/Template/shared/data.job_template.json

@@ -101,8 +101,13 @@
           "copy": true
       },
       "labels": {
-          "count": 0,
+        "count": 1,
-          "results": []
+        "results": [
+            {
+                "id": 91,
+                "name": "L_91o2"
+            }
+        ]
     },
       "survey": {
           "title": "",
@@ -117,7 +122,14 @@
           }
       ],
       "extra_credentials": [],
-      "credentials": []
+      "credentials": [
+        {
+          "id": 1, "kind": "ssh" , "name": "Credential 1"
+        },
+        {
+          "id": 2, "kind": "awx" , "name": "Credential 2"
+        }
+      ]
   },
   "created": "2019-09-30T16:18:34.564820Z",
   "modified": "2019-10-01T14:47:31.818431Z",
@@ -127,17 +139,17 @@
   "inventory": 1,
   "project": 6,
   "playbook": "ping.yml",
-  "scm_branch": "",
+  "scm_branch": "Foo branch",
   "forks": 0,
   "limit": "",
   "verbosity": 0,
   "extra_vars": "",
-  "job_tags": "",
+  "job_tags": "T_100,T_200",
   "force_handlers": false,
-  "skip_tags": "",
+  "skip_tags": "S_100,S_200",
   "start_at_task": "",
   "timeout": 0,
-  "use_fact_cache": false,
+  "use_fact_cache": true,
   "last_job_run": "2019-10-01T14:34:35.142483Z",
   "last_job_failed": false,
   "next_job_run": null,