From 10131432b57f8dd722d4377903552ba2c633f766 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Thu, 23 Jan 2020 13:52:29 -0500 Subject: [PATCH 1/3] Refactor job template detail into functional component --- .../JobTemplateDetail/JobTemplateDetail.jsx | 567 +++++++++--------- 1 file changed, 269 insertions(+), 298 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index 94955e78f5..ef5f8e32f8 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -1,5 +1,5 @@ -import React, { Component, Fragment } from 'react'; -import { Link, withRouter } from 'react-router-dom'; +import React, { Fragment, useState, useEffect } from 'react'; +import { Link, useParams } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { Button, @@ -13,10 +13,10 @@ import { t } from '@lingui/macro'; 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 LaunchButton from '@components/LaunchButton'; import { JobTemplatesAPI } from '@api'; const MissingDetail = styled(Detail)` @@ -25,319 +25,290 @@ const MissingDetail = styled(Detail)` } `; -class JobTemplateDetail extends Component { - constructor(props) { - super(props); - this.state = { - contentError: null, - hasContentLoading: true, - instanceGroups: [], - }; - this.readInstanceGroups = this.readInstanceGroups.bind(this); - } +function JobTemplateDetail({ i18n, template }) { + const { + ask_inventory_on_launch, + allow_simultaneous, + become_enabled, + created, + description, + diff_mode, + forks, + host_config_key, + job_slice_count, + job_tags, + job_type, + name, + limit, + modified, + playbook, + skip_tags, + timeout, + summary_fields, + use_fact_cache, + url, + verbosity, + } = template; + const [contentError, setContentError] = useState(null); + const [hasContentLoading, setHasContentLoading] = useState(false); + const [instanceGroups, setInstanceGroups] = useState([]); + const { id: templateId } = useParams(); - componentDidMount() { - this.readInstanceGroups(); - } + useEffect(() => { + (async () => { + setContentError(null); + setHasContentLoading(true); + try { + const { + data: { results = [] }, + } = await JobTemplatesAPI.readInstanceGroups(templateId); + setInstanceGroups(results); + } catch (error) { + setContentError(error); + } finally { + setHasContentLoading(false); + } + })(); + }, [templateId]); - 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 }); - } - } + 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)`) }, + { verbosity: 2, details: i18n._(t`2 (More Verbose)`) }, + { verbosity: 3, details: i18n._(t`3 (Debug)`) }, + { verbosity: 4, details: i18n._(t`4 (Connection Debug)`) }, + { verbosity: 5, details: i18n._(t`5 (WinRM Debug)`) }, + ]; + const verbosityDetails = verbosityOptions.filter( + option => option.verbosity === verbosity + ); + const generateCallBackUrl = `${window.location.origin + url}callback/`; + const renderOptionsField = + become_enabled || host_config_key || allow_simultaneous || use_fact_cache; - render() { - const { - template: { - ask_inventory_on_launch, - allow_simultaneous, - become_enabled, - created, - description, - diff_mode, - forks, - host_config_key, - job_slice_count, - job_tags, - job_type, - name, - limit, - modified, - playbook, - skip_tags, - timeout, - summary_fields, - use_fact_cache, - url, - verbosity, - }, - hasTemplateLoading, - template, - i18n, - match, - } = this.props; - const canLaunch = summary_fields.user_capabilities.start; - const { instanceGroups, hasContentLoading, contentError } = this.state; - const verbosityOptions = [ - { verbosity: 0, details: i18n._(t`0 (Normal)`) }, - { verbosity: 1, details: i18n._(t`1 (Verbose)`) }, - { verbosity: 2, details: i18n._(t`2 (More Verbose)`) }, - { verbosity: 3, details: i18n._(t`3 (Debug)`) }, - { verbosity: 4, details: i18n._(t`4 (Connection Debug)`) }, - { verbosity: 5, details: i18n._(t`5 (WinRM Debug)`) }, - ]; - const verbosityDetails = verbosityOptions.filter( - option => option.verbosity === verbosity - ); - const generateCallBackUrl = `${window.location.origin + url}callback/`; - const isInitialized = !hasTemplateLoading && !hasContentLoading; + const renderOptions = ( + + {become_enabled && ( + + {i18n._(t`Enable Privilege Escalation`)} + + )} + {host_config_key && ( + + {i18n._(t`Allow Provisioning Callbacks`)} + + )} + {allow_simultaneous && ( + + {i18n._(t`Enable Concurrent Jobs`)} + + )} + {use_fact_cache && ( + + {i18n._(t`Use Fact Cache`)} + + )} + + ); - const renderOptionsField = - become_enabled || host_config_key || allow_simultaneous || use_fact_cache; + const renderMissingDataDetail = value => ( + + ); - const renderOptions = ( - - {become_enabled && ( - - {i18n._(t`Enable Privilege Escalation`)} - - )} - {host_config_key && ( - - {i18n._(t`Allow Provisioning Callbacks`)} - - )} - {allow_simultaneous && ( - - {i18n._(t`Enable Concurrent Jobs`)} - - )} - {use_fact_cache && ( - - {i18n._(t`Use Fact Cache`)} - - )} - - ); + const inventoryValue = (kind, id) => { + const inventorykind = kind === 'smart' ? 'smart_inventory' : 'inventory'; - const renderMissingDataDetail = value => ( - - ); - - const inventoryValue = (kind, id) => { - const inventorykind = - kind === 'smart' ? (kind = 'smart_inventory') : (kind = 'inventory'); - - return ask_inventory_on_launch ? ( - - - {summary_fields.inventory.name} - - {i18n._(t`(Prompt on Launch)`)} - - ) : ( + return ask_inventory_on_launch ? ( + {summary_fields.inventory.name} - ); - }; + {i18n._(t`(Prompt on Launch)`)} + + ) : ( + + {summary_fields.inventory.name} + + ); + }; - if (contentError) { - return ; - } + if (contentError) { + return ; + } - if (hasContentLoading) { - return ; - } + if (hasContentLoading) { + return ; + } - return ( - isInitialized && ( - - - - - - - {summary_fields.inventory ? ( - - ) : ( - !ask_inventory_on_launch && - renderMissingDataDetail(i18n._(t`Inventory`)) + return ( + + + + + + {summary_fields.inventory ? ( + - {summary_fields.project - ? summary_fields.project.name - : i18n._(t`Deleted`)} - - } - /> - ) : ( - renderMissingDataDetail(i18n._(t`Project`)) - )} - - - - + /> + ) : ( + !ask_inventory_on_launch && + renderMissingDataDetail(i18n._(t`Inventory`)) + )} + {summary_fields.project ? ( + + {summary_fields.project.name} + + } + /> + ) : ( + renderMissingDataDetail(i18n._(t`Project`)) + )} + + + + + + + + + + + {host_config_key && ( + - - - - - {host_config_key && ( - - - - - )} - {renderOptionsField && ( - - )} - {summary_fields.credentials && - summary_fields.credentials.length > 0 && ( - - {summary_fields.credentials.map(c => ( - - ))} - - } - /> - )} - {summary_fields.labels && summary_fields.labels.results.length > 0 && ( - - {summary_fields.labels.results.map(l => ( - - {l.name} - - ))} - - } - /> - )} - {instanceGroups.length > 0 && ( - - {instanceGroups.map(ig => ( - - {ig.name} - - ))} - - } - /> - )} - {job_tags && job_tags.length > 0 && ( - - {job_tags.split(',').map(jobTag => ( - - {jobTag} - - ))} - - } - /> - )} - {skip_tags && skip_tags.length > 0 && ( - - {skip_tags.split(',').map(skipTag => ( - - {skipTag} - - ))} - - } - /> - )} - - - {summary_fields.user_capabilities.edit && ( - + )} + {canLaunch && ( + + {({ handleLaunch }) => ( + )} - {canLaunch && ( - - {({ handleLaunch }) => ( - - )} - - )} - - - ) - ); - } + + )} + + + ); } + export { JobTemplateDetail as _JobTemplateDetail }; -export default withI18n()(withRouter(JobTemplateDetail)); +export default withI18n()(JobTemplateDetail); From 6efa751157e85c048b2b4fe6c23d5f3a7d3264f7 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Thu, 23 Jan 2020 13:58:49 -0500 Subject: [PATCH 2/3] Add DeleteButton component to job template details --- .../JobTemplateDetail/JobTemplateDetail.jsx | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index ef5f8e32f8..6af3b424ed 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -1,5 +1,5 @@ import React, { Fragment, useState, useEffect } from 'react'; -import { Link, useParams } from 'react-router-dom'; +import { Link, useHistory, useParams } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { Button, @@ -11,11 +11,14 @@ 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 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'; @@ -50,9 +53,11 @@ function JobTemplateDetail({ i18n, template }) { verbosity, } = template; const [contentError, setContentError] = useState(null); + const [deletionError, setDeletionError] = useState(null); const [hasContentLoading, setHasContentLoading] = useState(false); const [instanceGroups, setInstanceGroups] = useState([]); const { id: templateId } = useParams(); + const history = useHistory(); useEffect(() => { (async () => { @@ -71,6 +76,17 @@ function JobTemplateDetail({ i18n, template }) { })(); }, [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 = [ @@ -305,7 +321,29 @@ function JobTemplateDetail({ i18n, template }) { )} )} + {summary_fields.user_capabilities && + summary_fields.user_capabilities.delete && ( + + {i18n._(t`Delete`)} + + )} + {/* Update delete modal to show dependencies https://github.com/ansible/awx/issues/5546 */} + {deletionError && ( + setDeletionError(null)} + > + {i18n._(t`Failed to delete job template.`)} + + + )} ); } From afc1f856684b3584e30589af79ec199045478236 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Thu, 23 Jan 2020 13:59:20 -0500 Subject: [PATCH 3/3] Update job template detail unit tests --- .../JobTemplateDetail.test.jsx | 234 ++++++++---------- .../Template/JobTemplateDetail/index.js | 5 +- .../Template/shared/data.job_template.json | 28 ++- 3 files changed, 128 insertions(+), 139 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx index 7ae6b0319c..6be3c3af47 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx @@ -1,161 +1,141 @@ 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'); +const mockInstanceGroups = { + count: 5, + data: { + results: [{ id: 1, name: 'IG1' }, { id: 2, name: 'IG2' }], + }, +}; + describe('', () => { - 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', - }; + let wrapper; - const mockInstanceGroups = { - count: 5, - data: { - results: [{ id: 1, name: 'IG1' }, { id: 2, name: 'IG2' }], - }, - }; - - const readInstanceGroups = jest.spyOn( - _JobTemplateDetail.prototype, - 'readInstanceGroups' - ); - - beforeEach(() => { + beforeEach(async () => { JobTemplatesAPI.readInstanceGroups.mockResolvedValue(mockInstanceGroups); + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); }); afterEach(() => { jest.clearAllMocks(); }); - test('Can load with missing summary fields', async () => { - const mockTemplate = { ...template }; - mockTemplate.summary_fields = { user_capabilities: {} }; - - const wrapper = mountWithContexts( - - ); + test('should render successfully with missing summary fields', async () => { + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + 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 => { - const wrapper = mountWithContexts( - - ); - await waitForElement( - wrapper, - 'JobTemplateDetail', - el => el.state('hasContentLoading') === true - ); - expect(readInstanceGroups).toHaveBeenCalled(); - await waitForElement( - wrapper, - 'JobTemplateDetail', - el => el.state('hasContentLoading') === false - ); + test('should request instance groups from api', async () => { expect(JobTemplatesAPI.readInstanceGroups).toHaveBeenCalledTimes(1); - done(); }); - test('Edit button is absent when user does not have edit privilege', async done => { - const regularUser = { - forks: 1, - host_config_key: 'ssh', - name: 'Temp 1', - job_tags: 'cookies,pizza', - job_type: 'run', - inventory: 1, - limit: '1', - project: 7, - 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( - - ); - const jobTemplateDetail = wrapper.find('JobTemplateDetail'); - const editButton = jobTemplateDetail.find('button[aria-label="Edit"]'); - - jobTemplateDetail.setState({ - instanceGroups: mockInstanceGroups, - hasContentLoading: false, - contentError: false, + test('should hide edit button for users without edit permission', async () => { + JobTemplatesAPI.readInstanceGroups.mockResolvedValue({ data: {} }); + await act(async () => { + wrapper = mountWithContexts( + + ); }); - expect(editButton.length).toBe(0); - done(); + expect(wrapper.find('button[aria-label="Edit"]').length).toBe(0); }); - test('should render CredentialChip', () => { - template.summary_fields.credentials = [{ id: 1, name: 'cred', kind: null }]; - const wrapper = mountWithContexts( - - ); - wrapper.find('JobTemplateDetail').setState({ - instanceGroups: mockInstanceGroups, - hasContentLoading: false, - contentError: false, + test('should render credential chips', () => { + const chips = wrapper.find('CredentialChip'); + expect(chips).toHaveLength(2); + chips.forEach((chip, id) => { + expect(chip.prop('credential')).toEqual( + mockTemplate.summary_fields.credentials[id] + ); }); - - const chip = wrapper.find('CredentialChip'); - expect(chip).toHaveLength(1); - expect(chip.prop('credential')).toEqual( - template.summary_fields.credentials[0] - ); }); + test('should render SCM_Branch', async () => { - const mockTemplate = { ...template }; - mockTemplate.scm_branch = 'Foo branch'; - - const wrapper = mountWithContexts( - - ); - 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( + + ); + }); + 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 + ); + }); }); diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/index.js b/awx/ui_next/src/screens/Template/JobTemplateDetail/index.js index fedf6e28d5..c07a99c058 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/index.js +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/index.js @@ -1,4 +1 @@ -import JobTemplateDetail from './JobTemplateDetail'; - -export { JobTemplateDetail as _JobTemplateDetail }; -export default JobTemplateDetail; +export { default } from './JobTemplateDetail'; diff --git a/awx/ui_next/src/screens/Template/shared/data.job_template.json b/awx/ui_next/src/screens/Template/shared/data.job_template.json index a8f9da56a7..2fc2e460d7 100644 --- a/awx/ui_next/src/screens/Template/shared/data.job_template.json +++ b/awx/ui_next/src/screens/Template/shared/data.job_template.json @@ -101,9 +101,14 @@ "copy": true }, "labels": { - "count": 0, - "results": [] - }, + "count": 1, + "results": [ + { + "id": 91, + "name": "L_91o2" + } + ] + }, "survey": { "title": "", "description": "" @@ -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,