diff --git a/frontend/packages/dev-console/src/components/pipelines/PipelineDetailsPage.tsx b/frontend/packages/dev-console/src/components/pipelines/PipelineDetailsPage.tsx index 1f3b7dcea5a..6d0b8151f2d 100644 --- a/frontend/packages/dev-console/src/components/pipelines/PipelineDetailsPage.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/PipelineDetailsPage.tsx @@ -7,7 +7,7 @@ import { rerunPipelineAndRedirect, startPipeline, handlePipelineRunSubmit, - // editPipeline, + editPipeline, } from '../../utils/pipeline-actions'; import { getLatestRun } from '../../utils/pipeline-augment'; import { PipelineRunModel, PipelineModel } from '../../models'; @@ -49,7 +49,7 @@ class PipelineDetailsPage extends React.Component rerunPipelineAndRedirect(PipelineRunModel, latestRun)] : []), - // editPipeline, + editPipeline, Kebab.factory.Delete, ], }); diff --git a/frontend/packages/dev-console/src/components/pipelines/PipelinesResourceList.tsx b/frontend/packages/dev-console/src/components/pipelines/PipelinesResourceList.tsx index d4f0f6f7995..e4174c2cd1b 100644 --- a/frontend/packages/dev-console/src/components/pipelines/PipelinesResourceList.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/PipelinesResourceList.tsx @@ -30,12 +30,9 @@ const PipelinesResourceList: React.FC = (props) => { canCreate createButtonText={`Create ${PipelineModel.label}`} createProps={{ - // to: namespace - // ? `/k8s/ns/${namespace}/${referenceForModel(PipelineModel)}/~new/builder` - // : `/k8s/cluster/${referenceForModel(PipelineModel)}/~new`, - to: `/k8s/${namespace ? `ns/${namespace}` : 'cluster'}/${referenceForModel( - PipelineModel, - )}/~new`, + to: namespace + ? `/k8s/ns/${namespace}/${referenceForModel(PipelineModel)}/~new/builder` + : `/k8s/cluster/${referenceForModel(PipelineModel)}/~new`, }} filterLabel="by name" textFilter="name" diff --git a/frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/pipeline-details/PipelineVisualizationTask.tsx b/frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/pipeline-details/PipelineVisualizationTask.tsx index 4563c5dbe77..9e86db60a80 100644 --- a/frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/pipeline-details/PipelineVisualizationTask.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/pipeline-details/PipelineVisualizationTask.tsx @@ -24,6 +24,7 @@ interface TaskProps { status?: TaskStatus; namespace: string; isPipelineRun: boolean; + disableTooltip?: boolean; } interface PipelineVisualizationTaskProp { @@ -39,6 +40,7 @@ interface PipelineVisualizationTaskProp { }; taskRun?: string; pipelineRunStatus?: string; + disableTooltip?: boolean; } export const PipelineVisualizationTask: React.FC = ({ @@ -46,6 +48,7 @@ export const PipelineVisualizationTask: React.FC task, namespace, pipelineRunStatus, + disableTooltip, }) => { const taskStatus = task.status || { duration: '', @@ -60,6 +63,22 @@ export const PipelineVisualizationTask: React.FC taskStatus.reason = runStatus.Cancelled; } } + + const taskComponent = ( + + ); + + if (disableTooltip) { + return taskComponent; + } + let resources; if (task.taskRef.kind === ClusterTaskModel.kind) { resources = [ @@ -79,17 +98,7 @@ export const PipelineVisualizationTask: React.FC }, ]; } - return ( - - - - ); + return {taskComponent}; }; const TaskComponent: React.FC = ({ pipelineRunName, @@ -98,18 +107,35 @@ const TaskComponent: React.FC = ({ status, name, isPipelineRun, + disableTooltip, }) => { - const taskData: K8sResourceKind = task.data; - const stepList = _.get(taskData, ['spec', 'steps'], []); + const stepList = _.get(task, ['data', 'spec', 'steps'], []); const stepStatusList: StepStatus[] = stepList.map((step) => createStepStatus(step, status)); const showStatusState: boolean = isPipelineRun && !!status && !!status.reason; const visualName = name || _.get(task, ['metadata', 'name'], ''); const path = pipelineRunName ? `${resourcePathFromModel(PipelineRunModel, pipelineRunName, namespace)}/logs/${name}` : undefined; - const visTask = ( - <> -
+ + let taskPill = ( +
+
+
{visualName}
+ {showStatusState && } +
+ {isPipelineRun && ( +
+ {showStatusState && } +
+ )} +
+ ); + if (!disableTooltip) { + taskPill = ( = ({ /> } > -
-
-
{visualName}
- {showStatusState && } -
- {isPipelineRun && ( -
- {showStatusState && ( - - )} -
- )} -
+ {taskPill}
+ ); + } + + const visTask = ( + <> +
+ {taskPill} ); return ( diff --git a/frontend/packages/dev-console/src/components/pipelines/list-page/PipelineRow.tsx b/frontend/packages/dev-console/src/components/pipelines/list-page/PipelineRow.tsx index 567c235592f..a102a2d8839 100644 --- a/frontend/packages/dev-console/src/components/pipelines/list-page/PipelineRow.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/list-page/PipelineRow.tsx @@ -10,7 +10,7 @@ import { rerunPipelineAndRedirect, startPipeline, handlePipelineRunSubmit, - // editPipeline, + editPipeline, } from '../../../utils/pipeline-actions'; import LinkedPipelineRunTaskStatus from '../../pipelineruns/status/LinkedPipelineRunTaskStatus'; import { tableColumnClasses } from './pipeline-table'; @@ -31,7 +31,7 @@ const PipelineRow: React.FC = ({ obj, index, key, style }) => ...(obj.latestRun && obj.latestRun.metadata ? [() => rerunPipelineAndRedirect(PipelineRunModel, obj.latestRun)] : []), - // editPipeline, + editPipeline, Kebab.factory.Delete, ]; return ( diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.scss b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.scss index f057050d69f..5ce7c9ecabb 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.scss +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.scss @@ -1,9 +1,9 @@ .odc-pipeline-builder-form { - height: 100%; - overflow: hidden; + &__grid { + grid-template-columns: minmax(0, 1fr); + } &__content { - height: 100%; overflow: auto; padding: 30px; } diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.tsx index e4b4576cd76..ca716694c35 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.tsx @@ -1,50 +1,47 @@ import * as React from 'react'; import * as _ from 'lodash'; import { FormikProps, FormikValues } from 'formik'; -import { ActionGroup, Button, ButtonVariant, Form, TextInputTypes } from '@patternfly/react-core'; -import { ButtonBar } from '@console/internal/components/utils'; +import { + ActionGroup, + Button, + ButtonVariant, + Form, + Stack, + StackItem, + TextInputTypes, +} from '@patternfly/react-core'; import { InputField } from '@console/shared'; -import { PipelineTask } from '../../../utils/pipeline-augment'; +import { ButtonBar } from '@console/internal/components/utils'; +import { K8sResourceKind } from '@console/internal/module/k8s'; import { PipelineParameters, PipelineResources } from '../detail-page-tabs'; -import { TASK_INCOMPLETE_ERROR_MESSAGE } from './const'; +import { UpdateOperationType } from './const'; +import { useResourceValidation } from './hooks'; import { removeTaskModal } from './modals'; +import PipelineBuilderHeader from './PipelineBuilderHeader'; import PipelineBuilderVisualization from './PipelineBuilderVisualization'; import Sidebar from './task-sidebar/Sidebar'; import TaskSidebar from './task-sidebar/TaskSidebar'; -import { SelectedBuilderTask, TaskErrorMap } from './types'; +import { + CleanupResults, + PipelineBuilderTaskGroup, + SelectedBuilderTask, + UpdateErrors, + UpdateOperationUpdateTaskData, +} from './types'; +import { applyChange } from './update-utils'; import './PipelineBuilderForm.scss'; type PipelineBuilderFormProps = FormikProps & { - editingPipeline: boolean; + existingPipeline: K8sResourceKind; namespace: string; }; -const pruneErrors = (taskErrors: TaskErrorMap): TaskErrorMap => { - return Object.keys(taskErrors).reduce((newTaskMap, taskName) => { - const taskValue = taskErrors[taskName]; - - if ( - !taskValue.inputResourceCount && - !taskValue.outputResourceCount && - !taskValue.paramsMissingDefaults - ) { - return newTaskMap; - } - - return { - ...newTaskMap, - [taskName]: taskValue, - }; - }, {} as TaskErrorMap); -}; - const PipelineBuilderForm: React.FC = (props) => { const [selectedTask, setSelectedTask] = React.useState(null); - const [taskErrors, setTaskErrors] = React.useState({}); const { - editingPipeline, + existingPipeline, status, isSubmitting, dirty, @@ -53,101 +50,134 @@ const PipelineBuilderForm: React.FC = (props) => { errors, namespace, setFieldValue, + setStatus, values, } = props; + const statusRef = React.useRef(status); + statusRef.current = status; + + const updateErrors: UpdateErrors = React.useCallback( + (taskErrors) => { + if (taskErrors) { + setStatus({ + ...statusRef.current, + tasks: _.omitBy(_.merge({}, statusRef.current?.tasks, taskErrors), (v) => !v), + }); + } + }, + [setStatus], + ); + + useResourceValidation(values.tasks, values.resources, updateErrors); + + const updateTasks = (changes: CleanupResults): void => { + const { tasks, listTasks, errors: taskErrors } = changes; + + setFieldValue('tasks', tasks); + setFieldValue('listTasks', listTasks); + updateErrors(taskErrors); + }; + + const taskGroup: PipelineBuilderTaskGroup = { tasks: values.tasks, listTasks: values.listTasks }; return ( -
-
-
- -
+ + + + + + +
+ +
-
-

Tasks

- { - setSelectedTask({ - taskIndex: values.tasks.findIndex(({ name }) => name === task.name), - resource, - }); - }} - onSetError={( - taskName, - inputResourceCount, - outputResourceCount, - paramsMissingDefaults, - ) => { - setTaskErrors({ - ...taskErrors, - [taskName]: { - inputResourceCount, - outputResourceCount, - paramsMissingDefaults, - message: TASK_INCOMPLETE_ERROR_MESSAGE, - }, - }); - }} - onUpdateTasks={(updatedTasks: PipelineTask[]) => setFieldValue('tasks', updatedTasks)} - pipelineTasks={values.tasks} - /> -
+
+

Tasks

+ { + setSelectedTask({ + taskIndex: values.tasks.findIndex(({ name }) => name === task.name), + resource, + }); + }} + onUpdateTasks={(updatedTaskGroup, op) => + updateTasks(applyChange(updatedTaskGroup, op)) + } + taskGroup={taskGroup} + /> +
-
-

Parameters

- -
+
+

Parameters

+ +
-
-

Resources

- -
+
+

Resources

+ +
- - - - - - -
- setSelectedTask(null)}> - {() => ( - setTaskErrors(pruneErrors(newTaskErrors))} - setFieldValue={setFieldValue} - onRemoveTask={(taskName) => { - removeTaskModal(taskName, () => { - setSelectedTask(null); - setFieldValue( - 'tasks', - values.tasks.filter((task) => task.name !== taskName), - ); - }); - }} - selectedPipelineTaskIndex={selectedTask.taskIndex} - taskResource={selectedTask.resource} - /> - )} - -
+ + + + + + + setSelectedTask(null)}> + {() => ( + { + updateTasks( + applyChange(taskGroup, { type: UpdateOperationType.UPDATE_TASK, data }), + ); + }} + onRemoveTask={(taskName) => { + removeTaskModal(taskName, () => { + setSelectedTask(null); + updateTasks( + applyChange(taskGroup, { + type: UpdateOperationType.REMOVE_TASK, + data: { taskName }, + }), + ); + }); + }} + selectedPipelineTaskIndex={selectedTask.taskIndex} + taskResource={selectedTask.resource} + /> + )} + + + + ); }; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.scss b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.scss new file mode 100644 index 00000000000..2d533143bd9 --- /dev/null +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.scss @@ -0,0 +1,13 @@ +.odc-pipeline-builder-header { + &__content { + padding: var(--pf-global--spacer--xl); + } + + &__title { + margin: var(--pf-global--spacer--xs) 0 0 0; + } + + hr { + margin: 0; + } +} diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.tsx new file mode 100644 index 00000000000..f44eb85026a --- /dev/null +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.tsx @@ -0,0 +1,52 @@ +import * as React from 'react'; +import { Button, Flex, FlexItem, FlexItemModifiers } from '@patternfly/react-core'; +import { PipelineModel } from '../../../models'; +import { warnAction } from './modals'; +import { getBadgeFromType } from '@console/shared/src'; +import { Pipeline } from '../../../utils/pipeline-augment'; +import { goToYAML } from './utils'; + +import './PipelineBuilderHeader.scss'; + +type PipelineBuilderHeaderProps = { + existingPipeline: Pipeline; + formIsDirty: boolean; + namespace: string; +}; + +const PipelineBuilderHeader: React.FC = (props) => { + const { existingPipeline, formIsDirty, namespace } = props; + + return ( +
+ + +

Pipeline Builder

+
+ + + + {getBadgeFromType(PipelineModel.badge)} +
+
+
+ ); +}; + +export default PipelineBuilderHeader; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.scss b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.scss index b25c5cb55c6..0204fabd4fa 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.scss +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.scss @@ -1,13 +1,4 @@ .odc-pipeline-builder-page { - &__header { - padding: var(--pf-global--spacer--xl); - } - - &__title { - margin: var(--pf-global--spacer--xs) 0 0 0; - } - - hr { - margin: 0; - } + height: 100%; + position: relative; } diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.tsx index a9b9a3761bd..1ebfecc954b 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderPage.tsx @@ -1,15 +1,11 @@ import * as React from 'react'; -import * as _ from 'lodash'; import { RouteComponentProps } from 'react-router-dom'; import { Helmet } from 'react-helmet'; import { Formik, FormikBag } from 'formik'; -import { Bullseye, Button, Split, SplitItem } from '@patternfly/react-core'; -import { history, resourcePathFromModel } from '@console/internal/components/utils'; +import { history } from '@console/internal/components/utils'; import { k8sCreate, k8sUpdate } from '@console/internal/module/k8s'; -import { getBadgeFromType } from '@console/shared/src'; import { PipelineModel } from '../../../models'; import { Pipeline } from '../../../utils/pipeline-augment'; -import { warnAction } from './modals'; import PipelineBuilderForm from './PipelineBuilderForm'; import { PipelineBuilderFormValues, PipelineBuilderFormikValues } from './types'; import { @@ -33,13 +29,12 @@ const PipelineBuilderPage: React.FC = (props) => { }, } = props; - const editingPipeline = !!existingPipeline; - const initialValues: PipelineBuilderFormValues = { name: 'new-pipeline', params: [], resources: [], tasks: [], + listTasks: [], ...(convertPipelineToBuilderForm(existingPipeline) || {}), }; @@ -48,10 +43,10 @@ const PipelineBuilderPage: React.FC = (props) => { actions: FormikBag, ) => { let resourceCall; - if (editingPipeline) { + if (existingPipeline) { resourceCall = k8sUpdate( PipelineModel, - _.merge({}, existingPipeline, convertBuilderFormToPipeline(values, ns)), + convertBuilderFormToPipeline(values, ns, existingPipeline), ns, existingPipeline.metadata.name, ); @@ -74,51 +69,17 @@ const PipelineBuilderPage: React.FC = (props) => { Pipeline Builder -
- - -

Pipeline Builder

-
- - - - - - - {getBadgeFromType(PipelineModel.badge)} - -
-
-
( - + )} />
diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderVisualization.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderVisualization.tsx index 24f0ff8a813..a9f369f24ce 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderVisualization.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderVisualization.tsx @@ -1,41 +1,37 @@ import * as React from 'react'; import { Alert } from '@patternfly/react-core'; import { LoadingBox } from '@console/internal/components/utils'; -import { PipelineTask } from '../../../utils/pipeline-augment'; import { PipelineLayout } from '../pipeline-topology/const'; import PipelineTopologyGraph from '../pipeline-topology/PipelineTopologyGraph'; import { getEdgesFromNodes } from '../pipeline-topology/utils'; import { useNodes } from './hooks'; import { + PipelineBuilderTaskGroup, SelectTaskCallback, - SetTaskErrorCallback, TaskErrorMap, - UpdateTaskCallback, + UpdateTasksCallback, } from './types'; type PipelineBuilderVisualizationProps = { namespace: string; - onSetError: SetTaskErrorCallback; onTaskSelection: SelectTaskCallback; - onUpdateTasks: UpdateTaskCallback; - pipelineTasks: PipelineTask[]; + onUpdateTasks: UpdateTasksCallback; + taskGroup: PipelineBuilderTaskGroup; tasksInError: TaskErrorMap; }; const PipelineBuilderVisualization: React.FC = ({ namespace, - onSetError, onTaskSelection, onUpdateTasks, - pipelineTasks, + taskGroup, tasksInError, }) => { const { tasksLoaded, tasksCount, nodes, loadingTasksError } = useNodes( namespace, - onSetError, onTaskSelection, onUpdateTasks, - pipelineTasks, + taskGroup, tasksInError, ); @@ -49,7 +45,7 @@ const PipelineBuilderVisualization: React.FC if (!tasksLoaded) { return ; } - if (tasksCount === 0 && pipelineTasks.length === 0) { + if (tasksCount === 0 && taskGroup.tasks.length === 0) { // No tasks, nothing we can do here... return ; } diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/const.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/const.ts index e83d9357442..d2e2aa2b203 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/const.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/const.ts @@ -1 +1,26 @@ export const TASK_INCOMPLETE_ERROR_MESSAGE = 'Incomplete Task'; + +export enum UpdateOperationType { + ADD_LIST_TASK, + CONVERT_LIST_TO_TASK, + UPDATE_TASK, + REMOVE_TASK, + DELETE_LIST_TASK, +} + +export enum TaskErrorType { + MISSING_REQUIRED_PARAMS = 'missingParams', + MISSING_NAME = 'nameMissing', + MISSING_RESOURCES = 'missingResources', +} + +export const TASK_ERROR_STRINGS = { + [TaskErrorType.MISSING_RESOURCES]: 'Missing Resources', + [TaskErrorType.MISSING_REQUIRED_PARAMS]: 'Missing Parameters', + [TaskErrorType.MISSING_NAME]: 'Task Name is Required', +}; + +export const nodeTaskErrors = [ + TaskErrorType.MISSING_REQUIRED_PARAMS, + TaskErrorType.MISSING_RESOURCES, +]; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/hooks.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/hooks.ts index dc29ae85952..8a3b67359a2 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/hooks.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/hooks.ts @@ -2,40 +2,51 @@ import * as React from 'react'; import { k8sList } from '@console/internal/module/k8s'; import { ClusterTaskModel, TaskModel } from '../../../models'; import { + PipelineResource, PipelineResourceTask, PipelineTask, PipelineTaskRef, } from '../../../utils/pipeline-augment'; import { PipelineVisualizationTaskItem } from '../../../utils/pipeline-utils'; -import { getRandomChars } from '../pipeline-resource/pipelineResource-utils'; import { AddNodeDirection } from '../pipeline-topology/const'; -import { PipelineMixedNodeModel, PipelineTaskListNode } from '../pipeline-topology/types'; +import { + PipelineBuilderTaskNodeModel, + PipelineMixedNodeModel, + PipelineTaskListNodeModel, +} from '../pipeline-topology/types'; import { createTaskListNode, handleParallelToParallelNodes, tasksToBuilderNodes, } from '../pipeline-topology/utils'; import { + PipelineBuilderTaskGroup, SelectTaskCallback, - SetTaskErrorCallback, TaskErrorMap, - UpdateTaskCallback, + UpdateErrors, + UpdateOperationAddData, + UpdateOperationConvertToTaskData, + UpdateTasksCallback, } from './types'; -import { convertResourceToTask } from './utils'; +import { nodeTaskErrors, TaskErrorType, UpdateOperationType } from './const'; +import { getErrorMessage } from './utils'; type UseTasks = { namespacedTasks: PipelineResourceTask[] | null; clusterTasks: PipelineResourceTask[] | null; errorMsg?: string; - getTask: (taskRef: PipelineTaskRef) => PipelineResourceTask; }; -export const useTasks = (namespace?: string, taskNames?: string[]): UseTasks => { +export const useTasks = (namespace?: string): UseTasks => { const [namespacedTasks, setNamespacedTasks] = React.useState(null); const [clusterTasks, setClusterTasks] = React.useState(null); const [loadErrorMsg, setLoadErrorMsg] = React.useState(undefined); React.useEffect(() => { let ignore = false; + if (loadErrorMsg) { + return null; + } + if (!namespacedTasks) { if (!namespace) { setNamespacedTasks([]); @@ -78,19 +89,10 @@ export const useTasks = (namespace?: string, taskNames?: string[]): UseTasks => loadErrorMsg, ]); - const includedValues = (resource: PipelineResourceTask) => - !taskNames?.includes(resource.metadata.name); - return { - namespacedTasks: namespacedTasks?.filter(includedValues), - clusterTasks: clusterTasks?.filter(includedValues), + namespacedTasks, + clusterTasks, errorMsg: loadErrorMsg, - getTask: (taskRef) => { - if (taskRef.kind === ClusterTaskModel.kind) { - return clusterTasks.find((task) => task.metadata.name === taskRef.name); - } - return namespacedTasks.find((task) => task.metadata.name === taskRef.name); - }, }; }; @@ -102,61 +104,41 @@ type UseNodes = { }; export const useNodes = ( namespace: string, - onSetError: SetTaskErrorCallback, onTaskSelection: SelectTaskCallback, - onUpdateTasks: UpdateTaskCallback, - pipelineTasks: PipelineTask[], + onUpdateTasks: UpdateTasksCallback, + taskGroup: PipelineBuilderTaskGroup, tasksInError: TaskErrorMap, ): UseNodes => { - const { clusterTasks, namespacedTasks, errorMsg, getTask } = useTasks( - namespace, - pipelineTasks.map((plTask) => plTask.name), - ); - const [listNodes, setListNodes] = React.useState([]); + const { clusterTasks, namespacedTasks, errorMsg } = useTasks(namespace); - const pipelineTaskList = React.useRef(pipelineTasks); - pipelineTaskList.current = pipelineTasks; + const getTask = (taskRef: PipelineTaskRef) => { + if (taskRef.kind === ClusterTaskModel.kind) { + return clusterTasks.find((task) => task.metadata.name === taskRef.name); + } + return namespacedTasks.find((task) => task.metadata.name === taskRef.name); + }; - const newListNode = (name: string, runAfter?: string[]): PipelineTaskListNode => - createTaskListNode(name, { - namespaceTaskList: namespacedTasks, - clusterTaskList: clusterTasks, - onNewTask: (resource: PipelineResourceTask) => { - const newPipelineTask: PipelineTask = convertResourceToTask(resource, runAfter); - onUpdateTasks([ - ...pipelineTaskList.current.map((task) => { - if (!task.runAfter?.includes(name)) { - return task; - } - return { - ...task, - runAfter: task.runAfter.map((taskName) => { - if (taskName === name) { - return resource.metadata.name; - } - return taskName; - }), - }; - }), - newPipelineTask, - ]); - setListNodes(listNodes.filter((n) => n.id !== name)); + const taskGroupRef = React.useRef(taskGroup); + taskGroupRef.current = taskGroup; - const hasNonDefaultParams = newPipelineTask.params - ?.map(({ value }) => !value) - .reduce((acc, missingDefault) => missingDefault || acc, false); - const inputResourceCount = resource.spec?.inputs?.resources?.length || 0; - const outputResourceCount = resource.spec?.outputs?.resources?.length || 0; - const totalResourceCount = inputResourceCount + outputResourceCount; + const onNewListNode = (task: PipelineVisualizationTaskItem, direction: AddNodeDirection) => { + const data: UpdateOperationAddData = { direction, relatedTask: task }; + onUpdateTasks(taskGroupRef.current, { type: UpdateOperationType.ADD_LIST_TASK, data }); + }; + const onNewTask = (resource: PipelineResourceTask, name: string, runAfter?: string[]) => { + const data: UpdateOperationConvertToTaskData = { resource, name, runAfter }; + onUpdateTasks(taskGroupRef.current, { type: UpdateOperationType.CONVERT_LIST_TO_TASK, data }); + }; - if (hasNonDefaultParams || totalResourceCount > 0) { - onSetError( - newPipelineTask.name, - inputResourceCount, - outputResourceCount, - hasNonDefaultParams, - ); - } + // TODO: Fix id collisions then remove this utility; we shouldn't need to trim the tasks + const noDuplicates = (resource: PipelineResourceTask) => + !taskGroupRef.current.tasks.find((pt) => pt.name === resource.metadata.name); + const newListNode = (name: string, runAfter?: string[]): PipelineTaskListNodeModel => + createTaskListNode(name, { + namespaceTaskList: namespacedTasks?.filter(noDuplicates), + clusterTaskList: clusterTasks?.filter(noDuplicates), + onNewTask: (resource: PipelineResourceTask) => { + onNewTask(resource, name, runAfter); }, task: { name, @@ -164,91 +146,23 @@ export const useNodes = ( }, }); - const onNewListNode = (task: PipelineVisualizationTaskItem, direction: AddNodeDirection) => { - let newNode: PipelineTaskListNode; - switch (direction) { - case AddNodeDirection.AFTER: { - const taskName = `new-after-node-${getRandomChars(6)}`; - - onUpdateTasks( - pipelineTaskList.current.map((pipelineTask) => { - if (!pipelineTask?.runAfter?.includes(task.name)) { - return pipelineTask; - } - - const remainingRunAfters = (pipelineTask.runAfter || []).filter( - (runAfterName) => runAfterName !== task.name, - ); - - return { - ...pipelineTask, - runAfter: [...remainingRunAfters, taskName], - }; - }), - ); - newNode = newListNode(taskName, [task.name]); - break; - } - case AddNodeDirection.BEFORE: { - const taskName = `new-before-node-${getRandomChars(6)}`; - const pipelineMap: { [name: string]: PipelineTask } = pipelineTaskList.current.reduce( - (map, pipelineTask) => ({ ...map, [pipelineTask.name]: pipelineTask }), - {}, - ); - - const runAfterItem = pipelineMap[task.name]; - const existingRunAfters = runAfterItem.runAfter || []; - pipelineMap[task.name] = { - ...pipelineMap[task.name], - runAfter: [taskName], - }; - - newNode = newListNode(taskName, existingRunAfters); - onUpdateTasks(Object.values(pipelineMap)); - break; - } - case AddNodeDirection.PARALLEL: { - const taskName = `new-parallel-node-${getRandomChars(6)}`; - const pipelineMap: { [name: string]: PipelineTask } = pipelineTaskList.current.reduce( - (map, pipelineTask) => ({ ...map, [pipelineTask.name]: pipelineTask }), - {}, - ); - - const runParallelItem = pipelineMap[task.name]; - const myRunAfters: string[] = runParallelItem.runAfter || []; - onUpdateTasks( - pipelineTaskList.current.map((pipelineTask) => { - const currentRunAfters = pipelineTask?.runAfter || []; - if (!currentRunAfters.includes(runParallelItem.name)) { - return pipelineTask; - } - - return { - ...pipelineTask, - runAfter: [...currentRunAfters, taskName], - }; - }), - ); - - newNode = newListNode(taskName, myRunAfters); - break; - } - default: - throw new Error(`Invalid direction ${direction}`); - } - setListNodes([...listNodes, newNode]); - }; - - const existingNodes: PipelineMixedNodeModel[] = - pipelineTasks.length > 0 - ? tasksToBuilderNodes(pipelineTasks, tasksInError, onNewListNode, (task) => - onTaskSelection(task, getTask(task.taskRef)), + const taskNodes: PipelineBuilderTaskNodeModel[] = + taskGroup.tasks.length > 0 + ? tasksToBuilderNodes( + taskGroup.tasks, + onNewListNode, + (task) => onTaskSelection(task, getTask(task.taskRef)), + getErrorMessage(nodeTaskErrors, tasksInError), ) - : [newListNode('initial-node')]; + : []; + const taskListNodes: PipelineTaskListNodeModel[] = + taskGroup.tasks.length === 0 && taskGroup.listTasks.length === 0 + ? [newListNode('initial-node')] + : taskGroup.listTasks.map((listTask) => newListNode(listTask.name, listTask.runAfter)); const nodes: PipelineMixedNodeModel[] = handleParallelToParallelNodes([ - ...existingNodes, - ...listNodes, + ...taskNodes, + ...taskListNodes, ]); const localTaskCount = namespacedTasks?.length || 0; @@ -261,3 +175,28 @@ export const useNodes = ( nodes, }; }; + +export const useResourceValidation = ( + tasks: PipelineTask[], + resourceValues: PipelineResource[], + onError: UpdateErrors, +) => { + React.useEffect(() => { + const resourceNames = resourceValues.map((r) => r.name); + + const errors = tasks.reduce((acc, task) => { + const output = task.resources?.outputs || []; + const input = task.resources?.inputs || []; + const missingResources = [...output, ...input].filter( + (r) => !resourceNames.includes(r.resource), + ); + + return { + ...acc, + [task.name]: missingResources.length !== 0 ? [TaskErrorType.MISSING_RESOURCES] : null, + }; + }, {}); + + onError(errors); + }, [tasks, resourceValues, onError]); +}; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/Sidebar.scss b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/Sidebar.scss index b620e0b15f8..5c56bd96140 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/Sidebar.scss +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/Sidebar.scss @@ -21,7 +21,7 @@ $overview-sidebar-width: 550px; } &__content { - padding: var(--pf-global--spacer--lg); + padding: var(--pf-global--spacer--lg) 0; } // CSSTransitionGroup classes diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.scss b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.scss index 4527f89cab3..bc33dc30c34 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.scss +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.scss @@ -1,5 +1,14 @@ .odc-task-sidebar { &__header { font-size: var(--pf-global--FontSize--md); + margin: 0; + } + + &__header, &__content { + padding: 0 var(--pf-global--spacer--lg); + } + + &__param, &__resource { + margin-bottom: var(--pf-global--spacer--md); } } diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.tsx index 1663aac3d35..f015c91ce40 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.tsx @@ -1,12 +1,9 @@ import * as React from 'react'; -import * as _ from 'lodash'; import { useField } from 'formik'; -import { TextInputTypes } from '@patternfly/react-core'; -import { InputField } from '@console/shared'; import { ActionsMenu, ResourceIcon } from '@console/internal/components/utils'; import { referenceFor } from '@console/internal/module/k8s'; import { - getResourceModelFromTask, + getResourceModelFromTaskKind, PipelineResource, PipelineResourceTask, PipelineResourceTaskResource, @@ -14,37 +11,38 @@ import { PipelineTaskParam, PipelineTaskResource, } from '../../../../utils/pipeline-augment'; +import { ResourceTarget, TaskErrorMap, UpdateOperationUpdateTaskData } from '../types'; +import { TaskErrorType } from '../const'; import TaskSidebarParam from './TaskSidebarParam'; -import { convertResourceToTask } from '../utils'; import TaskSidebarResource from './TaskSidebarResource'; -import { TaskErrorMap } from '../types'; +import TaskSidebarName from './TaskSidebarName'; + +import './TaskSidebar.scss'; type TaskSidebarProps = { errorMap: TaskErrorMap; onRemoveTask: (taskName: string) => void; + onUpdateTask: (data: UpdateOperationUpdateTaskData) => void; resourceList: PipelineResource[]; - setFieldValue: (formikId: string, newValue: any) => void; selectedPipelineTaskIndex: number; taskResource: PipelineResourceTask; - updateErrorMap: (errorMap: TaskErrorMap) => void; }; const TaskSidebar: React.FC = (props) => { const { errorMap, onRemoveTask, + onUpdateTask, resourceList, - setFieldValue, selectedPipelineTaskIndex, taskResource, - updateErrorMap, } = props; const formikTaskReference = `tasks.${selectedPipelineTaskIndex}`; const [taskField] = useField(formikTaskReference); - if (!taskField?.value?.name) { - return null; - } + const updateTask = (newData: Partial) => { + onUpdateTask({ thisPipelineTask: taskField.value, taskResource, ...newData }); + }; const thisTaskError = errorMap[taskField.value.name]; @@ -52,141 +50,106 @@ const TaskSidebar: React.FC = (props) => { const inputResources = taskResource.spec?.inputs?.resources; const outputResources = taskResource.spec?.outputs?.resources; - const renderResource = (type: 'inputs' | 'outputs') => ( - resource: PipelineResourceTaskResource, - ) => { + const renderResource = (type: ResourceTarget) => (resource: PipelineResourceTaskResource) => { const taskResources: PipelineTaskResource[] = taskField.value?.resources?.[type] || []; const thisResource = taskResources.find( (taskFieldResource) => taskFieldResource.name === resource.name, ); return ( - { - const newResources = [ - ...taskResources - .map((tResource) => { - if (tResource.name === resourceName) { - return null; - } - return taskResource; - }) - .filter((r) => !!r), - { - name: resourceName, - resource: selectedResource.name, - }, - ]; - setFieldValue(`${formikTaskReference}.resources.${type}`, newResources); - - const id = type === 'inputs' ? 'inputResourceCount' : 'outputResourceCount'; - if (thisTaskError && thisTaskError[id] === newResources.length) { - // Has errors but no longer resource errors - if (!thisTaskError[id]) { - return; - } - - updateErrorMap({ - ...errorMap, - [taskField.value.name]: _.omit(thisTaskError, id), +
+ { + updateTask({ + resources: { + resourceTarget: type, + selectedPipelineResource: selectedResource, + taskResourceName: resourceName, + }, }); - } - }} - taskResource={thisResource} - resource={resource} - /> + }} + taskResource={thisResource} + resource={resource} + /> +
); }; return (
-

-
- - {taskResource.metadata.name} -
-
- onRemoveTask(taskField.value.name), - }, - ]} - /> -
-

- - - {params && ( - <> -

Parameters

- {params.map((param) => { - const taskParams = taskField.value?.params || []; - const thisParam = taskParams.find( - (taskFieldParam) => taskFieldParam.name === param.name, - ); - return ( - { - const newParams = taskParams.map((taskParam) => { - if (taskParam === thisParam) { - return { - ...taskParam, - value, - } as PipelineTaskParam; - } - return taskParam; - }); - setFieldValue(formikTaskReference, { - ...taskField.value, - params: newParams, - }); +
+

+
+ + {taskResource.metadata.name} +
+
+ onRemoveTask(taskField.value.name), + }, + ]} + /> +
+

+
+
- const datalessParams = newParams.filter((p) => !p.value).length > 0; - if (thisTaskError && !datalessParams) { - // Has errors but no longer param errors - if (!thisTaskError.paramsMissingDefaults) { - return; - } +
+ updateTask({ newName })} + /> - updateErrorMap({ - ...errorMap, - [taskField.value.name]: _.omit(thisTaskError, 'paramsMissingDefaults'), - }); - } - }} - /> - ); - })} - - )} + {params && ( + <> +

Parameters

+ {params.map((param) => { + const taskParams: PipelineTaskParam[] = taskField.value?.params || []; + const thisParam = taskParams.find( + (taskFieldParam) => taskFieldParam.name === param.name, + ); + return ( +
+ { + updateTask({ + params: { + newValue: value, + taskParamName: param.name, + }, + }); + }} + /> +
+ ); + })} + + )} - {inputResources && ( - <> -

Input Resources

- {inputResources && inputResources.map(renderResource('inputs'))} - - )} - {outputResources && ( - <> -

Output Resources

- {outputResources && outputResources.map(renderResource('outputs'))} - - )} + {inputResources && ( + <> +

Input Resources

+ {inputResources.map(renderResource('inputs'))} + + )} + {outputResources && ( + <> +

Output Resources

+ {outputResources.map(renderResource('outputs'))} + + )} +
); }; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarName.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarName.tsx new file mode 100644 index 00000000000..20f9982b572 --- /dev/null +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarName.tsx @@ -0,0 +1,59 @@ +import * as React from 'react'; +import { FormGroup, TextInput, TextInputTypes } from '@patternfly/react-core'; + +type TaskSidebarNameProps = { + initialName: string; + onChange: (newName: string) => void; + taskName: string; +}; + +const VALID_NAME = /^([a-z]([-a-z0-9]*[a-z0-9])?)*$/; +const INVALID_ERROR_MESSAGE = + 'Name must consist of lower-case letters, numbers and hyphens. It must start with a letter and end with a letter or number.'; + +const getError = (value: string): string | null => { + let error = null; + if (value === '') { + error = 'Required'; + } else if (!VALID_NAME.test(value)) { + error = INVALID_ERROR_MESSAGE; + } + return error; +}; + +const TaskSidebarName: React.FC = (props) => { + const { initialName, onChange, taskName } = props; + const [interimName, setInterimName] = React.useState(initialName); + const [error, setError] = React.useState(null); + const isValid = !error; + + return ( + + { + setInterimName(value); + setError(getError(value)); + }} + onBlur={() => { + if (isValid) { + onChange(interimName); + } + }} + placeholder={taskName} + type={TextInputTypes.text} + value={interimName} + /> + + ); +}; + +export default TaskSidebarName; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarParam.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarParam.tsx index 966d630b3d2..f1765bceecf 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarParam.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebarParam.tsx @@ -3,30 +3,40 @@ import { FormGroup, TextInput } from '@patternfly/react-core'; import { PipelineResourceTaskParam, PipelineTaskParam } from '../../../../utils/pipeline-augment'; type TaskSidebarParamProps = { + hasParamError?: boolean; resourceParam: PipelineResourceTaskParam; taskParam?: PipelineTaskParam; onChange: (newValue: string) => void; }; const TaskSidebarParam: React.FC = (props) => { - const { onChange, resourceParam, taskParam } = props; + const { hasParamError, onChange, resourceParam, taskParam } = props; + const [dirty, setDirty] = React.useState(false); + + const currentValue = taskParam.value?.toString() || ''; + const emptyIsInvalid = !resourceParam.default; + + const isValid = !(dirty && hasParamError && emptyIsInvalid && currentValue === ''); return ( setDirty(true)} onChange={(value) => { onChange(value); }} + placeholder={resourceParam.default} + value={currentValue} /> ); diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/types.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/types.ts index 2dfd6340505..962ffe19a99 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/types.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/types.ts @@ -6,12 +6,24 @@ import { PipelineTask, } from '../../../utils/pipeline-augment'; import { PipelineVisualizationTaskItem } from '../../../utils/pipeline-utils'; +import { AddNodeDirection } from '../pipeline-topology/const'; +import { TaskErrorType, UpdateOperationType } from './const'; -export type PipelineBuilderFormValues = { +export type UpdateErrors = (errors?: TaskErrorMap) => void; + +export type PipelineBuilderTaskBase = { name: string; runAfter?: string[] }; + +export type PipelineBuilderListTask = PipelineBuilderTaskBase; + +export type PipelineBuilderTaskGroup = { + tasks: PipelineTask[]; + listTasks: PipelineBuilderListTask[]; +}; + +export type PipelineBuilderFormValues = PipelineBuilderTaskGroup & { name: string; params: PipelineParam[]; resources: PipelineResource[]; - tasks: PipelineTask[]; }; export type PipelineBuilderFormikValues = FormikValues & PipelineBuilderFormValues; @@ -21,25 +33,69 @@ export type SelectedBuilderTask = { taskIndex: number; }; -export type TaskErrorMapData = { - inputResourceCount: number; - outputResourceCount: number; - paramsMissingDefaults: boolean; - message: string; -}; export type TaskErrorMap = { - [pipelineInErrorName: string]: TaskErrorMapData; + [pipelineInErrorName: string]: TaskErrorType[]; }; -export type SetTaskErrorCallback = ( - pipelineInErrorName: string, - inputResourceCount: number, - outputResourceCount: number, - paramsMissingDefaults: boolean, -) => void; - export type SelectTaskCallback = ( task: PipelineVisualizationTaskItem, taskResource: PipelineResourceTask, ) => void; -export type UpdateTaskCallback = (updatedTaskList: PipelineTask[]) => void; + +export type UpdateOperation = { + type: UpdateOperationType; + data: D; +}; + +export type UpdateTasksCallback = ( + taskGroup: PipelineBuilderTaskGroup, + op: UpdateOperation, +) => void; + +type UpdateOperationBaseData = {}; + +export type UpdateOperationAddData = UpdateOperationBaseData & { + direction: AddNodeDirection; + relatedTask: PipelineVisualizationTaskItem; +}; +export type UpdateOperationConvertToTaskData = UpdateOperationBaseData & { + name: string; + resource: PipelineResourceTask; + runAfter?: string[]; +}; +export type UpdateOperationRemoveTaskData = UpdateOperationBaseData & { + taskName: string; +}; + +export type ResourceTarget = 'inputs' | 'outputs'; +export type UpdateTaskResourceData = { + resourceTarget: ResourceTarget; + selectedPipelineResource: PipelineResource; + taskResourceName: string; +}; +export type UpdateTaskParamData = { + newValue: string; + taskParamName: string; +}; +export type UpdateOperationUpdateTaskData = UpdateOperationBaseData & { + // Task information + thisPipelineTask: PipelineTask; + taskResource: PipelineResourceTask; + + // Change information + newName?: string; + params?: UpdateTaskParamData; + resources?: UpdateTaskResourceData; +}; + +export type CleanupResults = { + tasks: PipelineTask[]; + listTasks: PipelineBuilderListTask[]; + errors?: TaskErrorMap; +}; + +export type UpdateOperationAction = ( + tasks: PipelineTask[], + listTasks: PipelineBuilderListTask[], + data: D, +) => CleanupResults; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/update-utils.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/update-utils.ts new file mode 100644 index 00000000000..14653073fe5 --- /dev/null +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/update-utils.ts @@ -0,0 +1,358 @@ +import { + PipelineResource, + PipelineResourceTask, + PipelineTask, + PipelineTaskParam, + PipelineTaskResource, +} from '../../../utils/pipeline-augment'; +import { getRandomChars } from '../pipeline-resource/pipelineResource-utils'; +import { AddNodeDirection } from '../pipeline-topology/const'; +import { TaskErrorType, UpdateOperationType } from './const'; +import { + CleanupResults, + PipelineBuilderListTask, + PipelineBuilderTaskBase, + PipelineBuilderTaskGroup, + TaskErrorMap, + UpdateOperation, + UpdateOperationAction, + UpdateOperationAddData, + UpdateOperationConvertToTaskData, + UpdateOperationRemoveTaskData, + UpdateOperationUpdateTaskData, + UpdateTaskParamData, + UpdateTaskResourceData, +} from './types'; +import { convertResourceToTask } from './utils'; + +const mapReplaceRelatedInOthers = ( + taskName: string, + relatedTaskName: string, + iterationTask: TaskType, +): TaskType => { + if (!iterationTask?.runAfter?.includes(relatedTaskName)) { + return iterationTask; + } + + const remainingRunAfters = iterationTask.runAfter.filter( + (runAfterName) => runAfterName !== relatedTaskName, + ); + + return { + ...iterationTask, + runAfter: [...remainingRunAfters, taskName], + }; +}; + +const mapRemoveRelatedInOthers = ( + taskName: string, + iterationTask: TaskType, +): TaskType => { + if (!iterationTask?.runAfter?.includes(taskName)) { + return iterationTask; + } + + return { + ...iterationTask, + runAfter: iterationTask.runAfter.filter((runAfterName) => runAfterName !== taskName), + }; +}; + +const mapStitchReplaceInOthers = ( + removalTask: PipelineBuilderTaskBase, + iterationTask: TaskType, +): TaskType => { + if (!removalTask?.runAfter) { + return mapRemoveRelatedInOthers(removalTask.name, iterationTask); + } + if (!iterationTask?.runAfter?.includes(removalTask.name)) { + return iterationTask; + } + + const updatedIterationTask = mapRemoveRelatedInOthers(removalTask.name, iterationTask); + if (updatedIterationTask.runAfter.length > 0) { + return updatedIterationTask; + } + + return { + ...updatedIterationTask, + runAfter: removalTask.runAfter, + }; +}; + +const mapBeRelated = ( + newTaskName: string, + relatedTaskName: string, + iterationTask: TaskType, +): TaskType => { + if (iterationTask.name !== relatedTaskName) { + return iterationTask; + } + + return { + ...iterationTask, + runAfter: [newTaskName], + }; +}; + +const mapAddRelatedToOthers = ( + taskName: string, + relatedTaskName: string, + iterationTask: TaskType, +): TaskType => { + if (!iterationTask?.runAfter?.includes(relatedTaskName)) { + return iterationTask; + } + + return { + ...iterationTask, + runAfter: [...iterationTask.runAfter, taskName], + }; +}; + +// TODO: Can we use yup? Do we need this level of checking for errors? +const getErrors = (task: PipelineTask, resource: PipelineResourceTask): TaskErrorMap => { + const resourceParams = resource?.spec?.inputs?.params || []; + const requiredParamNames = resourceParams + .filter((param) => !param.default) + .map((param) => param.name); + const hasNonDefaultParams = task.params + ?.filter(({ name }) => requiredParamNames.includes(name)) + ?.map(({ value }) => !value) + .reduce((acc, missingDefault) => missingDefault || acc, false); + + const needsName = !task.name; + + const taskInputResources = task.resources?.inputs?.length || 0; + const requiredInputResources = resource.spec?.inputs?.resources?.length || 0; + const missingInputResources = requiredInputResources - taskInputResources > 0; + + const taskOutputResources = task.resources?.outputs?.length || 0; + const requiredOutputResources = resource.spec?.outputs?.resources?.length || 0; + const missingOutputResources = requiredOutputResources - taskOutputResources > 0; + + const errorListing: TaskErrorType[] = []; + if (hasNonDefaultParams) { + errorListing.push(TaskErrorType.MISSING_REQUIRED_PARAMS); + } + if (missingInputResources || missingOutputResources) { + errorListing.push(TaskErrorType.MISSING_RESOURCES); + } + if (needsName) { + errorListing.push(TaskErrorType.MISSING_NAME); + } + + return { [task.name]: errorListing.length > 0 ? errorListing : null }; +}; + +const addListNode: UpdateOperationAction = (tasks, listTasks, data) => { + const { direction, relatedTask } = data; + + const newTaskName = `${direction}-${getRandomChars(6)}`; + const relatedTaskName = relatedTask.name; + + switch (direction) { + case AddNodeDirection.BEFORE: + return { + tasks: tasks.map((pipelineTask) => + mapBeRelated(newTaskName, relatedTaskName, pipelineTask), + ), + listTasks: [ + ...listTasks.map((listTask) => + mapBeRelated(newTaskName, relatedTaskName, listTask), + ), + { name: newTaskName, runAfter: relatedTask.runAfter }, + ], + }; + case AddNodeDirection.AFTER: + return { + tasks: tasks.map((pipelineTask) => + mapReplaceRelatedInOthers(newTaskName, relatedTaskName, pipelineTask), + ), + listTasks: [ + ...listTasks.map((listTask) => + mapReplaceRelatedInOthers( + newTaskName, + relatedTaskName, + listTask, + ), + ), + { name: newTaskName, runAfter: [relatedTaskName] }, + ], + }; + case AddNodeDirection.PARALLEL: + return { + tasks: tasks.map((pipelineTask) => + mapAddRelatedToOthers(newTaskName, relatedTaskName, pipelineTask), + ), + listTasks: [ + ...listTasks.map((listTask) => + mapAddRelatedToOthers(newTaskName, relatedTaskName, listTask), + ), + { name: newTaskName, runAfter: relatedTask.runAfter }, + ], + }; + default: + throw new Error(`Invalid direction ${direction}`); + } +}; + +const convertListToTask: UpdateOperationAction = ( + tasks, + listTasks, + data, +) => { + const { name, resource, runAfter } = data; + + const newPipelineTask: PipelineTask = convertResourceToTask(resource, runAfter); + + return { + tasks: [ + ...tasks.map((pipelineTask) => + mapReplaceRelatedInOthers(newPipelineTask.name, name, pipelineTask), + ), + newPipelineTask, + ], + listTasks: listTasks + .filter((n) => n.name !== name) + .map((listTask) => mapReplaceRelatedInOthers(newPipelineTask.name, name, listTask)), + errors: getErrors(newPipelineTask, resource), + }; +}; + +export const removeTask: UpdateOperationAction = ( + tasks, + listTasks, + data, +) => { + const { taskName } = data; + + const removalTask = tasks.find((pipelineTask) => pipelineTask.name === taskName); + return { + tasks: tasks + .filter((pipelineTask) => pipelineTask.name !== taskName) + .map((pipelineTask) => mapStitchReplaceInOthers(removalTask, pipelineTask)), + listTasks: listTasks.map((pipelineTask) => + mapStitchReplaceInOthers(removalTask, pipelineTask), + ), + errors: null, + }; +}; + +const applyResourceUpdate = ( + pipelineTask: PipelineTask, + resources: UpdateTaskResourceData, +): PipelineTask => { + const { resourceTarget, selectedPipelineResource, taskResourceName } = resources; + + const existingResources: PipelineResource[] = pipelineTask.resources?.[resourceTarget] || []; + const filteredResources = existingResources.filter((resource: PipelineTaskResource) => { + return resource.name !== taskResourceName; + }); + + return { + ...pipelineTask, + resources: { + ...pipelineTask.resources, + [resourceTarget]: [ + ...filteredResources, + { + name: taskResourceName, + resource: selectedPipelineResource.name, + }, + ], + }, + }; +}; + +const applyParamsUpdate = ( + pipelineTask: PipelineTask, + params: UpdateTaskParamData, +): PipelineTask => { + const { newValue, taskParamName } = params; + + return { + ...pipelineTask, + params: pipelineTask.params.map( + (param): PipelineTaskParam => { + if (param.name !== taskParamName) { + return param; + } + + return { + ...param, + value: newValue, + }; + }, + ), + }; +}; + +const updateTask: UpdateOperationAction = ( + tasks, + listTasks, + data, +) => { + const { thisPipelineTask, taskResource, newName, params, resources } = data; + + const canRename = !!newName; + + const updatedResourceIndex = tasks.findIndex( + (pipelineTask) => pipelineTask.name === thisPipelineTask.name, + ); + const updatedTasks = tasks.map((pipelineTask) => { + if (pipelineTask.name !== thisPipelineTask.name) { + if (canRename) { + return mapReplaceRelatedInOthers(newName, thisPipelineTask.name, pipelineTask); + } + return pipelineTask; + } + + let updatedResource = pipelineTask; + if (resources) { + updatedResource = applyResourceUpdate(updatedResource, resources); + } + if (params) { + updatedResource = applyParamsUpdate(updatedResource, params); + } + if (canRename) { + updatedResource = { + ...updatedResource, + name: newName, + }; + } + + return updatedResource; + }); + const updatedResource = updatedTasks[updatedResourceIndex]; + + return { + tasks: updatedTasks, + listTasks: listTasks.map( + (listTask) => + canRename && mapReplaceRelatedInOthers(newName, thisPipelineTask.name, listTask), + ), + errors: getErrors(updatedResource, taskResource), + }; +}; + +export const applyChange = ( + taskGroup: PipelineBuilderTaskGroup, + op: UpdateOperation, +): CleanupResults => { + const { type, data } = op; + const { tasks, listTasks } = taskGroup; + + switch (type) { + case UpdateOperationType.ADD_LIST_TASK: + return addListNode(tasks, listTasks, data as UpdateOperationAddData); + case UpdateOperationType.CONVERT_LIST_TO_TASK: + return convertListToTask(tasks, listTasks, data as UpdateOperationConvertToTaskData); + case UpdateOperationType.REMOVE_TASK: + return removeTask(tasks, listTasks, data as UpdateOperationRemoveTaskData); + case UpdateOperationType.UPDATE_TASK: + return updateTask(tasks, listTasks, data as UpdateOperationUpdateTaskData); + default: + throw new Error(`Invalid update operation ${type}`); + } +}; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/utils.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/utils.ts index 5a5155f856a..4f9f34d7717 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/utils.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/utils.ts @@ -1,7 +1,23 @@ +import { history, resourcePathFromModel } from '@console/internal/components/utils'; import { apiVersionForModel, referenceForModel } from '@console/internal/module/k8s'; -import { Pipeline, PipelineResourceTask, PipelineTask } from '../../../utils/pipeline-augment'; import { ClusterTaskModel, PipelineModel } from '../../../models'; -import { PipelineBuilderFormikValues, PipelineBuilderFormValues } from './types'; +import { Pipeline, PipelineResourceTask, PipelineTask } from '../../../utils/pipeline-augment'; +import { TASK_ERROR_STRINGS, TaskErrorType } from './const'; +import { PipelineBuilderFormikValues, PipelineBuilderFormValues, TaskErrorMap } from './types'; + +export const getErrorMessage = (errorTypes: TaskErrorType[], errorMap: TaskErrorMap) => ( + taskName: string, +): string => { + if (!taskName) { + return TASK_ERROR_STRINGS[TaskErrorType.MISSING_NAME]; + } + + const errorList: TaskErrorType[] | undefined = errorMap?.[taskName]; + if (!errorList) return null; + + const hasRequestedError = errorList.filter((error) => errorTypes.includes(error)); + return hasRequestedError.length > 0 ? TASK_ERROR_STRINGS[hasRequestedError[0]] : null; +}; export const convertResourceToTask = ( resource: PipelineResourceTask, @@ -25,23 +41,56 @@ export const getPipelineURL = (namespace: string) => { return `/k8s/ns/${namespace}/${referenceForModel(PipelineModel)}`; }; +const removeListRunAfters = (task: PipelineTask, listIds: string[]): PipelineTask => { + if (task?.runAfter && listIds.length > 0) { + // Trim out any runAfters pointing at list nodes + const runAfter = (task.runAfter || []).filter( + (runAfterName) => !listIds.includes(runAfterName), + ); + + return { + ...task, + runAfter, + }; + } + + return task; +}; + +const removeEmptyDefaultParams = (task: PipelineTask): PipelineTask => { + if (task.params?.length > 0) { + // Since we can submit, this param has a default; check for empty values and remove + return { + ...task, + params: task.params.filter((param) => !!param.value), + }; + } + + return task; +}; + export const convertBuilderFormToPipeline = ( formValues: PipelineBuilderFormikValues, namespace: string, + existingPipeline: Pipeline = {}, ): Pipeline => { - const { name, resources, params, tasks } = formValues; + const { name, resources, params, tasks, listTasks } = formValues; + const listIds = listTasks.map((listTask) => listTask.name); return { + ...existingPipeline, apiVersion: apiVersionForModel(PipelineModel), kind: PipelineModel.kind, metadata: { + ...existingPipeline.metadata, name, namespace, }, spec: { + ...existingPipeline.spec, params, resources, - tasks, + tasks: tasks.map((task) => removeEmptyDefaultParams(removeListRunAfters(task, listIds))), }, }; }; @@ -59,5 +108,18 @@ export const convertPipelineToBuilderForm = (pipeline: Pipeline): PipelineBuilde params, resources, tasks, + listTasks: [], }; }; + +export const goToYAML = (existingPipeline?: Pipeline, namespace?: string) => { + history.push( + existingPipeline + ? `${resourcePathFromModel( + PipelineModel, + existingPipeline?.metadata?.name, + existingPipeline?.metadata?.namespace, + )}/yaml` + : `${getPipelineURL(namespace)}/~new`, + ); +}; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/validation-utils.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/validation-utils.ts index 774cf5422c5..6c9faafa51d 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/validation-utils.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-builder/validation-utils.ts @@ -1,28 +1,51 @@ import * as yup from 'yup'; +const taskResourceValidation = yup.array().of( + yup.object({ + name: yup.string().required('Required'), + resource: yup.string().required('Required'), + }), +); + export const validationSchema = yup.object({ name: yup.string().required('Required'), params: yup.array().of( yup.object({ - name: yup - .string() - .min(1) - .required('Required'), + name: yup.string().required('Required'), description: yup.string(), default: yup.string(), }), ), resources: yup.array().of( yup.object({ - name: yup - .string() - .min(1) - .required('Required'), + name: yup.string().required('Required'), type: yup.string().required('Required'), }), ), tasks: yup .array() + .of( + yup.object({ + name: yup.string().required('Required'), + runAfter: yup.array().of(yup.string()), + taskRef: yup + .object({ + name: yup.string().required('Required'), + kind: yup.string(), + }) + .required('Required'), + resources: yup.object({ + inputs: taskResourceValidation, + outputs: taskResourceValidation, + }), + }), + ) .min(1, 'Must define at least one task') .required('Required'), + taskList: yup.array().of( + yup.object({ + name: yup.string().required('Required'), + runAfter: yup.string(), + }), + ), }); diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/BuilderNode.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/BuilderNode.tsx index 0d08de4ebb9..f99a035b9f6 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/BuilderNode.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/BuilderNode.tsx @@ -28,12 +28,12 @@ const BuilderNode: React.FC<{ element: Node }> = ({ element }) => { fill="transparent" /> onNodeSelection(data)}> - - {error?.message && ( + + {error && ( )} diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineTopologyGraph.scss b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineTopologyGraph.scss index 6dac5639727..3dac288154c 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineTopologyGraph.scss +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineTopologyGraph.scss @@ -5,5 +5,4 @@ font-size: var(--pf-global--FontSize--xs); max-width: 100%; overflow: auto; - padding: var(--pf-global--spacer--lg) var(--pf-global--spacer--xl); } diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineVisualizationSurface.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineVisualizationSurface.tsx index f8547271c2b..711e6af17dc 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineVisualizationSurface.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/PipelineVisualizationSurface.tsx @@ -1,6 +1,11 @@ import * as React from 'react'; -import { Model, Visualization, VisualizationSurface } from '@console/topology'; -import { LayoutCallback } from '@console/topology/src/layouts/DagreLayout'; +import { + GRAPH_LAYOUT_END_EVENT, + Model, + Node, + Visualization, + VisualizationSurface, +} from '@console/topology'; import { componentFactory, layoutFactory } from './factories'; import { DROP_SHADOW_SPACING, NODE_WIDTH, NODE_HEIGHT, PipelineLayout } from './const'; import { getLayoutData } from './utils'; @@ -15,11 +20,15 @@ const PipelineVisualizationSurface: React.FC const layout: PipelineLayout = model.graph.layout as PipelineLayout; - const onLayoutUpdate: LayoutCallback = React.useCallback( - (nodes) => { + const onLayoutUpdate = React.useCallback( + (nodes: Node[]) => { const nodeBounds = nodes.map((node) => node.getBounds()); - const maxX = nodeBounds.map((bounds) => bounds.x).reduce((x1, x2) => Math.max(x1, x2), 0); - const maxY = nodeBounds.map((bounds) => bounds.y).reduce((y1, y2) => Math.max(y1, y2), 0); + const maxX = Math.floor( + nodeBounds.map((bounds) => bounds.x).reduce((x1, x2) => Math.max(x1, x2), 0), + ); + const maxY = Math.floor( + nodeBounds.map((bounds) => bounds.y).reduce((y1, y2) => Math.max(y1, y2), 0), + ); let horizontalMargin = 0; let verticalMargin = 0; @@ -30,8 +39,8 @@ const PipelineVisualizationSurface: React.FC setMaxSize({ // Nodes are rendered from the top-left - height: maxY + NODE_HEIGHT + DROP_SHADOW_SPACING + verticalMargin, - width: maxX + NODE_WIDTH + horizontalMargin, + height: maxY + NODE_HEIGHT + DROP_SHADOW_SPACING + verticalMargin * 2, + width: maxX + NODE_WIDTH + horizontalMargin * 2, }); }, [setMaxSize, layout], @@ -40,9 +49,12 @@ const PipelineVisualizationSurface: React.FC React.useEffect(() => { if (vis === null) { const visualization = new Visualization(); - visualization.registerLayoutFactory(layoutFactory(onLayoutUpdate)); + visualization.registerLayoutFactory(layoutFactory); visualization.registerComponentFactory(componentFactory); visualization.fromModel(model); + visualization.addEventListener(GRAPH_LAYOUT_END_EVENT, () => { + onLayoutUpdate(visualization.getGraph().getNodes()); + }); setVis(visualization); } else { vis.fromModel(model); diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/TaskNode.tsx b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/TaskNode.tsx index ef28e7da3c0..f9eb9627f62 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/TaskNode.tsx +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/TaskNode.tsx @@ -5,7 +5,10 @@ import { PipelineVisualizationTask } from '../detail-page-tabs/pipeline-details/ import { DROP_SHADOW_SPACING } from './const'; import { TaskNodeModelData } from './types'; -const TaskNode: React.FC<{ element: Node }> = ({ element }) => { +const TaskNode: React.FC<{ element: Node; disableTooltip?: boolean }> = ({ + element, + disableTooltip, +}) => { const { height, width } = element.getBounds(); const { pipeline, pipelineRun, task } = element.getData() as TaskNodeModelData; @@ -16,6 +19,7 @@ const TaskNode: React.FC<{ element: Node }> = ({ element }) => { task={task} pipelineRunStatus={pipelineRun && pipelineRunFilterReducer(pipelineRun)} namespace={pipeline?.metadata?.namespace} + disableTooltip={disableTooltip} /> ); diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/const.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/const.ts index 4ca046489f8..7c7bfbc6c8e 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/const.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/const.ts @@ -25,25 +25,45 @@ export enum PipelineLayout { } export enum AddNodeDirection { + /** + * Rules today: + * - the `relatedTask` is pointing at ONLY us + * - we inherit all that `relatedTask` is pointing at + */ BEFORE = 'in-run-after', + + /** + * Rules today: + * - the `relatedTask` must be our ONLY runAfter + * - we are added to all that is pointing at `relatedTask` + */ AFTER = 'has-run-after', + + /** + * Rules today: + * - we inherit all that `relatedTask` is pointing at + * - we are added to all that is pointing at `relatedTask` + */ PARALLEL = 'shared-parallel', } const DAGRE_SHARED_PROPS: dagre.GraphLabel = { nodesep: NODE_SEPARATION_VERTICAL, ranksep: NODE_SEPARATION_HORIZONTAL, - edgesep: 0, + edgesep: 25, ranker: 'longest-path', rankdir: 'LR', align: 'UL', + marginx: 20, + marginy: 20, }; export const DAGRE_VIEWER_PROPS: dagre.GraphLabel = { ...DAGRE_SHARED_PROPS, }; export const DAGRE_BUILDER_PROPS: dagre.GraphLabel = { ...DAGRE_SHARED_PROPS, - ranksep: NODE_SEPARATION_HORIZONTAL + BUILDER_NODE_ADD_RADIUS, - marginx: 30, - marginy: 30, + ranksep: NODE_SEPARATION_HORIZONTAL + BUILDER_NODE_ADD_RADIUS * 2, + nodesep: NODE_SEPARATION_VERTICAL + BUILDER_NODE_ADD_RADIUS, + marginx: DAGRE_SHARED_PROPS.marginx + BUILDER_NODE_ADD_RADIUS * 2, + marginy: DAGRE_SHARED_PROPS.marginy + BUILDER_NODE_ADD_RADIUS * 2, }; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/draw-utils.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/draw-utils.ts index 7d710426ca0..393c704c7a6 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/draw-utils.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/draw-utils.ts @@ -65,7 +65,7 @@ export const integralShapePath: DoubleDraw = (start, finish) => { let secondCurve: string = null; if (start.y !== finish.y) { - const cornerX = start.x + Math.floor(NODE_SEPARATION_HORIZONTAL / 2); + const cornerX = Math.floor(start.x + NODE_SEPARATION_HORIZONTAL / 2); const firstCorner = new Point(cornerX, start.y); const secondCorner = new Point(cornerX, finish.y); diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/factories.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/factories.ts index 18d8f96e8cb..e07b9eeb988 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/factories.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/factories.ts @@ -6,7 +6,6 @@ import { ModelKind, Graph, } from '@console/topology'; -import { LayoutCallback } from '@console/topology/src/layouts/DagreLayout'; import { NodeType, PipelineLayout } from './const'; import SpacerNode from './SpacerNode'; import TaskNode from './TaskNode'; @@ -39,29 +38,23 @@ export const componentFactory: ComponentFactory = (kind: ModelKind, type: string } }; -// TODO: Fix this hack as it's not the best way to get the layout update -type CallbackLayout = (onLayout: LayoutCallback) => LayoutFactory; -export const layoutFactory: CallbackLayout = (onLayout) => (type: string, graph: Graph) => { +export const layoutFactory: LayoutFactory = (type: string, graph: Graph) => { switch (type) { case PipelineLayout.DAGRE_BUILDER: case PipelineLayout.DAGRE_VIEWER: - return new DagreLayout( - graph, - { - // Hack to get around undesirable defaults - // TODO: fix this, it's not ideal but it works for now - linkDistance: 20, - nodeDistance: 20, - groupDistance: 0, - collideDistance: 0, - simulationSpeed: 0, - chargeStrength: 0, - allowDrag: false, - layoutOnDrag: false, - ...getLayoutData(type), - }, - onLayout, - ); + return new DagreLayout(graph, { + // Hack to get around undesirable defaults + // TODO: fix this, it's not ideal but it works for now + linkDistance: 0, + nodeDistance: 0, + groupDistance: 0, + collideDistance: 0, + simulationSpeed: 0, + chargeStrength: 0, + allowDrag: false, + layoutOnDrag: false, + ...getLayoutData(type), + }); default: return undefined; } diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/types.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/types.ts index 2bd8d7c9f10..b35c4487904 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/types.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/types.ts @@ -1,7 +1,6 @@ import { EdgeModel, NodeModel } from '@console/topology'; import { Pipeline, PipelineResourceTask, PipelineRun } from '../../../utils/pipeline-augment'; import { PipelineVisualizationTaskItem } from '../../../utils/pipeline-utils'; -import { TaskErrorMapData } from '../pipeline-builder/types'; import { AddNodeDirection, NodeType } from './const'; // Builder Callbacks @@ -22,7 +21,7 @@ export type TaskListNodeModelData = PipelineRunAfterNodeModelData & { onNewTask: NewTaskNodeCallback; }; export type BuilderNodeModelData = PipelineRunAfterNodeModelData & { - error?: TaskErrorMapData; + error?: string; task: PipelineVisualizationTaskItem; onAddNode: NewTaskListNodeCallback; onNodeSelection: NodeSelectionCallback; @@ -41,7 +40,8 @@ type PipelineNodeModel = NodeModel & { }; export type PipelineMixedNodeModel = PipelineNodeModel; export type PipelineTaskNodeModel = PipelineNodeModel; -export type PipelineTaskListNode = PipelineNodeModel; +export type PipelineBuilderTaskNodeModel = PipelineNodeModel; +export type PipelineTaskListNodeModel = PipelineNodeModel; export type PipelineEdgeModel = EdgeModel; diff --git a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/utils.ts b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/utils.ts index ee16dde4070..69adafe57c1 100644 --- a/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/utils.ts +++ b/frontend/packages/dev-console/src/components/pipelines/pipeline-topology/utils.ts @@ -23,7 +23,6 @@ import { BuilderNodeModelData, PipelineRunAfterNodeModelData, } from './types'; -import { TaskErrorMap } from '../pipeline-builder/types'; const createGenericNode: NodeCreatorSetup = (type, width?) => (name, data) => ({ id: name, @@ -181,13 +180,13 @@ export const tasksToNodes = ( export const tasksToBuilderNodes = ( taskList: PipelineVisualizationTaskItem[], - taskErrorMap: TaskErrorMap, onAddNode: (task: PipelineVisualizationTaskItem, direction: AddNodeDirection) => void, onNodeSelection: (task: PipelineVisualizationTaskItem) => void, + getError: (taskName: string) => string, ): PipelineMixedNodeModel[] => { return taskList.map((task) => { return createBuilderNode(task.name, { - error: taskErrorMap[task.name], + error: getError(task.name), task, onNodeSelection: () => { onNodeSelection(task); diff --git a/frontend/packages/dev-console/src/utils/pipeline-augment.ts b/frontend/packages/dev-console/src/utils/pipeline-augment.ts index e278d90f3ad..86c4d8f20bc 100644 --- a/frontend/packages/dev-console/src/utils/pipeline-augment.ts +++ b/frontend/packages/dev-console/src/utils/pipeline-augment.ts @@ -343,10 +343,13 @@ export const getTaskStatus = (pipelinerun: PipelineRun, pipeline: Pipeline): Tas return taskStatus; }; +export const getResourceModelFromTaskKind = (kind: string): K8sKind => + kind === ClusterTaskModel.kind ? ClusterTaskModel : TaskModel; + export const getResourceModelFromTask = (task: PipelineTask): K8sKind => { const { taskRef: { kind }, } = task; - return kind === ClusterTaskModel.kind ? ClusterTaskModel : TaskModel; + return getResourceModelFromTaskKind(kind); }; diff --git a/frontend/packages/topology/src/layouts/BaseLayout.ts b/frontend/packages/topology/src/layouts/BaseLayout.ts index 8a58e47bfb9..64de104d592 100644 --- a/frontend/packages/topology/src/layouts/BaseLayout.ts +++ b/frontend/packages/topology/src/layouts/BaseLayout.ts @@ -196,6 +196,17 @@ type LayoutOptions = { layoutOnDrag: boolean; }; +const LAYOUT_DEFAULTS: LayoutOptions = { + linkDistance: 60, + nodeDistance: 35, + groupDistance: 35, + collideDistance: 0, + simulationSpeed: 10, + chargeStrength: 0, + allowDrag: true, + layoutOnDrag: true, +}; + class BaseLayout implements Layout { private graph: Graph; @@ -218,16 +229,7 @@ class BaseLayout implements Layout { constructor(graph: Graph, options?: Partial) { this.graph = graph; this.options = { - ...{ - linkDistance: 60, - nodeDistance: 35, - groupDistance: 35, - collideDistance: 0, - simulationSpeed: 10, - chargeStrength: 0, - allowDrag: true, - layoutOnDrag: true, - }, + ...LAYOUT_DEFAULTS, ...options, }; @@ -630,4 +632,4 @@ class BaseLayout implements Layout { } } -export { BaseLayout, LayoutNode, LayoutGroup, LayoutLink, LayoutOptions }; +export { BaseLayout, LayoutNode, LayoutGroup, LayoutLink, LayoutOptions, LAYOUT_DEFAULTS }; diff --git a/frontend/packages/topology/src/layouts/DagreLayout.ts b/frontend/packages/topology/src/layouts/DagreLayout.ts index 101c08ca1ec..4234fd5de53 100644 --- a/frontend/packages/topology/src/layouts/DagreLayout.ts +++ b/frontend/packages/topology/src/layouts/DagreLayout.ts @@ -2,7 +2,14 @@ import * as dagre from 'dagre'; import * as _ from 'lodash'; import { Edge, Graph, Layout, Node } from '../types'; import Point from '../geom/Point'; -import { BaseLayout, LayoutGroup, LayoutLink, LayoutNode, LayoutOptions } from './BaseLayout'; +import { + BaseLayout, + LayoutGroup, + LayoutLink, + LayoutNode, + LayoutOptions, + LAYOUT_DEFAULTS, +} from './BaseLayout'; class DagreNode extends LayoutNode implements dagre.Node { getUpdatableNode(): dagre.Node { @@ -38,36 +45,23 @@ class DagreLink extends LayoutLink { } } -type DagreLayoutOptions = LayoutOptions & { - marginx: number; - marginy: number; - nodesep: number; - edgesep: number; - ranker: string; - rankdir: string; -}; +type DagreLayoutOptions = LayoutOptions & dagre.GraphLabel; -export type LayoutCallback = (nodes: Node[], edges: Edge[]) => void; class DagreLayout extends BaseLayout implements Layout { private dagreOptions: DagreLayoutOptions; - private hackOnRender: LayoutCallback | undefined; - - constructor(graph: Graph, options?: Partial, hackOnRender?: LayoutCallback) { + constructor(graph: Graph, options?: Partial) { super(graph, options); this.dagreOptions = { ...this.options, - ...{ - marginx: 0, - marginy: 0, - nodesep: this.options.nodeDistance, - edgesep: this.options.linkDistance, - ranker: 'tight-tree', - rankdir: 'TB', - }, + marginx: 0, + marginy: 0, + nodesep: this.options.nodeDistance, + edgesep: this.options.linkDistance, + ranker: 'tight-tree', + rankdir: 'TB', ...options, }; - this.hackOnRender = hackOnRender; } protected createLayoutNode(node: Node, nodeDistance: number, index: number) { @@ -97,14 +91,7 @@ class DagreLayout extends BaseLayout implements Layout { protected startLayout(graph: Graph, initialRun: boolean, addingNodes: boolean): void { if (initialRun || addingNodes) { const dagreGraph = new dagre.graphlib.Graph({ compound: true }); - dagreGraph.setGraph({ - marginx: this.dagreOptions.marginx, - marginy: this.dagreOptions.marginy, - nodesep: this.dagreOptions.nodesep, - edgesep: this.dagreOptions.edgesep, - ranker: this.dagreOptions.ranker, - rankdir: this.dagreOptions.rankdir, - }); + dagreGraph.setGraph(_.omit(this.dagreOptions, Object.keys(LAYOUT_DEFAULTS))); _.forEach(this.groups, (group) => { dagreGraph.setNode(group.id, group); @@ -134,8 +121,6 @@ class DagreLayout extends BaseLayout implements Layout { if (this.options.layoutOnDrag) { this.forceSimulation.useForceSimulation(this.nodes, this.edges, this.getFixedNodeDistance); } - - this.hackOnRender && this.hackOnRender(graph.getNodes(), graph.getEdges()); } }