From d77040a7a945a0558ce939d5f2950b597ad97154 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Sun, 3 Nov 2019 21:28:10 -0500 Subject: [PATCH 1/3] Add Project Edit form and refactor how the form handles credentials --- .../Project/ProjectEdit/ProjectEdit.jsx | 66 +++++++++- .../screens/Project/shared/ProjectForm.jsx | 123 +++++++++++++----- .../shared/ProjectSubForms/GitSubForm.jsx | 8 +- .../shared/ProjectSubForms/HgSubForm.jsx | 8 +- .../ProjectSubForms/InsightsSubForm.jsx | 17 +-- .../shared/ProjectSubForms/SharedFields.jsx | 15 +-- .../shared/ProjectSubForms/SvnSubForm.jsx | 8 +- 7 files changed, 171 insertions(+), 74 deletions(-) diff --git a/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.jsx b/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.jsx index 1127e5e8ba..9e8f9f67c4 100644 --- a/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.jsx +++ b/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.jsx @@ -1,10 +1,62 @@ -import React, { Component } from 'react'; -import { CardBody } from '@patternfly/react-core'; +import React, { useState } from 'react'; +import { withRouter } from 'react-router-dom'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import styled from 'styled-components'; +import { + Card as _Card, + CardBody, + CardHeader, + Tooltip, +} from '@patternfly/react-core'; +import CardCloseButton from '@components/CardCloseButton'; +import ProjectForm from '../shared/ProjectForm'; +import { ProjectsAPI } from '@api'; -class ProjectEdit extends Component { - render() { - return Coming soon :); - } +const Card = styled(_Card)` + --pf-c-card--child--PaddingLeft: 0; + --pf-c-card--child--PaddingRight: 0; +`; + +function ProjectEdit({ project, history, i18n }) { + const [formSubmitError, setFormSubmitError] = useState(null); + + const handleSubmit = async values => { + try { + const { + data: { id }, + } = await ProjectsAPI.update(project.id, values); + history.push(`/projects/${id}/details`); + } catch (error) { + setFormSubmitError(error); + } + }; + + const handleCancel = () => { + history.push(`/projects/${project.id}/details`); + }; + + return ( + + + + + + + + + + {formSubmitError ? ( +
formSubmitError
+ ) : ( + '' + )} +
+ ); } -export default ProjectEdit; +export default withI18n()(withRouter(ProjectEdit)); diff --git a/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx index ea1e784848..94126eb1c7 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx @@ -30,19 +30,19 @@ const ScmTypeFormRow = styled(FormRow)` padding: 24px; `; -function ProjectForm(props) { - const { project, handleCancel, handleSubmit, i18n } = props; +function ProjectForm({ project, ...props }) { + const { i18n, handleCancel, handleSubmit } = props; + const { summary_fields = {} } = project; const [contentError, setContentError] = useState(null); const [isLoading, setIsLoading] = useState(true); - const [organization, setOrganization] = useState(null); + const [organization, setOrganization] = useState( + summary_fields.organization || null + ); + const [scmSubFormState, setScmSubFormState] = useState(null); const [scmTypeOptions, setScmTypeOptions] = useState(null); - const [scmCredential, setScmCredential] = useState({ - typeId: null, - value: null, - }); - const [insightsCredential, setInsightsCredential] = useState({ - typeId: null, - value: null, + const [credentials, setCredentials] = useState({ + scm: { typeId: null, value: null }, + insights: { typeId: null, value: null }, }); useEffect(() => { @@ -74,9 +74,32 @@ function ProjectForm(props) { ProjectsAPI.readOptions(), ]); - setScmCredential({ typeId: scmCredentialType.id }); - setInsightsCredential({ typeId: insightsCredentialType.id }); setScmTypeOptions(choices); + + const { credential } = summary_fields; + if (!credential) { + setCredentials({ + scm: { typeId: scmCredentialType.id }, + insights: { typeId: insightsCredentialType.id }, + }); + return; + } + + const { credential_type_id } = credential; + setCredentials({ + scm: { + typeId: scmCredentialType.id, + value: + credential_type_id === scmCredentialType.id ? credential : null, + }, + insights: { + typeId: insightsCredentialType.id, + value: + credential_type_id === insightsCredentialType.id + ? credential + : null, + }, + }); } catch (error) { setContentError(error); } finally { @@ -87,22 +110,50 @@ function ProjectForm(props) { fetchData(); }, []); - const resetScmTypeFields = form => { - const scmFormFields = [ - 'scm_url', - 'scm_branch', - 'scm_refspec', - 'credential', - 'scm_clean', - 'scm_delete_on_update', - 'scm_update_on_launch', - 'allow_override', - 'scm_update_cache_timeout', - ]; + const scmFormFields = { + scm_url: '', + scm_branch: '', + scm_refspec: '', + credential: '', + scm_clean: false, + scm_delete_on_update: false, + scm_update_on_launch: false, + allow_override: false, + scm_update_cache_timeout: 0, + }; - scmFormFields.forEach(field => { - form.setFieldValue(field, form.initialValues[field]); - form.setFieldTouched(field, false); + const saveSubFormState = form => { + const updatedScmFormFields = { ...scmFormFields }; + + Object.keys(updatedScmFormFields).forEach(label => { + updatedScmFormFields[label] = form.values[label]; + }); + + setScmSubFormState(updatedScmFormFields); + }; + + const resetScmTypeFields = (value, form) => { + if (form.values.scm_type === form.initialValues.scm_type) { + saveSubFormState(form); + } + + Object.keys(scmFormFields).forEach(label => { + if (value === form.initialValues.scm_type) { + form.setFieldValue(label, scmSubFormState[label]); + } else { + form.setFieldValue(label, scmFormFields[label]); + } + form.setFieldTouched(label, false); + }); + }; + + const handleCredentialSelection = (type, value) => { + setCredentials({ + ...credentials, + [type]: { + ...credentials[type], + value, + }, }); }; @@ -213,7 +264,7 @@ function ProjectForm(props) { ]} onChange={(event, value) => { form.setFieldValue('scm_type', value); - resetScmTypeFields(form); + resetScmTypeFields(value, form); }} /> @@ -226,29 +277,29 @@ function ProjectForm(props) { { git: ( ), hg: ( ), svn: ( ), insights: ( ), diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/GitSubForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/GitSubForm.jsx index 6bcbb83c48..3b5929b508 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/GitSubForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/GitSubForm.jsx @@ -11,8 +11,8 @@ import { const GitSubForm = ({ i18n, - scmCredential, - setScmCredential, + credential, + onCredentialSelection, scmUpdateOnLaunch, }) => ( <> @@ -74,8 +74,8 @@ const GitSubForm = ({ } /> diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/HgSubForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/HgSubForm.jsx index 6cc642686e..3bce5bbbd8 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/HgSubForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/HgSubForm.jsx @@ -10,8 +10,8 @@ import { const HgSubForm = ({ i18n, - scmCredential, - setScmCredential, + credential, + onCredentialSelection, scmUpdateOnLaunch, }) => ( <> @@ -34,8 +34,8 @@ const HgSubForm = ({ /> diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/InsightsSubForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/InsightsSubForm.jsx index 4e854c8b20..d0193076ff 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/InsightsSubForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/InsightsSubForm.jsx @@ -8,8 +8,8 @@ import { ScmTypeOptions } from './SharedFields'; const InsightsSubForm = ({ i18n, - setInsightsCredential, - insightsCredential, + credential, + onCredentialSelection, scmUpdateOnLaunch, }) => ( <> @@ -18,19 +18,16 @@ const InsightsSubForm = ({ validate={required(i18n._(t`Select a value for this field`), i18n)} render={({ form }) => ( form.setFieldTouched('credential')} - onChange={credential => { - form.setFieldValue('credential', credential.id); - setInsightsCredential({ - ...insightsCredential, - value: credential, - }); + onChange={value => { + onCredentialSelection('insights', value); + form.setFieldValue('credential', value.id); }} - value={insightsCredential.value} + value={credential.value} required /> )} diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx index 40b1dce826..127ba54936 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx @@ -41,20 +41,17 @@ export const BranchFormField = withI18n()(({ i18n, label }) => ( )); export const ScmCredentialFormField = withI18n()( - ({ i18n, setScmCredential, scmCredential }) => ( + ({ i18n, credential, onCredentialSelection }) => ( ( { - form.setFieldValue('credential', credential.id); - setScmCredential({ - ...scmCredential, - value: credential, - }); + value={credential.value} + onChange={value => { + onCredentialSelection('scm', value); + form.setFieldValue('credential', value.id); }} /> )} diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SvnSubForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SvnSubForm.jsx index 9f7e9a4659..2b63c13f96 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SvnSubForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SvnSubForm.jsx @@ -10,8 +10,8 @@ import { const SvnSubForm = ({ i18n, - scmCredential, - setScmCredential, + credential, + onCredentialSelection, scmUpdateOnLaunch, }) => ( <> @@ -30,8 +30,8 @@ const SvnSubForm = ({ /> From ecf340f7227c60f3263686582411ae10138f48c2 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Sun, 3 Nov 2019 21:28:51 -0500 Subject: [PATCH 2/3] Add Project Edit test coverage --- .../Project/ProjectAdd/ProjectAdd.test.jsx | 6 +- .../Project/ProjectEdit/ProjectEdit.test.jsx | 153 ++++++++++++++++++ .../screens/Project/shared/ProjectForm.jsx | 15 +- .../Project/shared/ProjectForm.test.jsx | 7 + 4 files changed, 176 insertions(+), 5 deletions(-) create mode 100644 awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx diff --git a/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx b/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx index 2b9857bc89..3bd0469dea 100644 --- a/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx +++ b/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx @@ -108,7 +108,11 @@ describe('', () => { ); }); await changeState; - wrapper.find('form').simulate('submit'); + await act(async () => { + wrapper.find('form').simulate('submit'); + }); + wrapper.update(); + expect(ProjectsAPI.create).toHaveBeenCalledTimes(1); }); test('handleSubmit should throw an error', async () => { diff --git a/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx b/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx new file mode 100644 index 0000000000..99975a807b --- /dev/null +++ b/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx @@ -0,0 +1,153 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { createMemoryHistory } from 'history'; +import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; +import ProjectEdit from './ProjectEdit'; +import { ProjectsAPI, CredentialTypesAPI } from '@api'; + +jest.mock('@api'); + +describe('', () => { + let wrapper; + const projectData = { + id: 123, + name: 'foo', + description: 'bar', + scm_type: 'git', + scm_url: 'https://foo.bar', + scm_clean: true, + credential: 100, + organization: 2, + scm_update_on_launch: true, + scm_update_cache_timeout: 3, + allow_override: false, + custom_virtualenv: '/venv/custom-env', + summary_fields: { + credential: { + id: 100, + credential_type_id: 5, + kind: 'insights', + }, + }, + }; + + const projectOptionsResolve = { + data: { + actions: { + GET: { + scm_type: { + choices: [ + ['', 'Manual'], + ['git', 'Git'], + ['hg', 'Mercurial'], + ['svn', 'Subversion'], + ['insights', 'Red Hat Insights'], + ], + }, + }, + }, + }, + }; + + const scmCredentialResolve = { + data: { + results: [ + { + id: 4, + name: 'Source Control', + kind: 'scm', + }, + ], + }, + }; + + const insightsCredentialResolve = { + data: { + results: [ + { + id: 5, + name: 'Insights', + kind: 'insights', + }, + ], + }, + }; + + beforeEach(async () => { + await ProjectsAPI.readOptions.mockImplementation( + () => projectOptionsResolve + ); + await CredentialTypesAPI.read.mockImplementationOnce( + () => scmCredentialResolve + ); + await CredentialTypesAPI.read.mockImplementationOnce( + () => insightsCredentialResolve + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('initially renders successfully', async () => { + await act(async () => { + wrapper = mountWithContexts(); + }); + expect(wrapper.length).toBe(1); + }); + + test('handleSubmit should post to the api', async () => { + const history = createMemoryHistory(); + ProjectsAPI.update.mockResolvedValueOnce({ + data: { ...projectData }, + }); + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + await act(async () => { + wrapper.find('form').simulate('submit'); + }); + wrapper.update(); + expect(ProjectsAPI.update).toHaveBeenCalledTimes(1); + }); + + test('handleSubmit should throw an error', async () => { + ProjectsAPI.update.mockImplementation(() => Promise.reject(new Error())); + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + await act(async () => { + wrapper.find('form').simulate('submit'); + }); + wrapper.update(); + expect(ProjectsAPI.update).toHaveBeenCalledTimes(1); + expect(wrapper.find('ProjectEdit .formSubmitError').length).toBe(1); + }); + + test('CardHeader close button should navigate to project details', async () => { + const history = createMemoryHistory(); + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); + }); + wrapper.find('CardCloseButton').simulate('click'); + expect(history.location.pathname).toEqual('/projects/123/details'); + }); + + test('CardBody cancel button should navigate to project details', async () => { + const history = createMemoryHistory(); + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); + }); + await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); + wrapper.find('ProjectEdit button[aria-label="Cancel"]').simulate('click'); + expect(history.location.pathname).toEqual('/projects/123/details'); + }); +}); diff --git a/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx index 94126eb1c7..046e90fa27 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx @@ -122,16 +122,23 @@ function ProjectForm({ project, ...props }) { scm_update_cache_timeout: 0, }; + /* Save current scm subform field values to state */ const saveSubFormState = form => { - const updatedScmFormFields = { ...scmFormFields }; + const currentScmFormFields = { ...scmFormFields }; - Object.keys(updatedScmFormFields).forEach(label => { - updatedScmFormFields[label] = form.values[label]; + Object.keys(currentScmFormFields).forEach(label => { + currentScmFormFields[label] = form.values[label]; }); - setScmSubFormState(updatedScmFormFields); + setScmSubFormState(currentScmFormFields); }; + /** + * If scm type is !== the initial scm type value, + * reset scm subform field values to defaults. + * If scm type is === the initial scm type value, + * reset scm subform field values to scmSubFormState. + */ const resetScmTypeFields = (value, form) => { if (form.values.scm_type === form.initialValues.scm_type) { saveSubFormState(form); diff --git a/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx b/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx index 18b4c5f680..550f60f7c4 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx @@ -21,6 +21,13 @@ describe('', () => { scm_update_cache_timeout: 3, allow_override: false, custom_virtualenv: '/venv/custom-env', + summary_fields: { + credential: { + id: 100, + credential_type_id: 4, + kind: 'scm', + }, + }, }; const projectOptionsResolve = { From 0f32161df0889b64ea67bf32677abcf687ae5b07 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 5 Nov 2019 11:41:01 -0500 Subject: [PATCH 3/3] Pull credential api request outside of ProjectEdit --- .../screens/Project/shared/ProjectForm.jsx | 101 +++++++++--------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx b/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx index 046e90fa27..e1757f4002 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectForm.jsx @@ -1,3 +1,4 @@ +/* eslint no-nested-ternary: 0 */ import React, { useState, useEffect } from 'react'; import PropTypes from 'prop-types'; import { withI18n } from '@lingui/react'; @@ -30,6 +31,44 @@ const ScmTypeFormRow = styled(FormRow)` padding: 24px; `; +const fetchCredentials = async credential => { + const [ + { + data: { + results: [scmCredentialType], + }, + }, + { + data: { + results: [insightsCredentialType], + }, + }, + ] = await Promise.all([ + CredentialTypesAPI.read({ kind: 'scm' }), + CredentialTypesAPI.read({ name: 'Insights' }), + ]); + + if (!credential) { + return { + scm: { typeId: scmCredentialType.id }, + insights: { typeId: insightsCredentialType.id }, + }; + } + + const { credential_type_id } = credential; + return { + scm: { + typeId: scmCredentialType.id, + value: credential_type_id === scmCredentialType.id ? credential : null, + }, + insights: { + typeId: insightsCredentialType.id, + value: + credential_type_id === insightsCredentialType.id ? credential : null, + }, + }; +}; + function ProjectForm({ project, ...props }) { const { i18n, handleCancel, handleSubmit } = props; const { summary_fields = {} } = project; @@ -48,58 +87,19 @@ function ProjectForm({ project, ...props }) { useEffect(() => { async function fetchData() { try { - const [ - { - data: { - results: [scmCredentialType], - }, - }, - { - data: { - results: [insightsCredentialType], - }, - }, - { - data: { - actions: { - GET: { - scm_type: { choices }, - }, + const credentialResponse = fetchCredentials(summary_fields.credential); + const { + data: { + actions: { + GET: { + scm_type: { choices }, }, }, }, - ] = await Promise.all([ - CredentialTypesAPI.read({ kind: 'scm' }), - CredentialTypesAPI.read({ name: 'Insights' }), - ProjectsAPI.readOptions(), - ]); + } = await ProjectsAPI.readOptions(); + setCredentials(await credentialResponse); setScmTypeOptions(choices); - - const { credential } = summary_fields; - if (!credential) { - setCredentials({ - scm: { typeId: scmCredentialType.id }, - insights: { typeId: insightsCredentialType.id }, - }); - return; - } - - const { credential_type_id } = credential; - setCredentials({ - scm: { - typeId: scmCredentialType.id, - value: - credential_type_id === scmCredentialType.id ? credential : null, - }, - insights: { - typeId: insightsCredentialType.id, - value: - credential_type_id === insightsCredentialType.id - ? credential - : null, - }, - }); } catch (error) { setContentError(error); } finally { @@ -185,7 +185,12 @@ function ProjectForm({ project, ...props }) { scm_clean: project.scm_clean || false, scm_delete_on_update: project.scm_delete_on_update || false, scm_refspec: project.scm_refspec || '', - scm_type: project.scm_type || '', + scm_type: + project.scm_type === '' + ? 'manual' + : project.scm_type === undefined + ? '' + : project.scm_type, scm_update_cache_timeout: project.scm_update_cache_timeout || 0, scm_update_on_launch: project.scm_update_on_launch || false, scm_url: project.scm_url || '',