Fixes failing test and addresses PR issues

awx/ui_next/src/api/Base.js

@@ -1,8 +1,6 @@
 import axios from 'axios';
 
-import {
+import { encodeQueryString } from '@util/qs';
-  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,
     });
   }
 

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) {
+  associateLabel(id, label) {
-    return this.http.post(`${this.baseUrl}${id}/labels/`, data)
+    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);
   }
 }
 

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') {
+    if (event.key !== 'Tab') {
-      this.setState({
+      return;
-        chipItems: chipItems.concat(newChip),
-        isExpanded: false,
-        input: '',
-      });
-
-      onAddNewItem(input);
     }
+    this.setState({
+      chipItems: chipItems.concat(newChip),
+      isExpanded: false,
+      input: '',
+    });
+
+    onAddNewItem(input);
   }
 
   handleInputChange(e) {

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('<MultiSelect />', () => {
   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(
       <MultiSelect
         onAddNewItem={jest.fn()}
@@ -21,7 +25,7 @@ describe('<MultiSelect />', () => {
     );
     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('<MultiSelect />', () => {
         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', () => {

awx/ui_next/src/components/MultiSelect/index.js

@@ -1,4 +1 @@
-export {
+export { default } from './MultiSelect';
-  default
-}
-from './MultiSelect';

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
+    const disassociatedLabels = removedLabels.forEach(removedLabel =>
-      ? removedLabels.forEach(removedLabel =>
+      JobTemplatesAPI.disassociateLabel(id, removedLabel)
-          JobTemplatesAPI.updateLabels(id, removedLabel)
+    );
-        )
-      : null;
     const associatedLabels = newLabels
-      ? newLabels.forEach(newLabel =>
+      .filter(newLabel => !newLabel.organization)
-          JobTemplatesAPI.updateLabels(id, newLabel)
+      .forEach(newLabel => JobTemplatesAPI.associateLabel(id, newLabel));
-        )
+    const generatedLabels = newLabels
-      : null;
+      .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) {

awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx

@@ -36,9 +36,26 @@ describe('<JobTemplateEdit />', () => {
       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', () => {

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) {
+  removeLabel(label) {
-    const { removedLabels, loadedLabels, newLabels } = this.state;
+    const { removedLabels, newLabels } = this.state;
-    const isNewCreatedLabel = loadedLabels.some(
+    const { template } = this.props;
-      loadedLabel => loadedLabel.name !== label.name
+
+    const isAssociatedLabel = template.summary_fields.labels.results.some(
+      tempLabel => tempLabel.id === label.id
     );
-    if (isNewCreatedLabel) {
+
-      const filteredLabels = newLabels.filter(
+    if (isAssociatedLabel) {
-        newLabel => newLabel.name !== label.name
-      );
-      this.setState({ newLabels: filteredLabels });
-    } else {
       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 {
                 </Tooltip>
                 <MultiSelect
                   onAddNewItem={this.handleNewLabel}
-                  onRemoveItem={this.disassociateLabel}
+                  onRemoveItem={this.removeLabel}
                   associatedItems={template.summary_fields.labels.results}
                   options={loadedLabels}
                 />

awx/ui_next/src/screens/Template/shared/JobTemplateForm.test.jsx

@@ -36,14 +36,16 @@ describe('<JobTemplateForm />', () => {
   });
 
   test('initially renders successfully', () => {
-    mountWithContexts(
+    const wrapper = mountWithContexts(
       <JobTemplateForm
         template={mockData}
         handleSubmit={jest.fn()}
         handleCancel={jest.fn()}
       />
     );
+    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('<JobTemplateForm />', () => {
 
     wrapper.setState({ newLabels: [], loadedLabels: [], removedLabels: [] });
     multiSelect.setState({ input: 'Foo' });
-
+    component.find('input[aria-label="labels"]').prop('onKeyDown')(event);
-    wrapper.find('input[aria-label="labels"]').prop('onKeyDown')(event);
     expect(handleNewLabel).toHaveBeenCalledWith('Foo');
 
     component.instance().handleNewLabel({ name: 'Bar', id: 2 });
@@ -149,21 +150,16 @@ describe('<JobTemplateForm />', () => {
         handleCancel={jest.fn()}
       />
     );
-    const multiSelect = wrapper.find('MultiSelect');
     const component = wrapper.find('JobTemplateForm');
-
+    // This asserts that the user generated a label or clicked
-    component.setState({
+    // on a label option, and then changed their mind and
-      newLabels: [{ name: 'Foo', id: 1 }],
+    // removed the label.
-      loadedLabels: [{ name: 'Bar', id: 3 }],
+    component.instance().removeLabel({ name: 'Alex', id: 17 });
-      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);
-
+    // This asserts that the user removed a label that was associated
-    component.instance().disassociateLabel({ name: 'Bar', id: 3 });
+    // 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);
   });