From 89a4b03d458d70a4bc60e4cbb87a727cc14a025f Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 20 Feb 2020 16:21:58 -0800 Subject: [PATCH 1/4] convert TeamList to hooks --- .../src/screens/Team/TeamList/TeamList.jsx | 356 ++++++++---------- .../screens/Team/TeamList/TeamList.test.jsx | 289 +++++++------- 2 files changed, 302 insertions(+), 343 deletions(-) diff --git a/awx/ui_next/src/screens/Team/TeamList/TeamList.jsx b/awx/ui_next/src/screens/Team/TeamList/TeamList.jsx index cf02be5fac..261bfb2641 100644 --- a/awx/ui_next/src/screens/Team/TeamList/TeamList.jsx +++ b/awx/ui_next/src/screens/Team/TeamList/TeamList.jsx @@ -1,10 +1,11 @@ -import React, { Component, Fragment } from 'react'; -import { withRouter } from 'react-router-dom'; +import React, { Fragment, useState, useEffect, useCallback } from 'react'; +import { useLocation, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Card, PageSection } from '@patternfly/react-core'; import { TeamsAPI } from '@api'; +import useRequest, { useDeleteItems } from '@util/useRequest'; import AlertModal from '@components/AlertModal'; import DataListToolbar from '@components/DataListToolbar'; import ErrorDetail from '@components/ErrorDetail'; @@ -22,218 +23,167 @@ const QS_CONFIG = getQSConfig('team', { order_by: 'name', }); -class TeamsList extends Component { - constructor(props) { - super(props); +function TeamList({ i18n }) { + const location = useLocation(); + const match = useRouteMatch(); + const [selected, setSelected] = useState([]); - this.state = { - hasContentLoading: true, - contentError: null, - deletionError: null, + const { + result: { teams, itemCount, actions }, + error: contentError, + isLoading, + request: fetchTeams, + } = useRequest( + useCallback(async () => { + const params = parseQueryString(QS_CONFIG, location.search); + const [response, actionsResponse] = await Promise.all([ + TeamsAPI.read(params), + TeamsAPI.readOptions(), + ]); + return { + teams: response.data.results, + itemCount: response.data.count, + actions: actionsResponse.data.actions, + }; + }, [location]), + { teams: [], - selected: [], itemCount: 0, - actions: null, - }; - - this.handleSelectAll = this.handleSelectAll.bind(this); - this.handleSelect = this.handleSelect.bind(this); - this.handleTeamDelete = this.handleTeamDelete.bind(this); - this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); - this.loadTeams = this.loadTeams.bind(this); - } - - componentDidMount() { - this.loadTeams(); - } - - componentDidUpdate(prevProps) { - const { location } = this.props; - if (location !== prevProps.location) { - this.loadTeams(); + actions: {}, } - } + ); - handleSelectAll(isSelected) { - const { teams } = this.state; + useEffect(() => { + fetchTeams(); + }, [fetchTeams]); - const selected = isSelected ? [...teams] : []; - this.setState({ selected }); - } + const isAllSelected = selected.length === teams.length && selected.length > 0; + const { + isLoading: isDeleteLoading, + deleteItems: deleteTeams, + deletionError, + clearDeletionError, + } = useDeleteItems( + useCallback(async () => { + return Promise.all(selected.map(team => TeamsAPI.destroy(team.id))); + }, [selected]), + { + qsConfig: QS_CONFIG, + allItemsSelected: isAllSelected, + fetchItems: fetchTeams, + } + ); - handleSelect(row) { - const { selected } = this.state; + const handleTeamDelete = async () => { + await deleteTeams(); + setSelected([]); + }; + const hasContentLoading = isDeleteLoading || isLoading; + const canAdd = actions && actions.POST; + + const handleSelectAll = isSelected => { + setSelected(isSelected ? [...teams] : []); + }; + + const handleSelect = row => { if (selected.some(s => s.id === row.id)) { - this.setState({ selected: selected.filter(s => s.id !== row.id) }); + setSelected(selected.filter(s => s.id !== row.id)); } else { - this.setState({ selected: selected.concat(row) }); + setSelected(selected.concat(row)); } - } + }; - handleDeleteErrorClose() { - this.setState({ deletionError: null }); - } - - async handleTeamDelete() { - const { selected } = this.state; - - this.setState({ hasContentLoading: true }); - try { - await Promise.all(selected.map(team => TeamsAPI.destroy(team.id))); - } catch (err) { - this.setState({ deletionError: err }); - } finally { - await this.loadTeams(); - } - } - - async loadTeams() { - 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 = TeamsAPI.readOptions(); - } - - const promises = Promise.all([TeamsAPI.read(params), optionsPromise]); - - this.setState({ contentError: null, hasContentLoading: true }); - try { - const [ - { - data: { count, results }, - }, - { - data: { actions }, - }, - ] = await promises; - this.setState({ - actions, - itemCount: count, - teams: results, - selected: [], - }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); - } - } - - render() { - const { - actions, - itemCount, - contentError, - hasContentLoading, - deletionError, - selected, - teams, - } = this.state; - const { match, i18n } = this.props; - - const canAdd = - actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); - const isAllSelected = - selected.length > 0 && selected.length === teams.length; - - return ( - - - - ( - , - ] - : []), - , - ]} - /> - )} - renderItem={o => ( - row.id === o.id)} - onSelect={() => this.handleSelect(o)} - /> - )} - emptyStateControls={ - canAdd ? ( - - ) : null - } - /> - - - - {i18n._(t`Failed to delete one or more teams.`)} - - - - ); - } + return ( + + + + ( + , + ] + : []), + , + ]} + /> + )} + renderItem={o => ( + row.id === o.id)} + onSelect={() => handleSelect(o)} + /> + )} + emptyStateControls={ + canAdd ? ( + + ) : null + } + /> + + + + {i18n._(t`Failed to delete one or more teams.`)} + + + + ); } -export { TeamsList as _TeamsList }; -export default withI18n()(withRouter(TeamsList)); +export default withI18n()(TeamList); diff --git a/awx/ui_next/src/screens/Team/TeamList/TeamList.test.jsx b/awx/ui_next/src/screens/Team/TeamList/TeamList.test.jsx index 063b958a70..da6520581b 100644 --- a/awx/ui_next/src/screens/Team/TeamList/TeamList.test.jsx +++ b/awx/ui_next/src/screens/Team/TeamList/TeamList.test.jsx @@ -1,12 +1,13 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { TeamsAPI } from '@api'; -import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; -import TeamsList, { _TeamsList } from './TeamList'; +import TeamList from './TeamList'; jest.mock('@api'); -const mockAPITeamsList = { +const mockAPITeamList = { data: { count: 3, results: [ @@ -50,15 +51,14 @@ const mockAPITeamsList = { warningMsg: 'message', }; -describe('', () => { - let wrapper; - +describe('', () => { beforeEach(() => { - TeamsAPI.read = () => + TeamsAPI.read = jest.fn(() => Promise.resolve({ - data: mockAPITeamsList.data, - }); - TeamsAPI.readOptions = () => + data: mockAPITeamList.data, + }) + ); + TeamsAPI.readOptions = jest.fn(() => Promise.resolve({ data: { actions: { @@ -66,105 +66,119 @@ describe('', () => { POST: {}, }, }, - }); - }); - - test('initially renders succesfully', () => { - mountWithContexts(); - }); - - test('Selects one team when row is checked', async () => { - wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'TeamsList', - el => el.state('hasContentLoading') === false + }) ); - expect( - wrapper - .find('input[type="checkbox"]') - .findWhere(n => n.prop('checked') === true).length - ).toBe(0); - wrapper - .find('TeamListItem') - .at(0) - .find('DataListCheck') - .props() - .onChange(true); - wrapper.update(); - expect( - wrapper - .find('input[type="checkbox"]') - .findWhere(n => n.prop('checked') === true).length - ).toBe(1); }); - test('Select all checkbox selects and unselects all rows', async () => { - wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'TeamsList', - el => el.state('hasContentLoading') === false - ); - expect( - wrapper - .find('input[type="checkbox"]') - .findWhere(n => n.prop('checked') === true).length - ).toBe(0); - wrapper - .find('Checkbox#select-all') - .props() - .onChange(true); - wrapper.update(); - expect( - wrapper - .find('input[type="checkbox"]') - .findWhere(n => n.prop('checked') === true).length - ).toBe(4); - wrapper - .find('Checkbox#select-all') - .props() - .onChange(false); - wrapper.update(); - expect( - wrapper - .find('input[type="checkbox"]') - .findWhere(n => n.prop('checked') === true).length - ).toBe(0); - }); - - test('api is called to delete Teams for each team in selected.', () => { - wrapper = mountWithContexts(); - const component = wrapper.find('TeamsList'); - wrapper.find('TeamsList').setState({ - teams: mockAPITeamsList.data.results, - itemCount: 3, - isInitialized: true, - isModalOpen: mockAPITeamsList.isModalOpen, - selected: mockAPITeamsList.data.results, + test('should load and render teams', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); - expect(TeamsAPI.destroy).toHaveBeenCalledTimes( - component.state('selected').length - ); + wrapper.update(); + + expect(wrapper.find('TeamListItem')).toHaveLength(3); }); - test('call loadTeams after team(s) have been deleted', () => { - const fetchTeams = jest.spyOn(_TeamsList.prototype, 'loadTeams'); - const event = { preventDefault: () => {} }; - wrapper = mountWithContexts(); - wrapper.find('TeamsList').setState({ - teams: mockAPITeamsList.data.results, - itemCount: 3, - isInitialized: true, - selected: mockAPITeamsList.data.results.slice(0, 1), + test('should select team when checked', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - const component = wrapper.find('TeamsList'); - component.instance().handleTeamDelete(event); - expect(fetchTeams).toBeCalled(); + wrapper.update(); + + await act(async () => { + wrapper + .find('TeamListItem') + .first() + .invoke('onSelect')(); + }); + wrapper.update(); + + expect( + wrapper + .find('TeamListItem') + .first() + .prop('isSelected') + ).toEqual(true); }); - test('error is shown when team not successfully deleted from api', async done => { + test('should select all', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper.find('DataListToolbar').invoke('onSelectAll')(true); + }); + wrapper.update(); + + const items = wrapper.find('TeamListItem'); + expect(items).toHaveLength(3); + items.forEach(item => { + expect(item.prop('isSelected')).toEqual(true); + }); + + expect( + wrapper + .find('TeamListItem') + .first() + .prop('isSelected') + ).toEqual(true); + }); + + test('should call delete api', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper + .find('TeamListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper + .find('TeamListItem') + .at(1) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + + expect(TeamsAPI.destroy).toHaveBeenCalledTimes(2); + }); + + test('should re-fetch teams after team(s) have been deleted', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + expect(TeamsAPI.read).toHaveBeenCalledTimes(1); + await act(async () => { + wrapper + .find('TeamListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + + expect(TeamsAPI.read).toHaveBeenCalledTimes(2); + }); + + test('should show deletion error', async () => { TeamsAPI.destroy.mockRejectedValue( new Error({ response: { @@ -176,40 +190,41 @@ describe('', () => { }, }) ); - - wrapper = mountWithContexts(); - wrapper.find('TeamsList').setState({ - teams: mockAPITeamsList.data.results, - itemCount: 3, - isInitialized: true, - selected: mockAPITeamsList.data.results.slice(0, 1), + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); - await waitForElement( - wrapper, - 'Modal', - el => el.props().isOpen === true && el.props().title === 'Error!' - ); - done(); + wrapper.update(); + expect(TeamsAPI.read).toHaveBeenCalledTimes(1); + await act(async () => { + wrapper + .find('TeamListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); + + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + wrapper.update(); + + const modal = wrapper.find('Modal'); + expect(modal).toHaveLength(1); + expect(modal.prop('title')).toEqual('Error!'); }); - test('Add button shown for users without ability to POST', async done => { - wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'TeamsList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'TeamsList', - el => el.state('hasContentLoading') === false - ); + test('Add button shown for users without ability to POST', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + expect(wrapper.find('ToolbarAddButton').length).toBe(1); - done(); }); - test('Add button hidden for users without ability to POST', async done => { + test('Add button hidden for users without ability to POST', async () => { TeamsAPI.readOptions = () => Promise.resolve({ data: { @@ -218,18 +233,12 @@ describe('', () => { }, }, }); - wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'TeamsList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'TeamsList', - el => el.state('hasContentLoading') === false - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + expect(wrapper.find('ToolbarAddButton').length).toBe(0); - done(); }); }); From 779d1908557a44643e52ca5b9452eac0591e6e41 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 21 Feb 2020 12:13:52 -0800 Subject: [PATCH 2/4] convert ProjectList to hooks --- .../Project/ProjectList/ProjectList.jsx | 381 ++++++++---------- .../Project/ProjectList/ProjectList.test.jsx | 298 +++++++------- 2 files changed, 310 insertions(+), 369 deletions(-) diff --git a/awx/ui_next/src/screens/Project/ProjectList/ProjectList.jsx b/awx/ui_next/src/screens/Project/ProjectList/ProjectList.jsx index 5d305fc448..8fbf43bc4a 100644 --- a/awx/ui_next/src/screens/Project/ProjectList/ProjectList.jsx +++ b/awx/ui_next/src/screens/Project/ProjectList/ProjectList.jsx @@ -1,10 +1,11 @@ -import React, { Component, Fragment } from 'react'; -import { withRouter } from 'react-router-dom'; +import React, { Fragment, useState, useEffect, useCallback } from 'react'; +import { useLocation, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Card, PageSection } from '@patternfly/react-core'; import { ProjectsAPI } from '@api'; +import useRequest, { useDeleteItems } from '@util/useRequest'; import AlertModal from '@components/AlertModal'; import DataListToolbar from '@components/DataListToolbar'; import ErrorDetail from '@components/ErrorDetail'; @@ -22,231 +23,179 @@ const QS_CONFIG = getQSConfig('project', { order_by: 'name', }); -class ProjectsList extends Component { - constructor(props) { - super(props); +function ProjectList({ i18n }) { + const location = useLocation(); + const match = useRouteMatch(); + const [selected, setSelected] = useState([]); - this.state = { - hasContentLoading: true, - contentError: null, - deletionError: null, + const { + result: { projects, itemCount, actions }, + error: contentError, + isLoading, + request: fetchProjects, + } = useRequest( + useCallback(async () => { + const params = parseQueryString(QS_CONFIG, location.search); + const [response, actionsResponse] = await Promise.all([ + ProjectsAPI.read(params), + ProjectsAPI.readOptions(), + ]); + return { + projects: response.data.results, + itemCount: response.data.count, + actions: actionsResponse.data.actions, + }; + }, [location]), + { projects: [], - selected: [], itemCount: 0, - actions: null, - }; - - this.handleSelectAll = this.handleSelectAll.bind(this); - this.handleSelect = this.handleSelect.bind(this); - this.handleProjectDelete = this.handleProjectDelete.bind(this); - this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); - this.loadProjects = this.loadProjects.bind(this); - } - - componentDidMount() { - this.loadProjects(); - } - - componentDidUpdate(prevProps) { - const { location } = this.props; - if (location !== prevProps.location) { - this.loadProjects(); + actions: {}, } - } + ); - handleSelectAll(isSelected) { - const { projects } = this.state; + useEffect(() => { + fetchProjects(); + }, [fetchProjects]); - const selected = isSelected ? [...projects] : []; - this.setState({ selected }); - } + const isAllSelected = + selected.length === projects.length && selected.length > 0; + const { + isLoading: isDeleteLoading, + deleteItems: deleteProjects, + deletionError, + clearDeletionError, + } = useDeleteItems( + useCallback(async () => { + return Promise.all(selected.map(({ id }) => ProjectsAPI.destroy(id))); + }, [selected]), + { + qsConfig: QS_CONFIG, + allItemsSelected: isAllSelected, + fetchItems: fetchProjects, + } + ); - handleSelect(row) { - const { selected } = this.state; + const handleProjectDelete = async () => { + await deleteProjects(); + setSelected([]); + }; + const hasContentLoading = isDeleteLoading || isLoading; + const canAdd = actions && actions.POST; + + const handleSelectAll = isSelected => { + setSelected(isSelected ? [...projects] : []); + }; + + const handleSelect = row => { if (selected.some(s => s.id === row.id)) { - this.setState({ selected: selected.filter(s => s.id !== row.id) }); + setSelected(selected.filter(s => s.id !== row.id)); } else { - this.setState({ selected: selected.concat(row) }); + setSelected(selected.concat(row)); } - } + }; - handleDeleteErrorClose() { - this.setState({ deletionError: null }); - } - - async handleProjectDelete() { - const { selected } = this.state; - - this.setState({ hasContentLoading: true }); - try { - await Promise.all( - selected.map(project => ProjectsAPI.destroy(project.id)) - ); - } catch (err) { - this.setState({ deletionError: err }); - } finally { - await this.loadProjects(); - } - } - - async loadProjects() { - 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 = ProjectsAPI.readOptions(); - } - - const promises = Promise.all([ProjectsAPI.read(params), optionsPromise]); - - this.setState({ contentError: null, hasContentLoading: true }); - try { - const [ - { - data: { count, results }, - }, - { - data: { actions }, - }, - ] = await promises; - this.setState({ - actions, - itemCount: count, - projects: results, - selected: [], - }); - } catch (err) { - this.setState({ contentError: err }); - } finally { - this.setState({ hasContentLoading: false }); - } - } - - render() { - const { - actions, - itemCount, - contentError, - hasContentLoading, - deletionError, - selected, - projects, - } = this.state; - const { match, i18n } = this.props; - - const canAdd = - actions && Object.prototype.hasOwnProperty.call(actions, 'POST'); - const isAllSelected = - selected.length > 0 && selected.length === projects.length; - - return ( - - - - ( - , - ] - : []), - , - ]} - /> - )} - renderItem={o => ( - row.id === o.id)} - onSelect={() => this.handleSelect(o)} - /> - )} - emptyStateControls={ - canAdd ? ( - - ) : null - } - /> - - - - {i18n._(t`Failed to delete one or more projects.`)} - - - - ); - } + return ( + + + + ( + , + ] + : []), + , + ]} + /> + )} + renderItem={o => ( + row.id === o.id)} + onSelect={() => handleSelect(o)} + /> + )} + emptyStateControls={ + canAdd ? ( + + ) : null + } + /> + + + + {i18n._(t`Failed to delete one or more projects.`)} + + + + ); } -export { ProjectsList as _ProjectsList }; -export default withI18n()(withRouter(ProjectsList)); +export default withI18n()(ProjectList); diff --git a/awx/ui_next/src/screens/Project/ProjectList/ProjectList.test.jsx b/awx/ui_next/src/screens/Project/ProjectList/ProjectList.test.jsx index f9a0f55327..6fcebf02ea 100644 --- a/awx/ui_next/src/screens/Project/ProjectList/ProjectList.test.jsx +++ b/awx/ui_next/src/screens/Project/ProjectList/ProjectList.test.jsx @@ -1,8 +1,8 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { ProjectsAPI } from '@api'; -import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; - -import ProjectsList, { _ProjectsList } from './ProjectList'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import ProjectList from './ProjectList'; jest.mock('@api'); @@ -63,7 +63,7 @@ const mockProjects = [ }, ]; -describe('', () => { +describe('', () => { beforeEach(() => { ProjectsAPI.read.mockResolvedValue({ data: { @@ -86,117 +86,114 @@ describe('', () => { jest.clearAllMocks(); }); - test('initially renders successfully', () => { - mountWithContexts( - - ); - }); - - test('Projects are retrieved from the api and the components finishes loading', async done => { - const loadProjects = jest.spyOn(_ProjectsList.prototype, 'loadProjects'); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === true - ); - expect(loadProjects).toHaveBeenCalled(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === false - ); - done(); - }); - - test('handleSelect is called when a project list item is selected', async done => { - const handleSelect = jest.spyOn(_ProjectsList.prototype, 'handleSelect'); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === false - ); - await wrapper - .find('input#select-project-1') - .closest('DataListCheck') - .props() - .onChange(); - expect(handleSelect).toBeCalled(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('selected').length === 1 - ); - done(); - }); - - test('handleSelectAll is called when select all checkbox is clicked', async done => { - const handleSelectAll = jest.spyOn( - _ProjectsList.prototype, - 'handleSelectAll' - ); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === false - ); - wrapper - .find('Checkbox#select-all') - .props() - .onChange(true); - expect(handleSelectAll).toBeCalled(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('selected').length === 3 - ); - done(); - }); - - test('delete button is disabled if user does not have delete capabilities on a selected project', async done => { - const wrapper = mountWithContexts(); - wrapper.find('ProjectsList').setState({ - projects: mockProjects, - itemCount: 3, - isInitialized: true, - selected: mockProjects.slice(0, 1), + test('should load and render projects', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - await waitForElement( - wrapper, - 'ToolbarDeleteButton * button', - el => el.getDOMNode().disabled === false - ); - wrapper.find('ProjectsList').setState({ - selected: mockProjects, - }); - await waitForElement( - wrapper, - 'ToolbarDeleteButton * button', - el => el.getDOMNode().disabled === true - ); - done(); + wrapper.update(); + + expect(wrapper.find('ProjectListItem')).toHaveLength(3); }); - test('api is called to delete projects for each selected project.', () => { - ProjectsAPI.destroy = jest.fn(); - const wrapper = mountWithContexts(); - wrapper.find('ProjectsList').setState({ - projects: mockProjects, - itemCount: 2, - isInitialized: true, - isModalOpen: true, - selected: mockProjects.slice(0, 2), + test('should select project when checked', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); + wrapper.update(); + + await act(async () => { + wrapper + .find('ProjectListItem') + .first() + .invoke('onSelect')(); + }); + wrapper.update(); + + expect( + wrapper + .find('ProjectListItem') + .first() + .prop('isSelected') + ).toEqual(true); + }); + + test('should select all', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper.find('DataListToolbar').invoke('onSelectAll')(true); + }); + wrapper.update(); + + const items = wrapper.find('ProjectListItem'); + expect(items).toHaveLength(3); + items.forEach(item => { + expect(item.prop('isSelected')).toEqual(true); + }); + + expect( + wrapper + .find('ProjectListItem') + .first() + .prop('isSelected') + ).toEqual(true); + }); + + test('should disable delete button', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper + .find('ProjectListItem') + .at(2) + .invoke('onSelect')(); + }); + wrapper.update(); + + expect(wrapper.find('ToolbarDeleteButton button').prop('disabled')).toEqual( + true + ); + }); + + test('should call delete api', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper + .find('ProjectListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper + .find('ProjectListItem') + .at(1) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + expect(ProjectsAPI.destroy).toHaveBeenCalledTimes(2); }); - test('error is shown when project not successfully deleted from api', async done => { + test('should show deletion error', async () => { ProjectsAPI.destroy.mockRejectedValue( new Error({ response: { @@ -208,60 +205,55 @@ describe('', () => { }, }) ); - const wrapper = mountWithContexts(); - wrapper.find('ProjectsList').setState({ - projects: mockProjects, - itemCount: 1, - isInitialized: true, - isModalOpen: true, - selected: mockProjects.slice(0, 1), + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); - await waitForElement( - wrapper, - 'Modal', - el => el.props().isOpen === true && el.props().title === 'Error!' - ); + wrapper.update(); + expect(ProjectsAPI.read).toHaveBeenCalledTimes(1); + await act(async () => { + wrapper + .find('ProjectListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); - done(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + wrapper.update(); + + const modal = wrapper.find('Modal'); + expect(modal).toHaveLength(1); + expect(modal.prop('title')).toEqual('Error!'); }); - test('Add button shown for users without ability to POST', async done => { - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === false - ); + test('Add button shown for users without ability to POST', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + expect(wrapper.find('ToolbarAddButton').length).toBe(1); - done(); }); - test('Add button hidden for users without ability to POST', async done => { - ProjectsAPI.readOptions.mockResolvedValue({ - data: { - actions: { - GET: {}, + test('Add button hidden for users without ability to POST', async () => { + ProjectsAPI.readOptions = () => + Promise.resolve({ + data: { + actions: { + GET: {}, + }, }, - }, + }); + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'ProjectsList', - el => el.state('hasContentLoading') === false - ); + wrapper.update(); + expect(wrapper.find('ToolbarAddButton').length).toBe(0); - done(); }); }); From 8e9fc550f60e07d9810997ca4e3d2c60b4c3a12a Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 21 Feb 2020 12:32:29 -0800 Subject: [PATCH 3/4] convert InventoryList to hooks --- .../Inventory/InventoryList/InventoryList.jsx | 364 ++++++++---------- .../InventoryList/InventoryList.test.jsx | 303 +++++++-------- 2 files changed, 302 insertions(+), 365 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx index 39c7d503ec..4a5cf17947 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx @@ -1,11 +1,12 @@ -import React, { Component } from 'react'; -import { withRouter } from 'react-router-dom'; +import React, { useState, useCallback, useEffect } from 'react'; +import { useLocation, useRouteMatch } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Card, PageSection } from '@patternfly/react-core'; import { InventoriesAPI } from '@api'; +import useRequest, { useDeleteItems } from '@util/useRequest'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; import ErrorDetail from '@components/ErrorDetail'; @@ -23,225 +24,172 @@ const QS_CONFIG = getQSConfig('inventory', { order_by: 'name', }); -class InventoriesList extends Component { - constructor(props) { - super(props); +function InventoryList({ i18n }) { + const location = useLocation(); + const match = useRouteMatch(); + const [selected, setSelected] = useState([]); - this.state = { - hasContentLoading: true, - contentError: null, - deletionError: null, - selected: [], + const { + result: { inventories, itemCount, actions }, + error: contentError, + isLoading, + request: fetchInventories, + } = useRequest( + useCallback(async () => { + const params = parseQueryString(QS_CONFIG, location.search); + const [response, actionsResponse] = await Promise.all([ + InventoriesAPI.read(params), + InventoriesAPI.readOptions(), + ]); + return { + inventories: response.data.results, + itemCount: response.data.count, + actions: actionsResponse.data.actions, + }; + }, [location]), + { inventories: [], itemCount: 0, - }; - - this.loadInventories = this.loadInventories.bind(this); - this.handleSelectAll = this.handleSelectAll.bind(this); - this.handleSelect = this.handleSelect.bind(this); - this.handleInventoryDelete = this.handleInventoryDelete.bind(this); - this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); - } - - componentDidMount() { - this.loadInventories(); - } - - componentDidUpdate(prevProps) { - const { location } = this.props; - - if (location !== prevProps.location) { - this.loadInventories(); + actions: {}, } - } + ); - handleDeleteErrorClose() { - this.setState({ deletionError: null }); - } + useEffect(() => { + fetchInventories(); + }, [fetchInventories]); - handleSelectAll(isSelected) { - const { inventories } = this.state; - const selected = isSelected ? [...inventories] : []; - this.setState({ selected }); - } + const isAllSelected = + selected.length === inventories.length && selected.length > 0; + const { + isLoading: isDeleteLoading, + deleteItems: deleteTeams, + deletionError, + clearDeletionError, + } = useDeleteItems( + useCallback(async () => { + return Promise.all(selected.map(team => InventoriesAPI.destroy(team.id))); + }, [selected]), + { + qsConfig: QS_CONFIG, + allItemsSelected: isAllSelected, + fetchItems: fetchInventories, + } + ); - handleSelect(inventory) { - const { selected } = this.state; - if (selected.some(s => s.id === inventory.id)) { - this.setState({ selected: selected.filter(s => s.id !== inventory.id) }); + const handleInventoryDelete = async () => { + await deleteTeams(); + setSelected([]); + }; + + const hasContentLoading = isDeleteLoading || isLoading; + const canAdd = actions && actions.POST; + + const handleSelectAll = isSelected => { + setSelected(isSelected ? [...inventories] : []); + }; + + const handleSelect = row => { + if (selected.some(s => s.id === row.id)) { + setSelected(selected.filter(s => s.id !== row.id)); } else { - this.setState({ selected: selected.concat(inventory) }); + setSelected(selected.concat(row)); } - } + }; - async handleInventoryDelete() { - const { selected, itemCount } = this.state; - - this.setState({ hasContentLoading: true }); - try { - await Promise.all( - selected.map(({ id }) => { - return InventoriesAPI.destroy(id); - }) - ); - this.setState({ itemCount: itemCount - selected.length }); - } catch (err) { - this.setState({ deletionError: err }); - } finally { - await this.loadInventories(); - } - } - - async loadInventories() { - 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 = InventoriesAPI.readOptions(); - } - - const promises = Promise.all([InventoriesAPI.read(params), optionsPromise]); - - this.setState({ contentError: null, hasContentLoading: true }); - - try { - const [ + const addButton = ( + - ); - return ( - - - ( - , - ]} - /> - )} - renderItem={inventory => ( - this.handleSelect(inventory)} - isSelected={selected.some(row => row.id === inventory.id)} - /> - )} - emptyStateControls={canAdd && addButton} - /> - - - {i18n._(t`Failed to delete one or more inventories.`)} - - - - ); - } + ]} + /> + ); + return ( + + + ( + , + ]} + /> + )} + renderItem={inventory => ( + handleSelect(inventory)} + isSelected={selected.some(row => row.id === inventory.id)} + /> + )} + emptyStateControls={canAdd && addButton} + /> + + + {i18n._(t`Failed to delete one or more inventories.`)} + + + + ); } -export { InventoriesList as _InventoriesList }; -export default withI18n()(withRouter(InventoriesList)); +export default withI18n()(InventoryList); diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx index 796ae9eff1..ebe630f82c 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx @@ -1,8 +1,9 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { InventoriesAPI } from '@api'; -import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; -import InventoriesList, { _InventoriesList } from './InventoryList'; +import InventoryList from './InventoryList'; jest.mock('@api'); @@ -117,7 +118,7 @@ const mockInventories = [ }, ]; -describe('', () => { +describe('', () => { beforeEach(() => { InventoriesAPI.read.mockResolvedValue({ data: { @@ -140,186 +141,174 @@ describe('', () => { jest.clearAllMocks(); }); - test('initially renders successfully', () => { - mountWithContexts( - - ); - }); - - test('Inventories are retrieved from the api and the components finishes loading', async done => { - const loadInventories = jest.spyOn( - _InventoriesList.prototype, - 'loadInventories' - ); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === true - ); - expect(loadInventories).toHaveBeenCalled(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === false - ); - expect(wrapper.find('InventoryListItem').length).toBe(3); - done(); - }); - - test('handleSelect is called when a inventory list item is selected', async done => { - const handleSelect = jest.spyOn(_InventoriesList.prototype, 'handleSelect'); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === false - ); - await wrapper - .find('input#select-inventory-1') - .closest('DataListCheck') - .props() - .onChange(); - expect(handleSelect).toBeCalled(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('selected').length === 1 - ); - done(); - }); - - test('handleSelectAll is called when a inventory list item is selected', async done => { - const handleSelectAll = jest.spyOn( - _InventoriesList.prototype, - 'handleSelectAll' - ); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === false - ); - wrapper - .find('Checkbox#select-all') - .props() - .onChange(true); - expect(handleSelectAll).toBeCalled(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('selected').length === 3 - ); - done(); - }); - - test('delete button is disabled if user does not have delete capabilities on a selected inventory', async done => { - const wrapper = mountWithContexts(); - wrapper.find('InventoriesList').setState({ - inventories: mockInventories, - itemCount: 3, - isInitialized: true, - selected: mockInventories.slice(0, 2), + test('should load and render teams', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - await waitForElement( - wrapper, - 'ToolbarDeleteButton * button', - el => el.getDOMNode().disabled === false - ); - wrapper.find('InventoriesList').setState({ - selected: mockInventories, - }); - await waitForElement( - wrapper, - 'ToolbarDeleteButton * button', - el => el.getDOMNode().disabled === true - ); - done(); + wrapper.update(); + + expect(wrapper.find('InventoryListItem')).toHaveLength(3); }); - test('api is called to delete inventories for each selected inventory.', () => { - InventoriesAPI.destroy = jest.fn(); - const wrapper = mountWithContexts(); - wrapper.find('InventoriesList').setState({ - inventories: mockInventories, - itemCount: 3, - isInitialized: true, - isModalOpen: true, - selected: mockInventories.slice(0, 2), + test('should select team when checked', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); + wrapper.update(); + + await act(async () => { + wrapper + .find('InventoryListItem') + .first() + .invoke('onSelect')(); + }); + wrapper.update(); + + expect( + wrapper + .find('InventoryListItem') + .first() + .prop('isSelected') + ).toEqual(true); + }); + + test('should select all', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper.find('DataListToolbar').invoke('onSelectAll')(true); + }); + wrapper.update(); + + const items = wrapper.find('InventoryListItem'); + expect(items).toHaveLength(3); + items.forEach(item => { + expect(item.prop('isSelected')).toEqual(true); + }); + + expect( + wrapper + .find('InventoryListItem') + .first() + .prop('isSelected') + ).toEqual(true); + }); + + test('should disable delete button', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper + .find('InventoryListItem') + .at(2) + .invoke('onSelect')(); + }); + wrapper.update(); + + expect(wrapper.find('ToolbarDeleteButton button').prop('disabled')).toEqual( + true + ); + }); + + test('should call delete api', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + + await act(async () => { + wrapper + .find('InventoryListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper + .find('InventoryListItem') + .at(1) + .invoke('onSelect')(); + }); + wrapper.update(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + expect(InventoriesAPI.destroy).toHaveBeenCalledTimes(2); }); - test('error is shown when inventory not successfully deleted from api', async done => { + test('should show deletion error', async () => { InventoriesAPI.destroy.mockRejectedValue( new Error({ response: { config: { method: 'delete', - url: '/api/v2/inventories/1', + url: '/api/v2/teams/1', }, data: 'An error occurred', }, }) ); - const wrapper = mountWithContexts(); - wrapper.find('InventoriesList').setState({ - inventories: mockInventories, - itemCount: 1, - isInitialized: true, - isModalOpen: true, - selected: mockInventories.slice(0, 1), + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - wrapper.find('ToolbarDeleteButton').prop('onDelete')(); - await waitForElement( - wrapper, - 'Modal', - el => el.props().isOpen === true && el.props().title === 'Error!' - ); + wrapper.update(); + expect(InventoriesAPI.read).toHaveBeenCalledTimes(1); + await act(async () => { + wrapper + .find('InventoryListItem') + .at(0) + .invoke('onSelect')(); + }); + wrapper.update(); - done(); + await act(async () => { + wrapper.find('ToolbarDeleteButton').invoke('onDelete')(); + }); + wrapper.update(); + + const modal = wrapper.find('Modal'); + expect(modal).toHaveLength(1); + expect(modal.prop('title')).toEqual('Error!'); }); - test('Add button shown for users with ability to POST', async done => { - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === false - ); + test('Add button shown for users without ability to POST', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + expect(wrapper.find('ToolbarAddButton').length).toBe(1); - done(); }); - test('Add button hidden for users without ability to POST', async done => { - InventoriesAPI.readOptions.mockResolvedValue({ - data: { - actions: { - GET: {}, + test('Add button hidden for users without ability to POST', async () => { + InventoriesAPI.readOptions = () => + Promise.resolve({ + data: { + actions: { + GET: {}, + }, }, - }, + }); + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); }); - const wrapper = mountWithContexts(); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === true - ); - await waitForElement( - wrapper, - 'InventoriesList', - el => el.state('hasContentLoading') === false - ); + wrapper.update(); + expect(wrapper.find('ToolbarAddButton').length).toBe(0); - done(); }); }); From 6ffc78bcb0e759fd1f6e49449e02389a498a91e8 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Mon, 24 Feb 2020 14:05:10 -0800 Subject: [PATCH 4/4] add missing i18n; fix test names --- .../src/screens/Inventory/InventoryList/InventoryList.jsx | 2 +- .../screens/Inventory/InventoryList/InventoryList.test.jsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx index 4a5cf17947..e786181682 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.jsx @@ -157,7 +157,7 @@ function InventoryList({ i18n }) { key="delete" onDelete={handleInventoryDelete} itemsToDelete={selected} - pluralizedItemName="Inventories" + pluralizedItemName={i18n._(t`Inventories`)} />, ]} /> diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx index ebe630f82c..6caf0f0afd 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryList.test.jsx @@ -141,7 +141,7 @@ describe('', () => { jest.clearAllMocks(); }); - test('should load and render teams', async () => { + test('should load and render inventories', async () => { let wrapper; await act(async () => { wrapper = mountWithContexts(); @@ -151,7 +151,7 @@ describe('', () => { expect(wrapper.find('InventoryListItem')).toHaveLength(3); }); - test('should select team when checked', async () => { + test('should select inventory when checked', async () => { let wrapper; await act(async () => { wrapper = mountWithContexts(); @@ -254,7 +254,7 @@ describe('', () => { response: { config: { method: 'delete', - url: '/api/v2/teams/1', + url: '/api/v2/inventory/1', }, data: 'An error occurred', },