diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index 646f168df8..a6af575b44 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -27,11 +27,17 @@ class JobTemplates extends InstanceGroupsMixin(Base) { } disassociateLabel(id, label) { - return this.http.post(`${this.baseUrl}${id}/labels/`, label); + return this.http.post(`${this.baseUrl}${id}/labels/`, { + id: label.id, + disassociate: true, + }); } - generateLabel(orgId, label) { - return this.http.post(`${this.baseUrl}${orgId}/labels/`, label); + generateLabel(id, label, orgId) { + return this.http.post(`${this.baseUrl}${id}/labels/`, { + name: label.name, + organization: orgId, + }); } readCredentials(id, params) { diff --git a/awx/ui_next/src/components/Chip/Chip.jsx b/awx/ui_next/src/components/Chip/Chip.jsx index f8837812f0..8c86115919 100644 --- a/awx/ui_next/src/components/Chip/Chip.jsx +++ b/awx/ui_next/src/components/Chip/Chip.jsx @@ -1,6 +1,7 @@ import { Chip } from '@patternfly/react-core'; import styled from 'styled-components'; +Chip.displayName = 'PFChip'; export default styled(Chip)` --pf-c-chip--m-read-only--PaddingTop: 3px; --pf-c-chip--m-read-only--PaddingRight: 8px; diff --git a/awx/ui_next/src/components/FormField/FieldTooltip.jsx b/awx/ui_next/src/components/FormField/FieldTooltip.jsx new file mode 100644 index 0000000000..59031bb365 --- /dev/null +++ b/awx/ui_next/src/components/FormField/FieldTooltip.jsx @@ -0,0 +1,26 @@ +import React from 'react'; +import { node } from 'prop-types'; +import { Tooltip } from '@patternfly/react-core'; +import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; +import styled from 'styled-components'; + +const QuestionCircleIcon = styled(PFQuestionCircleIcon)` + margin-left: 10px; +`; + +function FieldTooltip({ content }) { + return ( + + + + ); +} +FieldTooltip.propTypes = { + content: node.isRequired, +}; + +export default FieldTooltip; diff --git a/awx/ui_next/src/components/FormField/index.js b/awx/ui_next/src/components/FormField/index.js index c592b7c800..4ce1944f17 100644 --- a/awx/ui_next/src/components/FormField/index.js +++ b/awx/ui_next/src/components/FormField/index.js @@ -1,2 +1,3 @@ export { default } from './FormField'; export { default as CheckboxField } from './CheckboxField'; +export { default as FieldTooltip } from './FieldTooltip'; diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index 92dcad3a3e..5db0c677fb 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -113,6 +113,7 @@ class ListHeader extends React.Component { columns, onSearch: this.handleSearch, onSort: this.handleSort, + qsConfig, })} InventoriesAPI.read(params); @@ -26,11 +20,7 @@ class InventoryLookup extends React.Component { isRequired={required} fieldId="inventory-lookup" > - {tooltip && ( - - - - )} + {tooltip && } ProjectsAPI.read(params); @@ -36,11 +31,7 @@ class ProjectLookup extends React.Component { isValid={isValid} label={i18n._(t`Project`)} > - {tooltip && ( - - - - )} + {tooltip && } ({ ...item })); - } - handleClick(e, option) { if (this.node && this.node.contains(e.target)) { if (option) { - this.handleSelection(e, option); + e.preventDefault(); + this.addItem(option); } } else { this.setState({ input: '', isExpanded: false }); } } - handleSelection(e, item) { - const { chipItems } = this.state; - const { onAddNewItem, onChange } = this.props; - e.preventDefault(); - - const items = chipItems.concat({ name: item.name, id: item.id }); - this.setState({ - chipItems: items, - isExpanded: false, - }); + addItem(item) { + const { value, onAddNewItem, onChange } = this.props; + const items = value.concat(item); onAddNewItem(item); onChange(items); + this.close(); + } + + // TODO: UpArrow & DownArrow for menu navigation + handleKeyDown(event) { + const { value, options } = this.props; + const { input } = this.state; + if (event.key === 'Tab') { + this.close(); + return; + } + if (!input || event.key !== 'Enter') { + return; + } + + const isAlreadySelected = value.some(i => i.name === input); + if (isAlreadySelected) { + event.preventDefault(); + this.close(); + return; + } + + const match = options.find(item => item.name === input); + const isNewItem = !match || !value.find(item => item.id === match.id); + if (isNewItem) { + event.preventDefault(); + this.addItem(match || this.createNewItem(input)); + } + } + + close() { + this.setState({ + isExpanded: false, + input: '', + }); } createNewItem(name) { @@ -124,66 +146,28 @@ class MultiSelect extends Component { }; } - handleAddItem(event) { - const { input, chipItems } = this.state; - const { options, onAddNewItem, onChange } = this.props; - const match = options.find(item => item.name === input); - const isIncluded = chipItems.some(chipItem => chipItem.name === input); - - if (!input) { - return; - } - - if (isIncluded) { - // This event.preventDefault prevents the form from submitting - // if the user tries to create 2 chips of the same name - event.preventDefault(); - this.setState({ input: '', isExpanded: false }); - return; - } - const isNewItem = !match || !chipItems.find(item => item.id === match.id); - if (event.key === 'Enter' && isNewItem) { - event.preventDefault(); - const items = chipItems.concat({ name: input, id: input }); - const newItem = match || this.createNewItem(input); - this.setState({ - chipItems: items, - isExpanded: false, - input: '', - }); - onAddNewItem(newItem); - onChange(items); - } else if (!isNewItem || event.key === 'Tab') { - this.setState({ isExpanded: false, input: '' }); - } - } - handleInputChange(value) { this.setState({ input: value, isExpanded: true }); } - removeChip(e, item) { - const { onRemoveItem, onChange } = this.props; - const { chipItems } = this.state; - const chips = chipItems.filter(chip => chip.id !== item.id); + removeItem(item) { + const { value, onRemoveItem, onChange } = this.props; + const remainingItems = value.filter(chip => chip.id !== item.id); - this.setState({ chipItems: chips }); onRemoveItem(item); - onChange(chips); - - e.preventDefault(); + onChange(remainingItems); } render() { - const { options } = this.props; - const { chipItems, input, isExpanded } = this.state; + const { value, options } = this.props; + const { input, isExpanded } = this.state; - const list = options.map(option => ( + const dropdownOptions = options.map(option => ( {option.name.includes(input) ? ( item.id === option.id)} + isDisabled={value.some(item => item.id === option.id)} value={option.name} onClick={e => { this.handleClick(e, option); @@ -195,21 +179,6 @@ class MultiSelect extends Component { )); - const chips = ( - - {chipItems && - chipItems.map(item => ( - { - this.removeChip(e, item); - }} - > - {item.name} - - ))} - - ); return ( @@ -222,21 +191,34 @@ class MultiSelect extends Component { type="text" aria-label="labels" value={input} - onClick={() => this.setState({ isExpanded: true })} + onFocus={() => this.setState({ isExpanded: true })} onChange={this.handleInputChange} - onKeyDown={this.handleAddItem} + onKeyDown={this.handleKeyDown} /> Labels} - // Above is not rendered but is a required prop from Patternfly + // Above is not visible but is a required prop from Patternfly isOpen={isExpanded} - dropdownItems={list} + dropdownItems={dropdownOptions} /> -
{chips}
+
+ + {value.map(item => ( + { + this.removeItem(item); + }} + > + {item.name} + + ))} + +
); diff --git a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx index e52e56b3c1..e18ed64eff 100644 --- a/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx +++ b/awx/ui_next/src/components/MultiSelect/MultiSelect.test.jsx @@ -1,48 +1,51 @@ import React from 'react'; -import { mount } from 'enzyme'; -import { sleep } from '@testUtils/testUtils'; +import { mount, shallow } from 'enzyme'; import MultiSelect from './MultiSelect'; describe('', () => { - const associatedItems = [ + const value = [ { 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 wrapper = mount( + test('should render successfully', () => { + const wrapper = shallow( + ); + + expect(wrapper.find('Chip')).toHaveLength(2); + }); + + test('should add item when typed', async () => { + const onChange = jest.fn(); + const onAdd = jest.fn(); + const wrapper = mount( + ); const component = wrapper.find('MultiSelect'); + const input = component.find('TextInput'); + input.invoke('onChange')('Flabadoo'); + input.simulate('keydown', { key: 'Enter' }); - expect(component.state().chipItems.length).toBe(2); + expect(onAdd.mock.calls[0][0].name).toEqual('Flabadoo'); + const newVal = onChange.mock.calls[0][0]; + expect(newVal).toHaveLength(3); + expect(newVal[2].name).toEqual('Flabadoo'); }); - test('handleSelection add item to chipItems', async () => { - const wrapper = mount( - - ); - const component = wrapper.find('MultiSelect'); - component - .find('input[aria-label="labels"]') - .simulate('keydown', { key: 'Enter' }); - component.update(); - await sleep(1); - expect(component.state().chipItems.length).toBe(2); - }); - - test('handleAddItem adds a chip only when Tab is pressed', () => { + test('should add item when clicked from menu', () => { const onAddNewItem = jest.fn(); const onChange = jest.fn(); const wrapper = mount( @@ -50,48 +53,53 @@ describe('', () => { onAddNewItem={onAddNewItem} onRemoveItem={jest.fn()} onChange={onChange} - associatedItems={associatedItems} + value={value} options={options} /> ); + + const input = wrapper.find('TextInput'); + input.simulate('focus'); + wrapper.update(); const event = { preventDefault: () => {}, - key: 'Enter', + target: wrapper + .find('DropdownItem') + .at(1) + .getDOMNode(), }; - const component = wrapper.find('MultiSelect'); + wrapper + .find('DropdownItem') + .at(1) + .invoke('onClick')(event); - 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(); - expect(onChange).toBeCalled(); + expect(onAddNewItem).toHaveBeenCalledWith(options[1]); + const newVal = onChange.mock.calls[0][0]; + expect(newVal).toHaveLength(3); + expect(newVal[2]).toEqual(options[1]); }); - test('removeChip removes chip properly', () => { + test('should remove item', () => { const onRemoveItem = jest.fn(); const onChange = jest.fn(); - const wrapper = mount( ); - 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(); - expect(onChange).toBeCalled(); + + wrapper + .find('Chip') + .at(1) + .invoke('onClick')(); + + expect(onRemoveItem).toHaveBeenCalledWith(value[1]); + const newVal = onChange.mock.calls[0][0]; + expect(newVal).toHaveLength(1); + expect(newVal).toEqual([value[0]]); }); }); diff --git a/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx b/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx index 472327aaad..a398bc0311 100644 --- a/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx +++ b/awx/ui_next/src/components/MultiSelect/TagMultiSelect.jsx @@ -33,7 +33,7 @@ function TagMultiSelect({ onChange, value }) { setOptions(options.concat(newItem)); } }} - associatedItems={stringToArray(value)} + value={stringToArray(value)} options={options} createNewItem={name => ({ id: name, name })} /> diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap index 043a609933..d83714dcbb 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap @@ -693,7 +693,7 @@ exports[` initially renders succesfully 1`] = ` isReadOnly={false} onClick={[Function]} > - initially renders succesfully 1`] = ` - + diff --git a/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap index ac85a02e36..3049367398 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap @@ -402,6 +402,20 @@ exports[` initially renders succesfully 1`] = ` } onSearch={[Function]} onSort={[Function]} + qsConfig={ + Object { + "defaultParams": Object { + "order_by": "name", + "page": 1, + "page_size": 5, + }, + "integerFields": Array [ + "page", + "page_size", + ], + "namespace": "notification", + } + } sortOrder="ascending" sortedColumnKey="name" > @@ -442,6 +456,20 @@ exports[` initially renders succesfully 1`] = ` onSearch={[Function]} onSelectAll={null} onSort={[Function]} + qsConfig={ + Object { + "defaultParams": Object { + "order_by": "name", + "page": 1, + "page_size": 5, + }, + "integerFields": Array [ + "page", + "page_size", + ], + "namespace": "notification", + } + } showSelectAll={false} sortOrder="ascending" sortedColumnKey="name" diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index 85bec5dfff..0623ad7ab8 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -18,10 +18,10 @@ function JobTemplateAdd({ history, i18n }) { async function handleSubmit(values) { const { - newLabels, - removedLabels, - addedInstanceGroups, - removedInstanceGroups, + labels, + organizationId, + instanceGroups, + initialInstanceGroups, ...remainingValues } = values; @@ -31,8 +31,8 @@ function JobTemplateAdd({ history, i18n }) { data: { id, type }, } = await JobTemplatesAPI.create(remainingValues); await Promise.all([ - submitLabels(id, newLabels, removedLabels), - submitInstanceGroups(id, addedInstanceGroups, removedInstanceGroups), + submitLabels(id, labels, organizationId), + submitInstanceGroups(id, instanceGroups), ]); history.push(`/templates/${type}/${id}/details`); } catch (error) { @@ -40,22 +40,17 @@ function JobTemplateAdd({ history, i18n }) { } } - function submitLabels(id, newLabels = [], removedLabels = []) { - const disassociationPromises = removedLabels.map(label => - JobTemplatesAPI.disassociateLabel(id, label) - ); - const associationPromises = newLabels - .filter(label => !label.organization) - .map(label => JobTemplatesAPI.associateLabel(id, label)); - const creationPromises = newLabels - .filter(label => label.organization) - .map(label => JobTemplatesAPI.generateLabel(id, label)); + function submitLabels(templateId, labels = [], organizationId) { + const associationPromises = labels + .filter(label => !label.isNew) + .map(label => JobTemplatesAPI.associateLabel(templateId, label)); + const creationPromises = labels + .filter(label => label.isNew) + .map(label => + JobTemplatesAPI.generateLabel(templateId, label, organizationId) + ); - return Promise.all([ - ...disassociationPromises, - ...associationPromises, - ...creationPromises, - ]); + return Promise.all([...associationPromises, ...creationPromises]); } function submitInstanceGroups(templateId, addedGroups = []) { 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 f889f8fe97..41032535bf 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -27,6 +27,8 @@ const jobTemplateData = { host_config_key: '', }; +// TODO: Needs React/React-router upgrade to remove `act()` warnings +// See https://github.com/ansible/awx/issues/4817 describe('', () => { const defaultProps = { description: '', @@ -55,7 +57,7 @@ describe('', () => { expect(wrapper.find('JobTemplateForm').length).toBe(1); }); - test('should render Job Template Form with default values', async done => { + test('should render Job Template Form with default values', async () => { const wrapper = mountWithContexts(); await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); expect(wrapper.find('input#template-description').text()).toBe( @@ -76,14 +78,11 @@ describe('', () => { ).toEqual(true); expect(wrapper.find('input#template-name').text()).toBe(defaultProps.name); - expect(wrapper.find('AnsibleSelect[name="playbook"]').text()).toBe( - 'Choose a playbook' - ); + expect(wrapper.find('PlaybookSelect')).toHaveLength(1); expect(wrapper.find('ProjectLookup').prop('value')).toBe(null); - done(); }); - test('handleSubmit should post to api', async done => { + test('handleSubmit should post to api', async () => { JobTemplatesAPI.create.mockResolvedValueOnce({ data: { id: 1, @@ -96,7 +95,10 @@ describe('', () => { const changeState = new Promise(resolve => { formik.setState( { - values: jobTemplateData, + values: { + ...jobTemplateData, + labels: [], + }, }, () => resolve() ); @@ -105,10 +107,9 @@ describe('', () => { wrapper.find('form').simulate('submit'); await sleep(1); expect(JobTemplatesAPI.create).toHaveBeenCalledWith(jobTemplateData); - done(); }); - test('should navigate to job template detail after form submission', async done => { + test('should navigate to job template detail after form submission', async () => { const history = { push: jest.fn(), }; @@ -130,10 +131,9 @@ describe('', () => { expect(history.push).toHaveBeenCalledWith( '/templates/job_template/1/details' ); - done(); }); - test('should navigate to templates list when cancel is clicked', async done => { + test('should navigate to templates list when cancel is clicked', async () => { const history = { push: jest.fn(), }; @@ -143,6 +143,5 @@ describe('', () => { await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); expect(history.push).toHaveBeenCalledWith('/templates'); - done(); }); }); diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 8771b7c3bf..a0cf904ab2 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -6,6 +6,7 @@ import ContentError from '@components/ContentError'; import ContentLoading from '@components/ContentLoading'; import { JobTemplatesAPI, ProjectsAPI } from '@api'; import { JobTemplate } from '@types'; +import { getAddedAndRemoved } from '@util/lists'; import JobTemplateForm from '../shared/JobTemplateForm'; class JobTemplateEdit extends Component { @@ -104,10 +105,10 @@ class JobTemplateEdit extends Component { async handleSubmit(values) { const { template, history } = this.props; const { - newLabels, - removedLabels, - addedInstanceGroups, - removedInstanceGroups, + labels, + organizationId, + instanceGroups, + initialInstanceGroups, ...remainingValues } = values; @@ -115,8 +116,8 @@ class JobTemplateEdit extends Component { try { await JobTemplatesAPI.update(template.id, remainingValues); await Promise.all([ - this.submitLabels(newLabels, removedLabels), - this.submitInstanceGroups(addedInstanceGroups, removedInstanceGroups), + this.submitLabels(labels, organizationId), + this.submitInstanceGroups(instanceGroups, initialInstanceGroups), ]); history.push(this.detailsUrl); } catch (formSubmitError) { @@ -124,17 +125,23 @@ class JobTemplateEdit extends Component { } } - async submitLabels(newLabels = [], removedLabels = []) { + async submitLabels(labels = [], organizationId) { const { template } = this.props; - const disassociationPromises = removedLabels.map(label => + const { added, removed } = getAddedAndRemoved( + template.summary_fields.labels.results, + labels + ); + const disassociationPromises = removed.map(label => JobTemplatesAPI.disassociateLabel(template.id, label) ); - const associationPromises = newLabels - .filter(label => !label.organization) + const associationPromises = added + .filter(label => !label.isNew) .map(label => JobTemplatesAPI.associateLabel(template.id, label)); - const creationPromises = newLabels - .filter(label => label.organization) - .map(label => JobTemplatesAPI.generateLabel(template.id, label)); + const creationPromises = added + .filter(label => label.isNew) + .map(label => + JobTemplatesAPI.generateLabel(template.id, label, organizationId) + ); const results = await Promise.all([ ...disassociationPromises, @@ -144,12 +151,13 @@ class JobTemplateEdit extends Component { return results; } - async submitInstanceGroups(addedGroups, removedGroups) { + async submitInstanceGroups(groups, initialGroups) { const { template } = this.props; - const associatePromises = addedGroups.map(group => + const { added, removed } = getAddedAndRemoved(initialGroups, groups); + const associatePromises = added.map(group => JobTemplatesAPI.associateInstanceGroup(template.id, group.id) ); - const disassociatePromises = removedGroups.map(group => + const disassociatePromises = removed.map(group => JobTemplatesAPI.disassociateInstanceGroup(template.id, group.id) ); return Promise.all([...associatePromises, ...disassociatePromises]); 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 3f43faf279..363c1ca920 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -34,6 +34,9 @@ const mockJobTemplate = { labels: { results: [{ name: 'Sushi', id: 1 }, { name: 'Major', id: 2 }], }, + inventory: { + organization_id: 1, + }, }, }; @@ -170,15 +173,11 @@ describe('', () => { description: 'new description', job_type: 'check', }; - const newLabels = [ - { associate: true, id: 3 }, - { associate: true, id: 3 }, - { name: 'Maple', organization: 1 }, - { name: 'Tree', organization: 1 }, - ]; - const removedLabels = [ - { disassociate: true, id: 1 }, - { disassociate: true, id: 2 }, + const labels = [ + { id: 3, name: 'Foo', isNew: true }, + { id: 4, name: 'Bar', isNew: true }, + { id: 5, name: 'Maple' }, + { id: 6, name: 'Tree' }, ]; JobTemplatesAPI.update.mockResolvedValue({ data: { ...updatedTemplateData }, @@ -190,8 +189,7 @@ describe('', () => { values: { ...mockJobTemplate, ...updatedTemplateData, - newLabels, - removedLabels, + labels, }, }, () => resolve() diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 93fe52dd53..87cff2fa7e 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -7,31 +7,30 @@ import { withFormik, Field } from 'formik'; import { Form, FormGroup, - Tooltip, Card, Switch, Checkbox, TextInput, } from '@patternfly/react-core'; -import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; import ContentError from '@components/ContentError'; import ContentLoading from '@components/ContentLoading'; import AnsibleSelect from '@components/AnsibleSelect'; -import MultiSelect, { TagMultiSelect } from '@components/MultiSelect'; +import { TagMultiSelect } from '@components/MultiSelect'; import FormActionGroup from '@components/FormActionGroup'; -import FormField, { CheckboxField } from '@components/FormField'; +import FormField, { CheckboxField, FieldTooltip } from '@components/FormField'; import FormRow from '@components/FormRow'; import CollapsibleSection from '@components/CollapsibleSection'; import { required } from '@util/validators'; import styled from 'styled-components'; import { JobTemplate } from '@types'; -import { InventoryLookup, InstanceGroupsLookup } from '@components/Lookup'; -import ProjectLookup from './ProjectLookup'; -import { JobTemplatesAPI, LabelsAPI, ProjectsAPI } from '@api'; - -const QuestionCircleIcon = styled(PFQuestionCircleIcon)` - margin-left: 10px; -`; +import { + InventoryLookup, + InstanceGroupsLookup, + ProjectLookup, +} from '@components/Lookup'; +import { JobTemplatesAPI } from '@api'; +import LabelSelect from './LabelSelect'; +import PlaybookSelect from './PlaybookSelect'; const GridFormGroup = styled(FormGroup)` & > label { @@ -73,152 +72,38 @@ class JobTemplateForm extends Component { this.state = { hasContentLoading: true, contentError: false, - loadedLabels: [], - newLabels: [], - removedLabels: [], project: props.template.summary_fields.project, inventory: props.template.summary_fields.inventory, - relatedProjectPlaybooks: props.relatedProjectPlaybooks, - relatedInstanceGroups: [], allowCallbacks: !!props.template.host_config_key, }; - this.handleNewLabel = this.handleNewLabel.bind(this); - this.loadLabels = this.loadLabels.bind(this); - this.removeLabel = this.removeLabel.bind(this); this.handleProjectValidation = this.handleProjectValidation.bind(this); this.loadRelatedInstanceGroups = this.loadRelatedInstanceGroups.bind(this); - this.loadRelatedProjectPlaybooks = this.loadRelatedProjectPlaybooks.bind( - this - ); - this.handleInstanceGroupsChange = this.handleInstanceGroupsChange.bind( - this - ); } componentDidMount() { const { validateField } = this.props; this.setState({ contentError: null, hasContentLoading: true }); - Promise.all([this.loadLabels(), this.loadRelatedInstanceGroups()]).then( - () => { - this.setState({ hasContentLoading: false }); - validateField('project'); - } - ); - } - - async loadLabels() { - // This function 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. - let loadedLabels; - try { - const { data } = await LabelsAPI.read({ - page: 1, - page_size: 200, - order_by: 'name', - }); - loadedLabels = [...data.results]; - if (data.next && data.next.includes('page=2')) { - 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 }); - } + // TODO: determine when LabelSelect has finished loading labels + Promise.all([this.loadRelatedInstanceGroups()]).then(() => { + this.setState({ hasContentLoading: false }); + validateField('project'); + }); } async loadRelatedInstanceGroups() { - const { template } = this.props; + const { setFieldValue, template } = this.props; if (!template.id) { return; } try { const { data } = await JobTemplatesAPI.readInstanceGroups(template.id); - this.setState({ - initialInstanceGroups: data.results, - relatedInstanceGroups: [...data.results], - }); + setFieldValue('initialInstanceGroups', data.results); + setFieldValue('instanceGroups', [...data.results]); } catch (err) { this.setState({ contentError: err }); } } - handleNewLabel(label) { - const { newLabels } = this.state; - const { template, setFieldValue } = 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 { - setFieldValue('newLabels', [ - ...newLabels, - { name: label.name, associate: true, id: label.id }, - ]); - this.setState({ - newLabels: [ - ...newLabels, - { - name: label.name, - associate: true, - id: label.id, - organization: template.summary_fields.inventory.organization_id, - }, - ], - }); - } - } - - removeLabel(label) { - const { removedLabels, newLabels } = this.state; - const { template, setFieldValue } = this.props; - - const isAssociatedLabel = template.summary_fields.labels.results.some( - tempLabel => tempLabel.id === label.id - ); - - if (isAssociatedLabel) { - setFieldValue( - 'removedLabels', - removedLabels.concat({ - disassociate: true, - id: label.id, - }) - ); - this.setState({ - removedLabels: removedLabels.concat({ - disassociate: true, - id: label.id, - }), - }); - } else { - const filteredLabels = newLabels.filter( - newLabel => newLabel.name !== label.name - ); - setFieldValue('newLabels', filteredLabels); - this.setState({ newLabels: filteredLabels }); - } - } - - async loadRelatedProjectPlaybooks(project) { - try { - const { data: playbooks = [] } = await ProjectsAPI.readPlaybooks(project); - this.setState({ relatedProjectPlaybooks: playbooks }); - } catch (contentError) { - this.setState({ contentError }); - } - } - handleProjectValidation() { const { i18n, touched } = this.props; const { project } = this.state; @@ -233,45 +118,19 @@ class JobTemplateForm extends Component { }; } - handleInstanceGroupsChange(relatedInstanceGroups) { - const { setFieldValue } = this.props; - const { initialInstanceGroups } = this.state; - let added = []; - const removed = []; - if (initialInstanceGroups) { - initialInstanceGroups.forEach(group => { - if (!relatedInstanceGroups.find(g => g.id === group.id)) { - removed.push(group); - } - }); - relatedInstanceGroups.forEach(group => { - if (!initialInstanceGroups.find(g => g.id === group.id)) { - added.push(group); - } - }); - } else { - added = relatedInstanceGroups; - } - setFieldValue('addedInstanceGroups', added); - setFieldValue('removedInstanceGroups', removed); - this.setState({ relatedInstanceGroups }); - } - render() { const { - loadedLabels, contentError, hasContentLoading, inventory, project, - relatedProjectPlaybooks = [], - relatedInstanceGroups, allowCallbacks, } = this.state; const { handleCancel, handleSubmit, handleBlur, + setFieldValue, i18n, template, } = this.props; @@ -290,28 +149,6 @@ class JobTemplateForm extends Component { isDisabled: false, }, ]; - const playbookOptions = relatedProjectPlaybooks - .map(playbook => { - return { - value: playbook, - key: playbook, - label: playbook, - isDisabled: false, - }; - }) - .reduce( - (arr, playbook) => { - return arr.concat(playbook); - }, - [ - { - value: '', - key: '', - label: i18n._(t`Choose a playbook`), - isDisabled: false, - }, - ] - ); const verbosityOptions = [ { value: '0', key: '0', label: i18n._(t`0 (Normal)`) }, @@ -374,15 +211,12 @@ class JobTemplateForm extends Component { isValid={isValid} label={i18n._(t`Job Type`)} > - - - + the playbook. Select check to only check playbook syntax, + test environment setup, and report problems without + executing the playbook.`)} + /> { form.setFieldValue('inventory', value.id); + form.setFieldValue('organizationId', value.organization); this.setState({ inventory: value }); }} required @@ -421,7 +256,6 @@ class JobTemplateForm extends Component { tooltip={i18n._(t`Select the project containing the playbook you want this job to execute.`)} onChange={value => { - this.loadRelatedProjectPlaybooks(value.id); form.setFieldValue('project', value.id); this.setState({ project: value }); }} @@ -443,20 +277,17 @@ class JobTemplateForm extends Component { isValid={isValid} label={i18n._(t`Playbook`)} > - - - - + this.setState({ contentError: err })} /> ); @@ -464,22 +295,23 @@ class JobTemplateForm extends Component { /> - - - - - - + ( + + + setFieldValue('labels', labels)} + onError={err => this.setState({ contentError: err })} + /> + + )} + /> @@ -519,13 +351,10 @@ class JobTemplateForm extends Component { fieldId="template-verbosity" label={i18n._(t`Verbosity`)} > - - - + produce as the playbook executes.`)} + /> - - - + Ansible tasks, where supported. This is equivalent + to Ansible’s --diff mode.`)} + />
- ( + form.setFieldValue(field.name, value)} + tooltip={i18n._(t`Select the Instance Groups for this Organization + to run on.`)} + /> )} /> - - - + playbook, and you want to run a specific part of a + play or task. Use commas to separate multiple tags. + Refer to Ansible Tower documentation for details on + the usage of tags.`)} + /> form.setFieldValue(field.name, value)} @@ -624,16 +451,13 @@ class JobTemplateForm extends Component { css="margin-top: 20px" fieldId="template-skip-tags" > - - - + /> form.setFieldValue(field.name, value)} @@ -651,9 +475,8 @@ class JobTemplateForm extends Component { id="option-privilege-escalation" name="become_enabled" label={i18n._(t`Privilege Escalation`)} - tooltip={i18n._( - t`If enabled, run this playbook as an administrator.` - )} + tooltip={i18n._(t`If enabled, run this playbook as an + administrator.`)} /> {i18n._(t`Provisioning Callbacks`)}   - - - + } id="option-callbacks" @@ -683,19 +502,15 @@ class JobTemplateForm extends Component { id="option-concurrent" name="allow_simultaneous" label={i18n._(t`Concurrent Jobs`)} - tooltip={i18n._( - t`If enabled, simultaneous runs of this job template will - be allowed.` - )} + tooltip={i18n._(t`If enabled, simultaneous runs of this job + template will be allowed.`)} />
bag.props.handleSubmit(values), + handleSubmit: (values, { props }) => props.handleSubmit(values), })(JobTemplateForm); export { JobTemplateForm as _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 71a470112c..cf2da14304 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx @@ -1,8 +1,8 @@ import React from 'react'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; -import JobTemplateForm, { _JobTemplateForm } from './JobTemplateForm'; -import { LabelsAPI, JobTemplatesAPI } from '@api'; +import JobTemplateForm from './JobTemplateForm'; +import { LabelsAPI, JobTemplatesAPI, ProjectsAPI } from '@api'; jest.mock('@api'); @@ -61,13 +61,16 @@ describe('', () => { JobTemplatesAPI.readInstanceGroups.mockReturnValue({ data: { results: mockInstanceGroups }, }); + ProjectsAPI.readPlaybooks.mockReturnValue({ + data: ['debug.yml'], + }); }); afterEach(() => { jest.clearAllMocks(); }); - test('should render labels MultiSelect', async () => { + test('should render LabelsSelect', async () => { const wrapper = mountWithContexts( ', () => { expect(LabelsAPI.read).toHaveBeenCalled(); expect(JobTemplatesAPI.readInstanceGroups).toHaveBeenCalled(); wrapper.update(); - expect( - wrapper - .find('FormGroup[fieldId="template-labels"] MultiSelect') - .prop('associatedItems') - ).toEqual(mockData.summary_fields.labels.results); + const select = wrapper.find('LabelSelect'); + expect(select).toHaveLength(1); + expect(select.prop('value')).toEqual( + mockData.summary_fields.labels.results + ); }); test('should update form values on input changes', async () => { @@ -155,75 +158,4 @@ describe('', () => { wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); expect(handleCancel).toBeCalled(); }); - - test('should call loadRelatedProjectPlaybooks when project value changes', async () => { - const loadRelatedProjectPlaybooks = jest.spyOn( - _JobTemplateForm.prototype, - 'loadRelatedProjectPlaybooks' - ); - const wrapper = mountWithContexts( - - ); - await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); - wrapper.find('ProjectLookup').prop('onChange')({ - id: 10, - name: 'project', - }); - expect(loadRelatedProjectPlaybooks).toHaveBeenCalledWith(10); - }); - - test('handleNewLabel should arrange new labels properly', async () => { - const event = { key: 'Enter', preventDefault: () => {} }; - const wrapper = mountWithContexts( - - ); - await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); - const multiSelect = wrapper.find( - 'FormGroup[fieldId="template-labels"] MultiSelect' - ); - const component = wrapper.find('JobTemplateForm'); - - wrapper.setState({ newLabels: [], loadedLabels: [], removedLabels: [] }); - multiSelect.setState({ input: 'Foo' }); - component - .find('FormGroup[fieldId="template-labels"] input[aria-label="labels"]') - .prop('onKeyDown')(event); - - component.instance().handleNewLabel({ name: 'Bar', id: 2 }); - const newLabels = component.state('newLabels'); - expect(newLabels).toHaveLength(2); - expect(newLabels[0].name).toEqual('Foo'); - expect(newLabels[0].organization).toEqual(1); - }); - - test('disassociateLabel should arrange new labels properly', async () => { - const wrapper = mountWithContexts( - - ); - await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); - const component = wrapper.find('JobTemplateForm'); - // 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); - // 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); - }); }); diff --git a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx new file mode 100644 index 0000000000..3443aeb3b4 --- /dev/null +++ b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx @@ -0,0 +1,61 @@ +import React, { useState, useEffect } from 'react'; +import { func, arrayOf, number, shape, string, oneOfType } from 'prop-types'; +import MultiSelect from '@components/MultiSelect'; +import { LabelsAPI } from '@api'; + +async function loadLabelOptions(setLabels, onError) { + let labels; + try { + const { data } = await LabelsAPI.read({ + page: 1, + page_size: 200, + order_by: 'name', + }); + labels = data.results; + setLabels(labels); + if (data.next && data.next.includes('page=2')) { + const { + data: { results }, + } = await LabelsAPI.read({ + page: 2, + page_size: 200, + order_by: 'name', + }); + labels = labels.concat(results); + } + setLabels(labels); + } catch (err) { + onError(err); + } +} + +function LabelSelect({ value, onChange, onError }) { + const [options, setOptions] = useState([]); + useEffect(() => { + loadLabelOptions(setOptions, onError); + }, []); + + return ( + ({ + id: name, + name, + isNew: true, + })} + /> + ); +} +LabelSelect.propTypes = { + value: arrayOf( + shape({ + id: oneOfType([number, string]).isRequired, + name: string.isRequired, + }) + ).isRequired, + onError: func.isRequired, +}; + +export default LabelSelect; diff --git a/awx/ui_next/src/screens/Template/shared/LabelSelect.test.jsx b/awx/ui_next/src/screens/Template/shared/LabelSelect.test.jsx new file mode 100644 index 0000000000..b03d4e1572 --- /dev/null +++ b/awx/ui_next/src/screens/Template/shared/LabelSelect.test.jsx @@ -0,0 +1,50 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import { LabelsAPI } from '@api'; +import { sleep } from '@testUtils/testUtils'; +import LabelSelect from './LabelSelect'; + +jest.mock('@api'); + +const options = [{ id: 1, name: 'one' }, { id: 2, name: 'two' }]; + +describe('', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + test('should fetch labels', async () => { + LabelsAPI.read.mockReturnValue({ + data: { results: options }, + }); + const wrapper = mount(); + await sleep(1); + wrapper.update(); + + expect(LabelsAPI.read).toHaveBeenCalledTimes(1); + expect(wrapper.find('MultiSelect').prop('options')).toEqual(options); + }); + + test('should fetch two pages labels if present', async () => { + LabelsAPI.read.mockReturnValueOnce({ + data: { + results: options, + next: '/foo?page=2', + }, + }); + LabelsAPI.read.mockReturnValueOnce({ + data: { + results: options, + }, + }); + const wrapper = mount(); + await sleep(1); + wrapper.update(); + + expect(LabelsAPI.read).toHaveBeenCalledTimes(2); + expect(wrapper.find('MultiSelect').prop('options')).toEqual([ + ...options, + ...options, + ]); + }); +}); diff --git a/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx new file mode 100644 index 0000000000..3e7d03c37e --- /dev/null +++ b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.jsx @@ -0,0 +1,54 @@ +import React, { useState, useEffect } from 'react'; +import { number, string, oneOfType } from 'prop-types'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import AnsibleSelect from '@components/AnsibleSelect'; +import { ProjectsAPI } from '@api'; + +function PlaybookSelect({ projectId, isValid, form, field, onError, i18n }) { + const [options, setOptions] = useState([]); + useEffect(() => { + if (!projectId) { + return; + } + (async () => { + try { + const { data } = await ProjectsAPI.readPlaybooks(projectId); + const opts = (data || []).map(playbook => ({ + value: playbook, + key: playbook, + label: playbook, + isDisabled: false, + })); + opts.unshift({ + value: '', + key: '', + label: i18n._(t`Choose a playbook`), + isDisabled: false, + }); + setOptions(opts); + } catch (contentError) { + onError(contentError); + } + })(); + }, [projectId]); + + return ( + + ); +} +PlaybookSelect.propTypes = { + projectId: oneOfType([number, string]), +}; +PlaybookSelect.defaultProps = { + projectId: null, +}; + +export { PlaybookSelect as _PlaybookSelect }; +export default withI18n()(PlaybookSelect); diff --git a/awx/ui_next/src/screens/Template/shared/PlaybookSelect.test.jsx b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.test.jsx new file mode 100644 index 0000000000..3d1a26355a --- /dev/null +++ b/awx/ui_next/src/screens/Template/shared/PlaybookSelect.test.jsx @@ -0,0 +1,36 @@ +import React from 'react'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import PlaybookSelect from './PlaybookSelect'; +import { ProjectsAPI } from '@api'; + +jest.mock('@api'); + +describe('', () => { + beforeEach(() => { + ProjectsAPI.readPlaybooks.mockReturnValue({ + data: ['debug.yml'], + }); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + test('should reload playbooks when project value changes', () => { + const wrapper = mountWithContexts( + {}} + /> + ); + + expect(ProjectsAPI.readPlaybooks).toHaveBeenCalledWith(1); + wrapper.setProps({ projectId: 15 }); + + expect(ProjectsAPI.readPlaybooks).toHaveBeenCalledTimes(2); + expect(ProjectsAPI.readPlaybooks).toHaveBeenCalledWith(15); + }); +}); diff --git a/awx/ui_next/src/util/lists.js b/awx/ui_next/src/util/lists.js new file mode 100644 index 0000000000..561c306faf --- /dev/null +++ b/awx/ui_next/src/util/lists.js @@ -0,0 +1,18 @@ +/* eslint-disable import/prefer-default-export */ +export function getAddedAndRemoved(original, current) { + original = original || []; + current = current || []; + const added = []; + const removed = []; + original.forEach(orig => { + if (!current.find(cur => cur.id === orig.id)) { + removed.push(orig); + } + }); + current.forEach(cur => { + if (!original.find(orig => orig.id === cur.id)) { + added.push(cur); + } + }); + return { added, removed }; +} diff --git a/awx/ui_next/src/util/lists.test.js b/awx/ui_next/src/util/lists.test.js new file mode 100644 index 0000000000..126de4f07e --- /dev/null +++ b/awx/ui_next/src/util/lists.test.js @@ -0,0 +1,51 @@ +import { getAddedAndRemoved } from './lists'; + +const one = { id: 1 }; +const two = { id: 2 }; +const three = { id: 3 }; + +describe('getAddedAndRemoved', () => { + test('should handle no original list', () => { + const items = [one, two, three]; + expect(getAddedAndRemoved(null, items)).toEqual({ + added: items, + removed: [], + }); + }); + + test('should list added item', () => { + const original = [one, two]; + const current = [one, two, three]; + expect(getAddedAndRemoved(original, current)).toEqual({ + added: [three], + removed: [], + }); + }); + + test('should list removed item', () => { + const original = [one, two, three]; + const current = [one, three]; + expect(getAddedAndRemoved(original, current)).toEqual({ + added: [], + removed: [two], + }); + }); + + test('should handle both added and removed together', () => { + const original = [two]; + const current = [one, three]; + expect(getAddedAndRemoved(original, current)).toEqual({ + added: [one, three], + removed: [two], + }); + }); + + test('should handle different list order', () => { + const original = [three, two]; + const current = [one, two, three]; + expect(getAddedAndRemoved(original, current)).toEqual({ + added: [one], + removed: [], + }); + }); +});