From 8a0be5b111c1c693fb09b5449121efab3380ca0b Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 9 Apr 2020 11:35:56 -0700 Subject: [PATCH 1/4] add survey questions --- .../components/LaunchPrompt/LaunchPrompt.jsx | 2 +- .../components/LaunchPrompt/SurveyStep.jsx | 65 +++++++++++++++++-- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx b/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx index 4dfac93e23..f130e7f2d8 100644 --- a/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/LaunchPrompt.jsx @@ -71,7 +71,7 @@ function LaunchPrompt({ config, resource, onLaunch, onCancel, i18n }) { if (config.survey_enabled) { steps.push({ name: i18n._(t`Survey`), - component: , + component: , }); } steps.push({ diff --git a/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx b/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx index c0b8e6ec14..5d18040037 100644 --- a/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx @@ -1,7 +1,64 @@ -import React from 'react'; +import React, { useCallback, useEffect } from 'react'; +import { withI18n } from '@lingui/react'; +import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '@api'; +import { Form } from '@patternfly/react-core'; +import FormField from '@components/FormField'; +import ContentLoading from '@components/ContentLoading'; +import ContentError from '@components/ContentError'; +import useRequest from '@util/useRequest'; +import { required } from '@util/validators'; -function InventoryStep() { - return
; +function InventoryStep({ template, i18n }) { + const { result: survey, request: fetchSurvey, isLoading, error } = useRequest( + useCallback(async () => { + const { data } = + template.type === 'workflow_job_template' + ? await WorkflowJobTemplatesAPI.readSurvey(template.id) + : await JobTemplatesAPI.readSurvey(template.id); + return data; + }, [template]) + ); + useEffect(() => { + fetchSurvey(); + }, [fetchSurvey]); + + if (error) { + return ; + } + if (isLoading || !survey) { + return ; + } + + return ( +
+ {survey.spec.map(question => ( + + ))} + + ); } -export default InventoryStep; +function SurveyQuestion({ question, i18n }) { + const isNumeric = question.type === 'number' || question.type === 'integer'; + return ( + + ); +} + +export default withI18n()(InventoryStep); From 669d67b8fbb30266791606e7b7a83b21e40d7607 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 15 Apr 2020 09:21:03 -0700 Subject: [PATCH 2/4] flush out validators, survey questions --- .../components/LaunchPrompt/SurveyStep.jsx | 109 +++++++++++++++--- .../components/StatusIcon/StatusIcon.test.jsx | 1 - .../Template/Survey/SurveyQuestionForm.jsx | 1 + awx/ui_next/src/util/validators.jsx | 22 +++- awx/ui_next/src/util/validators.test.js | 52 ++++++++- 5 files changed, 164 insertions(+), 21 deletions(-) diff --git a/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx b/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx index 5d18040037..8b89b3c566 100644 --- a/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx +++ b/awx/ui_next/src/components/LaunchPrompt/SurveyStep.jsx @@ -1,14 +1,23 @@ import React, { useCallback, useEffect } from 'react'; import { withI18n } from '@lingui/react'; +import { useField } from 'formik'; import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '@api'; import { Form } from '@patternfly/react-core'; import FormField from '@components/FormField'; +import AnsibleSelect from '@components/AnsibleSelect'; import ContentLoading from '@components/ContentLoading'; import ContentError from '@components/ContentError'; import useRequest from '@util/useRequest'; -import { required } from '@util/validators'; +import { + required, + minMaxValue, + maxLength, + minLength, + integer, + combine, +} from '@util/validators'; -function InventoryStep({ template, i18n }) { +function SurveyStep({ template, i18n }) { const { result: survey, request: fetchSurvey, isLoading, error } = useRequest( useCallback(async () => { const { data } = @@ -29,21 +38,33 @@ function InventoryStep({ template, i18n }) { return ; } + const fieldTypes = { + text: TextField, + textarea: TextField, + password: TextField, + multiplechoice: MultipleChoiceField, + multiselect: MultiSelectField, + integer: NumberField, + float: NumberField, + }; return (
- {survey.spec.map(question => ( - - ))} + {survey.spec.map(question => { + const Field = fieldTypes[question.type]; + return ( + + ); + })} ); } -function SurveyQuestion({ question, i18n }) { - const isNumeric = question.type === 'number' || question.type === 'integer'; +function TextField({ question, i18n }) { + const validators = [ + question.required ? required(null, i18n) : null, + question.min ? minLength(question.min, i18n) : null, + question.max ? maxLength(question.max, i18n) : null, + ]; return ( ); } -export default withI18n()(InventoryStep); +function NumberField({ question, i18n }) { + const validators = [ + question.required ? required(null, i18n) : null, + minMaxValue(question.min, question.max, i18n), + question.type === 'integer' ? integer(i18n) : null, + ]; + return ( + + ); +} + +function MultipleChoiceField({ question, i18n }) { + const [field, meta] = useField(question.question_name); + console.log(question, field); + return ( + ({ + key: opt, + value: opt, + label: opt, + }))} + /> + ); +} + +function MultiSelectField({ question, i18n }) { + const [field, meta] = useField(question.question_name); + return ( + ({ + key: opt, + value: opt, + label: opt, + }))} + /> + ); +} + +export default withI18n()(SurveyStep); diff --git a/awx/ui_next/src/components/StatusIcon/StatusIcon.test.jsx b/awx/ui_next/src/components/StatusIcon/StatusIcon.test.jsx index fd47a309c8..d0b3bf8c17 100644 --- a/awx/ui_next/src/components/StatusIcon/StatusIcon.test.jsx +++ b/awx/ui_next/src/components/StatusIcon/StatusIcon.test.jsx @@ -27,7 +27,6 @@ describe('StatusIcon', () => { }); test('renders a successful status when host status is "ok"', () => { const wrapper = mount(); - wrapper.debug(); expect(wrapper).toHaveLength(1); expect(wrapper.find('StatusIcon__SuccessfulTop')).toHaveLength(1); expect(wrapper.find('StatusIcon__SuccessfulBottom')).toHaveLength(1); diff --git a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionForm.jsx b/awx/ui_next/src/screens/Template/Survey/SurveyQuestionForm.jsx index 78c59d274f..f2f1e8860c 100644 --- a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionForm.jsx +++ b/awx/ui_next/src/screens/Template/Survey/SurveyQuestionForm.jsx @@ -199,6 +199,7 @@ function SurveyQuestionForm({ t`Each answer choice must be on a separate line.` )} isRequired + rows="10" /> { + if (value.trim().length < min) { + return i18n._(t`This field must be at least ${min} characters`); + } + return undefined; + }; +} + export function minMaxValue(min, max, i18n) { return value => { if (value < min || value > max) { @@ -57,10 +66,21 @@ export function noWhiteSpace(i18n) { }; } +export function integer(i18n) { + return value => { + const str = String(value); + if (/[^0-9]/.test(str)) { + return i18n._(t`This field must be an integer`); + } + return undefined; + }; +} + export function combine(validators) { return value => { for (let i = 0; i < validators.length; i++) { - const error = validators[i](value); + const validate = validators[i]; + const error = validate ? validate(value) : null; if (error) { return error; } diff --git a/awx/ui_next/src/util/validators.test.js b/awx/ui_next/src/util/validators.test.js index 067f753263..f3e37c6cde 100644 --- a/awx/ui_next/src/util/validators.test.js +++ b/awx/ui_next/src/util/validators.test.js @@ -1,4 +1,11 @@ -import { required, maxLength, noWhiteSpace, combine } from './validators'; +import { + required, + minLength, + maxLength, + noWhiteSpace, + integer, + combine, +} from './validators'; const i18n = { _: val => val }; @@ -52,6 +59,21 @@ describe('validators', () => { }); }); + test('minLength accepts value above min', () => { + expect(minLength(3, i18n)('snazzy')).toBeUndefined(); + }); + + test('minLength accepts value equal to min', () => { + expect(minLength(10, i18n)('abracadbra')).toBeUndefined(); + }); + + test('minLength rejects value below min', () => { + expect(minLength(12, i18n)('abracadbra')).toEqual({ + id: 'This field must be at least {min} characters', + values: { min: 12 }, + }); + }); + test('noWhiteSpace returns error', () => { expect(noWhiteSpace(i18n)('this has spaces')).toEqual({ id: 'This field must not contain spaces', @@ -68,6 +90,26 @@ describe('validators', () => { expect(noWhiteSpace(i18n)('this_has_no_whitespace')).toBeUndefined(); }); + test('integer should accept integer (number)', () => { + expect(integer(i18n)(13)).toBeUndefined(); + }); + + test('integer should accept integer (string)', () => { + expect(integer(i18n)('13')).toBeUndefined(); + }); + + test('integer should reject decimal/float', () => { + expect(integer(i18n)(13.1)).toEqual({ + id: 'This field must be an integer', + }); + }); + + test('integer should reject string containing alphanum', () => { + expect(integer(i18n)('15a')).toEqual({ + id: 'This field must be an integer', + }); + }); + test('combine should run all validators', () => { const validators = [required(null, i18n), noWhiteSpace(i18n)]; expect(combine(validators)('')).toEqual({ @@ -78,4 +120,12 @@ describe('validators', () => { }); expect(combine(validators)('ok')).toBeUndefined(); }); + + test('combine should skip null validators', () => { + const validators = [required(null, i18n), null]; + expect(combine(validators)('')).toEqual({ + id: 'This field must not be blank', + }); + expect(combine(validators)('ok')).toBeUndefined(); + }); }); From 08381577f5542b3df6dd9a44d89ac5f6f79bb4d2 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 16 Apr 2020 15:40:28 -0700 Subject: [PATCH 3/4] Merge prompt extra_vars before POSTing * Merge the extra_vars field with survey question responses before sending to API * Clean up select and multi-select survey fields --- .../src/components/FormField/FieldTooltip.jsx | 11 +- .../src/components/FormField/FormField.jsx | 34 +---- .../components/LaunchPrompt/LaunchPrompt.jsx | 4 +- .../components/LaunchPrompt/SurveyStep.jsx | 140 +++++++++++++----- .../components/LaunchPrompt/mergeExtraVars.js | 11 ++ .../LaunchPrompt/mergeExtraVars.test.js | 34 +++++ .../DeleteRoleConfirmationModal.test.jsx.snap | 2 + 7 files changed, 166 insertions(+), 70 deletions(-) create mode 100644 awx/ui_next/src/components/LaunchPrompt/mergeExtraVars.js create mode 100644 awx/ui_next/src/components/LaunchPrompt/mergeExtraVars.test.js diff --git a/awx/ui_next/src/components/FormField/FieldTooltip.jsx b/awx/ui_next/src/components/FormField/FieldTooltip.jsx index 59031bb365..56dfc43172 100644 --- a/awx/ui_next/src/components/FormField/FieldTooltip.jsx +++ b/awx/ui_next/src/components/FormField/FieldTooltip.jsx @@ -8,19 +8,26 @@ const QuestionCircleIcon = styled(PFQuestionCircleIcon)` margin-left: 10px; `; -function FieldTooltip({ content }) { +function FieldTooltip({ content, ...rest }) { + if (!content) { + return null; + } return ( ); } FieldTooltip.propTypes = { - content: node.isRequired, + content: node, +}; +FieldTooltip.defaultProps = { + content: null, }; export default FieldTooltip; diff --git a/awx/ui_next/src/components/FormField/FormField.jsx b/awx/ui_next/src/components/FormField/FormField.jsx index 49d77cfae3..c94c370e47 100644 --- a/awx/ui_next/src/components/FormField/FormField.jsx +++ b/awx/ui_next/src/components/FormField/FormField.jsx @@ -1,18 +1,8 @@ import React from 'react'; import PropTypes from 'prop-types'; import { useField } from 'formik'; -import { - FormGroup, - TextInput, - TextArea, - Tooltip, -} from '@patternfly/react-core'; -import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; -import styled from 'styled-components'; - -const QuestionCircleIcon = styled(PFQuestionCircleIcon)` - margin-left: 10px; -`; +import { FormGroup, TextInput, TextArea } from '@patternfly/react-core'; +import FieldTooltip from './FieldTooltip'; function FormField(props) { const { @@ -40,15 +30,7 @@ function FormField(props) { isValid={isValid} label={label} > - {tooltip && ( - - - - )} +