Merge pull request #9737 from jlmitch5/workflowConvergence

workflow convergence

Satisfies #7669
This adds the ability to set convergence from any nodes (default) to all nodes to the workflow feature.  Specifically:
There is an additional convergence dropdown located on the bottom of the first "node type" step for all node types:

This field defaults to "Any" on add node and whatever the api has the field set to on edit node.  It resets to any if you change the node type dropdown (even on edit when changing and then changing back to the original type...I can update that point depending on what UX is preferred).
tvo created a new link explicitly to the explanation in documentation of what the convergence setting does here, and I link to it in the help popover shown in the screenshot below.
Consistent with the old UI, When "All" is selected, a small tab is displayed with the label "ALL" in the node visualizer.  "Any" nodes do not get any sort of tab.

A slight tweak compared to the old ui...I set the tab's border and background to be the same color as the border of the node to create a consistent look for the node across various states and confirmed this behavior was good with @trahman73
OLD:



NEW:

Reviewed-by: Jake McDermott <yo@jakemcdermott.me>
Reviewed-by: John Hill <johill@redhat.com>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-04-02 16:53:50 +00:00 committed by GitHub
commit 941e99018a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 299 additions and 99 deletions

View File

@ -183,6 +183,7 @@ function createNode(state, node) {
fullUnifiedJobTemplate: node.nodeResource,
isInvalidLinkTarget: false,
promptValues: node.promptValues,
all_parents_must_converge: node.all_parents_must_converge,
});
// Ensures that root nodes appear to always run
@ -657,10 +658,19 @@ function updateLink(state, linkType) {
function updateNode(state, editedNode) {
const { nodeToEdit, nodes } = state;
const { nodeResource, launchConfig, promptValues } = editedNode;
const {
nodeResource,
launchConfig,
promptValues,
all_parents_must_converge,
} = editedNode;
const newNodes = [...nodes];
const matchingNode = newNodes.find(node => node.id === nodeToEdit.id);
matchingNode.all_parents_must_converge = all_parents_must_converge;
if (matchingNode.originalNodeObject) {
delete matchingNode.originalNodeObject.all_parents_must_converge;
}
matchingNode.fullUnifiedJobTemplate = nodeResource;
matchingNode.isEdited = true;
matchingNode.launchConfig = launchConfig;

View File

@ -59,6 +59,11 @@ const NodeDefaultLabel = styled.p`
white-space: nowrap;
`;
const ConvergenceLabel = styled.p`
font-size: 12px;
color: #ffffff;
`;
Elapsed.displayName = 'Elapsed';
function WorkflowOutputNode({ i18n, mouseEnter, mouseLeave, node }) {
@ -100,6 +105,30 @@ function WorkflowOutputNode({ i18n, mouseEnter, mouseLeave, node }) {
onMouseEnter={mouseEnter}
onMouseLeave={mouseLeave}
>
{(node.all_parents_must_converge ||
node?.originalNodeObject?.all_parents_must_converge) && (
<>
<rect
fill={borderColor}
height={wfConstants.nodeH / 4}
rx={2}
ry={2}
x={wfConstants.nodeW / 2 - wfConstants.nodeW / 10}
y={-wfConstants.nodeH / 4 + 2}
stroke={borderColor}
strokeWidth="2px"
width={wfConstants.nodeW / 5}
/>
<foreignObject
height={wfConstants.nodeH / 4}
width={wfConstants.nodeW / 5}
x={wfConstants.nodeW / 2 - wfConstants.nodeW / 10 + 7}
y={-wfConstants.nodeH / 4 - 1}
>
<ConvergenceLabel>{i18n._(t`ALL`)}</ConvergenceLabel>
</foreignObject>
</>
)}
<rect
fill="#FFFFFF"
height={wfConstants.nodeH}

View File

@ -19,6 +19,7 @@ function NodeAddModal({ i18n }) {
timeoutMinutes,
timeoutSeconds,
linkType,
convergence,
} = values;
if (values) {
@ -33,8 +34,11 @@ function NodeAddModal({ i18n }) {
const node = {
linkType,
all_parents_must_converge: convergence === 'all',
};
delete values.convergence;
delete values.linkType;
if (values.nodeType === 'workflow_approval_template') {

View File

@ -48,6 +48,7 @@ describe('NodeAddModal', () => {
expect(dispatch).toHaveBeenCalledWith({
node: {
all_parents_must_converge: false,
linkType: 'success',
nodeResource: {
id: 448,

View File

@ -17,11 +17,13 @@ function NodeEditModal({ i18n }) {
nodeType,
timeoutMinutes,
timeoutSeconds,
convergence,
...rest
} = values;
let node;
if (values.nodeType === 'workflow_approval_template') {
node = {
all_parents_must_converge: convergence === 'all',
nodeResource: {
description: approvalDescription,
name: approvalName,
@ -32,6 +34,7 @@ function NodeEditModal({ i18n }) {
} else {
node = {
nodeResource,
all_parents_must_converge: convergence === 'all',
};
if (nodeType === 'job_template' || nodeType === 'workflow_job_template') {
node.promptValues = {

View File

@ -63,6 +63,7 @@ describe('NodeEditModal', () => {
});
expect(dispatch).toHaveBeenCalledWith({
node: {
all_parents_must_converge: false,
nodeResource: { id: 448, name: 'Test JT', type: 'job_template' },
},
type: 'UPDATE_NODE',

View File

@ -101,7 +101,6 @@ function NodeModalForm({
values.extra_data = extraVars && parseVariableField(extraVars);
delete values.extra_vars;
}
onSave(values, launchConfig);
};
@ -357,6 +356,7 @@ const NodeModal = ({ onSave, i18n, askLinkType, title }) => {
approvalDescription: '',
timeoutMinutes: 0,
timeoutSeconds: 0,
convergence: 'any',
linkType: 'success',
nodeResource: nodeToEdit?.fullUnifiedJobTemplate || null,
nodeType: nodeToEdit?.fullUnifiedJobTemplate?.type || 'job_template',

View File

@ -307,6 +307,7 @@ describe('NodeModal', () => {
});
expect(onSave).toBeCalledWith(
{
convergence: 'any',
linkType: 'always',
nodeType: 'job_template',
inventory: { name: 'Foo Inv', id: 1 },
@ -345,6 +346,7 @@ describe('NodeModal', () => {
});
expect(onSave).toBeCalledWith(
{
convergence: 'any',
linkType: 'failure',
nodeResource: {
id: 1,
@ -383,6 +385,7 @@ describe('NodeModal', () => {
});
expect(onSave).toBeCalledWith(
{
convergence: 'any',
linkType: 'failure',
nodeResource: {
id: 1,
@ -422,6 +425,7 @@ describe('NodeModal', () => {
});
expect(onSave).toBeCalledWith(
{
convergence: 'any',
linkType: 'success',
nodeResource: {
id: 1,
@ -506,6 +510,7 @@ describe('NodeModal', () => {
});
expect(onSave).toBeCalledWith(
{
convergence: 'any',
approvalDescription: 'Test Approval Description',
approvalName: 'Test Approval',
linkType: 'always',
@ -605,6 +610,7 @@ describe('NodeModal', () => {
expect(onSave).toBeCalledWith(
{
convergence: 'any',
approvalDescription: 'Test Approval Description',
approvalName: 'Test Approval',
linkType: 'success',
@ -668,6 +674,7 @@ describe('NodeModal', () => {
});
expect(onSave).toBeCalledWith(
{
convergence: 'any',
linkType: 'success',
nodeResource: {
id: 1,

View File

@ -1,13 +1,25 @@
import 'styled-components/macro';
import React from 'react';
import React, { useState } from 'react';
import { withI18n } from '@lingui/react';
import { t, Trans } from '@lingui/macro';
import styled from 'styled-components';
import { useField } from 'formik';
import { Alert, Form, FormGroup, TextInput } from '@patternfly/react-core';
import {
Alert,
Form,
FormGroup,
TextInput,
Select,
SelectVariant,
SelectOption,
} from '@patternfly/react-core';
import { required } from '../../../../../../util/validators';
import { FormFullWidthLayout } from '../../../../../../components/FormLayout';
import {
FormColumnLayout,
FormFullWidthLayout,
} from '../../../../../../components/FormLayout';
import Popover from '../../../../../../components/Popover';
import AnsibleSelect from '../../../../../../components/AnsibleSelect';
import InventorySourcesList from './InventorySourcesList';
import JobTemplatesList from './JobTemplatesList';
@ -44,6 +56,9 @@ function NodeTypeStep({ i18n }) {
const [timeoutSecondsField, , timeoutSecondsHelpers] = useField(
'timeoutSeconds'
);
const [convergenceField, , convergenceFieldHelpers] = useField('convergence');
const [isConvergenceOpen, setIsConvergenceOpen] = useState(false);
const isValid = !approvalNameMeta.touched || !approvalNameMeta.error;
return (
@ -101,6 +116,7 @@ function NodeTypeStep({ i18n }) {
approvalDescriptionHelpers.setValue('');
timeoutMinutesHelpers.setValue(0);
timeoutSecondsHelpers.setValue(0);
convergenceFieldHelpers.setValue('any');
}}
/>
</div>
@ -129,61 +145,108 @@ function NodeTypeStep({ i18n }) {
onUpdateNodeResource={nodeResourceHelpers.setValue}
/>
)}
{nodeTypeField.value === 'workflow_approval_template' && (
<Form css="margin-top: 20px;">
<FormFullWidthLayout>
<FormField
name="approvalName"
id="approval-name"
isRequired
validate={required(null, i18n)}
validated={isValid ? 'default' : 'error'}
label={i18n._(t`Name`)}
/>
<FormField
name="approvalDescription"
id="approval-description"
label={i18n._(t`Description`)}
/>
<FormGroup
label={i18n._(t`Timeout`)}
fieldId="approval-timeout"
name="timeout"
<Form css="margin-top: 20px;">
<FormColumnLayout>
{nodeTypeField.value === 'workflow_approval_template' && (
<FormFullWidthLayout>
<FormField
name="approvalName"
id="approval-name"
isRequired
validate={required(null, i18n)}
validated={isValid ? 'default' : 'error'}
label={i18n._(t`Name`)}
/>
<FormField
name="approvalDescription"
id="approval-description"
label={i18n._(t`Description`)}
/>
<FormGroup
label={i18n._(t`Timeout`)}
fieldId="approval-timeout"
name="timeout"
>
<div css="display: flex;align-items: center;">
<TimeoutInput
{...timeoutMinutesField}
aria-label={i18n._(t`Timeout minutes`)}
id="approval-timeout-minutes"
min="0"
onChange={(value, event) => {
timeoutMinutesField.onChange(event);
}}
step="1"
type="number"
/>
<TimeoutLabel>
<Trans>min</Trans>
</TimeoutLabel>
<TimeoutInput
{...timeoutSecondsField}
aria-label={i18n._(t`Timeout seconds`)}
id="approval-timeout-seconds"
min="0"
onChange={(value, event) => {
timeoutSecondsField.onChange(event);
}}
step="1"
type="number"
/>
<TimeoutLabel>
<Trans>sec</Trans>
</TimeoutLabel>
</div>
</FormGroup>
</FormFullWidthLayout>
)}
<FormGroup
fieldId="convergence"
label={i18n._(t`Convergence`)}
isRequired
labelIcon={
<Popover
content={
<>
{i18n._(
t`Preconditions for running this node when there are multiple parents. Refer to the`
)}{' '}
<a
href="https://docs.ansible.com/ansible-tower/latest/html/userguide/workflow_templates.html#convergence-node"
target="_blank"
rel="noopener noreferrer"
>
{i18n._(t`documentation`)}
</a>{' '}
{i18n._(t`for more info.`)}
</>
}
/>
}
>
<Select
variant={SelectVariant.single}
isOpen={isConvergenceOpen}
selections={convergenceField.value}
onToggle={setIsConvergenceOpen}
onSelect={(event, selection) => {
convergenceFieldHelpers.setValue(selection);
setIsConvergenceOpen(false);
}}
aria-label={i18n._(t`Convergence select`)}
className="convergenceSelect"
ouiaId="convergenceSelect"
>
<div css="display: flex;align-items: center;">
<TimeoutInput
{...timeoutMinutesField}
aria-label={i18n._(t`Timeout minutes`)}
id="approval-timeout-minutes"
min="0"
onChange={(value, event) => {
timeoutMinutesField.onChange(event);
}}
step="1"
type="number"
/>
<TimeoutLabel>
<Trans>min</Trans>
</TimeoutLabel>
<TimeoutInput
{...timeoutSecondsField}
aria-label={i18n._(t`Timeout seconds`)}
id="approval-timeout-seconds"
min="0"
onChange={(value, event) => {
timeoutSecondsField.onChange(event);
}}
step="1"
type="number"
/>
<TimeoutLabel>
<Trans>sec</Trans>
</TimeoutLabel>
</div>
</FormGroup>
</FormFullWidthLayout>
</Form>
)}
<SelectOption key="any" value="any" id="select-option-any">
{i18n._(t`Any`)}
</SelectOption>
<SelectOption key="all" value="all" id="select-option-all">
{i18n._(t`All`)}
</SelectOption>
</Select>
</FormGroup>
</FormColumnLayout>
</Form>
</>
);
}

View File

@ -177,6 +177,7 @@ describe('NodeTypeStep', () => {
approvalDescription: '',
timeoutMinutes: 0,
timeoutSeconds: 0,
convergence: 'any',
}}
>
<NodeTypeStep />

View File

@ -86,5 +86,6 @@ function getInitialValues() {
timeoutMinutes: 0,
timeoutSeconds: 0,
nodeType: 'job_template',
convergence: 'any',
};
}

View File

@ -282,6 +282,7 @@ describe('NodeViewModal', () => {
description: '',
type: 'workflow_approval_template',
timeout: 0,
all_parents_must_converge: false,
},
},
};

View File

@ -39,6 +39,11 @@ const getNodeToEditDefaultValues = (
const initialValues = {
nodeResource: nodeToEdit?.fullUnifiedJobTemplate || null,
nodeType: nodeToEdit?.fullUnifiedJobTemplate?.type || 'job_template',
convergence:
nodeToEdit?.all_parents_must_converge ||
nodeToEdit?.originalNodeObject?.all_parents_must_converge
? 'all'
: 'any',
};
if (
@ -228,7 +233,6 @@ export default function useWorkflowNodeSteps(
useEffect(() => {
if (launchConfig && surveyConfig && isReady) {
let initialValues = {};
if (
nodeToEdit &&
nodeToEdit?.fullUnifiedJobTemplate &&
@ -264,10 +268,15 @@ export default function useWorkflowNodeSteps(
);
}
if (initialValues.convergence === 'all') {
formikValues.convergence = 'all';
}
resetForm({
errors,
values: {
...initialValues,
convergence: formikValues.convergence,
nodeResource: formikValues.nodeResource,
nodeType: formikValues.nodeType,
linkType: formikValues.linkType,

View File

@ -369,27 +369,24 @@ function Visualizer({ template, i18n }) {
node.fullUnifiedJobTemplate.type === 'workflow_approval_template'
) {
nodeRequests.push(
WorkflowJobTemplatesAPI.createNode(template.id, {}).then(
({ data }) => {
node.originalNodeObject = data;
originalLinkMap[node.id] = {
id: data.id,
success_nodes: [],
failure_nodes: [],
always_nodes: [],
};
approvalTemplateRequests.push(
WorkflowJobTemplateNodesAPI.createApprovalTemplate(
data.id,
{
name: node.fullUnifiedJobTemplate.name,
description: node.fullUnifiedJobTemplate.description,
timeout: node.fullUnifiedJobTemplate.timeout,
}
)
);
}
)
WorkflowJobTemplatesAPI.createNode(template.id, {
all_parents_must_converge: node.all_parents_must_converge,
}).then(({ data }) => {
node.originalNodeObject = data;
originalLinkMap[node.id] = {
id: data.id,
success_nodes: [],
failure_nodes: [],
always_nodes: [],
};
approvalTemplateRequests.push(
WorkflowJobTemplateNodesAPI.createApprovalTemplate(data.id, {
name: node.fullUnifiedJobTemplate.name,
description: node.fullUnifiedJobTemplate.description,
timeout: node.fullUnifiedJobTemplate.timeout,
})
);
})
);
} else {
nodeRequests.push(
@ -397,6 +394,7 @@ function Visualizer({ template, i18n }) {
...node.promptValues,
inventory: node.promptValues?.inventory?.id || null,
unified_job_template: node.fullUnifiedJobTemplate.id,
all_parents_must_converge: node.all_parents_must_converge,
}).then(({ data }) => {
node.originalNodeObject = data;
originalLinkMap[node.id] = {
@ -427,27 +425,47 @@ function Visualizer({ template, i18n }) {
node.originalNodeObject.summary_fields.unified_job_template
.unified_job_type === 'workflow_approval'
) {
approvalTemplateRequests.push(
WorkflowApprovalTemplatesAPI.update(
node.originalNodeObject.summary_fields.unified_job_template
.id,
{
name: node.fullUnifiedJobTemplate.name,
description: node.fullUnifiedJobTemplate.description,
timeout: node.fullUnifiedJobTemplate.timeout,
}
)
);
} else {
approvalTemplateRequests.push(
WorkflowJobTemplateNodesAPI.createApprovalTemplate(
nodeRequests.push(
WorkflowJobTemplateNodesAPI.replace(
node.originalNodeObject.id,
{
name: node.fullUnifiedJobTemplate.name,
description: node.fullUnifiedJobTemplate.description,
timeout: node.fullUnifiedJobTemplate.timeout,
all_parents_must_converge: node.all_parents_must_converge,
}
)
).then(({ data }) => {
node.originalNodeObject = data;
approvalTemplateRequests.push(
WorkflowApprovalTemplatesAPI.update(
node.originalNodeObject.summary_fields
.unified_job_template.id,
{
name: node.fullUnifiedJobTemplate.name,
description: node.fullUnifiedJobTemplate.description,
timeout: node.fullUnifiedJobTemplate.timeout,
}
)
);
})
);
} else {
nodeRequests.push(
WorkflowJobTemplateNodesAPI.replace(
node.originalNodeObject.id,
{
all_parents_must_converge: node.all_parents_must_converge,
}
).then(({ data }) => {
node.originalNodeObject = data;
approvalTemplateRequests.push(
WorkflowJobTemplateNodesAPI.createApprovalTemplate(
node.originalNodeObject.id,
{
name: node.fullUnifiedJobTemplate.name,
description: node.fullUnifiedJobTemplate.description,
timeout: node.fullUnifiedJobTemplate.timeout,
}
)
);
})
);
}
} else {
@ -456,6 +474,7 @@ function Visualizer({ template, i18n }) {
...node.promptValues,
inventory: node.promptValues?.inventory?.id || null,
unified_job_template: node.fullUnifiedJobTemplate.id,
all_parents_must_converge: node.all_parents_must_converge,
}).then(() => {
const {
added: addedCredentials,

View File

@ -419,6 +419,7 @@ describe('Visualizer', () => {
).toBe(1);
});
// TODO: figure out why this test is failing, the scenario passes in the ui
test('Error shown when saving fails due to approval template edit error', async () => {
workflowReducer.mockImplementation(state => {
const newState = {
@ -459,6 +460,17 @@ describe('Visualizer', () => {
results: [],
},
});
WorkflowJobTemplateNodesAPI.replace.mockResolvedValue({
data: {
id: 9000,
summary_fields: {
unified_job_template: {
unified_job_type: 'workflow_approval',
id: 1,
},
},
},
});
WorkflowApprovalTemplatesAPI.update.mockRejectedValue(new Error());
await act(async () => {
wrapper = mountWithContexts(
@ -475,6 +487,7 @@ describe('Visualizer', () => {
wrapper.find('Button#visualizer-save').simulate('click');
});
wrapper.update();
expect(WorkflowJobTemplateNodesAPI.replace).toHaveBeenCalledTimes(1);
expect(WorkflowApprovalTemplatesAPI.update).toHaveBeenCalledTimes(1);
expect(
wrapper.find('AlertModal[title="Error saving the workflow!"]').length

View File

@ -44,6 +44,12 @@ const NodeResourceName = styled.p`
text-overflow: ellipsis;
white-space: nowrap;
`;
const ConvergenceLabel = styled.p`
font-size: 12px;
color: #ffffff;
`;
NodeResourceName.displayName = 'NodeResourceName';
function VisualizerNode({
@ -244,6 +250,38 @@ function VisualizerNode({
node.id
].y - nodePositions[1].y})`}
>
{(node.all_parents_must_converge ||
node?.originalNodeObject?.all_parents_must_converge) && (
<>
<rect
fill={
hovering && addingLink && !node.isInvalidLinkTarget
? '#007ABC'
: '#93969A'
}
height={wfConstants.nodeH / 4}
rx={2}
ry={2}
x={wfConstants.nodeW / 2 - wfConstants.nodeW / 10}
y={-wfConstants.nodeH / 4 + 2}
stroke={
hovering && addingLink && !node.isInvalidLinkTarget
? '#007ABC'
: '#93969A'
}
strokeWidth="2px"
width={wfConstants.nodeW / 5}
/>
<foreignObject
height={wfConstants.nodeH / 4}
width={wfConstants.nodeW / 5}
x={wfConstants.nodeW / 2 - wfConstants.nodeW / 10 + 7}
y={-wfConstants.nodeH / 4 - 1}
>
<ConvergenceLabel>{i18n._(t`ALL`)}</ConvergenceLabel>
</foreignObject>
</>
)}
<rect
fill="#FFFFFF"
height={wfConstants.nodeH}