From 46a7ca4dc36c0344ff74d4200e9a1d1ee3b321cd Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 14 Jan 2020 12:01:02 -0500 Subject: [PATCH 1/5] Fixes navigation bug in InventoryAdd Adds SCM Branch field on JTForm --- .../Inventory/InventoryAdd/InventoryAdd.jsx | 4 +++- .../Inventory/InventoryAdd/InventoryAdd.test.jsx | 4 ++-- .../JobTemplateDetail/JobTemplateDetail.jsx | 6 ++++++ .../JobTemplateDetail/JobTemplateDetail.test.jsx | 15 +++++++++++++++ .../screens/Template/shared/JobTemplateForm.jsx | 12 ++++++++++++ .../Template/shared/JobTemplateForm.test.jsx | 3 +++ 6 files changed, 41 insertions(+), 3 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx b/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx index db7f5be27c..e01a336390 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx @@ -57,7 +57,9 @@ function InventoryAdd() { ); await Promise.all(associatePromises); } - const url = history.location.pathname.search('smart') + const url = history.location.pathname.startsWith( + '/inventories/smart_inventory' + ) ? `/inventories/smart_inventory/${inventoryId}/details` : `/inventories/inventory/${inventoryId}/details`; diff --git a/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.test.jsx index 1616d17499..26b0619a7d 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.test.jsx @@ -41,8 +41,7 @@ describe('', () => { test('Initially renders successfully', () => { expect(wrapper.length).toBe(1); }); - - test('handleSubmit should call the api', async () => { + test('handleSubmit should call the api and redirect to details page', async () => { const instanceGroups = [{ name: 'Bizz', id: 1 }, { name: 'Buzz', id: 2 }]; await waitForElement(wrapper, 'isLoading', el => el.length === 0); @@ -64,6 +63,7 @@ describe('', () => { IG.id ) ); + expect(history.location.pathname).toBe('/inventories/inventory/13/details'); }); test('handleCancel should return the user back to the inventories list', async () => { diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index 8783e93297..44390be54d 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -196,6 +196,12 @@ class JobTemplateDetail extends Component { ) : ( renderMissingDataDetail(i18n._(t`Project`)) )} + {template.scm_branch && ( + + )} 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 a06d1e8db5..7ae6b0319c 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.test.jsx @@ -143,4 +143,19 @@ describe('', () => { 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'); + }); }); diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index b62f7603ec..74d5687f25 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -124,6 +124,9 @@ class JobTemplateForm extends Component { handleProjectUpdate(project) { const { setFieldValue } = this.props; setFieldValue('project', project.id); + if (!project.allow_override) { + setFieldValue('scm_branch', null); + } this.setState({ project }); } @@ -269,6 +272,14 @@ class JobTemplateForm extends Component { /> )} + {project && project.allow_override && ( + + )} ', () => { project: 3, playbook: 'Baz', type: 'job_template', + scm_branch: 'Foo', summary_fields: { inventory: { id: 2, @@ -26,6 +27,7 @@ describe('', () => { project: { id: 3, name: 'qux', + allow_override: true, }, labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, credentials: [ @@ -145,6 +147,7 @@ describe('', () => { wrapper.find('ProjectLookup').invoke('onChange')({ id: 4, name: 'project', + allow_override: true, }); wrapper.find('AnsibleSelect[name="playbook"]').simulate('change', { target: { value: 'new baz type', name: 'playbook' }, From 2daefcd94e1965e243d2c36ba15ea957b907b31c Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 14 Jan 2020 14:53:38 -0500 Subject: [PATCH 2/5] Removes code from serializer in favor to api call of Project.readDetails Adds necessary tests. --- .../JobTemplateDetail/JobTemplateDetail.jsx | 7 +----- .../JobTemplateEdit/JobTemplateEdit.test.jsx | 5 ++++ .../Template/shared/JobTemplateForm.jsx | 25 +++++++++++++++---- .../Template/shared/JobTemplateForm.test.jsx | 6 ++++- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index 44390be54d..94955e78f5 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -196,12 +196,7 @@ class JobTemplateDetail extends Component { ) : ( renderMissingDataDetail(i18n._(t`Project`)) )} - {template.scm_branch && ( - - )} + diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index e7750e2a87..3299476322 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -161,6 +161,11 @@ describe('', () => { JobTemplatesAPI.readInstanceGroups.mockReturnValue({ data: { results: mockInstanceGroups }, }); + ProjectsAPI.readDetail.mockReturnValue({ + id: 1, + allow_override: true, + name: 'foo', + }); }); afterEach(() => { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 74d5687f25..4d3242d2d0 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -28,7 +28,7 @@ import { ProjectLookup, MultiCredentialsLookup, } from '@components/Lookup'; -import { JobTemplatesAPI } from '@api'; +import { JobTemplatesAPI, ProjectsAPI } from '@api'; import LabelSelect from './LabelSelect'; import PlaybookSelect from './PlaybookSelect'; @@ -81,16 +81,31 @@ class JobTemplateForm extends Component { this.loadRelatedInstanceGroups = this.loadRelatedInstanceGroups.bind(this); this.handleProjectUpdate = this.handleProjectUpdate.bind(this); this.setContentError = this.setContentError.bind(this); + this.fetchProject = this.fetchProject.bind(this); } componentDidMount() { const { validateField } = this.props; this.setState({ contentError: null, hasContentLoading: true }); // TODO: determine when LabelSelect has finished loading labels - Promise.all([this.loadRelatedInstanceGroups()]).then(() => { - this.setState({ hasContentLoading: false }); - validateField('project'); - }); + Promise.all([this.loadRelatedInstanceGroups(), this.fetchProject()]).then( + () => { + this.setState({ hasContentLoading: false }); + validateField('project'); + } + ); + } + + async fetchProject() { + const { project } = this.state; + if (project && project.id) { + try { + const { data } = await ProjectsAPI.readDetail(project.id); + this.setState({ project: data }); + } catch (err) { + this.setState({ contentError: err }); + } + } } async loadRelatedInstanceGroups() { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx index 41d218c01a..7deb756399 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -27,7 +27,6 @@ describe('', () => { project: { id: 3, name: 'qux', - allow_override: true, }, labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, credentials: [ @@ -90,6 +89,11 @@ describe('', () => { ProjectsAPI.readPlaybooks.mockReturnValue({ data: ['debug.yml'], }); + ProjectsAPI.readDetail.mockReturnValue({ + name: 'foo', + id: 1, + allow_override: true, + }); }); afterEach(() => { From fd1e574fcb70a17eba003753c3b0912ae5800dc2 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 15 Jan 2020 13:27:31 -0500 Subject: [PATCH 3/5] Resets playbook and scm-branch fields when project is changed The playbook field becomes undefined and the scm-branch field becomes ''. This ensures that the user has to assign a playbook to the template that is associated with the project and suggests to the user to review their scm-branch. TODO: when the user updates project with scm-branch override allow the user to type in playbook in dropdown. Then, check if playbook is present in list of playbooks. If no, add it to the list of playbooks. --- .../src/screens/Template/shared/JobTemplateForm.jsx | 10 +++++----- .../src/screens/Template/shared/PlaybookSelect.jsx | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 4d3242d2d0..0ff31b887f 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -100,8 +100,8 @@ class JobTemplateForm extends Component { const { project } = this.state; if (project && project.id) { try { - const { data } = await ProjectsAPI.readDetail(project.id); - this.setState({ project: data }); + const { data: projectData } = await ProjectsAPI.readDetail(project.id); + this.setState({ project: projectData }); } catch (err) { this.setState({ contentError: err }); } @@ -139,9 +139,8 @@ class JobTemplateForm extends Component { handleProjectUpdate(project) { const { setFieldValue } = this.props; setFieldValue('project', project.id); - if (!project.allow_override) { - setFieldValue('scm_branch', null); - } + setFieldValue('playbook', undefined); + setFieldValue('scm_branch', ''); this.setState({ project }); } @@ -165,6 +164,7 @@ class JobTemplateForm extends Component { i18n, template, } = this.props; + const jobTypeOptions = [ { value: '', diff --git a/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx index 8f0687ba2a..b0e24fe283 100644 --- a/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx +++ b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx @@ -15,6 +15,7 @@ function PlaybookSelect({ i18n, }) { const [options, setOptions] = useState([]); + useEffect(() => { if (!projectId) { return; @@ -28,6 +29,7 @@ function PlaybookSelect({ label: playbook, isDisabled: false, })); + opts.unshift({ value: '', key: '', @@ -40,7 +42,6 @@ function PlaybookSelect({ } })(); }, [projectId, i18n, onError]); - return ( Date: Tue, 14 Jan 2020 12:01:02 -0500 Subject: [PATCH 4/5] Fixes navigation bug in InventoryAdd Adds SCM Branch field on JTForm --- .../Template/JobTemplateDetail/JobTemplateDetail.jsx | 7 ++++++- .../src/screens/Template/shared/JobTemplateForm.test.jsx | 1 + 2 files changed, 7 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 94955e78f5..44390be54d 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -196,7 +196,12 @@ class JobTemplateDetail extends Component { ) : ( renderMissingDataDetail(i18n._(t`Project`)) )} - + {template.scm_branch && ( + + )} diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx index 7deb756399..ba9bf8a102 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -27,6 +27,7 @@ describe('', () => { project: { id: 3, name: 'qux', + allow_override: true, }, labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, credentials: [ From 078dc666c1ff98499ea1048ee782dc27f3b154b7 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 14 Jan 2020 14:53:38 -0500 Subject: [PATCH 5/5] Removes code from serializer in favor to api call of Project.readDetails Adds necessary tests. --- .../JobTemplateAdd/JobTemplateAdd.test.jsx | 1 + .../JobTemplateDetail/JobTemplateDetail.jsx | 7 +------ .../JobTemplateEdit/JobTemplateEdit.test.jsx | 1 + .../screens/Template/shared/JobTemplateForm.jsx | 2 +- .../Template/shared/JobTemplateForm.test.jsx | 14 ++++++++++++-- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx index adfeb31b37..508b85de82 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -26,6 +26,7 @@ const jobTemplateData = { allow_simultaneous: false, use_fact_cache: false, host_config_key: '', + scm_branch: '', }; describe('', () => { diff --git a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx index 44390be54d..94955e78f5 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateDetail/JobTemplateDetail.jsx @@ -196,12 +196,7 @@ class JobTemplateDetail extends Component { ) : ( renderMissingDataDetail(i18n._(t`Project`)) )} - {template.scm_branch && ( - - )} + diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index 3299476322..42fd375188 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -29,6 +29,7 @@ const mockJobTemplate = { allow_simultaneous: false, use_fact_cache: false, host_config_key: '', + scm_branch: '', summary_fields: { user_capabilities: { edit: true, diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 0ff31b887f..c2f1ca9c4c 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -139,7 +139,7 @@ class JobTemplateForm extends Component { handleProjectUpdate(project) { const { setFieldValue } = this.props; setFieldValue('project', project.id); - setFieldValue('playbook', undefined); + setFieldValue('playbook', 0); setFieldValue('scm_branch', ''); this.setState({ project }); } diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx index ba9bf8a102..95a02c7edb 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -27,7 +27,6 @@ describe('', () => { project: { id: 3, name: 'qux', - allow_override: true, }, labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, credentials: [ @@ -133,7 +132,6 @@ describe('', () => { /> ); }); - await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); await act(async () => { wrapper.find('input#template-name').simulate('change', { @@ -154,15 +152,25 @@ describe('', () => { name: 'project', allow_override: true, }); + }); + wrapper.update(); + await act(async () => { + wrapper.find('input#scm_branch').simulate('change', { + target: { value: 'devel', name: 'scm_branch' }, + }); wrapper.find('AnsibleSelect[name="playbook"]').simulate('change', { target: { value: 'new baz type', name: 'playbook' }, }); + }); + + await act(async () => { wrapper .find('CredentialChip') .at(0) .prop('onClick')(); }); wrapper.update(); + expect(wrapper.find('input#template-name').prop('value')).toEqual( 'new foo' ); @@ -179,7 +187,9 @@ describe('', () => { expect(wrapper.find('ProjectLookup').prop('value')).toEqual({ id: 4, name: 'project', + allow_override: true, }); + expect(wrapper.find('input#scm_branch').prop('value')).toEqual('devel'); expect( wrapper.find('AnsibleSelect[name="playbook"]').prop('value') ).toEqual('new baz type');