From 4a2a6949a8ce83f33af701789412459d65c546c7 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 20 Jan 2021 10:20:30 -0500 Subject: [PATCH] Fixes missing credential types and makes credential type drop down a typeahead component --- .../CredentialAdd/CredentialAdd.jsx | 51 +++++----- .../CredentialEdit/CredentialEdit.jsx | 94 ++++++++++--------- .../CredentialEdit/CredentialEdit.test.jsx | 6 ++ .../Credential/shared/CredentialForm.jsx | 80 ++++++++-------- .../Credential/shared/CredentialForm.test.jsx | 51 ++++++++-- 5 files changed, 163 insertions(+), 119 deletions(-) diff --git a/awx/ui_next/src/screens/Credential/CredentialAdd/CredentialAdd.jsx b/awx/ui_next/src/screens/Credential/CredentialAdd/CredentialAdd.jsx index c5df022772..ecffa873c1 100644 --- a/awx/ui_next/src/screens/Credential/CredentialAdd/CredentialAdd.jsx +++ b/awx/ui_next/src/screens/Credential/CredentialAdd/CredentialAdd.jsx @@ -1,4 +1,4 @@ -import React, { useCallback, useState, useEffect } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { useHistory } from 'react-router-dom'; import { PageSection, Card } from '@patternfly/react-core'; import { CardBody } from '../../../components/Card'; @@ -14,9 +14,6 @@ import CredentialForm from '../shared/CredentialForm'; import useRequest from '../../../util/useRequest'; function CredentialAdd({ me }) { - const [error, setError] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [credentialTypes, setCredentialTypes] = useState(null); const history = useHistory(); const { @@ -85,34 +82,38 @@ function CredentialAdd({ me }) { history.push(`/credentials/${credentialId}/details`); } }, [credentialId, history]); - - useEffect(() => { - const loadData = async () => { - try { + const { isLoading, error, request: loadData, result } = useRequest( + useCallback(async () => { + const { data } = await CredentialTypesAPI.read({ page_size: 200 }); + const credTypes = data.results; + if (data.next && data.next.includes('page=2')) { const { - data: { results: loadedCredentialTypes }, - } = await CredentialTypesAPI.read(); - setCredentialTypes( - loadedCredentialTypes.reduce((credentialTypesMap, credentialType) => { - credentialTypesMap[credentialType.id] = credentialType; - return credentialTypesMap; - }, {}) - ); - } catch (err) { - setError(err); - } finally { - setIsLoading(false); + data: { results }, + } = await CredentialTypesAPI.read({ + page_size: 200, + page: 2, + }); + credTypes.concat(results); } - }; + + const creds = credTypes.reduce((credentialTypesMap, credentialType) => { + credentialTypesMap[credentialType.id] = credentialType; + return credentialTypesMap; + }, {}); + return creds; + }, []), + {} + ); + useEffect(() => { loadData(); - }, []); + }, [loadData]); const handleCancel = () => { history.push('/credentials'); }; const handleSubmit = async values => { - await submitRequest(values, credentialTypes); + await submitRequest(values, result); }; if (error) { @@ -126,7 +127,7 @@ function CredentialAdd({ me }) { ); } - if (isLoading) { + if (isLoading && !result) { return ( @@ -144,7 +145,7 @@ function CredentialAdd({ me }) { diff --git a/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.jsx b/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.jsx index 83e64e81f8..9ffc29f39b 100644 --- a/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.jsx +++ b/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.jsx @@ -1,5 +1,5 @@ -import React, { useCallback, useState, useEffect } from 'react'; -import { useHistory } from 'react-router-dom'; +import React, { useCallback, useEffect } from 'react'; +import { useHistory, useParams } from 'react-router-dom'; import { object } from 'prop-types'; import { CardBody } from '../../../components/Card'; import { @@ -13,11 +13,8 @@ import CredentialForm from '../shared/CredentialForm'; import useRequest from '../../../util/useRequest'; function CredentialEdit({ credential, me }) { - const [error, setError] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [credentialTypes, setCredentialTypes] = useState(null); - const [inputSources, setInputSources] = useState({}); const history = useHistory(); + const { id: credId } = useParams(); const { error: submitError, request: submitRequest, result } = useRequest( useCallback( @@ -55,7 +52,7 @@ function CredentialEdit({ credential, me }) { input_field_name: fieldName, metadata: fieldValue.inputs, source_credential: fieldValue.credential.id, - target_credential: credential.id, + target_credential: credId, }); } if (fieldValue.touched) { @@ -88,7 +85,7 @@ function CredentialEdit({ credential, me }) { modifiedData.user = me.id; } const [{ data }] = await Promise.all([ - CredentialsAPI.update(credential.id, modifiedData), + CredentialsAPI.update(credId, modifiedData), ...destroyInputSources(), ]); @@ -96,7 +93,7 @@ function CredentialEdit({ credential, me }) { return data; }, - [me, credential.id] + [me, credId] ) ); @@ -105,56 +102,63 @@ function CredentialEdit({ credential, me }) { history.push(`/credentials/${result.id}/details`); } }, [result, history]); + const { + isLoading, + error, + request: loadData, + result: { credentialTypes, loadedInputSources }, + } = useRequest( + useCallback(async () => { + const [ + { data }, + { + data: { results }, + }, + ] = await Promise.all([ + CredentialTypesAPI.read({ page_size: 200 }), + CredentialsAPI.readInputSources(credId, { page_size: 200 }), + ]); + const credTypes = data.results; + if (data.next && data.next.includes('page=2')) { + const { + data: { results: additionalCredTypes }, + } = await CredentialTypesAPI.read({ + page_size: 200, + page: 2, + }); + credTypes.concat([...additionalCredTypes]); + } + const creds = credTypes.reduce((credentialTypesMap, credentialType) => { + credentialTypesMap[credentialType.id] = credentialType; + return credentialTypesMap; + }, {}); + const inputSources = results.reduce((inputSourcesMap, inputSource) => { + inputSourcesMap[inputSource.input_field_name] = inputSource; + return inputSourcesMap; + }, {}); + return { credentialTypes: creds, loadedInputSources: inputSources }; + }, [credId]), + { credentialTypes: {}, loadedInputSources: {} } + ); useEffect(() => { - const loadData = async () => { - try { - const [ - { - data: { results: loadedCredentialTypes }, - }, - { - data: { results: loadedInputSources }, - }, - ] = await Promise.all([ - CredentialTypesAPI.read(), - CredentialsAPI.readInputSources(credential.id, { page_size: 200 }), - ]); - setCredentialTypes( - loadedCredentialTypes.reduce((credentialTypesMap, credentialType) => { - credentialTypesMap[credentialType.id] = credentialType; - return credentialTypesMap; - }, {}) - ); - setInputSources( - loadedInputSources.reduce((inputSourcesMap, inputSource) => { - inputSourcesMap[inputSource.input_field_name] = inputSource; - return inputSourcesMap; - }, {}) - ); - } catch (err) { - setError(err); - } finally { - setIsLoading(false); - } - }; loadData(); - }, [credential.id]); + }, [loadData]); const handleCancel = () => { - const url = `/credentials/${credential.id}/details`; + const url = `/credentials/${credId}/details`; history.push(`${url}`); }; const handleSubmit = async values => { - await submitRequest(values, credentialTypes, inputSources); + await submitRequest(values, credentialTypes, loadedInputSources); }; if (error) { return ; } - if (isLoading) { + if (isLoading && !credentialTypes) { return ; } @@ -165,7 +169,7 @@ function CredentialEdit({ credential, me }) { onSubmit={handleSubmit} credential={credential} credentialTypes={credentialTypes} - inputSources={inputSources} + inputSources={loadedInputSources} submitError={submitError} /> diff --git a/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.test.jsx b/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.test.jsx index ac85aa7939..17ab9d3ed4 100644 --- a/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.test.jsx +++ b/awx/ui_next/src/screens/Credential/CredentialEdit/CredentialEdit.test.jsx @@ -14,6 +14,12 @@ import { import CredentialEdit from './CredentialEdit'; jest.mock('../../../api'); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: () => ({ + id: 3, + }), +})); const mockCredential = { id: 3, diff --git a/awx/ui_next/src/screens/Credential/shared/CredentialForm.jsx b/awx/ui_next/src/screens/Credential/shared/CredentialForm.jsx index c352b0b7e0..afb7fc9bc0 100644 --- a/awx/ui_next/src/screens/Credential/shared/CredentialForm.jsx +++ b/awx/ui_next/src/screens/Credential/shared/CredentialForm.jsx @@ -3,26 +3,28 @@ import { Formik, useField, useFormikContext } from 'formik'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { arrayOf, func, object, shape } from 'prop-types'; -import { ActionGroup, Button, Form, FormGroup } from '@patternfly/react-core'; +import { + ActionGroup, + Button, + Form, + FormGroup, + Select, + SelectOption, + SelectVariant, +} from '@patternfly/react-core'; import FormField, { FormSubmitError } from '../../../components/FormField'; import { FormColumnLayout, FormFullWidthLayout, } from '../../../components/FormLayout'; -import AnsibleSelect from '../../../components/AnsibleSelect'; import { required } from '../../../util/validators'; import OrganizationLookup from '../../../components/Lookup/OrganizationLookup'; import TypeInputsSubForm from './TypeInputsSubForm'; import ExternalTestModal from './ExternalTestModal'; -function CredentialFormFields({ - i18n, - credentialTypes, - formik, - initialValues, -}) { - const { setFieldValue } = useFormikContext(); - +function CredentialFormFields({ i18n, credentialTypes }) { + const { setFieldValue, initialValues, setFieldTouched } = useFormikContext(); + const [isSelectOpen, setIsSelectOpen] = useState(false); const [credTypeField, credTypeMeta, credTypeHelpers] = useField({ name: 'credential_type', validate: required(i18n._(t`Select a value for this field`), i18n), @@ -30,7 +32,7 @@ function CredentialFormFields({ const isGalaxyCredential = !!credTypeField.value && - credentialTypes[credTypeField.value].kind === 'galaxy'; + credentialTypes[credTypeField.value]?.kind === 'galaxy'; const [orgField, orgMeta, orgHelpers] = useField({ name: 'organization', @@ -52,16 +54,14 @@ function CredentialFormFields({ }) .sort((a, b) => (a.label.toLowerCase() > b.label.toLowerCase() ? 1 : -1)); - const resetSubFormFields = (newCredentialType, form) => { + const resetSubFormFields = newCredentialType => { const fields = credentialTypes[newCredentialType].inputs.fields || []; fields.forEach( ({ ask_at_runtime, type, id, choices, default: defaultValue }) => { - if ( - parseInt(newCredentialType, 10) === form.initialValues.credential_type - ) { - form.setFieldValue(`inputs.${id}`, initialValues.inputs[id]); + if (parseInt(newCredentialType, 10) === initialValues.credential_type) { + setFieldValue(`inputs.${id}`, initialValues.inputs[id]); if (ask_at_runtime) { - form.setFieldValue( + setFieldValue( `passwordPrompts.${id}`, initialValues.passwordPrompts[id] ); @@ -69,24 +69,24 @@ function CredentialFormFields({ } else { switch (type) { case 'string': - form.setFieldValue(`inputs.${id}`, defaultValue || ''); + setFieldValue(`inputs.${id}`, defaultValue || ''); break; case 'boolean': - form.setFieldValue(`inputs.${id}`, defaultValue || false); + setFieldValue(`inputs.${id}`, defaultValue || false); break; default: break; } if (choices) { - form.setFieldValue(`inputs.${id}`, defaultValue); + setFieldValue(`inputs.${id}`, defaultValue); } if (ask_at_runtime) { - form.setFieldValue(`passwordPrompts.${id}`, false); + setFieldValue(`passwordPrompts.${id}`, false); } } - form.setFieldTouched(`inputs.${id}`, false); + setFieldTouched(`inputs.${id}`, false); } ); }; @@ -133,23 +133,27 @@ function CredentialFormFields({ } label={i18n._(t`Credential Type`)} > - { + onToggle={setIsSelectOpen} + onSelect={(event, value) => { credTypeHelpers.setValue(value); - resetSubFormFields(value, formik); + resetSubFormFields(value); }} - /> + selections={credTypeField.value} + placeholder={i18n._(t`Select a credential Type`)} + isCreatable={false} + maxHeight="300px" + > + {credentialTypeOptions.map(credType => ( + + {credType.label} + + ))} + {credTypeField.value !== undefined && credTypeField.value !== '' && @@ -177,7 +181,7 @@ function CredentialForm({ name: credential.name || '', description: credential.description || '', organization: credential?.summary_fields?.organization || null, - credential_type: credential.credential_type || '', + credential_type: credential?.credential_type || '', inputs: {}, passwordPrompts: {}, }; @@ -235,8 +239,6 @@ function CredentialForm({
', () => { test('should display cred type subform when scm type select has a value', async () => { await act(async () => { await wrapper - .find('AnsibleSelect[id="credential-type"]') - .invoke('onChange')(null, 1); + .find('Select[aria-label="Credential Type"]') + .invoke('onToggle')(); }); wrapper.update(); + await act(async () => { + await wrapper + .find('Select[aria-label="Credential Type"]') + .invoke('onSelect')(null, 1); + }); + wrapper.update(); + machineFieldExpects(); await act(async () => { await wrapper - .find('AnsibleSelect[id="credential-type"]') - .invoke('onChange')(null, 2); + .find('Select[aria-label="Credential Type"]') + .invoke('onToggle')(); + }); + wrapper.update(); + await act(async () => { + await wrapper + .find('Select[aria-label="Credential Type"]') + .invoke('onSelect')(null, 2); }); wrapper.update(); sourceFieldExpects(); @@ -154,8 +167,14 @@ describe('', () => { test('should update expected fields when gce service account json file uploaded', async () => { await act(async () => { await wrapper - .find('AnsibleSelect[id="credential-type"]') - .invoke('onChange')(null, 10); + .find('Select[aria-label="Credential Type"]') + .invoke('onToggle')(); + }); + wrapper.update(); + await act(async () => { + await wrapper + .find('Select[aria-label="Credential Type"]') + .invoke('onSelect')(null, 10); }); wrapper.update(); gceFieldExpects(); @@ -215,8 +234,14 @@ describe('', () => { test('should show error when error thrown parsing JSON', async () => { await act(async () => { await wrapper - .find('AnsibleSelect[id="credential-type"]') - .invoke('onChange')(null, 10); + .find('Select[aria-label="Credential Type"]') + .invoke('onToggle')(); + }); + wrapper.update(); + await act(async () => { + await wrapper + .find('Select[aria-label="Credential Type"]') + .invoke('onSelect')(null, 10); }); wrapper.update(); expect(wrapper.find('#credential-gce-file-helper').text()).toBe( @@ -246,8 +271,14 @@ describe('', () => { test('should show Test button when external credential type is selected', async () => { await act(async () => { await wrapper - .find('AnsibleSelect[id="credential-type"]') - .invoke('onChange')(null, 21); + .find('Select[aria-label="Credential Type"]') + .invoke('onToggle')(); + }); + wrapper.update(); + await act(async () => { + await wrapper + .find('Select[aria-label="Credential Type"]') + .invoke('onSelect')(null, 21); }); wrapper.update(); expect(wrapper.find('Button[children="Test"]').length).toBe(1);