From 935c7a5328cb3a891d578fd2ced9446b788ad9f4 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 1 Oct 2020 17:03:23 -0400 Subject: [PATCH 1/2] refactors useSteps to allow each step to handle errors --- .../components/LaunchPrompt/LaunchPrompt.jsx | 133 ++++++++++++------ .../LaunchPrompt/LaunchPrompt.test.jsx | 4 +- .../LaunchPrompt/steps/OtherPromptsStep.jsx | 5 + .../LaunchPrompt/steps/SurveyStep.jsx | 18 ++- .../LaunchPrompt/steps/useCredentialsStep.jsx | 22 +-- .../LaunchPrompt/steps/useInventoryStep.jsx | 56 +++----- .../steps/useOtherPromptsStep.jsx | 57 +------- .../LaunchPrompt/steps/usePreviewStep.jsx | 28 +++- .../LaunchPrompt/steps/useSurveyStep.jsx | 70 +++++---- .../components/LaunchPrompt/useLaunchSteps.js | 69 +++++++++ .../src/components/LaunchPrompt/useSteps.js | 68 --------- 11 files changed, 272 insertions(+), 258 deletions(-) create mode 100644 awx/ui_next/src/components/LaunchPrompt/useLaunchSteps.js delete mode 100644 awx/ui_next/src/components/LaunchPrompt/useSteps.js diff --git a/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx b/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx index 6eb9ec0ecc..e1fc3bbdad 100644 --- a/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx @@ -2,32 +2,27 @@ import React from 'react'; import { Wizard } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { Formik } from 'formik'; +import { Formik, useFormikContext } from 'formik'; import ContentError from '../ContentError'; import ContentLoading from '../ContentLoading'; +import { useDismissableError } from '../../util/useRequest'; import mergeExtraVars from './mergeExtraVars'; -import useSteps from './useSteps'; +import useLaunchSteps from './useLaunchSteps'; +import AlertModal from '../AlertModal'; import getSurveyValues from './getSurveyValues'; -function LaunchPrompt({ config, resource, onLaunch, onCancel, i18n }) { +function PromptModalForm({ onSubmit, onCancel, i18n, config, resource }) { + const { values, setTouched, validateForm } = useFormikContext(); + const { steps, - initialValues, isReady, - validate, visitStep, visitAllSteps, contentError, - } = useSteps(config, resource, i18n); + } = useLaunchSteps(config, resource, i18n); - if (contentError) { - return ; - } - if (!isReady) { - return ; - } - - const submit = values => { + const handleSave = () => { const postValues = {}; const setValue = (key, value) => { if (typeof value !== 'undefined' && value !== null) { @@ -49,39 +44,89 @@ function LaunchPrompt({ config, resource, onLaunch, onCancel, i18n }) { : resource.extra_vars; setValue('extra_vars', mergeExtraVars(extraVars, surveyValues)); setValue('scm_branch', values.scm_branch); - onLaunch(postValues); + + onSubmit(postValues); }; + const { error, dismissError } = useDismissableError(contentError); + + if (error) { + return ( + { + dismissError(); + }} + > + + + ); + } return ( - - {({ validateForm, setTouched, handleSubmit }) => ( - { - if (nextStep.id === 'preview') { - visitAllSteps(setTouched); - } else { - visitStep(prevStep.prevId); - } - await validateForm(); - }} - onGoToStep={async (newStep, prevStep) => { - if (newStep.id === 'preview') { - visitAllSteps(setTouched); - } else { - visitStep(prevStep.prevId); - } - await validateForm(); - }} - title={i18n._(t`Prompts`)} - steps={steps} - backButtonText={i18n._(t`Back`)} - cancelButtonText={i18n._(t`Cancel`)} - nextButtonText={i18n._(t`Next`)} - /> - )} + { + if (nextStep.id === 'preview') { + visitAllSteps(setTouched); + } else { + visitStep(prevStep.prevId); + } + await validateForm(); + }} + onGoToStep={async (nextStep, prevStep) => { + if (nextStep.id === 'preview') { + visitAllSteps(setTouched); + } else { + visitStep(prevStep.prevId); + } + await validateForm(); + }} + title={i18n._(t`Prompts`)} + steps={ + isReady + ? steps + : [ + { + name: i18n._(t`Content Loading`), + component: , + }, + ] + } + backButtonText={i18n._(t`Back`)} + cancelButtonText={i18n._(t`Cancel`)} + nextButtonText={i18n._(t`Next`)} + /> + ); +} + +function LaunchPrompt({ config, resource = {}, onLaunch, onCancel, i18n }) { + return ( + onLaunch(values)} + > + onLaunch(values)} + onCancel={onCancel} + i18n={i18n} + config={config} + resource={resource} + /> ); } diff --git a/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.test.jsx b/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.test.jsx index 25f8660585..37abb8c830 100644 --- a/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.test.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.test.jsx @@ -95,7 +95,7 @@ describe('LaunchPrompt', () => { expect(steps).toHaveLength(5); expect(steps[0].name.props.children).toEqual('Inventory'); expect(steps[1].name).toEqual('Credentials'); - expect(steps[2].name.props.children).toEqual('Other Prompts'); + expect(steps[2].name).toEqual('Other Prompts'); expect(steps[3].name.props.children).toEqual('Survey'); expect(steps[4].name).toEqual('Preview'); }); @@ -167,7 +167,7 @@ describe('LaunchPrompt', () => { const steps = wizard.prop('steps'); expect(steps).toHaveLength(2); - expect(steps[0].name.props.children).toEqual('Other Prompts'); + expect(steps[0].name).toEqual('Other Prompts'); expect(isElementOfType(steps[0].component, OtherPromptsStep)).toEqual(true); expect(isElementOfType(steps[1].component, PreviewStep)).toEqual(true); }); diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/OtherPromptsStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/OtherPromptsStep.jsx index 91ec37a68c..ff504646c8 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/OtherPromptsStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/OtherPromptsStep.jsx @@ -51,6 +51,7 @@ function OtherPromptsStep({ config, i18n }) { id="prompt-job-tags" name="job_tags" label={i18n._(t`Job Tags`)} + aria-label={i18n._(t`Job Tags`)} tooltip={i18n._(t`Tags are useful when you have a large playbook, and you want to run a specific part of a play or task. Use commas to separate multiple tags. Refer to Ansible Tower @@ -62,6 +63,7 @@ function OtherPromptsStep({ config, i18n }) { id="prompt-skip-tags" name="skip_tags" label={i18n._(t`Skip Tags`)} + aria-label={i18n._(t`Skip Tags`)} tooltip={i18n._(t`Skip tags are useful when you have a large playbook, and you want to skip specific parts of a play or task. Use commas to separate multiple tags. Refer to Ansible Tower @@ -108,6 +110,7 @@ function JobTypeField({ i18n }) { and report problems without executing the playbook.`)} /> } + isRequired validated={isValid ? 'default' : 'error'} > 0; + const isValid = !meta.touched || (!meta.error && hasActualValue); + return ( { - if (field.value.includes(option)) { + if (field?.value?.includes(option)) { helpers.setValue(field.value.filter(o => o !== option)); } else { helpers.setValue(field.value.concat(option)); diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/useCredentialsStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/useCredentialsStep.jsx index 85299a08c1..b5506bde31 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/useCredentialsStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/useCredentialsStep.jsx @@ -4,20 +4,9 @@ import CredentialsStep from './CredentialsStep'; const STEP_ID = 'credentials'; -export default function useCredentialsStep( - config, - resource, - visitedSteps, - i18n -) { - const validate = () => { - return {}; - }; - +export default function useCredentialsStep(config, i18n) { return { step: getStep(config, i18n), - initialValues: getInitialValues(config, resource), - validate, isReady: true, contentError: null, formError: null, @@ -39,12 +28,3 @@ function getStep(config, i18n) { component: , }; } - -function getInitialValues(config, resource) { - if (!config.ask_credential_on_launch) { - return {}; - } - return { - credentials: resource?.summary_fields?.credentials || [], - }; -} diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/useInventoryStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/useInventoryStep.jsx index ebc875d506..4724be21e0 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/useInventoryStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/useInventoryStep.jsx @@ -1,38 +1,19 @@ -import React, { useState } from 'react'; +import React from 'react'; import { t } from '@lingui/macro'; +import { useField } from 'formik'; import InventoryStep from './InventoryStep'; import StepName from './StepName'; const STEP_ID = 'inventory'; -export default function useInventoryStep(config, resource, visitedSteps, i18n) { - const [stepErrors, setStepErrors] = useState({}); - - const validate = values => { - if ( - !config.ask_inventory_on_launch || - (['workflow_job', 'workflow_job_template'].includes(resource.type) && - !resource.inventory) - ) { - return {}; - } - const errors = {}; - if (!values.inventory) { - errors.inventory = i18n._(t`An inventory must be selected`); - } - setStepErrors(errors); - return errors; - }; - - const hasErrors = visitedSteps[STEP_ID] && Object.keys(stepErrors).length > 0; +export default function useInventoryStep(config, visitedSteps, i18n) { + const [, meta] = useField('inventory'); return { - step: getStep(config, hasErrors, i18n), - initialValues: getInitialValues(config, resource), - validate, + step: getStep(config, meta, i18n, visitedSteps), isReady: true, contentError: null, - formError: stepErrors, + formError: !meta.value, setTouched: setFieldsTouched => { setFieldsTouched({ inventory: true, @@ -40,23 +21,24 @@ export default function useInventoryStep(config, resource, visitedSteps, i18n) { }, }; } - -function getStep(config, hasErrors, i18n) { +function getStep(config, meta, i18n, visitedSteps) { if (!config.ask_inventory_on_launch) { return null; } return { id: STEP_ID, - name: {i18n._(t`Inventory`)}, + key: 3, + name: ( + + {i18n._(t`Inventory`)} + + ), component: , - }; -} - -function getInitialValues(config, resource) { - if (!config.ask_inventory_on_launch) { - return {}; - } - return { - inventory: resource?.summary_fields?.inventory || null, + enableNext: true, }; } diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/useOtherPromptsStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/useOtherPromptsStep.jsx index 1d33987c92..c03565b358 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/useOtherPromptsStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/useOtherPromptsStep.jsx @@ -1,31 +1,15 @@ -import React, { useState } from 'react'; +import React from 'react'; import { t } from '@lingui/macro'; import OtherPromptsStep from './OtherPromptsStep'; -import StepName from './StepName'; const STEP_ID = 'other'; -export default function useOtherPrompt(config, resource, visitedSteps, i18n) { - const [stepErrors, setStepErrors] = useState({}); - - const validate = values => { - const errors = {}; - if (config.ask_job_type_on_launch && !values.job_type) { - errors.job_type = i18n._(t`This field must not be blank`); - } - setStepErrors(errors); - return errors; - }; - - const hasErrors = visitedSteps[STEP_ID] && Object.keys(stepErrors).length > 0; - +export default function useOtherPrompt(config, i18n) { return { - step: getStep(config, hasErrors, i18n), - initialValues: getInitialValues(config, resource), - validate, + step: getStep(config, i18n), isReady: true, contentError: null, - formError: stepErrors, + formError: null, setTouched: setFieldsTouched => { setFieldsTouched({ job_type: true, @@ -40,13 +24,13 @@ export default function useOtherPrompt(config, resource, visitedSteps, i18n) { }; } -function getStep(config, hasErrors, i18n) { +function getStep(config, i18n) { if (!shouldShowPrompt(config)) { return null; } return { id: STEP_ID, - name: {i18n._(t`Other Prompts`)}, + name: i18n._(t`Other Prompts`), component: , }; } @@ -63,32 +47,3 @@ function shouldShowPrompt(config) { config.ask_diff_mode_on_launch ); } - -function getInitialValues(config, resource) { - const initialValues = {}; - if (config.ask_job_type_on_launch) { - initialValues.job_type = resource.job_type || ''; - } - if (config.ask_limit_on_launch) { - initialValues.limit = resource.limit || ''; - } - if (config.ask_verbosity_on_launch) { - initialValues.verbosity = resource.verbosity || 0; - } - if (config.ask_tags_on_launch) { - initialValues.job_tags = resource.job_tags || ''; - } - if (config.ask_skip_tags_on_launch) { - initialValues.skip_tags = resource.skip_tags || ''; - } - if (config.ask_variables_on_launch) { - initialValues.extra_vars = resource.extra_vars || '---'; - } - if (config.ask_scm_branch_on_launch) { - initialValues.scm_branch = resource.scm_branch || ''; - } - if (config.ask_diff_mode_on_launch) { - initialValues.diff_mode = resource.diff_mode || false; - } - return initialValues; -} diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/usePreviewStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/usePreviewStep.jsx index a7ae2c61d1..e033f7eb93 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/usePreviewStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/usePreviewStep.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { useFormikContext } from 'formik'; import { t } from '@lingui/macro'; import PreviewStep from './PreviewStep'; @@ -8,9 +9,29 @@ export default function usePreviewStep( config, resource, survey, - formErrors, + hasErrors, i18n ) { + const { values: formikValues, errors } = useFormikContext(); + + const formErrorsContent = []; + if (config.ask_inventory_on_launch && !formikValues.inventory) { + formErrorsContent.push({ + inventory: true, + }); + } + const hasSurveyError = Object.keys(errors).find(e => e.includes('survey')); + if ( + config.survey_enabled && + (config.variables_needed_to_start || + config.variables_needed_to_start.length === 0) && + hasSurveyError + ) { + formErrorsContent.push({ + survey: true, + }); + } + return { step: { id: STEP_ID, @@ -20,14 +41,13 @@ export default function usePreviewStep( config={config} resource={resource} survey={survey} - formErrors={formErrors} + formErrors={hasErrors} /> ), - enableNext: Object.keys(formErrors).length === 0, + enableNext: !hasErrors, nextButtonText: i18n._(t`Launch`), }, initialValues: {}, - validate: () => ({}), isReady: true, error: null, setTouched: () => {}, diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx index ac0fbe0c3c..7c150117e0 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx @@ -1,5 +1,6 @@ -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useEffect, useCallback } from 'react'; import { t } from '@lingui/macro'; +import { useFormikContext } from 'formik'; import useRequest from '../../../util/useRequest'; import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '../../../api'; import SurveyStep from './SurveyStep'; @@ -7,27 +8,27 @@ import StepName from './StepName'; const STEP_ID = 'survey'; -export default function useSurveyStep(config, resource, visitedSteps, i18n) { - const [stepErrors, setStepErrors] = useState({}); - +export default function useSurveyStep(config, visitedSteps, i18n) { + const { values } = useFormikContext(); const { result: survey, request: fetchSurvey, isLoading, error } = useRequest( useCallback(async () => { if (!config.survey_enabled) { return {}; } - const { data } = - resource.type === 'workflow_job_template' - ? await WorkflowJobTemplatesAPI.readSurvey(resource.id) - : await JobTemplatesAPI.readSurvey(resource.id); + const { data } = config?.workflow_job_template_data + ? await WorkflowJobTemplatesAPI.readSurvey( + config?.workflow_job_template_data?.id + ) + : await JobTemplatesAPI.readSurvey(config?.job_template_data?.id); return data; - }, [config.survey_enabled, resource]) + }, [config]) ); useEffect(() => { fetchSurvey(); }, [fetchSurvey]); - const validate = values => { + const validate = () => { if (!config.survey_enabled || !survey || !survey.spec) { return {}; } @@ -42,20 +43,22 @@ export default function useSurveyStep(config, resource, visitedSteps, i18n) { errors[`survey_${question.variable}`] = errMessage; } }); - setStepErrors(errors); return errors; }; - - const hasErrors = visitedSteps[STEP_ID] && Object.keys(stepErrors).length > 0; - + const formError = validate(); return { - step: getStep(config, survey, hasErrors, i18n), + step: getStep( + config, + survey, + Object.keys(formError).length > 0, + i18n, + visitedSteps + ), + formError, initialValues: getInitialValues(config, survey), - validate, survey, isReady: !isLoading && !!survey, contentError: error, - formError: stepErrors, setTouched: setFieldsTouched => { if (!survey || !survey.spec) { return; @@ -87,34 +90,49 @@ function validateField(question, value, i18n) { ); } } - if (question.required && !value && value !== 0) { + if ( + question.required && + ((!value && value !== 0) || (Array.isArray(value) && value.length === 0)) + ) { return i18n._(t`This field must not be blank`); } return null; } - -function getStep(config, survey, hasErrors, i18n) { +function getStep(config, survey, hasErrors, i18n, visitedSteps) { if (!config.survey_enabled) { return null; } return { id: STEP_ID, - name: {i18n._(t`Survey`)}, + key: 6, + name: ( + + {i18n._(t`Survey`)} + + ), component: , + enableNext: true, }; } - function getInitialValues(config, survey) { if (!config.survey_enabled || !survey) { return {}; } - const values = {}; + const surveyValues = {}; survey.spec.forEach(question => { if (question.type === 'multiselect') { - values[`survey_${question.variable}`] = question.default.split('\n'); + if (question.default === '') { + surveyValues[`survey_${question.variable}`] = []; + } else { + surveyValues[`survey_${question.variable}`] = question.default.split( + '\n' + ); + } } else { - values[`survey_${question.variable}`] = question.default; + surveyValues[`survey_${question.variable}`] = question.default; } }); - return values; + return surveyValues; } diff --git a/awx/ui_next/src/components/LaunchPrompt/useLaunchSteps.js b/awx/ui_next/src/components/LaunchPrompt/useLaunchSteps.js new file mode 100644 index 0000000000..caa32fe8ba --- /dev/null +++ b/awx/ui_next/src/components/LaunchPrompt/useLaunchSteps.js @@ -0,0 +1,69 @@ +import { useState, useEffect } from 'react'; +import { useFormikContext } from 'formik'; +import useInventoryStep from './steps/useInventoryStep'; +import useCredentialsStep from './steps/useCredentialsStep'; +import useOtherPromptsStep from './steps/useOtherPromptsStep'; +import useSurveyStep from './steps/useSurveyStep'; +import usePreviewStep from './steps/usePreviewStep'; + +export default function useLaunchSteps(config, resource, i18n) { + const [visited, setVisited] = useState({}); + const steps = [ + useInventoryStep(config, visited, i18n), + useCredentialsStep(config, i18n), + useOtherPromptsStep(config, i18n), + useSurveyStep(config, visited, i18n), + ]; + const { resetForm, values: formikValues } = useFormikContext(); + const hasErrors = steps.some(step => step.formError); + + const surveyStepIndex = steps.findIndex(step => step.survey); + steps.push( + usePreviewStep( + config, + resource, + steps[surveyStepIndex]?.survey, + hasErrors, + i18n + ) + ); + + const pfSteps = steps.map(s => s.step).filter(s => s != null); + const isReady = !steps.some(s => !s.isReady); + + useEffect(() => { + if (surveyStepIndex > -1 && isReady) { + resetForm({ + values: { + ...formikValues, + ...steps[surveyStepIndex].initialValues, + }, + }); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isReady]); + + const stepWithError = steps.find(s => s.contentError); + const contentError = stepWithError ? stepWithError.contentError : null; + + return { + steps: pfSteps, + isReady, + visitStep: stepId => + setVisited({ + ...visited, + [stepId]: true, + }), + visitAllSteps: setFieldsTouched => { + setVisited({ + inventory: true, + credentials: true, + other: true, + survey: true, + preview: true, + }); + steps.forEach(s => s.setTouched(setFieldsTouched)); + }, + contentError, + }; +} diff --git a/awx/ui_next/src/components/LaunchPrompt/useSteps.js b/awx/ui_next/src/components/LaunchPrompt/useSteps.js deleted file mode 100644 index e8519c58a4..0000000000 --- a/awx/ui_next/src/components/LaunchPrompt/useSteps.js +++ /dev/null @@ -1,68 +0,0 @@ -import { useState } from 'react'; -import useInventoryStep from './steps/useInventoryStep'; -import useCredentialsStep from './steps/useCredentialsStep'; -import useOtherPromptsStep from './steps/useOtherPromptsStep'; -import useSurveyStep from './steps/useSurveyStep'; -import usePreviewStep from './steps/usePreviewStep'; - -export default function useSteps(config, resource, i18n) { - const [visited, setVisited] = useState({}); - const steps = [ - useInventoryStep(config, resource, visited, i18n), - useCredentialsStep(config, resource, visited, i18n), - useOtherPromptsStep(config, resource, visited, i18n), - useSurveyStep(config, resource, visited, i18n), - ]; - - const formErrorsContent = steps - .filter(s => s?.formError && Object.keys(s.formError).length > 0) - .map(({ formError }) => formError); - - steps.push( - usePreviewStep(config, resource, steps[3].survey, formErrorsContent, i18n) - ); - - const pfSteps = steps.map(s => s.step).filter(s => s != null); - const initialValues = steps.reduce((acc, cur) => { - return { - ...acc, - ...cur.initialValues, - }; - }, {}); - const isReady = !steps.some(s => !s.isReady); - - const stepWithError = steps.find(s => s.contentError); - const contentError = stepWithError ? stepWithError.contentError : null; - - const validate = values => { - const errors = steps.reduce((acc, cur) => { - return { - ...acc, - ...cur.validate(values), - }; - }, {}); - if (Object.keys(errors).length) { - return errors; - } - return false; - }; - - return { - steps: pfSteps, - initialValues, - isReady, - validate, - visitStep: stepId => setVisited({ ...visited, [stepId]: true }), - visitAllSteps: setFieldsTouched => { - setVisited({ - inventory: true, - credentials: true, - other: true, - survey: true, - preview: true, - }); - steps.forEach(s => s.setTouched(setFieldsTouched)); - }, - contentError, - }; -} From b1a1c821697fe39be274d7f9cb62e13d36934da7 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Mon, 5 Oct 2020 17:13:22 -0400 Subject: [PATCH 2/2] fixes unresponsive clear all on survey step --- .../LaunchPrompt/getSurveyValues.js | 5 +++- .../LaunchPrompt/steps/PreviewStep.jsx | 2 +- .../LaunchPrompt/steps/PreviewStep.test.jsx | 27 +++++++++++++++++++ .../LaunchPrompt/steps/SurveyStep.jsx | 5 ++++ .../LaunchPrompt/steps/useSurveyStep.jsx | 10 ++----- 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/awx/ui_next/src/components/LaunchPrompt/getSurveyValues.js b/awx/ui_next/src/components/LaunchPrompt/getSurveyValues.js index 0559eefc1f..17b9fd1aed 100644 --- a/awx/ui_next/src/components/LaunchPrompt/getSurveyValues.js +++ b/awx/ui_next/src/components/LaunchPrompt/getSurveyValues.js @@ -1,7 +1,10 @@ export default function getSurveyValues(values) { const surveyValues = {}; Object.keys(values).forEach(key => { - if (key.startsWith('survey_')) { + if (key.startsWith('survey_') && values[key] !== []) { + if (Array.isArray(values[key]) && values[key].length === 0) { + return; + } surveyValues[key.substr(7)] = values[key]; } }); diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/PreviewStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/PreviewStep.jsx index 14e26f4d10..bc94fea258 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/PreviewStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/PreviewStep.jsx @@ -48,7 +48,7 @@ function PreviewStep({ resource, config, survey, formErrors, i18n }) { return ( - {formErrors.length > 0 && ( + {formErrors && ( {i18n._(t`Some of the previous step(s) have errors`)} { extra_vars: 'one: 1', }); }); + + test('should remove survey with empty array value', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + + const detail = wrapper.find('PromptDetail'); + expect(detail).toHaveLength(1); + expect(detail.prop('resource')).toEqual(resource); + expect(detail.prop('overrides')).toEqual({ + extra_vars: 'one: 1', + }); + }); }); diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/SurveyStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/SurveyStep.jsx index 1d5906de5d..d5f4a42df4 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/SurveyStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/SurveyStep.jsx @@ -146,9 +146,14 @@ function MultiSelectField({ question, i18n }) { } else { helpers.setValue(field.value.concat(option)); } + helpers.setTouched(true); }} isOpen={isOpen} selections={field.value} + onClear={() => { + helpers.setTouched(true); + helpers.setValue([]); + }} > {question.choices.split('\n').map(opt => ( diff --git a/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx b/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx index 7c150117e0..7137536f09 100644 --- a/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/steps/useSurveyStep.jsx @@ -45,15 +45,9 @@ export default function useSurveyStep(config, visitedSteps, i18n) { }); return errors; }; - const formError = validate(); + const formError = Object.keys(validate()).length > 0; return { - step: getStep( - config, - survey, - Object.keys(formError).length > 0, - i18n, - visitedSteps - ), + step: getStep(config, survey, formError, i18n, visitedSteps), formError, initialValues: getInitialValues(config, survey), survey,