diff --git a/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js b/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js index e6745ac522..c82909df47 100644 --- a/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js +++ b/awx/ui_next/src/api/mixins/InstanceGroups.mixin.js @@ -1,3 +1,12 @@ +function isEqual(array1, array2) { + return ( + array1.length === array2.length && + array1.every((element, index) => { + return element.id === array2[index].id; + }) + ); +} + const InstanceGroupsMixin = parent => class extends parent { readInstanceGroups(resourceId, params) { @@ -18,6 +27,19 @@ const InstanceGroupsMixin = parent => disassociate: true, }); } + + async orderInstanceGroups(resourceId, current, original) { + /* eslint-disable no-await-in-loop, no-restricted-syntax */ + if (!isEqual(current, original)) { + for (const group of original) { + await this.disassociateInstanceGroup(resourceId, group.id); + } + for (const group of current) { + await this.associateInstanceGroup(resourceId, group.id); + } + } + } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ }; export default InstanceGroupsMixin; diff --git a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx index a6da540a70..12f2ceb3d6 100644 --- a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx @@ -2,7 +2,7 @@ import React, { useCallback, useEffect } from 'react'; import { arrayOf, string, func, bool } from 'prop-types'; import { withRouter } from 'react-router-dom'; -import { t } from '@lingui/macro'; +import { t, Trans } from '@lingui/macro'; import { FormGroup } from '@patternfly/react-core'; import { InstanceGroupsAPI } from '../../api'; import { InstanceGroup } from '../../types'; @@ -82,6 +82,18 @@ function InstanceGroupsLookup({ multiple required={required} isLoading={isLoading} + modalDescription={ + <> + + Selected + +
+ + Note: The order in which these are selected sets the execution + precedence. + + + } renderOptionsList={({ state, dispatch, canDelete }) => ( dispatch({ type: 'SELECT_ITEM', item })} deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + sortSelectedItems={selectedItems => + dispatch({ type: 'SET_SELECTED_ITEMS', selectedItems }) + } + isSelectedDraggable /> )} /> diff --git a/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx b/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx index cad2449b3a..703ab9a946 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryAdd/InventoryAdd.jsx @@ -23,12 +23,11 @@ function InventoryAdd() { organization: organization.id, ...remainingValues, }); - if (instanceGroups) { - const associatePromises = instanceGroups.map(async ig => - InventoriesAPI.associateInstanceGroup(inventoryId, ig.id) - ); - await Promise.all(associatePromises); + /* eslint-disable no-await-in-loop, no-restricted-syntax */ + for (const group of instanceGroups) { + await InventoriesAPI.associateInstanceGroup(inventoryId, group.id); } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ const url = history.location.pathname.startsWith( '/inventories/smart_inventory' ) diff --git a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx index 535656bfab..bb7ea5eef5 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx @@ -6,7 +6,6 @@ import { CardBody } from '../../../components/Card'; import { InventoriesAPI } from '../../../api'; import ContentLoading from '../../../components/ContentLoading'; import InventoryForm from '../shared/InventoryForm'; -import { getAddedAndRemoved } from '../../../util/lists'; import useIsMounted from '../../../util/useIsMounted'; function InventoryEdit({ inventory }) { @@ -54,20 +53,12 @@ function InventoryEdit({ inventory }) { organization: organization.id, ...remainingValues, }); - if (instanceGroups) { - const { added, removed } = getAddedAndRemoved( - associatedInstanceGroups, - instanceGroups - ); + await InventoriesAPI.orderInstanceGroups( + inventory.id, + instanceGroups, + associatedInstanceGroups + ); - const associatePromises = added.map(async ig => - InventoriesAPI.associateInstanceGroup(inventory.id, ig.id) - ); - const disassociatePromises = removed.map(async ig => - InventoriesAPI.disassociateInstanceGroup(inventory.id, ig.id) - ); - await Promise.all([...associatePromises, ...disassociatePromises]); - } const url = history.location.pathname.search('smart') > -1 ? `/inventories/smart_inventory/${inventory.id}/details` diff --git a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.test.jsx index 441050104c..5b5e21017e 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.test.jsx @@ -106,17 +106,10 @@ describe('', () => { }); }); await sleep(0); - instanceGroups.map(IG => - expect(InventoriesAPI.associateInstanceGroup).toHaveBeenCalledWith( - 1, - IG.id - ) - ); - associatedInstanceGroups.map(async aIG => - expect(InventoriesAPI.disassociateInstanceGroup).toHaveBeenCalledWith( - 1, - aIG.id - ) + expect(InventoriesAPI.orderInstanceGroups).toHaveBeenCalledWith( + mockInventory.id, + instanceGroups, + associatedInstanceGroups ); }); }); diff --git a/awx/ui_next/src/screens/Inventory/SmartInventoryAdd/SmartInventoryAdd.jsx b/awx/ui_next/src/screens/Inventory/SmartInventoryAdd/SmartInventoryAdd.jsx index 3accf3d0c5..91ec24df6d 100644 --- a/awx/ui_next/src/screens/Inventory/SmartInventoryAdd/SmartInventoryAdd.jsx +++ b/awx/ui_next/src/screens/Inventory/SmartInventoryAdd/SmartInventoryAdd.jsx @@ -19,11 +19,11 @@ function SmartInventoryAdd() { data: { id: invId }, } = await InventoriesAPI.create(values); - await Promise.all( - groupsToAssociate.map(({ id }) => - InventoriesAPI.associateInstanceGroup(invId, id) - ) - ); + /* eslint-disable no-await-in-loop, no-restricted-syntax */ + for (const group of groupsToAssociate) { + await InventoriesAPI.associateInstanceGroup(invId, group.id); + } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ return invId; }, []) ); diff --git a/awx/ui_next/src/screens/Inventory/SmartInventoryEdit/SmartInventoryEdit.jsx b/awx/ui_next/src/screens/Inventory/SmartInventoryEdit/SmartInventoryEdit.jsx index b499efd3f7..8516ae4e76 100644 --- a/awx/ui_next/src/screens/Inventory/SmartInventoryEdit/SmartInventoryEdit.jsx +++ b/awx/ui_next/src/screens/Inventory/SmartInventoryEdit/SmartInventoryEdit.jsx @@ -1,7 +1,6 @@ import React, { useCallback, useEffect } from 'react'; import { useHistory } from 'react-router-dom'; import { Inventory } from '../../../types'; -import { getAddedAndRemoved } from '../../../util/lists'; import useRequest from '../../../util/useRequest'; import { InventoriesAPI } from '../../../api'; import { CardBody } from '../../../components/Card'; @@ -17,7 +16,7 @@ function SmartInventoryEdit({ inventory }) { error: contentError, isLoading: hasContentLoading, request: fetchInstanceGroups, - result: instanceGroups, + result: initialInstanceGroups, } = useRequest( useCallback(async () => { const { @@ -40,15 +39,10 @@ function SmartInventoryEdit({ inventory }) { useCallback( async (values, groupsToAssociate, groupsToDisassociate) => { const { data } = await InventoriesAPI.update(inventory.id, values); - await Promise.all( - groupsToAssociate.map(id => - InventoriesAPI.associateInstanceGroup(inventory.id, id) - ) - ); - await Promise.all( - groupsToDisassociate.map(id => - InventoriesAPI.disassociateInstanceGroup(inventory.id, id) - ) + await InventoriesAPI.orderInstanceGroups( + inventory.id, + groupsToAssociate, + groupsToDisassociate ); return data; }, @@ -68,20 +62,13 @@ function SmartInventoryEdit({ inventory }) { const handleSubmit = async form => { const { instance_groups, organization, ...remainingForm } = form; - const { added, removed } = getAddedAndRemoved( - instanceGroups, - instance_groups - ); - const addedIds = added.map(({ id }) => id); - const removedIds = removed.map(({ id }) => id); - await submitRequest( { organization: organization?.id, ...remainingForm, }, - addedIds, - removedIds + instance_groups, + initialInstanceGroups ); }; @@ -104,7 +91,7 @@ function SmartInventoryEdit({ inventory }) { ', () => { }); }); expect(InventoriesAPI.update).toHaveBeenCalledTimes(1); - expect(InventoriesAPI.associateInstanceGroup).toHaveBeenCalledTimes(1); - expect(InventoriesAPI.disassociateInstanceGroup).toHaveBeenCalledTimes(1); + expect(InventoriesAPI.orderInstanceGroups).toHaveBeenCalledTimes(1); }); test('successful form submission should trigger redirect to details', async () => { diff --git a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx index c65114c6c4..0b4e38df91 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx @@ -41,13 +41,11 @@ function OrganizationAdd() { ...values, default_environment: values.default_environment?.id, }); - await Promise.all( - groupsToAssociate.map(id => - OrganizationsAPI.associateInstanceGroup(response.id, id) - ) - ); /* eslint-disable no-await-in-loop, no-restricted-syntax */ // Resolve Promises sequentially to maintain order and avoid race condition + for (const group of groupsToAssociate) { + await OrganizationsAPI.associateInstanceGroup(response.id, group.id); + } for (const credential of values.galaxy_credentials) { await OrganizationsAPI.associateGalaxyCredential( response.id, diff --git a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx index 5fced4aa9d..36c973f350 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx @@ -95,6 +95,12 @@ describe('', () => { description: 'new description', galaxy_credentials: [], }; + const mockInstanceGroups = [ + { + name: 'mock ig', + id: 3, + }, + ]; OrganizationsAPI.create.mockResolvedValueOnce({ data: { id: 5, @@ -109,7 +115,10 @@ describe('', () => { wrapper = mountWithContexts(); }); await waitForElement(wrapper, 'button[aria-label="Save"]'); - await wrapper.find('OrganizationForm').prop('onSubmit')(orgData, [3]); + await wrapper.find('OrganizationForm').prop('onSubmit')( + orgData, + mockInstanceGroups + ); expect(OrganizationsAPI.associateInstanceGroup).toHaveBeenCalledWith(5, 3); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx index 0fc7a243f9..38ff6641ba 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx @@ -26,16 +26,12 @@ function OrganizationEdit({ organization }) { ...values, default_environment: values.default_environment?.id || null, }); - await Promise.all( - groupsToAssociate.map(id => - OrganizationsAPI.associateInstanceGroup(organization.id, id) - ) - ); - await Promise.all( - groupsToDisassociate.map(id => - OrganizationsAPI.disassociateInstanceGroup(organization.id, id) - ) + await OrganizationsAPI.orderInstanceGroups( + organization.id, + groupsToAssociate, + groupsToDisassociate ); + /* eslint-disable no-await-in-loop, no-restricted-syntax */ // Resolve Promises sequentially to avoid race condition if ( diff --git a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx index a6324eb702..a1fbdb0831 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx @@ -54,18 +54,34 @@ describe('', () => { name: 'new name', description: 'new description', }; + const newInstanceGroups = [ + { + name: 'mock three', + id: 3, + }, + { + name: 'mock four', + id: 4, + }, + ]; + const oldInstanceGroups = [ + { + name: 'mock two', + id: 2, + }, + ]; + await act(async () => { wrapper.find('OrganizationForm').invoke('onSubmit')( updatedOrgData, - [3, 4], - [2] + newInstanceGroups, + oldInstanceGroups ); }); - expect(OrganizationsAPI.associateInstanceGroup).toHaveBeenCalledWith(1, 3); - expect(OrganizationsAPI.associateInstanceGroup).toHaveBeenCalledWith(1, 4); - expect(OrganizationsAPI.disassociateInstanceGroup).toHaveBeenCalledWith( - 1, - 2 + expect(OrganizationsAPI.orderInstanceGroups).toHaveBeenCalledWith( + mockData.id, + newInstanceGroups, + oldInstanceGroups ); }); diff --git a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx index f5d37b56dc..355db7b531 100644 --- a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx +++ b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx @@ -15,7 +15,6 @@ import { InstanceGroupsLookup, ExecutionEnvironmentLookup, } from '../../../components/Lookup'; -import { getAddedAndRemoved } from '../../../util/lists'; import { required, minMaxValue } from '../../../util/validators'; import { FormColumnLayout } from '../../../components/FormLayout'; import CredentialLookup from '../../../components/Lookup/CredentialLookup'; @@ -143,19 +142,13 @@ function OrganizationForm({ }; const handleSubmit = values => { - const { added, removed } = getAddedAndRemoved( - initialInstanceGroups, - instanceGroups - ); - const addedIds = added.map(({ id }) => id); - const removedIds = removed.map(({ id }) => id); if ( typeof values.max_hosts !== 'number' || values.max_hosts === 'undefined' ) { values.max_hosts = 0; } - onSubmit(values, addedIds, removedIds); + onSubmit(values, instanceGroups, initialInstanceGroups); }; useEffect(() => { diff --git a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.test.jsx b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.test.jsx index 38d8b5d578..11b2c58449 100644 --- a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.test.jsx +++ b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.test.jsx @@ -258,7 +258,14 @@ describe('', () => { await act(async () => { wrapper.find('button[aria-label="Save"]').simulate('click'); }); - expect(onSubmit).toHaveBeenCalledWith(mockDataForm, [3], [2]); + expect(onSubmit).toHaveBeenCalledWith( + mockDataForm, + [ + { name: 'One', id: 1 }, + { name: 'Three', id: 3 }, + ], + mockInstanceGroups + ); }); test('onSubmit does not get called if max_hosts value is out of range', async () => { @@ -332,8 +339,8 @@ describe('', () => { max_hosts: 0, default_environment: null, }, - [], - [] + mockInstanceGroups, + mockInstanceGroups ); }); diff --git a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx index 1461cd1fca..89a387cb33 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.jsx @@ -63,11 +63,12 @@ function JobTemplateAdd() { return Promise.all([...associationPromises]); } - function submitInstanceGroups(templateId, addedGroups = []) { - const associatePromises = addedGroups.map(group => - JobTemplatesAPI.associateInstanceGroup(templateId, group.id) - ); - return Promise.all(associatePromises); + async function submitInstanceGroups(templateId, addedGroups = []) { + /* eslint-disable no-await-in-loop, no-restricted-syntax */ + for (const group of addedGroups) { + await JobTemplatesAPI.associateInstanceGroup(templateId, group.id); + } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ } function submitCredentials(templateId, credentials = []) { diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index d85e07cfa0..ccd3aef20a 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -61,8 +61,12 @@ function JobTemplateEdit({ template, reloadTemplate }) { await JobTemplatesAPI.update(template.id, remainingValues); await Promise.all([ submitLabels(labels, template?.organization), - submitInstanceGroups(instanceGroups, initialInstanceGroups), submitCredentials(credentials), + JobTemplatesAPI.orderInstanceGroups( + template.id, + instanceGroups, + initialInstanceGroups + ), ]); reloadTemplate(); history.push(detailsUrl); @@ -93,17 +97,6 @@ function JobTemplateEdit({ template, reloadTemplate }) { return results; }; - const submitInstanceGroups = async (groups, initialGroups) => { - const { added, removed } = getAddedAndRemoved(initialGroups, groups); - const disassociatePromises = await removed.map(group => - JobTemplatesAPI.disassociateInstanceGroup(template.id, group.id) - ); - const associatePromises = await added.map(group => - JobTemplatesAPI.associateInstanceGroup(template.id, group.id) - ); - return Promise.all([...disassociatePromises, ...associatePromises]); - }; - const submitCredentials = async newCredentials => { const { added, removed } = getAddedAndRemoved( template.summary_fields.credentials,