From fa7a459e5082a4753946b03c61c3395c4bc0d911 Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 26 May 2021 16:32:22 -0400 Subject: [PATCH] Fixes bug where user/team role add modal state is not cleared on close --- .../components/AddRole/SelectResourceStep.jsx | 4 +- .../CheckboxListItem/CheckboxListItem.jsx | 2 +- .../UserAndTeamAccessAdd.jsx | 87 +++++-------------- .../UserAndTeamAccessAdd.test.jsx | 30 +++---- .../screens/Team/TeamRoles/TeamRolesList.jsx | 38 ++++++-- .../screens/User/UserRoles/UserRolesList.jsx | 36 ++++++-- 6 files changed, 101 insertions(+), 96 deletions(-) diff --git a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx index 7489b707a8..6a99f3234b 100644 --- a/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx +++ b/awx/ui_next/src/components/AddRole/SelectResourceStep.jsx @@ -101,7 +101,9 @@ function SelectResourceStep({ headerRow={ {sortColumns.map(({ name, key }) => ( - {name} + + {name} + ))} } diff --git a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx index c76a542ea2..9ede0807da 100644 --- a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx +++ b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx @@ -33,7 +33,7 @@ const CheckboxListItem = ({ {columns?.length > 0 ? ( columns.map(col => ( - + {item[col.key]} )) diff --git a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.jsx b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.jsx index 09cd75c868..b4f0fd46cc 100644 --- a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.jsx +++ b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.jsx @@ -1,14 +1,9 @@ -import React, { useState, useCallback, useEffect, useContext } from 'react'; - +import React, { useState, useCallback } from 'react'; import { t } from '@lingui/macro'; import { useParams } from 'react-router-dom'; import styled from 'styled-components'; -import { Button, DropdownItem } from '@patternfly/react-core'; -import { KebabifiedContext } from '../../contexts/Kebabified'; -import useRequest, { useDismissableError } from '../../util/useRequest'; +import useRequest from '../../util/useRequest'; import SelectableCard from '../SelectableCard'; -import AlertModal from '../AlertModal'; -import ErrorDetail from '../ErrorDetail'; import Wizard from '../Wizard/Wizard'; import useSelected from '../../util/useSelected'; import SelectResourceStep from '../AddRole/SelectResourceStep'; @@ -22,21 +17,20 @@ const Grid = styled.div` grid-template-columns: repeat(auto-fill, minmax(250px, 1fr)); `; -function UserAndTeamAccessAdd({ title, onFetchData, apiModel }) { +function UserAndTeamAccessAdd({ + title, + onFetchData, + apiModel, + onClose, + onError, +}) { const [selectedResourceType, setSelectedResourceType] = useState(null); - const [isWizardOpen, setIsWizardOpen] = useState(false); const [stepIdReached, setStepIdReached] = useState(1); const { id: userId } = useParams(); - const { isKebabified, onKebabModalChange } = useContext(KebabifiedContext); const { selected: resourcesSelected, handleSelect: handleResourceSelect, } = useSelected([]); - useEffect(() => { - if (isKebabified) { - onKebabModalChange(isWizardOpen); - } - }, [isKebabified, isWizardOpen, onKebabModalChange]); const { selected: rolesSelected, @@ -60,13 +54,10 @@ function UserAndTeamAccessAdd({ title, onFetchData, apiModel }) { await Promise.all(roleRequests); onFetchData(); - setIsWizardOpen(false); }, [onFetchData, rolesSelected, apiModel, userId, resourcesSelected]), {} ); - const { error, dismissError } = useDismissableError(saveError); - const steps = [ { id: 1, @@ -128,56 +119,22 @@ function UserAndTeamAccessAdd({ title, onFetchData, apiModel }) { }, ]; - if (error) { - return ( - - {t`Failed to associate role`} - - - ); + if (saveError) { + onError(saveError); + onClose(); } return ( - <> - {isKebabified ? ( - setIsWizardOpen(true)} - > - {t`Add`} - - ) : ( - - )} - {isWizardOpen && ( - setIsWizardOpen(false)} - onNext={({ id }) => - setStepIdReached(stepIdReached < id ? id : stepIdReached) - } - onSave={handleWizardSave} - /> - )} - + + setStepIdReached(stepIdReached < id ? id : stepIdReached) + } + onSave={handleWizardSave} + /> ); } diff --git a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx index 343eca7126..ea0341c83c 100644 --- a/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx +++ b/awx/ui_next/src/components/UserAndTeamAccessAdd/UserAndTeamAccessAdd.test.jsx @@ -9,6 +9,9 @@ import UserAndTeamAccessAdd from './UserAndTeamAccessAdd'; jest.mock('../../api'); +const onError = jest.fn(); +const onClose = jest.fn(); + describe('', () => { const resources = { data: { @@ -57,24 +60,21 @@ describe('', () => { {}} + onClose={onClose} title="Add user permissions" + onError={onError} /> ); }); - await waitForElement(wrapper, 'Button[aria-label="Add"]'); + wrapper.update(); }); afterEach(() => { jest.resetAllMocks(); }); test('should mount properly', async () => { - expect(wrapper.find('Button[aria-label="Add"]').length).toBe(1); - act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')()); - wrapper.update(); expect(wrapper.find('PFWizard').length).toBe(1); }); test('should disable steps', async () => { - act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')()); - wrapper.update(); expect(wrapper.find('Button[type="submit"]').prop('isDisabled')).toBe(true); expect( wrapper @@ -122,8 +122,6 @@ describe('', () => { JobTemplatesAPI.read.mockResolvedValue(resources); JobTemplatesAPI.readOptions.mockResolvedValue(options); UsersAPI.associateRole.mockResolvedValue({}); - act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')()); - wrapper.update(); await act(async () => wrapper.find('SelectableCard[label="Job templates"]').prop('onClick')({ fetchItems: JobTemplatesAPI.read, @@ -179,15 +177,16 @@ describe('', () => { await expect(UsersAPI.associateRole).toHaveBeenCalled(); }); - test('should close wizard', async () => { - act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')()); + test('should close wizard on cancel', async () => { + await act(async () => + wrapper.find('Button[children="Cancel"]').prop('onClick')() + ); wrapper.update(); - act(() => wrapper.find('PFWizard').prop('onClose')()); - wrapper.update(); - expect(wrapper.find('PFWizard').length).toBe(0); + expect(onClose).toHaveBeenCalledTimes(1); }); test('should throw error', async () => { + expect(onError).toHaveBeenCalledTimes(0); JobTemplatesAPI.read.mockResolvedValue(resources); JobTemplatesAPI.readOptions.mockResolvedValue(options); UsersAPI.associateRole.mockRejectedValue( @@ -210,9 +209,6 @@ describe('', () => { }), })); - act(() => wrapper.find('Button[aria-label="Add"]').prop('onClick')()); - wrapper.update(); - await act(async () => wrapper.find('SelectableCard[label="Job templates"]').prop('onClick')({ fetchItems: JobTemplatesAPI.read, @@ -261,6 +257,6 @@ describe('', () => { await expect(UsersAPI.associateRole).toHaveBeenCalled(); wrapper.update(); - expect(wrapper.find('AlertModal').length).toBe(1); + expect(onError).toHaveBeenCalled(); }); }); diff --git a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx index fbe9c99063..809ec843ae 100644 --- a/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx +++ b/awx/ui_next/src/screens/Team/TeamRoles/TeamRolesList.jsx @@ -1,8 +1,6 @@ import React, { useCallback, useEffect, useState } from 'react'; import { useLocation } from 'react-router-dom'; - import { t } from '@lingui/macro'; - import { Button, EmptyState, @@ -22,6 +20,7 @@ import { getQSConfig, parseQueryString } from '../../../util/qs'; import ErrorDetail from '../../../components/ErrorDetail'; import AlertModal from '../../../components/AlertModal'; import TeamRoleListItem from './TeamRoleListItem'; +import { ToolbarAddButton } from '../../../components/PaginatedDataList'; import UserAndTeamAccessAdd from '../../../components/UserAndTeamAccessAdd/UserAndTeamAccessAdd'; const QS_CONFIG = getQSConfig('roles', { @@ -33,6 +32,8 @@ const QS_CONFIG = getQSConfig('roles', { function TeamRolesList({ me, team }) { const { search } = useLocation(); const [roleToDisassociate, setRoleToDisassociate] = useState(null); + const [showAddModal, setShowAddModal] = useState(false); + const [associateError, setAssociateError] = useState(null); const { isLoading, @@ -165,10 +166,10 @@ function TeamRolesList({ me, team }) { additionalControls={[ ...(canAdd ? [ - setShowAddModal(true)} />, ] : []), @@ -192,7 +193,18 @@ function TeamRolesList({ me, team }) { /> )} /> - + {showAddModal && ( + { + setShowAddModal(false); + fetchRoles(); + }} + title={t`Add team permissions`} + onClose={() => setShowAddModal(false)} + onError={err => setAssociateError(err)} + /> + )} {roleToDisassociate && ( )} + {associateError && ( + setAssociateError(null)} + > + {t`Failed to associate role`} + + + )} {disassociationError && ( setShowAddModal(true)} />, ] : []), @@ -189,6 +191,18 @@ function UserRolesList({ user }) { /> )} /> + {showAddModal && ( + { + setShowAddModal(false); + fetchRoles(); + }} + title={t`Add user permissions`} + onClose={() => setShowAddModal(false)} + onError={err => setAssociateError(err)} + /> + )} {roleToDisassociate && ( )} + {associateError && ( + setAssociateError(null)} + > + {t`Failed to associate role`} + + + )} {disassociationError && (