From 2a722ba8d028c64aafbc9ac9b093e867e3b25e37 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 6 Nov 2019 12:03:13 -0500 Subject: [PATCH 01/19] Refactors Lookup --- .../components/Lookup/CredentialLookup.jsx | 2 +- .../Lookup/InstanceGroupsLookup.jsx | 2 +- .../src/components/Lookup/InventoryLookup.jsx | 2 +- awx/ui_next/src/components/Lookup/Lookup.jsx | 197 +++++++++++------- .../Lookup/MultiCredentialsLookup.jsx | 45 ++-- .../components/Lookup/OrganizationLookup.jsx | 2 +- .../src/components/Lookup/ProjectLookup.jsx | 2 +- .../shared/ProjectSubForms/SharedFields.jsx | 2 +- 8 files changed, 143 insertions(+), 111 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx index 6872e09784..aae1871e42 100644 --- a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx @@ -36,7 +36,7 @@ function CredentialLookup({ name="credential" value={value} onBlur={onBlur} - onLookupSave={onChange} + onChange={onChange} getItems={getCredentials} required={required} sortedColumnKey="name" diff --git a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx index 1e58f3eafa..c720ae2363 100644 --- a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx @@ -39,7 +39,7 @@ class InstanceGroupsLookup extends React.Component { lookupHeader={i18n._(t`Instance Groups`)} name="instanceGroups" value={value} - onLookupSave={onChange} + onChange={onChange} getItems={getInstanceGroups} qsNamespace="instance-group" multiple diff --git a/awx/ui_next/src/components/Lookup/InventoryLookup.jsx b/awx/ui_next/src/components/Lookup/InventoryLookup.jsx index d570e79128..494b4d3069 100644 --- a/awx/ui_next/src/components/Lookup/InventoryLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InventoryLookup.jsx @@ -37,7 +37,7 @@ class InventoryLookup extends React.Component { lookupHeader={i18n._(t`Inventory`)} name="inventory" value={value} - onLookupSave={onChange} + onChange={onChange} onBlur={onBlur} getItems={getInventories} required={required} diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index ce2f85f24b..b1913e0100 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -59,13 +59,13 @@ class Lookup extends React.Component { super(props); this.assertCorrectValueType(); - let lookupSelectedItems = []; + let selectedItems = []; if (props.value) { - lookupSelectedItems = props.multiple ? [...props.value] : [props.value]; + selectedItems = props.multiple ? [...props.value] : [props.value]; } this.state = { isModalOpen: false, - lookupSelectedItems, + selectedItems, results: [], count: 0, error: null, @@ -76,7 +76,8 @@ class Lookup extends React.Component { order_by: props.sortedColumnKey, }); this.handleModalToggle = this.handleModalToggle.bind(this); - this.toggleSelected = this.toggleSelected.bind(this); + this.addItem = this.addItem.bind(this); + this.removeItem = this.removeItem.bind(this); this.saveModal = this.saveModal.bind(this); this.getData = this.getData.bind(this); this.clearQSParams = this.clearQSParams.bind(this); @@ -132,46 +133,82 @@ class Lookup extends React.Component { } } - toggleSelected(row) { - const { - name, - onLookupSave, - multiple, - onToggleItem, - selectCategoryOptions, - } = this.props; - const { - lookupSelectedItems: updatedSelectedItems, - isModalOpen, - } = this.state; - - const selectedIndex = updatedSelectedItems.findIndex( - selectedRow => selectedRow.id === row.id - ); - if (multiple) { - if (selectCategoryOptions) { - onToggleItem(row, isModalOpen); - } - if (selectedIndex > -1) { - updatedSelectedItems.splice(selectedIndex, 1); - this.setState({ lookupSelectedItems: updatedSelectedItems }); - } else { - this.setState(prevState => ({ - lookupSelectedItems: [...prevState.lookupSelectedItems, row], - })); - } - } else { - this.setState({ lookupSelectedItems: [row] }); - } - - // Updates the selected items from parent state - // This handles the case where the user removes chips from the lookup input - // while the modal is closed - if (!isModalOpen) { - onLookupSave(updatedSelectedItems, name); + removeItem(row) { + const { selectedItems } = this.state; + const { onToggleItem } = this.props; + if (onToggleItem) { + this.setState({ selectedItems: onToggleItem(selectedItems, row) }); + return; } + this.setState({ + selectedItems: selectedItems.filter(item => item.id !== row.id), + }); } + addItem(row) { + const { selectedItems } = this.state; + const { multiple, onToggleItem } = this.props; + if (onToggleItem) { + this.setState({ selectedItems: onToggleItem(selectedItems, row) }); + return; + } + const index = selectedItems.findIndex(item => item.id === row.id); + + if (!multiple) { + this.setState({ selectedItems: [row] }); + return; + } + if (index > -1) { + return; + } + this.setState({ selectedItems: [...selectedItems, row] }); + } + // toggleSelected(row) { + // const { + // name, + // onChange, + // multiple, + // onToggleItem, + // selectCategoryOptions, + // onChange, + // value + // } = this.props; + // const { + // selectedItems: updatedSelectedItems, + // isModalOpen, + // } = this.state; + + // const selectedIndex = updatedSelectedItems.findIndex( + // selectedRow => selectedRow.id === row.id + // ); + // + // if (multiple) { + // + // if (selectCategoryOptions) { + // + // onToggleItem(row, isModalOpen); + // } + // if (selectedIndex > -1) { + // + // const valueToUpdate = value.filter(itemValue => itemValue.id !==row.id ); + // onChange(valueToUpdate) + // } else { + // + // onChange([...value, row]) + // } + // } else { + // + // onChange(row) + // } + + // Updates the selected items from parent state + // This handles the case where the user removes chips from the lookup input + // while the modal is closed + // if (!isModalOpen) { + // onChange(updatedSelectedItems, name); + // } + // } + handleModalToggle() { const { isModalOpen } = this.state; const { value, multiple, selectCategory } = this.props; @@ -179,11 +216,11 @@ class Lookup extends React.Component { // This handles the case where the user closes/cancels the modal and // opens it again if (!isModalOpen) { - let lookupSelectedItems = []; + let selectedItems = []; if (value) { - lookupSelectedItems = multiple ? [...value] : [value]; + selectedItems = multiple ? [...value] : [value]; } - this.setState({ lookupSelectedItems }); + this.setState({ selectedItems }); } else { this.clearQSParams(); if (selectCategory) { @@ -195,15 +232,22 @@ class Lookup extends React.Component { })); } + removeItemAndSave(row) { + const { value, onChange, multiple } = this.props; + if (multiple) { + onChange(value.filter(item => item.id !== row.id)); + } else if (value.id === row.id) { + onChange(null); + } + } + saveModal() { - const { onLookupSave, name, multiple } = this.props; - const { lookupSelectedItems } = this.state; - const value = multiple - ? lookupSelectedItems - : lookupSelectedItems[0] || null; + const { onChange, multiple } = this.props; + const { selectedItems } = this.state; + const value = multiple ? selectedItems : selectedItems[0] || null; this.handleModalToggle(); - onLookupSave(value, name); + onChange(value); } clearQSParams() { @@ -215,13 +259,7 @@ class Lookup extends React.Component { } render() { - const { - isModalOpen, - lookupSelectedItems, - error, - results, - count, - } = this.state; + const { isModalOpen, selectedItems, error, results, count } = this.state; const { form, id, @@ -245,7 +283,7 @@ class Lookup extends React.Component { {(multiple ? value : [value]).map(chip => ( this.toggleSelected(chip)} + onClick={() => this.removeItemAndSave(chip)} isReadOnly={!canDelete} credential={chip} /> @@ -256,7 +294,7 @@ class Lookup extends React.Component { {(multiple ? value : [value]).map(chip => ( this.toggleSelected(chip)} + onClick={() => this.removeItemAndSave(chip)} isReadOnly={!canDelete} > {chip.name} @@ -287,19 +325,19 @@ class Lookup extends React.Component { onClose={this.handleModalToggle} actions={[ , , ]} > @@ -318,6 +356,18 @@ class Lookup extends React.Component { /> )} + {selectedItems.length > 0 && ( + 0 + } + /> + )} i.id === item.id) - : lookupSelectedItems.some(i => i.id === item.id) - } - onSelect={() => this.toggleSelected(item)} + isSelected={selectedItems.some(i => i.id === item.id)} + onSelect={() => this.addItem(item)} isRadio={ !multiple || (selectCategoryOptions && @@ -347,17 +393,6 @@ class Lookup extends React.Component { renderToolbar={props => } showPageSizeOptions={false} /> - {lookupSelectedItems.length > 0 && ( - 0 - } - /> - )} {error ?
error
: ''} @@ -374,7 +409,7 @@ Lookup.propTypes = { getItems: func.isRequired, lookupHeader: string, name: string, - onLookupSave: func.isRequired, + onChange: func.isRequired, value: oneOfType([Item, arrayOf(Item)]), sortedColumnKey: string.isRequired, multiple: bool, diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 5b978b2c66..9e40ebccec 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -12,7 +12,25 @@ import Lookup from '@components/Lookup'; const QuestionCircleIcon = styled(PFQuestionCircleIcon)` margin-left: 10px; `; +function toggleCredentialSelection(credentialsToUpdate, newCredential) { + let newCredentialsList; + const isSelectedCredentialInState = + credentialsToUpdate.filter(cred => cred.id === newCredential.id).length > + 0; + if (isSelectedCredentialInState) { + newCredentialsList = credentialsToUpdate.filter( + cred => cred.id !== newCredential.id + ); + } else { + newCredentialsList = credentialsToUpdate.filter( + credential => + credential.kind === 'vault' || credential.kind !== newCredential.kind + ); + newCredentialsList = [...newCredentialsList, newCredential]; + } + return newCredentialsList; +} class MultiCredentialsLookup extends React.Component { constructor(props) { super(props); @@ -26,7 +44,6 @@ class MultiCredentialsLookup extends React.Component { this ); this.loadCredentials = this.loadCredentials.bind(this); - this.toggleCredentialSelection = this.toggleCredentialSelection.bind(this); } componentDidMount() { @@ -69,27 +86,7 @@ class MultiCredentialsLookup extends React.Component { return CredentialsAPI.read(params); } - toggleCredentialSelection(newCredential) { - const { onChange, credentials: credentialsToUpdate } = this.props; - let newCredentialsList; - const isSelectedCredentialInState = - credentialsToUpdate.filter(cred => cred.id === newCredential.id).length > - 0; - - if (isSelectedCredentialInState) { - newCredentialsList = credentialsToUpdate.filter( - cred => cred.id !== newCredential.id - ); - } else { - newCredentialsList = credentialsToUpdate.filter( - credential => - credential.kind === 'vault' || credential.kind !== newCredential.kind - ); - newCredentialsList = [...newCredentialsList, newCredential]; - } - onChange(newCredentialsList); - } handleCredentialTypeSelect(value, type) { const { credentialTypes } = this.state; @@ -99,7 +96,7 @@ class MultiCredentialsLookup extends React.Component { render() { const { selectedCredentialType, credentialTypes } = this.state; - const { tooltip, i18n, credentials } = this.props; + const { tooltip, i18n, credentials, onChange } = this.props; return ( {tooltip && ( @@ -112,14 +109,14 @@ class MultiCredentialsLookup extends React.Component { selectCategoryOptions={credentialTypes} selectCategory={this.handleCredentialTypeSelect} selectedCategory={selectedCredentialType} - onToggleItem={this.toggleCredentialSelection} + onToggleItem={toggleCredentialSelection} onloadCategories={this.loadCredentialTypes} id="multiCredential" lookupHeader={i18n._(t`Credentials`)} name="credentials" value={credentials} multiple - onLookupSave={() => {}} + onChange={onChange} getItems={this.loadCredentials} qsNamespace="credentials" columns={[ diff --git a/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx b/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx index 8efb43b091..32c93f2588 100644 --- a/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx +++ b/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx @@ -32,7 +32,7 @@ function OrganizationLookup({ name="organization" value={value} onBlur={onBlur} - onLookupSave={onChange} + onChange={onChange} getItems={getOrganizations} required={required} sortedColumnKey="name" diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx index 90d36a64a7..f83d30eb02 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx @@ -45,7 +45,7 @@ class ProjectLookup extends React.Component { name="project" value={value} onBlur={onBlur} - onLookupSave={onChange} + onChange={onChange} getItems={loadProjects} required={required} sortedColumnKey="name" diff --git a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx index 127ba54936..5a7e1434c1 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectSubForms/SharedFields.jsx @@ -51,7 +51,7 @@ export const ScmCredentialFormField = withI18n()( value={credential.value} onChange={value => { onCredentialSelection('scm', value); - form.setFieldValue('credential', value.id); + form.setFieldValue('credential', value ? value.id : ''); }} /> )} From 5a207f155edb1d8e1536a36a88aa29c9fe859f61 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 14 Nov 2019 15:35:08 -0800 Subject: [PATCH 02/19] WIP split Lookup into Lookup & CategoryLookup --- .../src/components/Lookup/CategoryLookup.jsx | 348 ++++++++++++++++++ .../Lookup/InstanceGroupsLookup.jsx | 146 +++++--- awx/ui_next/src/components/Lookup/Lookup.jsx | 202 ++-------- .../Lookup/MultiCredentialsLookup.jsx | 62 +++- awx/ui_next/src/components/Lookup/index.js | 1 + .../components/SelectedList/SelectedList.jsx | 3 + .../Template/shared/JobTemplateForm.jsx | 2 +- 7 files changed, 516 insertions(+), 248 deletions(-) create mode 100644 awx/ui_next/src/components/Lookup/CategoryLookup.jsx diff --git a/awx/ui_next/src/components/Lookup/CategoryLookup.jsx b/awx/ui_next/src/components/Lookup/CategoryLookup.jsx new file mode 100644 index 0000000000..f3e8935bdf --- /dev/null +++ b/awx/ui_next/src/components/Lookup/CategoryLookup.jsx @@ -0,0 +1,348 @@ +import React, { Fragment } from 'react'; +import { + string, + bool, + arrayOf, + func, + number, + oneOfType, + shape, +} from 'prop-types'; +import { withRouter } from 'react-router-dom'; +import { SearchIcon } from '@patternfly/react-icons'; +import { + Button, + ButtonVariant, + InputGroup as PFInputGroup, + Modal, + ToolbarItem, +} from '@patternfly/react-core'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import styled from 'styled-components'; + +import AnsibleSelect from '../AnsibleSelect'; +import PaginatedDataList from '../PaginatedDataList'; +import VerticalSeperator from '../VerticalSeparator'; +import DataListToolbar from '../DataListToolbar'; +import CheckboxListItem from '../CheckboxListItem'; +import SelectedList from '../SelectedList'; +import { ChipGroup, Chip, CredentialChip } from '../Chip'; +import { QSConfig } from '@types'; + +const SearchButton = styled(Button)` + ::after { + border: var(--pf-c-button--BorderWidth) solid + var(--pf-global--BorderColor--200); + } +`; + +const InputGroup = styled(PFInputGroup)` + ${props => + props.multiple && + ` + --pf-c-form-control--Height: 90px; + overflow-y: auto; + `} +`; + +const ChipHolder = styled.div` + --pf-c-form-control--BorderTopColor: var(--pf-global--BorderColor--200); + --pf-c-form-control--BorderRightColor: var(--pf-global--BorderColor--200); + border-top-right-radius: 3px; + border-bottom-right-radius: 3px; +`; + +class CategoryLookup extends React.Component { + constructor(props) { + super(props); + + this.assertCorrectValueType(); + let selectedItems = []; + if (props.value) { + selectedItems = props.multiple ? [...props.value] : [props.value]; + } + this.state = { + isModalOpen: false, + selectedItems, + error: null, + }; + this.handleModalToggle = this.handleModalToggle.bind(this); + this.addItem = this.addItem.bind(this); + this.removeItem = this.removeItem.bind(this); + this.saveModal = this.saveModal.bind(this); + this.clearQSParams = this.clearQSParams.bind(this); + } + + assertCorrectValueType() { + const { multiple, value, selectCategoryOptions } = this.props; + if (selectCategoryOptions) { + return; + } + if (!multiple && Array.isArray(value)) { + throw new Error( + 'CategoryLookup value must not be an array unless `multiple` is set' + ); + } + if (multiple && !Array.isArray(value)) { + throw new Error( + 'CategoryLookup value must be an array if `multiple` is set' + ); + } + } + + removeItem(row) { + const { selectedItems } = this.state; + const { onToggleItem } = this.props; + if (onToggleItem) { + this.setState({ selectedItems: onToggleItem(selectedItems, row) }); + return; + } + this.setState({ + selectedItems: selectedItems.filter(item => item.id !== row.id), + }); + } + + addItem(row) { + const { selectedItems } = this.state; + const { multiple, onToggleItem } = this.props; + if (onToggleItem) { + this.setState({ selectedItems: onToggleItem(selectedItems, row) }); + return; + } + const index = selectedItems.findIndex(item => item.id === row.id); + + if (!multiple) { + this.setState({ selectedItems: [row] }); + return; + } + if (index > -1) { + return; + } + this.setState({ selectedItems: [...selectedItems, row] }); + } + + // TODO: clean up + handleModalToggle() { + const { isModalOpen } = this.state; + const { value, multiple, selectCategory } = this.props; + // Resets the selected items from parent state whenever modal is opened + // This handles the case where the user closes/cancels the modal and + // opens it again + if (!isModalOpen) { + let selectedItems = []; + if (value) { + selectedItems = multiple ? [...value] : [value]; + } + this.setState({ selectedItems }); + } else { + this.clearQSParams(); + if (selectCategory) { + selectCategory(null, 'Machine'); + } + } + this.setState(prevState => ({ + isModalOpen: !prevState.isModalOpen, + })); + } + + removeItemAndSave(row) { + const { value, onChange, multiple } = this.props; + if (multiple) { + onChange(value.filter(item => item.id !== row.id)); + } else if (value.id === row.id) { + onChange(null); + } + } + + saveModal() { + const { onChange, multiple } = this.props; + const { selectedItems } = this.state; + const value = multiple ? selectedItems : selectedItems[0] || null; + + this.handleModalToggle(); + onChange(value); + } + + clearQSParams() { + const { qsConfig, history } = this.props; + const parts = history.location.search.replace(/^\?/, '').split('&'); + const ns = qsConfig.namespace; + const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); + history.push(`${history.location.pathname}?${otherParts.join('&')}`); + } + + render() { + const { isModalOpen, selectedItems, error } = this.state; + const { + id, + items, + count, + lookupHeader, + value, + columns, + multiple, + name, + onBlur, + qsConfig, + required, + selectCategory, + selectCategoryOptions, + selectedCategory, + i18n, + } = this.props; + const header = lookupHeader || i18n._(t`Items`); + const canDelete = !required || (multiple && value.length > 1); + const chips = () => { + return selectCategoryOptions && selectCategoryOptions.length > 0 ? ( + + {(multiple ? value : [value]).map(chip => ( + this.removeItemAndSave(chip)} + isReadOnly={!canDelete} + credential={chip} + /> + ))} + + ) : ( + + {(multiple ? value : [value]).map(chip => ( + this.removeItemAndSave(chip)} + isReadOnly={!canDelete} + > + {chip.name} + + ))} + + ); + }; + return ( + + + + + + + {value ? chips(value) : null} + + + + {i18n._(t`Select`)} + , + , + ]} + > + {selectCategoryOptions && selectCategoryOptions.length > 0 && ( + + Selected Category + + + + )} + {selectedItems.length > 0 && ( + 0 + } + /> + )} + ( + i.id === item.id)} + onSelect={() => this.addItem(item)} + isRadio={ + !multiple || + (selectCategoryOptions && + selectCategoryOptions.length && + selectedCategory.value !== 'Vault') + } + /> + )} + renderToolbar={props => } + showPageSizeOptions={false} + /> + {error ?
error: {error.message}
: ''} + + + ); + } +} + +const Item = shape({ + id: number.isRequired, +}); + +CategoryLookup.propTypes = { + id: string, + items: arrayOf(shape({})).isRequired, + // TODO: change to `header` + lookupHeader: string, + name: string, + onChange: func.isRequired, + value: oneOfType([Item, arrayOf(Item)]), + multiple: bool, + required: bool, + qsConfig: QSConfig.isRequired, + selectCategory: func.isRequired, + selectCategoryOptions: oneOfType(shape({})).isRequired, + selectedCategory: shape({}).isRequired, +}; + +CategoryLookup.defaultProps = { + id: 'lookup-search', + lookupHeader: null, + name: null, + value: null, + multiple: false, + required: false, +}; + +export { CategoryLookup as _CategoryLookup }; +export default withI18n()(withRouter(CategoryLookup)); diff --git a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx index c720ae2363..6641294235 100644 --- a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx @@ -1,84 +1,114 @@ -import React from 'react'; -import PropTypes from 'prop-types'; +import React, { useState, useEffect } from 'react'; +import { arrayOf, string, func, object } from 'prop-types'; +import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { FormGroup, Tooltip } from '@patternfly/react-core'; import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; import styled from 'styled-components'; - import { InstanceGroupsAPI } from '@api'; import Lookup from '@components/Lookup'; +import { getQSConfig, parseQueryString } from '@util/qs'; const QuestionCircleIcon = styled(PFQuestionCircleIcon)` margin-left: 10px; `; -const getInstanceGroups = async params => InstanceGroupsAPI.read(params); +const QS_CONFIG = getQSConfig('instance-groups', { + page: 1, + page_size: 5, + order_by: 'name', +}); +// const getInstanceGroups = async params => InstanceGroupsAPI.read(params); -class InstanceGroupsLookup extends React.Component { - render() { - const { value, tooltip, onChange, className, i18n } = this.props; +function InstanceGroupsLookup({ + value, + onChange, + tooltip, + className, + history, + i18n, +}) { + const [instanceGroups, setInstanceGroups] = useState([]); + const [count, setCount] = useState(0); + const [error, setError] = useState(null); - /* + useEffect(() => { + (async () => { + const params = parseQueryString(QS_CONFIG, history.location.search); + try { + const { data } = await InstanceGroupsAPI.read(params); + setInstanceGroups(data.results); + setCount(data.count); + } catch (err) { + setError(err); + } + })(); + }, [history.location]); + + /* Wrapping
added to workaround PF bug: https://github.com/patternfly/patternfly-react/issues/2855 */ - return ( -
- - {tooltip && ( - - - - )} - - -
- ); - } + return ( +
+ + {tooltip && ( + + + + )} + + {error ?
error {error.message}
: ''} + +
+ ); } InstanceGroupsLookup.propTypes = { - value: PropTypes.arrayOf(PropTypes.object).isRequired, - tooltip: PropTypes.string, - onChange: PropTypes.func.isRequired, + value: arrayOf(object).isRequired, + tooltip: string, + onChange: func.isRequired, + className: string, }; InstanceGroupsLookup.defaultProps = { tooltip: '', + className: '', }; -export default withI18n()(InstanceGroupsLookup); +export default withI18n()(withRouter(InstanceGroupsLookup)); diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index b1913e0100..7446d8f09f 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -15,20 +15,17 @@ import { ButtonVariant, InputGroup as PFInputGroup, Modal, - ToolbarItem, } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import styled from 'styled-components'; -import AnsibleSelect from '../AnsibleSelect'; import PaginatedDataList from '../PaginatedDataList'; -import VerticalSeperator from '../VerticalSeparator'; import DataListToolbar from '../DataListToolbar'; import CheckboxListItem from '../CheckboxListItem'; import SelectedList from '../SelectedList'; -import { ChipGroup, Chip, CredentialChip } from '../Chip'; -import { getQSConfig, parseQueryString } from '../../util/qs'; +import { ChipGroup, Chip } from '../Chip'; +import { QSConfig } from '@types'; const SearchButton = styled(Button)` ::after { @@ -66,42 +63,16 @@ class Lookup extends React.Component { this.state = { isModalOpen: false, selectedItems, - results: [], - count: 0, - error: null, }; - this.qsConfig = getQSConfig(props.qsNamespace, { - page: 1, - page_size: 5, - order_by: props.sortedColumnKey, - }); this.handleModalToggle = this.handleModalToggle.bind(this); this.addItem = this.addItem.bind(this); this.removeItem = this.removeItem.bind(this); this.saveModal = this.saveModal.bind(this); - this.getData = this.getData.bind(this); this.clearQSParams = this.clearQSParams.bind(this); } - componentDidMount() { - this.getData(); - } - - componentDidUpdate(prevProps) { - const { location, selectedCategory } = this.props; - if ( - location !== prevProps.location || - prevProps.selectedCategory !== selectedCategory - ) { - this.getData(); - } - } - assertCorrectValueType() { - const { multiple, value, selectCategoryOptions } = this.props; - if (selectCategoryOptions) { - return; - } + const { multiple, value } = this.props; if (!multiple && Array.isArray(value)) { throw new Error( 'Lookup value must not be an array unless `multiple` is set' @@ -112,27 +83,6 @@ class Lookup extends React.Component { } } - async getData() { - const { - getItems, - location: { search }, - } = this.props; - const queryParams = parseQueryString(this.qsConfig, search); - - this.setState({ error: false }); - try { - const { data } = await getItems(queryParams); - const { results, count } = data; - - this.setState({ - results, - count, - }); - } catch (err) { - this.setState({ error: true }); - } - } - removeItem(row) { const { selectedItems } = this.state; const { onToggleItem } = this.props; @@ -163,55 +113,11 @@ class Lookup extends React.Component { } this.setState({ selectedItems: [...selectedItems, row] }); } - // toggleSelected(row) { - // const { - // name, - // onChange, - // multiple, - // onToggleItem, - // selectCategoryOptions, - // onChange, - // value - // } = this.props; - // const { - // selectedItems: updatedSelectedItems, - // isModalOpen, - // } = this.state; - - // const selectedIndex = updatedSelectedItems.findIndex( - // selectedRow => selectedRow.id === row.id - // ); - // - // if (multiple) { - // - // if (selectCategoryOptions) { - // - // onToggleItem(row, isModalOpen); - // } - // if (selectedIndex > -1) { - // - // const valueToUpdate = value.filter(itemValue => itemValue.id !==row.id ); - // onChange(valueToUpdate) - // } else { - // - // onChange([...value, row]) - // } - // } else { - // - // onChange(row) - // } - - // Updates the selected items from parent state - // This handles the case where the user removes chips from the lookup input - // while the modal is closed - // if (!isModalOpen) { - // onChange(updatedSelectedItems, name); - // } - // } + // TODO: cleanup handleModalToggle() { const { isModalOpen } = this.state; - const { value, multiple, selectCategory } = this.props; + const { value, multiple } = this.props; // Resets the selected items from parent state whenever modal is opened // This handles the case where the user closes/cancels the modal and // opens it again @@ -223,20 +129,17 @@ class Lookup extends React.Component { this.setState({ selectedItems }); } else { this.clearQSParams(); - if (selectCategory) { - selectCategory(null, 'Machine'); - } } this.setState(prevState => ({ isModalOpen: !prevState.isModalOpen, })); } - removeItemAndSave(row) { + removeItemAndSave(item) { const { value, onChange, multiple } = this.props; if (multiple) { - onChange(value.filter(item => item.id !== row.id)); - } else if (value.id === row.id) { + onChange(value.filter(i => i.id !== item.id)); + } else if (value.id === item.id) { onChange(null); } } @@ -251,58 +154,31 @@ class Lookup extends React.Component { } clearQSParams() { - const { history } = this.props; + const { qsConfig, history } = this.props; const parts = history.location.search.replace(/^\?/, '').split('&'); - const ns = this.qsConfig.namespace; + const ns = qsConfig.namespace; const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); history.push(`${history.location.pathname}?${otherParts.join('&')}`); } render() { - const { isModalOpen, selectedItems, error, results, count } = this.state; + const { isModalOpen, selectedItems } = this.state; const { - form, id, lookupHeader, value, + items, + count, columns, multiple, name, onBlur, - selectCategory, required, + qsConfig, i18n, - selectCategoryOptions, - selectedCategory, } = this.props; const header = lookupHeader || i18n._(t`Items`); const canDelete = !required || (multiple && value.length > 1); - const chips = () => { - return selectCategoryOptions && selectCategoryOptions.length > 0 ? ( - - {(multiple ? value : [value]).map(chip => ( - this.removeItemAndSave(chip)} - isReadOnly={!canDelete} - credential={chip} - /> - ))} - - ) : ( - - {(multiple ? value : [value]).map(chip => ( - this.removeItemAndSave(chip)} - isReadOnly={!canDelete} - > - {chip.name} - - ))} - - ); - }; return ( @@ -315,7 +191,17 @@ class Lookup extends React.Component { - {value ? chips(value) : null} + + {(multiple ? value : [value]).map(chip => ( + this.removeItemAndSave(chip)} + isReadOnly={!canDelete} + > + {chip.name} + + ))} + , ]} > - {selectCategoryOptions && selectCategoryOptions.length > 0 && ( - - Selected Category - - - - )} {selectedItems.length > 0 && ( 0 - } /> )} ( i.id === item.id)} onSelect={() => this.addItem(item)} - isRadio={ - !multiple || - (selectCategoryOptions && - selectCategoryOptions.length && - selectedCategory.value !== 'Vault') - } + isRadio={!multiple} /> )} renderToolbar={props => } showPageSizeOptions={false} /> - {error ?
error
: ''} ); @@ -406,15 +268,16 @@ const Item = shape({ Lookup.propTypes = { id: string, - getItems: func.isRequired, + items: arrayOf(shape({})).isRequired, + count: number.isRequired, + // TODO: change to `header` lookupHeader: string, name: string, onChange: func.isRequired, value: oneOfType([Item, arrayOf(Item)]), - sortedColumnKey: string.isRequired, multiple: bool, required: bool, - qsNamespace: string, + qsConfig: QSConfig.isRequired, }; Lookup.defaultProps = { @@ -424,7 +287,6 @@ Lookup.defaultProps = { value: null, multiple: false, required: false, - qsNamespace: 'lookup', }; export { Lookup as _Lookup }; diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 9e40ebccec..0277aa0ad3 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -1,22 +1,29 @@ import React from 'react'; +import { withRouter } from 'react-router-dom'; import PropTypes from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { FormGroup, Tooltip } from '@patternfly/react-core'; import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; import styled from 'styled-components'; - import { CredentialsAPI, CredentialTypesAPI } from '@api'; -import Lookup from '@components/Lookup'; +import CategoryLookup from '@components/Lookup/CategoryLookup'; +import { getQSConfig, parseQueryString } from '@util/qs'; const QuestionCircleIcon = styled(PFQuestionCircleIcon)` margin-left: 10px; `; + +const QS_CONFIG = getQSConfig('credentials', { + page: 1, + page_size: 5, + order_by: 'name', +}); + function toggleCredentialSelection(credentialsToUpdate, newCredential) { let newCredentialsList; const isSelectedCredentialInState = - credentialsToUpdate.filter(cred => cred.id === newCredential.id).length > - 0; + credentialsToUpdate.filter(cred => cred.id === newCredential.id).length > 0; if (isSelectedCredentialInState) { newCredentialsList = credentialsToUpdate.filter( @@ -31,6 +38,7 @@ function toggleCredentialSelection(credentialsToUpdate, newCredential) { } return newCredentialsList; } + class MultiCredentialsLookup extends React.Component { constructor(props) { super(props); @@ -48,6 +56,7 @@ class MultiCredentialsLookup extends React.Component { componentDidMount() { this.loadCredentialTypes(); + this.loadCredentials(); } async loadCredentialTypes() { @@ -80,23 +89,38 @@ class MultiCredentialsLookup extends React.Component { } } - async loadCredentials(params) { + async loadCredentials() { + const { history, onError } = this.props; const { selectedCredentialType } = this.state; + const params = parseQueryString(QS_CONFIG, history.location.search); params.credential_type = selectedCredentialType.id || 1; - return CredentialsAPI.read(params); + try { + const { data } = await CredentialsAPI.read(params); + this.setState({ + credentials: data.results, + count: data.count, + }); + } catch (err) { + onError(err); + } } - - handleCredentialTypeSelect(value, type) { const { credentialTypes } = this.state; const selectedType = credentialTypes.filter(item => item.label === type); - this.setState({ selectedCredentialType: selectedType[0] }); + this.setState({ selectedCredentialType: selectedType[0] }, () => { + this.loadCredentials(); + }); } render() { - const { selectedCredentialType, credentialTypes } = this.state; - const { tooltip, i18n, credentials, onChange } = this.props; + const { + selectedCredentialType, + credentialTypes, + credentials, + count, + } = this.state; + const { tooltip, i18n, value, onChange } = this.props; return ( {tooltip && ( @@ -105,7 +129,7 @@ class MultiCredentialsLookup extends React.Component { )} {credentialTypes && ( - )} @@ -137,7 +161,7 @@ class MultiCredentialsLookup extends React.Component { MultiCredentialsLookup.propTypes = { tooltip: PropTypes.string, - credentials: PropTypes.arrayOf( + value: PropTypes.arrayOf( PropTypes.shape({ id: PropTypes.number, name: PropTypes.string, @@ -152,8 +176,8 @@ MultiCredentialsLookup.propTypes = { MultiCredentialsLookup.defaultProps = { tooltip: '', - credentials: [], + value: [], }; export { MultiCredentialsLookup as _MultiCredentialsLookup }; -export default withI18n()(MultiCredentialsLookup); +export default withI18n()(withRouter(MultiCredentialsLookup)); diff --git a/awx/ui_next/src/components/Lookup/index.js b/awx/ui_next/src/components/Lookup/index.js index cde48e2bcd..5e2959c00e 100644 --- a/awx/ui_next/src/components/Lookup/index.js +++ b/awx/ui_next/src/components/Lookup/index.js @@ -1,4 +1,5 @@ export { default } from './Lookup'; +export { default as CategoryLookup } from './CategoryLookup'; export { default as InstanceGroupsLookup } from './InstanceGroupsLookup'; export { default as InventoryLookup } from './InventoryLookup'; export { default as ProjectLookup } from './ProjectLookup'; diff --git a/awx/ui_next/src/components/SelectedList/SelectedList.jsx b/awx/ui_next/src/components/SelectedList/SelectedList.jsx index 8d7c716ef9..683404d35f 100644 --- a/awx/ui_next/src/components/SelectedList/SelectedList.jsx +++ b/awx/ui_next/src/components/SelectedList/SelectedList.jsx @@ -28,6 +28,7 @@ class SelectedList extends Component { isReadOnly, isCredentialList, } = this.props; + // TODO: replace isCredentialList with renderChip ? const chips = isCredentialList ? selected.map(item => ( null, isReadOnly: false, + isCredentialList: false, }; export default SelectedList; diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 846d7f4fb8..67e94ced20 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -317,7 +317,7 @@ class JobTemplateForm extends Component { fieldId="template-credentials" render={({ field }) => ( setFieldValue('credentials', newCredentials) } From 8ec856f3b6fa939b38da19264ea22ca0cd622a17 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 18 Nov 2019 15:20:58 -0800 Subject: [PATCH 03/19] start Lookup reducer --- .../CheckboxListItem/CheckboxListItem.jsx | 4 +- .../src/components/Lookup/CategoryLookup.jsx | 73 ++--- .../Lookup/InstanceGroupsLookup.jsx | 2 +- awx/ui_next/src/components/Lookup/Lookup.jsx | 42 +-- .../src/components/Lookup/NewLookup.jsx | 192 +++++++++++++ awx/ui_next/src/components/Lookup/README.md | 5 + .../components/Lookup/shared/SelectList.jsx | 84 ++++++ .../src/components/Lookup/shared/reducer.js | 110 ++++++++ .../components/Lookup/shared/reducer.test.js | 262 ++++++++++++++++++ 9 files changed, 696 insertions(+), 78 deletions(-) create mode 100644 awx/ui_next/src/components/Lookup/NewLookup.jsx create mode 100644 awx/ui_next/src/components/Lookup/README.md create mode 100644 awx/ui_next/src/components/Lookup/shared/SelectList.jsx create mode 100644 awx/ui_next/src/components/Lookup/shared/reducer.js create mode 100644 awx/ui_next/src/components/Lookup/shared/reducer.test.js diff --git a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx index 65958e47e8..9508672789 100644 --- a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx +++ b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx @@ -16,6 +16,7 @@ const CheckboxListItem = ({ label, isSelected, onSelect, + onDeselect, isRadio, }) => { const CheckboxRadio = isRadio ? DataListRadio : DataListCheck; @@ -25,7 +26,7 @@ const CheckboxListItem = ({ 1); - const chips = () => { - return selectCategoryOptions && selectCategoryOptions.length > 0 ? ( - - {(multiple ? value : [value]).map(chip => ( - this.removeItemAndSave(chip)} - isReadOnly={!canDelete} - credential={chip} - /> - ))} - - ) : ( - - {(multiple ? value : [value]).map(chip => ( - this.removeItemAndSave(chip)} - isReadOnly={!canDelete} - > - {chip.name} - - ))} - - ); - }; return ( @@ -231,7 +205,16 @@ class CategoryLookup extends React.Component { - {value ? chips(value) : null} + + {(multiple ? value : [value]).map(chip => ( + this.removeItemAndSave(chip)} + isReadOnly={!canDelete} + credential={chip} + /> + ))} + , ]} > - {selectedItems.length > 0 && ( - - )} - this.setState({ selectedItems: newVal })} + options={items} + optionCount={count} + columns={columns} + multiple={multiple} + header={lookupHeader} + name={name} qsConfig={qsConfig} - toolbarColumns={columns} - renderItem={item => ( - i.id === item.id)} - onSelect={() => this.addItem(item)} - isRadio={!multiple} - /> - )} - renderToolbar={props => } - showPageSizeOptions={false} + readOnly={!canDelete} /> diff --git a/awx/ui_next/src/components/Lookup/NewLookup.jsx b/awx/ui_next/src/components/Lookup/NewLookup.jsx new file mode 100644 index 0000000000..0721e4e4d1 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/NewLookup.jsx @@ -0,0 +1,192 @@ +import React, { Fragment, useReducer, useEffect } from 'react'; +import { + string, + bool, + arrayOf, + func, + number, + oneOfType, + shape, +} from 'prop-types'; +import { withRouter } from 'react-router-dom'; +import { SearchIcon } from '@patternfly/react-icons'; +import { + Button, + ButtonVariant, + InputGroup as PFInputGroup, + Modal, +} from '@patternfly/react-core'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import styled from 'styled-components'; + +import reducer, { initReducer } from './shared/reducer'; +import SelectList from './shared/SelectList'; +import { ChipGroup, Chip } from '../Chip'; +import { QSConfig } from '@types'; + +const SearchButton = styled(Button)` + ::after { + border: var(--pf-c-button--BorderWidth) solid + var(--pf-global--BorderColor--200); + } +`; + +const InputGroup = styled(PFInputGroup)` + ${props => + props.multiple && + ` + --pf-c-form-control--Height: 90px; + overflow-y: auto; + `} +`; + +const ChipHolder = styled.div` + --pf-c-form-control--BorderTopColor: var(--pf-global--BorderColor--200); + --pf-c-form-control--BorderRightColor: var(--pf-global--BorderColor--200); + border-top-right-radius: 3px; + border-bottom-right-radius: 3px; +`; + +function Lookup(props) { + const { + id, + items, + count, + header, + name, + onChange, + onBlur, + columns, + value, + multiple, + required, + qsConfig, + i18n, + } = props; + const [state, dispatch] = useReducer(reducer, props, initReducer); + + useEffect(() => { + dispatch({ type: 'SET_MULTIPLE', value: multiple }); + }, [multiple]); + + useEffect(() => { + dispatch({ type: 'SET_VALUE', value }); + }, [value]); + + const save = () => { + const { selectedItems } = state; + const val = multiple ? selectedItems : selectedItems[0] || null; + onChange(val); + dispatch({ type: 'CLOSE_MODAL' }); + }; + + const removeItem = item => { + if (multiple) { + onChange(value.filter(i => i.id !== item.id)); + } else { + onChange(null); + } + }; + + const { isModalOpen, selectedItems } = state; + + const canDelete = !required || (multiple && value.length > 1); + return ( + + + dispatch({ type: 'TOGGLE_MODAL' })} + variant={ButtonVariant.tertiary} + > + + + + + {(multiple ? value : [value]).map(item => ( + removeItem(item)} + isReadOnly={!canDelete} + > + {item.name} + + ))} + + + + dispatch({ type: 'TOGGLE_MODAL' })} + actions={[ + , + , + ]} + > + + + + ); +} + +const Item = shape({ + id: number.isRequired, +}); + +Lookup.propTypes = { + id: string, + items: arrayOf(shape({})).isRequired, + count: number.isRequired, + // TODO: change to `header` + header: string, + name: string, + onChange: func.isRequired, + value: oneOfType([Item, arrayOf(Item)]), + multiple: bool, + required: bool, + onBlur: func, + qsConfig: QSConfig.isRequired, +}; + +Lookup.defaultProps = { + id: 'lookup-search', + header: null, + name: null, + value: null, + multiple: false, + required: false, + onBlur: () => {}, +}; + +export { Lookup as _Lookup }; +export default withI18n()(withRouter(Lookup)); diff --git a/awx/ui_next/src/components/Lookup/README.md b/awx/ui_next/src/components/Lookup/README.md new file mode 100644 index 0000000000..4d5dc69674 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/README.md @@ -0,0 +1,5 @@ +# Lookup + +required single select lookups should not include a close X on the tag... you would have to select something else to change it + +optional single select lookups should include a close X to remove it on the spot diff --git a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx b/awx/ui_next/src/components/Lookup/shared/SelectList.jsx new file mode 100644 index 0000000000..96db387c72 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/shared/SelectList.jsx @@ -0,0 +1,84 @@ +import React from 'react'; +import { + arrayOf, + shape, + bool, + func, + number, + string, + oneOfType, +} from 'prop-types'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import SelectedList from '../../SelectedList'; +import PaginatedDataList from '../../PaginatedDataList'; +import CheckboxListItem from '../../CheckboxListItem'; +import DataListToolbar from '../../DataListToolbar'; +import { QSConfig } from '@types'; + +function SelectList({ + value, + options, + optionCount, + columns, + multiple, + header, + name, + qsConfig, + readOnly, + dispatch, + i18n, +}) { + return ( +
+ {value.length > 0 && ( + dispatch({ type: 'DESELECT_ITEM', item })} + isReadOnly={readOnly} + /> + )} + ( + i.id === item.id)} + onSelect={() => dispatch({ type: 'SELECT_ITEM', item })} + onDeselect={() => dispatch({ type: 'DESELECT_ITEM', item })} + isRadio={!multiple} + /> + )} + renderToolbar={props => } + showPageSizeOptions={false} + /> +
+ ); +} + +const Item = shape({ + id: oneOfType([number, string]).isRequired, +}); +SelectList.propTypes = { + value: arrayOf(Item).isRequired, + options: arrayOf(Item).isRequired, + optionCount: number.isRequired, + columns: arrayOf(shape({})).isRequired, + multiple: bool, + qsConfig: QSConfig.isRequired, + dispatch: func.isRequired, +}; +SelectList.defaultProps = { + multiple: false, +}; + +export default withI18n()(SelectList); diff --git a/awx/ui_next/src/components/Lookup/shared/reducer.js b/awx/ui_next/src/components/Lookup/shared/reducer.js new file mode 100644 index 0000000000..2e2c88f096 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/shared/reducer.js @@ -0,0 +1,110 @@ +export default function reducer(state, action) { + // console.log(action, state); + switch (action.type) { + case 'SELECT_ITEM': + return selectItem(state, action.item); + case 'DESELECT_ITEM': + return deselectItem(state, action.item); + case 'TOGGLE_MODAL': + return toggleModal(state); + case 'CLOSE_MODAL': + return closeModal(state); + case 'SET_MULTIPLE': + return { ...state, multiple: action.value }; + case 'SET_VALUE': + return { ...state, value: action.value }; + default: + throw new Error(`Unrecognized action type: ${action.type}`); + } +} + +function selectItem(state, item) { + const { selectedItems, multiple } = state; + if (!multiple) { + return { + ...state, + selectedItems: [item], + }; + } + const index = selectedItems.findIndex(i => i.id === item.id); + if (index > -1) { + return state; + } + return { + ...state, + selectedItems: [...selectedItems, item], + }; +} + +function deselectItem(state, item) { + return { + ...state, + selectedItems: state.selectedItems.filter(i => i.id !== item.id), + }; +} + +function toggleModal(state) { + const { isModalOpen, value, multiple } = state; + if (isModalOpen) { + return closeModal(state); + } + return { + ...state, + isModalOpen: !isModalOpen, + selectedItems: multiple ? [...value] : [value], + }; +} + +function closeModal(state) { + // TODO clear QSParams & push history state? + // state.clearQSParams(); + return { + ...state, + isModalOpen: false, + }; +} +// clearQSParams() { +// const { qsConfig, history } = this.props; +// const parts = history.location.search.replace(/^\?/, '').split('&'); +// const ns = qsConfig.namespace; +// const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); +// history.push(`${history.location.pathname}?${otherParts.join('&')}`); +// } + +export function initReducer({ + id, + items, + count, + header, + name, + onChange, + value, + multiple = false, + required = false, + qsConfig, +}) { + assertCorrectValueType(value, multiple); + let selectedItems = []; + if (value) { + selectedItems = multiple ? [...value] : [value]; + } + return { + selectedItems, + value, + multiple, + isModalOpen: false, + required, + onChange, + }; +} + +function assertCorrectValueType(value, multiple) { + if (!multiple && Array.isArray(value)) { + throw new Error( + 'Lookup value must not be an array unless `multiple` is set' + ); + } + if (multiple && !Array.isArray(value)) { + throw new Error('Lookup value must be an array if `multiple` is set'); + } +} diff --git a/awx/ui_next/src/components/Lookup/shared/reducer.test.js b/awx/ui_next/src/components/Lookup/shared/reducer.test.js new file mode 100644 index 0000000000..22bf9da106 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/shared/reducer.test.js @@ -0,0 +1,262 @@ +import reducer, { initReducer } from './reducer'; + +describe('Lookup reducer', () => { + describe('SELECT_ITEM', () => { + it('should add item to selected items (multiple select)', () => { + const state = { + selectedItems: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'SELECT_ITEM', + item: { id: 2 }, + }); + expect(result).toEqual({ + selectedItems: [{ id: 1 }, { id: 2 }], + multiple: true, + }); + }); + + it('should not duplicate item if already selected (multiple select)', () => { + const state = { + selectedItems: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'SELECT_ITEM', + item: { id: 1 }, + }); + expect(result).toEqual({ + selectedItems: [{ id: 1 }], + multiple: true, + }); + }); + + it('should replace selected item (single select)', () => { + const state = { + selectedItems: [{ id: 1 }], + multiple: false, + }; + const result = reducer(state, { + type: 'SELECT_ITEM', + item: { id: 2 }, + }); + expect(result).toEqual({ + selectedItems: [{ id: 2 }], + multiple: false, + }); + }); + + it('should not duplicate item if already selected (single select)', () => { + const state = { + selectedItems: [{ id: 1 }], + multiple: false, + }; + const result = reducer(state, { + type: 'SELECT_ITEM', + item: { id: 1 }, + }); + expect(result).toEqual({ + selectedItems: [{ id: 1 }], + multiple: false, + }); + }); + }); + + describe('DESELECT_ITEM', () => { + it('should de-select item (multiple)', () => { + const state = { + selectedItems: [{ id: 1 }, { id: 2 }], + multiple: true, + }; + const result = reducer(state, { + type: 'DESELECT_ITEM', + item: { id: 1 }, + }); + expect(result).toEqual({ + selectedItems: [{ id: 2 }], + multiple: true, + }); + }); + + it('should not change list if item not selected (multiple)', () => { + const state = { + selectedItems: [{ id: 1 }, { id: 2 }], + multiple: true, + }; + const result = reducer(state, { + type: 'DESELECT_ITEM', + item: { id: 3 }, + }); + expect(result).toEqual({ + selectedItems: [{ id: 1 }, { id: 2 }], + multiple: true, + }); + }); + + it('should de-select item (single select)', () => { + const state = { + selectedItems: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'DESELECT_ITEM', + item: { id: 1 }, + }); + expect(result).toEqual({ + selectedItems: [], + multiple: true, + }); + }); + }); + + describe('TOGGLE_MODAL', () => { + it('should open the modal (single)', () => { + const state = { + isModalOpen: false, + selectedItems: [], + value: { id: 1 }, + multiple: false, + }; + const result = reducer(state, { + type: 'TOGGLE_MODAL', + }); + expect(result).toEqual({ + isModalOpen: true, + selectedItems: [{ id: 1 }], + value: { id: 1 }, + multiple: false, + }); + }); + + it('should open the modal (multiple)', () => { + const state = { + isModalOpen: false, + selectedItems: [], + value: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'TOGGLE_MODAL', + }); + expect(result).toEqual({ + isModalOpen: true, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }); + }); + + it('should close the modal', () => { + const state = { + isModalOpen: true, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'TOGGLE_MODAL', + }); + expect(result).toEqual({ + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }); + }); + }); + + describe('CLOSE_MODAL', () => { + it('should close the modal', () => { + const state = { + isModalOpen: true, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'CLOSE_MODAL', + }); + expect(result).toEqual({ + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }); + }); + }); + + describe('SET_MULTIPLE', () => { + it('should set multiple to true', () => { + const state = { + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: false, + }; + const result = reducer(state, { + type: 'SET_MULTIPLE', + value: true, + }); + expect(result).toEqual({ + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }); + }); + + it('should set multiple to false', () => { + const state = { + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'SET_MULTIPLE', + value: false, + }); + expect(result).toEqual({ + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: [{ id: 1 }], + multiple: false, + }); + }); + }); + + describe('SET_VALUE', () => { + it('should set the value', () => { + const state = { + value: [{ id: 1 }], + multiple: true, + }; + const result = reducer(state, { + type: 'SET_VALUE', + value: [{ id: 3 }], + }); + expect(result).toEqual({ + value: [{ id: 3 }], + multiple: true, + }); + }); + }); +}); + +describe('initReducer', () => { + it('should init', () => { + const state = initReducer({ + value: [], + multiple: true, + required: true, + }); + expect(state).toEqual({ + selectedItems: [], + value: [], + multiple: true, + isModalOpen: false, + required: true, + }); + }); +}); From 62606339748a71d112a7ba958b9824a9ce521b3c Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 21 Nov 2019 16:11:19 -0800 Subject: [PATCH 04/19] flushing out new approach to MultiCredentialsLookup --- .../Lookup/InstanceGroupsLookup.jsx | 125 ++++---- .../Lookup/MultiCredentialsLookup.jsx | 289 ++++++++++-------- .../src/components/Lookup/NewLookup.jsx | 93 +++--- .../components/Lookup/shared/SelectList.jsx | 12 +- .../src/components/Lookup/shared/reducer.js | 40 ++- 5 files changed, 305 insertions(+), 254 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx index 089ec6d969..8071c8b26f 100644 --- a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx @@ -3,32 +3,21 @@ import { arrayOf, string, func, object } from 'prop-types'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { FormGroup, Tooltip } from '@patternfly/react-core'; -import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; -import styled from 'styled-components'; +import { FormGroup } from '@patternfly/react-core'; import { InstanceGroupsAPI } from '@api'; import { getQSConfig, parseQueryString } from '@util/qs'; +import { FieldTooltip } from '@components/FormField'; import Lookup from './NewLookup'; +import SelectList from './shared/SelectList'; -const QuestionCircleIcon = styled(PFQuestionCircleIcon)` - margin-left: 10px; -`; - -const QS_CONFIG = getQSConfig('instance-groups', { +const QS_CONFIG = getQSConfig('instance_groups', { page: 1, page_size: 5, order_by: 'name', }); -// const getInstanceGroups = async params => InstanceGroupsAPI.read(params); -function InstanceGroupsLookup({ - value, - onChange, - tooltip, - className, - history, - i18n, -}) { +function InstanceGroupsLookup(props) { + const { value, onChange, tooltip, className, history, i18n } = props; const [instanceGroups, setInstanceGroups] = useState([]); const [count, setCount] = useState(0); const [error, setError] = useState(null); @@ -46,56 +35,62 @@ function InstanceGroupsLookup({ })(); }, [history.location]); - /* - Wrapping
added to workaround PF bug: - https://github.com/patternfly/patternfly-react/issues/2855 - */ return ( -
- - {tooltip && ( - - - + + {tooltip && } + ( + dispatch({ type: 'SELECT_ITEM', item })} + deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + /> )} - - {error ?
error {error.message}
: ''} - -
+ /> + {error ?
error {error.message}
: ''} + ); } diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 0277aa0ad3..005c8d620d 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -1,18 +1,18 @@ -import React from 'react'; +import React, { useState, useEffect } from 'react'; import { withRouter } from 'react-router-dom'; import PropTypes from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { FormGroup, Tooltip } from '@patternfly/react-core'; -import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; -import styled from 'styled-components'; +import { FormGroup, ToolbarItem } from '@patternfly/react-core'; import { CredentialsAPI, CredentialTypesAPI } from '@api'; -import CategoryLookup from '@components/Lookup/CategoryLookup'; +import AnsibleSelect from '@components/AnsibleSelect'; +import { FieldTooltip } from '@components/FormField'; +import { CredentialChip } from '@components/Chip'; +import VerticalSeperator from '@components/VerticalSeparator'; import { getQSConfig, parseQueryString } from '@util/qs'; - -const QuestionCircleIcon = styled(PFQuestionCircleIcon)` - margin-left: 10px; -`; +import Lookup from './NewLookup'; +import SelectList from './shared/SelectList'; +import multiCredentialReducer from './shared/multiCredentialReducer'; const QS_CONFIG = getQSConfig('credentials', { page: 1, @@ -20,6 +20,7 @@ const QS_CONFIG = getQSConfig('credentials', { order_by: 'name', }); +// TODO: move into reducer function toggleCredentialSelection(credentialsToUpdate, newCredential) { let newCredentialsList; const isSelectedCredentialInState = @@ -39,124 +40,164 @@ function toggleCredentialSelection(credentialsToUpdate, newCredential) { return newCredentialsList; } -class MultiCredentialsLookup extends React.Component { - constructor(props) { - super(props); - - this.state = { - selectedCredentialType: { label: 'Machine', id: 1, kind: 'ssh' }, - credentialTypes: [], - }; - this.loadCredentialTypes = this.loadCredentialTypes.bind(this); - this.handleCredentialTypeSelect = this.handleCredentialTypeSelect.bind( - this - ); - this.loadCredentials = this.loadCredentials.bind(this); - } - - componentDidMount() { - this.loadCredentialTypes(); - this.loadCredentials(); - } - - async loadCredentialTypes() { - const { onError } = this.props; - try { - const { data } = await CredentialTypesAPI.read(); - const acceptableTypes = ['machine', 'cloud', 'net', 'ssh', 'vault']; - const credentialTypes = []; - data.results.forEach(cred => { - acceptableTypes.forEach(aT => { - if (aT === cred.kind) { - // This object has several repeated values as some of it's children - // require different field values. - cred = { - id: cred.id, - key: cred.id, - kind: cred.kind, - type: cred.namespace, - value: cred.name, - label: cred.name, - isDisabled: false, - }; - credentialTypes.push(cred); - } - }); - }); - this.setState({ credentialTypes }); - } catch (err) { - onError(err); - } - } - - async loadCredentials() { - const { history, onError } = this.props; - const { selectedCredentialType } = this.state; - const params = parseQueryString(QS_CONFIG, history.location.search); - params.credential_type = selectedCredentialType.id || 1; - try { - const { data } = await CredentialsAPI.read(params); - this.setState({ - credentials: data.results, - count: data.count, - }); - } catch (err) { - onError(err); - } - } - - handleCredentialTypeSelect(value, type) { - const { credentialTypes } = this.state; - const selectedType = credentialTypes.filter(item => item.label === type); - this.setState({ selectedCredentialType: selectedType[0] }, () => { - this.loadCredentials(); +async function loadCredentialTypes() { + const { data } = await CredentialTypesAPI.read(); + const acceptableTypes = ['machine', 'cloud', 'net', 'ssh', 'vault']; + const credentialTypes = []; + // TODO: cleanup + data.results.forEach(cred => { + acceptableTypes.forEach(aT => { + if (aT === cred.kind) { + // This object has several repeated values as some of it's children + // require different field values. + cred = { + id: cred.id, + key: cred.id, + kind: cred.kind, + type: cred.namespace, + value: cred.name, + label: cred.name, + isDisabled: false, + }; + credentialTypes.push(cred); + } }); - } + }); + return credentialTypes; +} - render() { - const { - selectedCredentialType, - credentialTypes, - credentials, - count, - } = this.state; - const { tooltip, i18n, value, onChange } = this.props; - return ( - - {tooltip && ( - - - - )} - {credentialTypes && ( - { + (async () => { + try { + const types = await loadCredentialTypes(); + setCredentialTypes(types); + setSelectedType(types[0]); + } catch (err) { + onError(err); + } + })(); + }, []); + + useEffect(() => { + console.log('useEffect', selectedType); + (async () => { + if (!selectedType) { + return; + } + try { + const params = parseQueryString(QS_CONFIG, history.location.search); + const { results, count } = await loadCredentials( + params, + selectedType.id + ); + setCredentials(results); + setCredentialsCount(count); + } catch (err) { + onError(err); + } + })(); + }, [selectedType]); + + // handleCredentialTypeSelect(value, type) { + // const { credentialTypes } = this.state; + // const selectedType = credentialTypes.filter(item => item.label === type); + // this.setState({ selectedCredentialType: selectedType[0] }, () => { + // this.loadCredentials(); + // }); + // } + + // const { + // selectedCredentialType, + // credentialTypes, + // credentials, + // credentialsCount, + // } = state; + + return ( + + {tooltip && } + ( + removeItem(item)} + isReadOnly={!canDelete} + credential={item} /> )} - - ); - } + renderSelectList={({ state, dispatch, canDelete }) => { + return ( + <> + {credentialTypes && credentialTypes.length > 0 && ( + +
{i18n._(t`Selected Category`)}
+ + { + setSelectedType( + credentialTypes.find(o => o.label === label) + ); + }} + /> + + )} + {}} + deselectItem={() => {}} + /> + + ); + }} + /> + + ); } MultiCredentialsLookup.propTypes = { @@ -178,6 +219,6 @@ MultiCredentialsLookup.defaultProps = { tooltip: '', value: [], }; -export { MultiCredentialsLookup as _MultiCredentialsLookup }; +export { MultiCredentialsLookup as _MultiCredentialsLookup }; export default withI18n()(withRouter(MultiCredentialsLookup)); diff --git a/awx/ui_next/src/components/Lookup/NewLookup.jsx b/awx/ui_next/src/components/Lookup/NewLookup.jsx index 0721e4e4d1..0f3342e5b5 100644 --- a/awx/ui_next/src/components/Lookup/NewLookup.jsx +++ b/awx/ui_next/src/components/Lookup/NewLookup.jsx @@ -21,7 +21,6 @@ import { t } from '@lingui/macro'; import styled from 'styled-components'; import reducer, { initReducer } from './shared/reducer'; -import SelectList from './shared/SelectList'; import { ChipGroup, Chip } from '../Chip'; import { QSConfig } from '@types'; @@ -51,20 +50,28 @@ const ChipHolder = styled.div` function Lookup(props) { const { id, - items, - count, + // items, + // count, header, - name, + // name, onChange, onBlur, - columns, + // columns, value, multiple, required, qsConfig, + renderItemChip, + renderSelectList, + history, i18n, } = props; - const [state, dispatch] = useReducer(reducer, props, initReducer); + + const [state, dispatch] = useReducer( + reducer, + { value, multiple, required }, + initReducer + ); useEffect(() => { dispatch({ type: 'SET_MULTIPLE', value: multiple }); @@ -74,10 +81,18 @@ function Lookup(props) { dispatch({ type: 'SET_VALUE', value }); }, [value]); + const clearQSParams = () => { + const parts = history.location.search.replace(/^\?/, '').split('&'); + const ns = qsConfig.namespace; + const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); + history.push(`${history.location.pathname}?${otherParts.join('&')}`); + }; + const save = () => { const { selectedItems } = state; const val = multiple ? selectedItems : selectedItems[0] || null; onChange(val); + clearQSParams(); dispatch({ type: 'CLOSE_MODAL' }); }; @@ -89,8 +104,12 @@ function Lookup(props) { } }; - const { isModalOpen, selectedItems } = state; + const closeModal = () => { + clearQSParams(); + dispatch({ type: 'CLOSE_MODAL' }); + }; + const { isModalOpen, selectedItems } = state; const canDelete = !required || (multiple && value.length > 1); return ( @@ -105,15 +124,13 @@ function Lookup(props) { - {(multiple ? value : [value]).map(item => ( - removeItem(item)} - isReadOnly={!canDelete} - > - {item.name} - - ))} + {(multiple ? value : [value]).map(item => + renderItemChip({ + item, + removeItem, + canDelete, + }) + )} @@ -121,7 +138,7 @@ function Lookup(props) { className="awx-c-modal" title={i18n._(t`Select ${header || i18n._(t`Items`)}`)} isOpen={isModalOpen} - onClose={() => dispatch({ type: 'TOGGLE_MODAL' })} + onClose={closeModal} actions={[ , - , ]} > - + {renderSelectList({ + state, + dispatch, + canDelete, + })} ); @@ -165,27 +171,38 @@ const Item = shape({ Lookup.propTypes = { id: string, - items: arrayOf(shape({})).isRequired, - count: number.isRequired, + // items: arrayOf(shape({})).isRequired, + // count: number.isRequired, // TODO: change to `header` header: string, - name: string, + // name: string, onChange: func.isRequired, value: oneOfType([Item, arrayOf(Item)]), multiple: bool, required: bool, onBlur: func, qsConfig: QSConfig.isRequired, + renderItemChip: func, + renderSelectList: func.isRequired, }; Lookup.defaultProps = { id: 'lookup-search', header: null, - name: null, + // name: null, value: null, multiple: false, required: false, onBlur: () => {}, + renderItemChip: ({ item, removeItem, canDelete }) => ( + removeItem(item)} + isReadOnly={!canDelete} + > + {item.name} + + ), }; export { Lookup as _Lookup }; diff --git a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx b/awx/ui_next/src/components/Lookup/shared/SelectList.jsx index 96db387c72..128d9371d8 100644 --- a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx +++ b/awx/ui_next/src/components/Lookup/shared/SelectList.jsx @@ -26,7 +26,8 @@ function SelectList({ name, qsConfig, readOnly, - dispatch, + selectItem, + deselectItem, i18n, }) { return ( @@ -36,7 +37,7 @@ function SelectList({ label={i18n._(t`Selected`)} selected={value} showOverflowAfter={5} - onRemove={item => dispatch({ type: 'DESELECT_ITEM', item })} + onRemove={item => deselectItem(item)} isReadOnly={readOnly} /> )} @@ -53,8 +54,8 @@ function SelectList({ name={multiple ? item.name : name} label={item.name} isSelected={value.some(i => i.id === item.id)} - onSelect={() => dispatch({ type: 'SELECT_ITEM', item })} - onDeselect={() => dispatch({ type: 'DESELECT_ITEM', item })} + onSelect={() => selectItem(item)} + onDeselect={() => deselectItem(item)} isRadio={!multiple} /> )} @@ -75,7 +76,8 @@ SelectList.propTypes = { columns: arrayOf(shape({})).isRequired, multiple: bool, qsConfig: QSConfig.isRequired, - dispatch: func.isRequired, + selectItem: func.isRequired, + deselectItem: func.isRequired, }; SelectList.defaultProps = { multiple: false, diff --git a/awx/ui_next/src/components/Lookup/shared/reducer.js b/awx/ui_next/src/components/Lookup/shared/reducer.js index 2e2c88f096..9f7f6b37e0 100644 --- a/awx/ui_next/src/components/Lookup/shared/reducer.js +++ b/awx/ui_next/src/components/Lookup/shared/reducer.js @@ -1,3 +1,5 @@ +// import { useReducer, useEffect } from 'react'; + export default function reducer(state, action) { // console.log(action, state); switch (action.type) { @@ -56,33 +58,13 @@ function toggleModal(state) { } function closeModal(state) { - // TODO clear QSParams & push history state? - // state.clearQSParams(); return { ...state, isModalOpen: false, }; } -// clearQSParams() { -// const { qsConfig, history } = this.props; -// const parts = history.location.search.replace(/^\?/, '').split('&'); -// const ns = qsConfig.namespace; -// const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); -// history.push(`${history.location.pathname}?${otherParts.join('&')}`); -// } -export function initReducer({ - id, - items, - count, - header, - name, - onChange, - value, - multiple = false, - required = false, - qsConfig, -}) { +export function initReducer({ value, multiple = false, required = false }) { assertCorrectValueType(value, multiple); let selectedItems = []; if (value) { @@ -94,7 +76,6 @@ export function initReducer({ multiple, isModalOpen: false, required, - onChange, }; } @@ -108,3 +89,18 @@ function assertCorrectValueType(value, multiple) { throw new Error('Lookup value must be an array if `multiple` is set'); } } +// +// export function useLookup(config) { +// const { value, multiple, required, onChange, history } = config; +// const [state, dispatch] = useReducer( +// config.reducer || reducer, +// { +// value, +// multiple, +// required, +// }, +// config.initReducer || initReducer +// ); +// +// return [state, dispatch]; +// } From 4341d67fb0f7aa6ea989f77ef7158b7b54e73135 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 25 Nov 2019 10:02:22 -0800 Subject: [PATCH 05/19] add MultiCredentialsLookup select/deselect logic --- .../Lookup/MultiCredentialsLookup.jsx | 40 ++++++++----------- .../src/components/Lookup/NewLookup.jsx | 5 --- .../src/components/Lookup/shared/reducer.js | 2 + 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 005c8d620d..7578205923 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -92,7 +92,6 @@ function MultiCredentialsLookup(props) { }, []); useEffect(() => { - console.log('useEffect', selectedType); (async () => { if (!selectedType) { return; @@ -111,21 +110,7 @@ function MultiCredentialsLookup(props) { })(); }, [selectedType]); - // handleCredentialTypeSelect(value, type) { - // const { credentialTypes } = this.state; - // const selectedType = credentialTypes.filter(item => item.label === type); - // this.setState({ selectedCredentialType: selectedType[0] }, () => { - // this.loadCredentials(); - // }); - // } - - // const { - // selectedCredentialType, - // credentialTypes, - // credentials, - // credentialsCount, - // } = state; - + const isMultiple = selectedType && selectedType.value === 'Vault'; return ( {tooltip && } @@ -134,15 +119,10 @@ function MultiCredentialsLookup(props) { onToggleItem={toggleCredentialSelection} id="multiCredential" lookupHeader={i18n._(t`Credentials`)} - // name="credentials" value={value} multiple onChange={onChange} - // items={credentials} - // count={credentialsCount} qsConfig={QS_CONFIG} - // columns={} - // TODO bind removeItem renderItemChip={({ item, removeItem, canDelete }) => ( {}} - deselectItem={() => {}} + selectItem={item => { + if (isMultiple) { + return dispatch({ type: 'SELECT_ITEM', item }); + } + const selectedItems = state.selectedItems.filter( + i => i.kind !== item.kind + ); + selectedItems.push(item); + return dispatch({ + type: 'SET_SELECTED_ITEMS', + selectedItems, + }); + }} + deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} /> ); diff --git a/awx/ui_next/src/components/Lookup/NewLookup.jsx b/awx/ui_next/src/components/Lookup/NewLookup.jsx index 0f3342e5b5..c0e8cfa943 100644 --- a/awx/ui_next/src/components/Lookup/NewLookup.jsx +++ b/awx/ui_next/src/components/Lookup/NewLookup.jsx @@ -171,11 +171,7 @@ const Item = shape({ Lookup.propTypes = { id: string, - // items: arrayOf(shape({})).isRequired, - // count: number.isRequired, - // TODO: change to `header` header: string, - // name: string, onChange: func.isRequired, value: oneOfType([Item, arrayOf(Item)]), multiple: bool, @@ -189,7 +185,6 @@ Lookup.propTypes = { Lookup.defaultProps = { id: 'lookup-search', header: null, - // name: null, value: null, multiple: false, required: false, diff --git a/awx/ui_next/src/components/Lookup/shared/reducer.js b/awx/ui_next/src/components/Lookup/shared/reducer.js index 9f7f6b37e0..d608c58cde 100644 --- a/awx/ui_next/src/components/Lookup/shared/reducer.js +++ b/awx/ui_next/src/components/Lookup/shared/reducer.js @@ -15,6 +15,8 @@ export default function reducer(state, action) { return { ...state, multiple: action.value }; case 'SET_VALUE': return { ...state, value: action.value }; + case 'SET_SELECTED_ITEMS': + return { ...state, selectedItems: action.selectedItems }; default: throw new Error(`Unrecognized action type: ${action.type}`); } From 639b297027452a55580e15ec7aab72e2ac848134 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 25 Nov 2019 14:22:34 -0800 Subject: [PATCH 06/19] fix credential chips in SelectedList, MultiCredential cleanup --- .../AnsibleSelect/AnsibleSelect.jsx | 19 +- .../src/components/Lookup/CategoryLookup.jsx | 331 ------------------ .../Lookup/MultiCredentialsLookup.jsx | 90 ++--- .../src/components/Lookup/NewLookup.jsx | 4 - awx/ui_next/src/components/Lookup/index.js | 1 - .../components/Lookup/shared/SelectList.jsx | 4 + .../components/SelectedList/SelectedList.jsx | 48 ++- 7 files changed, 69 insertions(+), 428 deletions(-) delete mode 100644 awx/ui_next/src/components/Lookup/CategoryLookup.jsx diff --git a/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx b/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx index 1de791ab58..6c87032341 100644 --- a/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -36,12 +36,12 @@ class AnsibleSelect extends React.Component { aria-label={i18n._(t`Select Input`)} isValid={isValid} > - {data.map(datum => ( + {data.map(option => ( ))} @@ -49,6 +49,13 @@ class AnsibleSelect extends React.Component { } } +const Option = shape({ + id: oneOfType([string, number]).isRequired, + value: oneOfType([string, number]).isRequired, + label: string.isRequired, + isDisabled: bool, +}); + AnsibleSelect.defaultProps = { data: [], isValid: true, @@ -56,7 +63,7 @@ AnsibleSelect.defaultProps = { }; AnsibleSelect.propTypes = { - data: arrayOf(shape()), + data: arrayOf(Option), id: string.isRequired, isValid: bool, onBlur: func, diff --git a/awx/ui_next/src/components/Lookup/CategoryLookup.jsx b/awx/ui_next/src/components/Lookup/CategoryLookup.jsx deleted file mode 100644 index 90009079bb..0000000000 --- a/awx/ui_next/src/components/Lookup/CategoryLookup.jsx +++ /dev/null @@ -1,331 +0,0 @@ -import React, { Fragment } from 'react'; -import { - string, - bool, - arrayOf, - func, - number, - oneOfType, - shape, -} from 'prop-types'; -import { withRouter } from 'react-router-dom'; -import { SearchIcon } from '@patternfly/react-icons'; -import { - Button, - ButtonVariant, - InputGroup as PFInputGroup, - Modal, - ToolbarItem, -} from '@patternfly/react-core'; -import { withI18n } from '@lingui/react'; -import { t } from '@lingui/macro'; -import styled from 'styled-components'; - -import AnsibleSelect from '../AnsibleSelect'; -import PaginatedDataList from '../PaginatedDataList'; -import VerticalSeperator from '../VerticalSeparator'; -import DataListToolbar from '../DataListToolbar'; -import CheckboxListItem from '../CheckboxListItem'; -import SelectedList from '../SelectedList'; -import { ChipGroup, CredentialChip } from '../Chip'; -import { QSConfig } from '@types'; - -const SearchButton = styled(Button)` - ::after { - border: var(--pf-c-button--BorderWidth) solid - var(--pf-global--BorderColor--200); - } -`; - -const InputGroup = styled(PFInputGroup)` - ${props => - props.multiple && - ` - --pf-c-form-control--Height: 90px; - overflow-y: auto; - `} -`; - -const ChipHolder = styled.div` - --pf-c-form-control--BorderTopColor: var(--pf-global--BorderColor--200); - --pf-c-form-control--BorderRightColor: var(--pf-global--BorderColor--200); - border-top-right-radius: 3px; - border-bottom-right-radius: 3px; -`; - -class CategoryLookup extends React.Component { - constructor(props) { - super(props); - - // this.assertCorrectValueType(); - let selectedItems = []; - if (props.value) { - selectedItems = props.multiple ? [...props.value] : [props.value]; - } - this.state = { - isModalOpen: false, - selectedItems, - error: null, - }; - this.handleModalToggle = this.handleModalToggle.bind(this); - this.addItem = this.addItem.bind(this); - this.removeItem = this.removeItem.bind(this); - this.saveModal = this.saveModal.bind(this); - this.clearQSParams = this.clearQSParams.bind(this); - } - - // assertCorrectValueType() { - // const { multiple, value, selectCategoryOptions } = this.props; - // if (selectCategoryOptions) { - // return; - // } - // if (!multiple && Array.isArray(value)) { - // throw new Error( - // 'CategoryLookup value must not be an array unless `multiple` is set' - // ); - // } - // if (multiple && !Array.isArray(value)) { - // throw new Error( - // 'CategoryLookup value must be an array if `multiple` is set' - // ); - // } - // } - - removeItem(row) { - const { selectedItems } = this.state; - const { onToggleItem } = this.props; - if (onToggleItem) { - this.setState({ selectedItems: onToggleItem(selectedItems, row) }); - return; - } - this.setState({ - selectedItems: selectedItems.filter(item => item.id !== row.id), - }); - } - - addItem(row) { - const { selectedItems } = this.state; - const { multiple, onToggleItem } = this.props; - if (onToggleItem) { - this.setState({ selectedItems: onToggleItem(selectedItems, row) }); - return; - } - const index = selectedItems.findIndex(item => item.id === row.id); - - if (!multiple) { - this.setState({ selectedItems: [row] }); - return; - } - if (index > -1) { - return; - } - this.setState({ selectedItems: [...selectedItems, row] }); - } - - // TODO: clean up - handleModalToggle() { - const { isModalOpen } = this.state; - const { value, multiple, selectCategory } = this.props; - // Resets the selected items from parent state whenever modal is opened - // This handles the case where the user closes/cancels the modal and - // opens it again - if (!isModalOpen) { - let selectedItems = []; - if (value) { - selectedItems = multiple ? [...value] : [value]; - } - this.setState({ selectedItems }); - } else { - this.clearQSParams(); - if (selectCategory) { - selectCategory(null, 'Machine'); - } - } - this.setState(prevState => ({ - isModalOpen: !prevState.isModalOpen, - })); - } - - removeItemAndSave(row) { - const { value, onChange, multiple } = this.props; - if (multiple) { - onChange(value.filter(item => item.id !== row.id)); - } else if (value.id === row.id) { - onChange(null); - } - } - - saveModal() { - const { onChange, multiple } = this.props; - const { selectedItems } = this.state; - const value = multiple ? selectedItems : selectedItems[0] || null; - - this.handleModalToggle(); - onChange(value); - } - - clearQSParams() { - const { qsConfig, history } = this.props; - const parts = history.location.search.replace(/^\?/, '').split('&'); - const ns = qsConfig.namespace; - const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); - history.push(`${history.location.pathname}?${otherParts.join('&')}`); - } - - render() { - const { isModalOpen, selectedItems, error } = this.state; - const { - id, - items, - count, - lookupHeader, - value, - columns, - multiple, - name, - onBlur, - qsConfig, - required, - selectCategory, - selectCategoryOptions, - selectedCategory, - i18n, - } = this.props; - const header = lookupHeader || i18n._(t`Items`); - const canDelete = !required || (multiple && value.length > 1); - return ( - - - - - - - - {(multiple ? value : [value]).map(chip => ( - this.removeItemAndSave(chip)} - isReadOnly={!canDelete} - credential={chip} - /> - ))} - - - - - {i18n._(t`Select`)} - , - , - ]} - > - {selectCategoryOptions && selectCategoryOptions.length > 0 && ( - - Selected Category - - - - )} - {selectedItems.length > 0 && ( - 0 - } - /> - )} - ( - i.id === item.id)} - onSelect={() => this.addItem(item)} - isRadio={ - !multiple || - (selectCategoryOptions && - selectCategoryOptions.length && - selectedCategory.value !== 'Vault') - } - /> - )} - renderToolbar={props => } - showPageSizeOptions={false} - /> - {error ?
error: {error.message}
: ''} - - - ); - } -} - -const Item = shape({ - id: number.isRequired, -}); - -CategoryLookup.propTypes = { - id: string, - items: arrayOf(shape({})).isRequired, - // TODO: change to `header` - lookupHeader: string, - name: string, - onChange: func.isRequired, - value: oneOfType([Item, arrayOf(Item)]), - multiple: bool, - required: bool, - qsConfig: QSConfig.isRequired, - selectCategory: func.isRequired, - selectCategoryOptions: oneOfType(shape({})).isRequired, - selectedCategory: shape({}).isRequired, -}; - -CategoryLookup.defaultProps = { - id: 'lookup-search', - lookupHeader: null, - name: null, - value: null, - multiple: false, - required: false, -}; - -export { CategoryLookup as _CategoryLookup }; -export default withI18n()(withRouter(CategoryLookup)); diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 7578205923..20d5180e7c 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { Fragment, useState, useEffect } from 'react'; import { withRouter } from 'react-router-dom'; import PropTypes from 'prop-types'; import { withI18n } from '@lingui/react'; @@ -12,7 +12,6 @@ import VerticalSeperator from '@components/VerticalSeparator'; import { getQSConfig, parseQueryString } from '@util/qs'; import Lookup from './NewLookup'; import SelectList from './shared/SelectList'; -import multiCredentialReducer from './shared/multiCredentialReducer'; const QS_CONFIG = getQSConfig('credentials', { page: 1, @@ -20,50 +19,10 @@ const QS_CONFIG = getQSConfig('credentials', { order_by: 'name', }); -// TODO: move into reducer -function toggleCredentialSelection(credentialsToUpdate, newCredential) { - let newCredentialsList; - const isSelectedCredentialInState = - credentialsToUpdate.filter(cred => cred.id === newCredential.id).length > 0; - - if (isSelectedCredentialInState) { - newCredentialsList = credentialsToUpdate.filter( - cred => cred.id !== newCredential.id - ); - } else { - newCredentialsList = credentialsToUpdate.filter( - credential => - credential.kind === 'vault' || credential.kind !== newCredential.kind - ); - newCredentialsList = [...newCredentialsList, newCredential]; - } - return newCredentialsList; -} - async function loadCredentialTypes() { const { data } = await CredentialTypesAPI.read(); const acceptableTypes = ['machine', 'cloud', 'net', 'ssh', 'vault']; - const credentialTypes = []; - // TODO: cleanup - data.results.forEach(cred => { - acceptableTypes.forEach(aT => { - if (aT === cred.kind) { - // This object has several repeated values as some of it's children - // require different field values. - cred = { - id: cred.id, - key: cred.id, - kind: cred.kind, - type: cred.namespace, - value: cred.name, - label: cred.name, - isDisabled: false, - }; - credentialTypes.push(cred); - } - }); - }); - return credentialTypes; + return data.results.filter(type => acceptableTypes.includes(type.kind)); } async function loadCredentials(params, selectedCredentialTypeId) { @@ -78,13 +37,16 @@ function MultiCredentialsLookup(props) { const [selectedType, setSelectedType] = useState(null); const [credentials, setCredentials] = useState([]); const [credentialsCount, setCredentialsCount] = useState(0); + const [isLoading, setIsLoading] = useState(false); useEffect(() => { (async () => { try { const types = await loadCredentialTypes(); setCredentialTypes(types); - setSelectedType(types[0]); + setSelectedType( + types.find(type => type.name === 'Machine') || types[0] + ); } catch (err) { onError(err); } @@ -98,10 +60,12 @@ function MultiCredentialsLookup(props) { } try { const params = parseQueryString(QS_CONFIG, history.location.search); + setIsLoading(true); const { results, count } = await loadCredentials( params, selectedType.id ); + setIsLoading(false); setCredentials(results); setCredentialsCount(count); } catch (err) { @@ -111,29 +75,29 @@ function MultiCredentialsLookup(props) { }, [selectedType]); const isMultiple = selectedType && selectedType.value === 'Vault'; + const renderChip = ({ item, removeItem, canDelete }) => ( + removeItem(item)} + isReadOnly={!canDelete} + credential={item} + /> + ); + return ( {tooltip && } ( - removeItem(item)} - isReadOnly={!canDelete} - credential={item} - /> - )} + renderItemChip={renderChip} renderSelectList={({ state, dispatch, canDelete }) => { return ( - <> + {credentialTypes && credentialTypes.length > 0 && (
{i18n._(t`Selected Category`)}
@@ -142,11 +106,16 @@ function MultiCredentialsLookup(props) { css="flex: 1 1 75%;" id="multiCredentialsLookUp-select" label={i18n._(t`Selected Category`)} - data={credentialTypes} - value={selectedType && selectedType.label} - onChange={(e, label) => { + data={credentialTypes.map(type => ({ + id: type.id, + value: type.id, + label: type.name, + isDisabled: false, + }))} + value={selectedType && selectedType.id} + onChange={(e, id) => { setSelectedType( - credentialTypes.find(o => o.label === label) + credentialTypes.find(o => o.id === parseInt(id, 10)) ); }} /> @@ -183,8 +152,9 @@ function MultiCredentialsLookup(props) { }); }} deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + renderItemChip={renderChip} /> - + ); }} /> diff --git a/awx/ui_next/src/components/Lookup/NewLookup.jsx b/awx/ui_next/src/components/Lookup/NewLookup.jsx index c0e8cfa943..8c93b18d87 100644 --- a/awx/ui_next/src/components/Lookup/NewLookup.jsx +++ b/awx/ui_next/src/components/Lookup/NewLookup.jsx @@ -50,13 +50,9 @@ const ChipHolder = styled.div` function Lookup(props) { const { id, - // items, - // count, header, - // name, onChange, onBlur, - // columns, value, multiple, required, diff --git a/awx/ui_next/src/components/Lookup/index.js b/awx/ui_next/src/components/Lookup/index.js index 5e2959c00e..cde48e2bcd 100644 --- a/awx/ui_next/src/components/Lookup/index.js +++ b/awx/ui_next/src/components/Lookup/index.js @@ -1,5 +1,4 @@ export { default } from './Lookup'; -export { default as CategoryLookup } from './CategoryLookup'; export { default as InstanceGroupsLookup } from './InstanceGroupsLookup'; export { default as InventoryLookup } from './InventoryLookup'; export { default as ProjectLookup } from './ProjectLookup'; diff --git a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx b/awx/ui_next/src/components/Lookup/shared/SelectList.jsx index 128d9371d8..b9a2ec1f84 100644 --- a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx +++ b/awx/ui_next/src/components/Lookup/shared/SelectList.jsx @@ -28,6 +28,7 @@ function SelectList({ readOnly, selectItem, deselectItem, + renderItemChip, i18n, }) { return ( @@ -39,6 +40,7 @@ function SelectList({ showOverflowAfter={5} onRemove={item => deselectItem(item)} isReadOnly={readOnly} + renderItemChip={renderItemChip} /> )} ( - onRemove(item)} - credential={item} - > - {item[displayKey]} - - )) - : selected.map(item => ( - onRemove(item)} - > - {item[displayKey]} - - )); + + const renderChip = + renderItemChip || + (({ item, removeItem }) => ( + + {item[displayKey]} + + )); + return ( {label} - {chips} + + {selected.map(item => + renderChip({ + item, + removeItem: () => onRemove(item), + canDelete: !isReadOnly, + }) + )} + ); @@ -67,7 +63,7 @@ SelectedList.propTypes = { onRemove: PropTypes.func, selected: PropTypes.arrayOf(PropTypes.object).isRequired, isReadOnly: PropTypes.bool, - isCredentialList: PropTypes.bool, + renderItemChip: PropTypes.func, }; SelectedList.defaultProps = { @@ -75,7 +71,7 @@ SelectedList.defaultProps = { label: 'Selected', onRemove: () => null, isReadOnly: false, - isCredentialList: false, + renderItemChip: null, }; export default SelectedList; From cb07e9c757b9eca693102721e5715c80940bf9eb Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 26 Nov 2019 13:38:44 -0800 Subject: [PATCH 07/19] convert all lookups to use new Lookup component --- .../AnsibleSelect/AnsibleSelect.jsx | 4 +- .../components/Lookup/CredentialLookup.jsx | 60 +++- .../Lookup/InstanceGroupsLookup.jsx | 16 +- .../src/components/Lookup/InventoryLookup.jsx | 141 +++++--- awx/ui_next/src/components/Lookup/Lookup.jsx | 310 +++++++----------- .../Lookup/MultiCredentialsLookup.jsx | 15 +- .../src/components/Lookup/NewLookup.jsx | 200 ----------- .../components/Lookup/OrganizationLookup.jsx | 52 ++- .../src/components/Lookup/ProjectLookup.jsx | 121 ++++--- .../Lookup/shared/LookupErrorMessage.jsx | 15 + .../{SelectList.jsx => OptionsList.jsx} | 8 +- 11 files changed, 406 insertions(+), 536 deletions(-) delete mode 100644 awx/ui_next/src/components/Lookup/NewLookup.jsx create mode 100644 awx/ui_next/src/components/Lookup/shared/LookupErrorMessage.jsx rename awx/ui_next/src/components/Lookup/shared/{SelectList.jsx => OptionsList.jsx} (94%) diff --git a/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx b/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx index 6c87032341..c6e427309d 100644 --- a/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -38,7 +38,7 @@ class AnsibleSelect extends React.Component { > {data.map(option => ( - CredentialsAPI.read( - mergeParams(params, { credential_type: credentialTypeId }) - ); + const [credentials, setCredentials] = useState([]); + const [count, setCount] = useState(0); + const [error, setError] = useState(null); + + useEffect(() => { + (async () => { + const params = parseQueryString(QS_CONFIG, history.location.search); + try { + const { data } = await CredentialsAPI.read( + mergeParams(params, { credential_type: credentialTypeId }) + ); + setCredentials(data.results); + setCount(data.count); + } catch (err) { + if (setError) { + setError(err); + } + } + })(); + }); return ( ( + dispatch({ type: 'SELECT_ITEM', item })} + deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + /> + )} /> + ); } @@ -65,4 +101,4 @@ CredentialLookup.defaultProps = { }; export { CredentialLookup as _CredentialLookup }; -export default withI18n()(CredentialLookup); +export default withI18n()(withRouter(CredentialLookup)); diff --git a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx index 8071c8b26f..1c551e14d7 100644 --- a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx @@ -7,8 +7,9 @@ import { FormGroup } from '@patternfly/react-core'; import { InstanceGroupsAPI } from '@api'; import { getQSConfig, parseQueryString } from '@util/qs'; import { FieldTooltip } from '@components/FormField'; -import Lookup from './NewLookup'; -import SelectList from './shared/SelectList'; +import Lookup from './Lookup'; +import OptionsList from './shared/OptionsList'; +import LookupErrorMessage from './shared/LookupErrorMessage'; const QS_CONFIG = getQSConfig('instance_groups', { page: 1, @@ -45,17 +46,12 @@ function InstanceGroupsLookup(props) { ( - ( + )} /> - {error ?
error {error.message}
: ''} + ); } diff --git a/awx/ui_next/src/components/Lookup/InventoryLookup.jsx b/awx/ui_next/src/components/Lookup/InventoryLookup.jsx index 494b4d3069..cd687b18c5 100644 --- a/awx/ui_next/src/components/Lookup/InventoryLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InventoryLookup.jsx @@ -1,5 +1,6 @@ -import React from 'react'; +import React, { useState, useEffect } from 'react'; import { string, func, bool } from 'prop-types'; +import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { FormGroup } from '@patternfly/react-core'; @@ -7,61 +8,93 @@ import { InventoriesAPI } from '@api'; import { Inventory } from '@types'; import Lookup from '@components/Lookup'; import { FieldTooltip } from '@components/FormField'; +import { getQSConfig, parseQueryString } from '@util/qs'; +import OptionsList from './shared/OptionsList'; +import LookupErrorMessage from './shared/LookupErrorMessage'; -const getInventories = async params => InventoriesAPI.read(params); +const QS_CONFIG = getQSConfig('inventory', { + page: 1, + page_size: 5, + order_by: 'name', +}); -class InventoryLookup extends React.Component { - render() { - const { - value, - tooltip, - onChange, - onBlur, - required, - isValid, - helperTextInvalid, - i18n, - } = this.props; +function InventoryLookup({ + value, + tooltip, + onChange, + onBlur, + required, + isValid, + helperTextInvalid, + i18n, + history, +}) { + const [inventories, setInventories] = useState([]); + const [count, setCount] = useState(0); + const [error, setError] = useState(null); - return ( - - {tooltip && } - - - ); - } + useEffect(() => { + (async () => { + const params = parseQueryString(QS_CONFIG, history.location.search); + try { + const { data } = await InventoriesAPI.read(params); + setInventories(data.results); + setCount(data.count); + } catch (err) { + setError(err); + } + })(); + }, [history.location]); + + return ( + + {tooltip && } + ( + dispatch({ type: 'SELECT_ITEM', item })} + deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + /> + )} + /> + + + ); } InventoryLookup.propTypes = { @@ -77,4 +110,4 @@ InventoryLookup.defaultProps = { required: false, }; -export default withI18n()(InventoryLookup); +export default withI18n()(withRouter(InventoryLookup)); diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index 0a9b7c473b..afb67b54c4 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -1,4 +1,4 @@ -import React, { Fragment } from 'react'; +import React, { Fragment, useReducer, useEffect } from 'react'; import { string, bool, @@ -20,7 +20,7 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import styled from 'styled-components'; -import SelectList from './shared/SelectList'; +import reducer, { initReducer } from './shared/reducer'; import { ChipGroup, Chip } from '../Chip'; import { QSConfig } from '@types'; @@ -48,198 +48,118 @@ const ChipHolder = styled.div` border-bottom-right-radius: 3px; `; -class Lookup extends React.Component { - constructor(props) { - super(props); +function Lookup(props) { + const { + id, + header, + onChange, + onBlur, + value, + multiple, + required, + qsConfig, + renderItemChip, + renderOptionsList, + history, + i18n, + } = props; - this.assertCorrectValueType(); - let selectedItems = []; - if (props.value) { - selectedItems = props.multiple ? [...props.value] : [props.value]; - } - this.state = { - isModalOpen: false, - selectedItems, - }; - this.handleModalToggle = this.handleModalToggle.bind(this); - this.addItem = this.addItem.bind(this); - this.removeItem = this.removeItem.bind(this); - this.saveModal = this.saveModal.bind(this); - this.clearQSParams = this.clearQSParams.bind(this); - } + const [state, dispatch] = useReducer( + reducer, + { value, multiple, required }, + initReducer + ); - assertCorrectValueType() { - const { multiple, value } = this.props; - if (!multiple && Array.isArray(value)) { - throw new Error( - 'Lookup value must not be an array unless `multiple` is set' - ); - } - if (multiple && !Array.isArray(value)) { - throw new Error('Lookup value must be an array if `multiple` is set'); - } - } + useEffect(() => { + dispatch({ type: 'SET_MULTIPLE', value: multiple }); + }, [multiple]); - removeItem(row) { - const { selectedItems } = this.state; - const { onToggleItem } = this.props; - if (onToggleItem) { - this.setState({ selectedItems: onToggleItem(selectedItems, row) }); - return; - } - this.setState({ - selectedItems: selectedItems.filter(item => item.id !== row.id), - }); - } + useEffect(() => { + dispatch({ type: 'SET_VALUE', value }); + }, [value]); - addItem(row) { - const { selectedItems } = this.state; - const { multiple, onToggleItem } = this.props; - if (onToggleItem) { - this.setState({ selectedItems: onToggleItem(selectedItems, row) }); - return; - } - const index = selectedItems.findIndex(item => item.id === row.id); - - if (!multiple) { - this.setState({ selectedItems: [row] }); - return; - } - if (index > -1) { - return; - } - this.setState({ selectedItems: [...selectedItems, row] }); - } - - // TODO: cleanup - handleModalToggle() { - const { isModalOpen } = this.state; - const { value, multiple } = this.props; - // Resets the selected items from parent state whenever modal is opened - // This handles the case where the user closes/cancels the modal and - // opens it again - if (!isModalOpen) { - let selectedItems = []; - if (value) { - selectedItems = multiple ? [...value] : [value]; - } - this.setState({ selectedItems }); - } else { - this.clearQSParams(); - } - this.setState(prevState => ({ - isModalOpen: !prevState.isModalOpen, - })); - } - - removeItemAndSave(item) { - const { value, onChange, multiple } = this.props; - if (multiple) { - onChange(value.filter(i => i.id !== item.id)); - } else if (value.id === item.id) { - onChange(null); - } - } - - saveModal() { - const { onChange, multiple } = this.props; - const { selectedItems } = this.state; - const value = multiple ? selectedItems : selectedItems[0] || null; - - this.handleModalToggle(); - onChange(value); - } - - clearQSParams() { - const { qsConfig, history } = this.props; + const clearQSParams = () => { const parts = history.location.search.replace(/^\?/, '').split('&'); const ns = qsConfig.namespace; const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); history.push(`${history.location.pathname}?${otherParts.join('&')}`); - } + }; - render() { - const { isModalOpen, selectedItems } = this.state; - const { - id, - lookupHeader, - value, - items, - count, - columns, - multiple, - name, - onBlur, - required, - qsConfig, - i18n, - } = this.props; - const header = lookupHeader || i18n._(t`Items`); - const canDelete = !required || (multiple && value.length > 1); - return ( - - - - - - - - {(multiple ? value : [value]).map(chip => ( - this.removeItemAndSave(chip)} - isReadOnly={!canDelete} - > - {chip.name} - - ))} - - - - - {i18n._(t`Select`)} - , - , - ]} + const save = () => { + const { selectedItems } = state; + const val = multiple ? selectedItems : selectedItems[0] || null; + onChange(val); + clearQSParams(); + dispatch({ type: 'CLOSE_MODAL' }); + }; + + const removeItem = item => { + if (multiple) { + onChange(value.filter(i => i.id !== item.id)); + } else { + onChange(null); + } + }; + + const closeModal = () => { + clearQSParams(); + dispatch({ type: 'CLOSE_MODAL' }); + }; + + const { isModalOpen, selectedItems } = state; + const canDelete = !required || (multiple && value.length > 1); + return ( + + + dispatch({ type: 'TOGGLE_MODAL' })} + variant={ButtonVariant.tertiary} > - this.setState({ selectedItems: newVal })} - options={items} - optionCount={count} - columns={columns} - multiple={multiple} - header={lookupHeader} - name={name} - qsConfig={qsConfig} - readOnly={!canDelete} - /> - - - ); - } + + + + + {(multiple ? value : [value]).map(item => + renderItemChip({ + item, + removeItem, + canDelete, + }) + )} + + + + + {i18n._(t`Select`)} + , + , + ]} + > + {renderOptionsList({ + state, + dispatch, + canDelete, + })} + + + ); } const Item = shape({ @@ -248,25 +168,33 @@ const Item = shape({ Lookup.propTypes = { id: string, - items: arrayOf(shape({})).isRequired, - count: number.isRequired, - // TODO: change to `header` - lookupHeader: string, - name: string, + header: string, onChange: func.isRequired, value: oneOfType([Item, arrayOf(Item)]), multiple: bool, required: bool, + onBlur: func, qsConfig: QSConfig.isRequired, + renderItemChip: func, + renderOptionsList: func.isRequired, }; Lookup.defaultProps = { id: 'lookup-search', - lookupHeader: null, - name: null, + header: null, value: null, multiple: false, required: false, + onBlur: () => {}, + renderItemChip: ({ item, removeItem, canDelete }) => ( + removeItem(item)} + isReadOnly={!canDelete} + > + {item.name} + + ), }; export { Lookup as _Lookup }; diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 20d5180e7c..db00354305 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -10,8 +10,8 @@ import { FieldTooltip } from '@components/FormField'; import { CredentialChip } from '@components/Chip'; import VerticalSeperator from '@components/VerticalSeparator'; import { getQSConfig, parseQueryString } from '@util/qs'; -import Lookup from './NewLookup'; -import SelectList from './shared/SelectList'; +import Lookup from './Lookup'; +import OptionsList from './shared/OptionsList'; const QS_CONFIG = getQSConfig('credentials', { page: 1, @@ -37,7 +37,6 @@ function MultiCredentialsLookup(props) { const [selectedType, setSelectedType] = useState(null); const [credentials, setCredentials] = useState([]); const [credentialsCount, setCredentialsCount] = useState(0); - const [isLoading, setIsLoading] = useState(false); useEffect(() => { (async () => { @@ -60,12 +59,10 @@ function MultiCredentialsLookup(props) { } try { const params = parseQueryString(QS_CONFIG, history.location.search); - setIsLoading(true); const { results, count } = await loadCredentials( params, selectedType.id ); - setIsLoading(false); setCredentials(results); setCredentialsCount(count); } catch (err) { @@ -74,7 +71,7 @@ function MultiCredentialsLookup(props) { })(); }, [selectedType]); - const isMultiple = selectedType && selectedType.value === 'Vault'; + const isMultiple = selectedType && selectedType.name === 'Vault'; const renderChip = ({ item, removeItem, canDelete }) => ( { + renderOptionsList={({ state, dispatch, canDelete }) => { return ( {credentialTypes && credentialTypes.length > 0 && ( @@ -107,7 +104,7 @@ function MultiCredentialsLookup(props) { id="multiCredentialsLookUp-select" label={i18n._(t`Selected Category`)} data={credentialTypes.map(type => ({ - id: type.id, + key: type.id, value: type.id, label: type.name, isDisabled: false, @@ -121,7 +118,7 @@ function MultiCredentialsLookup(props) { /> )} - - props.multiple && - ` - --pf-c-form-control--Height: 90px; - overflow-y: auto; - `} -`; - -const ChipHolder = styled.div` - --pf-c-form-control--BorderTopColor: var(--pf-global--BorderColor--200); - --pf-c-form-control--BorderRightColor: var(--pf-global--BorderColor--200); - border-top-right-radius: 3px; - border-bottom-right-radius: 3px; -`; - -function Lookup(props) { - const { - id, - header, - onChange, - onBlur, - value, - multiple, - required, - qsConfig, - renderItemChip, - renderSelectList, - history, - i18n, - } = props; - - const [state, dispatch] = useReducer( - reducer, - { value, multiple, required }, - initReducer - ); - - useEffect(() => { - dispatch({ type: 'SET_MULTIPLE', value: multiple }); - }, [multiple]); - - useEffect(() => { - dispatch({ type: 'SET_VALUE', value }); - }, [value]); - - const clearQSParams = () => { - const parts = history.location.search.replace(/^\?/, '').split('&'); - const ns = qsConfig.namespace; - const otherParts = parts.filter(param => !param.startsWith(`${ns}.`)); - history.push(`${history.location.pathname}?${otherParts.join('&')}`); - }; - - const save = () => { - const { selectedItems } = state; - const val = multiple ? selectedItems : selectedItems[0] || null; - onChange(val); - clearQSParams(); - dispatch({ type: 'CLOSE_MODAL' }); - }; - - const removeItem = item => { - if (multiple) { - onChange(value.filter(i => i.id !== item.id)); - } else { - onChange(null); - } - }; - - const closeModal = () => { - clearQSParams(); - dispatch({ type: 'CLOSE_MODAL' }); - }; - - const { isModalOpen, selectedItems } = state; - const canDelete = !required || (multiple && value.length > 1); - return ( - - - dispatch({ type: 'TOGGLE_MODAL' })} - variant={ButtonVariant.tertiary} - > - - - - - {(multiple ? value : [value]).map(item => - renderItemChip({ - item, - removeItem, - canDelete, - }) - )} - - - - - {i18n._(t`Select`)} - , - , - ]} - > - {renderSelectList({ - state, - dispatch, - canDelete, - })} - - - ); -} - -const Item = shape({ - id: number.isRequired, -}); - -Lookup.propTypes = { - id: string, - header: string, - onChange: func.isRequired, - value: oneOfType([Item, arrayOf(Item)]), - multiple: bool, - required: bool, - onBlur: func, - qsConfig: QSConfig.isRequired, - renderItemChip: func, - renderSelectList: func.isRequired, -}; - -Lookup.defaultProps = { - id: 'lookup-search', - header: null, - value: null, - multiple: false, - required: false, - onBlur: () => {}, - renderItemChip: ({ item, removeItem, canDelete }) => ( - removeItem(item)} - isReadOnly={!canDelete} - > - {item.name} - - ), -}; - -export { Lookup as _Lookup }; -export default withI18n()(withRouter(Lookup)); diff --git a/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx b/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx index 32c93f2588..17d98acd9e 100644 --- a/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx +++ b/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx @@ -1,13 +1,17 @@ -import React from 'react'; +import React, { useState, useEffect } from 'react'; +import { string, func, bool } from 'prop-types'; +import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { string, func, bool } from 'prop-types'; import { OrganizationsAPI } from '@api'; import { Organization } from '@types'; import { FormGroup } from '@patternfly/react-core'; -import Lookup from '@components/Lookup'; +import { getQSConfig, parseQueryString } from '@util/qs'; +import Lookup from './Lookup'; +import OptionsList from './shared/OptionsList'; +import LookupErrorMessage from './shared/LookupErrorMessage'; -const getOrganizations = async params => OrganizationsAPI.read(params); +const QS_CONFIG = getQSConfig('organizations', {}); function OrganizationLookup({ helperTextInvalid, @@ -17,7 +21,25 @@ function OrganizationLookup({ onChange, required, value, + history, }) { + const [organizations, setOrganizations] = useState([]); + const [count, setCount] = useState(0); + const [error, setError] = useState(null); + + useEffect(() => { + (async () => { + const params = parseQueryString(QS_CONFIG, history.location.search); + try { + const { data } = await OrganizationsAPI.read(params); + setOrganizations(data.results); + setCount(data.count); + } catch (err) { + setError(err); + } + })(); + }, [history.location]); + return ( ( + dispatch({ type: 'SELECT_ITEM', item })} + deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + /> + )} /> + ); } @@ -58,5 +94,5 @@ OrganizationLookup.defaultProps = { value: null, }; -export default withI18n()(OrganizationLookup); export { OrganizationLookup as _OrganizationLookup }; +export default withI18n()(withRouter(OrganizationLookup)); diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx index f83d30eb02..3203cadb30 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx @@ -1,59 +1,87 @@ -import React from 'react'; +import React, { useState, useEffect } from 'react'; import { string, func, bool } from 'prop-types'; +import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { FormGroup } from '@patternfly/react-core'; import { ProjectsAPI } from '@api'; import { Project } from '@types'; -import Lookup from '@components/Lookup'; import { FieldTooltip } from '@components/FormField'; +import { getQSConfig, parseQueryString } from '@util/qs'; +import Lookup from './Lookup'; +import OptionsList from './shared/OptionsList'; +import LookupErrorMessage from './shared/LookupErrorMessage'; -class ProjectLookup extends React.Component { - render() { - const { - helperTextInvalid, - i18n, - isValid, - onChange, - required, - tooltip, - value, - onBlur, - } = this.props; +const QS_CONFIG = getQSConfig('project', { + page: 1, + page_size: 5, + order_by: 'name', +}); - const loadProjects = async params => { - const response = await ProjectsAPI.read(params); - const { results, count } = response.data; - if (count === 1) { - onChange(results[0], 'project'); +function ProjectLookup({ + helperTextInvalid, + i18n, + isValid, + onChange, + required, + tooltip, + value, + onBlur, + history, +}) { + const [projects, setProjects] = useState([]); + const [count, setCount] = useState(0); + const [error, setError] = useState(null); + + useEffect(() => { + (async () => { + const params = parseQueryString(QS_CONFIG, history.location.search); + try { + const { data } = await ProjectsAPI.read(params); + setProjects(data.results); + setCount(data.count); + } catch (err) { + setError(err); } - return response; - }; + })(); + }, [history.location]); - return ( - - {tooltip && } - - - ); - } + return ( + + {tooltip && } + ( + dispatch({ type: 'SELECT_ITEM', item })} + deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} + /> + )} + /> + + + ); } ProjectLookup.propTypes = { @@ -75,4 +103,5 @@ ProjectLookup.defaultProps = { onBlur: () => {}, }; -export default withI18n()(ProjectLookup); +export { ProjectLookup as _ProjectLookup }; +export default withI18n()(withRouter(ProjectLookup)); diff --git a/awx/ui_next/src/components/Lookup/shared/LookupErrorMessage.jsx b/awx/ui_next/src/components/Lookup/shared/LookupErrorMessage.jsx new file mode 100644 index 0000000000..588fe18719 --- /dev/null +++ b/awx/ui_next/src/components/Lookup/shared/LookupErrorMessage.jsx @@ -0,0 +1,15 @@ +import React from 'react'; + +function LookupErrorMessage({ error }) { + if (!error) { + return null; + } + + return ( +
+ {error.message || 'An error occured'} +
+ ); +} + +export default LookupErrorMessage; diff --git a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx b/awx/ui_next/src/components/Lookup/shared/OptionsList.jsx similarity index 94% rename from awx/ui_next/src/components/Lookup/shared/SelectList.jsx rename to awx/ui_next/src/components/Lookup/shared/OptionsList.jsx index b9a2ec1f84..73e7b6b5ca 100644 --- a/awx/ui_next/src/components/Lookup/shared/SelectList.jsx +++ b/awx/ui_next/src/components/Lookup/shared/OptionsList.jsx @@ -16,7 +16,7 @@ import CheckboxListItem from '../../CheckboxListItem'; import DataListToolbar from '../../DataListToolbar'; import { QSConfig } from '@types'; -function SelectList({ +function OptionsList({ value, options, optionCount, @@ -71,7 +71,7 @@ function SelectList({ const Item = shape({ id: oneOfType([number, string]).isRequired, }); -SelectList.propTypes = { +OptionsList.propTypes = { value: arrayOf(Item).isRequired, options: arrayOf(Item).isRequired, optionCount: number.isRequired, @@ -82,9 +82,9 @@ SelectList.propTypes = { deselectItem: func.isRequired, renderItemChip: func, }; -SelectList.defaultProps = { +OptionsList.defaultProps = { multiple: false, renderItemChip: null, }; -export default withI18n()(SelectList); +export default withI18n()(OptionsList); From f8153393b11157cb15a2a87205feb0fe34455433 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 26 Nov 2019 15:01:13 -0800 Subject: [PATCH 08/19] fix minor lookup bugs --- .../components/Lookup/InstanceGroupsLookup.jsx | 15 +++++++++++++-- .../src/components/Lookup/InventoryLookup.jsx | 1 + .../src/components/Lookup/Lookup.test.jsx | 8 ++++---- .../components/Lookup/MultiCredentialsLookup.jsx | 2 +- .../src/components/Lookup/ProjectLookup.jsx | 2 +- .../src/components/Lookup/shared/OptionsList.jsx | 5 ++++- .../src/components/Lookup/shared/reducer.js | 16 ---------------- 7 files changed, 24 insertions(+), 25 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx index 1c551e14d7..20c2e0cf20 100644 --- a/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/InstanceGroupsLookup.jsx @@ -1,5 +1,5 @@ import React, { useState, useEffect } from 'react'; -import { arrayOf, string, func, object } from 'prop-types'; +import { arrayOf, string, func, object, bool } from 'prop-types'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -18,7 +18,15 @@ const QS_CONFIG = getQSConfig('instance_groups', { }); function InstanceGroupsLookup(props) { - const { value, onChange, tooltip, className, history, i18n } = props; + const { + value, + onChange, + tooltip, + className, + required, + history, + i18n, + } = props; const [instanceGroups, setInstanceGroups] = useState([]); const [count, setCount] = useState(0); const [error, setError] = useState(null); @@ -50,6 +58,7 @@ function InstanceGroupsLookup(props) { onChange={onChange} qsConfig={QS_CONFIG} multiple + required={required} renderOptionsList={({ state, dispatch, canDelete }) => ( ( ', () => { wrapper = mountWithContexts( ', () => { document.body.innerHTML = ''; wrapper = mountWithContexts( ', () => { <_Lookup multiple name="foo" - lookupHeader="Foo Bar" + header="Foo Bar" onLookupSave={() => {}} value={mockData} columns={mockColumns} @@ -369,7 +369,7 @@ describe('', () => { <_Lookup multiple name="foo" - lookupHeader="Foo Bar" + header="Foo Bar" onLookupSave={() => {}} value={mockData} columns={mockColumns} diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index db00354305..02764f3de3 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -86,7 +86,7 @@ function MultiCredentialsLookup(props) { {tooltip && } } ( Date: Tue, 3 Dec 2019 09:18:28 -0800 Subject: [PATCH 09/19] update MultiCredentialsLookup tests --- awx/ui_next/src/components/Lookup/Lookup.jsx | 3 +- .../Lookup/MultiCredentialsLookup.jsx | 6 +- .../Lookup/MultiCredentialsLookup.test.jsx | 179 ++++++++++++++---- 3 files changed, 145 insertions(+), 43 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index afb67b54c4..b431ce4ac2 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -30,6 +30,7 @@ const SearchButton = styled(Button)` var(--pf-global--BorderColor--200); } `; +SearchButton.displayName = 'SearchButton'; const InputGroup = styled(PFInputGroup)` ${props => @@ -120,7 +121,7 @@ function Lookup(props) { - + {(multiple ? value : [value]).map(item => renderItemChip({ item, diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 02764f3de3..2d1eff8965 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -32,7 +32,7 @@ async function loadCredentials(params, selectedCredentialTypeId) { } function MultiCredentialsLookup(props) { - const { history, tooltip, value, onChange, onError, i18n } = props; + const { tooltip, value, onChange, onError, history, i18n } = props; const [credentialTypes, setCredentialTypes] = useState([]); const [selectedType, setSelectedType] = useState(null); const [credentials, setCredentials] = useState([]); @@ -50,7 +50,7 @@ function MultiCredentialsLookup(props) { onError(err); } })(); - }, []); + }, [onError]); useEffect(() => { (async () => { @@ -69,7 +69,7 @@ function MultiCredentialsLookup(props) { onError(err); } })(); - }, [selectedType]); + }, [selectedType, history.location.search, onError]); const isMultiple = selectedType && selectedType.name === 'Vault'; const renderChip = ({ item, removeItem, canDelete }) => ( diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx index cc00396525..baaea96776 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx @@ -1,6 +1,7 @@ import React from 'react'; - -import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import { act } from 'react-dom/test-utils'; +import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; +import { sleep } from '@testUtils/testUtils'; import MultiCredentialsLookup from './MultiCredentialsLookup'; import { CredentialsAPI, CredentialTypesAPI } from '@api'; @@ -8,9 +9,6 @@ jest.mock('@api'); describe('', () => { let wrapper; - let lookup; - let credLookup; - let onChange; const credentials = [ { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, @@ -18,8 +16,9 @@ describe('', () => { { name: 'Gatsby', id: 21, kind: 'vault' }, { name: 'Gatsby', id: 8, kind: 'Machine' }, ]; + beforeEach(() => { - CredentialTypesAPI.read.mockResolvedValue({ + CredentialTypesAPI.read.mockResolvedValueOnce({ data: { results: [ { @@ -46,17 +45,6 @@ describe('', () => { count: 3, }, }); - onChange = jest.fn(); - wrapper = mountWithContexts( - {}} - credentials={credentials} - onChange={onChange} - tooltip="This is credentials look up" - /> - ); - lookup = wrapper.find('Lookup'); - credLookup = wrapper.find('MultiCredentialsLookup'); }); afterEach(() => { @@ -64,16 +52,40 @@ describe('', () => { wrapper.unmount(); }); - test('MultiCredentialsLookup renders properly', () => { + test('MultiCredentialsLookup renders properly', async () => { + const onChange = jest.fn(); + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); expect(wrapper.find('MultiCredentialsLookup')).toHaveLength(1); expect(CredentialTypesAPI.read).toHaveBeenCalled(); }); test('onChange is called when you click to remove a credential from input', async () => { - const chip = wrapper.find('PFChip').find({ isOverflowChip: false }); - const button = chip.at(1).find('ChipButton'); + const onChange = jest.fn(); + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); + const chip = wrapper.find('CredentialChip'); expect(chip).toHaveLength(4); - button.prop('onClick')(); + const button = chip.at(1).find('ChipButton'); + await act(async () => { + button.invoke('onClick')(); + }); expect(onChange).toBeCalledWith([ { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, { id: 21, kind: 'vault', name: 'Gatsby' }, @@ -81,33 +93,122 @@ describe('', () => { ]); }); - test('can change credential types', () => { - lookup.prop('selectCategory')({}, 'Vault'); - expect(credLookup.state('selectedCredentialType')).toEqual({ - id: 500, - key: 500, - kind: 'vault', - type: 'buzz', - value: 'Vault', - label: 'Vault', - isDisabled: false, + test('should change credential types', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} + onError={() => {}} + /> + ); }); - expect(CredentialsAPI.read).toHaveBeenCalled(); + const searchButton = await waitForElement(wrapper, 'SearchButton'); + await act(async () => { + searchButton.invoke('onClick')(); + }); + const select = await waitForElement(wrapper, 'AnsibleSelect'); + CredentialsAPI.read.mockResolvedValueOnce({ + data: { + results: [ + { id: 1, kind: 'cloud', name: 'New Cred', url: 'www.google.com' }, + ], + count: 1, + }, + }); + expect(CredentialsAPI.read).toHaveBeenCalledTimes(2); + await act(async () => { + select.invoke('onChange')({}, 500); + }); + wrapper.update(); + expect(CredentialsAPI.read).toHaveBeenCalledTimes(3); + expect(wrapper.find('OptionsList').prop('options')).toEqual([ + { id: 1, kind: 'cloud', name: 'New Cred', url: 'www.google.com' }, + ]); }); - test('Toggle credentials only adds 1 credential per credential type except vault(see below)', () => { - lookup.prop('onToggleItem')({ name: 'Party', id: 9, kind: 'Machine' }); + + test('should only add 1 credential per credential type except vault(see below)', async () => { + const onChange = jest.fn(); + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); + const searchButton = await waitForElement(wrapper, 'SearchButton'); + await act(async () => { + searchButton.invoke('onClick')(); + }); + wrapper.update(); + const optionsList = wrapper.find('OptionsList'); + expect(optionsList.prop('multiple')).toEqual(false); + act(() => { + optionsList.invoke('selectItem')({ + id: 5, + kind: 'Machine', + name: 'Cred 5', + url: 'www.google.com', + }); + }); + wrapper.update(); + act(() => { + wrapper.find('Button[variant="primary"]').invoke('onClick')(); + }); expect(onChange).toBeCalledWith([ { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' }, { id: 21, kind: 'vault', name: 'Gatsby' }, - { id: 9, kind: 'Machine', name: 'Party' }, + { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' }, ]); }); - test('Toggle credentials only adds 1 credential per credential type', () => { - lookup.prop('onToggleItem')({ name: 'Party', id: 22, kind: 'vault' }); + + test('should allow multiple vault credentials', async () => { + const onChange = jest.fn(); + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); + const searchButton = await waitForElement(wrapper, 'SearchButton'); + await act(async () => { + searchButton.invoke('onClick')(); + }); + wrapper.update(); + const typeSelect = wrapper.find('AnsibleSelect'); + act(() => { + typeSelect.invoke('onChange')({}, 500); + }); + wrapper.update(); + const optionsList = wrapper.find('OptionsList'); + expect(optionsList.prop('multiple')).toEqual(true); + act(() => { + optionsList.invoke('selectItem')({ + id: 5, + kind: 'Machine', + name: 'Cred 5', + url: 'www.google.com', + }); + }); + wrapper.update(); + act(() => { + wrapper.find('Button[variant="primary"]').invoke('onClick')(); + }); expect(onChange).toBeCalledWith([ - ...credentials, - { name: 'Party', id: 22, kind: 'vault' }, + { id: 1, kind: 'cloud', name: 'Foo', url: 'www.google.com' }, + { id: 2, kind: 'ssh', name: 'Alex', url: 'www.google.com' }, + { id: 21, kind: 'vault', name: 'Gatsby' }, + { id: 8, kind: 'Machine', name: 'Gatsby' }, + { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' }, ]); }); }); From 2e525f89220a9577f9de99693cbb251ba648caab Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 3 Dec 2019 11:02:10 -0800 Subject: [PATCH 10/19] update tests for CredentialLookup, OrgLookup, ProjectLookup --- .../components/Lookup/CredentialLookup.jsx | 3 +- .../Lookup/CredentialLookup.test.jsx | 64 +++++++++++++++++-- .../Lookup/MultiCredentialsLookup.test.jsx | 1 - .../components/Lookup/OrganizationLookup.jsx | 6 +- .../Lookup/OrganizationLookup.test.jsx | 29 ++++++--- .../src/components/Lookup/ProjectLookup.jsx | 3 + .../components/Lookup/ProjectLookup.test.jsx | 11 +++- 7 files changed, 96 insertions(+), 21 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx index 5de5b1c955..bea32e63de 100644 --- a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx @@ -46,7 +46,7 @@ function CredentialLookup({ } } })(); - }); + }, [credentialTypeId, history.location.search]); return ( ( { let wrapper; beforeEach(() => { - wrapper = mountWithContexts( - {}} /> - ); + CredentialsAPI.read.mockResolvedValueOnce({ + data: { + results: [ + { id: 1, kind: 'cloud', name: 'Cred 1', url: 'www.google.com' }, + { id: 2, kind: 'ssh', name: 'Cred 2', url: 'www.google.com' }, + { id: 3, kind: 'Ansible', name: 'Cred 3', url: 'www.google.com' }, + { id: 4, kind: 'Machine', name: 'Cred 4', url: 'www.google.com' }, + { id: 5, kind: 'Machine', name: 'Cred 5', url: 'www.google.com' }, + ], + count: 5, + }, + }); }); afterEach(() => { jest.clearAllMocks(); + wrapper.unmount(); }); - test('initially renders successfully', () => { + test('should render successfully', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); expect(wrapper.find('CredentialLookup')).toHaveLength(1); }); - test('should fetch credentials', () => { + + test('should fetch credentials', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); expect(CredentialsAPI.read).toHaveBeenCalledTimes(1); expect(CredentialsAPI.read).toHaveBeenCalledWith({ credential_type: 1, @@ -30,11 +60,31 @@ describe('CredentialLookup', () => { page_size: 5, }); }); - test('should display label', () => { + + test('should display label', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); const title = wrapper.find('FormGroup .pf-c-form__label-text'); expect(title.text()).toEqual('Foo'); }); - test('should define default value for function props', () => { + + test('should define default value for function props', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} + /> + ); + }); expect(_CredentialLookup.defaultProps.onBlur).toBeInstanceOf(Function); expect(_CredentialLookup.defaultProps.onBlur).not.toThrow(); }); diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx index baaea96776..fa73edad3a 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.test.jsx @@ -1,7 +1,6 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; -import { sleep } from '@testUtils/testUtils'; import MultiCredentialsLookup from './MultiCredentialsLookup'; import { CredentialsAPI, CredentialTypesAPI } from '@api'; diff --git a/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx b/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx index 17d98acd9e..9fd5c4bb88 100644 --- a/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx +++ b/awx/ui_next/src/components/Lookup/OrganizationLookup.jsx @@ -11,7 +11,11 @@ import Lookup from './Lookup'; import OptionsList from './shared/OptionsList'; import LookupErrorMessage from './shared/LookupErrorMessage'; -const QS_CONFIG = getQSConfig('organizations', {}); +const QS_CONFIG = getQSConfig('organizations', { + page: 1, + page_size: 5, + order_by: 'name', +}); function OrganizationLookup({ helperTextInvalid, diff --git a/awx/ui_next/src/components/Lookup/OrganizationLookup.test.jsx b/awx/ui_next/src/components/Lookup/OrganizationLookup.test.jsx index fef9a90281..1470537e29 100644 --- a/awx/ui_next/src/components/Lookup/OrganizationLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/OrganizationLookup.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import OrganizationLookup, { _OrganizationLookup } from './OrganizationLookup'; import { OrganizationsAPI } from '@api'; @@ -8,18 +9,22 @@ jest.mock('@api'); describe('OrganizationLookup', () => { let wrapper; - beforeEach(() => { - wrapper = mountWithContexts( {}} />); - }); - afterEach(() => { jest.clearAllMocks(); + wrapper.unmount(); }); - test('initially renders successfully', () => { + test('should render successfully', async () => { + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); expect(wrapper).toHaveLength(1); }); - test('should fetch organizations', () => { + + test('should fetch organizations', async () => { + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); expect(OrganizationsAPI.read).toHaveBeenCalledTimes(1); expect(OrganizationsAPI.read).toHaveBeenCalledWith({ order_by: 'name', @@ -27,11 +32,19 @@ describe('OrganizationLookup', () => { page_size: 5, }); }); - test('should display "Organization" label', () => { + + test('should display "Organization" label', async () => { + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); const title = wrapper.find('FormGroup .pf-c-form__label-text'); expect(title.text()).toEqual('Organization'); }); - test('should define default value for function props', () => { + + test('should define default value for function props', async () => { + await act(async () => { + wrapper = mountWithContexts( {}} />); + }); expect(_OrganizationLookup.defaultProps.onBlur).toBeInstanceOf(Function); expect(_OrganizationLookup.defaultProps.onBlur).not.toThrow(); }); diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx index bf9d72face..a160ecc527 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx @@ -40,6 +40,9 @@ function ProjectLookup({ const { data } = await ProjectsAPI.read(params); setProjects(data.results); setCount(data.count); + if (data.count === 1) { + onChange(data.results[0]); + } } catch (err) { setError(err); } diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx index 00fd2ad4bf..743067745e 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; import { ProjectsAPI } from '@api'; @@ -15,9 +16,11 @@ describe('', () => { }, }); const onChange = jest.fn(); - mountWithContexts(); + await act(async () => { + mountWithContexts(); + }); await sleep(0); - expect(onChange).toHaveBeenCalledWith({ id: 1 }, 'project'); + expect(onChange).toHaveBeenCalledWith({ id: 1 }); }); test('should not auto-select project when multiple available', async () => { @@ -28,7 +31,9 @@ describe('', () => { }, }); const onChange = jest.fn(); - mountWithContexts(); + await act(async () => { + mountWithContexts(); + }); await sleep(0); expect(onChange).not.toHaveBeenCalled(); }); From 9ab9c6961bec4054642b35806309feeaafb497c8 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 3 Dec 2019 15:41:03 -0800 Subject: [PATCH 11/19] update Lookup tests, add OptionsList tests --- awx/ui_next/src/components/Lookup/Lookup.jsx | 8 +- .../src/components/Lookup/Lookup.test.jsx | 402 ++++-------------- .../src/components/Lookup/ProjectLookup.jsx | 2 +- .../components/Lookup/shared/OptionsList.jsx | 2 + .../Lookup/shared/OptionsList.test.jsx | 53 +++ 5 files changed, 148 insertions(+), 319 deletions(-) create mode 100644 awx/ui_next/src/components/Lookup/shared/OptionsList.test.jsx diff --git a/awx/ui_next/src/components/Lookup/Lookup.jsx b/awx/ui_next/src/components/Lookup/Lookup.jsx index b431ce4ac2..500c4ce986 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.jsx @@ -109,6 +109,12 @@ function Lookup(props) { const { isModalOpen, selectedItems } = state; const canDelete = !required || (multiple && value.length > 1); + let items = []; + if (multiple) { + items = value; + } else if (value) { + items.push(value); + } return ( @@ -122,7 +128,7 @@ function Lookup(props) { - {(multiple ? value : [value]).map(item => + {items.map(item => renderItemChip({ item, removeItem, diff --git a/awx/ui_next/src/components/Lookup/Lookup.test.jsx b/awx/ui_next/src/components/Lookup/Lookup.test.jsx index 8f784db8ec..143ba5a709 100644 --- a/awx/ui_next/src/components/Lookup/Lookup.test.jsx +++ b/awx/ui_next/src/components/Lookup/Lookup.test.jsx @@ -1,11 +1,9 @@ /* eslint-disable react/jsx-pascal-case */ import React from 'react'; -import { createMemoryHistory } from 'history'; +import { act } from 'react-dom/test-utils'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; -import Lookup, { _Lookup } from './Lookup'; - -let mockData = [{ name: 'foo', id: 1, isChecked: false }]; -const mockColumns = [{ name: 'Name', key: 'name', isSortable: true }]; +import { getQSConfig } from '@util/qs'; +import Lookup from './Lookup'; /** * Check that an element is present on the document body @@ -44,348 +42,118 @@ async function checkInputTagValues(wrapper, expected) { }); } -/** - * Check lookup modal list for expected values - * @param {wrapper} enzyme wrapper instance - * @param {expected} array of [selected, text] pairs describing - * the expected visible state of the modal data list - */ -async function checkModalListValues(wrapper, expected) { - // fail if modal isn't actually visible - checkRootElementPresent('body div[role="dialog"]'); - // check list item values - const rows = await waitForElement( - wrapper, - 'DataListItemRow', - el => el.length === expected.length - ); - expect(rows).toHaveLength(expected.length); - rows.forEach((el, index) => { - const [expectedChecked, expectedText] = expected[index]; - expect(expectedText).toEqual(el.text()); - expect(expectedChecked).toEqual(el.find('input').props().checked); - }); -} - -/** - * Check lookup modal selection tags for expected values - * @param {wrapper} enzyme wrapper instance - * @param {expected} array of expected tag values - */ -async function checkModalTagValues(wrapper, expected) { - // fail if modal isn't actually visible - checkRootElementPresent('body div[role="dialog"]'); - // check modal chip values - const chips = await waitForElement( - wrapper, - 'Modal Chip span', - el => el.length === expected.length - ); - expect(chips).toHaveLength(expected.length); - chips.forEach((el, index) => { - expect(el.text()).toEqual(expected[index]); - }); -} - -describe('', () => { - let wrapper; - let onChange; - - beforeEach(() => { - const mockSelected = [{ name: 'foo', id: 1, url: '/api/v2/item/1' }]; - onChange = jest.fn(); - document.body.innerHTML = ''; - wrapper = mountWithContexts( - ({ - data: { - count: 2, - results: [ - ...mockSelected, - { name: 'bar', id: 2, url: '/api/v2/item/2' }, - ], - }, - })} - columns={mockColumns} - sortedColumnKey="name" - /> - ); - }); - - test('Initially renders succesfully', () => { - expect(wrapper.find('Lookup')).toHaveLength(1); - }); - - test('Expected items are shown', async done => { - expect(wrapper.find('Lookup')).toHaveLength(1); - await checkInputTagValues(wrapper, ['foo']); - done(); - }); - - test('Open and close modal', async done => { - checkRootElementNotPresent('body div[role="dialog"]'); - wrapper.find('button[aria-label="Search"]').simulate('click'); - checkRootElementPresent('body div[role="dialog"]'); - // This check couldn't pass unless api response was formatted properly - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - wrapper.find('Modal button[aria-label="Close"]').simulate('click'); - checkRootElementNotPresent('body div[role="dialog"]'); - wrapper.find('button[aria-label="Search"]').simulate('click'); - checkRootElementPresent('body div[role="dialog"]'); - wrapper - .find('Modal button') - .findWhere(e => e.text() === 'Cancel') - .first() - .simulate('click'); - checkRootElementNotPresent('body div[role="dialog"]'); - done(); - }); - - test('Add item with checkbox then save', async done => { - wrapper.find('button[aria-label="Search"]').simulate('click'); - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - wrapper - .find('DataListItemRow') - .findWhere(el => el.text() === 'bar') - .find('input[type="checkbox"]') - .simulate('change'); - await checkModalListValues(wrapper, [[true, 'foo'], [true, 'bar']]); - wrapper - .find('Modal button') - .findWhere(e => e.text() === 'Save') - .first() - .simulate('click'); - expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange.mock.calls[0][0].map(({ name }) => name)).toEqual([ - 'foo', - 'bar', - ]); - done(); - }); - - test('Add item with checkbox then cancel', async done => { - wrapper.find('button[aria-label="Search"]').simulate('click'); - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - wrapper - .find('DataListItemRow') - .findWhere(el => el.text() === 'bar') - .find('input[type="checkbox"]') - .simulate('change'); - await checkModalListValues(wrapper, [[true, 'foo'], [true, 'bar']]); - wrapper - .find('Modal button') - .findWhere(e => e.text() === 'Cancel') - .first() - .simulate('click'); - expect(onChange).toHaveBeenCalledTimes(0); - await checkInputTagValues(wrapper, ['foo']); - done(); - }); - - test('Remove item with checkbox', async done => { - wrapper.find('button[aria-label="Search"]').simulate('click'); - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - await checkModalTagValues(wrapper, ['foo']); - wrapper - .find('DataListItemRow') - .findWhere(el => el.text() === 'foo') - .find('input[type="checkbox"]') - .simulate('change'); - await checkModalListValues(wrapper, [[false, 'foo'], [false, 'bar']]); - await checkModalTagValues(wrapper, []); - done(); - }); - - test('Remove item with selected icon button', async done => { - wrapper.find('button[aria-label="Search"]').simulate('click'); - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - await checkModalTagValues(wrapper, ['foo']); - wrapper - .find('Modal Chip') - .findWhere(el => el.text() === 'foo') - .first() - .find('button') - .simulate('click'); - await checkModalListValues(wrapper, [[false, 'foo'], [false, 'bar']]); - await checkModalTagValues(wrapper, []); - done(); - }); - - test('Remove item with input group button', async done => { - await checkInputTagValues(wrapper, ['foo']); - wrapper - .find('Lookup InputGroup Chip') - .findWhere(el => el.text() === 'foo') - .first() - .find('button') - .simulate('click'); - expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenCalledWith([], 'foobar'); - done(); - }); -}); +const QS_CONFIG = getQSConfig('test', {}); +const TestList = () =>
; describe('', () => { let wrapper; let onChange; + async function mountWrapper() { + const mockSelected = [{ name: 'foo', id: 1, url: '/api/v2/item/1' }]; + await act(async () => { + wrapper = mountWithContexts( + ( + + )} + /> + ); + }); + return wrapper; + } + beforeEach(() => { - const mockSelected = { name: 'foo', id: 1, url: '/api/v2/item/1' }; onChange = jest.fn(); document.body.innerHTML = ''; - wrapper = mountWithContexts( - ({ - data: { - count: 2, - results: [ - mockSelected, - { name: 'bar', id: 2, url: '/api/v2/item/2' }, - ], - }, - })} - columns={mockColumns} - sortedColumnKey="name" - /> - ); }); - test('Initially renders succesfully', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('should render succesfully', async () => { + wrapper = await mountWrapper(); expect(wrapper.find('Lookup')).toHaveLength(1); }); - test('Expected items are shown', async done => { + test('should show selected items', async () => { + wrapper = await mountWrapper(); expect(wrapper.find('Lookup')).toHaveLength(1); await checkInputTagValues(wrapper, ['foo']); - done(); }); - test('Open and close modal', async done => { - checkRootElementNotPresent('body div[role="dialog"]'); - wrapper.find('button[aria-label="Search"]').simulate('click'); - checkRootElementPresent('body div[role="dialog"]'); - // This check couldn't pass unless api response was formatted properly - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - wrapper.find('Modal button[aria-label="Close"]').simulate('click'); + test('should open and close modal', async () => { + wrapper = await mountWrapper(); checkRootElementNotPresent('body div[role="dialog"]'); wrapper.find('button[aria-label="Search"]').simulate('click'); checkRootElementPresent('body div[role="dialog"]'); + const list = wrapper.find('TestList'); + expect(list).toHaveLength(1); + expect(list.prop('state')).toEqual({ + selectedItems: [{ id: 1, name: 'foo', url: '/api/v2/item/1' }], + value: [{ id: 1, name: 'foo', url: '/api/v2/item/1' }], + multiple: true, + isModalOpen: true, + required: false, + }); + expect(list.prop('dispatch')).toBeTruthy(); + expect(list.prop('canDelete')).toEqual(true); wrapper .find('Modal button') .findWhere(e => e.text() === 'Cancel') .first() .simulate('click'); checkRootElementNotPresent('body div[role="dialog"]'); - done(); }); - test('Change selected item with radio control then save', async done => { - wrapper.find('button[aria-label="Search"]').simulate('click'); - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - await checkModalTagValues(wrapper, ['foo']); + test('should remove item when X button clicked', async () => { + wrapper = await mountWrapper(); + await checkInputTagValues(wrapper, ['foo']); wrapper - .find('DataListItemRow') - .findWhere(el => el.text() === 'bar') - .find('input[type="radio"]') - .simulate('change'); - await checkModalListValues(wrapper, [[false, 'foo'], [true, 'bar']]); - await checkModalTagValues(wrapper, ['bar']); - wrapper - .find('Modal button') - .findWhere(e => e.text() === 'Save') + .find('Lookup InputGroup Chip') + .findWhere(el => el.text() === 'foo') .first() - .simulate('click'); - expect(onChange).toHaveBeenCalledTimes(1); - const [[{ name }]] = onChange.mock.calls; - expect(name).toEqual('bar'); - done(); - }); - - test('Change selected item with checkbox then cancel', async done => { - wrapper.find('button[aria-label="Search"]').simulate('click'); - await checkModalListValues(wrapper, [[true, 'foo'], [false, 'bar']]); - await checkModalTagValues(wrapper, ['foo']); - wrapper - .find('DataListItemRow') - .findWhere(el => el.text() === 'bar') - .find('input[type="radio"]') - .simulate('change'); - await checkModalListValues(wrapper, [[false, 'foo'], [true, 'bar']]); - await checkModalTagValues(wrapper, ['bar']); - wrapper - .find('Modal button') - .findWhere(e => e.text() === 'Cancel') - .first() - .simulate('click'); - expect(onChange).toHaveBeenCalledTimes(0); - done(); - }); - - test('should re-fetch data when URL params change', async done => { - mockData = [{ name: 'foo', id: 1, isChecked: false }]; - const history = createMemoryHistory({ - initialEntries: ['/organizations/add'], - }); - const getItems = jest.fn(); - const LookupWrapper = mountWithContexts( - <_Lookup - multiple - name="foo" - header="Foo Bar" - onLookupSave={() => {}} - value={mockData} - columns={mockColumns} - sortedColumnKey="name" - getItems={getItems} - location={{ history }} - i18n={{ _: val => val.toString() }} - /> - ); - expect(getItems).toHaveBeenCalledTimes(1); - history.push('organizations/add?page=2'); - LookupWrapper.setProps({ - location: { history }, - }); - LookupWrapper.update(); - expect(getItems).toHaveBeenCalledTimes(2); - done(); - }); - - test('should clear its query params when closed', async () => { - mockData = [{ name: 'foo', id: 1, isChecked: false }]; - const history = createMemoryHistory({ - initialEntries: ['/organizations/add?inventory.name=foo&bar=baz'], - }); - wrapper = mountWithContexts( - <_Lookup - multiple - name="foo" - header="Foo Bar" - onLookupSave={() => {}} - value={mockData} - columns={mockColumns} - sortedColumnKey="name" - getItems={() => {}} - location={{ history }} - history={history} - qsNamespace="inventory" - i18n={{ _: val => val.toString() }} - /> - ); - wrapper - .find('InputGroup Button') - .at(0) .invoke('onClick')(); - wrapper.find('Modal').invoke('onClose')(); - expect(history.location.search).toEqual('?bar=baz'); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith([]); + }); + + test('should pass canDelete false if required single select', async () => { + await act(async () => { + const mockSelected = { name: 'foo', id: 1, url: '/api/v2/item/1' }; + wrapper = mountWithContexts( + ( + + )} + /> + ); + }); + wrapper.find('button[aria-label="Search"]').simulate('click'); + const list = wrapper.find('TestList'); + expect(list.prop('canDelete')).toEqual(false); }); }); diff --git a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx index a160ecc527..983a214661 100644 --- a/awx/ui_next/src/components/Lookup/ProjectLookup.jsx +++ b/awx/ui_next/src/components/Lookup/ProjectLookup.jsx @@ -47,7 +47,7 @@ function ProjectLookup({ setError(err); } })(); - }, [history.location]); + }, [onChange, history.location]); return ( ', () => { + it('should display list of options', () => { + const options = [ + { id: 1, name: 'foo', url: '/item/1' }, + { id: 2, name: 'bar', url: '/item/2' }, + { id: 3, name: 'baz', url: '/item/3' }, + ]; + const wrapper = mountWithContexts( + {}} + deselectItem={() => {}} + name="Item" + /> + ); + expect(wrapper.find('PaginatedDataList').prop('items')).toEqual(options); + expect(wrapper.find('SelectedList')).toHaveLength(0); + }); + + it('should render selected list', () => { + const options = [ + { id: 1, name: 'foo', url: '/item/1' }, + { id: 2, name: 'bar', url: '/item/2' }, + { id: 3, name: 'baz', url: '/item/3' }, + ]; + const wrapper = mountWithContexts( + {}} + deselectItem={() => {}} + name="Item" + /> + ); + const list = wrapper.find('SelectedList'); + expect(list).toHaveLength(1); + expect(list.prop('selected')).toEqual([options[1]]); + }); +}); From 569b5bc53364cd9b427fbeb17c0907c77633fb84 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 3 Dec 2019 16:28:52 -0800 Subject: [PATCH 12/19] clean up multiple test 'act()' warnings --- .../CheckboxListItem.test.jsx | 1 + .../components/Lookup/shared/OptionsList.jsx | 2 +- .../Project/ProjectAdd/ProjectAdd.test.jsx | 28 ++++++++----- .../Project/shared/ProjectForm.test.jsx | 42 ++++++++++--------- .../src/screens/Team/TeamAdd/TeamAdd.test.jsx | 30 +++++++++---- .../screens/Team/TeamEdit/TeamEdit.test.jsx | 13 ++++-- .../JobTemplateAdd/JobTemplateAdd.test.jsx | 24 ++++++----- .../Template/shared/JobTemplateForm.jsx | 11 +++-- .../screens/User/UserList/UserList.test.jsx | 4 +- 9 files changed, 95 insertions(+), 60 deletions(-) diff --git a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.test.jsx b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.test.jsx index a28003b71a..3e61d9c980 100644 --- a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.test.jsx +++ b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.test.jsx @@ -12,6 +12,7 @@ describe('CheckboxListItem', () => { label="Buzz" isSelected={false} onSelect={() => {}} + onDeselect={() => {}} /> ); expect(wrapper).toHaveLength(1); diff --git a/awx/ui_next/src/components/Lookup/shared/OptionsList.jsx b/awx/ui_next/src/components/Lookup/shared/OptionsList.jsx index e988eb5656..77b4611c61 100644 --- a/awx/ui_next/src/components/Lookup/shared/OptionsList.jsx +++ b/awx/ui_next/src/components/Lookup/shared/OptionsList.jsx @@ -72,8 +72,8 @@ function OptionsList({ const Item = shape({ id: oneOfType([number, string]).isRequired, - url: string.isRequired, name: string.isRequired, + url: string, }); OptionsList.propTypes = { value: arrayOf(Item).isRequired, diff --git a/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx b/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx index 554ebf83ef..c4d5bc16f1 100644 --- a/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx +++ b/awx/ui_next/src/screens/Project/ProjectAdd/ProjectAdd.test.jsx @@ -98,17 +98,19 @@ describe('', () => { }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); const formik = wrapper.find('Formik').instance(); - const changeState = new Promise(resolve => { - formik.setState( - { - values: { - ...projectData, + await act(async () => { + const changeState = new Promise(resolve => { + formik.setState( + { + values: { + ...projectData, + }, }, - }, - () => resolve() - ); + () => resolve() + ); + }); + await changeState; }); - await changeState; await act(async () => { wrapper.find('form').simulate('submit'); }); @@ -146,7 +148,9 @@ describe('', () => { context: { router: { history } }, }).find('ProjectAdd CardHeader'); }); - wrapper.find('CardCloseButton').simulate('click'); + await act(async () => { + wrapper.find('CardCloseButton').simulate('click'); + }); expect(history.location.pathname).toEqual('/projects'); }); @@ -158,7 +162,9 @@ describe('', () => { }); }); await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); - wrapper.find('ProjectAdd button[aria-label="Cancel"]').simulate('click'); + await act(async () => { + wrapper.find('ProjectAdd button[aria-label="Cancel"]').simulate('click'); + }); expect(history.location.pathname).toEqual('/projects'); }); }); diff --git a/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx b/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx index 584287444e..d4b901c47c 100644 --- a/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx +++ b/awx/ui_next/src/screens/Project/shared/ProjectForm.test.jsx @@ -131,17 +131,19 @@ describe('', () => { }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); const formik = wrapper.find('Formik').instance(); - const changeState = new Promise(resolve => { - formik.setState( - { - values: { - ...mockData, + await act(async () => { + const changeState = new Promise(resolve => { + formik.setState( + { + values: { + ...mockData, + }, }, - }, - () => resolve() - ); + () => resolve() + ); + }); + await changeState; }); - await changeState; wrapper.update(); expect(wrapper.find('FormGroup[label="SCM URL"]').length).toBe(1); expect( @@ -191,18 +193,20 @@ describe('', () => { }); await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); const formik = wrapper.find('Formik').instance(); - const changeState = new Promise(resolve => { - formik.setState( - { - values: { - ...mockData, - scm_type: 'insights', + await act(async () => { + const changeState = new Promise(resolve => { + formik.setState( + { + values: { + ...mockData, + scm_type: 'insights', + }, }, - }, - () => resolve() - ); + () => resolve() + ); + }); + await changeState; }); - await changeState; wrapper.update(); expect(wrapper.find('FormGroup[label="Insights Credential"]').length).toBe( 1 diff --git a/awx/ui_next/src/screens/Team/TeamAdd/TeamAdd.test.jsx b/awx/ui_next/src/screens/Team/TeamAdd/TeamAdd.test.jsx index 20eb762710..02225fa733 100644 --- a/awx/ui_next/src/screens/Team/TeamAdd/TeamAdd.test.jsx +++ b/awx/ui_next/src/screens/Team/TeamAdd/TeamAdd.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import TeamAdd from './TeamAdd'; @@ -7,32 +8,38 @@ import { TeamsAPI } from '@api'; jest.mock('@api'); describe('', () => { - test('handleSubmit should post to api', () => { + test('handleSubmit should post to api', async () => { const wrapper = mountWithContexts(); const updatedTeamData = { name: 'new name', description: 'new description', organization: 1, }; - wrapper.find('TeamForm').prop('handleSubmit')(updatedTeamData); + await act(async () => { + wrapper.find('TeamForm').invoke('handleSubmit')(updatedTeamData); + }); expect(TeamsAPI.create).toHaveBeenCalledWith(updatedTeamData); }); - test('should navigate to teams list when cancel is clicked', () => { + test('should navigate to teams list when cancel is clicked', async () => { const history = createMemoryHistory({}); const wrapper = mountWithContexts(, { context: { router: { history } }, }); - wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + await act(async () => { + wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); + }); expect(history.location.pathname).toEqual('/teams'); }); - test('should navigate to teams list when close (x) is clicked', () => { + test('should navigate to teams list when close (x) is clicked', async () => { const history = createMemoryHistory({}); const wrapper = mountWithContexts(, { context: { router: { history } }, }); - wrapper.find('button[aria-label="Close"]').prop('onClick')(); + await act(async () => { + wrapper.find('button[aria-label="Close"]').invoke('onClick')(); + }); expect(history.location.pathname).toEqual('/teams'); }); @@ -55,11 +62,16 @@ describe('', () => { }, }, }); - const wrapper = mountWithContexts(, { - context: { router: { history } }, + let wrapper; + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); }); await waitForElement(wrapper, 'button[aria-label="Save"]'); - await wrapper.find('TeamForm').prop('handleSubmit')(teamData); + await act(async () => { + await wrapper.find('TeamForm').invoke('handleSubmit')(teamData); + }); expect(history.location.pathname).toEqual('/teams/5'); }); }); diff --git a/awx/ui_next/src/screens/Team/TeamEdit/TeamEdit.test.jsx b/awx/ui_next/src/screens/Team/TeamEdit/TeamEdit.test.jsx index 7ea335361a..cb410405b1 100644 --- a/awx/ui_next/src/screens/Team/TeamEdit/TeamEdit.test.jsx +++ b/awx/ui_next/src/screens/Team/TeamEdit/TeamEdit.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { TeamsAPI } from '@api'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; @@ -19,25 +20,29 @@ describe('', () => { }, }; - test('handleSubmit should call api update', () => { + test('handleSubmit should call api update', async () => { const wrapper = mountWithContexts(); const updatedTeamData = { name: 'new name', description: 'new description', }; - wrapper.find('TeamForm').prop('handleSubmit')(updatedTeamData); + await act(async () => { + wrapper.find('TeamForm').invoke('handleSubmit')(updatedTeamData); + }); expect(TeamsAPI.update).toHaveBeenCalledWith(1, updatedTeamData); }); - test('should navigate to team detail when cancel is clicked', () => { + test('should navigate to team detail when cancel is clicked', async () => { const history = createMemoryHistory({}); const wrapper = mountWithContexts(, { context: { router: { history } }, }); - wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + await act(async () => { + wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); + }); expect(history.location.pathname).toEqual('/teams/1/details'); }); 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 0539c657fe..bedb63e678 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateAdd/JobTemplateAdd.test.jsx @@ -101,19 +101,21 @@ describe('', () => { }); await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); const formik = wrapper.find('Formik').instance(); - const changeState = new Promise(resolve => { - formik.setState( - { - values: { - ...jobTemplateData, - labels: [], - instanceGroups: [], + await act(async () => { + const changeState = new Promise(resolve => { + formik.setState( + { + values: { + ...jobTemplateData, + labels: [], + instanceGroups: [], + }, }, - }, - () => resolve() - ); + () => resolve() + ); + }); + await changeState; }); - await changeState; wrapper.find('form').simulate('submit'); await sleep(1); expect(JobTemplatesAPI.create).toHaveBeenCalledWith(jobTemplateData); diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 67e94ced20..2f5ab24c64 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -79,6 +79,7 @@ class JobTemplateForm extends Component { }; this.handleProjectValidation = this.handleProjectValidation.bind(this); this.loadRelatedInstanceGroups = this.loadRelatedInstanceGroups.bind(this); + this.setContentError = this.setContentError.bind(this); } componentDidMount() { @@ -119,6 +120,10 @@ class JobTemplateForm extends Component { }; } + setContentError(contentError) { + this.setState({ contentError }); + } + render() { const { contentError, @@ -285,7 +290,7 @@ class JobTemplateForm extends Component { form={form} field={field} onBlur={() => form.setFieldTouched('playbook')} - onError={err => this.setState({ contentError: err })} + onError={this.setContentError} /> ); @@ -305,7 +310,7 @@ class JobTemplateForm extends Component { setFieldValue('labels', labels)} - onError={err => this.setState({ contentError: err })} + onError={this.setContentError} /> )} @@ -321,7 +326,7 @@ class JobTemplateForm extends Component { onChange={newCredentials => setFieldValue('credentials', newCredentials) } - onError={err => this.setState({ contentError: err })} + onError={this.setContentError} tooltip={i18n._( t`Select credentials that allow Tower to access the nodes this job will be ran against. You can only select one credential of each type. For machine credentials (SSH), checking "Prompt on launch" without selecting credentials will require you to select a machine credential at run time. If you select credentials and check "Prompt on launch", the selected credential(s) become the defaults that can be updated at run time.` )} diff --git a/awx/ui_next/src/screens/User/UserList/UserList.test.jsx b/awx/ui_next/src/screens/User/UserList/UserList.test.jsx index cc54d9d0f5..a727cdae81 100644 --- a/awx/ui_next/src/screens/User/UserList/UserList.test.jsx +++ b/awx/ui_next/src/screens/User/UserList/UserList.test.jsx @@ -214,7 +214,7 @@ describe('UsersList with full permissions', () => { ); }); - test('api is called to delete users for each selected user.', () => { + test('api is called to delete users for each selected user.', async () => { UsersAPI.destroy = jest.fn(); wrapper.find('UsersList').setState({ users: mockUsers, @@ -223,7 +223,7 @@ describe('UsersList with full permissions', () => { isModalOpen: true, selected: mockUsers, }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); + await wrapper.find('ToolbarDeleteButton').prop('onDelete')(); expect(UsersAPI.destroy).toHaveBeenCalledTimes(2); }); From 75b7d74f915fffb670da8117a839e624e35669ed Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 4 Dec 2019 08:48:49 -0800 Subject: [PATCH 13/19] Lookup tweaks/bug fixes --- .../src/components/Lookup/CredentialLookup.jsx | 2 +- .../Lookup/MultiCredentialsLookup.jsx | 6 ++---- .../Lookup/shared/LookupErrorMessage.jsx | 8 +++++--- .../src/components/Lookup/shared/reducer.js | 10 +++++++--- .../components/Lookup/shared/reducer.test.js | 18 ++++++++++++++++++ 5 files changed, 33 insertions(+), 11 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx index bea32e63de..6b61b3c486 100644 --- a/awx/ui_next/src/components/Lookup/CredentialLookup.jsx +++ b/awx/ui_next/src/components/Lookup/CredentialLookup.jsx @@ -71,7 +71,7 @@ function CredentialLookup({ optionCount={count} header={label} qsConfig={QS_CONFIG} - readOnly={canDelete} + readOnly={!canDelete} selectItem={item => dispatch({ type: 'SELECT_ITEM', item })} deselectItem={item => dispatch({ type: 'DESELECT_ITEM', item })} /> diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 2d1eff8965..1a020cab26 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -43,9 +43,7 @@ function MultiCredentialsLookup(props) { try { const types = await loadCredentialTypes(); setCredentialTypes(types); - setSelectedType( - types.find(type => type.name === 'Machine') || types[0] - ); + setSelectedType(types.find(type => type.kind === 'ssh') || types[0]); } catch (err) { onError(err); } @@ -71,7 +69,7 @@ function MultiCredentialsLookup(props) { })(); }, [selectedType, history.location.search, onError]); - const isMultiple = selectedType && selectedType.name === 'Vault'; + const isMultiple = selectedType && selectedType.kind === 'vault'; const renderChip = ({ item, removeItem, canDelete }) => ( - {error.message || 'An error occured'} + {error.message || i18n._(t`An error occured`)}
); } -export default LookupErrorMessage; +export default withI18n()(LookupErrorMessage); diff --git a/awx/ui_next/src/components/Lookup/shared/reducer.js b/awx/ui_next/src/components/Lookup/shared/reducer.js index bf97284f88..315f652846 100644 --- a/awx/ui_next/src/components/Lookup/shared/reducer.js +++ b/awx/ui_next/src/components/Lookup/shared/reducer.js @@ -1,5 +1,3 @@ -// import { useReducer, useEffect } from 'react'; - export default function reducer(state, action) { switch (action.type) { case 'SELECT_ITEM': @@ -51,10 +49,16 @@ function toggleModal(state) { if (isModalOpen) { return closeModal(state); } + let selectedItems = []; + if (multiple) { + selectedItems = [...value]; + } else if (value) { + selectedItems.push(value); + } return { ...state, isModalOpen: !isModalOpen, - selectedItems: multiple ? [...value] : [value], + selectedItems, }; } diff --git a/awx/ui_next/src/components/Lookup/shared/reducer.test.js b/awx/ui_next/src/components/Lookup/shared/reducer.test.js index 22bf9da106..62c963cbfb 100644 --- a/awx/ui_next/src/components/Lookup/shared/reducer.test.js +++ b/awx/ui_next/src/components/Lookup/shared/reducer.test.js @@ -129,6 +129,24 @@ describe('Lookup reducer', () => { }); }); + it('should set null value to empty array', () => { + const state = { + isModalOpen: false, + selectedItems: [{ id: 1 }], + value: null, + multiple: false, + }; + const result = reducer(state, { + type: 'TOGGLE_MODAL', + }); + expect(result).toEqual({ + isModalOpen: true, + selectedItems: [], + value: null, + multiple: false, + }); + }); + it('should open the modal (multiple)', () => { const state = { isModalOpen: false, From 6e64b5c07083ff7abbb6b29e70a39b2c7018deba Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 4 Dec 2019 10:36:25 -0800 Subject: [PATCH 14/19] clean up act() errors in form tests after Lookup changes --- .../AnsibleSelect/AnsibleSelect.jsx | 5 +- .../src/screens/Host/HostAdd/HostAdd.test.jsx | 39 ++- .../OrganizationAdd/OrganizationAdd.test.jsx | 42 ++- .../OrganizationEdit.test.jsx | 45 +-- .../shared/OrganizationForm.test.jsx | 270 ++++++++++-------- .../Project/ProjectEdit/ProjectEdit.test.jsx | 6 +- .../src/screens/Team/shared/TeamForm.test.jsx | 40 +-- 7 files changed, 261 insertions(+), 186 deletions(-) diff --git a/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx b/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx index c6e427309d..cf860ab8f0 100644 --- a/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/awx/ui_next/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -25,7 +25,7 @@ class AnsibleSelect extends React.Component { } render() { - const { id, data, i18n, isValid, onBlur, value } = this.props; + const { id, data, i18n, isValid, onBlur, value, className } = this.props; return ( {data.map(option => ( {}, + className: '', }; AnsibleSelect.propTypes = { @@ -69,6 +71,7 @@ AnsibleSelect.propTypes = { onBlur: func, onChange: func.isRequired, value: oneOfType([string, number]).isRequired, + className: string, }; export { AnsibleSelect as _AnsibleSelect }; diff --git a/awx/ui_next/src/screens/Host/HostAdd/HostAdd.test.jsx b/awx/ui_next/src/screens/Host/HostAdd/HostAdd.test.jsx index ab1be4ed6f..cd561c5815 100644 --- a/awx/ui_next/src/screens/Host/HostAdd/HostAdd.test.jsx +++ b/awx/ui_next/src/screens/Host/HostAdd/HostAdd.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import HostAdd from './HostAdd'; @@ -7,8 +8,11 @@ import { HostsAPI } from '@api'; jest.mock('@api'); describe('', () => { - test('handleSubmit should post to api', () => { - const wrapper = mountWithContexts(); + test('handleSubmit should post to api', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); const updatedHostData = { name: 'new name', description: 'new description', @@ -19,21 +23,27 @@ describe('', () => { expect(HostsAPI.create).toHaveBeenCalledWith(updatedHostData); }); - test('should navigate to hosts list when cancel is clicked', () => { + test('should navigate to hosts list when cancel is clicked', async () => { const history = createMemoryHistory({}); - const wrapper = mountWithContexts(, { - context: { router: { history } }, + let wrapper; + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); }); - wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); expect(history.location.pathname).toEqual('/hosts'); }); - test('should navigate to hosts list when close (x) is clicked', () => { + test('should navigate to hosts list when close (x) is clicked', async () => { const history = createMemoryHistory({}); - const wrapper = mountWithContexts(, { - context: { router: { history } }, + let wrapper; + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); }); - wrapper.find('button[aria-label="Close"]').prop('onClick')(); + wrapper.find('button[aria-label="Close"]').invoke('onClick')(); expect(history.location.pathname).toEqual('/hosts'); }); @@ -51,11 +61,14 @@ describe('', () => { ...hostData, }, }); - const wrapper = mountWithContexts(, { - context: { router: { history } }, + let wrapper; + await act(async () => { + wrapper = mountWithContexts(, { + context: { router: { history } }, + }); }); await waitForElement(wrapper, 'button[aria-label="Save"]'); - await wrapper.find('HostForm').prop('handleSubmit')(hostData); + await wrapper.find('HostForm').invoke('handleSubmit')(hostData); expect(history.location.pathname).toEqual('/hosts/5'); }); }); 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 b5823fa1c4..6d5974b217 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx @@ -9,6 +9,10 @@ jest.mock('@api'); describe('', () => { test('handleSubmit should post to api', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); const updatedOrgData = { name: 'new name', description: 'new description', @@ -27,22 +31,24 @@ describe('', () => { test('should navigate to organizations list when cancel is clicked', async () => { const history = createMemoryHistory({}); + let wrapper; await act(async () => { - const wrapper = mountWithContexts(, { + wrapper = mountWithContexts(, { context: { router: { history } }, }); - wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); }); expect(history.location.pathname).toEqual('/organizations'); }); test('should navigate to organizations list when close (x) is clicked', async () => { const history = createMemoryHistory({}); + let wrapper; await act(async () => { - const wrapper = mountWithContexts(, { + wrapper = mountWithContexts(, { context: { router: { history } }, }); - wrapper.find('button[aria-label="Close"]').prop('onClick')(); + wrapper.find('button[aria-label="Close"]').invoke('onClick')(); }); expect(history.location.pathname).toEqual('/organizations'); }); @@ -63,8 +69,9 @@ describe('', () => { ...orgData, }, }); + let wrapper; await act(async () => { - const wrapper = mountWithContexts(, { + wrapper = mountWithContexts(, { context: { router: { history } }, }); await waitForElement(wrapper, 'button[aria-label="Save"]'); @@ -92,23 +99,27 @@ describe('', () => { ...orgData, }, }); + let wrapper; await act(async () => { - const wrapper = mountWithContexts(); - await waitForElement(wrapper, 'button[aria-label="Save"]'); - await wrapper.find('OrganizationForm').prop('handleSubmit')( - orgData, - [3], - [] - ); + wrapper = mountWithContexts(); }); + await waitForElement(wrapper, 'button[aria-label="Save"]'); + await wrapper.find('OrganizationForm').prop('handleSubmit')( + orgData, + [3], + [] + ); expect(OrganizationsAPI.associateInstanceGroup).toHaveBeenCalledWith(5, 3); }); test('AnsibleSelect component renders if there are virtual environments', async () => { + const config = { + custom_virtualenvs: ['foo', 'bar'], + }; let wrapper; await act(async () => { wrapper = mountWithContexts(, { - context: { config: { custom_virtualenvs: ['foo', 'bar'] } }, + context: { config }, }).find('AnsibleSelect'); }); expect(wrapper.find('FormSelect')).toHaveLength(1); @@ -122,10 +133,13 @@ describe('', () => { }); test('AnsibleSelect component does not render if there are 0 virtual environments', async () => { + const config = { + custom_virtualenvs: [], + }; let wrapper; await act(async () => { wrapper = mountWithContexts(, { - context: { config: { custom_virtualenvs: [] } }, + context: { config }, }).find('AnsibleSelect'); }); expect(wrapper.find('FormSelect')).toHaveLength(0); 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 0a9a4f9e82..42093807ae 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { createMemoryHistory } from 'history'; import { OrganizationsAPI } from '@api'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; @@ -19,10 +20,11 @@ describe('', () => { }, }; - test('handleSubmit should call api update', () => { - const wrapper = mountWithContexts( - - ); + test('handleSubmit should call api update', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); const updatedOrgData = { name: 'new name', @@ -39,21 +41,23 @@ describe('', () => { }); test('handleSubmit associates and disassociates instance groups', async () => { - const wrapper = mountWithContexts( - - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); const updatedOrgData = { name: 'new name', description: 'new description', custom_virtualenv: 'Buzz', }; - wrapper.find('OrganizationForm').prop('handleSubmit')( - updatedOrgData, - [3, 4], - [2] - ); - await sleep(1); + await act(async () => { + wrapper.find('OrganizationForm').invoke('handleSubmit')( + updatedOrgData, + [3, 4], + [2] + ); + }); expect(OrganizationsAPI.associateInstanceGroup).toHaveBeenCalledWith(1, 3); expect(OrganizationsAPI.associateInstanceGroup).toHaveBeenCalledWith(1, 4); @@ -63,14 +67,17 @@ describe('', () => { ); }); - test('should navigate to organization detail when cancel is clicked', () => { + test('should navigate to organization detail when cancel is clicked', async () => { const history = createMemoryHistory({}); - const wrapper = mountWithContexts( - , - { context: { router: { history } } } - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + , + { context: { router: { history } } } + ); + }); - wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + wrapper.find('button[aria-label="Cancel"]').invoke('onClick')(); expect(history.location.pathname).toEqual('/organizations/1/details'); }); 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 52e234cf9c..9fda0b279d 100644 --- a/awx/ui_next/src/screens/Organization/shared/OrganizationForm.test.jsx +++ b/awx/ui_next/src/screens/Organization/shared/OrganizationForm.test.jsx @@ -1,5 +1,5 @@ import React from 'react'; - +import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; import { OrganizationsAPI } from '@api'; @@ -30,18 +30,20 @@ describe('', () => { jest.clearAllMocks(); }); - test('should request related instance groups from api', () => { - mountWithContexts( - , - { - context: { network }, - } - ); + test('should request related instance groups from api', async () => { + await act(async () => { + mountWithContexts( + , + { + context: { network }, + } + ); + }); expect(OrganizationsAPI.readInstanceGroups).toHaveBeenCalledTimes(1); }); @@ -53,34 +55,39 @@ describe('', () => { results: mockInstanceGroups, }, }); - const wrapper = mountWithContexts( - , - { - context: { network }, - } - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + , + { + context: { network }, + } + ); + }); - await sleep(0); expect(OrganizationsAPI.readInstanceGroups).toHaveBeenCalled(); expect(wrapper.find('OrganizationForm').state().instanceGroups).toEqual( mockInstanceGroups ); }); - test('changing instance group successfully sets instanceGroups state', () => { - const wrapper = mountWithContexts( - - ); + test('changing instance group successfully sets instanceGroups state', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); const lookup = wrapper.find('InstanceGroupsLookup'); expect(lookup.length).toBe(1); @@ -102,15 +109,18 @@ describe('', () => { ]); }); - test('changing inputs should update form values', () => { - const wrapper = mountWithContexts( - - ); + test('changing inputs should update form values', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); const form = wrapper.find('Formik'); wrapper.find('input#org-name').simulate('change', { @@ -127,21 +137,24 @@ describe('', () => { expect(form.state('values').max_hosts).toEqual('134'); }); - test('AnsibleSelect component renders if there are virtual environments', () => { + test('AnsibleSelect component renders if there are virtual environments', async () => { const config = { custom_virtualenvs: ['foo', 'bar'], }; - const wrapper = mountWithContexts( - , - { - context: { config }, - } - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + , + { + context: { config }, + } + ); + }); expect(wrapper.find('FormSelect')).toHaveLength(1); expect(wrapper.find('FormSelectOption')).toHaveLength(3); expect( @@ -154,14 +167,17 @@ describe('', () => { test('calls handleSubmit when form submitted', async () => { const handleSubmit = jest.fn(); - const wrapper = mountWithContexts( - - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); expect(handleSubmit).not.toHaveBeenCalled(); wrapper.find('button[aria-label="Save"]').simulate('click'); await sleep(1); @@ -194,18 +210,20 @@ describe('', () => { OrganizationsAPI.update.mockResolvedValue(1, mockDataForm); OrganizationsAPI.associateInstanceGroup.mockResolvedValue('done'); OrganizationsAPI.disassociateInstanceGroup.mockResolvedValue('done'); - const wrapper = mountWithContexts( - , - { - context: { network }, - } - ); - await sleep(0); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + , + { + context: { network }, + } + ); + }); wrapper.find('InstanceGroupsLookup').prop('onChange')( [{ name: 'One', id: 1 }, { name: 'Three', id: 3 }], 'instanceGroups' @@ -219,15 +237,17 @@ describe('', () => { test('handleSubmit is called with max_hosts value if it is in range', async () => { const handleSubmit = jest.fn(); - // normal mount - const wrapper = mountWithContexts( - - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); wrapper.find('button[aria-label="Save"]').simulate('click'); await sleep(0); expect(handleSubmit).toHaveBeenCalledWith( @@ -245,32 +265,38 @@ describe('', () => { test('handleSubmit does not get called if max_hosts value is out of range', async () => { const handleSubmit = jest.fn(); - // not mount with Negative value + // mount with negative value + let wrapper1; const mockDataNegative = JSON.parse(JSON.stringify(mockData)); mockDataNegative.max_hosts = -5; - const wrapper1 = mountWithContexts( - - ); + await act(async () => { + wrapper1 = mountWithContexts( + + ); + }); wrapper1.find('button[aria-label="Save"]').simulate('click'); await sleep(0); expect(handleSubmit).not.toHaveBeenCalled(); - // not mount with Out of Range value + // mount with out of range value + let wrapper2; const mockDataOoR = JSON.parse(JSON.stringify(mockData)); mockDataOoR.max_hosts = 999999999999; - const wrapper2 = mountWithContexts( - - ); + await act(async () => { + wrapper2 = mountWithContexts( + + ); + }); wrapper2.find('button[aria-label="Save"]').simulate('click'); await sleep(0); expect(handleSubmit).not.toHaveBeenCalled(); @@ -282,14 +308,17 @@ describe('', () => { // mount with String value (default to zero) const mockDataString = JSON.parse(JSON.stringify(mockData)); mockDataString.max_hosts = 'Bee'; - const wrapper = mountWithContexts( - - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); wrapper.find('button[aria-label="Save"]').simulate('click'); await sleep(0); expect(handleSubmit).toHaveBeenCalledWith( @@ -304,17 +333,20 @@ describe('', () => { ); }); - test('calls "handleCancel" when Cancel button is clicked', () => { + test('calls "handleCancel" when Cancel button is clicked', async () => { const handleCancel = jest.fn(); - const wrapper = mountWithContexts( - - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); expect(handleCancel).not.toHaveBeenCalled(); wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); expect(handleCancel).toBeCalled(); diff --git a/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx b/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx index 8f8b400cf1..8927dff7c2 100644 --- a/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx +++ b/awx/ui_next/src/screens/Project/ProjectEdit/ProjectEdit.test.jsx @@ -144,8 +144,8 @@ describe('', () => { wrapper = mountWithContexts(, { context: { router: { history } }, }); + wrapper.find('CardCloseButton').simulate('click'); }); - wrapper.find('CardCloseButton').simulate('click'); expect(history.location.pathname).toEqual('/projects/123/details'); }); @@ -157,7 +157,9 @@ describe('', () => { }); }); await waitForElement(wrapper, 'EmptyStateBody', el => el.length === 0); - wrapper.find('ProjectEdit button[aria-label="Cancel"]').simulate('click'); + await act(async () => { + wrapper.find('ProjectEdit button[aria-label="Cancel"]').simulate('click'); + }); expect(history.location.pathname).toEqual('/projects/123/details'); }); }); diff --git a/awx/ui_next/src/screens/Team/shared/TeamForm.test.jsx b/awx/ui_next/src/screens/Team/shared/TeamForm.test.jsx index 0d8a483417..da8a5d282e 100644 --- a/awx/ui_next/src/screens/Team/shared/TeamForm.test.jsx +++ b/awx/ui_next/src/screens/Team/shared/TeamForm.test.jsx @@ -30,15 +30,17 @@ describe('', () => { jest.clearAllMocks(); }); - test('changing inputs should update form values', () => { - wrapper = mountWithContexts( - - ); + test('changing inputs should update form values', async () => { + await act(async () => { + wrapper = mountWithContexts( + + ); + }); const form = wrapper.find('Formik'); wrapper.find('input#team-name').simulate('change', { @@ -78,17 +80,19 @@ describe('', () => { expect(handleSubmit).toBeCalled(); }); - test('calls handleCancel when Cancel button is clicked', () => { + test('calls handleCancel when Cancel button is clicked', async () => { const handleCancel = jest.fn(); - wrapper = mountWithContexts( - - ); + await act(async () => { + wrapper = mountWithContexts( + + ); + }); expect(handleCancel).not.toHaveBeenCalled(); wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); expect(handleCancel).toBeCalled(); From c003e89ea92242559e8f2f57258d316526ec3e0c Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 4 Dec 2019 10:57:01 -0800 Subject: [PATCH 15/19] fix loading jank in MultiCredentialLookup --- .../ContentLoading/ContentLoading.jsx | 4 +- .../Lookup/MultiCredentialsLookup.jsx | 4 ++ .../PaginatedDataList/PaginatedDataList.jsx | 39 +++++++++++++++++-- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/awx/ui_next/src/components/ContentLoading/ContentLoading.jsx b/awx/ui_next/src/components/ContentLoading/ContentLoading.jsx index f242bbca10..90747007da 100644 --- a/awx/ui_next/src/components/ContentLoading/ContentLoading.jsx +++ b/awx/ui_next/src/components/ContentLoading/ContentLoading.jsx @@ -4,8 +4,8 @@ import { withI18n } from '@lingui/react'; import { EmptyState, EmptyStateBody } from '@patternfly/react-core'; // TODO: Better loading state - skeleton lines / spinner, etc. -const ContentLoading = ({ i18n }) => ( - +const ContentLoading = ({ className, i18n }) => ( + {i18n._(t`Loading...`)} ); diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 1a020cab26..0a472e361f 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -37,6 +37,7 @@ function MultiCredentialsLookup(props) { const [selectedType, setSelectedType] = useState(null); const [credentials, setCredentials] = useState([]); const [credentialsCount, setCredentialsCount] = useState(0); + const [isLoading, setIsLoading] = useState(false); useEffect(() => { (async () => { @@ -56,6 +57,7 @@ function MultiCredentialsLookup(props) { return; } try { + setIsLoading(true); const params = parseQueryString(QS_CONFIG, history.location.search); const { results, count } = await loadCredentials( params, @@ -63,6 +65,7 @@ function MultiCredentialsLookup(props) { ); setCredentials(results); setCredentialsCount(count); + setIsLoading(false); } catch (err) { onError(err); } @@ -133,6 +136,7 @@ function MultiCredentialsLookup(props) { name="credentials" qsConfig={QS_CONFIG} readOnly={!canDelete} + isLoading={isLoading} selectItem={item => { if (isMultiple) { return dispatch({ type: 'SELECT_ITEM', item }); diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index e89431d3a8..223bcd5950 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -25,10 +25,34 @@ import PaginatedDataListItem from './PaginatedDataListItem'; class PaginatedDataList extends React.Component { constructor(props) { super(props); + this.state = { + height: 0, + }; + this.ref = React.createRef(); this.handleSetPage = this.handleSetPage.bind(this); this.handleSetPageSize = this.handleSetPageSize.bind(this); } + componentDidUpdate(prevProps) { + const { items } = this.props; + if (prevProps.items !== items) { + this.findHeight(); + } + } + + findHeight() { + if (!this.ref || !this.ref.current) { + return; + } + const { state } = this; + const height = this.ref.current.scrollHeight; + if (height && height !== state.height) { + this.setState({ + height: this.ref.current.scrollHeight, + }); + } + } + handleSetPage(event, pageNumber) { const { history, qsConfig } = this.props; const { search } = history.location; @@ -66,6 +90,7 @@ class PaginatedDataList extends React.Component { i18n, renderToolbar, } = this.props; + const { height } = this.state; const columns = toolbarColumns.length ? toolbarColumns : [ @@ -85,8 +110,14 @@ class PaginatedDataList extends React.Component { const emptyContentTitle = i18n._(t`No ${pluralizedItemName} Found `); let Content; - if (hasContentLoading && items.length <= 0) { - Content = ; + if (hasContentLoading) { + Content = ( + + ); } else if (contentError) { Content = ; } else if (items.length <= 0) { @@ -100,7 +131,7 @@ class PaginatedDataList extends React.Component { } return ( - +
) : null} - +
); } } From f54616912dfde2dd218ce05dc85638e2723dcec3 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 4 Dec 2019 11:02:02 -0800 Subject: [PATCH 16/19] de-lint --- .../src/components/PaginatedDataList/PaginatedDataList.jsx | 2 +- .../Organization/OrganizationEdit/OrganizationEdit.test.jsx | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index 223bcd5950..9c3c33dce9 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -1,4 +1,4 @@ -import React, { Fragment } from 'react'; +import React from 'react'; import PropTypes, { arrayOf, shape, string, bool } from 'prop-types'; import { DataList } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; 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 42093807ae..2ecf914440 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.test.jsx @@ -7,8 +7,6 @@ import OrganizationEdit from './OrganizationEdit'; jest.mock('@api'); -const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); - describe('', () => { const mockData = { name: 'Foo', From 9de165a676c31daaa7a39ffe96514a5994ca22b8 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 4 Dec 2019 11:41:47 -0800 Subject: [PATCH 17/19] revert MultiCredentialLookup loading jank fix --- .../Lookup/MultiCredentialsLookup.jsx | 10 ++--- .../PaginatedDataList/PaginatedDataList.jsx | 41 +++---------------- 2 files changed, 9 insertions(+), 42 deletions(-) diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx index 0a472e361f..1effa9282d 100644 --- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx +++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx @@ -37,14 +37,14 @@ function MultiCredentialsLookup(props) { const [selectedType, setSelectedType] = useState(null); const [credentials, setCredentials] = useState([]); const [credentialsCount, setCredentialsCount] = useState(0); - const [isLoading, setIsLoading] = useState(false); useEffect(() => { (async () => { try { const types = await loadCredentialTypes(); setCredentialTypes(types); - setSelectedType(types.find(type => type.kind === 'ssh') || types[0]); + const match = types.find(type => type.kind === 'ssh') || types[0]; + setSelectedType(match); } catch (err) { onError(err); } @@ -57,7 +57,6 @@ function MultiCredentialsLookup(props) { return; } try { - setIsLoading(true); const params = parseQueryString(QS_CONFIG, history.location.search); const { results, count } = await loadCredentials( params, @@ -65,14 +64,12 @@ function MultiCredentialsLookup(props) { ); setCredentials(results); setCredentialsCount(count); - setIsLoading(false); } catch (err) { onError(err); } })(); }, [selectedType, history.location.search, onError]); - const isMultiple = selectedType && selectedType.kind === 'vault'; const renderChip = ({ item, removeItem, canDelete }) => ( ); + const isMultiple = selectedType && selectedType.kind === 'vault'; + return ( {tooltip && } @@ -136,7 +135,6 @@ function MultiCredentialsLookup(props) { name="credentials" qsConfig={QS_CONFIG} readOnly={!canDelete} - isLoading={isLoading} selectItem={item => { if (isMultiple) { return dispatch({ type: 'SELECT_ITEM', item }); diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index 9c3c33dce9..e89431d3a8 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { Fragment } from 'react'; import PropTypes, { arrayOf, shape, string, bool } from 'prop-types'; import { DataList } from '@patternfly/react-core'; import { withI18n } from '@lingui/react'; @@ -25,34 +25,10 @@ import PaginatedDataListItem from './PaginatedDataListItem'; class PaginatedDataList extends React.Component { constructor(props) { super(props); - this.state = { - height: 0, - }; - this.ref = React.createRef(); this.handleSetPage = this.handleSetPage.bind(this); this.handleSetPageSize = this.handleSetPageSize.bind(this); } - componentDidUpdate(prevProps) { - const { items } = this.props; - if (prevProps.items !== items) { - this.findHeight(); - } - } - - findHeight() { - if (!this.ref || !this.ref.current) { - return; - } - const { state } = this; - const height = this.ref.current.scrollHeight; - if (height && height !== state.height) { - this.setState({ - height: this.ref.current.scrollHeight, - }); - } - } - handleSetPage(event, pageNumber) { const { history, qsConfig } = this.props; const { search } = history.location; @@ -90,7 +66,6 @@ class PaginatedDataList extends React.Component { i18n, renderToolbar, } = this.props; - const { height } = this.state; const columns = toolbarColumns.length ? toolbarColumns : [ @@ -110,14 +85,8 @@ class PaginatedDataList extends React.Component { const emptyContentTitle = i18n._(t`No ${pluralizedItemName} Found `); let Content; - if (hasContentLoading) { - Content = ( - - ); + if (hasContentLoading && items.length <= 0) { + Content = ; } else if (contentError) { Content = ; } else if (items.length <= 0) { @@ -131,7 +100,7 @@ class PaginatedDataList extends React.Component { } return ( -
+ ) : null} -
+ ); } } From 3409d39150058f73a5b6af2685e2fc6dd047e205 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Wed, 4 Dec 2019 12:59:28 -0800 Subject: [PATCH 18/19] fix ProjectLookup re-renders --- .../src/screens/Template/shared/JobTemplateForm.jsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx index 2f5ab24c64..68aff316f0 100644 --- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx +++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx @@ -79,6 +79,7 @@ class JobTemplateForm extends Component { }; this.handleProjectValidation = this.handleProjectValidation.bind(this); this.loadRelatedInstanceGroups = this.loadRelatedInstanceGroups.bind(this); + this.handleProjectUpdate = this.handleProjectUpdate.bind(this); this.setContentError = this.setContentError.bind(this); } @@ -120,6 +121,12 @@ class JobTemplateForm extends Component { }; } + handleProjectUpdate(project) { + const { setFieldValue } = this.props; + setFieldValue('project', project.id); + this.setState({ project }); + } + setContentError(contentError) { this.setState({ contentError }); } @@ -257,10 +264,7 @@ class JobTemplateForm extends Component { you want this job to execute.`)} isValid={!form.touched.project || !form.errors.project} helperTextInvalid={form.errors.project} - onChange={value => { - form.setFieldValue('project', value.id); - this.setState({ project: value }); - }} + onChange={this.handleProjectUpdate} required /> )} From 846fd676180f0a4d2dedd621fcbd583cc0de5b14 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Tue, 10 Dec 2019 12:13:22 -0800 Subject: [PATCH 19/19] de-lint --- .../Organization/OrganizationAdd/OrganizationAdd.test.jsx | 4 ---- 1 file changed, 4 deletions(-) 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 6d5974b217..c690c8b9f6 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.test.jsx @@ -9,10 +9,6 @@ jest.mock('@api'); describe('', () => { test('handleSubmit should post to api', async () => { - let wrapper; - await act(async () => { - wrapper = mountWithContexts(); - }); const updatedOrgData = { name: 'new name', description: 'new description',