From a293a60d5c88bdc9e5aa9de73fb783d7671bd904 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 25 Feb 2021 16:07:55 -0500 Subject: [PATCH 01/14] Disable job templates in node modal that are missing inv or project --- .../CheckboxListItem/CheckboxListItem.jsx | 16 +++- .../Modals/NodeModals/NodeModal.test.jsx | 1 + .../NodeTypeStep/JobTemplatesList.jsx | 57 ++++++++++--- .../NodeTypeStep/JobTemplatesList.test.jsx | 80 +++++++++++++++++++ 4 files changed, 138 insertions(+), 16 deletions(-) diff --git a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx index befaa10674..6ac79cc811 100644 --- a/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx +++ b/awx/ui_next/src/components/CheckboxListItem/CheckboxListItem.jsx @@ -1,5 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; +import styled from 'styled-components'; import { DataListItem, DataListItemRow, @@ -9,6 +10,14 @@ import { } from '@patternfly/react-core'; import DataListCell from '../DataListCell'; +const Label = styled.label` + ${({ isDisabled }) => + isDisabled && + ` + opacity: 0.5; + `} +`; + const CheckboxListItem = ({ isDisabled = false, isRadio = false, @@ -32,7 +41,7 @@ const CheckboxListItem = ({ aria-label={`check-action-item-${itemId}`} aria-labelledby={`check-action-item-${itemId}`} checked={isSelected} - disabled={isDisabled} + isDisabled={isDisabled} id={`selected-${itemId}`} isChecked={isSelected} name={name} @@ -42,13 +51,14 @@ const CheckboxListItem = ({ - + , ]} /> diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx index 7bcf1ea6ee..946718079a 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeModal.test.jsx @@ -94,6 +94,7 @@ const mockJobTemplate = { }, related: { webhook_receiver: '' }, inventory: 1, + project: 5, }; describe('NodeModal', () => { diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.jsx index 3de9f280b5..77a6de336b 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.jsx @@ -3,6 +3,7 @@ import { useLocation } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { func, shape } from 'prop-types'; +import { Tooltip } from '@patternfly/react-core'; import { JobTemplatesAPI } from '../../../../../../api'; import { getQSConfig, parseQueryString } from '../../../../../../util/qs'; import useRequest from '../../../../../../util/useRequest'; @@ -56,26 +57,56 @@ function JobTemplatesList({ i18n, nodeResource, onUpdateNodeResource }) { fetchJobTemplates(); }, [fetchJobTemplates]); + const onSelectRow = row => { + if ( + row.project && + row.project !== null && + ((row.inventory && row.inventory !== null) || row.ask_inventory_on_launch) + ) { + onUpdateNodeResource(row); + } + }; + return ( onUpdateNodeResource(row)} + onRowClick={row => onSelectRow(row)} qsConfig={QS_CONFIG} - renderItem={item => ( - onUpdateNodeResource(item)} - onDeselect={() => onUpdateNodeResource(null)} - isRadio - /> - )} + renderItem={item => { + const isDisabled = + !item.project || + item.project === null || + ((!item.inventory || item.inventory === null) && + !item.ask_inventory_on_launch); + const listItem = ( + onSelectRow(item)} + onDeselect={() => onUpdateNodeResource(null)} + isRadio + /> + ); + return isDisabled ? ( + + {listItem} + + ) : ( + listItem + ); + }} renderToolbar={props => } showPageSizeOptions={false} toolbarSearchColumns={[ diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.test.jsx index 3b9fdec0e9..e59b9a7b48 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Modals/NodeModals/NodeTypeStep/JobTemplatesList.test.jsx @@ -16,6 +16,7 @@ const onUpdateNodeResource = jest.fn(); describe('JobTemplatesList', () => { let wrapper; afterEach(() => { + jest.clearAllMocks(); wrapper.unmount(); }); test('Row selected when nodeResource id matches row id and clicking new row makes expected callback', async () => { @@ -28,12 +29,16 @@ describe('JobTemplatesList', () => { name: 'Test Job Template', type: 'job_template', url: '/api/v2/job_templates/1', + inventory: 1, + project: 2, }, { id: 2, name: 'Test Job Template 2', type: 'job_template', url: '/api/v2/job_templates/2', + inventory: 1, + project: 2, }, ], }, @@ -60,10 +65,18 @@ describe('JobTemplatesList', () => { wrapper.find('CheckboxListItem[name="Test Job Template"]').props() .isSelected ).toBe(true); + expect( + wrapper.find('CheckboxListItem[name="Test Job Template"]').props() + .isDisabled + ).toBe(false); expect( wrapper.find('CheckboxListItem[name="Test Job Template 2"]').props() .isSelected ).toBe(false); + expect( + wrapper.find('CheckboxListItem[name="Test Job Template 2"]').props() + .isDisabled + ).toBe(false); wrapper .find('CheckboxListItem[name="Test Job Template 2"]') .simulate('click'); @@ -72,8 +85,75 @@ describe('JobTemplatesList', () => { name: 'Test Job Template 2', type: 'job_template', url: '/api/v2/job_templates/2', + inventory: 1, + project: 2, }); }); + test('Row disabled when job template missing inventory or project', async () => { + JobTemplatesAPI.read.mockResolvedValueOnce({ + data: { + count: 2, + results: [ + { + id: 1, + name: 'Test Job Template', + type: 'job_template', + url: '/api/v2/job_templates/1', + inventory: 1, + project: null, + ask_inventory_on_launch: false, + }, + { + id: 2, + name: 'Test Job Template 2', + type: 'job_template', + url: '/api/v2/job_templates/2', + inventory: null, + project: 2, + ask_inventory_on_launch: false, + }, + ], + }, + }); + JobTemplatesAPI.readOptions.mockResolvedValue({ + data: { + actions: { + GET: {}, + POST: {}, + }, + related_search_fields: [], + }, + }); + await act(async () => { + wrapper = mountWithContexts( + + ); + }); + wrapper.update(); + expect( + wrapper.find('CheckboxListItem[name="Test Job Template"]').props() + .isSelected + ).toBe(true); + expect( + wrapper.find('CheckboxListItem[name="Test Job Template"]').props() + .isDisabled + ).toBe(true); + expect( + wrapper.find('CheckboxListItem[name="Test Job Template 2"]').props() + .isSelected + ).toBe(false); + expect( + wrapper.find('CheckboxListItem[name="Test Job Template 2"]').props() + .isDisabled + ).toBe(true); + wrapper + .find('CheckboxListItem[name="Test Job Template 2"]') + .simulate('click'); + expect(onUpdateNodeResource).not.toHaveBeenCalled(); + }); test('Error shown when read() request errors', async () => { JobTemplatesAPI.read.mockRejectedValue(new Error()); JobTemplatesAPI.readOptions.mockResolvedValue({ From 1f93d3ad69b4927c7cf5867acb45db234aefed33 Mon Sep 17 00:00:00 2001 From: mabashian Date: Tue, 2 Mar 2021 14:27:48 -0500 Subject: [PATCH 02/14] Adds tests for workflow save error handling. Removes unnecessary code that was attempting to remove credentials from a new node. --- .../Visualizer.jsx | 20 +- .../Visualizer.test.jsx | 660 +++++++++++++++++- 2 files changed, 664 insertions(+), 16 deletions(-) diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.jsx index 1deb449890..5e0b8e2006 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.jsx @@ -64,10 +64,10 @@ const getAggregatedCredentials = ( templateDefaultCred.credential_type === overrideCred.credential_type ) { if ( - (!templateDefaultCred.vault_id && !overrideCred.inputs.vault_id) || + (!templateDefaultCred.vault_id && !overrideCred.inputs?.vault_id) || (templateDefaultCred.vault_id && - overrideCred.inputs.vault_id && - templateDefaultCred.vault_id === overrideCred.inputs.vault_id) + overrideCred.inputs?.vault_id && + templateDefaultCred.vault_id === overrideCred.inputs?.vault_id) ) { credentialHasOverride = true; } @@ -405,16 +405,7 @@ function Visualizer({ template, i18n }) { failure_nodes: [], always_nodes: [], }; - if (node.promptValues?.removedCredentials?.length > 0) { - node.promptValues.removedCredentials.forEach(cred => { - disassociateCredentialRequests.push( - WorkflowJobTemplateNodesAPI.disassociateCredentials( - data.id, - cred.id - ) - ); - }); - } + if (node.promptValues?.addedCredentials?.length > 0) { node.promptValues.addedCredentials.forEach(cred => { associateCredentialRequests.push( @@ -583,8 +574,9 @@ function Visualizer({ template, i18n }) { {i18n._(t`There was an error saving the workflow.`)} diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.test.jsx index 26770f16f9..c9a5795929 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateVisualizer/Visualizer.test.jsx @@ -2,13 +2,37 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; import { + WorkflowApprovalTemplatesAPI, WorkflowJobTemplateNodesAPI, WorkflowJobTemplatesAPI, } from '../../../api'; import Visualizer from './Visualizer'; +import workflowReducer from '../../../components/Workflow/workflowReducer'; + +jest.mock('../../../components/Workflow/workflowReducer'); + +const realWorkflowReducer = jest.requireActual( + '../../../components/Workflow/workflowReducer' +).default; + jest.mock('../../../api'); +const startNode = { + id: 1, + fullUnifiedJobTemplate: { + name: 'START', + }, +}; + +const defaultLinks = [ + { + linkType: 'always', + source: { id: 1 }, + target: { id: 2 }, + }, +]; + const template = { id: 1, name: 'Foo WFJT', @@ -117,7 +141,6 @@ describe('Visualizer', () => { }); afterAll(() => { - jest.clearAllMocks(); wrapper.unmount(); delete window.SVGElement.prototype.getBBox; delete window.SVGElement.prototype.getBoundingClientRect; @@ -125,6 +148,12 @@ describe('Visualizer', () => { delete window.SVGElement.prototype.width; }); + beforeEach(() => { + jest.clearAllMocks(); + jest.resetModules(); + workflowReducer.mockImplementation(realWorkflowReducer); + }); + test('Renders successfully', async () => { await act(async () => { wrapper = mountWithContexts( @@ -185,7 +214,7 @@ describe('Visualizer', () => { wrapper.find('button#link-confirm').simulate('click'); expect(wrapper.find('LinkEditModal').length).toBe(0); await act(async () => { - wrapper.find('button[aria-label="Save"]').simulate('click'); + wrapper.find('Button#visualizer-save').simulate('click'); }); expect( WorkflowJobTemplateNodesAPI.disassociateAlwaysNode @@ -219,6 +248,633 @@ describe('Visualizer', () => { ).toBe(true); }); + test('Error shown when saving fails due to node add error', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + fullUnifiedJobTemplate: { + id: 3, + name: 'PING', + type: 'job_template', + }, + }, + ]; + newState.links = defaultLinks; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplatesAPI.createNode.mockRejectedValue(new Error()); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect(WorkflowJobTemplatesAPI.createNode).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to node edit error', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + isEdited: true, + fullUnifiedJobTemplate: { + id: 3, + name: 'PING', + type: 'job_template', + }, + originalNodeObject: { + id: 9000, + }, + }, + ]; + newState.links = defaultLinks; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplateNodesAPI.replace.mockRejectedValue(new Error()); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect(WorkflowJobTemplateNodesAPI.replace).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to approval template add error', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + fullUnifiedJobTemplate: { + id: 3, + name: 'Approval', + timeout: 1000, + type: 'workflow_approval_template', + }, + }, + ]; + newState.links = defaultLinks; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplatesAPI.createNode.mockResolvedValue({ + data: { + id: 9001, + }, + }); + WorkflowJobTemplateNodesAPI.createApprovalTemplate.mockRejectedValue( + new Error() + ); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect(WorkflowJobTemplatesAPI.createNode).toHaveBeenCalledTimes(1); + expect( + WorkflowJobTemplateNodesAPI.createApprovalTemplate + ).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to approval template edit error', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + isEdited: true, + fullUnifiedJobTemplate: { + id: 3, + name: 'Approval', + timeout: 1000, + type: 'workflow_approval_template', + }, + originalNodeObject: { + id: 9000, + summary_fields: { + unified_job_template: { + unified_job_type: 'workflow_approval', + }, + }, + }, + }, + ]; + newState.links = defaultLinks; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowApprovalTemplatesAPI.update.mockRejectedValue(new Error()); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect(WorkflowApprovalTemplatesAPI.update).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to node disassociate failure', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + fullUnifiedJobTemplate: { + id: 3, + name: 'Approval', + timeout: 1000, + type: 'workflow_approval_template', + }, + originalNodeObject: { + id: 9000, + summary_fields: { + unified_job_template: { + unified_job_type: 'workflow_approval', + }, + }, + success_nodes: [], + failure_nodes: [3], + always_nodes: [], + }, + success_nodes: [3], + failure_nodes: [], + always_nodes: [], + }, + { + id: 3, + fullUnifiedJobTemplate: { + id: 4, + name: 'Approval 2', + timeout: 1000, + type: 'workflow_approval_template', + }, + originalNodeObject: { + id: 9001, + summary_fields: { + unified_job_template: { + unified_job_type: 'workflow_approval', + }, + }, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + ]; + newState.links = [ + { + linkType: 'always', + source: { id: 1 }, + target: { id: 2 }, + }, + { + linkType: 'success', + source: { id: 2 }, + target: { id: 3 }, + }, + ]; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplateNodesAPI.disassociateFailuresNode.mockRejectedValue( + new Error() + ); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect( + WorkflowJobTemplateNodesAPI.disassociateFailuresNode + ).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to node associate failure', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + fullUnifiedJobTemplate: { + id: 3, + name: 'Approval', + timeout: 1000, + type: 'workflow_approval_template', + }, + originalNodeObject: { + id: 9000, + summary_fields: { + unified_job_template: { + unified_job_type: 'workflow_approval', + }, + }, + success_nodes: [], + failure_nodes: [3], + always_nodes: [], + }, + success_nodes: [3], + failure_nodes: [], + always_nodes: [], + }, + { + id: 3, + fullUnifiedJobTemplate: { + id: 4, + name: 'Approval 2', + timeout: 1000, + type: 'workflow_approval_template', + }, + originalNodeObject: { + id: 9001, + summary_fields: { + unified_job_template: { + unified_job_type: 'workflow_approval', + }, + }, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + ]; + newState.links = [ + { + linkType: 'always', + source: { id: 1 }, + target: { id: 2 }, + }, + { + linkType: 'success', + source: { id: 2 }, + target: { id: 3 }, + }, + ]; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplateNodesAPI.disassociateFailuresNode.mockResolvedValue(); + WorkflowJobTemplateNodesAPI.associateSuccessNode.mockRejectedValue( + new Error() + ); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect( + WorkflowJobTemplateNodesAPI.associateSuccessNode + ).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to credential disassociate failure', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + isEdited: true, + fullUnifiedJobTemplate: { + id: 3, + name: 'Ping', + type: 'job_template', + }, + originalNodeObject: { + id: 9000, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + originalNodeCredentials: [ + { + id: 456, + credential_type: 1, + }, + ], + promptValues: { + credentials: [ + { + id: 123, + credential_type: 1, + }, + ], + }, + launchConfig: { + defaults: { + credentials: [ + { + id: 456, + credential_type: 1, + }, + ], + }, + }, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + ]; + newState.links = defaultLinks; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplateNodesAPI.replace.mockResolvedValue(); + WorkflowJobTemplateNodesAPI.disassociateCredentials.mockRejectedValue( + new Error() + ); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect( + WorkflowJobTemplateNodesAPI.disassociateCredentials + ).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + + test('Error shown when saving fails due to credential associate failure', async () => { + workflowReducer.mockImplementation(state => { + const newState = { + ...state, + isLoading: false, + }; + + if (newState.nodes.length === 0) { + newState.nodes = [ + startNode, + { + id: 2, + isEdited: true, + fullUnifiedJobTemplate: { + id: 3, + name: 'Ping', + type: 'job_template', + }, + originalNodeObject: { + id: 9000, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + originalNodeCredentials: [ + { + id: 456, + credential_type: 1, + }, + ], + promptValues: { + credentials: [ + { + id: 123, + credential_type: 1, + }, + ], + }, + launchConfig: { + defaults: { + credentials: [ + { + id: 456, + credential_type: 1, + }, + ], + }, + }, + success_nodes: [], + failure_nodes: [], + always_nodes: [], + }, + ]; + newState.links = defaultLinks; + } + + return newState; + }); + WorkflowJobTemplatesAPI.readNodes.mockResolvedValue({ + data: { + count: 0, + results: [], + }, + }); + WorkflowJobTemplateNodesAPI.replace.mockResolvedValue(); + WorkflowJobTemplateNodesAPI.disassociateCredentials.mockResolvedValue(); + WorkflowJobTemplateNodesAPI.associateCredentials.mockRejectedValue( + new Error() + ); + await act(async () => { + wrapper = mountWithContexts( + + + + ); + }); + wrapper.update(); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(0); + await act(async () => { + wrapper.find('Button#visualizer-save').simulate('click'); + }); + wrapper.update(); + expect( + WorkflowJobTemplateNodesAPI.associateCredentials + ).toHaveBeenCalledTimes(1); + expect( + wrapper.find('AlertModal[title="Error saving the workflow!"]').length + ).toBe(1); + }); + test('Error shown to user when error thrown fetching workflow nodes', async () => { WorkflowJobTemplatesAPI.readNodes.mockRejectedValue(new Error()); await act(async () => { From fbd46f77997b524b1fb7145e1c609995c3ce11cf Mon Sep 17 00:00:00 2001 From: nixocio Date: Thu, 4 Mar 2021 17:11:28 -0500 Subject: [PATCH 03/14] Fix diassociate EE from JT and WFJT Allow EE to be removed from JT and WFJT. Also, add unit-test related to those changes. See: https://github.com/ansible/awx/issues/9487 --- .../JobTemplateEdit/JobTemplateEdit.jsx | 7 +++--- .../JobTemplateEdit/JobTemplateEdit.test.jsx | 24 +++++++++++++++++++ .../WorkflowJobTemplateEdit.jsx | 7 +++--- .../WorkflowJobTemplateEdit.test.jsx | 24 +++++++++++++++++++ 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx index 6f109604b5..fa0ad134cb 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.jsx @@ -49,6 +49,7 @@ function JobTemplateEdit({ template }) { webhook_credential, webhook_key, webhook_url, + execution_environment, ...remainingValues } = values; @@ -56,11 +57,9 @@ function JobTemplateEdit({ template }) { setIsLoading(true); remainingValues.project = values.project.id; remainingValues.webhook_credential = webhook_credential?.id || null; + remainingValues.execution_environment = execution_environment?.id || null; try { - await JobTemplatesAPI.update(template.id, { - ...remainingValues, - execution_environment: values.execution_environment?.id, - }); + await JobTemplatesAPI.update(template.id, remainingValues); await Promise.all([ submitLabels(labels, template?.organization), submitInstanceGroups(instanceGroups, initialInstanceGroups), diff --git a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx index 642c679969..0836d35bd5 100644 --- a/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/JobTemplateEdit/JobTemplateEdit.test.jsx @@ -13,6 +13,7 @@ import { LabelsAPI, ProjectsAPI, InventoriesAPI, + ExecutionEnvironmentsAPI, } from '../../../api'; import JobTemplateEdit from './JobTemplateEdit'; @@ -49,6 +50,12 @@ const mockJobTemplate = { scm_branch: '', skip_tags: '', summary_fields: { + execution_environment: { + id: 1, + name: 'Default EE', + description: '', + image: 'quay.io/ansible/awx-ee', + }, user_capabilities: { edit: true, }, @@ -81,6 +88,7 @@ const mockJobTemplate = { related: { webhook_receiver: '/api/v2/workflow_job_templates/57/gitlab/', }, + execution_environment: 1, }; const mockRelatedCredentials = { @@ -176,6 +184,15 @@ const mockInstanceGroups = [ }, ]; +const mockExecutionEnvironment = [ + { + id: 1, + name: 'Default EE', + description: '', + image: 'quay.io/ansible/awx-ee', + }, +]; + JobTemplatesAPI.readCredentials.mockResolvedValue({ data: mockRelatedCredentials, }); @@ -197,6 +214,10 @@ CredentialsAPI.read.mockResolvedValue({ }); CredentialTypesAPI.loadAllTypes.mockResolvedValue([]); +ExecutionEnvironmentsAPI.read.mockResolvedValue({ + data: mockExecutionEnvironment, +}); + describe('', () => { beforeEach(() => { LabelsAPI.read.mockResolvedValue({ data: { results: [] } }); @@ -266,6 +287,8 @@ describe('', () => { id: 1, organization: 1, }); + + wrapper.find('ExecutionEnvironmentLookup').invoke('onChange')(null); }); wrapper.update(); await act(async () => { @@ -277,6 +300,7 @@ describe('', () => { ...mockJobTemplate, project: mockJobTemplate.project, ...updatedTemplateData, + execution_environment: null, }; delete expected.summary_fields; delete expected.id; diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx index cb963e634f..dd47118316 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.jsx @@ -17,11 +17,13 @@ function WorkflowJobTemplateEdit({ template }) { organization, webhook_credential, webhook_key, + execution_environment, ...templatePayload } = values; templatePayload.inventory = inventory?.id || null; templatePayload.organization = organization?.id || null; templatePayload.webhook_credential = webhook_credential?.id || null; + templatePayload.execution_environment = execution_environment?.id || null; const formOrgId = organization?.id || inventory?.summary_fields?.organization.id || null; @@ -29,10 +31,7 @@ function WorkflowJobTemplateEdit({ template }) { await Promise.all( await submitLabels(labels, formOrgId, template.organization) ); - await WorkflowJobTemplatesAPI.update(template.id, { - ...templatePayload, - execution_environment: values.execution_environment?.id, - }); + await WorkflowJobTemplatesAPI.update(template.id, templatePayload); history.push(`/templates/workflow_job_template/${template.id}/details`); } catch (err) { setFormSubmitError(err); diff --git a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx index 81ae43e817..3e5d0fb0e3 100644 --- a/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplateEdit/WorkflowJobTemplateEdit.test.jsx @@ -6,6 +6,7 @@ import { WorkflowJobTemplatesAPI, OrganizationsAPI, LabelsAPI, + ExecutionEnvironmentsAPI, } from '../../../api'; import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; import WorkflowJobTemplateEdit from './WorkflowJobTemplateEdit'; @@ -14,12 +15,19 @@ jest.mock('../../../api/models/WorkflowJobTemplates'); jest.mock('../../../api/models/Labels'); jest.mock('../../../api/models/Organizations'); jest.mock('../../../api/models/Inventories'); +jest.mock('../../../api/models/ExecutionEnvironments'); const mockTemplate = { id: 6, name: 'Foo', description: 'Foo description', summary_fields: { + execution_environment: { + id: 1, + name: 'Default EE', + description: '', + image: 'quay.io/ansible/awx-ee', + }, inventory: { id: 1, name: 'Inventory 1' }, organization: { id: 1, name: 'Organization 1' }, labels: { @@ -32,7 +40,18 @@ const mockTemplate = { scm_branch: 'devel', limit: '5000', variables: '---', + execution_environment: 1, }; + +const mockExecutionEnvironment = [ + { + id: 1, + name: 'Default EE', + description: '', + image: 'quay.io/ansible/awx-ee', + }, +]; + describe('', () => { let wrapper; let history; @@ -48,6 +67,9 @@ describe('', () => { }, }); OrganizationsAPI.read.mockResolvedValue({ results: [{ id: 1 }] }); + ExecutionEnvironmentsAPI.read.mockResolvedValue({ + data: mockExecutionEnvironment, + }); await act(async () => { history = createMemoryHistory({ @@ -100,6 +122,7 @@ describe('', () => { .find('LabelSelect') .find('SelectToggle') .simulate('click'); + wrapper.find('ExecutionEnvironmentLookup').invoke('onChange')(null); }); wrapper.update(); @@ -142,6 +165,7 @@ describe('', () => { ask_limit_on_launch: false, ask_scm_branch_on_launch: false, ask_variables_on_launch: false, + execution_environment: null, }); wrapper.update(); await expect(WorkflowJobTemplatesAPI.disassociateLabel).toBeCalledWith(6, { From 7c2f6c95a6e40e7424cdd62a6c87803101016da3 Mon Sep 17 00:00:00 2001 From: nixocio Date: Tue, 23 Feb 2021 16:02:13 -0500 Subject: [PATCH 04/14] Update variables as returned by useRequest Update variables to be consistent with variables returned by useRequest. --- awx/ui_next/src/components/Schedule/shared/ScheduleForm.jsx | 2 +- .../src/components/Schedule/shared/ScheduleForm.test.jsx | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/awx/ui_next/src/components/Schedule/shared/ScheduleForm.jsx b/awx/ui_next/src/components/Schedule/shared/ScheduleForm.jsx index cbe1526331..e64eefbbff 100644 --- a/awx/ui_next/src/components/Schedule/shared/ScheduleForm.jsx +++ b/awx/ui_next/src/components/Schedule/shared/ScheduleForm.jsx @@ -223,7 +223,7 @@ function ScheduleForm({ const { request: loadScheduleData, error: contentError, - contentLoading, + isLoading: contentLoading, result: { zoneOptions, credentials }, } = useRequest( useCallback(async () => { diff --git a/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx b/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx index 508169ce3e..5455919ebc 100644 --- a/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx +++ b/awx/ui_next/src/components/Schedule/shared/ScheduleForm.test.jsx @@ -447,6 +447,7 @@ describe('', () => { /> ); }); + await waitForElement(wrapper, 'ContentLoading', el => el.length === 0); }); afterAll(() => { wrapper.unmount(); From 83a9c3470e9f29e536b9dc2594e687c648a7773a Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 15 Feb 2021 11:18:36 -0500 Subject: [PATCH 05/14] Adds toast to notification template list whenever test notification finishes --- .../NotificationTemplateList.jsx | 53 ++++++- .../NotificationTemplateList.test.jsx | 45 +++++- .../NotificationTemplateListItem.jsx | 135 ++++++++++-------- 3 files changed, 172 insertions(+), 61 deletions(-) diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx index c1b277b730..ed7d019072 100644 --- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx +++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx @@ -1,8 +1,14 @@ -import React, { useCallback, useEffect } from 'react'; +import React, { useCallback, useEffect, useState } 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 { + Alert, + AlertActionCloseButton, + AlertGroup, + Card, + PageSection, +} from '@patternfly/react-core'; import { NotificationTemplatesAPI } from '../../../api'; import PaginatedTable, { HeaderRow, @@ -29,6 +35,7 @@ const QS_CONFIG = getQSConfig('notification-templates', { function NotificationTemplatesList({ i18n }) { const location = useLocation(); const match = useRouteMatch(); + const [testToasts, setTestToasts] = useState([]); const addUrl = `${match.url}/add`; @@ -102,6 +109,14 @@ function NotificationTemplatesList({ i18n }) { setSelected([]); }; + const addTestToast = useCallback(notification => { + setTestToasts(oldToasts => [...oldToasts, notification]); + }, []); + + const removeTestToast = notificationId => { + setTestToasts(testToasts.filter(toast => toast.id !== notificationId)); + }; + const canAdd = actions && actions.POST; return ( @@ -185,6 +200,7 @@ function NotificationTemplatesList({ i18n }) { } renderRow={(template, index) => ( + + {testToasts + .filter(notification => notification.status !== 'pending') + .map(notification => ( + removeTestToast(notification.id)} + /> + } + onTimeout={() => removeTestToast(notification.id)} + timeout={notification.status !== 'failed'} + title={notification.summary_fields.notification_template.name} + variant={notification.status === 'failed' ? 'danger' : 'success'} + key={`notification-template-alert-${notification.id}`} + ouiaId={`notification-template-alert-${notification.id}`} + > + <> + {notification.status === 'successful' && ( +

{i18n._(t`Notification sent successfully`)}

+ )} + {notification.status === 'failed' && + notification?.error === 'timed out' && ( +

{i18n._(t`Notification timed out`)}

+ )} + {notification.status === 'failed' && + notification?.error !== 'timed out' && ( +

{notification.error}

+ )} + + + ))} + ); } diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx index 345d2d0a42..f0bef1326c 100644 --- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx +++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.test.jsx @@ -1,11 +1,17 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { OrganizationsAPI } from '../../../api'; +import { + NotificationsAPI, + NotificationTemplatesAPI, + OrganizationsAPI, +} from '../../../api'; import { mountWithContexts } from '../../../../testUtils/enzymeHelpers'; import NotificationTemplateList from './NotificationTemplateList'; jest.mock('../../../api'); +jest.useFakeTimers(); + const mockTemplates = { data: { count: 3, @@ -197,6 +203,43 @@ describe('', () => { expect(wrapper.find('ToolbarAddButton').length).toBe(1); }); + test('should show toast after test resolves', async () => { + NotificationTemplatesAPI.test.mockResolvedValueOnce({ + data: { + notification: 9182, + }, + }); + NotificationsAPI.readDetail.mockResolvedValueOnce({ + data: { + id: 9182, + status: 'failed', + error: 'There was an error with the notification', + summary_fields: { + notification_template: { + name: 'foobar', + }, + }, + }, + }); + await act(async () => { + wrapper = mountWithContexts(); + }); + wrapper.update(); + expect(wrapper.find('Alert').length).toBe(0); + await act(async () => { + wrapper + .find('button[aria-label="Test Notification"]') + .at(0) + .simulate('click'); + }); + wrapper.update(); + await act(async () => { + jest.runAllTimers(); + }); + wrapper.update(); + expect(wrapper.find('Alert').length).toBe(1); + }); + test('should hide add button (rbac)', async () => { OrganizationsAPI.readOptions.mockResolvedValue({ data: { diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx index 93d1f82999..71d32e9821 100644 --- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx +++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateListItem.jsx @@ -11,13 +11,16 @@ import { timeOfDay } from '../../../util/dates'; import { NotificationTemplatesAPI, NotificationsAPI } from '../../../api'; import StatusLabel from '../../../components/StatusLabel'; import CopyButton from '../../../components/CopyButton'; -import useRequest from '../../../util/useRequest'; +import AlertModal from '../../../components/AlertModal'; +import ErrorDetail from '../../../components/ErrorDetail'; +import useRequest, { useDismissableError } from '../../../util/useRequest'; import { NOTIFICATION_TYPES } from '../constants'; const NUM_RETRIES = 25; const RETRY_TIMEOUT = 5000; function NotificationTemplateListItem({ + onAddToast, template, detailUrl, fetchTemplates, @@ -66,6 +69,7 @@ function NotificationTemplateListItem({ notificationId ); if (notification.status !== 'pending') { + onAddToast(notification); setStatus(notification.status); return; } @@ -76,9 +80,11 @@ function NotificationTemplateListItem({ } setTimeout(pollForStatusChange, RETRY_TIMEOUT); - }, [template.id]) + }, [template.id, onAddToast]) ); + const { error: sendTestError, dismissError } = useDismissableError(error); + useEffect(() => { if (error) { setStatus('error'); @@ -88,65 +94,78 @@ function NotificationTemplateListItem({ const labelId = `template-name-${template.id}`; return ( - - - - - {template.name} - - - - {status && } - - - {NOTIFICATION_TYPES[template.notification_type] || - template.notification_type} - - - - + + - - - - - + + - - - - + + + + {sendTestError && ( + - - - - + {i18n._(t`Failed to send test notification.`)} + + + )} + ); } From d96383b3174d0f741d83791f765367a7fe711b8a Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 15 Feb 2021 15:48:04 -0500 Subject: [PATCH 06/14] Fixes bug where some toasts would reappear after being closed --- .../NotificationTemplateList/NotificationTemplateList.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx index ed7d019072..2e8b9a0c80 100644 --- a/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx +++ b/awx/ui_next/src/screens/NotificationTemplate/NotificationTemplateList/NotificationTemplateList.jsx @@ -114,7 +114,9 @@ function NotificationTemplatesList({ i18n }) { }, []); const removeTestToast = notificationId => { - setTestToasts(testToasts.filter(toast => toast.id !== notificationId)); + setTestToasts(oldToasts => + oldToasts.filter(toast => toast.id !== notificationId) + ); }; const canAdd = actions && actions.POST; From 811186308ca9292965b890b9a925cf377afb5dd1 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 18 Feb 2021 09:39:46 -0500 Subject: [PATCH 07/14] Adds note to changelog about notification toasts --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c527b004..39ebc4f856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This is a list of high-level changes for each release of AWX. A full list of com - Added ability to relaunch against failed hosts: https://github.com/ansible/awx/pull/9225 - Added pending workflow approval count to the application header https://github.com/ansible/awx/pull/9334 - Added user interface for management jobs: https://github.com/ansible/awx/pull/9224 +- Added toast message to show notification template test result to notification templates list https://github.com/ansible/awx/pull/9318 # 17.0.1 (January 26, 2021) - Fixed pgdocker directory permissions issue with Local Docker installer: https://github.com/ansible/awx/pull/9152 From 6a3216443829381da2692ddf3fa88329096a7855 Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 8 Mar 2021 16:11:22 -0500 Subject: [PATCH 08/14] Adds support for ouiaId's on copy buttons --- awx/ui_next/src/components/CopyButton/CopyButton.jsx | 4 ++++ .../NotificationTemplateList/NotificationTemplateList.jsx | 2 +- .../NotificationTemplateList/NotificationTemplateListItem.jsx | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/awx/ui_next/src/components/CopyButton/CopyButton.jsx b/awx/ui_next/src/components/CopyButton/CopyButton.jsx index 2856c69c0c..1f5dd9cff4 100644 --- a/awx/ui_next/src/components/CopyButton/CopyButton.jsx +++ b/awx/ui_next/src/components/CopyButton/CopyButton.jsx @@ -17,6 +17,7 @@ function CopyButton({ onCopyFinish, errorMessage, i18n, + ouiaId, }) { const { isLoading, error: copyError, request: copyItemToAPI } = useRequest( copyItem @@ -35,6 +36,7 @@ function CopyButton({ <>