diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.js b/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.js index 1f201b60e8..05dccc7b42 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.js +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCommands.test.js @@ -184,14 +184,6 @@ describe('', () => { ); wrapper.update(); - expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true); - - expect( - wrapper - .find('WizardNavItem[content="Machine credential"]') - .prop('isDisabled') - ).toBe(true); - await act(async () => { wrapper.find('AnsibleSelect[name="module_name"]').prop('onChange')( {}, @@ -247,6 +239,12 @@ describe('', () => { wrapper.find('CheckboxListItem[label="Cred 4"]').prop('isSelected') ).toBe(true); + await act(async () => + wrapper.find('Button[type="submit"]').prop('onClick')() + ); + wrapper.update(); + + // fourth step await act(async () => wrapper.find('Button[type="submit"]').prop('onClick')() ); @@ -353,13 +351,6 @@ describe('', () => { ); wrapper.update(); - expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true); - expect( - wrapper - .find('WizardNavItem[content="Machine credential"]') - .prop('isDisabled') - ).toBe(true); - await act(async () => { wrapper.find('AnsibleSelect[name="module_name"]').prop('onChange')( {}, @@ -423,7 +414,13 @@ describe('', () => { await act(async () => wrapper.find('Button[type="submit"]').prop('onClick')() ); + wrapper.update(); + // fourth step of wizard + + await act(async () => + wrapper.find('Button[type="submit"]').prop('onClick')() + ); await waitForElement(wrapper, 'ErrorDetail', el => el.length > 0); }); diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.js b/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.js index 66154f0a40..18bce7c97a 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.js +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.js @@ -1,26 +1,10 @@ -import React, { useState } from 'react'; - +import React from 'react'; import { t } from '@lingui/macro'; -import { ExclamationCircleIcon as PFExclamationCircleIcon } from '@patternfly/react-icons'; -import { Tooltip } from '@patternfly/react-core'; import { withFormik, useFormikContext } from 'formik'; import PropTypes from 'prop-types'; -import styled from 'styled-components'; import Wizard from '../Wizard'; -import AdHocCredentialStep from './AdHocCredentialStep'; -import AdHocDetailsStep from './AdHocDetailsStep'; -import AdHocExecutionEnvironmentStep from './AdHocExecutionEnvironmentStep'; - -const AlertText = styled.div` - color: var(--pf-global--danger-color--200); - font-weight: var(--pf-global--FontWeight--bold); -`; - -const ExclamationCircleIcon = styled(PFExclamationCircleIcon)` - margin-left: 10px; - color: var(--pf-global--danger-color--100); -`; +import useAdHocLaunchSteps from './useAdHocLaunchSteps'; function AdHocCommandsWizard({ onLaunch, @@ -30,100 +14,44 @@ function AdHocCommandsWizard({ credentialTypeId, organizationId, }) { - const [currentStepId, setCurrentStepId] = useState(1); - const [enableLaunch, setEnableLaunch] = useState(false); + const { setFieldTouched, values } = useFormikContext(); - const { values, errors, touched } = useFormikContext(); - - const enabledNextOnDetailsStep = () => { - if (!values.module_name) { - return false; - } - - if (values.module_name === 'shell' || values.module_name === 'command') { - if (values.module_args) { - return true; - // eslint-disable-next-line no-else-return - } else { - return false; - } - } - return undefined; // makes the linter happy; - }; - const hasDetailsStepError = errors.module_args && touched.module_args; - - const steps = [ - { - id: 1, - key: 1, - name: hasDetailsStepError ? ( - - {t`Details`} - - - - - ) : ( - t`Details` - ), - component: ( - - ), - enableNext: enabledNextOnDetailsStep(), - nextButtonText: t`Next`, - }, - { - id: 2, - key: 2, - name: t`Execution Environment`, - component: ( - - ), - // Removed this line when https://github.com/patternfly/patternfly-react/issues/5729 is fixed - stepNavItemProps: { style: { whiteSpace: 'nowrap' } }, - enableNext: true, - nextButtonText: t`Next`, - canJumpTo: currentStepId >= 2, - }, - { - id: 3, - key: 3, - name: t`Machine credential`, - component: ( - setEnableLaunch(true)} - /> - ), - enableNext: enableLaunch && Object.values(errors).length === 0, - nextButtonText: t`Launch`, - canJumpTo: currentStepId >= 2, - }, - ]; - - const currentStep = steps.find(step => step.id === currentStepId); + const { steps, validateStep, visitStep, visitAllSteps } = useAdHocLaunchSteps( + moduleOptions, + verbosityOptions, + organizationId, + credentialTypeId + ); return ( setCurrentStepId(step.id)} + onNext={(nextStep, prevStep) => { + if (nextStep.id === 'preview') { + visitAllSteps(setFieldTouched); + } else { + visitStep(prevStep.prevId, setFieldTouched); + validateStep(nextStep.id); + } + }} onClose={() => onCloseWizard()} onSave={() => { onLaunch(values); }} + onGoToStep={(nextStep, prevStep) => { + if (nextStep.id === 'preview') { + visitAllSteps(setFieldTouched); + } else { + visitStep(prevStep.prevId, setFieldTouched); + validateStep(nextStep.id); + } + }} steps={steps} title={t`Run command`} - nextButtonText={currentStep.nextButtonText || undefined} backButtonText={t`Back`} cancelButtonText={t`Cancel`} + nextButtonText={t`Next`} /> ); } diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.js b/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.js index c20da81f4f..1ccd1760ec 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.js +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCommandsWizard.test.js @@ -60,50 +60,29 @@ describe('', () => { expect(wrapper.find('AdHocCommandsWizard').length).toBe(1); }); - test('next and nav item should be disabled', async () => { - await waitForElement(wrapper, 'WizardNavItem', el => el.length > 0); - expect( - wrapper.find('WizardNavItem[content="Details"]').prop('isCurrent') - ).toBe(true); - expect( - wrapper.find('WizardNavItem[content="Details"]').prop('isDisabled') - ).toBe(false); - expect( - wrapper - .find('WizardNavItem[content="Machine credential"]') - .prop('isDisabled') - ).toBe(true); - expect( - wrapper - .find('WizardNavItem[content="Machine credential"]') - .prop('isCurrent') - ).toBe(false); - expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true); - }); + test('launch button should be disabled', async () => { + waitForElement(wrapper, 'WizardNavItem', el => el.length > 0); - test('next button should become active, and should navigate to the next step', async () => { - await waitForElement(wrapper, 'WizardNavItem', el => el.length > 0); - - await act(async () => { - wrapper.find('AnsibleSelect[name="module_name"]').prop('onChange')( - {}, - 'command' - ); - wrapper.find('input#module_args').simulate('change', { - target: { value: 'foo', name: 'module_args' }, - }); - wrapper.find('AnsibleSelect[name="verbosity"]').prop('onChange')({}, 1); - }); - wrapper.update(); expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe( false ); - await act(async () => - wrapper.find('Button[type="submit"]').prop('onClick')() + act(() => wrapper.find('Button[type="submit"]').prop('onClick')()); + expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe( + false ); - wrapper.update(); + act(() => wrapper.find('Button[type="submit"]').prop('onClick')()); + expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe( + false + ); + wrapper.update(); + act(() => wrapper.find('Button[type="submit"]').prop('onClick')()); + wrapper.update(); + + expect(wrapper.find('AdHocPreviewStep').prop('hasErrors')).toBe(true); + expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true); }); + test('launch button should become active', async () => { ExecutionEnvironmentsAPI.read.mockResolvedValue({ data: { @@ -184,7 +163,7 @@ describe('', () => { await waitForElement(wrapper, 'OptionsList', el => el.length > 0); expect(wrapper.find('CheckboxListItem').length).toBe(2); - expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true); + await act(async () => { wrapper .find('td#check-action-item-1') @@ -197,13 +176,17 @@ describe('', () => { expect( wrapper.find('CheckboxListItem[label="Cred 1"]').prop('isSelected') ).toBe(true); - expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe( - false - ); await act(async () => wrapper.find('Button[type="submit"]').prop('onClick')() ); + wrapper.update(); + await act(async () => + wrapper.find('Button[type="submit"]').prop('onClick')() + ); + expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe( + false + ); expect(onLaunch).toHaveBeenCalledWith({ become_enabled: '', diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocCredentialStep.js b/awx/ui_next/src/components/AdHocCommands/AdHocCredentialStep.js index 94a8038b54..35709b4c74 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocCredentialStep.js +++ b/awx/ui_next/src/components/AdHocCommands/AdHocCredentialStep.js @@ -1,10 +1,10 @@ import React, { useEffect, useCallback } from 'react'; import { useHistory } from 'react-router-dom'; - import { t } from '@lingui/macro'; +import styled from 'styled-components'; import PropTypes from 'prop-types'; import { useField } from 'formik'; -import { Form, FormGroup } from '@patternfly/react-core'; +import { Form, FormGroup, Alert } from '@patternfly/react-core'; import { CredentialsAPI } from 'api'; import { getQSConfig, parseQueryString, mergeParams } from 'util/qs'; import useRequest from 'hooks/useRequest'; @@ -15,13 +15,17 @@ import ContentError from '../ContentError'; import ContentLoading from '../ContentLoading'; import OptionsList from '../OptionsList'; +const CredentialErrorAlert = styled(Alert)` + margin-bottom: 20px; +`; + const QS_CONFIG = getQSConfig('credentials', { page: 1, page_size: 5, order_by: 'name', }); -function AdHocCredentialStep({ credentialTypeId, onEnableLaunch }) { +function AdHocCredentialStep({ credentialTypeId }) { const history = useHistory(); const { error, @@ -72,10 +76,11 @@ function AdHocCredentialStep({ credentialTypeId, onEnableLaunch }) { fetchCredentials(); }, [fetchCredentials]); - const [credentialField, credentialMeta, credentialHelpers] = useField({ + const [field, meta, helpers] = useField({ name: 'credential', validate: required(null), }); + if (error) { return ; } @@ -83,68 +88,69 @@ function AdHocCredentialStep({ credentialTypeId, onEnableLaunch }) { return ; } return ( -
- + {meta.touched && meta.error && ( + + )} + + + } + > + { + helpers.setValue([value]); + }} + deselectItem={() => { + helpers.setValue([]); + }} + searchableKeys={searchableKeys} + relatedSearchableKeys={relatedSearchableKeys} /> - } - > - { - credentialHelpers.setValue([value]); - onEnableLaunch(); - }} - deselectItem={() => { - credentialHelpers.setValue([]); - }} - /> - - +
+ + ); } AdHocCredentialStep.propTypes = { credentialTypeId: PropTypes.number.isRequired, - onEnableLaunch: PropTypes.func.isRequired, }; export default AdHocCredentialStep; diff --git a/awx/ui_next/src/components/AdHocCommands/AdHocDetailsStep.js b/awx/ui_next/src/components/AdHocCommands/AdHocDetailsStep.js index 0b92e14d84..4d717ab449 100644 --- a/awx/ui_next/src/components/AdHocCommands/AdHocDetailsStep.js +++ b/awx/ui_next/src/components/AdHocCommands/AdHocDetailsStep.js @@ -1,6 +1,5 @@ /* eslint-disable react/no-unescaped-entities */ import React from 'react'; - import { t } from '@lingui/macro'; import PropTypes from 'prop-types'; import { useField } from 'formik'; @@ -41,12 +40,14 @@ function AdHocDetailsStep({ verbosityOptions, moduleOptions }) { const argumentsRequired = moduleNameField.value === 'command' || moduleNameField.value === 'shell'; - const [, argumentsMeta, argumentsHelpers] = useField({ + const [argumentsField, argumentsMeta, argumentsHelpers] = useField({ name: 'module_args', validate: argumentsRequired && required(null), }); - const isValid = !argumentsMeta.error || !argumentsMeta.touched; + const isValid = argumentsRequired + ? (!argumentsMeta.error || !argumentsMeta.touched) && argumentsField.value + : true; return (
@@ -103,10 +104,7 @@ function AdHocDetailsStep({ verbosityOptions, moduleOptions }) { label={t`Arguments`} validated={isValid ? 'default' : 'error'} onBlur={() => argumentsHelpers.setTouched(true)} - isRequired={ - moduleNameField.value === 'command' || - moduleNameField.value === 'shell' - } + isRequired={argumentsRequired} tooltip={ moduleNameField.value ? ( <> @@ -249,7 +247,6 @@ function AdHocDetailsStep({ verbosityOptions, moduleOptions }) { + {hasErrors && ( + + {t`Some of the previous step(s) have errors`} + + + + + )} + + {items.map( + ([key, value]) => + key !== 'extra_vars' && + key !== 'execution_environment' && + key !== 'credential' && ( + + ) + )} + {credential && ( + + )} + {execution_environment && ( + + )} + {extra_vars && ( + + )} + + + ); +} + +export default AdHocPreviewStep; diff --git a/awx/ui_next/src/components/AdHocCommands/useAdHocCredentialStep.jsx b/awx/ui_next/src/components/AdHocCommands/useAdHocCredentialStep.jsx new file mode 100644 index 0000000000..4060dfea41 --- /dev/null +++ b/awx/ui_next/src/components/AdHocCommands/useAdHocCredentialStep.jsx @@ -0,0 +1,41 @@ +import React from 'react'; +import { useField } from 'formik'; +import { t } from '@lingui/macro'; +import StepName from '../LaunchPrompt/steps/StepName'; +import AdHocCredentialStep from './AdHocCredentialStep'; + +const STEP_ID = 'credential'; +export default function useAdHocExecutionEnvironmentStep( + visited, + credentialTypeId +) { + const [field, meta, helpers] = useField('credential'); + const hasError = + Object.keys(visited).includes('credential') && + !field.value.length && + meta.touched; + + return { + step: { + id: STEP_ID, + key: 3, + name: ( + + {t`Credential`} + + ), + component: , + enableNext: true, + nextButtonText: t`Next`, + }, + hasError, + validate: () => { + if (!meta.value.length) { + helpers.setError('A credential must be selected'); + } + }, + setTouched: setFieldTouched => { + setFieldTouched('credential', true, false); + }, + }; +} diff --git a/awx/ui_next/src/components/AdHocCommands/useAdHocDetailsStep.jsx b/awx/ui_next/src/components/AdHocCommands/useAdHocDetailsStep.jsx new file mode 100644 index 0000000000..cc915bedfb --- /dev/null +++ b/awx/ui_next/src/components/AdHocCommands/useAdHocDetailsStep.jsx @@ -0,0 +1,70 @@ +import React from 'react'; +import { t } from '@lingui/macro'; +import { useFormikContext } from 'formik'; +import StepName from '../LaunchPrompt/steps/StepName'; +import AdHocDetailsStep from './AdHocDetailsStep'; + +const STEP_ID = 'details'; +export default function useAdHocDetailsStep( + visited, + moduleOptions, + verbosityOptions +) { + const { values, touched, setFieldError } = useFormikContext(); + + const hasError = () => { + if (!Object.keys(visited).includes(STEP_ID)) { + return false; + } + if (!values.module_name && touched.module_name) { + return true; + } + + if (values.module_name === 'shell' || values.module_name === 'command') { + if (values.module_args) { + return false; + // eslint-disable-next-line no-else-return + } else { + return true; + } + } + return false; + }; + return { + step: { + id: STEP_ID, + key: 1, + name: ( + + {t`Details`} + + ), + component: ( + + ), + enableNext: true, + nextButtonText: t`Next`, + }, + hasError: hasError(), + validate: () => { + if (Object.keys(touched).includes('module_name' || 'module_args')) { + if (!values.module_name) { + setFieldError('module_name', t`This field is must not be blank.`); + } + if ( + values.module_name === ('command' || 'shell') && + !values.module_args + ) { + setFieldError('module_args', t`This field is must not be blank`); + } + } + }, + setTouched: setFieldTouched => { + setFieldTouched('module_name', true, false); + setFieldTouched('module_args', true, false); + }, + }; +} diff --git a/awx/ui_next/src/components/AdHocCommands/useAdHocExecutionEnvironmentStep.jsx b/awx/ui_next/src/components/AdHocCommands/useAdHocExecutionEnvironmentStep.jsx new file mode 100644 index 0000000000..95949bce7d --- /dev/null +++ b/awx/ui_next/src/components/AdHocCommands/useAdHocExecutionEnvironmentStep.jsx @@ -0,0 +1,28 @@ +import React from 'react'; +import { t } from '@lingui/macro'; +import StepName from '../LaunchPrompt/steps/StepName'; +import AdHocExecutionEnvironmentStep from './AdHocExecutionEnvironmentStep'; + +const STEP_ID = 'executionEnvironment'; +export default function useAdHocExecutionEnvironmentStep(organizationId) { + return { + step: { + id: STEP_ID, + key: 2, + stepNavItemProps: { style: { whiteSpace: 'nowrap' } }, + name: ( + + {t`Execution Environment`} + + ), + component: ( + + ), + enableNext: true, + nextButtonText: t`Next`, + }, + hasError: false, + validate: () => {}, + setTouched: () => {}, + }; +} diff --git a/awx/ui_next/src/components/AdHocCommands/useAdHocLaunchSteps.js b/awx/ui_next/src/components/AdHocCommands/useAdHocLaunchSteps.js new file mode 100644 index 0000000000..7ade2695b0 --- /dev/null +++ b/awx/ui_next/src/components/AdHocCommands/useAdHocLaunchSteps.js @@ -0,0 +1,48 @@ +import { useState } from 'react'; +import useAdHocDetailsStep from './useAdHocDetailsStep'; +import useAdHocExecutionEnvironmentStep from './useAdHocExecutionEnvironmentStep'; +import useAdHocCredentialStep from './useAdHocCredentialStep'; +import useAdHocPreviewStep from './useAdHocPreviewStep'; + +export default function useAdHocLaunchSteps( + moduleOptions, + verbosityOptions, + organizationId, + credentialTypeId +) { + const [visited, setVisited] = useState({}); + const steps = [ + useAdHocDetailsStep(visited, moduleOptions, verbosityOptions), + useAdHocExecutionEnvironmentStep(organizationId), + useAdHocCredentialStep(visited, credentialTypeId), + ]; + + const hasErrors = steps.some(step => step.hasError); + + steps.push(useAdHocPreviewStep(hasErrors)); + return { + steps: steps.map(s => s.step).filter(s => s != null), + validateStep: stepId => + steps + .find(s => { + return s?.step.id === stepId; + }) + .validate(), + visitStep: (prevStepId, setFieldTouched) => { + setVisited({ + ...visited, + [prevStepId]: true, + }); + steps.find(s => s?.step?.id === prevStepId).setTouched(setFieldTouched); + }, + visitAllSteps: setFieldTouched => { + setVisited({ + details: true, + executionEnvironment: true, + credential: true, + preview: true, + }); + steps.forEach(s => s.setTouched(setFieldTouched)); + }, + }; +} diff --git a/awx/ui_next/src/components/AdHocCommands/useAdHocPreviewStep.jsx b/awx/ui_next/src/components/AdHocCommands/useAdHocPreviewStep.jsx new file mode 100644 index 0000000000..bc1d9dd171 --- /dev/null +++ b/awx/ui_next/src/components/AdHocCommands/useAdHocPreviewStep.jsx @@ -0,0 +1,28 @@ +import React from 'react'; +import { t } from '@lingui/macro'; +import { useFormikContext } from 'formik'; +import StepName from '../LaunchPrompt/steps/StepName'; +import AdHocPreviewStep from './AdHocPreviewStep'; + +const STEP_ID = 'preview'; +export default function useAdHocPreviewStep(hasErrors) { + const { values } = useFormikContext(); + + return { + step: { + id: STEP_ID, + key: 4, + name: ( + + {t`Preview`} + + ), + component: , + enableNext: !hasErrors, + nextButtonText: t`Launch`, + }, + hasErrors: false, + validate: () => {}, + setTouched: () => {}, + }; +}