Rework saving labels

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) {
+  generateLabel(id, label, orgId) {
-    return this.http.post(`${this.baseUrl}${orgId}/labels/`, label);
+    return this.http.post(`${this.baseUrl}${id}/labels/`, {
+      name: label.name,
+      organization: orgId,
+    });
   }
 
   readCredentials(id, params) {

awx/ui_next/src/components/MultiSelect/MultiSelect.jsx

@@ -144,8 +144,8 @@ class MultiSelect extends Component {
     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);
+      const items = chipItems.concat(newItem);
       this.setState({
         chipItems: items,
         isExpanded: false,

awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx

@@ -18,8 +18,8 @@ function JobTemplateAdd({ history, i18n }) {
 
   async function handleSubmit(values) {
     const {
-      newLabels,
+      labels,
-      removedLabels,
+      organizationId,
       addedInstanceGroups,
       removedInstanceGroups,
       ...remainingValues
@@ -31,7 +31,7 @@ function JobTemplateAdd({ history, i18n }) {
         data: { id, type },
       } = await JobTemplatesAPI.create(remainingValues);
       await Promise.all([
-        submitLabels(id, newLabels, removedLabels),
+        submitLabels(id, labels, organizationId),
         submitInstanceGroups(id, addedInstanceGroups, removedInstanceGroups),
       ]);
       history.push(`/templates/${type}/${id}/details`);
@@ -40,22 +40,17 @@ function JobTemplateAdd({ history, i18n }) {
     }
   }
 
-  function submitLabels(id, newLabels = [], removedLabels = []) {
+  function submitLabels(templateId, labels = [], organizationId) {
-    const disassociationPromises = removedLabels.map(label =>
+    const associationPromises = labels
-      JobTemplatesAPI.disassociateLabel(id, label)
+      .filter(label => !label.isNew)
-    );
+      .map(label => JobTemplatesAPI.associateLabel(templateId, label));
-    const associationPromises = newLabels
+    const creationPromises = labels
-      .filter(label => !label.organization)
+      .filter(label => label.isNew)
-      .map(label => JobTemplatesAPI.associateLabel(id, label));
+      .map(label =>
-    const creationPromises = newLabels
+        JobTemplatesAPI.generateLabel(templateId, label, organizationId)
-      .filter(label => label.organization)
+      );
-      .map(label => JobTemplatesAPI.generateLabel(id, label));
 
-    return Promise.all([
+    return Promise.all([...associationPromises, ...creationPromises]);
-      ...disassociationPromises,
-      ...associationPromises,
-      ...creationPromises,
-    ]);
   }
 
   function submitInstanceGroups(templateId, addedGroups = []) {

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,8 +105,8 @@ class JobTemplateEdit extends Component {
   async handleSubmit(values) {
     const { template, history } = this.props;
     const {
-      newLabels,
+      labels,
-      removedLabels,
+      organizationId,
       addedInstanceGroups,
       removedInstanceGroups,
       ...remainingValues
@@ -115,7 +116,7 @@ class JobTemplateEdit extends Component {
     try {
       await JobTemplatesAPI.update(template.id, remainingValues);
       await Promise.all([
-        this.submitLabels(newLabels, removedLabels),
+        this.submitLabels(labels, organizationId),
         this.submitInstanceGroups(addedInstanceGroups, removedInstanceGroups),
       ]);
       history.push(this.detailsUrl);
@@ -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
+    const associationPromises = added
-      .filter(label => !label.organization)
+      .filter(label => !label.isNew)
       .map(label => JobTemplatesAPI.associateLabel(template.id, label));
-    const creationPromises = newLabels
+    const creationPromises = added
-      .filter(label => label.organization)
+      .filter(label => label.isNew)
-      .map(label => JobTemplatesAPI.generateLabel(template.id, label));
+      .map(label =>
+        JobTemplatesAPI.generateLabel(template.id, label, organizationId)
+      );
 
     const results = await Promise.all([
       ...disassociationPromises,

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

@@ -87,13 +87,11 @@ class JobTemplateForm extends Component {
   componentDidMount() {
     const { validateField } = this.props;
     this.setState({ contentError: null, hasContentLoading: true });
-    // TODO: determine whene LabelSelect has finished loading labels?
+    // TODO: determine whene LabelSelect has finished loading labels
-    Promise.all([this.loadRelatedInstanceGroups()]).then(
+    Promise.all([this.loadRelatedInstanceGroups()]).then(() => {
-      () => {
+      this.setState({ hasContentLoading: false });
-        this.setState({ hasContentLoading: false });
+      validateField('project');
-        validateField('project');
+    });
-      }
-    );
   }
 
   async loadRelatedInstanceGroups() {
@@ -270,6 +268,7 @@ class JobTemplateForm extends Component {
                   you want this job to manage.`)}
                 onChange={value => {
                   form.setFieldValue('inventory', value.id);
+                  form.setFieldValue('organizationId', value.organization);
                   this.setState({ inventory: value });
                 }}
                 required
@@ -335,12 +334,7 @@ class JobTemplateForm extends Component {
             />
             <LabelSelect
               initialValues={template.summary_fields.labels.results}
-              onNewLabelsChange={newLabels => {
+              onChange={labels => setFieldValue('labels', labels)}
-                setFieldValue('newLabels', newLabels);
-              }}
-              onRemovedLabelsChange={removedLabels => {
-                setFieldValue('removedLabels', removedLabels);
-              }}
               onError={err => this.setState({ contentError: err })}
             />
           </FormGroup>
@@ -577,7 +571,10 @@ class JobTemplateForm extends Component {
 const FormikApp = withFormik({
   mapPropsToValues(props) {
     const { template = {} } = props;
-    const { summary_fields = { labels: { results: [] } } } = template;
+    const { summary_fields = {
+      labels: { results: [] },
+      inventory: { organization: null },
+    } } = template;
 
     return {
       name: template.name || '',
@@ -600,13 +597,12 @@ const FormikApp = withFormik({
       allow_simultaneous: template.allow_simultaneous || false,
       use_fact_cache: template.use_fact_cache || false,
       host_config_key: template.host_config_key || '',
+      organizationId: summary_fields.inventory.organization_id || null,
       addedInstanceGroups: [],
       removedInstanceGroups: [],
-      newLabels: [],
-      removedLabels: [],
     };
   },
-  handleSubmit: (values, bag) => bag.props.handleSubmit(values),
+  handleSubmit: (values, { props }) => props.handleSubmit(values),
 })(JobTemplateForm);
 
 export { JobTemplateForm as _JobTemplateForm };

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

@@ -1,7 +1,7 @@
 import React from 'react';
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
 import { sleep } from '@testUtils/testUtils';
-import JobTemplateForm, { _JobTemplateForm } from './JobTemplateForm';
+import JobTemplateForm from './JobTemplateForm';
 import { LabelsAPI, JobTemplatesAPI, ProjectsAPI } from '@api';
 
 jest.mock('@api');

awx/ui_next/src/screens/Template/shared/LabelSelect.jsx

@@ -30,63 +30,26 @@ async function loadLabelOptions(setLabels, onError) {
 }
 
 function LabelSelect({
-  initialValues,
+  initialValues, // todo: change to value, controlled ?
-  onNewLabelsChange,
+  onChange,
-  onRemovedLabelsChange,
   onError,
 }) {
   const [options, setOptions] = useState([]);
   // TODO: move newLabels into a prop?
-  const [newLabels, setNewLabels] = useState([]);
-  const [removedLabels, setRemovedLabels] = useState([]);
   useEffect(() => {
     loadLabelOptions(setOptions, onError);
   }, []);
 
-  const handleNewLabel = label => {
-    const isIncluded = newLabels.some(l => l.name === label.name);
-    if (isIncluded) {
-      const filteredLabels = newLabels.filter(
-        newLabel => newLabel.name !== label
-      );
-      setNewLabels(filteredLabels);
-    } else {
-      const updatedNewLabels = newLabels.concat({
-        name: label.name,
-        associate: true,
-        id: label.id,
-        // TODO: can this be null? what happens if inventory > org id changes?
-        // organization: organizationId,
-      });
-      setNewLabels(updatedNewLabels);
-      onNewLabelsChange(updatedNewLabels);
-    }
-  };
-
-  const handleRemoveLabel = label => {
-    const isAssociatedLabel = initialValues.some(
-      l => l.id === label.id
-    );
-    if (isAssociatedLabel) {
-      const updatedRemovedLabels = removedLabels.concat({
-        id: label.id,
-        disassociate: true,
-      });
-      setRemovedLabels(updatedRemovedLabels);
-      onRemovedLabelsChange(updatedRemovedLabels);
-    } else {
-      const filteredLabels = newLabels.filter(l => l.name !== label.name);
-      setNewLabels(filteredLabels);
-      onNewLabelsChange(filteredLabels);
-    }
-  };
-
   return (
     <MultiSelect
-      onAddNewItem={handleNewLabel}
+      onChange={onChange}
-      onRemoveItem={handleRemoveLabel}
       associatedItems={initialValues}
       options={options}
+      createNewItem={name => ({
+        id: name,
+        name,
+        isNew: true,
+      })}
     />
   );
 }
@@ -97,8 +60,6 @@ LabelSelect.propTypes = {
       name: string.isRequired,
     })
   ).isRequired,
-  onNewLabelsChange: func.isRequired,
-  onRemovedLabelsChange: func.isRequired,
   onError: func.isRequired,
 };
 

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 };
+}

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: [],
+    });
+  });
+});