From 9c291c2b50382648163b3a1b798735b7e423244a Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Thu, 12 Dec 2019 16:29:44 -0500 Subject: [PATCH 1/2] Move routed org views to functional components --- .../OrganizationDetail/OrganizationDetail.jsx | 183 +++++----- .../OrganizationDetail.test.jsx | 73 ++-- .../OrganizationEdit/OrganizationEdit.jsx | 95 ++--- .../OrganizationList/OrganizationList.jsx | 335 ++++++++---------- .../OrganizationList.test.jsx | 296 ++++++++++------ .../OrganizationList/OrganizationListItem.jsx | 131 +++---- .../OrganizationTeams/OrganizationTeams.jsx | 91 ++--- .../OrganizationTeams.test.jsx | 19 +- .../Organization/Organizations.test.jsx | 18 +- 9 files changed, 612 insertions(+), 629 deletions(-) diff --git a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx index c21c8cd068..dbd4acf341 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useEffect, useState } from 'react'; import { Link, withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -16,117 +16,92 @@ const CardBody = styled(PFCardBody)` padding-top: 20px; `; -class OrganizationDetail extends Component { - constructor(props) { - super(props); +function OrganizationDetail({ i18n, match, organization }) { + const { + params: { id }, + } = match; + const { + name, + description, + custom_virtualenv, + max_hosts, + created, + modified, + summary_fields, + } = organization; + const [contentError, setContentError] = useState(null); + const [hasContentLoading, setHasContentLoading] = useState(true); + const [instanceGroups, setInstanceGroups] = useState([]); - this.state = { - contentError: null, - hasContentLoading: true, - instanceGroups: [], - }; - this.loadInstanceGroups = this.loadInstanceGroups.bind(this); + useEffect(() => { + (async () => { + setContentError(null); + setHasContentLoading(true); + try { + const { + data: { results = [] }, + } = await OrganizationsAPI.readInstanceGroups(id); + setInstanceGroups(results); + } catch (error) { + setContentError(error); + } finally { + setHasContentLoading(false); + } + })(); + }, [id]); + + if (hasContentLoading) { + return ; } - componentDidMount() { - this.loadInstanceGroups(); + if (contentError) { + return ; } - async loadInstanceGroups() { - const { - match: { - params: { id }, - }, - } = this.props; - - this.setState({ hasContentLoading: true }); - try { - const { - data: { results = [] }, - } = await OrganizationsAPI.readInstanceGroups(id); - this.setState({ instanceGroups: [...results] }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); - } - } - - render() { - const { hasContentLoading, contentError, instanceGroups } = this.state; - const { - organization: { - name, - description, - custom_virtualenv, - max_hosts, - created, - modified, - summary_fields, - }, - match, - i18n, - } = this.props; - - if (hasContentLoading) { - return ; - } - - if (contentError) { - return ; - } - - return ( - - + return ( + + + + + + + + + {instanceGroups && instanceGroups.length > 0 && ( + {instanceGroups.map(ig => ( + + {ig.name} + + ))} + + } /> - - - - - - {instanceGroups && instanceGroups.length > 0 && ( - - {instanceGroups.map(ig => ( - - {ig.name} - - ))} - - } - /> - )} - - {summary_fields.user_capabilities.edit && ( -
- -
)} - - ); - } + + {summary_fields.user_capabilities.edit && ( +
+ +
+ )} + + ); } export default withI18n()(withRouter(OrganizationDetail)); diff --git a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.test.jsx index 0080946859..b3593fd952 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { OrganizationsAPI } from '@api'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; @@ -35,30 +36,42 @@ describe('', () => { jest.clearAllMocks(); }); - test('initially renders succesfully', () => { - mountWithContexts(); + test('initially renders succesfully', async () => { + await act(async () => { + mountWithContexts(); + }); }); - test('should request instance groups from api', () => { - mountWithContexts(); + test('should request instance groups from api', async () => { + await act(async () => { + mountWithContexts(); + }); expect(OrganizationsAPI.readInstanceGroups).toHaveBeenCalledTimes(1); }); - test('should handle setting instance groups to state', async done => { - const wrapper = mountWithContexts( - - ); - const component = await waitForElement(wrapper, 'OrganizationDetail'); - expect(component.state().instanceGroups).toEqual( - mockInstanceGroups.data.results - ); - done(); + test('should render the expected instance group', async () => { + let component; + await act(async () => { + component = mountWithContexts( + + ); + }); + await waitForElement(component, 'ContentLoading', el => el.length === 0); + expect( + component + .find('Chip') + .findWhere(el => el.text() === 'One') + .exists() + ).toBe(true); }); - test('should render Details', async done => { - const wrapper = mountWithContexts( - - ); + test('should render Details', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); const testParams = [ { label: 'Name', value: 'Foo' }, { label: 'Description', value: 'Bar' }, @@ -74,30 +87,34 @@ describe('', () => { expect(detail.find('dt').text()).toBe(label); expect(detail.find('dd').text()).toBe(value); } - done(); }); - test('should show edit button for users with edit permission', async done => { - const wrapper = mountWithContexts( - - ); + test('should show edit button for users with edit permission', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); const editButton = await waitForElement( wrapper, 'OrganizationDetail Button' ); expect(editButton.text()).toEqual('Edit'); expect(editButton.prop('to')).toBe('/organizations/undefined/edit'); - done(); }); - test('should hide edit button for users without edit permission', async done => { + test('should hide edit button for users without edit permission', async () => { const readOnlyOrg = { ...mockOrganization }; readOnlyOrg.summary_fields.user_capabilities.edit = false; - const wrapper = mountWithContexts( - - ); + + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + + ); + }); await waitForElement(wrapper, 'OrganizationDetail'); expect(wrapper.find('OrganizationDetail Button').length).toBe(0); - done(); }); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx index 0f6e0c36c6..5be5572550 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { withRouter } from 'react-router-dom'; import { CardBody } from '@patternfly/react-core'; @@ -8,50 +8,17 @@ import { Config } from '@contexts/Config'; import OrganizationForm from '../shared/OrganizationForm'; -class OrganizationEdit extends Component { - constructor(props) { - super(props); +function OrganizationEdit({ history, organization }) { + const detailsUrl = `/organizations/${organization.id}/details`; + const [formError, setFormError] = useState(null); - this.handleSubmit = this.handleSubmit.bind(this); - this.submitInstanceGroups = this.submitInstanceGroups.bind(this); - this.handleCancel = this.handleCancel.bind(this); - this.handleSuccess = this.handleSuccess.bind(this); - - this.state = { - error: '', - }; - } - - async handleSubmit(values, groupsToAssociate, groupsToDisassociate) { - const { organization } = this.props; + const handleSubmit = async ( + values, + groupsToAssociate, + groupsToDisassociate + ) => { try { await OrganizationsAPI.update(organization.id, values); - await this.submitInstanceGroups(groupsToAssociate, groupsToDisassociate); - this.handleSuccess(); - } catch (err) { - this.setState({ error: err }); - } - } - - handleCancel() { - const { - organization: { id }, - history, - } = this.props; - history.push(`/organizations/${id}/details`); - } - - handleSuccess() { - const { - organization: { id }, - history, - } = this.props; - history.push(`/organizations/${id}/details`); - } - - async submitInstanceGroups(groupsToAssociate, groupsToDisassociate) { - const { organization } = this.props; - try { await Promise.all( groupsToAssociate.map(id => OrganizationsAPI.associateInstanceGroup(organization.id, id) @@ -62,31 +29,31 @@ class OrganizationEdit extends Component { OrganizationsAPI.disassociateInstanceGroup(organization.id, id) ) ); - } catch (err) { - this.setState({ error: err }); + history.push(detailsUrl); + } catch (error) { + setFormError(error); } - } + }; - render() { - const { organization } = this.props; - const { error } = this.state; + const handleCancel = () => { + history.push(detailsUrl); + }; - return ( - - - {({ me }) => ( - - )} - - {error ?
error
: null} - - ); - } + return ( + + + {({ me }) => ( + + )} + + {formError ?
error
: null} + + ); } OrganizationEdit.propTypes = { diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx index d21ff74a9f..66b5e255ba 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -1,4 +1,4 @@ -import React, { Component, Fragment } from 'react'; +import React, { useEffect, useState } from 'react'; import { withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -22,90 +22,24 @@ const QS_CONFIG = getQSConfig('organization', { order_by: 'name', }); -class OrganizationsList extends Component { - constructor(props) { - super(props); +function OrganizationsList({ i18n, location, match }) { + const [contentError, setContentError] = useState(null); + const [deletionError, setDeletionError] = useState(null); + const [hasContentLoading, setHasContentLoading] = useState(true); + const [itemCount, setItemCount] = useState(0); + const [organizations, setOrganizations] = useState([]); + const [orgActions, setOrgActions] = useState(null); + const [selected, setSelected] = useState([]); - this.state = { - hasContentLoading: true, - contentError: null, - deletionError: null, - organizations: [], - selected: [], - itemCount: 0, - actions: null, - }; + const addUrl = `${match.url}/add`; + const canAdd = orgActions && orgActions.POST; + const isAllSelected = + selected.length === organizations.length && selected.length > 0; - this.handleSelectAll = this.handleSelectAll.bind(this); - this.handleSelect = this.handleSelect.bind(this); - this.handleOrgDelete = this.handleOrgDelete.bind(this); - this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); - this.loadOrganizations = this.loadOrganizations.bind(this); - } - - componentDidMount() { - this.loadOrganizations(); - } - - componentDidUpdate(prevProps) { - const { location } = this.props; - if (location !== prevProps.location) { - this.loadOrganizations(); - } - } - - handleSelectAll(isSelected) { - const { organizations } = this.state; - - const selected = isSelected ? [...organizations] : []; - this.setState({ selected }); - } - - handleSelect(row) { - const { selected } = this.state; - - if (selected.some(s => s.id === row.id)) { - this.setState({ selected: selected.filter(s => s.id !== row.id) }); - } else { - this.setState({ selected: selected.concat(row) }); - } - } - - handleDeleteErrorClose() { - this.setState({ deletionError: null }); - } - - async handleOrgDelete() { - const { selected } = this.state; - - this.setState({ hasContentLoading: true }); - try { - await Promise.all(selected.map(org => OrganizationsAPI.destroy(org.id))); - } catch (err) { - this.setState({ deletionError: err }); - } finally { - await this.loadOrganizations(); - } - } - - async loadOrganizations() { - const { location } = this.props; - const { actions: cachedActions } = this.state; - const params = parseQueryString(QS_CONFIG, location.search); - - let optionsPromise; - if (cachedActions) { - optionsPromise = Promise.resolve({ data: { actions: cachedActions } }); - } else { - optionsPromise = OrganizationsAPI.readOptions(); - } - - const promises = Promise.all([ - OrganizationsAPI.read(params), - optionsPromise, - ]); - - this.setState({ contentError: null, hasContentLoading: true }); + const loadOrganizations = async ({ search }) => { + const params = parseQueryString(QS_CONFIG, search); + setContentError(null); + setHasContentLoading(true); try { const [ { @@ -114,117 +48,140 @@ class OrganizationsList extends Component { { data: { actions }, }, - ] = await promises; - this.setState({ - actions, - itemCount: count, - organizations: results, - selected: [], - }); - } catch (err) { - this.setState({ contentError: err }); + ] = await Promise.all([ + OrganizationsAPI.read(params), + loadOrganizationActions(), + ]); + setItemCount(count); + setOrganizations(results); + setOrgActions(actions); + setSelected([]); + } catch (error) { + setContentError(error); } finally { - this.setState({ hasContentLoading: false }); + setHasContentLoading(false); } - } + }; - render() { - const { - actions, - itemCount, - contentError, - hasContentLoading, - deletionError, - selected, - organizations, - } = this.state; - const { match, i18n } = this.props; + const loadOrganizationActions = () => { + if (orgActions) { + return Promise.resolve({ data: { actions: orgActions } }); + } + return OrganizationsAPI.readOptions(); + }; - const canAdd = - actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); - const isAllSelected = - selected.length === organizations.length && selected.length > 0; + const handleOrgDelete = async () => { + setHasContentLoading(true); + try { + await Promise.all(selected.map(({ id }) => OrganizationsAPI.destroy(id))); + } catch (error) { + setDeletionError(error); + } finally { + await loadOrganizations(location); + } + }; - return ( - - - - ( - , - canAdd ? ( - - ) : null, - ]} - /> - )} - renderItem={o => ( - row.id === o.id)} - onSelect={() => this.handleSelect(o)} - /> - )} - emptyStateControls={ - canAdd ? ( - - ) : null - } - /> - - - - {i18n._(t`Failed to delete one or more organizations.`)} - - - - ); - } + const handleSelectAll = isSelected => { + if (isSelected) { + setSelected(organizations); + } else { + setSelected([]); + } + }; + + const handleSelect = row => { + if (selected.some(s => s.id === row.id)) { + setSelected(selected.filter(s => s.id !== row.id)); + } else { + setSelected(selected.concat(row)); + } + }; + + const handleDeleteErrorClose = () => { + setDeletionError(null); + }; + + useEffect(() => { + loadOrganizations(location); + }, [location]); // eslint-disable-line react-hooks/exhaustive-deps + + return ( + <> + + + ( + , + canAdd ? ( + + ) : null, + ]} + /> + )} + renderItem={o => ( + row.id === o.id)} + onSelect={() => handleSelect(o)} + /> + )} + emptyStateControls={ + canAdd ? : null + } + /> + + + + {i18n._(t`Failed to delete one or more organizations.`)} + + + + ); } export { OrganizationsList as _OrganizationsList }; diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.test.jsx index a07556f9eb..5c7fb53966 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.test.jsx @@ -1,12 +1,14 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; + import { OrganizationsAPI } from '@api'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; -import OrganizationsList, { _OrganizationsList } from './OrganizationList'; +import OrganizationsList from './OrganizationList'; jest.mock('@api'); -const mockAPIOrgsList = { +const mockOrganizations = { data: { count: 3, results: [ @@ -61,97 +63,165 @@ const mockAPIOrgsList = { describe('', () => { let wrapper; - beforeEach(() => { - OrganizationsAPI.read = () => - Promise.resolve({ - data: { - count: 0, - results: [], + OrganizationsAPI.read.mockResolvedValue(mockOrganizations); + OrganizationsAPI.readOptions.mockResolvedValue({ + data: { + actions: { + GET: {}, + POST: {}, }, - }); - OrganizationsAPI.readOptions = () => - Promise.resolve({ - data: { - actions: { - GET: {}, - POST: {}, - }, - }, - }); + }, + }); + }); + afterEach(() => { + jest.clearAllMocks(); }); - test('initially renders succesfully', () => { - mountWithContexts(); + test('Initially renders succesfully', async () => { + await act(async () => { + mountWithContexts(); + }); }); - test('Puts 1 selected Org in state when handleSelect is called.', () => { - wrapper = mountWithContexts().find( - 'OrganizationsList' + test('Items are rendered after loading', async () => { + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find('ContentLoading').length === 0 ); - - wrapper.setState({ - organizations: mockAPIOrgsList.data.results, - itemCount: 3, - isInitialized: true, - }); - wrapper.update(); - expect(wrapper.state('selected').length).toBe(0); - wrapper.instance().handleSelect(mockAPIOrgsList.data.results.slice(0, 1)); - expect(wrapper.state('selected').length).toBe(1); + expect(wrapper.find('OrganizationListItem').length).toBe(3); }); - test('Puts all Orgs in state when handleSelectAll is called.', () => { - wrapper = mountWithContexts(); - const list = wrapper.find('OrganizationsList'); - list.setState({ - organizations: mockAPIOrgsList.data.results, - itemCount: 3, - isInitialized: true, + test('Item appears selected after selecting it', async () => { + const itemCheckboxInput = 'input#select-organization-1'; + await act(async () => { + wrapper = mountWithContexts(); }); - expect(list.state('selected').length).toBe(0); - list.instance().handleSelectAll(true); - wrapper.update(); - expect(list.state('selected').length).toEqual( - list.state('organizations').length + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find('ContentLoading').length === 0 + ); + await act(async () => { + wrapper + .find(itemCheckboxInput) + .closest('DataListCheck') + .props() + .onChange(); + }); + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find(itemCheckboxInput).props().checked === true ); }); - test('api is called to delete Orgs for each org in selected.', () => { - wrapper = mountWithContexts(); - const component = wrapper.find('OrganizationsList'); - wrapper.find('OrganizationsList').setState({ - organizations: mockAPIOrgsList.data.results, - itemCount: 3, - isInitialized: true, - isModalOpen: mockAPIOrgsList.isModalOpen, - selected: mockAPIOrgsList.data.results, + test('All items appear selected after select-all and unselected after unselect-all', async () => { + const itemCheckboxInputs = [ + 'input#select-organization-1', + 'input#select-organization-2', + 'input#select-organization-3', + ]; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); - expect(OrganizationsAPI.destroy).toHaveBeenCalledTimes( - component.state('selected').length + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find('ContentLoading').length === 0 ); + // Check for initially unselected items + await waitForElement( + wrapper, + 'input#select-all', + el => el.props().checked === false + ); + itemCheckboxInputs.forEach(inputSelector => { + const checkboxInput = wrapper + .find('OrganizationsList') + .find(inputSelector); + expect(checkboxInput.props().checked === false); + }); + // Check select-all behavior + await act(async () => { + wrapper + .find('Checkbox#select-all') + .props() + .onChange(true); + }); + await waitForElement( + wrapper, + 'input#select-all', + el => el.props().checked === true + ); + itemCheckboxInputs.forEach(inputSelector => { + const checkboxInput = wrapper + .find('OrganizationsList') + .find(inputSelector); + expect(checkboxInput.props().checked === true); + }); + // Check unselect-all behavior + await act(async () => { + wrapper + .find('Checkbox#select-all') + .props() + .onChange(false); + }); + await waitForElement( + wrapper, + 'input#select-all', + el => el.props().checked === false + ); + itemCheckboxInputs.forEach(inputSelector => { + const checkboxInput = wrapper + .find('OrganizationsList') + .find(inputSelector); + expect(checkboxInput.props().checked === false); + }); }); - test('call loadOrganizations after org(s) have been deleted', () => { - const fetchOrgs = jest.spyOn( - _OrganizationsList.prototype, - 'loadOrganizations' - ); - const event = { preventDefault: () => {} }; - wrapper = mountWithContexts(); - wrapper.find('OrganizationsList').setState({ - organizations: mockAPIOrgsList.data.results, - itemCount: 3, - isInitialized: true, - selected: mockAPIOrgsList.data.results.slice(0, 1), + test('Expected api calls are made for multi-delete', async () => { + await act(async () => { + wrapper = mountWithContexts(); }); - const component = wrapper.find('OrganizationsList'); - component.instance().handleOrgDelete(event); - expect(fetchOrgs).toBeCalled(); + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find('ContentLoading').length === 0 + ); + expect(OrganizationsAPI.read).toHaveBeenCalledTimes(1); + await act(async () => { + wrapper + .find('Checkbox#select-all') + .props() + .onChange(true); + }); + await waitForElement( + wrapper, + 'input#select-all', + el => el.props().checked === true + ); + await act(async () => { + wrapper.find('button[aria-label="Delete"]').simulate('click'); + wrapper.update(); + }); + const deleteButton = global.document.querySelector( + 'body div[role="dialog"] button[aria-label="confirm delete"]' + ); + expect(deleteButton).not.toEqual(null); + await act(async () => { + deleteButton.click(); + }); + expect(OrganizationsAPI.destroy).toHaveBeenCalledTimes(3); + expect(OrganizationsAPI.read).toHaveBeenCalledTimes(2); }); - test('error is shown when org not successfully deleted from api', async done => { + test('Error dialog shown for failed deletion', async () => { + const itemCheckboxInput = 'input#select-organization-1'; OrganizationsAPI.destroy.mockRejectedValue( new Error({ response: { @@ -163,60 +233,72 @@ describe('', () => { }, }) ); - - wrapper = mountWithContexts(); - wrapper.find('OrganizationsList').setState({ - organizations: mockAPIOrgsList.data.results, - itemCount: 3, - isInitialized: true, - selected: mockAPIOrgsList.data.results.slice(0, 1), + await act(async () => { + wrapper = mountWithContexts(); + }); + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find('ContentLoading').length === 0 + ); + await act(async () => { + wrapper + .find(itemCheckboxInput) + .closest('DataListCheck') + .props() + .onChange(); + }); + await waitForElement( + wrapper, + 'OrganizationsList', + el => el.find(itemCheckboxInput).props().checked === true + ); + await act(async () => { + wrapper.find('button[aria-label="Delete"]').simulate('click'); + wrapper.update(); + }); + const deleteButton = global.document.querySelector( + 'body div[role="dialog"] button[aria-label="confirm delete"]' + ); + expect(deleteButton).not.toEqual(null); + await act(async () => { + deleteButton.click(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); await waitForElement( wrapper, 'Modal', el => el.props().isOpen === true && el.props().title === 'Error!' ); - done(); }); - test('Add button shown for users without ability to POST', async done => { - wrapper = mountWithContexts(); + test('Add button shown for users with ability to POST', async () => { + await act(async () => { + wrapper = mountWithContexts(); + }); await waitForElement( wrapper, 'OrganizationsList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'OrganizationsList', - el => el.state('hasContentLoading') === false + el => el.find('ContentLoading').length === 0 ); expect(wrapper.find('ToolbarAddButton').length).toBe(1); - done(); }); - test('Add button hidden for users without ability to POST', async done => { - OrganizationsAPI.readOptions = () => - Promise.resolve({ - data: { - actions: { - GET: {}, - }, + test('Add button hidden for users without ability to POST', async () => { + OrganizationsAPI.readOptions.mockResolvedValue({ + data: { + actions: { + GET: {}, }, - }); - wrapper = mountWithContexts(); + }, + }); + await act(async () => { + wrapper = mountWithContexts(); + }); await waitForElement( wrapper, 'OrganizationsList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'OrganizationsList', - el => el.state('hasContentLoading') === false + el => el.find('ContentLoading').length === 0 ); expect(wrapper.find('ToolbarAddButton').length).toBe(0); - done(); }); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx index d5e8ed7cf7..62ee06d279 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationListItem.jsx @@ -40,71 +40,72 @@ const ListGroup = styled.span` } `; -class OrganizationListItem extends React.Component { - static propTypes = { - organization: Organization.isRequired, - detailUrl: string.isRequired, - isSelected: bool.isRequired, - onSelect: func.isRequired, - }; - - render() { - const { organization, isSelected, onSelect, detailUrl, i18n } = this.props; - const labelId = `check-action-${organization.id}`; - return ( - - - - - - - - {organization.name} - - - , - - - {i18n._(t`Members`)} - - {organization.summary_fields.related_field_counts.users} - - - - {i18n._(t`Teams`)} - - {organization.summary_fields.related_field_counts.teams} - - - , - - {organization.summary_fields.user_capabilities.edit && ( - + + + + + + + {organization.name} + + + , + + + {i18n._(t`Members`)} + + {organization.summary_fields.related_field_counts.users} + + + + {i18n._(t`Teams`)} + + {organization.summary_fields.related_field_counts.teams} + + + , + + {organization.summary_fields.user_capabilities.edit && ( + + - - - - - )} - , - ]} - /> - - - ); - } + + + + )} + , + ]} + /> + + + ); } + +OrganizationListItem.propTypes = { + organization: Organization.isRequired, + detailUrl: string.isRequired, + isSelected: bool.isRequired, + onSelect: func.isRequired, +}; + export default withI18n()(OrganizationListItem); diff --git a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx index 40baa57d75..6c1b41a36e 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import { withRouter } from 'react-router-dom'; @@ -12,64 +12,41 @@ const QS_CONFIG = getQSConfig('team', { order_by: 'name', }); -class OrganizationTeams extends React.Component { - constructor(props) { - super(props); +function OrganizationTeams({ id, location }) { + const [contentError, setContentError] = useState(null); + const [hasContentLoading, setHasContentLoading] = useState(false); + const [itemCount, setItemCount] = useState(0); + const [teams, setTeams] = useState([]); - this.loadOrganizationTeamsList = this.loadOrganizationTeamsList.bind(this); + useEffect(() => { + (async () => { + const params = parseQueryString(QS_CONFIG, location.search); + setContentError(null); + setHasContentLoading(true); + try { + const { + data: { count = 0, results = [] }, + } = await OrganizationsAPI.readTeams(id, params); + setItemCount(count); + setTeams(results); + } catch (error) { + setContentError(error); + } finally { + setHasContentLoading(false); + } + })(); + }, [id, location]); - this.state = { - contentError: null, - hasContentLoading: true, - itemCount: 0, - teams: [], - }; - } - - componentDidMount() { - this.loadOrganizationTeamsList(); - } - - componentDidUpdate(prevProps) { - const { location } = this.props; - if (location !== prevProps.location) { - this.loadOrganizationTeamsList(); - } - } - - async loadOrganizationTeamsList() { - const { id, location } = this.props; - const params = parseQueryString(QS_CONFIG, location.search); - - this.setState({ hasContentLoading: true, contentError: null }); - try { - const { - data: { count = 0, results = [] }, - } = await OrganizationsAPI.readTeams(id, params); - this.setState({ - itemCount: count, - teams: results, - }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); - } - } - - render() { - const { contentError, hasContentLoading, teams, itemCount } = this.state; - return ( - - ); - } + return ( + + ); } OrganizationTeams.propTypes = { diff --git a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx index 1095c7751c..d4a3ed9838 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx @@ -1,4 +1,5 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { shallow } from 'enzyme'; import { OrganizationsAPI } from '@api'; @@ -41,10 +42,12 @@ describe('', () => { ); }); - test('should load teams on mount', () => { - mountWithContexts().find( - 'OrganizationTeams' - ); + test('should load teams on mount', async () => { + await act(async () => { + mountWithContexts().find( + 'OrganizationTeams' + ); + }); expect(OrganizationsAPI.readTeams).toHaveBeenCalledWith(1, { page: 1, page_size: 5, @@ -53,10 +56,10 @@ describe('', () => { }); test('should pass fetched teams to PaginatedDatalist', async () => { - const wrapper = mountWithContexts( - - ); - + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); await sleep(0); wrapper.update(); diff --git a/awx/ui_next/src/screens/Organization/Organizations.test.jsx b/awx/ui_next/src/screens/Organization/Organizations.test.jsx index 821a7c9e03..6a9a07ba19 100644 --- a/awx/ui_next/src/screens/Organization/Organizations.test.jsx +++ b/awx/ui_next/src/screens/Organization/Organizations.test.jsx @@ -1,16 +1,20 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; + import { mountWithContexts } from '@testUtils/enzymeHelpers'; import Organizations from './Organizations'; jest.mock('@api'); describe('', () => { - test('initially renders succesfully', () => { - mountWithContexts( - - ); + test('initially renders succesfully', async () => { + await act(async () => { + mountWithContexts( + + ); + }); }); }); From 7cc3a7c39d3a20f25c6f7e26c41e14533c3970d2 Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Thu, 12 Dec 2019 17:11:34 -0500 Subject: [PATCH 2/2] Replace withRouter HOC with route hooks --- .../OrganizationAdd/OrganizationAdd.jsx | 7 ++++--- .../OrganizationDetail/OrganizationDetail.jsx | 8 ++++---- .../OrganizationEdit/OrganizationEdit.jsx | 7 ++++--- .../OrganizationList/OrganizationList.jsx | 8 +++++--- .../OrganizationTeams/OrganizationTeams.jsx | 7 ++++--- .../OrganizationTeams.test.jsx | 19 ++++++++++--------- 6 files changed, 31 insertions(+), 25 deletions(-) diff --git a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx index 39045c7ff6..d8837a93c7 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationAdd/OrganizationAdd.jsx @@ -1,6 +1,6 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; -import { withRouter } from 'react-router-dom'; +import { useHistory } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { @@ -16,7 +16,8 @@ import { Config } from '@contexts/Config'; import CardCloseButton from '@components/CardCloseButton'; import OrganizationForm from '../shared/OrganizationForm'; -function OrganizationAdd({ history, i18n }) { +function OrganizationAdd({ i18n }) { + const history = useHistory(); const [formError, setFormError] = useState(null); const handleSubmit = async (values, groupsToAssociate) => { @@ -67,4 +68,4 @@ OrganizationAdd.contextTypes = { }; export { OrganizationAdd as _OrganizationAdd }; -export default withI18n()(withRouter(OrganizationAdd)); +export default withI18n()(OrganizationAdd); diff --git a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx index dbd4acf341..29fe94295e 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx @@ -1,5 +1,5 @@ import React, { useEffect, useState } from 'react'; -import { Link, withRouter } from 'react-router-dom'; +import { Link, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { CardBody as PFCardBody, Button } from '@patternfly/react-core'; @@ -16,10 +16,10 @@ const CardBody = styled(PFCardBody)` padding-top: 20px; `; -function OrganizationDetail({ i18n, match, organization }) { +function OrganizationDetail({ i18n, organization }) { const { params: { id }, - } = match; + } = useRouteMatch(); const { name, description, @@ -104,4 +104,4 @@ function OrganizationDetail({ i18n, match, organization }) { ); } -export default withI18n()(withRouter(OrganizationDetail)); +export default withI18n()(OrganizationDetail); diff --git a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx index 5be5572550..ae38719d8b 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationEdit/OrganizationEdit.jsx @@ -1,6 +1,6 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; -import { withRouter } from 'react-router-dom'; +import { useHistory } from 'react-router-dom'; import { CardBody } from '@patternfly/react-core'; import { OrganizationsAPI } from '@api'; @@ -8,8 +8,9 @@ import { Config } from '@contexts/Config'; import OrganizationForm from '../shared/OrganizationForm'; -function OrganizationEdit({ history, organization }) { +function OrganizationEdit({ organization }) { const detailsUrl = `/organizations/${organization.id}/details`; + const history = useHistory(); const [formError, setFormError] = useState(null); const handleSubmit = async ( @@ -65,4 +66,4 @@ OrganizationEdit.contextTypes = { }; export { OrganizationEdit as _OrganizationEdit }; -export default withRouter(OrganizationEdit); +export default OrganizationEdit; diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx index 66b5e255ba..401e1efd02 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -1,5 +1,5 @@ import React, { useEffect, useState } from 'react'; -import { withRouter } from 'react-router-dom'; +import { useLocation, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Card, PageSection } from '@patternfly/react-core'; @@ -22,7 +22,9 @@ const QS_CONFIG = getQSConfig('organization', { order_by: 'name', }); -function OrganizationsList({ i18n, location, match }) { +function OrganizationsList({ i18n }) { + const location = useLocation(); + const match = useRouteMatch(); const [contentError, setContentError] = useState(null); const [deletionError, setDeletionError] = useState(null); const [hasContentLoading, setHasContentLoading] = useState(true); @@ -185,4 +187,4 @@ function OrganizationsList({ i18n, location, match }) { } export { OrganizationsList as _OrganizationsList }; -export default withI18n()(withRouter(OrganizationsList)); +export default withI18n()(OrganizationsList); diff --git a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx index 6c1b41a36e..832eba41c4 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx @@ -1,6 +1,6 @@ import React, { useEffect, useState } from 'react'; import PropTypes from 'prop-types'; -import { withRouter } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; import { OrganizationsAPI } from '@api'; import PaginatedDataList from '@components/PaginatedDataList'; @@ -12,7 +12,8 @@ const QS_CONFIG = getQSConfig('team', { order_by: 'name', }); -function OrganizationTeams({ id, location }) { +function OrganizationTeams({ id }) { + const location = useLocation(); const [contentError, setContentError] = useState(null); const [hasContentLoading, setHasContentLoading] = useState(false); const [itemCount, setItemCount] = useState(0); @@ -54,4 +55,4 @@ OrganizationTeams.propTypes = { }; export { OrganizationTeams as _OrganizationTeams }; -export default withRouter(OrganizationTeams); +export default OrganizationTeams; diff --git a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx index d4a3ed9838..22e9b988a3 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationTeams/OrganizationTeams.test.jsx @@ -1,6 +1,5 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { shallow } from 'enzyme'; import { OrganizationsAPI } from '@api'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; @@ -32,14 +31,16 @@ describe('', () => { jest.clearAllMocks(); }); - test('renders succesfully', () => { - shallow( - - ); + test('renders succesfully', async () => { + await act(async () => { + mountWithContexts( + + ); + }); }); test('should load teams on mount', async () => {