From 8d31d09d4af525635f5f63a907f3b3b5f8c47e0f Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Wed, 22 Apr 2020 09:16:00 -0400 Subject: [PATCH 1/2] Adds copy button to JTList --- awx/ui_next/src/api/models/JobTemplates.js | 4 ++ .../Template/TemplateList/CopyButton.jsx | 54 ++++++++++++++++++ .../Template/TemplateList/TemplateList.jsx | 1 + .../TemplateList/TemplateListItem.jsx | 57 ++++++++++++++----- awx/ui_next/src/util/dates.jsx | 12 ++++ 5 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index 0e2eba8079..03647de4b0 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -19,6 +19,10 @@ class JobTemplates extends SchedulesMixin( this.readWebhookKey = this.readWebhookKey.bind(this); } + copyTemplate(id, data) { + return this.http.post(`${this.baseUrl}${id}/copy/`, data); + } + launch(id, data) { return this.http.post(`${this.baseUrl}${id}/launch/`, data); } diff --git a/awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx b/awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx new file mode 100644 index 0000000000..50725ad232 --- /dev/null +++ b/awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx @@ -0,0 +1,54 @@ +import React, { useCallback, useEffect } from 'react'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; + +import { Button, Tooltip } from '@patternfly/react-core'; +import { CopyIcon } from '@patternfly/react-icons'; +import useRequest, { useDismissableError } from '@util/useRequest'; +import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; + +function CopyButton({ i18n, itemName, copyItem, disableButtons }) { + const { isLoading, error, request: copyTemplateToAPI } = useRequest( + useCallback(async () => { + await copyItem(); + + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []), + {} + ); + + useEffect(() => { + if (isLoading) { + return disableButtons(true); + } + return disableButtons(false); + }, [isLoading, disableButtons]); + + const { dismissError } = useDismissableError(error); + + return ( + <> + + + + dismissError} + > + {i18n._(t`Failed to copy ${itemName}.`)} + + + + ); +} +export default withI18n()(CopyButton); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 1f7b5554a4..21d7dc0206 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -227,6 +227,7 @@ function TemplateList({ i18n }) { detailUrl={`/templates/${template.type}/${template.id}`} onSelect={() => handleSelect(template)} isSelected={selected.some(row => row.id === template.id)} + fetchTemplates={fetchTemplates} /> )} emptyStateControls={(canAddJT || canAddWFJT) && addButton} diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx index 991f9b91c0..c82216421f 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useState } from 'react'; import { Link } from 'react-router-dom'; import { Button, @@ -18,11 +18,14 @@ import { PencilAltIcon, RocketIcon, } from '@patternfly/react-icons'; +import { timeOfDay } from '@util/dates'; +import { JobTemplatesAPI } from '@api'; import LaunchButton from '@components/LaunchButton'; import Sparkline from '@components/Sparkline'; import { toTitleCase } from '@util/strings'; import styled from 'styled-components'; +import CopyButton from './CopyButton'; const DataListAction = styled(_DataListAction)` align-items: center; @@ -31,7 +34,15 @@ const DataListAction = styled(_DataListAction)` grid-template-columns: repeat(2, 40px); `; -function TemplateListItem({ i18n, template, isSelected, onSelect, detailUrl }) { +function TemplateListItem({ + i18n, + template, + isSelected, + onSelect, + detailUrl, + fetchTemplates, +}) { + const [disableButtons, setDisableButtons] = useState(false); const labelId = `check-action-${template.id}`; const canLaunch = template.summary_fields.user_capabilities.start; @@ -40,11 +51,11 @@ function TemplateListItem({ i18n, template, isSelected, onSelect, detailUrl }) { (!template.summary_fields.project || (!template.summary_fields.inventory && !template.ask_inventory_on_launch)); - return ( {({ handleLaunch }) => ( - + <> + + + + {template.summary_fields.user_capabilities.copy && ( + { + await JobTemplatesAPI.copyTemplate(template.id, { + name: `${template.name}@${timeOfDay()}`, + }); + await fetchTemplates(); + }} + /> + )} + ) : ( '' )} diff --git a/awx/ui_next/src/util/dates.jsx b/awx/ui_next/src/util/dates.jsx index 68d9461334..ba4df777bf 100644 --- a/awx/ui_next/src/util/dates.jsx +++ b/awx/ui_next/src/util/dates.jsx @@ -17,6 +17,18 @@ export function secondsToHHMMSS(seconds) { return new Date(seconds * 1000).toISOString().substr(11, 8); } +export function timeOfDay() { + const date = new Date(); + const hour = date.getHours(); + const minute = prependZeros(date.getMinutes()); + const second = prependZeros(date.getSeconds()); + const time = + hour > 12 + ? `${hour - 12}:${minute} :${second} PM` + : `${hour}:${minute}:${second}`; + return time; +} + export function dateToInputDateTime(dateObj) { // input type="date-time" expects values to be formatted // like: YYYY-MM-DDTHH-MM-SS From 008cd9985a27c8e39ac15303462a4421aac32992 Mon Sep 17 00:00:00 2001 From: Alex Corey Date: Thu, 30 Apr 2020 12:25:48 -0400 Subject: [PATCH 2/2] Adds Copy Button component --- awx/ui_next/src/api/Base.js | 4 + awx/ui_next/src/api/models/JobTemplates.js | 4 - .../src/components/CopyButton/CopyButton.jsx | 60 +++++++++++++ .../components/CopyButton/CopyButton.test.jsx | 36 ++++++++ .../src/components/CopyButton/index.js | 1 + .../CredentialList/CredentialList.jsx | 2 + .../CredentialList/CredentialList.test.jsx | 2 +- .../CredentialList/CredentialListItem.jsx | 35 ++++++-- .../CredentialListItem.test.jsx | 53 ++++++++++++ .../Template/TemplateList/CopyButton.jsx | 54 ------------ .../Template/TemplateList/TemplateList.jsx | 1 + .../TemplateList/TemplateList.test.jsx | 18 +++- .../TemplateList/TemplateListItem.jsx | 84 ++++++++++--------- .../TemplateList/TemplateListItem.test.jsx | 53 ++++++++++++ 14 files changed, 304 insertions(+), 103 deletions(-) create mode 100644 awx/ui_next/src/components/CopyButton/CopyButton.jsx create mode 100644 awx/ui_next/src/components/CopyButton/CopyButton.test.jsx create mode 100644 awx/ui_next/src/components/CopyButton/index.js delete mode 100644 awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx diff --git a/awx/ui_next/src/api/Base.js b/awx/ui_next/src/api/Base.js index 33e3626994..fbb6182679 100644 --- a/awx/ui_next/src/api/Base.js +++ b/awx/ui_next/src/api/Base.js @@ -45,6 +45,10 @@ class Base { update(id, data) { return this.http.patch(`${this.baseUrl}${id}/`, data); } + + copy(id, data) { + return this.http.post(`${this.baseUrl}${id}/copy/`, data); + } } export default Base; diff --git a/awx/ui_next/src/api/models/JobTemplates.js b/awx/ui_next/src/api/models/JobTemplates.js index 03647de4b0..0e2eba8079 100644 --- a/awx/ui_next/src/api/models/JobTemplates.js +++ b/awx/ui_next/src/api/models/JobTemplates.js @@ -19,10 +19,6 @@ class JobTemplates extends SchedulesMixin( this.readWebhookKey = this.readWebhookKey.bind(this); } - copyTemplate(id, data) { - return this.http.post(`${this.baseUrl}${id}/copy/`, data); - } - launch(id, data) { return this.http.post(`${this.baseUrl}${id}/launch/`, data); } diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.jsx new file mode 100644 index 0000000000..500c0c1106 --- /dev/null +++ b/awx/ui_next/src/components/CopyButton/CopyButton.jsx @@ -0,0 +1,60 @@ +import React, { useEffect } from 'react'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; +import PropTypes from 'prop-types'; + +import { Button, Tooltip } from '@patternfly/react-core'; +import { CopyIcon } from '@patternfly/react-icons'; +import useRequest, { useDismissableError } from '@util/useRequest'; +import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; + +function CopyButton({ i18n, copyItem, onLoading, onDoneLoading, helperText }) { + const { isLoading, error: copyError, request: copyItemToAPI } = useRequest( + copyItem + ); + + useEffect(() => { + if (isLoading) { + return onLoading(); + } + return onDoneLoading(); + }, [isLoading, onLoading, onDoneLoading]); + + const { error, dismissError } = useDismissableError(copyError); + + return ( + <> + + + + + {helperText.errorMessage} + + + + ); +} + +CopyButton.propTypes = { + copyItem: PropTypes.func.isRequired, + onLoading: PropTypes.func.isRequired, + onDoneLoading: PropTypes.func.isRequired, + helperText: PropTypes.shape({ + tooltip: PropTypes.string.isRequired, + errorMessage: PropTypes.string.isRequired, + }).isRequired, +}; +export default withI18n()(CopyButton); diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx new file mode 100644 index 0000000000..30708ad16f --- /dev/null +++ b/awx/ui_next/src/components/CopyButton/CopyButton.test.jsx @@ -0,0 +1,36 @@ +import React from 'react'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; +import CopyButton from './CopyButton'; + +jest.mock('@api'); + +describe('', () => { + test('shold mount properly', () => { + const wrapper = mountWithContexts( + {}} + onDoneLoading={() => {}} + copyItem={() => {}} + helperText={{ + tooltip: `Copy Template`, + errorMessage: `Failed to copy template.`, + }} + /> + ); + expect(wrapper.find('CopyButton').length).toBe(1); + }); + test('should render proper tooltip', () => { + const wrapper = mountWithContexts( + {}} + onDoneLoading={() => {}} + copyItem={() => {}} + helperText={{ + tooltip: `Copy Template`, + errorMessage: `Failed to copy template.`, + }} + /> + ); + expect(wrapper.find('Tooltip').prop('content')).toBe('Copy Template'); + }); +}); diff --git a/awx/ui_next/src/components/CopyButton/index.js b/awx/ui_next/src/components/CopyButton/index.js new file mode 100644 index 0000000000..90e9e6d204 --- /dev/null +++ b/awx/ui_next/src/components/CopyButton/index.js @@ -0,0 +1 @@ +export { default } from './CopyButton'; diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx index 92a45eb677..56fda42cf2 100644 --- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx +++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialList.jsx @@ -106,6 +106,7 @@ function CredentialList({ i18n }) { row.id === item.id)} onSelect={() => handleSelect(item)} @@ -134,6 +135,7 @@ function CredentialList({ i18n }) { /> ', () => { }); await waitForElement( wrapper, - 'Modal', + 'Modal[aria-label="Deletion Error"]', el => el.props().isOpen === true && el.props().title === 'Error!' ); await act(async () => { diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx index 5ef3a86c51..dd96480f00 100644 --- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx +++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.jsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useState, useCallback } from 'react'; import { string, bool, func } from 'prop-types'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -13,16 +13,19 @@ import { Tooltip, } from '@patternfly/react-core'; import DataListCell from '@components/DataListCell'; +import { timeOfDay } from '@util/dates'; import { PencilAltIcon } from '@patternfly/react-icons'; import { Credential } from '@types'; +import { CredentialsAPI } from '@api'; import styled from 'styled-components'; +import CopyButton from '@components/CopyButton'; const DataListAction = styled(_DataListAction)` align-items: center; display: grid; grid-gap: 16px; - grid-template-columns: 40px; + grid-template-columns: repeat(2, 40px); `; function CredentialListItem({ @@ -31,10 +34,20 @@ function CredentialListItem({ isSelected, onSelect, i18n, + fetchCredentials, }) { + const [isDisabled, setIsDisabled] = useState(false); + const labelId = `check-action-${credential.id}`; const canEdit = credential.summary_fields.user_capabilities.edit; + const copyCredential = useCallback(async () => { + await CredentialsAPI.copy(credential.id, { + name: `${credential.name} @ ${timeOfDay()}`, + }); + await fetchCredentials(); + }, [credential.id, credential.name, fetchCredentials]); + return ( - {canEdit ? ( + {canEdit && ( - ) : ( - '' + )} + {credential.summary_fields.user_capabilities.copy && ( + setIsDisabled(true)} + onDoneLoading={() => setIsDisabled(false)} + copyItem={copyCredential} + helperText={{ + tooltip: i18n._(t`Copy Credential`), + errorMessage: i18n._(t`Failed to copy credential.`), + }} + /> )} diff --git a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx index 0f23bf465d..66e85007c0 100644 --- a/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx +++ b/awx/ui_next/src/screens/Credential/CredentialList/CredentialListItem.test.jsx @@ -1,7 +1,11 @@ import React from 'react'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { CredentialListItem } from '.'; +import { act } from 'react-dom/test-utils'; import { mockCredentials } from '../shared'; +import { CredentialsAPI } from '@api'; + +jest.mock('@api'); describe('', () => { let wrapper; @@ -33,4 +37,53 @@ describe('', () => { ); expect(wrapper.find('PencilAltIcon').exists()).toBeFalsy(); }); + test('should call api to copy template', async () => { + CredentialsAPI.copy.mockResolvedValue(); + + wrapper = mountWithContexts( + {}} + /> + ); + + await act(async () => + wrapper.find('Button[aria-label="Copy"]').prop('onClick')() + ); + expect(CredentialsAPI.copy).toHaveBeenCalled(); + jest.clearAllMocks(); + }); + + test('should render proper alert modal on copy error', async () => { + CredentialsAPI.copy.mockRejectedValue(new Error()); + + wrapper = mountWithContexts( + {}} + credential={mockCredentials.results[0]} + /> + ); + await act(async () => + wrapper.find('Button[aria-label="Copy"]').prop('onClick')() + ); + wrapper.update(); + expect(wrapper.find('Modal').prop('isOpen')).toBe(true); + jest.clearAllMocks(); + }); + + test('should not render copy button', async () => { + wrapper = mountWithContexts( + {}} + credential={mockCredentials.results[1]} + /> + ); + expect(wrapper.find('CopyButton').length).toBe(0); + }); }); diff --git a/awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx b/awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx deleted file mode 100644 index 50725ad232..0000000000 --- a/awx/ui_next/src/screens/Template/TemplateList/CopyButton.jsx +++ /dev/null @@ -1,54 +0,0 @@ -import React, { useCallback, useEffect } from 'react'; -import { withI18n } from '@lingui/react'; -import { t } from '@lingui/macro'; - -import { Button, Tooltip } from '@patternfly/react-core'; -import { CopyIcon } from '@patternfly/react-icons'; -import useRequest, { useDismissableError } from '@util/useRequest'; -import AlertModal from '@components/AlertModal'; -import ErrorDetail from '@components/ErrorDetail'; - -function CopyButton({ i18n, itemName, copyItem, disableButtons }) { - const { isLoading, error, request: copyTemplateToAPI } = useRequest( - useCallback(async () => { - await copyItem(); - - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []), - {} - ); - - useEffect(() => { - if (isLoading) { - return disableButtons(true); - } - return disableButtons(false); - }, [isLoading, disableButtons]); - - const { dismissError } = useDismissableError(error); - - return ( - <> - - - - dismissError} - > - {i18n._(t`Failed to copy ${itemName}.`)} - - - - ); -} -export default withI18n()(CopyButton); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 21d7dc0206..b70e34304d 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -234,6 +234,7 @@ function TemplateList({ i18n }) { /> ', () => { .find('button[aria-label="confirm delete"]') .prop('onClick')(); }); + await waitForElement( wrapper, - 'Modal', + 'Modal[aria-label="Deletion Error"]', el => el.props().isOpen === true && el.props().title === 'Error!' ); }); + test('should properly copy template', async () => { + JobTemplatesAPI.copy.mockResolvedValue({}); + const wrapper = mountWithContexts(); + await act(async () => { + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); + }); + await act(async () => + wrapper.find('Button[aria-label="Copy"]').prop('onClick')() + ); + expect(JobTemplatesAPI.copy).toHaveBeenCalled(); + expect(UnifiedJobTemplatesAPI.read).toHaveBeenCalled(); + wrapper.update(); + }); }); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx index c82216421f..b9dc712eec 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.jsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useCallback } from 'react'; import { Link } from 'react-router-dom'; import { Button, @@ -20,18 +20,18 @@ import { } from '@patternfly/react-icons'; import { timeOfDay } from '@util/dates'; -import { JobTemplatesAPI } from '@api'; +import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '@api'; import LaunchButton from '@components/LaunchButton'; import Sparkline from '@components/Sparkline'; import { toTitleCase } from '@util/strings'; import styled from 'styled-components'; -import CopyButton from './CopyButton'; +import CopyButton from '@components/CopyButton'; const DataListAction = styled(_DataListAction)` align-items: center; display: grid; grid-gap: 16px; - grid-template-columns: repeat(2, 40px); + grid-template-columns: repeat(3, 40px); `; function TemplateListItem({ @@ -42,10 +42,24 @@ function TemplateListItem({ detailUrl, fetchTemplates, }) { - const [disableButtons, setDisableButtons] = useState(false); + const [isDisabled, setIsDisabled] = useState(false); + const labelId = `check-action-${template.id}`; const canLaunch = template.summary_fields.user_capabilities.start; + const copyTemplate = useCallback(async () => { + if (template.type === 'job_template') { + await JobTemplatesAPI.copy(template.id, { + name: `${template.name} @ ${timeOfDay()}`, + }); + } else { + await WorkflowJobTemplatesAPI.copy(template.id, { + name: `${template.name} @ ${timeOfDay()}`, + }); + } + await fetchTemplates(); + }, [fetchTemplates, template.id, template.name, template.type]); + const missingResourceIcon = template.type === 'job_template' && (!template.summary_fields.project || @@ -55,7 +69,7 @@ function TemplateListItem({ {({ handleLaunch }) => ( - - {template.summary_fields.user_capabilities.copy && ( - { - await JobTemplatesAPI.copyTemplate(template.id, { - name: `${template.name}@${timeOfDay()}`, - }); - await fetchTemplates(); - }} - /> - )} - - ) : ( - '' + {template.summary_fields.user_capabilities.edit && ( + + + + )} + {template.summary_fields.user_capabilities.copy && ( + setIsDisabled(true)} + onDoneLoading={() => setIsDisabled(false)} + copyItem={copyTemplate} + /> )} diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.test.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.test.jsx index df84a79cec..402c78f1ea 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.test.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateListItem.test.jsx @@ -2,8 +2,13 @@ import React from 'react'; import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { createMemoryHistory } from 'history'; +import { JobTemplatesAPI } from '@api'; +import { act } from 'react-dom/test-utils'; +import mockJobTemplateData from '../shared/data.job_template.json'; import TemplateListItem from './TemplateListItem'; +jest.mock('@api'); + describe('', () => { test('launch button shown to users with start capabilities', () => { const wrapper = mountWithContexts( @@ -186,4 +191,52 @@ describe('', () => { '/templates/job_template/1/details' ); }); + test('should call api to copy template', async () => { + JobTemplatesAPI.copy.mockResolvedValue(); + + const wrapper = mountWithContexts( + + ); + await act(async () => + wrapper.find('Button[aria-label="Copy"]').prop('onClick')() + ); + expect(JobTemplatesAPI.copy).toHaveBeenCalled(); + jest.clearAllMocks(); + }); + + test('should render proper alert modal on copy error', async () => { + JobTemplatesAPI.copy.mockRejectedValue(new Error()); + + const wrapper = mountWithContexts( + + ); + await act(async () => + wrapper.find('Button[aria-label="Copy"]').prop('onClick')() + ); + wrapper.update(); + expect(wrapper.find('Modal').prop('isOpen')).toBe(true); + jest.clearAllMocks(); + }); + + test('should not render copy button', async () => { + const wrapper = mountWithContexts( + + ); + expect(wrapper.find('CopyButton').length).toBe(0); + }); });