From d10e727b3cd80c9e25693cdd01e301416bc9a2c8 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 22 Oct 2019 16:00:02 -0400 Subject: [PATCH 1/7] Adds CredentialLookUp to JT Form --- awx/ui_next/src/api/index.js | 6 + awx/ui_next/src/api/models/CredentialTypes.js | 10 ++ awx/ui_next/src/api/models/Credentials.js | 10 ++ awx/ui_next/src/api/models/JobTemplates.js | 8 + .../components/Lookup/CredentialsLookup.jsx | 165 ++++++++++++++++++ awx/ui_next/src/components/Lookup/Lookup.jsx | 151 ++++++++++------ awx/ui_next/src/components/Lookup/index.js | 1 + .../components/SelectedList/SelectedList.jsx | 31 ++-- .../JobTemplateAdd/JobTemplateAdd.jsx | 9 + .../JobTemplateEdit/JobTemplateEdit.jsx | 30 +++- .../Template/shared/JobTemplateForm.jsx | 22 ++- 11 files changed, 375 insertions(+), 68 deletions(-) create mode 100644 awx/ui_next/src/api/models/CredentialTypes.js create mode 100644 awx/ui_next/src/api/models/Credentials.js create mode 100644 awx/ui_next/src/components/Lookup/CredentialsLookup.jsx diff --git a/awx/ui_next/src/api/index.js b/awx/ui_next/src/api/index.js index 7d05309b3a..9d76cc8bd8 100644 --- a/awx/ui_next/src/api/index.js +++ b/awx/ui_next/src/api/index.js @@ -1,5 +1,7 @@ import AdHocCommands from './models/AdHocCommands'; import Config from './models/Config'; +import CredentialTypes from './models/CredentialTypes' +import Credentials from './models/Credentials' import InstanceGroups from './models/InstanceGroups'; import Inventories from './models/Inventories'; import InventorySources from './models/InventorySources'; @@ -23,6 +25,8 @@ import WorkflowJobTemplates from './models/WorkflowJobTemplates'; const AdHocCommandsAPI = new AdHocCommands(); const ConfigAPI = new Config(); +const CredentialsAPI = new Credentials(); +const CredentialTypesAPI = new CredentialTypes(); const InstanceGroupsAPI = new InstanceGroups(); const InventoriesAPI = new Inventories(); const InventorySourcesAPI = new InventorySources(); @@ -47,6 +51,8 @@ const WorkflowJobTemplatesAPI = new WorkflowJobTemplates(); export { AdHocCommandsAPI, ConfigAPI, + CredentialsAPI, + CredentialTypesAPI, InstanceGroupsAPI, InventoriesAPI, InventorySourcesAPI, diff --git a/awx/ui_next/src/api/models/CredentialTypes.js b/awx/ui_next/src/api/models/CredentialTypes.js new file mode 100644 index 0000000000..65906cdcbd --- /dev/null +++ b/awx/ui_next/src/api/models/CredentialTypes.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class CredentialTypes extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/credential_types/'; + } +} + +export default CredentialTypes; diff --git a/awx/ui_next/src/api/models/Credentials.js b/awx/ui_next/src/api/models/Credentials.js new file mode 100644 index 0000000000..2e3634b4e5 --- /dev/null +++ b/awx/ui_next/src/api/models/Credentials.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class Credentials extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/credentials/'; + } +} + +export default Credentials; diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index cff6858db7..9941ad220d 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -44,6 +44,14 @@ class JobTemplates extends InstanceGroupsMixin(NotificationsMixin(Base)) { readCredentials(id, params) { return this.http.get(`${this.baseUrl}${id}/credentials/`, { params }); } + + associateCredentials(id, credential) { + return this.http.post(`${this.baseUrl}${id}/credentials/`, { id: credential }); + } + + disassociateCredentials(id, credential) { + return this.http.post(`${this.baseUrl}${id}/credentials/`, { id: credential, disassociate: true }); + } } export default JobTemplates; diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx new file mode 100644 index 0000000000..1648d7d910 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx @@ -0,0 +1,165 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { withI18n } from '@lingui/react'; +import { withRouter } from 'react-router-dom'; +import { t } from '@lingui/macro'; +import { FormGroup, Tooltip } from '@patternfly/react-core'; +import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; +import styled from 'styled-components'; + +import { CredentialsAPI, CredentialTypesAPI } from '@api'; +import Lookup from '@components/Lookup'; + +const QuestionCircleIcon = styled(PFQuestionCircleIcon)` + margin-left: 10px; +`; + +class CredentialsLookup extends React.Component { + constructor(props) { + super(props) + + this.state = { + credentials: props.credentials, + selectedCredentialType: {label: "Machine", id: 1, kind: 'ssh'}, + credentialTypes: [] + } + + this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind(this); + this.loadCredentials = this.loadCredentials.bind(this); + this.loadCredentialTypes = this.loadCredentialTypes.bind(this); + this.toggleCredential = this.toggleCredential.bind(this); + } + + componentDidMount() { + this.loadCredentials({page: 1, page_size: 5, order_by:'name'}) + this.loadCredentialTypes(); + } + + componentDidUpdate(prevState) { + const {selectedType} = this.state + if (prevState.selectedType !== selectedType) { + Promise.all([this.loadCredentials()]); + } + } + + async loadCredentialTypes() { + const {onError} =this.props + try { + const { data } = await CredentialTypesAPI.read() + const acceptableTypes = ["machine", "cloud", "net", "ssh", "vault"] + const credentialTypes =[] + data.results + .forEach(cred => { + acceptableTypes.forEach(aT => { + if (aT === cred.kind) { + // This object has several repeated values as some of it's children + // require different field values. + cred = { + id: cred.id, kind: cred.kind, type: cred.namespace, + value: cred.name, label: cred.name, isDisabled: false + } + credentialTypes.push(cred) + }}) + }) + this.setState({ credentialTypes }) + } catch (err){ + onError(err) + } + } + + async loadCredentials(params) { + const { selectedCredentialType } = this.state; + params.credential_type = selectedCredentialType.id || 1 + return CredentialsAPI.read(params) + } + + handleCredentialTypeSelect(value, type) { + const {credentialTypes} = this.state + const selectedType = credentialTypes.filter(item => item.label === type) + this.setState({selectedCredentialType: selectedType[0]}) + } + + toggleCredential(item) { + const { credentials: stateToUpdate, selectedCredentialType } = this.state; + const { onChange } = this.props; + const index = stateToUpdate.findIndex( + credential => credential.id === item.id + ) + if (index > -1) { + const newCredentialsList = stateToUpdate.filter(cred => cred.id !== item.id) + this.setState({ credentials: newCredentialsList }); + onChange(newCredentialsList) + return; + } + + const credentialTypeOccupied = stateToUpdate.some(cred => cred.kind === item.kind); + if (selectedCredentialType.value === "Vault" || !credentialTypeOccupied ) { + item.credentialType = selectedCredentialType + this.setState({ credentials: [...stateToUpdate, item] }) + onChange([...stateToUpdate, item]) + } else { + const credsList = [...stateToUpdate] + const occupyingCredIndex = stateToUpdate.findIndex(occupyingCred => + occupyingCred.kind === item.kind) + credsList.splice(occupyingCredIndex, 1, item) + this.setState({ credentials: credsList }) + onChange(credsList) + } + } + + render() { + const { tooltip, i18n } = this.props; + const { credentials, selectedCredentialType, credentialTypes } = this.state; + + return ( + + {tooltip && ( + + + + )} + {credentialTypes && ( + {}} + getItems={this.loadCredentials} + qsNamespace="credentials" + columns={[ + { + name: i18n._(t`Name`), + key: 'name', + isSortable: true, + isSearchable: true, + }, + ]} + sortedColumnKey="name" + /> + )} + + ); + } +} + +CredentialsLookup.propTypes = { + tooltip: PropTypes.string, + onChange: PropTypes.func.isRequired, +}; + +CredentialsLookup.defaultProps = { + tooltip: '', +}; +export { CredentialsLookup as _CredentialsLookup }; + +export default withI18n()(withRouter(CredentialsLookup)); diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 874571fcb0..4342bf5fbe 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -15,16 +15,19 @@ import { ButtonVariant, InputGroup as PFInputGroup, Modal, + ToolbarItem, } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import styled from 'styled-components'; +import AnsibleSelect from '../AnsibleSelect' import PaginatedDataList from '../PaginatedDataList'; +import VerticalSeperator from '../VerticalSeparator' import DataListToolbar from '../DataListToolbar'; import CheckboxListItem from '../CheckboxListItem'; import SelectedList from '../SelectedList'; -import { ChipGroup, Chip } from '../Chip'; +import { ChipGroup, Chip, CredentialChip } from '../Chip'; import { getQSConfig, parseQueryString } from '../../util/qs'; const SearchButton = styled(Button)` @@ -65,32 +68,49 @@ class Lookup extends React.Component { results: [], count: 0, error: null, + isDropdownOpen: false }; - this.qsConfig = getQSConfig(props.qsNamespace, { - page: 1, - page_size: 5, - order_by: props.sortedColumnKey, - }); + this.qsConfig = getQSConfig(props.qsNamespace, { + page: 1, + page_size: 5, + order_by: props.sortedColumnKey, + }); this.handleModalToggle = this.handleModalToggle.bind(this); this.toggleSelected = this.toggleSelected.bind(this); 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() { - this.getData(); - } - - componentDidUpdate(prevProps) { - const { location } = this.props; - if (location !== prevProps.location) { + const {onLoadCredentialTypes} = this.props + ; + if (onLoadCredentialTypes) { + Promise.all([onLoadCredentialTypes(), this.getData()]); + } else { this.getData(); } } + componentDidUpdate(prevProps) { + const { location, selectedCategory } = this.props; + if ((location !== prevProps.location) || + (prevProps.selectedCategory !== selectedCategory)) { + this.getData(); + } + } + + toggleDropdown() { + const { isDropdownOpen } = this.state; + this.setState({isDropdownOpen: !isDropdownOpen}) + } + assertCorrectValueType() { - const { multiple, value } = this.props; + const { multiple, value, selectCategoryOptions } = this.props; + if (selectCategoryOptions) { + return + } if (!multiple && Array.isArray(value)) { throw new Error( 'Lookup value must not be an array unless `multiple` is set' @@ -110,30 +130,31 @@ class Lookup extends React.Component { this.setState({ error: false }); try { - const { data } = await getItems(queryParams); + const { data } = await getItems(queryParams); const { results, count } = data; - this.setState({ - results, - count, - }); + this.setState({ + results, + count, + }); } catch (err) { this.setState({ error: true }); } } toggleSelected(row) { - const { name, onLookupSave, multiple } = this.props; + const { name, onLookupSave, multiple, onToggleItem, selectCategoryOptions } = this.props; const { - lookupSelectedItems: updatedSelectedItems, - isModalOpen, + lookupSelectedItems: updatedSelectedItems, isModalOpen } = this.state; const selectedIndex = updatedSelectedItems.findIndex( selectedRow => selectedRow.id === row.id - ); - + ); if (multiple) { + if (selectCategoryOptions) { + onToggleItem(row, isModalOpen) + } if (selectedIndex > -1) { updatedSelectedItems.splice(selectedIndex, 1); this.setState({ lookupSelectedItems: updatedSelectedItems }); @@ -150,13 +171,13 @@ class Lookup extends React.Component { // This handles the case where the user removes chips from the lookup input // while the modal is closed if (!isModalOpen) { - onLookupSave(updatedSelectedItems, name); - } + onLookupSave(updatedSelectedItems, name) + }; } handleModalToggle() { const { isModalOpen } = this.state; - const { value, multiple } = this.props; + const { value, multiple, selectCategory } = this.props; // Resets the selected items from parent state whenever modal is opened // This handles the case where the user closes/cancels the modal and // opens it again @@ -168,6 +189,9 @@ class Lookup extends React.Component { this.setState({ lookupSelectedItems }); } else { this.clearQSParams(); + if (selectCategory) { + selectCategory(null, "Machine"); + } } this.setState(prevState => ({ isModalOpen: !prevState.isModalOpen, @@ -177,11 +201,11 @@ class Lookup extends React.Component { saveModal() { const { onLookupSave, name, multiple } = this.props; const { lookupSelectedItems } = this.state; - const value = multiple - ? lookupSelectedItems - : lookupSelectedItems[0] || null; - onLookupSave(value, name); - this.handleModalToggle(); + const value = multiple ? lookupSelectedItems : lookupSelectedItems[0] || null; + + this.handleModalToggle(); + onLookupSave(value, name); + } clearQSParams() { @@ -201,6 +225,7 @@ class Lookup extends React.Component { count, } = this.state; const { + form, id, lookupHeader, value, @@ -208,27 +233,40 @@ class Lookup extends React.Component { multiple, name, onBlur, + selectCategory, required, i18n, + selectCategoryOptions, + selectedCategory } = this.props; - const header = lookupHeader || i18n._(t`Items`); const canDelete = !required || (multiple && value.length > 1); - - const chips = value ? ( - - {(multiple ? value : [value]).map(chip => ( - this.toggleSelected(chip)} - isReadOnly={!canDelete} - > - {chip.name} - - ))} - - ) : null; - + const chips = () => { + return (selectCategoryOptions && selectCategoryOptions.length > 0) ? ( + + {(multiple ? value : [value]).map(chip => ( + this.toggleSelected(chip)} + isReadOnly={!canDelete} + credential={chip} + /> + ))} + + ) : ( + + {(multiple ? value : [value]).map(chip => ( + this.toggleSelected(chip)} + isReadOnly={!canDelete} + > + {chip.name} + + ))} + + ) + } return ( @@ -240,7 +278,9 @@ class Lookup extends React.Component { > - {chips} + + {value ? chips(value) : null} + , ]} > + {(selectCategoryOptions && selectCategoryOptions.length > 0) && ( + + Selected Category + + + + )} i.id === item.id)} + isSelected={selectCategoryOptions ? value.some(i => i.id === item.id) + : lookupSelectedItems.some(i => i.id === item.id)} onSelect={() => this.toggleSelected(item)} - isRadio={!multiple} + isRadio={!multiple || ((selectCategoryOptions && selectCategoryOptions.length) && selectedCategory.value !== "Vault")} /> )} renderToolbar={props => } @@ -288,10 +336,11 @@ class Lookup extends React.Component { {lookupSelectedItems.length > 0 && ( 0} /> )} {error ?
error
: ''} diff --git a/awx/ui_next/src/components/Lookup/index.js b/awx/ui_next/src/components/Lookup/index.js index 99e10ac27f..5620da1a89 100644 --- a/awx/ui_next/src/components/Lookup/index.js +++ b/awx/ui_next/src/components/Lookup/index.js @@ -2,3 +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'; diff --git a/awx/ui_next/src/components/SelectedList/SelectedList.jsx b/awx/ui_next/src/components/SelectedList/SelectedList.jsx index 6fcaf05939..e773404054 100644 --- a/awx/ui_next/src/components/SelectedList/SelectedList.jsx +++ b/awx/ui_next/src/components/SelectedList/SelectedList.jsx @@ -2,7 +2,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { Split as PFSplit, SplitItem } from '@patternfly/react-core'; import styled from 'styled-components'; -import { ChipGroup, Chip } from '../Chip'; +import { ChipGroup, Chip, CredentialChip } from '../Chip'; import VerticalSeparator from '../VerticalSeparator'; const Split = styled(PFSplit)` @@ -27,22 +27,33 @@ class SelectedList extends Component { onRemove, displayKey, isReadOnly, + isCredentialList } = this.props; + const chips = isCredentialList ? selected.map(item => ( + onRemove(item)} + credential={item} + > + {item[displayKey]} + + )) : selected.map(item => ( + onRemove(item)} + > + {item[displayKey]} + + )) return ( {label} - {selected.map(item => ( - onRemove(item)} - > - {item[displayKey]} - - ))} + {chips} diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index 0623ad7ab8..9d25ad3fc7 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -22,6 +22,7 @@ function JobTemplateAdd({ history, i18n }) { organizationId, instanceGroups, initialInstanceGroups, + credentials, ...remainingValues } = values; @@ -33,6 +34,7 @@ function JobTemplateAdd({ history, i18n }) { await Promise.all([ submitLabels(id, labels, organizationId), submitInstanceGroups(id, instanceGroups), + submitCredentials(id, credentials) ]); history.push(`/templates/${type}/${id}/details`); } catch (error) { @@ -60,6 +62,13 @@ function JobTemplateAdd({ history, i18n }) { return Promise.all(associatePromises); } + function submitCredentials(templateId, credentials = []) { + const associateCredentials = credentials.map(cred => + JobTemplatesAPI.associateCredentials(templateId, cred.id) + ) + return Promise.all(associateCredentials) + } + function handleCancel() { history.push(`/templates`); } diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index a0cf904ab2..5272a46149 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -109,6 +109,8 @@ class JobTemplateEdit extends Component { organizationId, instanceGroups, initialInstanceGroups, + credentials, + initialCredentials, ...remainingValues } = values; @@ -118,6 +120,7 @@ class JobTemplateEdit extends Component { await Promise.all([ this.submitLabels(labels, organizationId), this.submitInstanceGroups(instanceGroups, initialInstanceGroups), + this.submitCredentials(credentials) ]); history.push(this.detailsUrl); } catch (formSubmitError) { @@ -154,13 +157,30 @@ class JobTemplateEdit extends Component { async submitInstanceGroups(groups, initialGroups) { const { template } = this.props; const { added, removed } = getAddedAndRemoved(initialGroups, groups); - const associatePromises = added.map(group => - JobTemplatesAPI.associateInstanceGroup(template.id, group.id) - ); - const disassociatePromises = removed.map(group => + const disassociatePromises = await removed.map(group => JobTemplatesAPI.disassociateInstanceGroup(template.id, group.id) ); - return Promise.all([...associatePromises, ...disassociatePromises]); + const associatePromises = await added.map(group => + JobTemplatesAPI.associateInstanceGroup(template.id, group.id) + ); + return Promise.all([...disassociatePromises, ...associatePromises, ]); + } + + async submitCredentials(newCredentials) { + const { template } = this.props; + const { added, removed } = getAddedAndRemoved( + template.summary_fields.credentials, + newCredentials + ); + const disassociateCredentials = removed.map(cred => + JobTemplatesAPI.disassociateCredentials(template.id, cred.id) + ); + const disassociatePromise = await Promise.all(disassociateCredentials); + const associateCredentials = added.map(cred => + JobTemplatesAPI.associateCredentials(template.id, cred.id) + ) + const associatePromise = Promise.all(associateCredentials) + return Promise.all([disassociatePromise, associatePromise]) } handleCancel() { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index bffdf85cef..df37d1319a 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -27,6 +27,7 @@ import { InventoryLookup, InstanceGroupsLookup, ProjectLookup, + CredentialsLookup } from '@components/Lookup'; import { JobTemplatesAPI } from '@api'; import LabelSelect from './LabelSelect'; @@ -62,6 +63,7 @@ class JobTemplateForm extends Component { inventory: null, labels: { results: [] }, project: null, + credentials: [], }, isNew: true, }, @@ -84,7 +86,8 @@ class JobTemplateForm extends Component { const { validateField } = this.props; this.setState({ contentError: null, hasContentLoading: true }); // TODO: determine when LabelSelect has finished loading labels - Promise.all([this.loadRelatedInstanceGroups()]).then(() => { + Promise.all([this.loadRelatedInstanceGroups(), + ]).then(() => { this.setState({ hasContentLoading: false }); validateField('project'); }); @@ -149,7 +152,6 @@ class JobTemplateForm extends Component { isDisabled: false, }, ]; - const verbosityOptions = [ { value: '0', key: '0', label: i18n._(t`0 (Normal)`) }, { value: '1', key: '1', label: i18n._(t`1 (Verbose)`) }, @@ -319,6 +321,20 @@ class JobTemplateForm extends Component { )} /> + + ( + this.setState({ contentError: err })} + credentials={template.summary_fields.credentials} + onChange={value => (form.setFieldValue('credentials', value))} + 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.`)} + /> + )} + /> + props.handleSubmit(values), From 2828d31141df0557d76ba7c48ea7f44db3d9683a Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 22 Oct 2019 16:20:59 -0400 Subject: [PATCH 2/7] Adds CredentialLookUp to JT Form --- awx/ui_next/src/api/index.js | 4 +- awx/ui_next/src/api/models/JobTemplates.js | 9 +- .../components/Lookup/CredentialsLookup.jsx | 132 ++++++++++-------- awx/ui_next/src/components/Lookup/Lookup.jsx | 107 ++++++++------ .../components/SelectedList/SelectedList.jsx | 44 +++--- .../JobTemplateAdd/JobTemplateAdd.jsx | 6 +- .../JobTemplateEdit/JobTemplateEdit.jsx | 10 +- .../Template/shared/JobTemplateForm.jsx | 13 +- 8 files changed, 183 insertions(+), 142 deletions(-) diff --git a/awx/ui_next/src/api/index.js b/awx/ui_next/src/api/index.js index 9d76cc8bd8..526cb7c35c 100644 --- a/awx/ui_next/src/api/index.js +++ b/awx/ui_next/src/api/index.js @@ -1,7 +1,7 @@ import AdHocCommands from './models/AdHocCommands'; import Config from './models/Config'; -import CredentialTypes from './models/CredentialTypes' -import Credentials from './models/Credentials' +import CredentialTypes from './models/CredentialTypes'; +import Credentials from './models/Credentials'; import InstanceGroups from './models/InstanceGroups'; import Inventories from './models/Inventories'; import InventorySources from './models/InventorySources'; diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index 9941ad220d..e84df42ec1 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -46,11 +46,16 @@ class JobTemplates extends InstanceGroupsMixin(NotificationsMixin(Base)) { } associateCredentials(id, credential) { - return this.http.post(`${this.baseUrl}${id}/credentials/`, { id: credential }); + return this.http.post(`${this.baseUrl}${id}/credentials/`, { + id: credential, + }); } disassociateCredentials(id, credential) { - return this.http.post(`${this.baseUrl}${id}/credentials/`, { id: credential, disassociate: true }); + return this.http.post(`${this.baseUrl}${id}/credentials/`, { + id: credential, + disassociate: true, + }); } } diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx index 1648d7d910..a57b3a76eb 100644 --- a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx @@ -16,67 +16,73 @@ const QuestionCircleIcon = styled(PFQuestionCircleIcon)` class CredentialsLookup extends React.Component { constructor(props) { - super(props) + super(props); this.state = { credentials: props.credentials, - selectedCredentialType: {label: "Machine", id: 1, kind: 'ssh'}, - credentialTypes: [] - } + selectedCredentialType: { label: 'Machine', id: 1, kind: 'ssh' }, + credentialTypes: [], + }; - this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind(this); + this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind( + this + ); this.loadCredentials = this.loadCredentials.bind(this); this.loadCredentialTypes = this.loadCredentialTypes.bind(this); this.toggleCredential = this.toggleCredential.bind(this); } componentDidMount() { - this.loadCredentials({page: 1, page_size: 5, order_by:'name'}) + this.loadCredentials({ page: 1, page_size: 5, order_by: 'name' }); this.loadCredentialTypes(); } componentDidUpdate(prevState) { - const {selectedType} = this.state + const { selectedType } = this.state; if (prevState.selectedType !== selectedType) { Promise.all([this.loadCredentials()]); } } async loadCredentialTypes() { - const {onError} =this.props + const { onError } = this.props; try { - const { data } = await CredentialTypesAPI.read() - const acceptableTypes = ["machine", "cloud", "net", "ssh", "vault"] - const credentialTypes =[] - data.results - .forEach(cred => { - acceptableTypes.forEach(aT => { - if (aT === cred.kind) { - // This object has several repeated values as some of it's children - // require different field values. - cred = { - id: cred.id, kind: cred.kind, type: cred.namespace, - value: cred.name, label: cred.name, isDisabled: false - } - credentialTypes.push(cred) - }}) - }) - this.setState({ credentialTypes }) - } catch (err){ - onError(err) - } - } + const { data } = await CredentialTypesAPI.read(); + const acceptableTypes = ['machine', 'cloud', 'net', 'ssh', 'vault']; + const credentialTypes = []; + data.results.forEach(cred => { + acceptableTypes.forEach(aT => { + if (aT === cred.kind) { + // This object has several repeated values as some of it's children + // require different field values. + cred = { + id: cred.id, + kind: cred.kind, + type: cred.namespace, + value: cred.name, + label: cred.name, + isDisabled: false, + }; + credentialTypes.push(cred); + } + }); + }); + this.setState({ credentialTypes }); + } catch (err) { + onError(err); + } + } async loadCredentials(params) { const { selectedCredentialType } = this.state; - params.credential_type = selectedCredentialType.id || 1 - return CredentialsAPI.read(params) + params.credential_type = selectedCredentialType.id || 1; + return CredentialsAPI.read(params); } handleCredentialTypeSelect(value, type) { - const {credentialTypes} = this.state - const selectedType = credentialTypes.filter(item => item.label === type) - this.setState({selectedCredentialType: selectedType[0]}) + const { credentialTypes } = this.state; + const selectedType = credentialTypes.filter(item => item.label === type); + this.setState({ selectedCredentialType: selectedType[0] }); } toggleCredential(item) { @@ -84,26 +90,31 @@ class CredentialsLookup extends React.Component { const { onChange } = this.props; const index = stateToUpdate.findIndex( credential => credential.id === item.id - ) + ); if (index > -1) { - const newCredentialsList = stateToUpdate.filter(cred => cred.id !== item.id) + const newCredentialsList = stateToUpdate.filter( + cred => cred.id !== item.id + ); this.setState({ credentials: newCredentialsList }); - onChange(newCredentialsList) + onChange(newCredentialsList); return; } - const credentialTypeOccupied = stateToUpdate.some(cred => cred.kind === item.kind); - if (selectedCredentialType.value === "Vault" || !credentialTypeOccupied ) { - item.credentialType = selectedCredentialType - this.setState({ credentials: [...stateToUpdate, item] }) - onChange([...stateToUpdate, item]) + const credentialTypeOccupied = stateToUpdate.some( + cred => cred.kind === item.kind + ); + if (selectedCredentialType.value === 'Vault' || !credentialTypeOccupied) { + item.credentialType = selectedCredentialType; + this.setState({ credentials: [...stateToUpdate, item] }); + onChange([...stateToUpdate, item]); } else { - const credsList = [...stateToUpdate] - const occupyingCredIndex = stateToUpdate.findIndex(occupyingCred => - occupyingCred.kind === item.kind) - credsList.splice(occupyingCredIndex, 1, item) - this.setState({ credentials: credsList }) - onChange(credsList) + const credsList = [...stateToUpdate]; + const occupyingCredIndex = stateToUpdate.findIndex( + occupyingCred => occupyingCred.kind === item.kind + ); + credsList.splice(occupyingCredIndex, 1, item); + this.setState({ credentials: credsList }); + onChange(credsList); } } @@ -112,21 +123,18 @@ class CredentialsLookup extends React.Component { const { credentials, selectedCredentialType, credentialTypes } = this.state; return ( - - {tooltip && ( - - - - )} - {credentialTypes && ( - + {tooltip && ( + + + + )} + {credentialTypes && ( + - )} - + )} + ); } } diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 4342bf5fbe..599a0576ae 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -21,9 +21,9 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import styled from 'styled-components'; -import AnsibleSelect from '../AnsibleSelect' +import AnsibleSelect from '../AnsibleSelect'; import PaginatedDataList from '../PaginatedDataList'; -import VerticalSeperator from '../VerticalSeparator' +import VerticalSeperator from '../VerticalSeparator'; import DataListToolbar from '../DataListToolbar'; import CheckboxListItem from '../CheckboxListItem'; import SelectedList from '../SelectedList'; @@ -68,13 +68,13 @@ class Lookup extends React.Component { results: [], count: 0, error: null, - isDropdownOpen: false + isDropdownOpen: false, }; - this.qsConfig = getQSConfig(props.qsNamespace, { - page: 1, - page_size: 5, - order_by: props.sortedColumnKey, - }); + this.qsConfig = getQSConfig(props.qsNamespace, { + page: 1, + page_size: 5, + order_by: props.sortedColumnKey, + }); this.handleModalToggle = this.handleModalToggle.bind(this); this.toggleSelected = this.toggleSelected.bind(this); this.saveModal = this.saveModal.bind(this); @@ -84,8 +84,7 @@ class Lookup extends React.Component { } componentDidMount() { - const {onLoadCredentialTypes} = this.props - ; + const { onLoadCredentialTypes } = this.props; if (onLoadCredentialTypes) { Promise.all([onLoadCredentialTypes(), this.getData()]); } else { @@ -95,21 +94,23 @@ class Lookup extends React.Component { componentDidUpdate(prevProps) { const { location, selectedCategory } = this.props; - if ((location !== prevProps.location) || - (prevProps.selectedCategory !== selectedCategory)) { + if ( + location !== prevProps.location || + prevProps.selectedCategory !== selectedCategory + ) { this.getData(); } } toggleDropdown() { const { isDropdownOpen } = this.state; - this.setState({isDropdownOpen: !isDropdownOpen}) + this.setState({ isDropdownOpen: !isDropdownOpen }); } assertCorrectValueType() { const { multiple, value, selectCategoryOptions } = this.props; if (selectCategoryOptions) { - return + return; } if (!multiple && Array.isArray(value)) { throw new Error( @@ -130,30 +131,37 @@ class Lookup extends React.Component { this.setState({ error: false }); try { - const { data } = await getItems(queryParams); + const { data } = await getItems(queryParams); const { results, count } = data; - this.setState({ - results, - count, - }); + this.setState({ + results, + count, + }); } catch (err) { this.setState({ error: true }); } } toggleSelected(row) { - const { name, onLookupSave, multiple, onToggleItem, selectCategoryOptions } = this.props; const { - lookupSelectedItems: updatedSelectedItems, isModalOpen + name, + onLookupSave, + multiple, + onToggleItem, + selectCategoryOptions, + } = this.props; + const { + lookupSelectedItems: updatedSelectedItems, + isModalOpen, } = this.state; const selectedIndex = updatedSelectedItems.findIndex( selectedRow => selectedRow.id === row.id - ); + ); if (multiple) { if (selectCategoryOptions) { - onToggleItem(row, isModalOpen) + onToggleItem(row, isModalOpen); } if (selectedIndex > -1) { updatedSelectedItems.splice(selectedIndex, 1); @@ -171,8 +179,8 @@ class Lookup extends React.Component { // This handles the case where the user removes chips from the lookup input // while the modal is closed if (!isModalOpen) { - onLookupSave(updatedSelectedItems, name) - }; + onLookupSave(updatedSelectedItems, name); + } } handleModalToggle() { @@ -190,7 +198,7 @@ class Lookup extends React.Component { } else { this.clearQSParams(); if (selectCategory) { - selectCategory(null, "Machine"); + selectCategory(null, 'Machine'); } } this.setState(prevState => ({ @@ -201,11 +209,12 @@ class Lookup extends React.Component { saveModal() { const { onLookupSave, name, multiple } = this.props; const { lookupSelectedItems } = this.state; - const value = multiple ? lookupSelectedItems : lookupSelectedItems[0] || null; - - this.handleModalToggle(); - onLookupSave(value, name); + const value = multiple + ? lookupSelectedItems + : lookupSelectedItems[0] || null; + this.handleModalToggle(); + onLookupSave(value, name); } clearQSParams() { @@ -237,12 +246,12 @@ class Lookup extends React.Component { required, i18n, selectCategoryOptions, - selectedCategory + selectedCategory, } = this.props; const header = lookupHeader || i18n._(t`Items`); const canDelete = !required || (multiple && value.length > 1); const chips = () => { - return (selectCategoryOptions && selectCategoryOptions.length > 0) ? ( + return selectCategoryOptions && selectCategoryOptions.length > 0 ? ( {(multiple ? value : [value]).map(chip => ( ))} - ) - } + ); + }; return ( @@ -305,11 +314,19 @@ class Lookup extends React.Component { , ]} > - {(selectCategoryOptions && selectCategoryOptions.length > 0) && ( + {selectCategoryOptions && selectCategoryOptions.length > 0 && ( Selected Category - + )} i.id === item.id) - : lookupSelectedItems.some(i => i.id === item.id)} + isSelected={ + selectCategoryOptions + ? value.some(i => i.id === item.id) + : lookupSelectedItems.some(i => i.id === item.id) + } onSelect={() => this.toggleSelected(item)} - isRadio={!multiple || ((selectCategoryOptions && selectCategoryOptions.length) && selectedCategory.value !== "Vault")} + isRadio={ + !multiple || + (selectCategoryOptions && + selectCategoryOptions.length && + selectedCategory.value !== 'Vault') + } /> )} renderToolbar={props => } @@ -340,7 +365,9 @@ class Lookup extends React.Component { showOverflowAfter={5} onRemove={this.toggleSelected} isReadOnly={!canDelete} - isCredentialList={selectCategoryOptions && selectCategoryOptions.length > 0} + isCredentialList={ + selectCategoryOptions && selectCategoryOptions.length > 0 + } /> )} {error ?
error
: ''} diff --git a/awx/ui_next/src/components/SelectedList/SelectedList.jsx b/awx/ui_next/src/components/SelectedList/SelectedList.jsx index e773404054..661eeec382 100644 --- a/awx/ui_next/src/components/SelectedList/SelectedList.jsx +++ b/awx/ui_next/src/components/SelectedList/SelectedList.jsx @@ -27,34 +27,34 @@ class SelectedList extends Component { onRemove, displayKey, isReadOnly, - isCredentialList + isCredentialList, } = this.props; - const chips = isCredentialList ? selected.map(item => ( - onRemove(item)} - credential={item} - > - {item[displayKey]} - - )) : selected.map(item => ( - onRemove(item)} - > - {item[displayKey]} - - )) + const chips = isCredentialList + ? selected.map(item => ( + onRemove(item)} + credential={item} + > + {item[displayKey]} + + )) + : selected.map(item => ( + onRemove(item)} + > + {item[displayKey]} + + )); return ( {label} - - {chips} - + {chips} ); diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index 9d25ad3fc7..979c2f9728 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -34,7 +34,7 @@ function JobTemplateAdd({ history, i18n }) { await Promise.all([ submitLabels(id, labels, organizationId), submitInstanceGroups(id, instanceGroups), - submitCredentials(id, credentials) + submitCredentials(id, credentials), ]); history.push(`/templates/${type}/${id}/details`); } catch (error) { @@ -65,8 +65,8 @@ function JobTemplateAdd({ history, i18n }) { function submitCredentials(templateId, credentials = []) { const associateCredentials = credentials.map(cred => JobTemplatesAPI.associateCredentials(templateId, cred.id) - ) - return Promise.all(associateCredentials) + ); + return Promise.all(associateCredentials); } function handleCancel() { diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 5272a46149..b6cfc65fd6 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -120,7 +120,7 @@ class JobTemplateEdit extends Component { await Promise.all([ this.submitLabels(labels, organizationId), this.submitInstanceGroups(instanceGroups, initialInstanceGroups), - this.submitCredentials(credentials) + this.submitCredentials(credentials), ]); history.push(this.detailsUrl); } catch (formSubmitError) { @@ -163,7 +163,7 @@ class JobTemplateEdit extends Component { const associatePromises = await added.map(group => JobTemplatesAPI.associateInstanceGroup(template.id, group.id) ); - return Promise.all([...disassociatePromises, ...associatePromises, ]); + return Promise.all([...disassociatePromises, ...associatePromises]); } async submitCredentials(newCredentials) { @@ -178,9 +178,9 @@ class JobTemplateEdit extends Component { const disassociatePromise = await Promise.all(disassociateCredentials); const associateCredentials = added.map(cred => JobTemplatesAPI.associateCredentials(template.id, cred.id) - ) - const associatePromise = Promise.all(associateCredentials) - return Promise.all([disassociatePromise, associatePromise]) + ); + const associatePromise = Promise.all(associateCredentials); + return Promise.all([disassociatePromise, associatePromise]); } handleCancel() { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index df37d1319a..310db6f5be 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 + CredentialsLookup, } from '@components/Lookup'; import { JobTemplatesAPI } from '@api'; import LabelSelect from './LabelSelect'; @@ -86,8 +86,7 @@ class JobTemplateForm extends Component { const { validateField } = this.props; this.setState({ contentError: null, hasContentLoading: true }); // TODO: determine when LabelSelect has finished loading labels - Promise.all([this.loadRelatedInstanceGroups(), - ]).then(() => { + Promise.all([this.loadRelatedInstanceGroups()]).then(() => { this.setState({ hasContentLoading: false }); validateField('project'); }); @@ -329,8 +328,10 @@ class JobTemplateForm extends Component { this.setState({ contentError: err })} credentials={template.summary_fields.credentials} - onChange={value => (form.setFieldValue('credentials', value))} - 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.`)} + onChange={value => form.setFieldValue('credentials', value)} + 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.` + )} /> )} /> @@ -604,7 +605,7 @@ const FormikApp = withFormik({ initialInstanceGroups: [], instanceGroups: [], initialCredentials: summary_fields.credentials || [], - credentials: [] + credentials: [], }; }, handleSubmit: (values, { props }) => props.handleSubmit(values), From 91721e09df746e281cfdc8b754a18902e7492608 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 22 Oct 2019 16:21:20 -0400 Subject: [PATCH 3/7] Adds tests --- .../Lookup/CredentialsLookup.test.jsx | 103 ++++++++++++++++++ .../JobTemplateEdit/JobTemplateEdit.test.jsx | 4 + .../Template/shared/JobTemplateForm.test.jsx | 4 + 3 files changed, 111 insertions(+) create mode 100644 awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx b/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx new file mode 100644 index 0000000000..c0cb0d19fb --- /dev/null +++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx @@ -0,0 +1,103 @@ +import React from 'react'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import CredentialsLookup, { _CredentialsLookup } from './CredentialsLookup'; +import { CredentialsAPI, CredentialTypesAPI } from '@api'; + +jest.mock('@api'); + +describe('', () => { + let wrapper; + let lookup; + let credLookup; + + const credentials = [ + { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, + { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' }, + ]; + beforeEach(() => { + CredentialTypesAPI.read.mockResolvedValue({ + data: { + results: [ + { + id: 400, + kind: 'ssh', + namespace: 'biz', + name: 'Amazon Web Services', + }, + { id: 500, kind: 'vault', namespace: 'buzz', name: 'Vault' }, + { id: 600, kind: 'machine', namespace: 'fuzz', name: 'Machine' }, + ], + count: 2, + }, + }); + CredentialsAPI.read.mockResolvedValueOnce({ + data: { + results: [ + { id: 1, kind: 'cloud', name: 'Cred 1', url: 'www.google.com' }, + { id: 2, kind: 'ssh', name: 'Cred 2', url: 'www.google.com' }, + { id: 3, kind: 'Ansible', name: 'Cred 3', url: 'www.google.com' }, + { id: 4, kind: 'Machine', name: 'Cred 4', url: 'www.google.com' }, + { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' }, + ], + count: 3, + }, + }); + + wrapper = mountWithContexts( + {}} + credentials={credentials} + onChange={() => {}} + tooltip="This is credentials look up" + /> + ); + lookup = wrapper.find('Lookup'); + credLookup = wrapper.find('CredentialsLookup'); + }); + + afterEach(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + + test('CredentialsLookup renders properly', () => { + expect(wrapper.find('CredentialsLookup')).toHaveLength(1); + }); + test('Removes credential from directly from input', () => { + const chip = wrapper.find('PFChip').at(0); + expect(chip).toHaveLength(1); + chip.find('ChipButton').invoke('onClick')(); + expect(wrapper.find('PFChip')).toHaveLength(1); + }); + test('can change credential types', async () => { + lookup.prop('selectCategory')({}, 'Vault'); + expect(credLookup.state('selectedCredentialType')).toEqual({ + id: 500, + kind: 'vault', + type: 'buzz', + value: 'Vault', + label: 'Vault', + isDisabled: false, + }); + expect(CredentialsAPI.read).toHaveBeenCalled(); + }); + test('Toggle credentials properly adds credentials', async () => { + function callOnToggle(item, index) { + lookup.prop('onToggleItem')(item); + expect(credLookup.state('credentials')[index].name).toEqual( + `${item.name}` + ); + } + callOnToggle({ name: 'Gatsby', id: 8, kind: 'Machine' }, 2); + callOnToggle({ name: 'Party', id: 9, kind: 'Machine' }, 2); + callOnToggle({ name: 'Animal', id: 10, kind: 'Ansible' }, 3); + + lookup.prop('onToggleItem')({ + id: 1, + kind: 'cloud', + name: 'Foo', + url: 'www.google.com', + }); + expect(credLookup.state('credentials').length).toBe(3); + }); +}); diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index 6d6c1e4083..dc5c22f4ee 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -40,6 +40,10 @@ const mockJobTemplate = { id: 2, organization_id: 1, }, + credentials: [ + { id: 1, kind: 'cloud', name: 'Foo' }, + { id: 2, kind: 'ssh', name: 'Bar' }, + ], }, }; 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 071edaecf8..2cac31bea9 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -28,6 +28,10 @@ describe('', () => { name: 'qux', }, labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, + credentials: [ + { id: 1, kind: 'cloud', name: 'Foo' }, + { id: 2, kind: 'ssh', name: 'Bar' }, + ], }, }; const mockInstanceGroups = [ From 491f4824b07e36cd5fca4e15204918aeda3977dd Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 23 Oct 2019 09:59:23 -0400 Subject: [PATCH 4/7] Addresses PR Issues Improves credential ID variable in JT model. Removes unused prop from Lookup ComponentDidMount. Removed unused function call from Credentials ComponentDidMount. Streamlines toggleCredential function and moves it to JobTemplateForm. This was done because the JobTemplateForm should handle the credential values passed to the CredentialsLookup. Adds tests for JobTemplateForm to ensure toggleCredentialSelection function is putting proper values in state. Removed withRouter wrapper on CredentialsLookup export. Improved CredentialsLookup test to ensure that onChange is called when user removes a credential from the input. --- awx/ui_next/src/api/models/JobTemplates.js | 8 +-- .../components/Lookup/CredentialsLookup.jsx | 62 ++--------------- .../Lookup/CredentialsLookup.test.jsx | 43 +++++------- awx/ui_next/src/components/Lookup/Lookup.jsx | 7 +- .../JobTemplateEdit/JobTemplateEdit.jsx | 1 - .../Template/shared/JobTemplateForm.jsx | 37 +++++++++-- .../Template/shared/JobTemplateForm.test.jsx | 66 ++++++++++++++++++- 7 files changed, 124 insertions(+), 100 deletions(-) diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index e84df42ec1..1587f6c86b 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -45,15 +45,15 @@ class JobTemplates extends InstanceGroupsMixin(NotificationsMixin(Base)) { return this.http.get(`${this.baseUrl}${id}/credentials/`, { params }); } - associateCredentials(id, credential) { + associateCredentials(id, credentialId) { return this.http.post(`${this.baseUrl}${id}/credentials/`, { - id: credential, + id: credentialId, }); } - disassociateCredentials(id, credential) { + disassociateCredentials(id, credentialId) { return this.http.post(`${this.baseUrl}${id}/credentials/`, { - id: credential, + id: credentialId, disassociate: true, }); } diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx index a57b3a76eb..77ae10570a 100644 --- a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { withI18n } from '@lingui/react'; -import { withRouter } from 'react-router-dom'; import { t } from '@lingui/macro'; import { FormGroup, Tooltip } from '@patternfly/react-core'; import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; @@ -19,31 +18,18 @@ class CredentialsLookup extends React.Component { super(props); this.state = { - credentials: props.credentials, selectedCredentialType: { label: 'Machine', id: 1, kind: 'ssh' }, credentialTypes: [], }; - - this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind( - this - ); - this.loadCredentials = this.loadCredentials.bind(this); this.loadCredentialTypes = this.loadCredentialTypes.bind(this); - this.toggleCredential = this.toggleCredential.bind(this); + this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind(this); + this.loadCredentials = this.loadCredentials.bind(this); } componentDidMount() { - this.loadCredentials({ page: 1, page_size: 5, order_by: 'name' }); this.loadCredentialTypes(); } - componentDidUpdate(prevState) { - const { selectedType } = this.state; - if (prevState.selectedType !== selectedType) { - Promise.all([this.loadCredentials()]); - } - } - async loadCredentialTypes() { const { onError } = this.props; try { @@ -57,6 +43,7 @@ class CredentialsLookup extends React.Component { // require different field values. cred = { id: cred.id, + key: cred.id, kind: cred.kind, type: cred.namespace, value: cred.name, @@ -85,43 +72,9 @@ class CredentialsLookup extends React.Component { this.setState({ selectedCredentialType: selectedType[0] }); } - toggleCredential(item) { - const { credentials: stateToUpdate, selectedCredentialType } = this.state; - const { onChange } = this.props; - const index = stateToUpdate.findIndex( - credential => credential.id === item.id - ); - if (index > -1) { - const newCredentialsList = stateToUpdate.filter( - cred => cred.id !== item.id - ); - this.setState({ credentials: newCredentialsList }); - onChange(newCredentialsList); - return; - } - - const credentialTypeOccupied = stateToUpdate.some( - cred => cred.kind === item.kind - ); - if (selectedCredentialType.value === 'Vault' || !credentialTypeOccupied) { - item.credentialType = selectedCredentialType; - this.setState({ credentials: [...stateToUpdate, item] }); - onChange([...stateToUpdate, item]); - } else { - const credsList = [...stateToUpdate]; - const occupyingCredIndex = stateToUpdate.findIndex( - occupyingCred => occupyingCred.kind === item.kind - ); - credsList.splice(occupyingCredIndex, 1, item); - this.setState({ credentials: credsList }); - onChange(credsList); - } - } - render() { - const { tooltip, i18n } = this.props; - const { credentials, selectedCredentialType, credentialTypes } = this.state; - + const { selectedCredentialType, credentialTypes } = this.state; + const { tooltip, i18n, credentials, onChange } = this.props; return ( {tooltip && ( @@ -134,7 +87,7 @@ class CredentialsLookup extends React.Component { selectCategoryOptions={credentialTypes} selectCategory={this.handleCredentialTypeSelect} selectedCategory={selectedCredentialType} - onToggleItem={this.toggleCredential} + onToggleItem={onChange} onloadCategories={this.loadCredentialTypes} id="org-credentials" lookupHeader={i18n._(t`Credentials`)} @@ -162,7 +115,6 @@ class CredentialsLookup extends React.Component { CredentialsLookup.propTypes = { tooltip: PropTypes.string, - onChange: PropTypes.func.isRequired, }; CredentialsLookup.defaultProps = { @@ -170,4 +122,4 @@ CredentialsLookup.defaultProps = { }; export { CredentialsLookup as _CredentialsLookup }; -export default withI18n()(withRouter(CredentialsLookup)); +export default withI18n()(CredentialsLookup); diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx b/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx index c0cb0d19fb..9831565b94 100644 --- a/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; -import CredentialsLookup, { _CredentialsLookup } from './CredentialsLookup'; +import CredentialsLookup from './CredentialsLookup'; import { CredentialsAPI, CredentialTypesAPI } from '@api'; jest.mock('@api'); @@ -9,6 +9,7 @@ describe('', () => { let wrapper; let lookup; let credLookup; + let onChange; const credentials = [ { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, @@ -42,12 +43,12 @@ describe('', () => { count: 3, }, }); - + onChange = jest.fn(); wrapper = mountWithContexts( {}} credentials={credentials} - onChange={() => {}} + onChange={onChange} tooltip="This is credentials look up" /> ); @@ -62,17 +63,22 @@ describe('', () => { test('CredentialsLookup renders properly', () => { expect(wrapper.find('CredentialsLookup')).toHaveLength(1); + expect(CredentialTypesAPI.read).toHaveBeenCalled(); }); - test('Removes credential from directly from input', () => { - const chip = wrapper.find('PFChip').at(0); - expect(chip).toHaveLength(1); - chip.find('ChipButton').invoke('onClick')(); - expect(wrapper.find('PFChip')).toHaveLength(1); + + test('onChange is called when you click to remove a credential from input', () => { + const chip = wrapper.find('PFChip'); + const button = chip.at(1).find('Button'); + expect(chip).toHaveLength(2); + button.prop('onClick')(); + expect(onChange).toBeCalledTimes(1); }); - test('can change credential types', async () => { + + test('can change credential types', () => { lookup.prop('selectCategory')({}, 'Vault'); expect(credLookup.state('selectedCredentialType')).toEqual({ id: 500, + key: 500, kind: 'vault', type: 'buzz', value: 'Vault', @@ -81,23 +87,4 @@ describe('', () => { }); expect(CredentialsAPI.read).toHaveBeenCalled(); }); - test('Toggle credentials properly adds credentials', async () => { - function callOnToggle(item, index) { - lookup.prop('onToggleItem')(item); - expect(credLookup.state('credentials')[index].name).toEqual( - `${item.name}` - ); - } - callOnToggle({ name: 'Gatsby', id: 8, kind: 'Machine' }, 2); - callOnToggle({ name: 'Party', id: 9, kind: 'Machine' }, 2); - callOnToggle({ name: 'Animal', id: 10, kind: 'Ansible' }, 3); - - lookup.prop('onToggleItem')({ - id: 1, - kind: 'cloud', - name: 'Foo', - url: 'www.google.com', - }); - expect(credLookup.state('credentials').length).toBe(3); - }); }); diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 599a0576ae..1895fd8ac7 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -84,12 +84,7 @@ class Lookup extends React.Component { } componentDidMount() { - const { onLoadCredentialTypes } = this.props; - if (onLoadCredentialTypes) { - Promise.all([onLoadCredentialTypes(), this.getData()]); - } else { - this.getData(); - } + Promise.all([this.getData()]); } componentDidUpdate(prevProps) { diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index b6cfc65fd6..eb30892719 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -110,7 +110,6 @@ class JobTemplateEdit extends Component { instanceGroups, initialInstanceGroups, credentials, - initialCredentials, ...remainingValues } = values; diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 310db6f5be..0c8b6d1225 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -77,9 +77,11 @@ 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() { @@ -106,6 +108,31 @@ 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; @@ -127,6 +154,7 @@ class JobTemplateForm extends Component { inventory, project, allowCallbacks, + credentials, } = this.state; const { handleCancel, @@ -324,11 +352,11 @@ class JobTemplateForm extends Component { ( + render={() => ( this.setState({ contentError: err })} - credentials={template.summary_fields.credentials} - onChange={value => form.setFieldValue('credentials', value)} 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.` )} @@ -604,8 +632,7 @@ const FormikApp = withFormik({ organizationId: summary_fields.inventory.organization_id || null, initialInstanceGroups: [], instanceGroups: [], - initialCredentials: summary_fields.credentials || [], - credentials: [], + credentials: summary_fields.credentials || [], }; }, handleSubmit: (values, { props }) => props.handleSubmit(values), 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 2cac31bea9..c939e678d1 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -3,7 +3,13 @@ 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 } from '@api'; +import { + LabelsAPI, + JobTemplatesAPI, + ProjectsAPI, + CredentialTypesAPI, + CredentialsAPI, +} from '@api'; jest.mock('@api'); @@ -59,10 +65,21 @@ describe('', () => { policy_instance_list: [], }, ]; + const mockCredentials = [ + { id: 1, kind: 'cloud', name: 'Cred 1', url: 'www.google.com' }, + { id: 2, kind: 'ssh', name: 'Cred 2', url: 'www.google.com' }, + { id: 3, kind: 'Ansible', name: 'Cred 3', url: 'www.google.com' }, + { id: 4, kind: 'Machine', name: 'Cred 4', url: 'www.google.com' }, + { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' }, + ]; + beforeEach(() => { LabelsAPI.read.mockReturnValue({ data: mockData.summary_fields.labels, }); + CredentialsAPI.read.mockReturnValue({ + data: { results: mockCredentials }, + }); JobTemplatesAPI.readInstanceGroups.mockReturnValue({ data: { results: mockInstanceGroups }, }); @@ -138,6 +155,13 @@ describe('', () => { target: { value: 'new baz type', name: 'playbook' }, }); expect(form.state('values').playbook).toEqual('new baz type'); + wrapper + .find('CredentialChip') + .at(0) + .prop('onClick')(); + expect(form.state('values').credentials).toEqual([ + { id: 2, kind: 'ssh', name: 'Bar' }, + ]); }); test('should call handleSubmit when Submit button is clicked', async () => { @@ -176,4 +200,44 @@ 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' }, + ]); + }); }); From 53b4dd5dbf7de76ded3c9576560905ff2f197e40 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 24 Oct 2019 12:35:30 -0400 Subject: [PATCH 5/7] Fixes linting errors --- awx/ui_next/src/components/Lookup/CredentialsLookup.jsx | 4 +++- awx/ui_next/src/components/Lookup/Lookup.jsx | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx index 77ae10570a..b3650a594f 100644 --- a/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialsLookup.jsx @@ -22,7 +22,9 @@ class CredentialsLookup extends React.Component { credentialTypes: [], }; this.loadCredentialTypes = this.loadCredentialTypes.bind(this); - this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind(this); + this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind( + this + ); this.loadCredentials = this.loadCredentials.bind(this); } diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 1895fd8ac7..da8cd3d98d 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -84,7 +84,7 @@ class Lookup extends React.Component { } componentDidMount() { - Promise.all([this.getData()]); + Promise.all([this.getData()]); } componentDidUpdate(prevProps) { From 7f409c6487c5df6f37f822fd24c2346b071fd48d Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Fri, 25 Oct 2019 10:20:22 -0400 Subject: [PATCH 6/7] 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' }, - ]); - }); }); From dab80fb842c3e98f38001cf75a3ee8ee500e09a9 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Fri, 25 Oct 2019 16:14:42 -0400 Subject: [PATCH 7/7] Adds Proptypes --- .../src/components/Lookup/MultiCredentialsLookup.jsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 107ace0867..b73ca373a3 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -140,10 +140,22 @@ class MultiCredentialsLookup extends React.Component { MultiCredentialsLookup.propTypes = { tooltip: PropTypes.string, + credentials: PropTypes.arrayOf( + PropTypes.shape({ + id: PropTypes.number, + name: PropTypes.string, + description: PropTypes.string, + kind: PropTypes.string, + clound: PropTypes.bool, + }) + ), + onChange: PropTypes.func.isRequired, + onError: PropTypes.func.isRequired, }; MultiCredentialsLookup.defaultProps = { tooltip: '', + credentials: [], }; export { MultiCredentialsLookup as _MultiCredentialsLookup };