From 12b87fca8cdfce2047d01d25b85562330dc89373 Mon Sep 17 00:00:00 2001 From: Vadiem Janssens Date: Thu, 18 Jun 2020 17:05:18 +0200 Subject: [PATCH 1/5] Use consistent layout for subforms in job/template forms Signed-off-by: Vadiem Janssens --- awx/ui_next/README.md | 2 +- .../Template/shared/JobTemplateForm.jsx | 71 ++++++++++----- .../shared/WorkflowJobTemplateForm.jsx | 86 +++++++++++-------- 3 files changed, 101 insertions(+), 58 deletions(-) diff --git a/awx/ui_next/README.md b/awx/ui_next/README.md index d473bcf79c..3d554915ec 100644 --- a/awx/ui_next/README.md +++ b/awx/ui_next/README.md @@ -8,7 +8,7 @@ The API development server will need to be running. See [CONTRIBUTING.md](../../ ```shell # Start the ui development server. While running, the ui will be reachable -# at https://127.0.01:3001 and updated automatically when code changes. +# at https://127.0.0.1:3001 and updated automatically when code changes. npm --prefix=awx/ui_next start ``` diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index ca5ad56073..8df4dc1875 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -9,6 +9,7 @@ import { Switch, Checkbox, TextInput, + Title, } from '@patternfly/react-core'; import ContentError from '../../../components/ContentError'; import ContentLoading from '../../../components/ContentLoading'; @@ -27,6 +28,7 @@ import { FormColumnLayout, FormFullWidthLayout, FormCheckboxLayout, + SubFormLayout, } from '../../../components/FormLayout'; import { VariablesField } from '../../../components/CodeMirrorInput'; import { required } from '../../../util/validators'; @@ -547,30 +549,53 @@ function JobTemplateForm({ - - {allowCallbacks && ( + + {(allowCallbacks || enableWebhooks) && ( <> - {callbackUrl && ( - - - - )} - + + {allowCallbacks && ( + <> + + {i18n._(t`Provisioning Callback details`)} + + + {callbackUrl && ( + + + + )} + + + +
+ + )} + {enableWebhooks && ( + <> + {i18n._(t`Webhook details`)} + + + + + )} +
)} diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx index 8306b849cf..ad6aedd2ce 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx @@ -4,7 +4,13 @@ import PropTypes, { shape } from 'prop-types'; import { withI18n } from '@lingui/react'; import { useField, withFormik } from 'formik'; -import { Form, FormGroup, Checkbox, TextInput } from '@patternfly/react-core'; +import { + Form, + FormGroup, + Checkbox, + TextInput, + Title, +} from '@patternfly/react-core'; import { required } from '../../../util/validators'; import FieldWithPrompt from '../../../components/FieldWithPrompt'; @@ -16,6 +22,7 @@ import { FormColumnLayout, FormFullWidthLayout, FormCheckboxLayout, + SubFormLayout, } from '../../../components/FormLayout'; import OrganizationLookup from '../../../components/Lookup/OrganizationLookup'; import { InventoryLookup } from '../../../components/Lookup'; @@ -175,39 +182,50 @@ function WorkflowJobTemplateForm({ )} /> - - - {i18n._(t`Enable Webhook`)} -   - - - } - id="wfjt-enabled-webhooks" - isChecked={enableWebhooks} - onChange={checked => { - setEnableWebhooks(checked); - }} - /> - - - + + + + {i18n._(t`Enable Webhook`)} +   + + + } + id="wfjt-enabled-webhooks" + isChecked={enableWebhooks} + onChange={checked => { + setEnableWebhooks(checked); + }} + /> + + + + + {enableWebhooks && ( + <> + + {i18n._(t`Webhook details`)} + + + + )} + {submitError && } From c9cfaf65a07ab2c5b316a91c84dbdf6ef2fbe96c Mon Sep 17 00:00:00 2001 From: Vadiem Janssens Date: Fri, 19 Jun 2020 09:47:54 +0200 Subject: [PATCH 2/5] Add headingLevels to Title, minor improvements Signed-off-by: Vadiem Janssens --- .../screens/Template/shared/JobTemplateForm.jsx | 9 +++++---- .../Template/shared/WorkflowJobTemplateForm.jsx | 16 +++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 8df4dc1875..98773323be 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -555,7 +555,7 @@ function JobTemplateForm({ {allowCallbacks && ( <> - + <Title size="md" headingLevel="h4"> {i18n._(t`Provisioning Callback details`)} @@ -580,13 +580,14 @@ function JobTemplateForm({ } /> - -
)} + + {allowCallbacks && enableWebhooks && (
)} + {enableWebhooks && ( <> - {i18n._(t`Webhook details`)} + {i18n._(t`Webhook details`)} {enableWebhooks && ( - <> - - {i18n._(t`Webhook details`)} - - - + + {i18n._(t`Webhook details`)} + + )} {submitError && } From be336277558461c6578d7c36a6f497c8e1d6aa74 Mon Sep 17 00:00:00 2001 From: Vadiem Janssens Date: Mon, 22 Jun 2020 11:25:50 +0200 Subject: [PATCH 3/5] Remove webhook_key and webhook_url from JT form payload Signed-off-by: Vadiem Janssens --- .../src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx | 2 ++ .../screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx | 2 -- .../src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx | 2 ++ .../screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index a0a594feca..e09a4b98f2 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -16,6 +16,8 @@ function JobTemplateAdd() { initialInstanceGroups, credentials, webhook_credential, + webhook_key, + webhook_url, ...remainingValues } = values; 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 6da1ac5b17..4bcae68e9d 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -172,9 +172,7 @@ describe('', () => { playbook: 'Baz', inventory: 2, webhook_credential: undefined, - webhook_key: '', webhook_service: '', - webhook_url: '', }); }); diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 8c8cb0f4b8..1e48523d85 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -101,6 +101,8 @@ class JobTemplateEdit extends Component { initialInstanceGroups, credentials, webhook_credential, + webhook_key, + webhook_url, ...remainingValues } = values; 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 c575ba3f9d..61b1467bcf 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -271,7 +271,8 @@ describe('', () => { delete expected.id; delete expected.type; delete expected.related; - expected.webhook_url = `${window.location.origin}${mockJobTemplate.related.webhook_receiver}`; + delete expected.webhook_key; + delete expected.webhook_url; expect(JobTemplatesAPI.update).toHaveBeenCalledWith(1, expected); expect(JobTemplatesAPI.disassociateLabel).toHaveBeenCalledTimes(2); expect(JobTemplatesAPI.associateLabel).toHaveBeenCalledTimes(4); From f9039703ee582386f61336c57a1f0a66778681a8 Mon Sep 17 00:00:00 2001 From: Vadiem Janssens Date: Mon, 22 Jun 2020 12:09:12 +0200 Subject: [PATCH 4/5] Remove enableWebhooks conditional in WebhookSubForm Signed-off-by: Vadiem Janssens --- .../Template/shared/JobTemplateForm.jsx | 11 +- .../Template/shared/WebhookSubForm.jsx | 210 ++++++++---------- .../Template/shared/WebhookSubForm.test.jsx | 9 +- .../shared/WorkflowJobTemplateForm.jsx | 9 +- 4 files changed, 109 insertions(+), 130 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 98773323be..4fa38d2fd6 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -583,16 +583,15 @@ function JobTemplateForm({ )} - {allowCallbacks && enableWebhooks && (
)} + {allowCallbacks && enableWebhooks &&
} {enableWebhooks && ( <> - {i18n._(t`Webhook details`)} + + {i18n._(t`Webhook details`)} + - + )} diff --git a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx index 94adbeb600..b6ced55dd6 100644 --- a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.jsx @@ -23,7 +23,7 @@ import { CredentialTypesAPI, } from '../../../api'; -function WebhookSubForm({ i18n, enableWebhooks, templateType }) { +function WebhookSubForm({ i18n, templateType }) { const { id } = useParams(); const { pathname } = useLocation(); @@ -36,6 +36,7 @@ function WebhookSubForm({ i18n, enableWebhooks, templateType }) { webhookServiceHelpers, ] = useField('webhook_service'); + // eslint-disable-next-line no-unused-vars const [webhookUrlField, webhookUrlMeta, webhookUrlHelpers] = useField( 'webhook_url' ); @@ -71,21 +72,6 @@ function WebhookSubForm({ i18n, enableWebhooks, templateType }) { loadCredentialType(); }, [loadCredentialType]); - useEffect(() => { - if (enableWebhooks) { - webhookServiceHelpers.setValue(webhookServiceMeta.initialValue); - webhookUrlHelpers.setValue(webhookUrlMeta.initialValue); - webhookKeyHelpers.setValue(webhookKeyMeta.initialValue); - webhookCredentialHelpers.setValue(webhookCredentialMeta.initialValue); - } else { - webhookServiceHelpers.setValue(''); - webhookUrlHelpers.setValue(''); - webhookKeyHelpers.setValue(''); - webhookCredentialHelpers.setValue(null); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [enableWebhooks]); - const { request: fetchWebhookKey, error: webhookKeyError } = useRequest( useCallback(async () => { const updateWebhookKey = @@ -134,108 +120,106 @@ function WebhookSubForm({ i18n, enableWebhooks, templateType }) { return ; } return ( - enableWebhooks && ( - - - - { - webhookServiceHelpers.setValue(val); - webhookUrlHelpers.setValue( - pathname.endsWith('/add') - ? i18n - ._(t`a new webhook url will be generated on save.`) - .toUpperCase() - : `${origin}/api/v2/${templateType}s/${id}/${val}/` - ); - if (val === webhookServiceMeta.initialValue || val === '') { - webhookKeyHelpers.setValue(webhookKeyMeta.initialValue); - webhookCredentialHelpers.setValue( - webhookCredentialMeta.initialValue - ); - } else { - webhookKeyHelpers.setValue( - i18n - ._(t`a new webhook key will be generated on save.`) + + + + { + webhookServiceHelpers.setValue(val); + webhookUrlHelpers.setValue( + pathname.endsWith('/add') + ? i18n + ._(t`a new webhook url will be generated on save.`) .toUpperCase() - ); - webhookCredentialHelpers.setValue(null); - } - }} + : `${origin}/api/v2/${templateType}s/${id}/${val}/` + ); + if (val === webhookServiceMeta.initialValue || val === '') { + webhookKeyHelpers.setValue(webhookKeyMeta.initialValue); + webhookCredentialHelpers.setValue( + webhookCredentialMeta.initialValue + ); + } else { + webhookKeyHelpers.setValue( + i18n + ._(t`a new webhook key will be generated on save.`) + .toUpperCase() + ); + webhookCredentialHelpers.setValue(null); + } + }} + /> + + <> + + + - <> - - - - - - - - - - - - - - {credTypeId && ( - + { - webhookCredentialHelpers.setValue(value || null); - }} - isValid={!webhookCredentialMeta.error} - helperTextInvalid={webhookCredentialMeta.error} - value={webhookCredentialField.value} /> - )} - - ) + + + + + + + + {credTypeId && ( + { + webhookCredentialHelpers.setValue(value || null); + }} + isValid={!webhookCredentialMeta.error} + helperTextInvalid={webhookCredentialMeta.error} + value={webhookCredentialField.value} + /> + )} + ); } export default withI18n()(WebhookSubForm); diff --git a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx index 9b9896d694..cb5f18dacb 100644 --- a/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/WebhookSubForm.test.jsx @@ -34,7 +34,7 @@ describe('', () => { wrapper = mountWithContexts( - + , { @@ -103,7 +103,7 @@ describe('', () => { webhook_key: 'A NEW WEBHOOK KEY WILL BE GENERATED ON SAVE.', }} > - + , { @@ -133,10 +133,7 @@ describe('', () => { newWrapper = mountWithContexts( - + , { diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx index f7dfeb3a16..ece98a5e20 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.jsx @@ -216,11 +216,10 @@ function WorkflowJobTemplateForm({ {enableWebhooks && ( - {i18n._(t`Webhook details`)} - + + {i18n._(t`Webhook details`)} + + )} From 3f33f1c97ddb4a6261d40ca6fdf7c6d8c73b784d Mon Sep 17 00:00:00 2001 From: Vadiem Janssens Date: Thu, 25 Jun 2020 09:51:31 +0200 Subject: [PATCH 5/5] Fix failing test Signed-off-by: Vadiem Janssens --- .../screens/Template/shared/WorkflowJobTemplateForm.test.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx index f572c320c4..f0edee3bbf 100644 --- a/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/WorkflowJobTemplateForm.test.jsx @@ -44,7 +44,9 @@ describe('', () => { related: { webhook_receiver: '/api/v2/workflow_job_templates/57/gitlab/', }, + webhook_credential: null, webhook_key: 'sdfghjklmnbvcdsew435678iokjhgfd', + webhook_service: 'github', }; beforeEach(async () => {