From a691caf34660cd8d3f1ce4f6f57d97427f4b8082 Mon Sep 17 00:00:00 2001 From: mabashian Date: Tue, 9 Feb 2021 11:22:25 -0500 Subject: [PATCH 1/2] Disable inventory copy button when inventory has sources. Refactor copy button props since tooltip is now handled by the ActionItem component. --- .../src/components/CopyButton/CopyButton.jsx | 31 +++++----- .../components/CopyButton/CopyButton.test.jsx | 58 +++++++++++-------- .../components/PaginatedTable/ActionItem.jsx | 2 +- .../PaginatedTable/ActionItem.test.jsx | 2 +- .../TemplateList/TemplateListItem.jsx | 10 ++-- .../CredentialList/CredentialListItem.jsx | 10 ++-- .../InventoryList/InventoryListItem.jsx | 21 +++---- .../NotificationTemplateListItem.jsx | 19 +++--- .../Project/ProjectList/ProjectListItem.jsx | 10 ++-- 9 files changed, 83 insertions(+), 80 deletions(-) diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.jsx index 30927f22c5..2856c69c0c 100644 --- a/awx/ui_next/src/components/CopyButton/CopyButton.jsx +++ b/awx/ui_next/src/components/CopyButton/CopyButton.jsx @@ -3,7 +3,7 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import PropTypes from 'prop-types'; -import { Button, Tooltip } from '@patternfly/react-core'; +import { Button } from '@patternfly/react-core'; import { CopyIcon } from '@patternfly/react-icons'; import useRequest, { useDismissableError } from '../../util/useRequest'; import AlertModal from '../AlertModal'; @@ -15,7 +15,7 @@ function CopyButton({ isDisabled, onCopyStart, onCopyFinish, - helperText, + errorMessage, i18n, }) { const { isLoading, error: copyError, request: copyItemToAPI } = useRequest( @@ -33,17 +33,15 @@ function CopyButton({ return ( <> - - - + - {helperText.errorMessage} + {errorMessage} @@ -62,10 +60,7 @@ CopyButton.propTypes = { copyItem: PropTypes.func.isRequired, onCopyStart: PropTypes.func.isRequired, onCopyFinish: PropTypes.func.isRequired, - helperText: PropTypes.shape({ - tooltip: PropTypes.string.isRequired, - errorMessage: PropTypes.string.isRequired, - }).isRequired, + errorMessage: PropTypes.string.isRequired, isDisabled: PropTypes.bool, }; diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx index f81c0eab1d..296f4aebf1 100644 --- a/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx +++ b/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx @@ -1,36 +1,44 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '../../../testUtils/enzymeHelpers'; import CopyButton from './CopyButton'; jest.mock('../../api'); +let wrapper; + describe('', () => { - test('shold mount properly', () => { - const wrapper = mountWithContexts( - {}} - onCopyFinish={() => {}} - copyItem={() => {}} - helperText={{ - tooltip: `Copy Template`, - errorMessage: `Failed to copy template.`, - }} - /> - ); + afterEach(() => { + wrapper.unmount(); + }); + test('should mount properly', async () => { + await act(async () => { + wrapper = mountWithContexts( + {}} + onCopyFinish={() => {}} + copyItem={() => {}} + errorMessage={`Failed to copy template.`} + /> + ); + }); expect(wrapper.find('CopyButton').length).toBe(1); }); - test('should render proper tooltip', () => { - const wrapper = mountWithContexts( - {}} - onCopyFinish={() => {}} - copyItem={() => {}} - helperText={{ - tooltip: `Copy Template`, - errorMessage: `Failed to copy template.`, - }} - /> - ); - expect(wrapper.find('Tooltip').prop('content')).toBe('Copy Template'); + test('should call the correct function on button click', async () => { + const copyItem = jest.fn(); + await act(async () => { + wrapper = mountWithContexts( + {}} + onCopyFinish={() => {}} + copyItem={copyItem} + errorMessage={`Failed to copy template.`} + /> + ); + }); + await act(async () => { + wrapper.find('button').simulate('click'); + }); + expect(copyItem).toHaveBeenCalledTimes(1); }); }); diff --git a/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx b/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx index a6c5e2b239..f9c423fee3 100644 --- a/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx +++ b/awx/ui_next/src/components/PaginatedTable/ActionItem.jsx @@ -14,7 +14,7 @@ export default function ActionItem({ column, tooltip, visible, children }) { `} > - {children} +
{children}
); diff --git a/awx/ui_next/src/components/PaginatedTable/ActionItem.test.jsx b/awx/ui_next/src/components/PaginatedTable/ActionItem.test.jsx index 202e556e83..d38653802f 100644 --- a/awx/ui_next/src/components/PaginatedTable/ActionItem.test.jsx +++ b/awx/ui_next/src/components/PaginatedTable/ActionItem.test.jsx @@ -12,7 +12,7 @@ describe('', () => { const tooltip = wrapper.find('Tooltip'); expect(tooltip.prop('content')).toEqual('a tooltip'); - expect(tooltip.prop('children')).toEqual('foo'); + expect(tooltip.prop('children')).toEqual(
foo
); }); test('should render null if not visible', async () => { diff --git a/awx/ui_next/src/components/TemplateList/TemplateListItem.jsx b/awx/ui_next/src/components/TemplateList/TemplateListItem.jsx index dc53622737..5638b8d123 100644 --- a/awx/ui_next/src/components/TemplateList/TemplateListItem.jsx +++ b/awx/ui_next/src/components/TemplateList/TemplateListItem.jsx @@ -177,13 +177,13 @@ function TemplateListItem({
- + - + diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx index 099fd78767..2b2f660efd 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx +++ b/awx/ui_next/src/screens/Inventory/InventoryList/InventoryListItem.jsx @@ -28,7 +28,7 @@ function InventoryListItem({ isSelected: bool.isRequired, onSelect: func.isRequired, }; - const [isDisabled, setIsDisabled] = useState(false); + const [isCopying, setIsCopying] = useState(false); const copyInventory = useCallback(async () => { await InventoriesAPI.copy(inventory.id, { @@ -38,11 +38,11 @@ function InventoryListItem({ }, [inventory.id, inventory.name, fetchInventories]); const handleCopyStart = useCallback(() => { - setIsDisabled(true); + setIsCopying(true); }, []); const handleCopyFinish = useCallback(() => { - setIsDisabled(false); + setIsCopying(false); }, []); const labelId = `check-action-${inventory.id}`; @@ -115,7 +115,7 @@ function InventoryListItem({ tooltip={i18n._(t`Edit Inventory`)} > - + From f6bddfd336b9b895aa413ef4788f652a5e6c4219 Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 15 Feb 2021 11:33:38 -0500 Subject: [PATCH 2/2] Fix linting error in CopyButton test --- awx/ui_next/src/components/CopyButton/CopyButton.test.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx index 296f4aebf1..894431bc2d 100644 --- a/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx +++ b/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx @@ -18,7 +18,7 @@ describe('', () => { onCopyStart={() => {}} onCopyFinish={() => {}} copyItem={() => {}} - errorMessage={`Failed to copy template.`} + errorMessage="Failed to copy template." /> ); }); @@ -32,7 +32,7 @@ describe('', () => { onCopyStart={() => {}} onCopyFinish={() => {}} copyItem={copyItem} - errorMessage={`Failed to copy template.`} + errorMessage="Failed to copy template." /> ); });