From a577be906e20792a73c1148a6e78bb38768be7ae Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Tue, 30 Jul 2019 13:00:22 -0400 Subject: [PATCH 1/3] Adds Multiselect functionality to labels on JTs --- awx/ui_next/src/api/Base.js | 8 +- awx/ui_next/src/api/index.js | 3 + awx/ui_next/src/api/models/JobTemplates.js | 5 + awx/ui_next/src/api/models/Labels.js | 10 + .../components/MultiSelect/MultiSelect.jsx | 202 ++++++++++++++++++ .../src/components/MultiSelect/index.js | 4 + .../JobTemplateAdd/JobTemplateAdd.test.jsx | 5 + .../JobTemplateEdit/JobTemplateEdit.jsx | 19 +- .../JobTemplateEdit/JobTemplateEdit.test.jsx | 3 + .../Template/shared/JobTemplateForm.jsx | 147 ++++++++++++- .../Template/shared/JobTemplateForm.test.jsx | 1 + 11 files changed, 396 insertions(+), 11 deletions(-) create mode 100644 awx/ui_next/src/api/models/Labels.js create mode 100644 awx/ui_next/src/components/MultiSelect/MultiSelect.jsx create mode 100644 awx/ui_next/src/components/MultiSelect/index.js diff --git a/awx/ui_next/src/api/Base.js b/awx/ui_next/src/api/Base.js index ef7c53460f..c821bb21e0 100644 --- a/awx/ui_next/src/api/Base.js +++ b/awx/ui_next/src/api/Base.js @@ -1,6 +1,8 @@ import axios from 'axios'; -import { encodeQueryString } from '@util/qs'; +import { + encodeQueryString +} from '@util/qs'; const defaultHttp = axios.create({ xsrfCookieName: 'csrftoken', @@ -25,7 +27,9 @@ class Base { } read(params) { - return this.http.get(this.baseUrl, { params }); + return this.http.get(this.baseUrl, { + params + }); } readDetail(id) { diff --git a/awx/ui_next/src/api/index.js b/awx/ui_next/src/api/index.js index 5e7b221c77..d295502309 100644 --- a/awx/ui_next/src/api/index.js +++ b/awx/ui_next/src/api/index.js @@ -3,6 +3,7 @@ import InstanceGroups from './models/InstanceGroups'; import Inventories from './models/Inventories'; import JobTemplates from './models/JobTemplates'; import Jobs from './models/Jobs'; +import Labels from './models/Labels'; import Me from './models/Me'; import Organizations from './models/Organizations'; import Root from './models/Root'; @@ -17,6 +18,7 @@ const InstanceGroupsAPI = new InstanceGroups(); const InventoriesAPI = new Inventories(); const JobTemplatesAPI = new JobTemplates(); const JobsAPI = new Jobs(); +const LabelsAPI = new Labels(); const MeAPI = new Me(); const OrganizationsAPI = new Organizations(); const RootAPI = new Root(); @@ -32,6 +34,7 @@ export { InventoriesAPI, JobTemplatesAPI, JobsAPI, + LabelsAPI, MeAPI, OrganizationsAPI, RootAPI, diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index de24e298a7..bdf330df09 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -8,6 +8,7 @@ class JobTemplates extends InstanceGroupsMixin(Base) { this.launch = this.launch.bind(this); this.readLaunch = this.readLaunch.bind(this); + this.updateLabels = this.updateLabels.bind(this); } launch(id, data) { @@ -17,6 +18,10 @@ class JobTemplates extends InstanceGroupsMixin(Base) { readLaunch(id) { return this.http.get(`${this.baseUrl}${id}/launch/`); } + + updateLabels(id, data) { + return this.http.post(`${this.baseUrl}${id}/labels/`, data) + } } export default JobTemplates; diff --git a/awx/ui_next/src/api/models/Labels.js b/awx/ui_next/src/api/models/Labels.js new file mode 100644 index 0000000000..0c0126b898 --- /dev/null +++ b/awx/ui_next/src/api/models/Labels.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class Labels extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/labels/'; + } +} + +export default Labels; diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx new file mode 100644 index 0000000000..0448f569e4 --- /dev/null +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx @@ -0,0 +1,202 @@ +import React, { Component, Fragment } from 'react'; +import PropTypes from 'prop-types'; +import { Chip, ChipGroup } from '@components/Chip'; +import { + Dropdown as PFDropdown, + DropdownItem, + TextInput as PFTextInput, + DropdownToggle, +} from '@patternfly/react-core'; +import styled from 'styled-components'; + +const InputGroup = styled.div` + border: 1px solid black; + margin-top: 2px; +`; +const TextInput = styled(PFTextInput)` + border: none; + width: 100%; + padding-left: 8px; +`; +const Dropdown = styled(PFDropdown)` + width: 100%; + .pf-c-dropdown__toggle.pf-m-plain { + display: none; + } + display: block; + .pf-c-dropdown__menu { + max-height: 200px; + overflow: scroll; + } + && button[disabled] { + color: var(--pf-c-button--m-plain--Color); + pointer-events: initial; + cursor: not-allowed; + color: var(--pf-global--disabled-color--200); + } +`; + +class MultiSelect extends Component { + static propTypes = { + associatedItems: PropTypes.arrayOf( + PropTypes.shape({ + name: PropTypes.string.isRequired, + }) + ).isRequired, + onAddNewItem: PropTypes.func.isRequired, + onRemoveItem: PropTypes.func.isRequired, + }; + + constructor(props) { + super(props); + this.myRef = React.createRef(); + this.state = { + input: '', + chipItems: [], + isExpanded: false, + }; + this.handleAddItem = this.handleAddItem.bind(this); + this.handleInputChange = this.handleInputChange.bind(this); + this.handleSelection = this.handleSelection.bind(this); + this.removeChip = this.removeChip.bind(this); + this.handleClick = this.handleClick.bind(this); + } + + componentDidMount() { + this.renderChips(); + document.addEventListener('mousedown', this.handleClick, false); + } + + handleClick(e, option) { + if (this.node && this.node.contains(e.target)) { + if (option) { + this.handleSelection(e, option); + } + this.setState({ isExpanded: true }); + } else { + this.setState({ isExpanded: false }); + } + } + + renderChips() { + const { associatedItems } = this.props; + const items = associatedItems.map(item => ({ + name: item.name, + id: item.id, + organization: item.organization, + })); + this.setState({ + chipItems: items, + }); + } + + handleSelection(e, item) { + const { chipItems } = this.state; + const { onAddNewItem } = this.props; + + this.setState({ + chipItems: chipItems.concat({ name: item.name, id: item.id }), + }); + onAddNewItem(item); + e.preventDefault(); + } + + handleAddItem(event) { + const { input, chipItems } = this.state; + const { onAddNewItem } = this.props; + const newChip = { name: input, id: Math.random() }; + if (event.key === 'Tab') { + this.setState({ + chipItems: chipItems.concat(newChip), + isExpanded: false, + input: '', + }); + + onAddNewItem(input); + } + } + + handleInputChange(e) { + this.setState({ input: e, isExpanded: true }); + } + + removeChip(e, item) { + const { onRemoveItem } = this.props; + const { chipItems } = this.state; + const chips = chipItems.filter(chip => chip.name !== item.name); + + this.setState({ chipItems: chips }); + onRemoveItem(item); + + e.preventDefault(); + } + + render() { + const { options } = this.props; + const { chipItems, input, isExpanded } = this.state; + + const list = options.map(option => ( + + {option.name.includes(input) ? ( + item.id === option.id)} + value={option.name} + onClick={e => { + this.handleClick(e, option); + }} + > + {option.name} + + ) : null} + + )); + + const chips = ( + + {chipItems && + chipItems.map(item => ( + { + this.removeChip(e, item); + }} + > + {item.name} + + ))} + + ); + return ( + + +
{ + this.node = node; + }} + > + this.setState({ isExpanded: true })} + onChange={this.handleInputChange} + onKeyDown={this.handleAddItem} + /> + Labels} + // Above is not rendered but is a required prop from Patternfly + isOpen={isExpanded} + dropdownItems={list} + /> +
+
{chips}
+
+
+ ); + } +} +export default MultiSelect; diff --git a/awx/ui_next/src/components/MultiSelect/index.js b/awx/ui_next/src/components/MultiSelect/index.js new file mode 100644 index 0000000000..95d32907ef --- /dev/null +++ b/awx/ui_next/src/components/MultiSelect/index.js @@ -0,0 +1,4 @@ +export { + default +} +from './MultiSelect'; diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx index 859e327663..e9e579b673 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -13,6 +13,11 @@ describe('', () => { name: '', playbook: '', project: '', + summary_fields: { + user_capabilities: { + edit: true, + }, + }, }; afterEach(() => { diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index c488b3c8f6..db7e50e18c 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -21,14 +21,29 @@ class JobTemplateEdit extends Component { this.handleSubmit = this.handleSubmit.bind(this); } - async handleSubmit(values) { + async handleSubmit(values, newLabels, removedLabels) { const { template: { id, type }, history, } = this.props; + const disassociatedLabels = removedLabels + ? removedLabels.forEach(removedLabel => + JobTemplatesAPI.updateLabels(id, removedLabel) + ) + : null; + const associatedLabels = newLabels + ? newLabels.forEach(newLabel => + JobTemplatesAPI.updateLabels(id, newLabel) + ) + : null; + try { - await JobTemplatesAPI.update(id, { ...values }); + await Promise.all([ + JobTemplatesAPI.update(id, { ...values }), + disassociatedLabels, + associatedLabels, + ]); history.push(`/templates/${type}/${id}/details`); } catch (error) { this.setState({ error }); 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 35900e81a9..9912f0e678 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -19,6 +19,9 @@ describe('', () => { user_capabilities: { edit: true, }, + labels: { + results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }], + }, }, }; diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index daebb1770f..98eaa6735e 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -4,9 +4,17 @@ import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Formik, Field } from 'formik'; -import { Form, FormGroup, Tooltip } from '@patternfly/react-core'; +import { + Form, + FormGroup, + Tooltip, + PageSection, + Card, +} from '@patternfly/react-core'; import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; +import ContentError from '@components/ContentError'; import AnsibleSelect from '@components/AnsibleSelect'; +import MultiSelect from '@components/MultiSelect'; import FormActionGroup from '@components/FormActionGroup'; import FormField from '@components/FormField'; import FormRow from '@components/FormRow'; @@ -14,10 +22,16 @@ import { required } from '@util/validators'; import styled from 'styled-components'; import { JobTemplate } from '@types'; import InventoriesLookup from './InventoriesLookup'; +import { LabelsAPI } from '@api'; const QuestionCircleIcon = styled(PFQuestionCircleIcon)` margin-left: 10px; `; +const QSConfig = { + page: 1, + page_size: 200, + order_by: 'name', +}; class JobTemplateForm extends Component { static propTypes = { @@ -36,22 +50,107 @@ class JobTemplateForm extends Component { playbook: '', summary_fields: { inventory: null, + labels: { results: [] }, }, }, }; constructor(props) { super(props); - this.state = { + hasContentLoading: true, + contentError: false, + loadedLabels: [], + newLabels: [], + removedLabels: [], inventory: props.template.summary_fields.inventory, }; + this.handleNewLabel = this.handleNewLabel.bind(this); + this.loadLabels = this.loadLabels.bind(this); + this.disassociateLabel = this.disassociateLabel.bind(this); + } + + componentDidMount() { + this.loadLabels(QSConfig); + } + + async loadLabels(QueryConfig) { + const { loadedLabels } = this.state; + this.setState({ contentError: null, hasContentLoading: true }); + try { + const { data } = await LabelsAPI.read(QueryConfig); + const labels = [...data.results]; + this.setState({ loadedLabels: loadedLabels.concat(labels) }); + if (data.next && data.next.includes('page=2')) { + this.loadLabels({ + page: 2, + page_size: 200, + order_by: 'name', + }); + } + } catch (err) { + this.setState({ contentError: err }); + } finally { + this.setState({ hasContentLoading: false }); + } + } + + handleNewLabel(label) { + const { newLabels } = this.state; + const { template } = this.props; + const isIncluded = newLabels.some(newLabel => newLabel.name === label.name); + if (isIncluded) { + const filteredLabels = newLabels.filter( + newLabel => newLabel.name !== label + ); + this.setState({ newLabels: filteredLabels }); + } else if (typeof label === 'string') { + this.setState({ + newLabels: [ + ...newLabels, + { + name: label, + organization: template.summary_fields.inventory.organization_id, + }, + ], + }); + } else { + this.setState({ + newLabels: [...newLabels, { associate: true, id: label.id }], + }); + } + } + + disassociateLabel(label) { + const { removedLabels, newLabels } = this.state; + const isNewCreatedLabel = newLabels.some( + newLabel => newLabel === label.name + ); + if (isNewCreatedLabel) { + const filteredLabels = newLabels.filter( + newLabel => newLabel !== label.name + ); + this.setState({ newLabels: filteredLabels }); + } else { + this.setState({ + removedLabels: removedLabels.concat({ + disassociate: true, + id: label.id, + }), + }); + } } render() { + const { + loadedLabels, + contentError, + hasContentLoading, + inventory, + newLabels, + removedLabels, + } = this.state; const { handleCancel, handleSubmit, i18n, template } = this.props; - const { inventory } = this.state; - const jobTypeOptions = [ { value: '', @@ -68,6 +167,15 @@ class JobTemplateForm extends Component { }, ]; + if (!hasContentLoading && contentError) { + return ( + + + + + + ); + } return ( { + handleSubmit(values, newLabels, removedLabels); }} - onSubmit={handleSubmit} render={formik => (
@@ -156,9 +267,31 @@ class JobTemplateForm extends Component { validate={required(null, i18n)} /> + + + + + + ( + + )} + /> + + handleSubmit(values)} /> )} @@ -166,5 +299,5 @@ class JobTemplateForm extends Component { ); } } - +export { JobTemplateForm as _JobTemplateForm }; export default withI18n()(withRouter(JobTemplateForm)); 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 2aaae29d12..2350298dbf 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -20,6 +20,7 @@ describe('', () => { id: 2, name: 'foo', }, + labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, }, }; From 74a1ebff32a06b8698dbe8101a4cf33c040ffe3a Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 1 Aug 2019 10:39:27 -0400 Subject: [PATCH 2/3] Adds tests and refines chip interaction in MultiSelect component --- .../components/MultiSelect/MultiSelect.jsx | 7 +- .../MultiSelect/MultiSelect.test.jsx | 86 +++++++++++++++++++ .../Template/shared/JobTemplateForm.jsx | 40 +++++---- .../Template/shared/JobTemplateForm.test.jsx | 65 +++++++++++++- 4 files changed, 176 insertions(+), 22 deletions(-) create mode 100644 awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx index 0448f569e4..9bc8ad2a86 100644 --- a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx @@ -1,5 +1,7 @@ import React, { Component, Fragment } from 'react'; import PropTypes from 'prop-types'; +import { withI18n } from '@lingui/react'; +import { withRouter } from 'react-router-dom'; import { Chip, ChipGroup } from '@components/Chip'; import { Dropdown as PFDropdown, @@ -123,7 +125,7 @@ class MultiSelect extends Component { removeChip(e, item) { const { onRemoveItem } = this.props; const { chipItems } = this.state; - const chips = chipItems.filter(chip => chip.name !== item.name); + const chips = chipItems.filter(chip => chip.id !== item.id); this.setState({ chipItems: chips }); onRemoveItem(item); @@ -199,4 +201,5 @@ class MultiSelect extends Component { ); } } -export default MultiSelect; +export { MultiSelect as _MultiSelect }; +export default withI18n()(withRouter(MultiSelect)); diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx new file mode 100644 index 0000000000..368f5ca27f --- /dev/null +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx @@ -0,0 +1,86 @@ +import React from 'react'; +import MultiSelect, { _MultiSelect } from './MultiSelect'; +import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; + +describe('', () => { + const associatedItems = [ + { name: 'Foo', id: 1, organization: 1 }, + { name: 'Bar', id: 2, organization: 1 }, + ]; + const options = [{ name: 'Angry', id: 3 }, { name: 'Potato', id: 4 }]; + + test('Initially render successfully', () => { + const renderChips = jest.spyOn(_MultiSelect.prototype, 'renderChips'); + const wrapper = mountWithContexts( + + ); + const component = wrapper.find('MultiSelect'); + + expect(renderChips).toBeCalled(); + expect(component.state().chipItems.length).toBe(2); + }); + test('handleSelection add item to chipItems', async () => { + const wrapper = mountWithContexts( + + ); + const event = { preventDefault: () => {} }; + const component = wrapper.find('MultiSelect'); + component.instance().handleSelection(event, { name: 'Apollo', id: 5 }); + expect(component.state().chipItems.length).toBe(3); + }); + test('handleAddItem adds a chip only when Tab is pressed', () => { + const onAddNewItem = jest.fn(); + const wrapper = mountWithContexts( + + ); + const event = { + preventDefault: () => {}, + key: 'Tab', + }; + const component = wrapper.find('MultiSelect'); + + component.setState({ input: 'newLabel' }); + component.update(); + component.instance().handleAddItem(event); + expect(component.state().chipItems.length).toBe(3); + expect(component.state().input.length).toBe(0); + expect(component.state().isExpanded).toBe(false); + expect(onAddNewItem).toBeCalled(); + }); + test('removeChip removes chip properly', () => { + const onRemoveItem = jest.fn(); + + const wrapper = mountWithContexts( + + ); + const event = { + preventDefault: () => {}, + }; + const component = wrapper.find('MultiSelect'); + component + .instance() + .removeChip(event, { name: 'Foo', id: 1, organization: 1 }); + expect(component.state().chipItems.length).toBe(1); + expect(onRemoveItem).toBeCalled(); + }); +}); diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 98eaa6735e..c1a041d4eb 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -75,19 +75,22 @@ class JobTemplateForm extends Component { } async loadLabels(QueryConfig) { - const { loadedLabels } = this.state; this.setState({ contentError: null, hasContentLoading: true }); + let loadedLabels; try { const { data } = await LabelsAPI.read(QueryConfig); - const labels = [...data.results]; - this.setState({ loadedLabels: loadedLabels.concat(labels) }); + loadedLabels = [...data.results]; if (data.next && data.next.includes('page=2')) { - this.loadLabels({ + const { + data: { results }, + } = await LabelsAPI.read({ page: 2, page_size: 200, order_by: 'name', }); + loadedLabels = loadedLabels.concat(results); } + this.setState({ loadedLabels }); } catch (err) { this.setState({ contentError: err }); } finally { @@ -116,19 +119,22 @@ class JobTemplateForm extends Component { }); } else { this.setState({ - newLabels: [...newLabels, { associate: true, id: label.id }], + newLabels: [ + ...newLabels, + { name: label.name, associate: true, id: label.id }, + ], }); } } disassociateLabel(label) { - const { removedLabels, newLabels } = this.state; - const isNewCreatedLabel = newLabels.some( - newLabel => newLabel === label.name + const { removedLabels, loadedLabels, newLabels } = this.state; + const isNewCreatedLabel = loadedLabels.some( + loadedLabel => loadedLabel.name !== label.name ); if (isNewCreatedLabel) { const filteredLabels = newLabels.filter( - newLabel => newLabel !== label.name + newLabel => newLabel.name !== label.name ); this.setState({ newLabels: filteredLabels }); } else { @@ -277,21 +283,17 @@ class JobTemplateForm extends Component { > - ( - - )} + handleSubmit(values)} + onSubmit={formik.handleSubmit} /> )} 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 2350298dbf..b0833f8168 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -1,7 +1,8 @@ import React from 'react'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; -import JobTemplateForm from './JobTemplateForm'; +import JobTemplateForm, { _JobTemplateForm } from './JobTemplateForm'; +import { LabelsAPI } from '@api'; jest.mock('@api'); @@ -19,10 +20,16 @@ describe('', () => { inventory: { id: 2, name: 'foo', + organization_id: 1, }, labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }] }, }, }; + beforeEach(() => { + LabelsAPI.read.mockReturnValue({ + data: mockData.summary_fields.labels, + }); + }); afterEach(() => { jest.clearAllMocks(); @@ -36,6 +43,7 @@ describe('', () => { handleCancel={jest.fn()} /> ); + expect(LabelsAPI.read).toHaveBeenCalled(); }); test('should update form values on input changes', async () => { @@ -104,4 +112,59 @@ describe('', () => { wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); expect(handleCancel).toBeCalled(); }); + + test('handleNewLabel should arrange new labels properly', async () => { + const handleNewLabel = jest.spyOn( + _JobTemplateForm.prototype, + 'handleNewLabel' + ); + const event = { key: 'Tab' }; + const wrapper = mountWithContexts( + + ); + const multiSelect = wrapper.find('MultiSelect'); + const component = wrapper.find('JobTemplateForm'); + + wrapper.setState({ newLabels: [], loadedLabels: [], removedLabels: [] }); + multiSelect.setState({ input: 'Foo' }); + + wrapper.find('input[aria-label="labels"]').prop('onKeyDown')(event); + expect(handleNewLabel).toHaveBeenCalledWith('Foo'); + + component.instance().handleNewLabel({ name: 'Bar', id: 2 }); + expect(component.state().newLabels).toEqual([ + { name: 'Foo', organization: 1 }, + { associate: true, id: 2, name: 'Bar' }, + ]); + }); + test('disassociateLabel should arrange new labels properly', async () => { + const wrapper = mountWithContexts( + + ); + const multiSelect = wrapper.find('MultiSelect'); + const component = wrapper.find('JobTemplateForm'); + + component.setState({ + newLabels: [{ name: 'Foo', id: 1 }], + loadedLabels: [{ name: 'Bar', id: 3 }], + removedLabels: [], + }); + component.update(); + multiSelect.setState({ input: 'Wowza' }); + component.instance().disassociateLabel({ name: 'Foo', id: 1 }); + expect(component.state().newLabels.length).toBe(0); + expect(component.state().removedLabels.length).toBe(0); + + component.instance().disassociateLabel({ name: 'Bar', id: 3 }); + expect(component.state().newLabels.length).toBe(0); + expect(component.state().removedLabels.length).toBe(1); + }); }); From 8b35642b085b0b31a2cc221d8506bac9704137ef Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 1 Aug 2019 14:25:41 -0400 Subject: [PATCH 3/3] Fixes failing test and addresses PR issues --- awx/ui_next/src/api/Base.js | 6 +-- awx/ui_next/src/api/models/JobTemplates.js | 16 ++++-- .../components/MultiSelect/MultiSelect.jsx | 50 +++++++++---------- .../MultiSelect/MultiSelect.test.jsx | 15 ++++-- .../src/components/MultiSelect/index.js | 5 +- .../JobTemplateEdit/JobTemplateEdit.jsx | 21 ++++---- .../JobTemplateEdit/JobTemplateEdit.test.jsx | 19 ++++++- .../Template/shared/JobTemplateForm.jsx | 32 +++++++----- .../Template/shared/JobTemplateForm.test.jsx | 26 ++++------ 9 files changed, 111 insertions(+), 79 deletions(-) diff --git a/awx/ui_next/src/api/Base.js b/awx/ui_next/src/api/Base.js index c821bb21e0..33e3626994 100644 --- a/awx/ui_next/src/api/Base.js +++ b/awx/ui_next/src/api/Base.js @@ -1,8 +1,6 @@ import axios from 'axios'; -import { - encodeQueryString -} from '@util/qs'; +import { encodeQueryString } from '@util/qs'; const defaultHttp = axios.create({ xsrfCookieName: 'csrftoken', @@ -28,7 +26,7 @@ class Base { read(params) { return this.http.get(this.baseUrl, { - params + params, }); } diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index bdf330df09..44eef5529b 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -8,7 +8,9 @@ class JobTemplates extends InstanceGroupsMixin(Base) { this.launch = this.launch.bind(this); this.readLaunch = this.readLaunch.bind(this); - this.updateLabels = this.updateLabels.bind(this); + this.associateLabel = this.associateLabel.bind(this); + this.disassociateLabel = this.disassociateLabel.bind(this); + this.generateLabel = this.generateLabel.bind(this); } launch(id, data) { @@ -19,8 +21,16 @@ class JobTemplates extends InstanceGroupsMixin(Base) { return this.http.get(`${this.baseUrl}${id}/launch/`); } - updateLabels(id, data) { - return this.http.post(`${this.baseUrl}${id}/labels/`, data) + associateLabel(id, label) { + return this.http.post(`${this.baseUrl}${id}/labels/`, label); + } + + disassociateLabel(id, label) { + return this.http.post(`${this.baseUrl}${id}/labels/`, label); + } + + generateLabel(orgId, label) { + return this.http.post(`${this.baseUrl}${orgId}/labels/`, label); } } diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx index 9bc8ad2a86..ea5a701ccd 100644 --- a/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.jsx @@ -51,10 +51,9 @@ class MultiSelect extends Component { constructor(props) { super(props); - this.myRef = React.createRef(); this.state = { input: '', - chipItems: [], + chipItems: this.getInitialChipItems(), isExpanded: false, }; this.handleAddItem = this.handleAddItem.bind(this); @@ -65,57 +64,58 @@ class MultiSelect extends Component { } componentDidMount() { - this.renderChips(); document.addEventListener('mousedown', this.handleClick, false); } + componentWillUnmount() { + document.removeEventListener('mousedown', this.handleClick, false); + } + + getInitialChipItems() { + const { associatedItems } = this.props; + return associatedItems.map(item => ({ + name: item.name, + id: item.id, + organization: item.organization, + })); + } + handleClick(e, option) { if (this.node && this.node.contains(e.target)) { if (option) { this.handleSelection(e, option); } - this.setState({ isExpanded: true }); } else { this.setState({ isExpanded: false }); } } - renderChips() { - const { associatedItems } = this.props; - const items = associatedItems.map(item => ({ - name: item.name, - id: item.id, - organization: item.organization, - })); - this.setState({ - chipItems: items, - }); - } - handleSelection(e, item) { const { chipItems } = this.state; const { onAddNewItem } = this.props; + e.preventDefault(); this.setState({ chipItems: chipItems.concat({ name: item.name, id: item.id }), + isExpanded: false, }); onAddNewItem(item); - e.preventDefault(); } handleAddItem(event) { const { input, chipItems } = this.state; const { onAddNewItem } = this.props; const newChip = { name: input, id: Math.random() }; - if (event.key === 'Tab') { - this.setState({ - chipItems: chipItems.concat(newChip), - isExpanded: false, - input: '', - }); - - onAddNewItem(input); + if (event.key !== 'Tab') { + return; } + this.setState({ + chipItems: chipItems.concat(newChip), + isExpanded: false, + input: '', + }); + + onAddNewItem(input); } handleInputChange(e) { diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx index 368f5ca27f..422baacb84 100644 --- a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { sleep } from '@testUtils/testUtils'; import MultiSelect, { _MultiSelect } from './MultiSelect'; import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; @@ -10,7 +11,10 @@ describe('', () => { const options = [{ name: 'Angry', id: 3 }, { name: 'Potato', id: 4 }]; test('Initially render successfully', () => { - const renderChips = jest.spyOn(_MultiSelect.prototype, 'renderChips'); + const getInitialChipItems = jest.spyOn( + _MultiSelect.prototype, + 'getInitialChipItems' + ); const wrapper = mountWithContexts( ', () => { ); const component = wrapper.find('MultiSelect'); - expect(renderChips).toBeCalled(); + expect(getInitialChipItems).toBeCalled(); expect(component.state().chipItems.length).toBe(2); }); test('handleSelection add item to chipItems', async () => { @@ -33,9 +37,12 @@ describe('', () => { options={options} /> ); - const event = { preventDefault: () => {} }; const component = wrapper.find('MultiSelect'); - component.instance().handleSelection(event, { name: 'Apollo', id: 5 }); + component + .find('input[aria-label="labels"]') + .simulate('keydown', { key: 'Tab' }); + component.update(); + await sleep(1); expect(component.state().chipItems.length).toBe(3); }); test('handleAddItem adds a chip only when Tab is pressed', () => { diff --git a/awx/ui_next/src/components/MultiSelect/index.js b/awx/ui_next/src/components/MultiSelect/index.js index 95d32907ef..8cda42c7cb 100644 --- a/awx/ui_next/src/components/MultiSelect/index.js +++ b/awx/ui_next/src/components/MultiSelect/index.js @@ -1,4 +1 @@ -export { - default -} -from './MultiSelect'; +export { default } from './MultiSelect'; diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index db7e50e18c..655ececfe5 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -21,28 +21,27 @@ class JobTemplateEdit extends Component { this.handleSubmit = this.handleSubmit.bind(this); } - async handleSubmit(values, newLabels, removedLabels) { + async handleSubmit(values, newLabels = [], removedLabels = []) { const { template: { id, type }, history, } = this.props; - const disassociatedLabels = removedLabels - ? removedLabels.forEach(removedLabel => - JobTemplatesAPI.updateLabels(id, removedLabel) - ) - : null; + const disassociatedLabels = removedLabels.forEach(removedLabel => + JobTemplatesAPI.disassociateLabel(id, removedLabel) + ); const associatedLabels = newLabels - ? newLabels.forEach(newLabel => - JobTemplatesAPI.updateLabels(id, newLabel) - ) - : null; - + .filter(newLabel => !newLabel.organization) + .forEach(newLabel => JobTemplatesAPI.associateLabel(id, newLabel)); + const generatedLabels = newLabels + .filter(newLabel => newLabel.organization) + .forEach(newLabel => JobTemplatesAPI.generateLabel(id, newLabel)); try { await Promise.all([ JobTemplatesAPI.update(id, { ...values }), disassociatedLabels, associatedLabels, + generatedLabels, ]); history.push(`/templates/${type}/${id}/details`); } catch (error) { 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 9912f0e678..1448887e95 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -36,9 +36,26 @@ describe('', () => { description: 'new description', job_type: 'check', }; + const newLabels = [ + { associate: true, id: 3 }, + { associate: true, id: 3 }, + { name: 'Mapel', organization: 1 }, + { name: 'Tree', organization: 1 }, + ]; + const removedLabels = [ + { disassociate: true, id: 1 }, + { disassociate: true, id: 2 }, + ]; - wrapper.find('JobTemplateForm').prop('handleSubmit')(updatedTemplateData); + wrapper.find('JobTemplateForm').prop('handleSubmit')( + updatedTemplateData, + newLabels, + removedLabels + ); expect(JobTemplatesAPI.update).toHaveBeenCalledWith(1, updatedTemplateData); + expect(JobTemplatesAPI.disassociateLabel).toHaveBeenCalledTimes(2); + expect(JobTemplatesAPI.associateLabel).toHaveBeenCalledTimes(2); + expect(JobTemplatesAPI.generateLabel).toHaveBeenCalledTimes(2); }); test('should navigate to job template detail when cancel is clicked', () => { diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index c1a041d4eb..fc2ff53556 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -67,13 +67,18 @@ class JobTemplateForm extends Component { }; this.handleNewLabel = this.handleNewLabel.bind(this); this.loadLabels = this.loadLabels.bind(this); - this.disassociateLabel = this.disassociateLabel.bind(this); + this.removeLabel = this.removeLabel.bind(this); } componentDidMount() { this.loadLabels(QSConfig); } + // The function below assumes that the user has no more than 400 + // labels. For the vast majority of users this will be more thans + // enough.This can be updated to allow more than 400 labels if we + // decide it is necessary. + async loadLabels(QueryConfig) { this.setState({ contentError: null, hasContentLoading: true }); let loadedLabels; @@ -127,23 +132,26 @@ class JobTemplateForm extends Component { } } - disassociateLabel(label) { - const { removedLabels, loadedLabels, newLabels } = this.state; - const isNewCreatedLabel = loadedLabels.some( - loadedLabel => loadedLabel.name !== label.name + removeLabel(label) { + const { removedLabels, newLabels } = this.state; + const { template } = this.props; + + const isAssociatedLabel = template.summary_fields.labels.results.some( + tempLabel => tempLabel.id === label.id ); - if (isNewCreatedLabel) { - const filteredLabels = newLabels.filter( - newLabel => newLabel.name !== label.name - ); - this.setState({ newLabels: filteredLabels }); - } else { + + if (isAssociatedLabel) { this.setState({ removedLabels: removedLabels.concat({ disassociate: true, id: label.id, }), }); + } else { + const filteredLabels = newLabels.filter( + newLabel => newLabel.name !== label.name + ); + this.setState({ newLabels: filteredLabels }); } } @@ -285,7 +293,7 @@ class JobTemplateForm extends Component { 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 b0833f8168..4fb2223d7f 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -36,14 +36,16 @@ describe('', () => { }); test('initially renders successfully', () => { - mountWithContexts( + const wrapper = mountWithContexts( ); + const component = wrapper.find('ChipGroup'); expect(LabelsAPI.read).toHaveBeenCalled(); + expect(component.find('span#pf-random-id-1').text()).toEqual('Sushi'); }); test('should update form values on input changes', async () => { @@ -131,8 +133,7 @@ describe('', () => { wrapper.setState({ newLabels: [], loadedLabels: [], removedLabels: [] }); multiSelect.setState({ input: 'Foo' }); - - wrapper.find('input[aria-label="labels"]').prop('onKeyDown')(event); + component.find('input[aria-label="labels"]').prop('onKeyDown')(event); expect(handleNewLabel).toHaveBeenCalledWith('Foo'); component.instance().handleNewLabel({ name: 'Bar', id: 2 }); @@ -149,21 +150,16 @@ describe('', () => { handleCancel={jest.fn()} /> ); - const multiSelect = wrapper.find('MultiSelect'); const component = wrapper.find('JobTemplateForm'); - - component.setState({ - newLabels: [{ name: 'Foo', id: 1 }], - loadedLabels: [{ name: 'Bar', id: 3 }], - removedLabels: [], - }); - component.update(); - multiSelect.setState({ input: 'Wowza' }); - component.instance().disassociateLabel({ name: 'Foo', id: 1 }); + // This asserts that the user generated a label or clicked + // on a label option, and then changed their mind and + // removed the label. + component.instance().removeLabel({ name: 'Alex', id: 17 }); expect(component.state().newLabels.length).toBe(0); expect(component.state().removedLabels.length).toBe(0); - - component.instance().disassociateLabel({ name: 'Bar', id: 3 }); + // This asserts that the user removed a label that was associated + // with the template when the template loaded. + component.instance().removeLabel({ name: 'Sushi', id: 1 }); expect(component.state().newLabels.length).toBe(0); expect(component.state().removedLabels.length).toBe(1); });