From 7f409c6487c5df6f37f822fd24c2346b071fd48d Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Fri, 25 Oct 2019 10:20:22 -0400 Subject: [PATCH] Moves JT CredentialsList Manipulation Back to CredentialsLookup Rename CredentialsLookup to MultiCredentialLookup Removes unnecessary functions in Lookup. Puts CredentialsList manipulation on CredsLookup and removes that work from JTForm. Upates tests for CredentialsLookup and JTForm to reflect changes above. --- awx/ui_next/src/components/Lookup/Lookup.jsx | 11 +---- ...sLookup.jsx => MultiCredentialsLookup.jsx} | 37 +++++++++++--- ...st.jsx => MultiCredentialsLookup.test.jsx} | 41 ++++++++++++---- awx/ui_next/src/components/Lookup/index.js | 2 +- .../Template/shared/JobTemplateForm.jsx | 40 +++------------- .../Template/shared/JobTemplateForm.test.jsx | 48 +------------------ 6 files changed, 73 insertions(+), 106 deletions(-) rename awx/ui_next/src/components/Lookup/{CredentialsLookup.jsx => MultiCredentialsLookup.jsx} (75%) rename awx/ui_next/src/components/Lookup/{CredentialsLookup.test.jsx => MultiCredentialsLookup.test.jsx} (61%) diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index da8cd3d98d..dfe6c2ad00 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -68,7 +68,6 @@ class Lookup extends React.Component { results: [], count: 0, error: null, - isDropdownOpen: false, }; this.qsConfig = getQSConfig(props.qsNamespace, { page: 1, @@ -80,11 +79,10 @@ class Lookup extends React.Component { this.saveModal = this.saveModal.bind(this); this.getData = this.getData.bind(this); this.clearQSParams = this.clearQSParams.bind(this); - this.toggleDropdown = this.toggleDropdown.bind(this); } componentDidMount() { - Promise.all([this.getData()]); + this.getData(); } componentDidUpdate(prevProps) { @@ -97,11 +95,6 @@ class Lookup extends React.Component { } } - toggleDropdown() { - const { isDropdownOpen } = this.state; - this.setState({ isDropdownOpen: !isDropdownOpen }); - } - assertCorrectValueType() { const { multiple, value, selectCategoryOptions } = this.props; if (selectCategoryOptions) { @@ -315,7 +308,7 @@ class Lookup extends React.Component { cred.id === newCredential.id).length > + 0; + + if (isSelectedCredentialInState) { + newCredentialsList = credentialsToUpdate.filter( + cred => cred.id !== newCredential.id + ); + } else { + newCredentialsList = credentialsToUpdate.filter( + credential => + credential.kind === 'vault' || credential.kind !== newCredential.kind + ); + newCredentialsList = [...newCredentialsList, newCredential]; + } + onChange(newCredentialsList); + } + handleCredentialTypeSelect(value, type) { const { credentialTypes } = this.state; const selectedType = credentialTypes.filter(item => item.label === type); @@ -76,7 +99,7 @@ class CredentialsLookup extends React.Component { render() { const { selectedCredentialType, credentialTypes } = this.state; - const { tooltip, i18n, credentials, onChange } = this.props; + const { tooltip, i18n, credentials } = this.props; return ( {tooltip && ( @@ -89,7 +112,7 @@ class CredentialsLookup extends React.Component { selectCategoryOptions={credentialTypes} selectCategory={this.handleCredentialTypeSelect} selectedCategory={selectedCredentialType} - onToggleItem={onChange} + onToggleItem={this.toggleCredentialSelection} onloadCategories={this.loadCredentialTypes} id="org-credentials" lookupHeader={i18n._(t`Credentials`)} @@ -115,13 +138,13 @@ class CredentialsLookup extends React.Component { } } -CredentialsLookup.propTypes = { +MultiCredentialsLookup.propTypes = { tooltip: PropTypes.string, }; -CredentialsLookup.defaultProps = { +MultiCredentialsLookup.defaultProps = { tooltip: '', }; -export { CredentialsLookup as _CredentialsLookup }; +export { MultiCredentialsLookup as _MultiCredentialsLookup }; -export default withI18n()(CredentialsLookup); +export default withI18n()(MultiCredentialsLookup); diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx similarity index 61% rename from awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx rename to awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx index 9831565b94..a525fb9e9a 100644 --- a/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx @@ -1,11 +1,12 @@ import React from 'react'; + import { mountWithContexts } from '@testUtils/enzymeHelpers'; -import CredentialsLookup from './CredentialsLookup'; +import MultiCredentialsLookup from './MultiCredentialsLookup'; import { CredentialsAPI, CredentialTypesAPI } from '@api'; jest.mock('@api'); -describe('', () => { +describe('', () => { let wrapper; let lookup; let credLookup; @@ -14,6 +15,8 @@ describe('', () => { const credentials = [ { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' }, + { name: 'Gatsby', id: 21, kind: 'vault' }, + { name: 'Gatsby', id: 8, kind: 'Machine' }, ]; beforeEach(() => { CredentialTypesAPI.read.mockResolvedValue({ @@ -45,7 +48,7 @@ describe('', () => { }); onChange = jest.fn(); wrapper = mountWithContexts( - {}} credentials={credentials} onChange={onChange} @@ -53,7 +56,7 @@ describe('', () => { /> ); lookup = wrapper.find('Lookup'); - credLookup = wrapper.find('CredentialsLookup'); + credLookup = wrapper.find('MultiCredentialsLookup'); }); afterEach(() => { @@ -61,17 +64,21 @@ describe('', () => { wrapper.unmount(); }); - test('CredentialsLookup renders properly', () => { - expect(wrapper.find('CredentialsLookup')).toHaveLength(1); + test('MultiCredentialsLookup renders properly', () => { + expect(wrapper.find('MultiCredentialsLookup')).toHaveLength(1); expect(CredentialTypesAPI.read).toHaveBeenCalled(); }); - test('onChange is called when you click to remove a credential from input', () => { + test('onChange is called when you click to remove a credential from input', async () => { const chip = wrapper.find('PFChip'); const button = chip.at(1).find('Button'); - expect(chip).toHaveLength(2); + expect(chip).toHaveLength(4); button.prop('onClick')(); - expect(onChange).toBeCalledTimes(1); + expect(onChange).toBeCalledWith([ + { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, + { id: 21, kind: 'vault', name: 'Gatsby' }, + { id: 8, kind: 'Machine', name: 'Gatsby' }, + ]); }); test('can change credential types', () => { @@ -87,4 +94,20 @@ describe('', () => { }); expect(CredentialsAPI.read).toHaveBeenCalled(); }); + test('Toggle credentials only adds 1 credential per credential type except vault(see below)', () => { + lookup.prop('onToggleItem')({ name: 'Party', id: 9, kind: 'Machine' }); + expect(onChange).toBeCalledWith([ + { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, + { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' }, + { id: 21, kind: 'vault', name: 'Gatsby' }, + { id: 9, kind: 'Machine', name: 'Party' }, + ]); + }); + test('Toggle credentials only adds 1 credential per credential type', () => { + lookup.prop('onToggleItem')({ name: 'Party', id: 22, kind: 'vault' }); + expect(onChange).toBeCalledWith([ + ...credentials, + { name: 'Party', id: 22, kind: 'vault' }, + ]); + }); }); diff --git a/awx/ui_next/src/components/Lookup/index.js b/awx/ui_next/src/components/Lookup/index.js index 5620da1a89..cde48e2bcd 100644 --- a/awx/ui_next/src/components/Lookup/index.js +++ b/awx/ui_next/src/components/Lookup/index.js @@ -2,4 +2,4 @@ export { default } from './Lookup'; export { default as InstanceGroupsLookup } from './InstanceGroupsLookup'; export { default as InventoryLookup } from './InventoryLookup'; export { default as ProjectLookup } from './ProjectLookup'; -export { default as CredentialsLookup } from './CredentialsLookup'; +export { default as MultiCredentialsLookup } from './MultiCredentialsLookup'; diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 0c8b6d1225..4da2c86534 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -27,7 +27,7 @@ import { InventoryLookup, InstanceGroupsLookup, ProjectLookup, - CredentialsLookup, + MultiCredentialsLookup, } from '@components/Lookup'; import { JobTemplatesAPI } from '@api'; import LabelSelect from './LabelSelect'; @@ -77,11 +77,9 @@ class JobTemplateForm extends Component { project: props.template.summary_fields.project, inventory: props.template.summary_fields.inventory, allowCallbacks: !!props.template.host_config_key, - credentials: props.template.summary_fields.credentials, }; this.handleProjectValidation = this.handleProjectValidation.bind(this); this.loadRelatedInstanceGroups = this.loadRelatedInstanceGroups.bind(this); - this.toggleCredentialSelection = this.toggleCredentialSelection.bind(this); } componentDidMount() { @@ -108,31 +106,6 @@ class JobTemplateForm extends Component { } } - toggleCredentialSelection(newCredential) { - const { credentials: credentialsToUpdate } = this.state; - const { setFieldValue } = this.props; - - let newCredentialsList; - const isSelectedCredentialInState = - credentialsToUpdate.filter(cred => cred.id === newCredential.id).length > - 0; - - if (isSelectedCredentialInState) { - newCredentialsList = credentialsToUpdate.filter( - cred => cred.id !== newCredential.id - ); - } else { - newCredentialsList = credentialsToUpdate.filter( - credential => - credential.kind === 'vault' || credential.kind !== newCredential.kind - ); - newCredentialsList = [...newCredentialsList, newCredential]; - } - - setFieldValue('credentials', newCredentialsList); - this.setState({ credentials: newCredentialsList }); - } - handleProjectValidation() { const { i18n, touched } = this.props; const { project } = this.state; @@ -154,7 +127,6 @@ class JobTemplateForm extends Component { inventory, project, allowCallbacks, - credentials, } = this.state; const { handleCancel, @@ -352,10 +324,12 @@ class JobTemplateForm extends Component { ( - ( + + setFieldValue('credentials', newCredentials) + } onError={err => this.setState({ contentError: err })} tooltip={i18n._( t`Select credentials that allow Tower to access the nodes this job will be ran against. You can only select one credential of each type. For machine credentials (SSH), checking "Prompt on launch" without selecting credentials will require you to select a machine credential at run time. If you select credentials and check "Prompt on launch", the selected credential(s) become the defaults that can be updated at run time.` diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx index c939e678d1..7ed98ad893 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -3,13 +3,7 @@ import { act } from 'react-dom/test-utils'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; import JobTemplateForm from './JobTemplateForm'; -import { - LabelsAPI, - JobTemplatesAPI, - ProjectsAPI, - CredentialTypesAPI, - CredentialsAPI, -} from '@api'; +import { LabelsAPI, JobTemplatesAPI, ProjectsAPI, CredentialsAPI } from '@api'; jest.mock('@api'); @@ -200,44 +194,4 @@ describe('', () => { wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); expect(handleCancel).toBeCalled(); }); - test('toggleCredentialSelection should handle credential selection properly', async () => { - let wrapper; - await act(async () => { - wrapper = mountWithContexts( - - ); - }); - - function callToggleCredSelection(credential, formState) { - JobTempForm.instance().toggleCredentialSelection(credential); - - expect(form.state('values').credentials).toEqual(formState); - } - const form = wrapper.find('Formik'); - const JobTempForm = wrapper.find('JobTemplateForm'); - - callToggleCredSelection( - { id: 3, kind: 'vault', name: 'Vault Credential' }, - [ - { id: 1, kind: 'cloud', name: 'Foo' }, - { id: 2, kind: 'ssh', name: 'Bar' }, - { id: 3, kind: 'vault', name: 'Vault Credential' }, - ] - ); - callToggleCredSelection({ id: 4, kind: 'ssh', name: 'New Bar' }, [ - { id: 1, kind: 'cloud', name: 'Foo' }, - { id: 3, kind: 'vault', name: 'Vault Credential' }, - { id: 4, kind: 'ssh', name: 'New Bar' }, - ]); - callToggleCredSelection({ id: 5, kind: 'vault', name: 'New Vault' }, [ - { id: 1, kind: 'cloud', name: 'Foo' }, - { id: 3, kind: 'vault', name: 'Vault Credential' }, - { id: 4, kind: 'ssh', name: 'New Bar' }, - { id: 5, kind: 'vault', name: 'New Vault' }, - ]); - }); });