From 9fddf7c5cf25992c2a830c5d03b05694bc676812 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 22 Jun 2021 17:19:13 -0400 Subject: [PATCH] Add draggable selected list to galaxy credential lookup --- awx/ui_next/.eslintrc | 1 + .../components/AddRole/SelectResourceStep.jsx | 2 +- .../src/components/AddRole/SelectRoleStep.jsx | 2 +- .../components/Lookup/CredentialLookup.jsx | 31 ++-- awx/ui_next/src/components/Lookup/Lookup.jsx | 4 + .../components/OptionsList/OptionsList.jsx | 53 ++++--- .../SelectedList/DraggableSelectedList.jsx | 132 ++++++++++++++++++ .../DraggableSelectedList.test.jsx | 75 ++++++++++ .../src/components/SelectedList/index.js | 3 +- .../OrganizationAdd/OrganizationAdd.jsx | 20 ++- .../OrganizationEdit/OrganizationEdit.jsx | 61 ++++---- .../Organization/shared/OrganizationForm.jsx | 15 +- 12 files changed, 329 insertions(+), 70 deletions(-) create mode 100644 awx/ui_next/src/components/SelectedList/DraggableSelectedList.jsx create mode 100644 awx/ui_next/src/components/SelectedList/DraggableSelectedList.test.jsx diff --git a/awx/ui_next/.eslintrc b/awx/ui_next/.eslintrc index 8634750ebc..1f83a5dd44 100644 --- a/awx/ui_next/.eslintrc +++ b/awx/ui_next/.eslintrc @@ -63,6 +63,7 @@ "aria-labelledby", "aria-hidden", "aria-controls", + "aria-pressed", "sortKey", "ouiaId", "credentialTypeNamespace", diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index ee20c2d706..abf3b8d10f 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -6,7 +6,7 @@ import useRequest from '../../util/useRequest'; import { SearchColumns, SortColumns } from '../../types'; import DataListToolbar from '../DataListToolbar'; import CheckboxListItem from '../CheckboxListItem'; -import SelectedList from '../SelectedList'; +import { SelectedList } from '../SelectedList'; import { getQSConfig, parseQueryString } from '../../util/qs'; import PaginatedTable, { HeaderCell, HeaderRow } from '../PaginatedTable'; diff --git a/awx/ui_next/src/components/AddRole/SelectRoleStep.jsx b/awx/ui_next/src/components/AddRole/SelectRoleStep.jsx index 09fea55174..4723c49369 100644 --- a/awx/ui_next/src/components/AddRole/SelectRoleStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectRoleStep.jsx @@ -4,7 +4,7 @@ import PropTypes from 'prop-types'; import { t } from '@lingui/macro'; import CheckboxCard from './CheckboxCard'; -import SelectedList from '../SelectedList'; +import { SelectedList } from '../SelectedList'; function RolesStep({ onRolesClick, diff --git a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx index 6ff48d3e6e..bd22183782 100644 --- a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx @@ -29,22 +29,24 @@ const QS_CONFIG = getQSConfig('credentials', { }); function CredentialLookup({ - helperTextInvalid, - label, - isValid, - onBlur, - onChange, - required, + autoPopulate, credentialTypeId, credentialTypeKind, credentialTypeNamespace, - value, - tooltip, - isDisabled, - autoPopulate, - multiple, - validate, + draggable, fieldName, + helperTextInvalid, + isDisabled, + isValid, + label, + modalDescription, + multiple, + onBlur, + onChange, + required, + tooltip, + validate, + value, }) { const history = useHistory(); const autoPopulateLookup = useAutoPopulateLookup(onChange); @@ -174,6 +176,7 @@ function CredentialLookup({ qsConfig={QS_CONFIG} isDisabled={isDisabled} multiple={multiple} + modalDescription={modalDescription} renderOptionsList={({ state, dispatch, canDelete }) => ( dispatch({ type: 'SELECT_ITEM', item })} deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + sortSelectedItems={selectedItems => + dispatch({ type: 'SET_SELECTED_ITEMS', selectedItems }) + } multiple={multiple} + draggable={draggable} /> )} /> diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 0be5b61b85..6a8a4130ed 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -49,6 +49,7 @@ function Lookup(props) { onDebounce, fieldName, validate, + modalDescription, } = props; const [typedText, setTypedText] = useState(''); const debounceRequest = useDebounce(onDebounce, 1000); @@ -166,6 +167,7 @@ function Lookup(props) { aria-label={t`Lookup modal`} isOpen={isModalOpen} onClose={closeModal} + description={state?.selectedItems?.length > 0 && modalDescription} actions={[ + + + + ); + })} + +
+ {liveText} +
+ + ); +} + +const ListItem = PropTypes.shape({ + id: PropTypes.number.isRequired, + name: PropTypes.string.isRequired, +}); +DraggableSelectedList.propTypes = { + onRemove: PropTypes.func, + onRowDrag: PropTypes.func, + selected: PropTypes.arrayOf(ListItem), +}; +DraggableSelectedList.defaultProps = { + onRemove: () => null, + onRowDrag: () => null, + selected: [], +}; + +export default DraggableSelectedList; diff --git a/awx/ui_next/src/components/SelectedList/DraggableSelectedList.test.jsx b/awx/ui_next/src/components/SelectedList/DraggableSelectedList.test.jsx new file mode 100644 index 0000000000..c0318069c7 --- /dev/null +++ b/awx/ui_next/src/components/SelectedList/DraggableSelectedList.test.jsx @@ -0,0 +1,75 @@ +import React from 'react'; +import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; +import DraggableSelectedList from './DraggableSelectedList'; + +describe('', () => { + let wrapper; + afterEach(() => { + jest.clearAllMocks(); + wrapper.unmount(); + }); + test('should render expected rows', () => { + const mockSelected = [ + { + id: 1, + name: 'foo', + }, + { + id: 2, + name: 'bar', + }, + ]; + wrapper = mountWithContexts( + {}} + onRowDrag={() => {}} + /> + ); + expect(wrapper.find('DraggableSelectedList').length).toBe(1); + expect(wrapper.find('DataListItem').length).toBe(2); + expect( + wrapper + .find('DataListItem DataListCell') + .first() + .containsMatchingElement(1. foo) + ).toEqual(true); + expect( + wrapper + .find('DataListItem DataListCell') + .last() + .containsMatchingElement(2. bar) + ).toEqual(true); + }); + + test('should not render when selected list is empty', () => { + wrapper = mountWithContexts( + {}} + onRowDrag={() => {}} + /> + ); + expect(wrapper.find('DataList').length).toBe(0); + }); + + test('should call onRemove callback prop on remove button click', () => { + const onRemove = jest.fn(); + const mockSelected = [ + { + id: 1, + name: 'foo', + }, + ]; + wrapper = mountWithContexts( + + ); + wrapper + .find('DataListItem[id="foo"] Button[aria-label="Remove"]') + .simulate('click'); + expect(onRemove).toBeCalledWith({ + id: 1, + name: 'foo', + }); + }); +}); diff --git a/awx/ui_next/src/components/SelectedList/index.js b/awx/ui_next/src/components/SelectedList/index.js index 678bdcc62c..34e5e53912 100644 --- a/awx/ui_next/src/components/SelectedList/index.js +++ b/awx/ui_next/src/components/SelectedList/index.js @@ -1 +1,2 @@ -export { default } from './SelectedList'; +export { default as SelectedList } from './SelectedList'; +export { default as DraggableSelectedList } from './DraggableSelectedList'; diff --git a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx index 6b821ac875..c65114c6c4 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx @@ -42,14 +42,20 @@ function OrganizationAdd() { default_environment: values.default_environment?.id, }); await Promise.all( - groupsToAssociate - .map(id => OrganizationsAPI.associateInstanceGroup(response.id, id)) - .concat( - values.galaxy_credentials.map(({ id: credId }) => - OrganizationsAPI.associateGalaxyCredential(response.id, credId) - ) - ) + 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 credential of values.galaxy_credentials) { + await OrganizationsAPI.associateGalaxyCredential( + response.id, + credential.id + ); + } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ + history.push(`/organizations/${response.id}`); } catch (error) { setFormError(error); diff --git a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx index 76a548bcba..0fc7a243f9 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx @@ -3,9 +3,14 @@ import PropTypes from 'prop-types'; import { useHistory } from 'react-router-dom'; import { CardBody } from '../../../components/Card'; import { OrganizationsAPI } from '../../../api'; -import { getAddedAndRemoved } from '../../../util/lists'; import OrganizationForm from '../shared/OrganizationForm'; +const isEqual = (array1, array2) => + array1.length === array2.length && + array1.every((element, index) => { + return element.id === array2[index].id; + }); + function OrganizationEdit({ organization }) { const detailsUrl = `/organizations/${organization.id}/details`; const history = useHistory(); @@ -17,43 +22,39 @@ function OrganizationEdit({ organization }) { groupsToDisassociate ) => { try { - const { - added: addedCredentials, - removed: removedCredentials, - } = getAddedAndRemoved( - organization.galaxy_credentials, - values.galaxy_credentials - ); - - const addedCredentialIds = addedCredentials.map(({ id }) => id); - const removedCredentialIds = removedCredentials.map(({ id }) => id); - await OrganizationsAPI.update(organization.id, { ...values, default_environment: values.default_environment?.id || null, }); await Promise.all( - groupsToAssociate - .map(id => - OrganizationsAPI.associateInstanceGroup(organization.id, id) - ) - .concat( - addedCredentialIds.map(id => - OrganizationsAPI.associateGalaxyCredential(organization.id, id) - ) - ) + groupsToAssociate.map(id => + OrganizationsAPI.associateInstanceGroup(organization.id, id) + ) ); await Promise.all( - groupsToDisassociate - .map(id => - OrganizationsAPI.disassociateInstanceGroup(organization.id, id) - ) - .concat( - removedCredentialIds.map(id => - OrganizationsAPI.disassociateGalaxyCredential(organization.id, id) - ) - ) + groupsToDisassociate.map(id => + OrganizationsAPI.disassociateInstanceGroup(organization.id, id) + ) ); + /* eslint-disable no-await-in-loop, no-restricted-syntax */ + // Resolve Promises sequentially to avoid race condition + if ( + !isEqual(organization.galaxy_credentials, values.galaxy_credentials) + ) { + for (const credential of organization.galaxy_credentials) { + await OrganizationsAPI.disassociateGalaxyCredential( + organization.id, + credential.id + ); + } + for (const credential of values.galaxy_credentials) { + await OrganizationsAPI.associateGalaxyCredential( + organization.id, + credential.id + ); + } + } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ history.push(detailsUrl); } catch (error) { setFormError(error); diff --git a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx index 45c5c48a0a..6a05672d3f 100644 --- a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx +++ b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.jsx @@ -2,7 +2,7 @@ import React, { useCallback, useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import { Formik, useField, useFormikContext } from 'formik'; -import { t } from '@lingui/macro'; +import { t, Trans } from '@lingui/macro'; import { Form } from '@patternfly/react-core'; import { OrganizationsAPI } from '../../../api'; @@ -106,7 +106,20 @@ function OrganizationFormFields({ onChange={handleCredentialUpdate} value={galaxyCredentialsField.value} multiple + draggable fieldName="galaxy_credentials" + modalDescription={ + <> + + Selected + +
+ + Note: The order of these credentials sets precedence for the sync + and lookup of the content. + + + } /> );