From f9fb9b120b5c46c86737aa703608a9cc8c432965 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 19 May 2020 13:33:32 -0400 Subject: [PATCH 1/2] Add inventory source detail sync button --- .../InventorySourceDetail.jsx | 131 ++++++++++++++++-- .../InventorySourceDetail.test.jsx | 106 ++++++++++---- .../InventorySourceListItem.jsx | 3 +- .../InventorySourceSyncButton.jsx | 48 ++++--- .../InventorySourceSyncButton.test.jsx | 9 +- 5 files changed, 238 insertions(+), 59 deletions(-) rename awx/ui_next/src/screens/Inventory/{InventorySources => shared}/InventorySourceSyncButton.jsx (66%) rename awx/ui_next/src/screens/Inventory/{InventorySources => shared}/InventorySourceSyncButton.test.jsx (95%) diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx index 8c790a8b71..0fc18e12c6 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx @@ -1,23 +1,33 @@ -import React, { useEffect, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { Link, useHistory } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; +import styled from 'styled-components'; +import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; -import { Button, Chip, List, ListItem } from '@patternfly/react-core'; +import { Button, Chip, List, ListItem, Tooltip } from '@patternfly/react-core'; import AlertModal from '../../../components/AlertModal'; import { CardBody, CardActionsRow } from '../../../components/Card'; import ChipGroup from '../../../components/ChipGroup'; import { VariablesDetail } from '../../../components/CodeMirrorInput'; +import ContentError from '../../../components/ContentError'; +import ContentLoading from '../../../components/ContentLoading'; import CredentialChip from '../../../components/CredentialChip'; import DeleteButton from '../../../components/DeleteButton'; +import InventorySourceSyncButton from '../shared/InventorySourceSyncButton'; import { DetailList, Detail, UserDateDetail, } from '../../../components/DetailList'; import ErrorDetail from '../../../components/ErrorDetail'; +import useRequest from '../../../util/useRequest'; import { InventorySourcesAPI } from '../../../api'; +const QuestionCircleIcon = styled(PFQuestionCircleIcon)` + margin-left: 10px; +`; + function InventorySourceDetail({ inventorySource, i18n }) { const { created, @@ -53,12 +63,28 @@ function InventorySourceDetail({ inventorySource, i18n }) { const history = useHistory(); const isMounted = useRef(null); + const { + result: sourceChoices, + error, + isLoading, + request: fetchSourceChoices, + } = useRequest( + useCallback(async () => { + const { data } = await InventorySourcesAPI.readOptions(); + return Object.assign( + ...data.actions.GET.source.choices.map(([key, val]) => ({ [key]: val })) + ); + }, []), + {} + ); + useEffect(() => { isMounted.current = true; + fetchSourceChoices(); return () => { isMounted.current = false; }; - }, []); + }, [fetchSourceChoices]); const handleDelete = async () => { try { @@ -68,9 +94,9 @@ function InventorySourceDetail({ inventorySource, i18n }) { InventorySourcesAPI.destroy(id), ]); history.push(`/inventories/inventory/${inventory.id}/sources`); - } catch (error) { + } catch (err) { if (isMounted.current) { - setDeletionError(error); + setDeletionError(err); } } }; @@ -90,24 +116,99 @@ function InventorySourceDetail({ inventorySource, i18n }) { ) { optionsList = ( - {overwrite && {i18n._(t`Overwrite`)}} - {overwrite_vars && ( - {i18n._(t`Overwrite variables`)} + {overwrite && ( + + {i18n._(t`Overwrite`)} + + {i18n._(t`If checked, any hosts and groups that were + previously present on the external source but are now removed + will be removed from the Tower inventory. Hosts and groups + that were not managed by the inventory source will be promoted + to the next manually created group or if there is no manually + created group to promote them into, they will be left in the "all" + default group for the inventory.`)} +
+
+ {i18n._(t`When not checked, local child + hosts and groups not found on the external source will remain + untouched by the inventory update process.`)} + + } + position="top" + > + + + + )} + {overwrite_vars && ( + + {i18n._(t`Overwrite variables`)} + + {i18n._(t`If checked, all variables for child groups + and hosts will be removed and replaced by those found + on the external source.`)} +
+
+ {i18n._(t`When not checked, a merge will be performed, + combining local variables with those found on the + external source.`)} + + } + position="top" + > + + + + )} + {update_on_launch && ( + + {i18n._(t`Update on launch`)} + + + + )} - {update_on_launch && {i18n._(t`Update on launch`)}} {update_on_project_update && ( - {i18n._(t`Update on project update`)} + + {i18n._(t`Update on project update`)} + + + + )} ); } + if (isLoading) { + return ; + } + + if (error) { + return ; + } + return ( - + {organization && ( )} - + )} + {user_capabilities?.start && ( + + )} {user_capabilities?.delete && ( { jest.clearAllMocks(); }); - test('should render expected details', () => { - wrapper = mountWithContexts( - - ); + test('should render expected details', async () => { + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); expect(wrapper.find('InventorySourceDetail')).toHaveLength(1); assertDetail(wrapper, 'Name', 'mock inv source'); assertDetail(wrapper, 'Description', 'mock description'); - assertDetail(wrapper, 'Source', 'scm'); + assertDetail(wrapper, 'Source', 'Sourced from a Project'); assertDetail(wrapper, 'Organization', 'Mock Org'); assertDetail(wrapper, 'Ansible environment', '/venv/custom'); assertDetail(wrapper, 'Project', 'Mock Project'); @@ -69,44 +96,51 @@ describe('InventorySourceDetail', () => { expect(wrapper.find('VariablesDetail').prop('value')).toEqual( '---\nfoo: bar' ); - expect( - wrapper - .find('Detail[label="Options"]') - .containsAllMatchingElements([ -
  • Overwrite
    • , -
  • Overwrite variables
    • , -
  • Update on launch
    • , -
  • Update on project update
    • , - ]) - ).toEqual(true); + wrapper.find('Detail[label="Options"] li').forEach(option => { + expect([ + 'Overwrite', + 'Overwrite variables', + 'Update on launch', + 'Update on project update', + ]).toContain(option.text()); + }); }); - test('should show edit and delete button for users with permissions', () => { - wrapper = mountWithContexts( - - ); + test('should display expected action buttons for users with permissions', async () => { + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); const editButton = wrapper.find('Button[aria-label="edit"]'); expect(editButton.text()).toEqual('Edit'); expect(editButton.prop('to')).toBe( '/inventories/inventory/2/source/123/edit' ); expect(wrapper.find('DeleteButton')).toHaveLength(1); + expect(wrapper.find('InventorySourceSyncButton')).toHaveLength(1); }); - test('should hide edit and delete button for users without permissions', () => { + test('should hide expected action buttons for users without permissions', async () => { const userCapabilities = { edit: false, delete: false, + start: false, }; const invSource = { ...mockInvSource, summary_fields: { ...userCapabilities }, }; - wrapper = mountWithContexts( - - ); + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); expect(wrapper.find('Button[aria-label="edit"]')).toHaveLength(0); expect(wrapper.find('DeleteButton')).toHaveLength(0); + expect(wrapper.find('InventorySourceSyncButton')).toHaveLength(0); }); test('expected api call is made for delete', async () => { @@ -135,13 +169,33 @@ describe('InventorySourceDetail', () => { ); }); + test('Content error shown for failed options request', async () => { + InventorySourcesAPI.readOptions.mockImplementationOnce(() => + Promise.reject(new Error()) + ); + expect(InventorySourcesAPI.readOptions).toHaveBeenCalledTimes(0); + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + expect(InventorySourcesAPI.readOptions).toHaveBeenCalledTimes(1); + await waitForElement(wrapper, 'ContentError', el => el.length === 1); + expect(wrapper.find('ContentError Title').text()).toEqual( + 'Something went wrong...' + ); + }); + test('Error dialog shown for failed deletion', async () => { InventorySourcesAPI.destroy.mockImplementationOnce(() => Promise.reject(new Error()) ); - wrapper = mountWithContexts( - - ); + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); expect(wrapper.find('Modal[title="Error!"]')).toHaveLength(0); await act(async () => { wrapper.find('DeleteButton').invoke('onConfirm')(); diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx index 948faf60ec..a85ea121d1 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceListItem.jsx @@ -14,8 +14,7 @@ import { } from '@patternfly/react-core'; import { PencilAltIcon } from '@patternfly/react-icons'; import StatusIcon from '../../../components/StatusIcon'; - -import InventorySourceSyncButton from './InventorySourceSyncButton'; +import InventorySourceSyncButton from '../shared/InventorySourceSyncButton'; function InventorySourceListItem({ source, diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSyncButton.jsx similarity index 66% rename from awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx rename to awx/ui_next/src/screens/Inventory/shared/InventorySourceSyncButton.jsx index 50ac9395fd..03e32442ed 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSyncButton.jsx @@ -9,7 +9,7 @@ import AlertModal from '../../../components/AlertModal/AlertModal'; import ErrorDetail from '../../../components/ErrorDetail/ErrorDetail'; import { InventoryUpdatesAPI, InventorySourcesAPI } from '../../../api'; -function InventorySourceSyncButton({ source, i18n }) { +function InventorySourceSyncButton({ source, icon, i18n }) { const { isLoading: startSyncLoading, error: startSyncError, @@ -43,21 +43,26 @@ function InventorySourceSyncButton({ source, i18n }) { }, [source.id]) ); - const { error, dismissError } = useDismissableError( - cancelSyncError || startSyncError - ); + const { + error: startError, + dismissError: dismissStartError, + } = useDismissableError(startSyncError); + const { + error: cancelError, + dismissError: dismissCancelError, + } = useDismissableError(cancelSyncError); return ( <> - {source.status === 'pending' ? ( + {['running', 'pending', 'updating'].includes(source.status) ? ( ) : ( @@ -65,24 +70,33 @@ function InventorySourceSyncButton({ source, i18n }) { )} - {error && ( + {startError && ( - {startSyncError - ? i18n._(t`Failed to sync inventory source.`) - : i18n._(t`Failed to cancel inventory source sync.`)} - + {i18n._(t`Failed to sync inventory source.`)} + + + )} + {cancelError && ( + + {i18n._(t`Failed to cancel inventory source sync.`)} + )} @@ -91,10 +105,12 @@ function InventorySourceSyncButton({ source, i18n }) { InventorySourceSyncButton.defaultProps = { source: {}, + icon: true, }; InventorySourceSyncButton.propTypes = { source: PropTypes.shape({}), + icon: PropTypes.bool, }; export default withI18n()(InventorySourceSyncButton); diff --git a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSyncButton.test.jsx similarity index 95% rename from awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx rename to awx/ui_next/src/screens/Inventory/shared/InventorySourceSyncButton.test.jsx index f4552be2f4..ad92975edd 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySources/InventorySourceSyncButton.test.jsx +++ b/awx/ui_next/src/screens/Inventory/shared/InventorySourceSyncButton.test.jsx @@ -25,18 +25,19 @@ describe('', () => { wrapper.unmount(); jest.clearAllMocks(); }); - test('should mount properly', async () => { + + test('should mount properly', () => { expect(wrapper.find('InventorySourceSyncButton').length).toBe(1); }); - test('should render start sync button', async () => { + test('should render start sync button', () => { expect(wrapper.find('SyncIcon').length).toBe(1); expect( wrapper.find('Button[aria-label="Start sync source"]').prop('isDisabled') ).toBe(false); }); - test('should render cancel sync button', async () => { + test('should render cancel sync button', () => { wrapper = mountWithContexts( ', () => { ); expect(InventorySourcesAPI.createSyncStart).toBeCalledWith(1); }); + test('should cancel sync properly', async () => { InventorySourcesAPI.readDetail.mockResolvedValue({ data: { summary_fields: { current_update: { id: 120 } } }, @@ -83,6 +85,7 @@ describe('', () => { expect(InventorySourcesAPI.readDetail).toBeCalledWith(1); expect(InventoryUpdatesAPI.createSyncCancel).toBeCalledWith(120); }); + test('should throw error on sync start properly', async () => { InventorySourcesAPI.createSyncStart.mockRejectedValueOnce( new Error({ From 902a31d0736fdd6e7049b4ddcfb9ee5a4b5e7231 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Thu, 21 May 2020 14:10:47 -0400 Subject: [PATCH 2/2] Switch tooltips to FieldTooltips --- .../InventorySourceDetail.jsx | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx index 0fc18e12c6..05dff6c141 100644 --- a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx +++ b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx @@ -2,10 +2,8 @@ import React, { useCallback, useEffect, useRef, useState } from 'react'; import { Link, useHistory } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import styled from 'styled-components'; -import { QuestionCircleIcon as PFQuestionCircleIcon } from '@patternfly/react-icons'; -import { Button, Chip, List, ListItem, Tooltip } from '@patternfly/react-core'; +import { Button, Chip, List, ListItem } from '@patternfly/react-core'; import AlertModal from '../../../components/AlertModal'; import { CardBody, CardActionsRow } from '../../../components/Card'; import ChipGroup from '../../../components/ChipGroup'; @@ -14,6 +12,7 @@ import ContentError from '../../../components/ContentError'; import ContentLoading from '../../../components/ContentLoading'; import CredentialChip from '../../../components/CredentialChip'; import DeleteButton from '../../../components/DeleteButton'; +import { FieldTooltip } from '../../../components/FormField'; import InventorySourceSyncButton from '../shared/InventorySourceSyncButton'; import { DetailList, @@ -24,10 +23,6 @@ import ErrorDetail from '../../../components/ErrorDetail'; import useRequest from '../../../util/useRequest'; import { InventorySourcesAPI } from '../../../api'; -const QuestionCircleIcon = styled(PFQuestionCircleIcon)` - margin-left: 10px; -`; - function InventorySourceDetail({ inventorySource, i18n }) { const { created, @@ -119,7 +114,7 @@ function InventorySourceDetail({ inventorySource, i18n }) { {overwrite && ( {i18n._(t`Overwrite`)} - {i18n._(t`If checked, any hosts and groups that were @@ -136,16 +131,13 @@ function InventorySourceDetail({ inventorySource, i18n }) { untouched by the inventory update process.`)} } - position="top" - > - - + /> )} {overwrite_vars && ( {i18n._(t`Overwrite variables`)} - {i18n._(t`If checked, all variables for child groups @@ -158,37 +150,28 @@ function InventorySourceDetail({ inventorySource, i18n }) { external source.`)} } - position="top" - > - - + /> )} {update_on_launch && ( {i18n._(t`Update on launch`)} - - - + /> )} {update_on_project_update && ( {i18n._(t`Update on project update`)} - - - + /> )}