From eda20076c38ef2c9d76cb63a51e407d34f60c190 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Sun, 24 May 2026 03:33:11 -0600 Subject: [PATCH 1/3] feat: add workflow macro UI support --- .../texera/amber/operator/LogicalOp.scala | 3 + .../metadata/OperatorMetadataGenerator.scala | 38 +- .../metadata/PropertyNameConstants.scala | 1 + frontend/src/app/common/type/workflow.ts | 9 +- .../operator-menu/operator-menu.component.ts | 12 + .../macro-property-edit-frame.component.html | 54 ++ .../macro-property-edit-frame.component.scss | 23 + .../macro-property-edit-frame.component.ts | 107 ++++ .../property-editor.component.ts | 13 +- .../context-menu/context-menu.component.html | 36 +- .../context-menu.component.spec.ts | 6 + .../context-menu/context-menu.component.ts | 17 +- .../workflow-editor.component.ts | 88 ++- .../service/drag-drop/drag-drop.service.ts | 10 + .../execute-workflow.service.spec.ts | 10 + .../service/joint-ui/joint-ui.service.ts | 13 +- .../operator-menu.service.spec.ts | 38 ++ .../operator-menu/operator-menu.service.ts | 29 +- .../operator-metadata.service.ts | 35 +- .../model/joint-graph-wrapper.spec.ts | 111 ++++ .../model/joint-graph-wrapper.ts | 566 +++++++++++++++++- .../model/shared-model-change-handler.ts | 42 ++ .../model/workflow-action.service.spec.ts | 99 +++ .../model/workflow-action.service.ts | 415 +++++++++++-- .../types/workflow-common.interface.ts | 11 + .../assets/operator_images/WorkflowMacro.png | Bin 0 -> 8218 bytes 26 files changed, 1685 insertions(+), 101 deletions(-) create mode 100644 frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.html create mode 100644 frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss create mode 100644 frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.ts create mode 100644 frontend/src/assets/operator_images/WorkflowMacro.png diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala index 4e9d6c6e2cd..6bdb066cfaa 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala @@ -439,6 +439,9 @@ abstract class LogicalOp extends PortDescriptor with Serializable { @JsonProperty(PropertyNameConstants.OPERATOR_VERSION) var operatorVersion: String = getOperatorVersion + @JsonProperty(PropertyNameConstants.MACRO_ID_PARENT) + var macroIdParent: String = _ + def operatorIdentifier: OperatorIdentity = OperatorIdentity(operatorId) def getPhysicalOp( diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala index fdfcbcf27dc..2ccfc1a05b6 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/OperatorMetadataGenerator.scala @@ -118,26 +118,24 @@ object OperatorMetadataGenerator { def generateOperatorJsonSchema(opDescClass: Class[_ <: LogicalOp]): JsonNode = { val jsonSchema = jsonSchemaGenerator.generateJsonSchema(opDescClass).asInstanceOf[ObjectNode] - // remove operatorID from json schema - jsonSchema.get("properties").asInstanceOf[ObjectNode].remove("operatorID") - // remove operatorId from json schema - jsonSchema.get("properties").asInstanceOf[ObjectNode].remove("operatorId") - // remove operatorType from json schema - jsonSchema.get("properties").asInstanceOf[ObjectNode].remove("operatorType") - // remove operatorVersion from json schema - jsonSchema.get("properties").asInstanceOf[ObjectNode].remove("operatorVersion") - // remove inputPorts/outputPorts from json schema - jsonSchema.get("properties").asInstanceOf[ObjectNode].remove("inputPorts") - jsonSchema.get("properties").asInstanceOf[ObjectNode].remove("outputPorts") - // remove operatorType from required list - val operatorTypeIndex = - jsonSchema - .get("required") - .asInstanceOf[ArrayNode] - .elements() - .asScala - .indexWhere(p => p.asText().equals("operatorType")) - jsonSchema.get("required").asInstanceOf[ArrayNode].remove(operatorTypeIndex) + val hiddenProperties = Seq( + "operatorID", + "operatorId", + "operatorType", + "operatorVersion", + "macroIdParent", + "inputPorts", + "outputPorts" + ) + val properties = jsonSchema.get("properties").asInstanceOf[ObjectNode] + val required = jsonSchema.get("required").asInstanceOf[ArrayNode] + hiddenProperties.foreach { propertyName => + properties.remove(propertyName) + val requiredIndex = required.elements().asScala.indexWhere(_.asText() == propertyName) + if (requiredIndex >= 0) { + required.remove(requiredIndex) + } + } // remove "title" for the operator - frontend uses userFriendlyName to show operator title jsonSchema.remove("title") jsonSchema diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala index 52bb0414692..20207f654a3 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala @@ -33,6 +33,7 @@ object PropertyNameConstants { // logical plan property names final val OPERATOR_LIST = "operators" final val OPERATOR_LINK_LIST = "links" final val OPERATOR_VERSION = "operatorVersion" + final val MACRO_ID_PARENT = "macroIdParent" // common operator property names final val ATTRIBUTE_NAMES = "attributes" final val ATTRIBUTE_NAME = "attribute" diff --git a/frontend/src/app/common/type/workflow.ts b/frontend/src/app/common/type/workflow.ts index 8e1c1c7e85b..1587d6cd5c2 100644 --- a/frontend/src/app/common/type/workflow.ts +++ b/frontend/src/app/common/type/workflow.ts @@ -18,7 +18,13 @@ */ import { WorkflowMetadata } from "../../dashboard/type/workflow-metadata.interface"; -import { CommentBox, OperatorLink, OperatorPredicate, Point } from "../../workspace/types/workflow-common.interface"; +import { + CommentBox, + OperatorLink, + OperatorPredicate, + Point, + WorkflowMacro, +} from "../../workspace/types/workflow-common.interface"; export enum ExecutionMode { PIPELINED = "PIPELINED", @@ -49,6 +55,7 @@ export interface WorkflowContent links: OperatorLink[]; commentBoxes: CommentBox[]; settings: WorkflowSettings; + macros?: WorkflowMacro[]; }> {} export type Workflow = { content: WorkflowContent } & WorkflowMetadata; diff --git a/frontend/src/app/workspace/component/left-panel/operator-menu/operator-menu.component.ts b/frontend/src/app/workspace/component/left-panel/operator-menu/operator-menu.component.ts index ed172ae5d13..0abc94e093f 100644 --- a/frontend/src/app/workspace/component/left-panel/operator-menu/operator-menu.component.ts +++ b/frontend/src/app/workspace/component/left-panel/operator-menu/operator-menu.component.ts @@ -36,6 +36,8 @@ import { FormsModule } from "@angular/forms"; import { NgFor, NgTemplateOutlet } from "@angular/common"; import { OperatorLabelComponent } from "./operator-label/operator-label.component"; import { NzCollapseComponent, NzCollapsePanelComponent } from "ng-zorro-antd/collapse"; +import { WORKFLOW_MACRO_OPERATOR_TYPE } from "../../../service/operator-metadata/operator-metadata.service"; +import { JointGraphWrapper } from "../../../service/workflow-graph/model/joint-graph-wrapper"; @UntilDestroy() @Component({ @@ -138,6 +140,12 @@ export class OperatorMenuComponent { // add the operator to the graph on select (position relative to the current viewpoint) const origin = this.workflowActionService.getJointGraphWrapper().getMainJointPaper()?.translate(); const point = { x: 400 - (origin?.tx ?? 0), y: 200 - (origin?.ty ?? 0) }; + if (selectSchema.operatorType === WORKFLOW_MACRO_OPERATOR_TYPE) { + const macroID = this.workflowActionService.createMacroAt(point); + this.workflowActionService.getJointGraphWrapper().highlightOperators(JointGraphWrapper.getMacroNodeID(macroID)); + this.clearSearch(); + return; + } this.workflowActionService.addOperator( this.workflowUtilService.getNewOperatorPredicate(selectSchema.operatorType), point @@ -145,6 +153,10 @@ export class OperatorMenuComponent { // asynchronously immediately clear the search input and suggestions // because ng-zorro shows the selected value if it's synchronously + this.clearSearch(); + } + + private clearSearch(): void { setTimeout(() => { this.searchInputValue = ""; this.autocompleteOptions = []; diff --git a/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.html b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.html new file mode 100644 index 00000000000..68b2a3908bb --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.html @@ -0,0 +1,54 @@ + + +

Workflow Macro

+ + + + + + + +
+ #{{ macro?.workflowId }} {{ macro?.workflowName }} +
+ +
+ +
diff --git a/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss new file mode 100644 index 00000000000..686c6353f1c --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss @@ -0,0 +1,23 @@ +.macro-field-label { + display: block; + margin-bottom: 6px; + font-weight: 600; +} + +nz-select { + width: 100%; +} + +.macro-selected-workflow { + margin-top: 10px; + color: #666; + font-size: 12px; +} + +.macro-actions { + margin-top: 14px; +} + +.macro-actions button { + width: 100%; +} diff --git a/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.ts new file mode 100644 index 00000000000..35684a530b0 --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.ts @@ -0,0 +1,107 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component, Input, OnChanges, OnDestroy, OnInit } from "@angular/core"; +import { FormsModule } from "@angular/forms"; +import { NgFor, NgIf } from "@angular/common"; +import { NzOptionComponent, NzSelectComponent } from "ng-zorro-antd/select"; +import { NzButtonComponent } from "ng-zorro-antd/button"; +import { NzIconDirective } from "ng-zorro-antd/icon"; +import { Subject } from "rxjs"; +import { finalize, take, takeUntil } from "rxjs/operators"; +import { WorkflowPersistService } from "../../../../common/service/workflow-persist/workflow-persist.service"; +import { DashboardWorkflow } from "../../../../dashboard/type/dashboard-workflow.interface"; +import { NotificationService } from "../../../../common/service/notification/notification.service"; +import { WorkflowActionService } from "../../../service/workflow-graph/model/workflow-action.service"; +import { JointGraphWrapper } from "../../../service/workflow-graph/model/joint-graph-wrapper"; +import { WorkflowMacro } from "../../../types/workflow-common.interface"; + +@Component({ + selector: "texera-macro-property-edit-frame", + templateUrl: "./macro-property-edit-frame.component.html", + styleUrls: ["./macro-property-edit-frame.component.scss"], + imports: [FormsModule, NgFor, NgIf, NzSelectComponent, NzOptionComponent, NzButtonComponent, NzIconDirective], +}) +export class MacroPropertyEditFrameComponent implements OnInit, OnChanges, OnDestroy { + @Input() macroNodeId?: string; + + public workflows: DashboardWorkflow[] = []; + public selectedWorkflowId?: number; + public loading = false; + public canModify = true; + private macroID?: string; + private destroy$ = new Subject(); + + constructor( + private workflowPersistService: WorkflowPersistService, + private workflowActionService: WorkflowActionService, + private notificationService: NotificationService + ) {} + + ngOnInit(): void { + this.canModify = this.workflowActionService.checkWorkflowModificationEnabled(); + this.workflowActionService + .getWorkflowModificationEnabledStream() + .pipe(takeUntil(this.destroy$)) + .subscribe(canModify => (this.canModify = canModify)); + + this.workflowPersistService + .retrieveWorkflowsBySessionUser() + .pipe(take(1)) + .subscribe(workflows => (this.workflows = workflows)); + } + + ngOnChanges(): void { + if (!this.macroNodeId) return; + this.macroID = JointGraphWrapper.getMacroIDFromNodeID(this.macroNodeId); + this.selectedWorkflowId = this.macro?.workflowId; + } + + public get macro(): WorkflowMacro | undefined { + return this.macroID ? this.workflowActionService.getWorkflowMacro(this.macroID) : undefined; + } + + public onWorkflowChange(workflowId: number | undefined): void { + if (!this.canModify || !this.macroID || workflowId === undefined) return; + this.loading = true; + this.workflowPersistService + .retrieveWorkflow(workflowId) + .pipe( + take(1), + finalize(() => (this.loading = false)) + ) + .subscribe({ + next: workflow => { + this.workflowActionService.replaceMacroWorkflow(this.macroID!, workflow.content, workflow.wid, workflow.name); + this.notificationService.info(`Imported workflow "${workflow.name}" into macro.`); + }, + error: () => this.notificationService.error("Failed to import workflow into macro."), + }); + } + + public toggleCollapsed(): void { + if (!this.canModify || !this.macroID || !this.macro) return; + this.workflowActionService.setMacroCollapsed(this.macroID, !(this.macro.collapsed ?? false)); + } + + ngOnDestroy(): void { + this.destroy$.next(); + this.destroy$.complete(); + } +} diff --git a/frontend/src/app/workspace/component/property-editor/property-editor.component.ts b/frontend/src/app/workspace/component/property-editor/property-editor.component.ts index c868151f270..1335390d6e2 100644 --- a/frontend/src/app/workspace/component/property-editor/property-editor.component.ts +++ b/frontend/src/app/workspace/component/property-editor/property-editor.component.ts @@ -45,6 +45,8 @@ import { CdkDrag, CdkDragHandle } from "@angular/cdk/drag-drop"; import { NzSpaceCompactItemDirective } from "ng-zorro-antd/space"; import { NzButtonComponent } from "ng-zorro-antd/button"; import { FormlyRepeatDndComponent } from "../../../common/formly/repeat-dnd/repeat-dnd.component"; +import { JointGraphWrapper } from "../../service/workflow-graph/model/joint-graph-wrapper"; +import { MacroPropertyEditFrameComponent } from "./macro-property-edit-frame/macro-property-edit-frame.component"; /** * PropertyEditorComponent is the panel that allows user to edit operator properties. @@ -74,6 +76,7 @@ import { FormlyRepeatDndComponent } from "../../../common/formly/repeat-dnd/repe NgComponentOutlet, NzResizeHandlesComponent, FormlyRepeatDndComponent, + MacroPropertyEditFrameComponent, ], }) export class PropertyEditorComponent implements OnInit, OnDestroy { @@ -165,7 +168,15 @@ export class PropertyEditorComponent implements OnInit, OnDestroy { this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedCommentBoxIDs(); const highlightedPorts = this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedPortIDs(); - if (highlightedOperators.length === 1 && highlightLinks.length === 0 && highlightedPorts.length === 0) { + if ( + highlightedOperators.length === 1 && + highlightLinks.length === 0 && + highlightedPorts.length === 0 && + JointGraphWrapper.isMacroNodeID(highlightedOperators[0]) + ) { + this.currentComponent = MacroPropertyEditFrameComponent; + this.componentInputs = { macroNodeId: highlightedOperators[0] }; + } else if (highlightedOperators.length === 1 && highlightLinks.length === 0 && highlightedPorts.length === 0) { this.currentComponent = OperatorPropertyEditFrameComponent; this.componentInputs = { currentOperatorId: highlightedOperators[0] }; } else if (highlightedPorts.length === 1 && highlightLinks.length === 0) { diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html index 4465d65cb27..ce9632d643c 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html @@ -30,6 +30,34 @@ nzTheme="outline">copy +
  • + + create macro +
    • +
  • + + remove from macro +
  • { getJointGraphWrapper: vi.fn(), getWorkflowModificationEnabledStream: vi.fn(), deleteOperatorsAndLinks: vi.fn(), + deleteMacros: vi.fn(), deleteCommentBox: vi.fn(), getWorkflowMetadata: vi.fn(), getTexeraGraph: vi.fn(), @@ -74,6 +75,7 @@ describe("ContextMenuComponent", () => { workflowActionServiceSpy.getWorkflowModificationEnabledStream.mockReturnValue(of(true)); workflowActionServiceSpy.getTexeraGraph.mockReturnValue(texeraGraphSpy); workflowActionServiceSpy.deleteOperatorsAndLinks.mockReturnValue(undefined); + workflowActionServiceSpy.deleteMacros.mockReturnValue(undefined); workflowActionServiceSpy.deleteCommentBox.mockReturnValue(undefined); workflowActionServiceSpy.deleteLinkWithID.mockReturnValue(undefined); workflowActionServiceSpy.getWorkflowMetadata.mockReturnValue({ name: "Test Workflow" }); // Mock return value @@ -101,6 +103,10 @@ describe("ContextMenuComponent", () => { viewResultHighlightedOperators: vi.fn(), reuseResultHighlightedOperator: vi.fn(), executeUpToOperator: vi.fn(), + canCreateMacroFromHighlightedOperators: vi.fn().mockReturnValue(false), + canRemoveHighlightedOperatorsFromMacro: vi.fn().mockReturnValue(false), + createMacroFromHighlightedOperators: vi.fn(), + removeHighlightedOperatorsFromMacro: vi.fn(), } as unknown as Mocked; const validationWorkflowServiceSpy = { validateOperator: vi.fn() }; diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts index 019f77f7afa..5839fe76e59 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts @@ -31,6 +31,7 @@ import { NzMenuDirective, NzMenuItemComponent } from "ng-zorro-antd/menu"; import { NgIf } from "@angular/common"; import { ɵNzTransitionPatchDirective } from "ng-zorro-antd/core/transition-patch"; import { NzIconDirective } from "ng-zorro-antd/icon"; +import { JointGraphWrapper } from "src/app/workspace/service/workflow-graph/model/joint-graph-wrapper"; @UntilDestroy() @Component({ @@ -90,6 +91,13 @@ export class ContextMenuComponent { return this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedLinkIDs().length > 0; } + public hasHighlightedMacroNode(): boolean { + return this.workflowActionService + .getJointGraphWrapper() + .getCurrentHighlightedOperatorIDs() + .some(operatorID => JointGraphWrapper.isMacroNodeID(operatorID)); + } + public onCopy(): void { this.operatorMenuService.saveHighlightedElements(); } @@ -107,7 +115,12 @@ export class ContextMenuComponent { // Capture all highlighted IDs before starting deletion to avoid modification during iteration const highlightedOperatorIDs = Array.from( this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs() - ); + ).filter(operatorID => !JointGraphWrapper.isMacroNodeID(operatorID)); + const highlightedMacroIDs = Array.from( + this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs() + ) + .filter(operatorID => JointGraphWrapper.isMacroNodeID(operatorID)) + .map(operatorID => JointGraphWrapper.getMacroIDFromNodeID(operatorID)); const highlightedCommentBoxIDs = Array.from( this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedCommentBoxIDs() ); @@ -117,6 +130,8 @@ export class ContextMenuComponent { // Bundle all deletions together for proper undo/redo support this.workflowActionService.getTexeraGraph().bundleActions(() => { + this.workflowActionService.deleteMacros(highlightedMacroIDs); + // Delete operators and their connected links this.workflowActionService.deleteOperatorsAndLinks(highlightedOperatorIDs); diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts index 979f131ad3c..a9ea00d84ab 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts @@ -27,8 +27,9 @@ import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-w import { fromJointPaperEvent, JointUIService, linkPathStrokeColor } from "../../service/joint-ui/joint-ui.service"; import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; +import { JointGraphWrapper } from "../../service/workflow-graph/model/joint-graph-wrapper"; import { WorkflowStatusService } from "../../service/workflow-status/workflow-status.service"; -import { ExecutionState, OperatorState } from "../../types/execute-workflow.interface"; +import { ExecutionState, OperatorState, OperatorStatistics } from "../../types/execute-workflow.interface"; import { LogicalPort, OperatorLink, OperatorPredicate } from "../../types/workflow-common.interface"; import { auditTime, filter, map, takeUntil, withLatestFrom } from "rxjs/operators"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; @@ -333,6 +334,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy this.isSink(op.operatorID) ); }); + this.updateCollapsedMacroStates(status); }); this.executeWorkflowService @@ -355,11 +357,48 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy .getAllOperators() .forEach(op => { this.jointUIService.changeOperatorState(this.paper, op.operatorID, operatorState); - }); + }); + } + }); + + this.workflowActionService + .getMacroVisualRefreshStream() + .pipe(untilDestroyed(this)) + .subscribe(() => this.updateCollapsedMacroStates(this.workflowStatusService.getCurrentStatus())); + } + + private updateCollapsedMacroStates(status: Record): void { + if (!this.paper) return; + const operators = this.workflowActionService.getTexeraGraph().getAllOperators(); + this.workflowActionService + .getWorkflowMacros() + .filter(macro => macro.collapsed) + .forEach(macro => { + const internalStates = operators + .filter(operator => operator.macroIdParent === macro.macroID) + .map(operator => status[operator.operatorID]?.operatorState); + const macroNodeID = JointGraphWrapper.getMacroNodeID(macro.macroID); + if (this.paper.getModelById(macroNodeID)) { + this.jointUIService.changeOperatorState(this.paper, macroNodeID, this.getMacroAggregateState(internalStates)); } }); } + private getMacroAggregateState(states: readonly (OperatorState | undefined)[]): OperatorState { + const knownStates = states.filter((state): state is OperatorState => state !== undefined); + if (!knownStates.length) return OperatorState.Uninitialized; + if (knownStates.includes(OperatorState.Running)) return OperatorState.Running; + if (knownStates.includes(OperatorState.Paused)) return OperatorState.Paused; + if (knownStates.includes(OperatorState.Pausing)) return OperatorState.Pausing; + if (knownStates.includes(OperatorState.Recovering)) return OperatorState.Recovering; + if (knownStates.includes(OperatorState.Resuming)) return OperatorState.Resuming; + if (knownStates.includes(OperatorState.Initializing)) return OperatorState.Initializing; + if (knownStates.includes(OperatorState.Ready)) return OperatorState.Ready; + return knownStates.every(state => state === OperatorState.Completed) + ? OperatorState.Completed + : OperatorState.Uninitialized; + } + private handleRegionEvents(): void { this.editor.classList.add("hide-region"); const Region = joint.dia.Element.define( @@ -592,7 +631,8 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy filter( event => this.workflowActionService.getTexeraGraph().hasOperator(event[0].model.id.toString()) || - this.workflowActionService.getTexeraGraph().hasCommentBox(event[0].model.id.toString()) + this.workflowActionService.getTexeraGraph().hasCommentBox(event[0].model.id.toString()) || + JointGraphWrapper.isMacroNodeID(event[0].model.id.toString()) ) ) .pipe(untilDestroyed(this)) @@ -642,6 +682,8 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy // else only highlight a single operator or group if (this.workflowActionService.getTexeraGraph().hasOperator(elementID)) { this.workflowActionService.highlightOperators(event[1].shiftKey, elementID); + } else if (JointGraphWrapper.isMacroNodeID(elementID)) { + this.wrapper.highlightOperators(elementID); } else if (this.workflowActionService.getTexeraGraph().hasCommentBox(elementID)) { this.wrapper.highlightCommentBoxes(elementID); } @@ -678,7 +720,12 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy .pipe(untilDestroyed(this)) .subscribe(elementIDs => elementIDs.forEach(elementID => { - this.paper.findViewByModel(elementID).highlight("rect.body", { highlighter: highlightOptions }); + const elementView = this.paper.findViewByModel(elementID); + if (JointGraphWrapper.isMacroNodeID(elementID)) { + elementView?.$el.children(".joint-highlight-stroke").remove(); + } else { + elementView.highlight("rect.body", { highlighter: highlightOptions }); + } }) ); @@ -911,7 +958,10 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy // Handle right-click on links fromJointPaperEvent(this.paper, "link:contextmenu") - .pipe(untilDestroyed(this)) + .pipe( + filter(event => !JointGraphWrapper.isMacroProxyLinkID(event[0].model.id.toString())), + untilDestroyed(this) + ) .subscribe(event => { const linkID = event[0].model.id.toString(); // Highlight the link when right-clicked @@ -940,7 +990,8 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy fromJointPaperEvent(this.paper, "tool:remove") .pipe( filter(() => this.interactive), - map(value => value[0]) + map(value => value[0]), + filter(elementView => !JointGraphWrapper.isMacroProxyLinkID(elementView.model.id.toString())) ) .pipe(untilDestroyed(this)) .subscribe(elementView => { @@ -1063,12 +1114,19 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy private deleteElements(): void { // Capture all highlighted IDs before starting deletion to avoid modification during iteration - const highlightedOperatorIDs = Array.from(this.wrapper.getCurrentHighlightedOperatorIDs()); + const highlightedOperatorIDs = Array.from(this.wrapper.getCurrentHighlightedOperatorIDs()).filter( + operatorID => !JointGraphWrapper.isMacroNodeID(operatorID) + ); + const highlightedMacroIDs = Array.from(this.wrapper.getCurrentHighlightedOperatorIDs()) + .filter(operatorID => JointGraphWrapper.isMacroNodeID(operatorID)) + .map(operatorID => JointGraphWrapper.getMacroIDFromNodeID(operatorID)); const highlightedCommentBoxIDs = Array.from(this.wrapper.getCurrentHighlightedCommentBoxIDs()); const highlightedLinkIDs = Array.from(this.wrapper.getCurrentHighlightedLinkIDs()); // Bundle all deletions together for proper undo/redo support this.workflowActionService.getTexeraGraph().bundleActions(() => { + this.workflowActionService.deleteMacros(highlightedMacroIDs); + // Delete operators and their connected links this.workflowActionService.deleteOperatorsAndLinks(highlightedOperatorIDs); @@ -1187,7 +1245,10 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy private handleLinkCursorHover(): void { // When the cursor hovers over a link, the delete button and the breakpoint button appear fromJointPaperEvent(this.paper, "link:mouseenter") - .pipe(map(value => value[0])) + .pipe( + map(value => value[0]), + filter(linkView => !JointGraphWrapper.isMacroProxyLinkID(linkView.model.id.toString())) + ) .pipe(untilDestroyed(this)) .subscribe(linkView => { // Create an array to hold the tools @@ -1208,7 +1269,10 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy * otherwise, the breakpoint button is not changed. */ fromJointPaperEvent(this.paper, "link:mouseleave") - .pipe(map(value => value[0])) + .pipe( + map(value => value[0]), + filter(elementView => !JointGraphWrapper.isMacroProxyLinkID(elementView.model.id.toString())) + ) .pipe(untilDestroyed(this)) .subscribe(elementView => { // ensure that the link element exists @@ -1238,7 +1302,11 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy private handleLinkBreakpointToolAttachment(): void { this.wrapper .getJointLinkCellAddStream() - .pipe(this.wrapper.jointGraphContext.bufferWhileAsync, untilDestroyed(this)) + .pipe( + filter(link => !JointGraphWrapper.isMacroProxyLinkID(link.id.toString())), + this.wrapper.jointGraphContext.bufferWhileAsync, + untilDestroyed(this) + ) .subscribe(link => { const linkView = link.findView(this.paper); const breakpointButtonTool = this.breakpointButton; diff --git a/frontend/src/app/workspace/service/drag-drop/drag-drop.service.ts b/frontend/src/app/workspace/service/drag-drop/drag-drop.service.ts index e7665582ba1..cfa7c66186b 100644 --- a/frontend/src/app/workspace/service/drag-drop/drag-drop.service.ts +++ b/frontend/src/app/workspace/service/drag-drop/drag-drop.service.ts @@ -26,6 +26,8 @@ import { Injectable } from "@angular/core"; import { filter, first, map } from "rxjs/operators"; import TinyQueue from "tinyqueue"; import * as joint from "jointjs"; +import { WORKFLOW_MACRO_OPERATOR_TYPE } from "../operator-metadata/operator-metadata.service"; +import { JointGraphWrapper } from "../workflow-graph/model/joint-graph-wrapper"; @Injectable({ providedIn: "root", @@ -63,6 +65,14 @@ export class DragDropService { .getMainJointPaper() ?.pageToLocalPoint(dropPoint.x, dropPoint.y)!; + if (this.op.operatorType === WORKFLOW_MACRO_OPERATOR_TYPE) { + const macroID = this.workflowActionService.createMacroAt(coordinates); + this.workflowActionService.getJointGraphWrapper().highlightOperators(JointGraphWrapper.getMacroNodeID(macroID)); + this.resetSuggestions(); + this.operatorDroppedSubject.next(); + return; + } + // Check if the operator is dropped on top of an existing edge const intersectedLink = this.findIntersectedLink(coordinates); diff --git a/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts b/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts index f38890eb2bf..b4237aa5d36 100644 --- a/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts +++ b/frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.spec.ts @@ -45,6 +45,8 @@ import { UserService } from "src/app/common/service/user/user.service"; import { StubUserService } from "src/app/common/service/user/stub-user.service"; import { MockComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/mock-computing-unit-status.service"; import { commonTestProviders } from "../../../common/testing/test-utils"; +import { WorkflowGraph } from "../workflow-graph/model/workflow-graph"; +import { mockScanPredicate } from "../workflow-graph/model/mock-workflow-data"; class StubHttpClient { public post(): Observable { @@ -97,6 +99,14 @@ describe("ExecuteWorkflowService", () => { expect(newLogicalPlan).toEqual(mockLogicalPlan_scan_result); }); + it("should keep macro parent metadata out of the logical execution plan", () => { + const graph = new WorkflowGraph([{ ...mockScanPredicate, macroIdParent: "macro-1" }], []); + + const plan = ExecuteWorkflowService.getLogicalPlanRequest(graph); + + expect((plan.operators[0] as any).macroIdParent).toBeUndefined(); + }); + it("should msg backend when executing workflow", fakeAsync(() => { const logicalPlan: LogicalPlan = ExecuteWorkflowService.getLogicalPlanRequest(mockWorkflowPlan_scan_result); const wsSendSpy = vi.spyOn((service as any).workflowWebsocketService, "send"); diff --git a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts index d5d8f78c584..63cc0438278 100644 --- a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts +++ b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts @@ -269,6 +269,10 @@ export class JointUIService { this.operatorMetadataService.getOperatorMetadata().subscribe(value => (this.operatorSchemas = value.operators)); } + public static getOperatorElementMarkup(dynamicInputPorts = false, dynamicOutputPorts = false): string { + return TexeraCustomJointElement.getMarkup(dynamicInputPorts, dynamicOutputPorts); + } + /** * Gets the JointJS UI Element object based on the operator predicate. * A JointJS Element could be added to the JointJS graph to let JointJS display the operator accordingly. @@ -465,6 +469,10 @@ export class JointUIService { } public changeOperatorState(jointPaper: joint.dia.Paper, operatorID: string, operatorState: OperatorState): void { + const element = jointPaper.getModelById(operatorID) as joint.shapes.devs.Model | undefined; + if (!element) { + return; + } let fillColor: string; switch (operatorState) { case OperatorState.Ready: @@ -484,13 +492,12 @@ export class JointUIService { fillColor = "gray"; break; } - jointPaper.getModelById(operatorID).attr({ + element.attr({ [`.${operatorStateClass}`]: { text: operatorState.toString(), fill: fillColor }, "rect.body": { stroke: fillColor }, [`.${operatorPortMetricsClass}`]: { fill: fillColor }, [`.${operatorWorkerCountClass}`]: { fill: fillColor }, }); - const element = jointPaper.getModelById(operatorID) as joint.shapes.devs.Model; const allPorts = element.getPorts(); const inPorts = allPorts.filter(p => p.group === "in"); inPorts.forEach(p => { @@ -944,7 +951,7 @@ export class JointUIService { visibility: "hidden", }, ".texera-operator-icon": { - "xlink:href": "assets/operator_images/" + operatorType + ".png", + "xlink:href": `assets/operator_images/${operatorType}.png`, width: 35, height: 35, "ref-x": 0.5, diff --git a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts index 88e4358028f..da4b80fe856 100644 --- a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts +++ b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts @@ -184,4 +184,42 @@ describe("OperatorMenuService", () => { expect(service.isToViewResult).toBe(false); }); }); + + it("groups highlighted operators under a macro parent and can remove them", () => { + workflowActionService.addOperatorsAndLinks( + [ + { op: mockScanPredicate, pos: mockPoint }, + { op: mockSentimentPredicate, pos: mockPoint }, + ], + [] + ); + const wrapper = workflowActionService.getJointGraphWrapper(); + wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs()); + wrapper.highlightOperators(mockScanPredicate.operatorID, mockSentimentPredicate.operatorID); + + service.createMacroFromHighlightedOperators(); + + const scanMacroId = workflowActionService.getTexeraGraph().getOperator(mockScanPredicate.operatorID).macroIdParent; + const sentimentMacroId = workflowActionService + .getTexeraGraph() + .getOperator(mockSentimentPredicate.operatorID).macroIdParent; + expect(scanMacroId).toBeTruthy(); + expect(sentimentMacroId).toBe(scanMacroId); + expect(workflowActionService.getWorkflowContent().macros).toEqual([ + expect.objectContaining({ macroID: scanMacroId, name: "Macro" }), + ]); + expect(service.canRemoveHighlightedOperatorsFromMacro()).toBe(true); + + service.removeHighlightedOperatorsFromMacro(); + + expect( + workflowActionService.getTexeraGraph().getOperator(mockScanPredicate.operatorID).macroIdParent + ).toBeUndefined(); + expect( + workflowActionService.getTexeraGraph().getOperator(mockSentimentPredicate.operatorID).macroIdParent + ).toBeUndefined(); + expect(workflowActionService.getWorkflowContent().macros).toEqual([ + expect.objectContaining({ macroID: scanMacroId, name: "Macro" }), + ]); + }); }); diff --git a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts index 1b800e9f7b7..1877b6ae171 100644 --- a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts +++ b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts @@ -92,7 +92,9 @@ export class OperatorMenuService { ) .pipe(untilDestroyed(this)) .subscribe(() => { - this._highlightedOperators$.next(jointGraphWrapper.getCurrentHighlightedOperatorIDs()); + this._highlightedOperators$.next( + jointGraphWrapper.getCurrentHighlightedOperatorIDs().filter(operatorID => texeraGraph.hasOperator(operatorID)) + ); this.recomputeMenuState(); }); @@ -264,6 +266,31 @@ export class OperatorMenuService { this.executeWorkflowService.executeWorkflow("", targetOperatorId); } + public canCreateMacroFromHighlightedOperators(): boolean { + return this._highlightedOperators$.value.length > 0; + } + + public canRemoveHighlightedOperatorsFromMacro(): boolean { + const texeraGraph = this.workflowActionService.getTexeraGraph(); + return this._highlightedOperators$.value.some(operatorID => + Boolean(texeraGraph.getOperator(operatorID).macroIdParent) + ); + } + + public createMacroFromHighlightedOperators(): void { + const operatorIDs = this._highlightedOperators$.value; + if (!operatorIDs.length) return; + this.workflowActionService.createMacroForOperators(operatorIDs); + this.notificationService.info("Created macro for selected operators."); + } + + public removeHighlightedOperatorsFromMacro(): void { + const operatorIDs = this._highlightedOperators$.value; + if (!operatorIDs.length) return; + this.workflowActionService.setOperatorsMacroParent(operatorIDs, undefined); + this.notificationService.info("Removed selected operators from macro."); + } + public performPasteOperation() { // by reading from the clipboard, permission needs to be granted // a permission prompt automatically shows up by calling readText() diff --git a/frontend/src/app/workspace/service/operator-metadata/operator-metadata.service.ts b/frontend/src/app/workspace/service/operator-metadata/operator-metadata.service.ts index 484a3c09818..9126f19d9d9 100644 --- a/frontend/src/app/workspace/service/operator-metadata/operator-metadata.service.ts +++ b/frontend/src/app/workspace/service/operator-metadata/operator-metadata.service.ts @@ -22,9 +22,23 @@ import { Injectable } from "@angular/core"; import { Observable } from "rxjs"; import { AppSettings } from "../../../common/app-setting"; import { OperatorMetadata, OperatorSchema } from "../../types/operator-schema.interface"; -import { shareReplay } from "rxjs/operators"; +import { map, shareReplay } from "rxjs/operators"; export const OPERATOR_METADATA_ENDPOINT = "resources/operator-metadata"; +export const WORKFLOW_MACRO_OPERATOR_TYPE = "WorkflowMacro"; + +export const WORKFLOW_MACRO_OPERATOR_SCHEMA: OperatorSchema = { + operatorType: WORKFLOW_MACRO_OPERATOR_TYPE, + operatorVersion: "frontend-macro", + jsonSchema: { type: "object", properties: {} }, + additionalMetadata: { + userFriendlyName: "Workflow Macro", + operatorGroupName: "Workflow", + operatorDescription: "Import an existing workflow as a reusable macro", + inputPorts: [], + outputPorts: [], + }, +}; const addDictionaryAPIAddress = "/api/resources/dictionary/"; const getDictionaryAPIAddress = "/api/upload/dictionary/"; @@ -54,7 +68,10 @@ export class OperatorMetadataService { private operatorMetadataObservable = this.httpClient .get(`${AppSettings.getApiEndpoint()}/${OPERATOR_METADATA_ENDPOINT}`) - .pipe(shareReplay(1)); + .pipe( + map(metadata => this.withWorkflowMacroOperator(metadata)), + shareReplay(1) + ); constructor(private httpClient: HttpClient) { this.getOperatorMetadata().subscribe(data => { @@ -119,4 +136,18 @@ export class OperatorMetadataService { } return true; } + + private withWorkflowMacroOperator(metadata: OperatorMetadata): OperatorMetadata { + if (metadata.operators.some(operator => operator.operatorType === WORKFLOW_MACRO_OPERATOR_TYPE)) { + return metadata; + } + return { + operators: [...metadata.operators, WORKFLOW_MACRO_OPERATOR_SCHEMA], + groups: metadata.groups.some( + group => group.groupName === WORKFLOW_MACRO_OPERATOR_SCHEMA.additionalMetadata.operatorGroupName + ) + ? metadata.groups + : [...metadata.groups, { groupName: WORKFLOW_MACRO_OPERATOR_SCHEMA.additionalMetadata.operatorGroupName }], + }; + } } diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts index 495208f5dce..2804f166f5e 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts @@ -40,6 +40,7 @@ import { map, share, tap } from "rxjs/operators"; import { commonTestProviders } from "../../../../common/testing/test-utils"; import { GuiConfigService } from "../../../../common/service/gui-config.service"; import { MockGuiConfigService } from "../../../../common/service/gui-config.service.mock"; +import { OperatorState } from "../../../types/execute-workflow.interface"; describe("JointGraphWrapperService", () => { let jointGraph: joint.dia.Graph; @@ -568,6 +569,116 @@ describe("JointGraphWrapperService", () => { expect(localJointGraphWrapper.getElementPosition(mockResultPredicate.operatorID)).toEqual(expectedPosition); }); + it("should anchor expanded macro tab and keep collapsed boundary ports", () => { + const macroID = "macro-1"; + const externalSource = { ...mockScanPredicate }; + const internalOperator = { ...mockSentimentPredicate, macroIdParent: macroID }; + const secondInternalOperator = { ...mockResultPredicate, operatorID: "internal-output", macroIdParent: macroID }; + const externalSink = { ...mockResultPredicate }; + const operators = [externalSource, internalOperator, secondInternalOperator, externalSink]; + const links = [mockScanSentimentLink, mockSentimentResultLink]; + + jointGraph.addCell(jointUIService.getJointOperatorElement(externalSource, { x: 100, y: 180 })); + jointGraph.addCell(jointUIService.getJointOperatorElement(internalOperator, { x: 300, y: 180 })); + jointGraph.addCell(jointUIService.getJointOperatorElement(secondInternalOperator, { x: 450, y: 180 })); + jointGraph.addCell(jointUIService.getJointOperatorElement(externalSink, { x: 500, y: 180 })); + links.forEach(link => jointGraph.addCell(JointUIService.getJointLinkCell(link))); + + jointGraphWrapper.refreshMacroFrames(operators, [{ macroID, name: "Macro 1", position: { x: 0, y: 0 } }], links); + + const frame = jointGraph.getCell(JointGraphWrapper.getMacroFrameID(macroID)) as joint.dia.Element; + const node = jointGraph.getCell(JointGraphWrapper.getMacroNodeID(macroID)) as joint.dia.Element; + expect(node.position()).toEqual(frame.position()); + expect( + (jointGraph.getCell(internalOperator.operatorID) as joint.dia.Element).position().y - frame.position().y + ).toBe(85); + expect(node.getPorts()).toEqual([]); + + const originalFramePosition = frame.position(); + const originalFrameSize = frame.size(); + (jointGraph.getCell(internalOperator.operatorID) as joint.dia.Element).position(220, 180); + jointGraphWrapper.refreshMacroFrames(operators, [{ macroID, name: "Macro 1", position: node.position() }], links); + expect(frame.position().x).toBeLessThan(originalFramePosition.x); + expect(frame.size().width).toBeGreaterThan(originalFrameSize.width); + expect(node.position()).toEqual(frame.position()); + expect( + (jointGraph.getCell(internalOperator.operatorID) as joint.dia.Element).position().y - frame.position().y + ).toBe(85); + + jointGraphWrapper.refreshMacroFrames( + operators, + [{ macroID, name: "Macro 1", position: node.position(), collapsed: true }], + links + ); + + const collapsedNode = jointGraph.getCell(JointGraphWrapper.getMacroNodeID(macroID)) as joint.dia.Element; + expect(jointGraph.getCell(JointGraphWrapper.getMacroFrameID(macroID))).toBeUndefined(); + expect(collapsedNode.size()).toEqual({ + width: JointUIService.DEFAULT_OPERATOR_WIDTH, + height: JointUIService.DEFAULT_OPERATOR_HEIGHT, + }); + expect(collapsedNode.attr(".texera-operator-friendly-name/text")).toBe("Workflow Macro"); + expect(collapsedNode.attr("rect.body/stroke")).toBe("#CFCFCF"); + expect(collapsedNode.getPorts().map(port => port.group)).toEqual(["in", "out"]); + jointUIService.changeOperatorState( + jointGraphWrapper.getMainJointPaper() ?? ({ getModelById: (id: string) => jointGraph.getCell(id) } as any), + collapsedNode.id.toString(), + OperatorState.Running + ); + expect(collapsedNode.attr("rect.body/stroke")).toBe("orange"); + expect(jointGraph.getLinks().filter(link => JointGraphWrapper.isMacroProxyLinkID(link.id.toString())).length).toBe( + 2 + ); + expect(jointGraph.getCell(internalOperator.operatorID).attr("root/display")).toBe("none"); + }); + + it("should route collapsed macro boundary links only through visible endpoints", () => { + const sourceMacroID = "macro-source"; + const targetMacroID = "macro-target"; + const sourceOperator = { ...mockSentimentPredicate, macroIdParent: sourceMacroID }; + const targetOperator = { ...mockResultPredicate, macroIdParent: targetMacroID }; + const crossMacroLink = { + linkID: "macro-cross-link", + source: { + operatorID: sourceOperator.operatorID, + portID: sourceOperator.outputPorts[0].portID, + }, + target: { + operatorID: targetOperator.operatorID, + portID: targetOperator.inputPorts[0].portID, + }, + }; + + jointGraph.addCell(jointUIService.getJointOperatorElement(sourceOperator, { x: 100, y: 100 })); + jointGraph.addCell(jointUIService.getJointOperatorElement(targetOperator, { x: 300, y: 100 })); + jointGraph.addCell(JointUIService.getJointLinkCell(crossMacroLink)); + + jointGraphWrapper.refreshMacroFrames( + [sourceOperator, targetOperator], + [ + { macroID: sourceMacroID, name: "Source Macro", position: { x: 20, y: 20 }, collapsed: true }, + { macroID: targetMacroID, name: "Target Macro", position: { x: 250, y: 20 }, collapsed: true }, + ], + [crossMacroLink] + ); + + const proxyLinks = jointGraph.getLinks().filter(link => JointGraphWrapper.isMacroProxyLinkID(link.id.toString())); + expect(proxyLinks.length).toBe(1); + expect(proxyLinks[0].source()).toEqual({ + id: JointGraphWrapper.getMacroNodeID(sourceMacroID), + port: "macro-out-macro-cross-link", + }); + expect(proxyLinks[0].target()).toEqual({ + id: JointGraphWrapper.getMacroNodeID(targetMacroID), + port: "macro-in-macro-cross-link", + }); + expect((jointGraph.getCell(JointGraphWrapper.getMacroNodeID(sourceMacroID)) as joint.dia.Element).getPorts().length) + .toBe(1); + expect((jointGraph.getCell(JointGraphWrapper.getMacroNodeID(targetMacroID)) as joint.dia.Element).getPorts().length) + .toBe(1); + expect(jointGraph.getCell(crossMacroLink.linkID).attr("root/display")).toBe("none"); + }); + describe("when linkBreakpoint is enabled", () => { let mockConfigService: MockGuiConfigService; diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts index f47aa8ab87d..2756cbe4788 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts @@ -19,13 +19,23 @@ import { fromEvent, Observable, ReplaySubject, Subject } from "rxjs"; import { filter, map } from "rxjs/operators"; -import { LogicalPort, Point } from "../../../types/workflow-common.interface"; +import { + LogicalPort, + OperatorLink, + OperatorPredicate, + Point, + WorkflowMacro, +} from "../../../types/workflow-common.interface"; import * as joint from "jointjs"; import * as dagre from "dagre"; import * as graphlib from "graphlib"; import { ObservableContextManager } from "src/app/common/util/context"; import { Coeditor, User } from "../../../../common/type/user"; -import { operatorCoeditorChangedPropertyClass, operatorCoeditorEditingClass } from "../../joint-ui/joint-ui.service"; +import { + JointUIService, + operatorCoeditorChangedPropertyClass, + operatorCoeditorEditingClass, +} from "../../joint-ui/joint-ui.service"; import { dia } from "jointjs/types/joint"; import * as _ from "lodash"; import Selectors = dia.Cell.Selectors; @@ -51,7 +61,7 @@ type JointModelEvent = [joint.dia.Cell, { graph: joint.dia.Graph; models: joint. type JointLinkChangeEvent = [joint.dia.Link, { x: number; y: number }, { ui: boolean; updateConnectionOnly: boolean }]; -type JointPositionChangeEvent = [joint.dia.Element, { x: number; y: number }]; +type JointPositionChangeEvent = [joint.dia.Element, { x: number; y: number }, { macroVisualSync?: boolean }?]; type PositionInfo = { currPos: Point; @@ -91,6 +101,19 @@ const DefaultContext: JointGraphContextType = { * For an overview of the services in WorkflowGraphModule, see workflow-graph-design.md */ export class JointGraphWrapper { + private static readonly MACRO_FRAME_PREFIX = "texera-macro-frame-"; + private static readonly MACRO_NODE_PREFIX = "texera-macro-node-"; + private static readonly MACRO_PROXY_LINK_PREFIX = "texera-macro-proxy-link-"; + private static readonly MACRO_FRAME_PADDING_X = 43; + private static readonly MACRO_NODE_WIDTH = 150; + private static readonly MACRO_NODE_HEIGHT = 48; + private static readonly MACRO_INTERNAL_TOP_GAP = 37; + private static readonly MACRO_FRAME_PADDING_TOP = + JointGraphWrapper.MACRO_NODE_HEIGHT + JointGraphWrapper.MACRO_INTERNAL_TOP_GAP; + private static readonly MACRO_FRAME_PADDING_BOTTOM = 70; + private static readonly MACRO_NODE_INSET = 0; + private static readonly MACRO_NODE_VIEW_KEY = "texeraMacroNodeView"; + // zoom diff represents the ratio that is zoom in/out everytime, for clicking +/- buttons or using mousewheel public static readonly ZOOM_CLICK_DIFF: number = 0.05; public static readonly INIT_ZOOM_VALUE: number = 1; @@ -110,6 +133,9 @@ export class JointGraphWrapper { private multiSelect: boolean = false; private reloadingWorkflow: boolean = false; + private collapsedMacroIDs = new Set(); + private workflowMacros: readonly WorkflowMacro[] = []; + private workflowLinks: readonly OperatorLink[] = []; // the currently highlighted operators' IDs private currentHighlightedOperators: string[] = []; @@ -260,6 +286,34 @@ export class JointGraphWrapper { }; } + public static getMacroFrameID(macroId: string): string { + return `${JointGraphWrapper.MACRO_FRAME_PREFIX}${macroId}`; + } + + public static getMacroNodeID(macroId: string): string { + return `${JointGraphWrapper.MACRO_NODE_PREFIX}${macroId}`; + } + + public static getMacroIDFromNodeID(elementID: string): string { + return elementID.substring(JointGraphWrapper.MACRO_NODE_PREFIX.length); + } + + public static isMacroFrameID(elementID: string): boolean { + return elementID.startsWith(JointGraphWrapper.MACRO_FRAME_PREFIX); + } + + public static isMacroNodeID(elementID: string): boolean { + return elementID.startsWith(JointGraphWrapper.MACRO_NODE_PREFIX); + } + + public static isMacroElementID(elementID: string): boolean { + return JointGraphWrapper.isMacroFrameID(elementID) || JointGraphWrapper.isMacroNodeID(elementID); + } + + public static isMacroProxyLinkID(linkID: string): boolean { + return linkID.startsWith(JointGraphWrapper.MACRO_PROXY_LINK_PREFIX); + } + public getCurrentHighlightedIDs(): readonly string[] { return [ ...this.currentHighlightedOperators, @@ -282,6 +336,7 @@ export class JointGraphWrapper { newPosition: Point; }> { return fromEvent(this.jointGraph, "change:position").pipe( + filter(e => !JointGraphWrapper.isMacroElementID(e[0].id.toString())), map(e => { const elementID = e[0].id.toString(); const oldPosition = this.elementPositions.get(elementID); @@ -309,6 +364,28 @@ export class JointGraphWrapper { ); } + public getMacroNodePositionChangeEvent(): Observable<{ macroID: string; oldPosition: Point; newPosition: Point }> { + return fromEvent(this.jointGraph, "change:position").pipe( + filter(e => JointGraphWrapper.isMacroNodeID(e[0].id.toString())), + filter(e => !(e[2]?.macroVisualSync ?? false)), + map(e => { + const elementID = e[0].id.toString(); + const newPosition = { x: e[1].x, y: e[1].y }; + const oldPosition = this.elementPositions.get(elementID); + const previousPosition = oldPosition?.currPos ?? newPosition; + this.elementPositions.set(elementID, { + currPos: newPosition, + lastPos: previousPosition, + }); + return { + macroID: JointGraphWrapper.getMacroIDFromNodeID(elementID), + oldPosition: previousPosition, + newPosition, + }; + }) + ); + } + public unhighlightElements(elements: JointHighlights): void { this.unhighlightOperators(...elements.operators); this.unhighlightLinks(...elements.links); @@ -586,7 +663,12 @@ export class JointGraphWrapper { public autoLayoutJoint(): void { joint.layout.DirectedGraph.layout( - [...this.jointGraph.getElements().filter(el => el.attributes.type !== "region"), ...this.jointGraph.getLinks()], + [ + ...this.jointGraph + .getElements() + .filter(el => el.attributes.type !== "region" && !JointGraphWrapper.isMacroElementID(el.id.toString())), + ...this.jointGraph.getLinks().filter(link => !JointGraphWrapper.isMacroProxyLinkID(link.id.toString())), + ], { dagre: dagre, graphlib: graphlib, @@ -598,6 +680,45 @@ export class JointGraphWrapper { resizeClusters: true, } ); + this.refreshPaperViews(); + } + + public autoLayoutMacroInternals( + macroID: string, + operators: readonly OperatorPredicate[], + links: readonly OperatorLink[], + origin: Point + ): string[] { + const internalOperatorIDs = new Set( + operators.filter(operator => operator.macroIdParent === macroID).map(operator => operator.operatorID) + ); + const internalElements = Array.from(internalOperatorIDs) + .map(operatorID => this.jointGraph.getCell(operatorID)) + .filter((cell): cell is joint.dia.Element => Boolean(cell?.isElement())); + if (!internalElements.length) return []; + + const internalLinks = links + .filter(link => internalOperatorIDs.has(link.source.operatorID) && internalOperatorIDs.has(link.target.operatorID)) + .map(link => this.jointGraph.getCell(link.linkID)) + .filter((cell): cell is joint.dia.Link => Boolean(cell?.isLink())); + + joint.layout.DirectedGraph.layout([...internalElements, ...internalLinks], { + dagre: dagre, + graphlib: graphlib, + nodeSep: 90, + edgeSep: 100, + rankSep: 80, + ranker: "tight-tree", + rankDir: "LR", + }); + + const minX = Math.min(...internalElements.map(element => element.position().x)); + const minY = Math.min(...internalElements.map(element => element.position().y)); + const offsetX = origin.x + JointGraphWrapper.MACRO_NODE_WIDTH + 70 - minX; + const offsetY = origin.y + JointGraphWrapper.MACRO_NODE_HEIGHT + JointGraphWrapper.MACRO_INTERNAL_TOP_GAP - minY; + internalElements.forEach(element => element.translate(offsetX, offsetY)); + this.refreshPaperViews(); + return internalElements.map(element => element.id.toString()); } /** @@ -646,6 +767,15 @@ export class JointGraphWrapper { return { x: position.x, y: position.y }; } + public getMacroFramePosition(macroID: string): Point | undefined { + const cell = this.jointGraph.getCell(JointGraphWrapper.getMacroFrameID(macroID)); + if (!cell?.isElement()) { + return undefined; + } + const position = (cell as joint.dia.Element).position(); + return { x: position.x, y: position.y }; + } + /** * This method repositions the element according to given offsets. * An element can be an operator or a group. @@ -678,6 +808,251 @@ export class JointGraphWrapper { element.position(posX, poY); } + public refreshMacroFrames( + operators: readonly OperatorPredicate[], + macros?: readonly WorkflowMacro[], + links?: readonly OperatorLink[] + ): void { + if (macros) this.workflowMacros = macros; + if (links) this.workflowLinks = links; + this.collapsedMacroIDs = new Set(this.workflowMacros.filter(macro => macro.collapsed).map(macro => macro.macroID)); + + const operatorGroups = new Map(); + const expandedMacroBounds = new Map(); + operators.forEach(operator => { + const macroId = operator.macroIdParent?.trim(); + const cell = this.jointGraph.getCell(operator.operatorID); + if (!macroId || this.collapsedMacroIDs.has(macroId) || !cell?.isElement()) return; + const position = (cell as joint.dia.Element).position(); + operatorGroups.set(macroId, [...(operatorGroups.get(macroId) ?? []), { x: position.x, y: position.y }]); + }); + + const activeFrameIDs = new Set(); + const activeNodeIDs = new Set(); + operatorGroups.forEach((positions, macroId) => { + const frameID = JointGraphWrapper.getMacroFrameID(macroId); + activeFrameIDs.add(frameID); + const minX = Math.min(...positions.map(pos => pos.x)); + const minY = Math.min(...positions.map(pos => pos.y)); + const maxX = Math.max(...positions.map(pos => pos.x + JointUIService.DEFAULT_OPERATOR_WIDTH)); + const maxY = Math.max(...positions.map(pos => pos.y + JointUIService.DEFAULT_OPERATOR_HEIGHT)); + const bounds = { + x: minX - JointGraphWrapper.MACRO_FRAME_PADDING_X, + y: minY - JointGraphWrapper.MACRO_FRAME_PADDING_TOP, + width: maxX - minX + JointGraphWrapper.MACRO_FRAME_PADDING_X * 2, + height: + maxY - + minY + + JointGraphWrapper.MACRO_FRAME_PADDING_TOP + + JointGraphWrapper.MACRO_FRAME_PADDING_BOTTOM, + }; + expandedMacroBounds.set(macroId, bounds); + const existingFrame = this.jointGraph.getCell(frameID); + const frame = existingFrame?.isElement() + ? (existingFrame as joint.dia.Element) + : new joint.shapes.standard.Rectangle(); + + frame.set("id", frameID); + frame.position(bounds.x, bounds.y, { macroVisualSync: true } as any); + frame.resize(bounds.width, bounds.height); + JointGraphWrapper.styleMacroFrame(frame); + if (!existingFrame) { + this.jointGraph.addCell(frame); + } + frame.toBack(); + }); + + if (this.workflowMacros) { + this.workflowMacros.forEach(macro => { + const nodeID = JointGraphWrapper.getMacroNodeID(macro.macroID); + activeNodeIDs.add(nodeID); + const collapsed = macro.collapsed ?? false; + const viewMode = collapsed ? "collapsed" : "expanded"; + const existingNode = this.jointGraph.getCell(nodeID); + let node = + existingNode?.isElement() && existingNode.get(JointGraphWrapper.MACRO_NODE_VIEW_KEY) === viewMode + ? (existingNode as joint.dia.Element) + : undefined; + if (!node) { + existingNode?.remove(); + node = JointGraphWrapper.createMacroNode(collapsed); + node.set("id", nodeID); + node.set(JointGraphWrapper.MACRO_NODE_VIEW_KEY, viewMode); + this.jointGraph.addCell(node); + } + const nodePosition = + collapsed || !expandedMacroBounds.has(macro.macroID) + ? macro.position + : JointGraphWrapper.getExpandedMacroTabPosition( + expandedMacroBounds.get(macro.macroID) as { x: number; y: number; width: number; height: number } + ); + + node.set("id", nodeID); + node.position(nodePosition.x, nodePosition.y, { macroVisualSync: true } as any); + node.resize( + collapsed ? JointUIService.DEFAULT_OPERATOR_WIDTH : JointGraphWrapper.MACRO_NODE_WIDTH, + collapsed ? JointUIService.DEFAULT_OPERATOR_HEIGHT : JointGraphWrapper.MACRO_NODE_HEIGHT + ); + JointGraphWrapper.styleMacroNode(node, macro.name, collapsed); + this.elementPositions.set(nodeID, { currPos: nodePosition, lastPos: undefined }); + }); + } + + this.jointGraph + .getCells() + .filter(cell => JointGraphWrapper.isMacroFrameID(cell.id.toString()) && !activeFrameIDs.has(cell.id.toString())) + .forEach(cell => cell.remove()); + if (this.workflowMacros) { + this.jointGraph + .getCells() + .filter(cell => JointGraphWrapper.isMacroNodeID(cell.id.toString()) && !activeNodeIDs.has(cell.id.toString())) + .forEach(cell => cell.remove()); + } + this.applyMacroCollapseState(operators); + this.refreshMacroBoundaryConnections(operators, this.workflowLinks, this.workflowMacros); + this.clearMacroSelectionChrome(); + this.refreshPaperViews(); + } + + private refreshMacroBoundaryConnections( + operators: readonly OperatorPredicate[], + links: readonly OperatorLink[], + macros: readonly WorkflowMacro[] + ): void { + this.jointGraph + .getLinks() + .filter(link => JointGraphWrapper.isMacroProxyLinkID(link.id.toString())) + .forEach(link => link.remove()); + + const macroByOperatorID = new Map( + operators + .filter(operator => operator.macroIdParent?.trim()) + .map(operator => [operator.operatorID, operator.macroIdParent?.trim() as string]) + ); + const collapsedMacroIDs = new Set(macros.filter(macro => macro.collapsed).map(macro => macro.macroID)); + const portsByMacroID = new Map(); + const proxyLinks: joint.dia.Link[] = []; + const addPort = (macroID: string, direction: "in" | "out", linkID: string): string => { + const ports = portsByMacroID.get(macroID) ?? []; + portsByMacroID.set(macroID, ports); + const portID = JointGraphWrapper.getMacroBoundaryPortID(direction, linkID); + if (!ports.some(port => port.id === portID)) { + ports.push({ + id: portID, + group: direction, + attrs: { + ".port-label": { + text: String(ports.filter(port => port.group === direction).length + 1), + }, + }, + }); + } + return portID; + }; + + links.forEach(link => { + const sourceMacroID = macroByOperatorID.get(link.source.operatorID); + const targetMacroID = macroByOperatorID.get(link.target.operatorID); + const sourceCollapsed = sourceMacroID !== undefined && collapsedMacroIDs.has(sourceMacroID); + const targetCollapsed = targetMacroID !== undefined && collapsedMacroIDs.has(targetMacroID); + if ((!sourceCollapsed && !targetCollapsed) || sourceMacroID === targetMacroID) return; + + const source = sourceCollapsed + ? { + operatorID: JointGraphWrapper.getMacroNodeID(sourceMacroID as string), + portID: addPort(sourceMacroID as string, "out", link.linkID), + } + : link.source; + const target = targetCollapsed + ? { + operatorID: JointGraphWrapper.getMacroNodeID(targetMacroID as string), + portID: addPort(targetMacroID as string, "in", link.linkID), + } + : link.target; + proxyLinks.push( + JointGraphWrapper.getMacroBoundaryLink( + JointGraphWrapper.getMacroProxyLinkID(sourceCollapsed ? "out" : "in", link.linkID), + source.operatorID, + source.portID, + target.operatorID, + target.portID + ) + ); + }); + + macros.forEach(macro => { + const node = this.jointGraph.getCell(JointGraphWrapper.getMacroNodeID(macro.macroID)) as + | joint.dia.Element + | undefined; + if (node?.isElement()) { + JointGraphWrapper.setMacroBoundaryPorts(node, portsByMacroID.get(macro.macroID) ?? []); + } + }); + this.jointGraph.addCells(proxyLinks); + } + + private applyMacroCollapseState(operators: readonly OperatorPredicate[]): void { + const collapsedOperatorIDs = new Set( + operators + .filter(operator => { + const macroID = operator.macroIdParent?.trim(); + return macroID !== undefined && this.collapsedMacroIDs.has(macroID); + }) + .map(operator => operator.operatorID) + ); + + operators.forEach(operator => { + this.setCellHidden(this.jointGraph.getCell(operator.operatorID), collapsedOperatorIDs.has(operator.operatorID)); + }); + this.jointGraph.getLinks().forEach(link => { + const sourceID = link.source()?.id; + const targetID = link.target()?.id; + this.setCellHidden( + link, + Boolean( + (sourceID && collapsedOperatorIDs.has(sourceID.toString())) || + (targetID && collapsedOperatorIDs.has(targetID.toString())) + ) + ); + }); + } + + private setCellHidden(cell: joint.dia.Cell | undefined, hidden: boolean): void { + if (!cell) return; + if (JointGraphWrapper.isMacroProxyLinkID(cell.id.toString())) return; + cell.attr("root/display", hidden ? "none" : null); + const view = this.getMainJointPaper()?.findViewByModel(cell); + if (view) { + view.el.style.display = hidden ? "none" : ""; + } + } + + private refreshPaperViews(): void { + if (!this.mainPaper) return; + const refresh = () => { + this.clearMacroSelectionChrome(); + this.mainPaper.updateViews(); + this.jointGraph.getLinks().forEach(link => { + const linkView = this.mainPaper.findViewByModel(link) as (joint.dia.LinkView & { + requestConnectionUpdate?: () => void; + }) | null; + linkView?.requestConnectionUpdate?.(); + }); + }; + refresh(); + if (typeof requestAnimationFrame === "function") { + requestAnimationFrame(refresh); + } + } + + private clearMacroSelectionChrome(): void { + if (!this.mainPaper) return; + this.workflowMacros.forEach(macro => { + const view = this.mainPaper.findViewByModel(JointGraphWrapper.getMacroNodeID(macro.macroID)); + view?.el.querySelectorAll(".joint-highlight-stroke").forEach(element => element.remove()); + }); + } + /** * Highlights the link with given linkID. * Emits an event to the link highlight stream. @@ -747,6 +1122,189 @@ export class JointGraphWrapper { public setListenPositionChange(listenPositionChange: boolean): void { this.listenPositionChange = listenPositionChange; } + + private static styleMacroFrame(frame: joint.dia.Element): void { + frame.set("type", "macro-frame"); + frame.set("z", 0); + frame.attr({ + body: { + fill: "rgba(47, 84, 235, 0.04)", + stroke: "#2f54eb", + "stroke-width": 2, + "stroke-dasharray": "8 4", + rx: 8, + ry: 8, + "pointer-events": "none", + }, + label: { text: "" }, + }); + } + + private static getExpandedMacroTabPosition(bounds: { x: number; y: number; width: number; height: number }): Point { + return { + x: bounds.x + JointGraphWrapper.MACRO_NODE_INSET, + y: bounds.y + JointGraphWrapper.MACRO_NODE_INSET, + }; + } + + private static createMacroNode(collapsed: boolean): joint.dia.Element { + return collapsed + ? (new joint.shapes.devs.Model({ + markup: JointUIService.getOperatorElementMarkup(), + ports: { groups: JointGraphWrapper.getMacroBoundaryPortGroups() }, + }) as joint.dia.Element) + : new joint.shapes.standard.Rectangle(); + } + + private static styleMacroNode(node: joint.dia.Element, name: string, collapsed: boolean): void { + node.set("type", "macro-node"); + node.set("z", 2); + if (collapsed) { + const macroOperator = { + operatorID: node.id.toString(), + operatorType: "WorkflowMacro", + operatorVersion: "", + operatorProperties: {}, + inputPorts: [], + outputPorts: [], + showAdvanced: false, + customDisplayName: name, + } as OperatorPredicate; + node.attr( + JointUIService.getCustomOperatorStyleAttrs( + macroOperator, + JointUIService.truncateOperatorDisplayName(name), + "WorkflowMacro", + "Workflow Macro" + ) + ); + node.attr({ + "rect.body": { stroke: "#CFCFCF", cursor: "move" }, + ".texera-operator-icon": { cursor: "move" }, + ".texera-operator-name": { cursor: "move" }, + ".texera-operator-friendly-name": { cursor: "move" }, + }); + JointGraphWrapper.setMacroBoundaryPortGroups(node); + return; + } + node.attr({ + body: { + fill: collapsed ? "#fff7e6" : "#e6f4ff", + stroke: collapsed ? "#fa8c16" : "#1677ff", + "stroke-width": 2, + rx: 8, + ry: 8, + cursor: "move", + }, + label: { + text: collapsed ? `${name}\ncollapsed` : name, + fill: collapsed ? "#ad6800" : "#2f54eb", + "font-size": 13, + "font-weight": 600, + "text-anchor": "middle", + "x-alignment": "middle", + "y-alignment": "middle", + refX: 0.5, + refY: 0.5, + ref: "body", + cursor: "move", + }, + }); + JointGraphWrapper.setMacroBoundaryPortGroups(node); + } + + private static getMacroBoundaryPortID(direction: "in" | "out", linkID: string): string { + return `macro-${direction}-${linkID}`; + } + + private static getMacroProxyLinkID(direction: "in" | "out", linkID: string): string { + return `${JointGraphWrapper.MACRO_PROXY_LINK_PREFIX}${direction}-${linkID}`; + } + + private static getMacroBoundaryLink( + linkID: string, + sourceOperatorID: string, + sourcePortID: string, + targetOperatorID: string, + targetPortID: string + ): joint.dia.Link { + const link = JointUIService.getDefaultLinkCell(); + link.set("id", linkID); + link.set("type", "macro-proxy-link"); + link.set("source", { id: sourceOperatorID, port: sourcePortID }); + link.set("target", { id: targetOperatorID, port: targetPortID }); + link.attr({ + ".connection": { + stroke: "#919191", + "stroke-width": "2px", + "stroke-dasharray": "6 3", + }, + ".connection-wrap": { + "stroke-width": "0px", + }, + ".tool-remove": { + display: "none", + }, + ".marker-source": { + display: "none", + }, + ".marker-arrowhead-group-source": { + display: "none", + }, + ".marker-arrowhead-group-target": { + display: "none", + }, + }); + return link; + } + + private static setMacroBoundaryPortGroups(node: joint.dia.Element): void { + node.set("portMarkup", ''); + node.set("portLabelMarkup", ''); + if (!node.prop("ports/groups")) { + node.prop("ports/groups", JointGraphWrapper.getMacroBoundaryPortGroups()); + } + } + + private static setMacroBoundaryPorts(node: joint.dia.Element, ports: joint.dia.Element.Port[]): void { + node.set("ports", { + groups: JointGraphWrapper.getMacroBoundaryPortGroups(), + items: ports, + }); + } + + private static getMacroBoundaryPortGroups(): Record { + return { + in: JointGraphWrapper.getMacroBoundaryPortGroup("left"), + out: JointGraphWrapper.getMacroBoundaryPortGroup("right"), + }; + } + + private static getMacroBoundaryPortGroup(position: "left" | "right"): joint.dia.Element.PortGroup { + return { + position: { name: position }, + attrs: { + ".port-body": { + fill: "#8c8c8c", + stroke: "#ffffff", + "stroke-width": 1, + r: 5, + magnet: false, + }, + ".port-label": { + fill: "#595959", + "font-size": 10, + }, + }, + label: { + position: { + name: position, + args: { y: 7 }, + }, + }, + }; + } + /** * Highlights the element with given elementID. * diff --git a/frontend/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts b/frontend/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts index e5eab7a812e..bf9af1c2cb4 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts @@ -44,6 +44,7 @@ import { GuiConfigService } from "../../../../common/service/gui-config.service" export class SharedModelChangeHandler { private config: GuiConfigService | null = null; + private macroFrameRefreshScheduled = false; constructor( private texeraGraph: WorkflowGraph, @@ -137,6 +138,10 @@ export class SharedModelChangeHandler { this.jointGraphWrapper.setMultiSelectMode(newOpIDs.length > 1); this.jointGraphWrapper.highlightOperators(...newOpIDs); } + this.refreshMacroFrames(); + if (newOpIDs.length > 0) { + this.scheduleMacroFrameRefresh(); + } }); } @@ -200,6 +205,10 @@ export class SharedModelChangeHandler { // // Only highlight when this is added by current user. // this.jointGraphWrapper.highlightLinks(...linksToAdd.map(link => link.linkID)); // } + if (linksToAdd.length > 0 || linksToDelete.length > 0) { + this.refreshMacroFrames(); + this.scheduleMacroFrameRefresh(); + } }); } @@ -236,6 +245,7 @@ export class SharedModelChangeHandler { */ private handleElementPositionChange(): void { this.texeraGraph.sharedModel.elementPositionMap?.observe((event: Y.YMapEvent) => { + let shouldRefreshMacroFrames = false; event.changes.keys.forEach((change, key) => { if (change.action === "update") { this.texeraGraph.setSyncTexeraGraph(false); @@ -244,10 +254,15 @@ export class SharedModelChangeHandler { this.jointGraphWrapper.setListenPositionChange(false); this.jointGraphWrapper.setAbsolutePosition(key, newPosition.x, newPosition.y); this.jointGraphWrapper.setListenPositionChange(true); + shouldRefreshMacroFrames = true; } this.texeraGraph.setSyncTexeraGraph(true); } }); + if (shouldRefreshMacroFrames) { + this.refreshMacroFrames(); + this.scheduleMacroFrameRefresh(); + } }); } @@ -336,6 +351,9 @@ export class SharedModelChangeHandler { } } else if (contentKey === "operatorProperties") { this.onOperatorPropertyChanged(operatorID, event.transaction.local); + } else if (contentKey === "macroIdParent") { + this.refreshMacroFrames(); + this.scheduleMacroFrameRefresh(); } } } else if (event.path[event.path.length - 1] === "customDisplayName") { @@ -429,6 +447,30 @@ export class SharedModelChangeHandler { } } + private refreshMacroFrames(): void { + this.jointGraphWrapper.refreshMacroFrames( + this.texeraGraph.getAllOperators(), + undefined, + this.texeraGraph.getAllLinks() + ); + } + + private scheduleMacroFrameRefresh(): void { + if (this.macroFrameRefreshScheduled) { + return; + } + this.macroFrameRefreshScheduled = true; + const refresh = () => { + this.macroFrameRefreshScheduled = false; + this.refreshMacroFrames(); + }; + if (typeof requestAnimationFrame === "function") { + requestAnimationFrame(refresh); + } else { + setTimeout(refresh, 0); + } + } + private onPortAdded(operatorID: string, isInput: boolean, port: PortDescription) { const operatorJointElement = this.jointGraph.getCell(operatorID); const portGroup = isInput ? "in" : "out"; diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts index 4ddf3b90068..32f76306a97 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts @@ -40,6 +40,7 @@ import { WorkflowActionService } from "./workflow-action.service"; import { OperatorPredicate } from "../../../types/workflow-common.interface"; import { WorkflowUtilService } from "../util/workflow-util.service"; import { commonTestProviders } from "../../../../common/testing/test-utils"; +import { JointGraphWrapper } from "./joint-graph-wrapper"; describe("WorkflowActionService", () => { let service: WorkflowActionService; @@ -250,6 +251,104 @@ describe("WorkflowActionService", () => { expect(texeraGraph.getAllLinks().length).toEqual(0); }); + it("should adopt an external operator only when its edges are fully internal to one macro", () => { + const macroID = "macro-1"; + const internalSource = { ...mockScanPredicate, operatorID: "internal-source", macroIdParent: macroID }; + const candidate = { ...mockSentimentPredicate, operatorID: "candidate" }; + const internalSink = { ...mockResultPredicate, operatorID: "internal-sink", macroIdParent: macroID }; + + service.addOperator(internalSource, mockPoint); + service.addOperator(candidate, mockPoint); + service.addOperator(internalSink, mockPoint); + + service.addLink({ + linkID: "macro-link-1", + source: { operatorID: internalSource.operatorID, portID: internalSource.outputPorts[0].portID }, + target: { operatorID: candidate.operatorID, portID: candidate.inputPorts[0].portID }, + }); + expect(texeraGraph.getOperator(candidate.operatorID)?.macroIdParent).toBeUndefined(); + + service.addLink({ + linkID: "macro-link-2", + source: { operatorID: candidate.operatorID, portID: candidate.outputPorts[0].portID }, + target: { operatorID: internalSink.operatorID, portID: internalSink.inputPorts[0].portID }, + }); + expect(texeraGraph.getOperator(candidate.operatorID)?.macroIdParent).toBe(macroID); + }); + + it("should not adopt macro boundary source or sink operators", () => { + const macroID = "macro-1"; + const internalSource = { ...mockScanPredicate, operatorID: "internal-source", macroIdParent: macroID }; + const internalSink = { ...mockResultPredicate, operatorID: "internal-sink", macroIdParent: macroID }; + const externalSource = { ...mockScanPredicate, operatorID: "external-source" }; + const externalSink = { ...mockResultPredicate, operatorID: "external-sink" }; + + service.addOperator(internalSource, mockPoint); + service.addOperator(internalSink, mockPoint); + service.addOperator(externalSource, mockPoint); + service.addOperator(externalSink, mockPoint); + + service.addLink({ + linkID: "boundary-link-1", + source: { operatorID: externalSource.operatorID, portID: externalSource.outputPorts[0].portID }, + target: { operatorID: internalSink.operatorID, portID: internalSink.inputPorts[0].portID }, + }); + service.addLink({ + linkID: "boundary-link-2", + source: { operatorID: internalSource.operatorID, portID: internalSource.outputPorts[0].portID }, + target: { operatorID: externalSink.operatorID, portID: externalSink.inputPorts[0].portID }, + }); + + expect(texeraGraph.getOperator(externalSource.operatorID)?.macroIdParent).toBeUndefined(); + expect(texeraGraph.getOperator(externalSink.operatorID)?.macroIdParent).toBeUndefined(); + }); + + it("should move expanded macro internals from the current frame position when dragging the macro tab", () => { + const macroID = service.createMacroAt({ x: 100, y: 100 }, "Macro 1"); + const internalSource = { ...mockScanPredicate, operatorID: "internal-source", macroIdParent: macroID }; + const internalSink = { ...mockResultPredicate, operatorID: "internal-sink", macroIdParent: macroID }; + + service.addOperator(internalSource, { x: 500, y: 300 }); + service.addOperator(internalSink, { x: 650, y: 300 }); + service.addLink({ + linkID: "macro-internal-link", + source: { operatorID: internalSource.operatorID, portID: internalSource.outputPorts[0].portID }, + target: { operatorID: internalSink.operatorID, portID: internalSink.inputPorts[0].portID }, + }); + + const framePosition = service.getJointGraphWrapper().getMacroFramePosition(macroID) as { x: number; y: number }; + const sourcePosition = service.getJointGraphWrapper().getElementPosition(internalSource.operatorID); + const macroNode = jointGraph.getCell(JointGraphWrapper.getMacroNodeID(macroID)) as joint.dia.Element; + macroNode.position(framePosition.x - 120, framePosition.y + 40); + + expect(service.getJointGraphWrapper().getElementPosition(internalSource.operatorID)).toEqual({ + x: sourcePosition.x - 120, + y: sourcePosition.y + 40, + }); + }); + + it("should keep macro dragging enabled after centering an empty workflow", () => { + service.calculateTopLeftOperatorPosition(); + + const macroID = service.createMacroAt({ x: 100, y: 100 }, "Macro 1"); + const internalSource = { ...mockScanPredicate, operatorID: "internal-source", macroIdParent: macroID }; + const internalSink = { ...mockResultPredicate, operatorID: "internal-sink", macroIdParent: macroID }; + + service.addOperator(internalSource, { x: 500, y: 300 }); + service.addOperator(internalSink, { x: 650, y: 300 }); + + const framePosition = service.getJointGraphWrapper().getMacroFramePosition(macroID) as { x: number; y: number }; + const sourcePosition = service.getJointGraphWrapper().getElementPosition(internalSource.operatorID); + const macroNode = jointGraph.getCell(JointGraphWrapper.getMacroNodeID(macroID)) as joint.dia.Element; + macroNode.position(framePosition.x - 80, framePosition.y + 30); + + expect(undoRedo.listenJointCommand).toBeTruthy(); + expect(service.getJointGraphWrapper().getElementPosition(internalSource.operatorID)).toEqual({ + x: sourcePosition.x - 80, + y: sourcePosition.y + 30, + }); + }); + it("should reformat the workflow", () => { service.addOperator(mockScanPredicate, mockPoint); service.addOperator(mockSentimentPredicate, mockPoint); diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts index e3ea66c024c..af7ad89d0a3 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts @@ -31,6 +31,7 @@ import { OperatorPredicate, Point, PortDescription, + WorkflowMacro, } from "../../../types/workflow-common.interface"; import { JointUIService } from "../../joint-ui/joint-ui.service"; import { OperatorMetadataService } from "../../operator-metadata/operator-metadata.service"; @@ -96,7 +97,10 @@ export class WorkflowActionService { public readonly resultPanelOpen$: Observable = this.resultPanelOpenSubject.asObservable(); private workflowSettings: WorkflowSettings; + private workflowMacros: WorkflowMacro[] = []; private workflowResetSubject = new Subject(); + private macroChangeSubject = new Subject(); + private macroVisualRefreshSubject = new Subject(); constructor( private operatorMetadataService: OperatorMetadataService, @@ -122,6 +126,7 @@ export class WorkflowActionService { this.undoRedoService.setUndoManager(this.texeraGraph.sharedModel.undoManager); this.handleJointElementDrag(); + this.handleMacroNodeDrag(); } private getDefaultSettings(): WorkflowSettings { @@ -354,6 +359,7 @@ export class WorkflowActionService { this.deleteOperator(operatorID); }); }); + this.refreshMacroVisuals(); } /** @@ -390,27 +396,29 @@ export class WorkflowActionService { public calculateTopLeftOperatorPosition(): void { this.texeraGraph.bundleActions(() => { this.undoRedoService.setListenJointCommand(false); - const allOperators = this.getTexeraGraph().getAllOperators(); - if (allOperators.length === 0) return; + try { + const allOperators = this.getTexeraGraph().getAllOperators(); + if (allOperators.length === 0) return; - let minX = Infinity; - let minY = Infinity; + let minX = Infinity; + let minY = Infinity; - for (const operator of allOperators) { - const operatorID = operator.operatorID; - const position = this.jointGraphWrapper.getElementPosition(operatorID); + for (const operator of allOperators) { + const operatorID = operator.operatorID; + const position = this.jointGraphWrapper.getElementPosition(operatorID); - if (position.x < minX) { - minX = position.x; - } - if (position.y < minY) { - minY = position.y; + if (position.x < minX) { + minX = position.x; + } + if (position.y < minY) { + minY = position.y; + } } - } - - this.centerPoint = { x: minX, y: minY }; - this.undoRedoService.setListenJointCommand(true); + this.centerPoint = { x: minX, y: minY }; + } finally { + this.undoRedoService.setListenJointCommand(true); + } }); } @@ -423,6 +431,10 @@ export class WorkflowActionService { this.texeraGraph.assertLinkNotExists(link); this.texeraGraph.assertLinkIsValid(link); this.texeraGraph.addLink(link); + if (this.adoptFullyInternalMacroOperators(link)) { + this.refreshMacroVisuals(); + this.macroChangeSubject.next(); + } } /** @@ -457,6 +469,150 @@ export class WorkflowActionService { }); } + public createMacroForOperators(operatorIDs: readonly string[], name = "Macro"): string { + const macroID = this.workflowUtilService.getGroupRandomUUID().replace(/^group-/, "macro-"); + this.workflowMacros = [ + ...this.workflowMacros.filter(macro => macro.macroID !== macroID), + { macroID, name, position: this.getMacroNodePositionForOperators(operatorIDs) }, + ]; + this.setOperatorsMacroParent(operatorIDs, macroID); + return macroID; + } + + public setOperatorsMacroParent(operatorIDs: readonly string[], macroIdParent?: string): void { + this.texeraGraph.bundleActions(() => { + operatorIDs.forEach(operatorID => { + this.texeraGraph.assertOperatorExists(operatorID); + const sharedOperator = this.texeraGraph.getSharedOperatorType(operatorID) as any; + if (macroIdParent) { + sharedOperator.set("macroIdParent", macroIdParent); + } else { + sharedOperator.delete("macroIdParent"); + } + }); + }); + this.refreshMacroVisuals(); + this.macroChangeSubject.next(); + } + + public createMacroAt(position: Point, name = "Workflow Macro"): string { + const macroID = this.workflowUtilService.getGroupRandomUUID().replace(/^group-/, "macro-"); + this.workflowMacros = [...this.workflowMacros, { macroID, name, position }]; + this.refreshMacroVisuals(); + this.macroChangeSubject.next(); + return macroID; + } + + public getWorkflowMacro(macroID: string): WorkflowMacro | undefined { + return this.workflowMacros.find(macro => macro.macroID === macroID); + } + + public getWorkflowMacros(): readonly WorkflowMacro[] { + return this.workflowMacros; + } + + public getMacroVisualRefreshStream(): Observable { + return this.macroVisualRefreshSubject.asObservable(); + } + + public setMacroCollapsed(macroID: string, collapsed: boolean): void { + const visiblePosition = collapsed ? this.getCurrentMacroNodePosition(macroID) : undefined; + this.workflowMacros = this.workflowMacros.map(macro => + macro.macroID === macroID ? { ...macro, collapsed, position: visiblePosition ?? macro.position } : macro + ); + this.refreshMacroVisuals(); + this.macroChangeSubject.next(); + } + + public deleteMacros(macroIDs: readonly string[]): void { + if (!macroIDs.length) return; + const macroIDSet = new Set(macroIDs); + const operatorIDs = this.texeraGraph + .getAllOperators() + .filter(operator => operator.macroIdParent !== undefined && macroIDSet.has(operator.macroIdParent)) + .map(operator => operator.operatorID); + if (operatorIDs.length) { + this.deleteOperatorsAndLinks(operatorIDs); + } + this.workflowMacros = this.workflowMacros.filter(macro => !macroIDSet.has(macro.macroID)); + this.refreshMacroVisuals(); + this.macroChangeSubject.next(); + } + + public replaceMacroWorkflow( + macroID: string, + workflowContent: WorkflowContent, + workflowId?: number, + workflowName?: string + ): void { + this.deleteOperatorsAndLinks( + this.texeraGraph + .getAllOperators() + .filter(operator => operator.macroIdParent === macroID) + .map(operator => operator.operatorID) + ); + this.workflowMacros = this.workflowMacros.map(macro => + macro.macroID === macroID + ? { + ...macro, + name: workflowName ?? macro.name, + workflowId, + workflowName, + } + : macro + ); + this.addWorkflowContentToMacro( + macroID, + workflowContent, + this.getWorkflowMacro(macroID)?.position ?? this.centerPoint + ); + this.refreshMacroVisuals(); + this.macroChangeSubject.next(); + } + + private addWorkflowContentToMacro(macroID: string, workflowContent: WorkflowContent, origin: Point): void { + const idMap = new Map(); + const sourcePositions = Object.values(workflowContent.operatorPositions ?? {}); + const minX = Math.min(...sourcePositions.map(pos => pos.x), 0); + const minY = Math.min(...sourcePositions.map(pos => pos.y), 0); + + const operatorsAndPositions = workflowContent.operators.map(operator => { + const operatorID = `${operator.operatorType}-${this.workflowUtilService.getOperatorRandomUUID()}`; + idMap.set(operator.operatorID, operatorID); + const sourcePosition = workflowContent.operatorPositions[operator.operatorID] ?? { x: minX, y: minY }; + return { + op: this.workflowUtilService.updateOperatorVersion({ + ...operator, + operatorID, + macroIdParent: macroID, + }), + pos: { + x: origin.x + sourcePosition.x - minX, + y: origin.y + sourcePosition.y - minY + 80, + }, + }; + }); + + const links = workflowContent.links + .filter(link => idMap.has(link.source.operatorID) && idMap.has(link.target.operatorID)) + .map(link => ({ + linkID: this.workflowUtilService.getLinkRandomUUID(), + source: { ...link.source, operatorID: idMap.get(link.source.operatorID) as string }, + target: { ...link.target, operatorID: idMap.get(link.target.operatorID) as string }, + })); + + const commentBoxes = (workflowContent.commentBoxes ?? []).map(commentBox => ({ + ...commentBox, + commentBoxID: this.workflowUtilService.getCommentBoxRandomUUID(), + commentBoxPosition: { + x: origin.x + commentBox.commentBoxPosition.x - minX, + y: origin.y + commentBox.commentBoxPosition.y - minY + 80, + }, + })); + + this.addOperatorsAndLinks(operatorsAndPositions, links, commentBoxes); + } + public setPortProperty(operatorPortID: LogicalPort, newProperty: object) { this.texeraGraph.bundleActions(() => { this.texeraGraph.setPortProperty(operatorPortID, newProperty); @@ -641,12 +797,14 @@ export class WorkflowActionService { this.jointGraphWrapper.jointGraph.clear(); if (workflow === undefined) { + this.workflowMacros = []; this.setNewSharedModel(); return; } const workflowContent: WorkflowContent = workflow.content; this.workflowSettings = workflowContent.settings || this.getDefaultSettings(); + this.workflowMacros = workflowContent.macros ?? []; let operatorsAndPositions: { op: OperatorPredicate; pos: Point }[] = []; workflowContent.operators.forEach(op => { @@ -664,6 +822,7 @@ export class WorkflowActionService { operatorsAndPositions = this.updateOperatorVersions(operatorsAndPositions); this.addOperatorsAndLinks(operatorsAndPositions, links, commentBoxes); + this.refreshMacroVisuals(); // restore the view point if (restoreViewport) { @@ -701,6 +860,7 @@ export class WorkflowActionService { this.getTexeraGraph().getOperatorVersionChangedStream(), this.getTexeraGraph().getPortDisplayNameChangedSubject(), this.getTexeraGraph().getPortPropertyChangedStream(), + this.macroChangeSubject.asObservable(), this.workflowResetSubject.asObservable() ); } @@ -748,6 +908,7 @@ export class WorkflowActionService { const operatorPositions: { [key: string]: Point } = {}; const commentBoxes = texeraGraph.getAllCommentBoxes(); const settings = this.workflowSettings; + const macros = this.workflowMacros; texeraGraph .getAllOperators() @@ -763,6 +924,7 @@ export class WorkflowActionService { links, commentBoxes, settings, + macros, }; } @@ -859,56 +1021,199 @@ export class WorkflowActionService { .pipe( filter(() => this.jointGraphWrapper.getListenPositionChange()), filter(() => this.undoRedoService.listenJointCommand), - filter(() => this.texeraGraph.getSyncTexeraGraph()), - filter(movedElement => - this.jointGraphWrapper - .getCurrentHighlightedOperatorIDs() - .concat(this.jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()) - .includes(movedElement.elementID) - ) + filter(() => this.texeraGraph.getSyncTexeraGraph()) ) .subscribe(movedElement => { - this.texeraGraph.bundleActions(() => { - if ( - this.texeraGraph.sharedModel.elementPositionMap.get(movedElement.elementID) !== movedElement.newPosition - ) { - // For syncing ops/comment boxes in shared editing + const movedOperator = this.texeraGraph.sharedModel.operatorIDMap.has(movedElement.elementID) + ? this.texeraGraph.getOperator(movedElement.elementID) + : undefined; + const selectedElements = this.jointGraphWrapper + .getCurrentHighlightedOperatorIDs() + .concat(this.jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()); + + if (selectedElements.includes(movedElement.elementID)) { + this.texeraGraph.bundleActions(() => { + if ( + this.texeraGraph.sharedModel.elementPositionMap.get(movedElement.elementID) !== movedElement.newPosition + ) { + // For syncing ops/comment boxes in shared editing + this.texeraGraph.sharedModel.elementPositionMap.set(movedElement.elementID, movedElement.newPosition); + // For moving all highlighted operators + const selectedElements = this.jointGraphWrapper + .getCurrentHighlightedOperatorIDs() + .concat(this.jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()); + const offsetX = movedElement.newPosition.x - movedElement.oldPosition.x; + const offsetY = movedElement.newPosition.y - movedElement.oldPosition.y; + this.jointGraphWrapper.setListenPositionChange(false); + this.undoRedoService.setListenJointCommand(false); + // Persistence and shared-editing syncing for comment boxes have different interfaces. + // Setting positions inside commentBoxes here only for persistence. + // Syncing uses elementPositionMap. + selectedElements + .filter(elementID => elementID.includes("commentBox")) + .forEach(elementID => { + this.texeraGraph.sharedModel.commentBoxMap + .get(elementID) + ?.set("commentBoxPosition", this.jointGraphWrapper.getElementPosition(elementID)); + }); + // Move other highlighted operators. + selectedElements + .filter(elementID => elementID !== movedElement.elementID) + .forEach(elementID => { + this.jointGraphWrapper.setElementPosition(elementID, offsetX, offsetY); + this.texeraGraph.sharedModel.elementPositionMap.set( + elementID, + this.jointGraphWrapper.getElementPosition(elementID) + ); + }); + this.jointGraphWrapper.setListenPositionChange(true); + this.undoRedoService.setListenJointCommand(true); + } + }); + } else { + if (movedOperator && this.texeraGraph.sharedModel.elementPositionMap.has(movedElement.elementID)) { this.texeraGraph.sharedModel.elementPositionMap.set(movedElement.elementID, movedElement.newPosition); - // For moving all highlighted operators - const selectedElements = this.jointGraphWrapper - .getCurrentHighlightedOperatorIDs() - .concat(this.jointGraphWrapper.getCurrentHighlightedCommentBoxIDs()); - const offsetX = movedElement.newPosition.x - movedElement.oldPosition.x; - const offsetY = movedElement.newPosition.y - movedElement.oldPosition.y; + } + } + + if (movedOperator?.macroIdParent) { + this.refreshMacroVisuals(); + } + }); + } + + private handleMacroNodeDrag(): void { + this.jointGraphWrapper + .getMacroNodePositionChangeEvent() + .pipe( + filter(() => this.jointGraphWrapper.getListenPositionChange()), + filter(() => this.undoRedoService.listenJointCommand) + ) + .subscribe(({ macroID, oldPosition, newPosition }) => { + const macro = this.getWorkflowMacro(macroID); + if (!macro) return; + const referencePosition = + macro.collapsed ?? false ? oldPosition : this.jointGraphWrapper.getMacroFramePosition(macroID) ?? oldPosition; + const offsetX = newPosition.x - referencePosition.x; + const offsetY = newPosition.y - referencePosition.y; + + this.workflowMacros = this.workflowMacros.map(macro => + macro.macroID === macroID ? { ...macro, position: newPosition } : macro + ); + if ((macro.collapsed ?? false) && (offsetX !== 0 || offsetY !== 0)) { + this.refreshMacroVisuals(); + } else if (offsetX !== 0 || offsetY !== 0) { + const internalOperatorIDs = this.texeraGraph + .getAllOperators() + .filter(operator => operator.macroIdParent === macroID) + .map(operator => operator.operatorID); + this.texeraGraph.bundleActions(() => { this.jointGraphWrapper.setListenPositionChange(false); this.undoRedoService.setListenJointCommand(false); - // Persistence and shared-editing syncing for comment boxes have different interfaces. - // Setting positions inside commentBoxes here only for persistence. - // Syncing uses elementPositionMap. - selectedElements - .filter(elementID => elementID.includes("commentBox")) - .forEach(elementID => { - this.texeraGraph.sharedModel.commentBoxMap - .get(elementID) - ?.set("commentBoxPosition", this.jointGraphWrapper.getElementPosition(elementID)); - }); - // Move other highlighted operators. - selectedElements - .filter(elementID => elementID !== movedElement.elementID) - .forEach(elementID => { - this.jointGraphWrapper.setElementPosition(elementID, offsetX, offsetY); + try { + internalOperatorIDs.forEach(operatorID => { + this.jointGraphWrapper.setElementPosition(operatorID, offsetX, offsetY); this.texeraGraph.sharedModel.elementPositionMap.set( - elementID, - this.jointGraphWrapper.getElementPosition(elementID) + operatorID, + this.jointGraphWrapper.getElementPosition(operatorID) ); }); - this.jointGraphWrapper.setListenPositionChange(true); - this.undoRedoService.setListenJointCommand(true); - } - }); + } finally { + this.jointGraphWrapper.setListenPositionChange(true); + this.undoRedoService.setListenJointCommand(true); + } + }); + this.refreshMacroVisuals(); + } + this.macroChangeSubject.next(); }); } + private refreshMacroVisuals(): void { + this.jointGraphWrapper.refreshMacroFrames( + this.texeraGraph.getAllOperators(), + this.workflowMacros, + this.texeraGraph.getAllLinks() + ); + this.macroVisualRefreshSubject.next(); + } + + private adoptFullyInternalMacroOperators(link: OperatorLink): boolean { + const adoptedOperatorIDs = [link.source.operatorID, link.target.operatorID].filter( + (operatorID, index, ids) => ids.indexOf(operatorID) === index && this.getMacroIDForFullyInternalOperator(operatorID) + ); + adoptedOperatorIDs.forEach(operatorID => { + const macroID = this.getMacroIDForFullyInternalOperator(operatorID); + if (macroID) { + (this.texeraGraph.getSharedOperatorType(operatorID) as any).set("macroIdParent", macroID); + } + }); + return adoptedOperatorIDs.length > 0; + } + + private getMacroIDForFullyInternalOperator(operatorID: string): string | undefined { + const operator = this.texeraGraph.getOperator(operatorID); + if (!operator || operator.macroIdParent) return undefined; + + const operatorsByID = new Map(this.texeraGraph.getAllOperators().map(operator => [operator.operatorID, operator])); + const incidentLinks = this.texeraGraph + .getAllLinks() + .filter(link => link.source.operatorID === operatorID || link.target.operatorID === operatorID); + if (!incidentLinks.length) return undefined; + + const neighborMacroIDs = incidentLinks.map(link => { + const neighborID = link.source.operatorID === operatorID ? link.target.operatorID : link.source.operatorID; + return operatorsByID.get(neighborID)?.macroIdParent; + }); + const macroIDs = Array.from(new Set(neighborMacroIDs.filter((macroID): macroID is string => Boolean(macroID)))); + if (macroIDs.length !== 1 || neighborMacroIDs.some(macroID => macroID !== macroIDs[0])) return undefined; + + const macroID = macroIDs[0]; + const hasInternalInput = incidentLinks.some( + link => link.target.operatorID === operatorID && operatorsByID.get(link.source.operatorID)?.macroIdParent === macroID + ); + const hasInternalOutput = incidentLinks.some( + link => link.source.operatorID === operatorID && operatorsByID.get(link.target.operatorID)?.macroIdParent === macroID + ); + return hasInternalInput && hasInternalOutput ? macroID : undefined; + } + + private getCurrentMacroNodePosition(macroID: string): Point | undefined { + try { + return this.jointGraphWrapper.getElementPosition(JointGraphWrapper.getMacroNodeID(macroID)); + } catch { + return undefined; + } + } + + private getMacroNodePositionForOperators(operatorIDs: readonly string[]): Point { + return this.getMacroNodePositionFromPositions( + operatorIDs.map(operatorID => this.jointGraphWrapper.getElementPosition(operatorID)) + ); + } + + private getMacroNodePositionFromPositions(positions: readonly Point[]): Point { + if (positions.length === 0) { + return this.getNextMacroImportOrigin(); + } + return { + x: Math.min(...positions.map(position => position.x)) - 70, + y: Math.min(...positions.map(position => position.y)) - 115, + }; + } + + private getNextMacroImportOrigin(): Point { + const operators = this.texeraGraph.getAllOperators(); + if (operators.length === 0) { + return this.centerPoint; + } + const positions = operators.map(operator => this.jointGraphWrapper.getElementPosition(operator.operatorID)); + return { + x: Math.max(...positions.map(position => position.x)) + JointUIService.DEFAULT_OPERATOR_WIDTH + 180, + y: Math.min(...positions.map(position => position.y)) - 80, + }; + } + private updateOperatorVersions(operatorsAndPositions: { op: OperatorPredicate; pos: Point }[]) { const updatedOperators: { op: OperatorPredicate; pos: Point }[] = []; for (const operatorsAndPosition of operatorsAndPositions) { diff --git a/frontend/src/app/workspace/types/workflow-common.interface.ts b/frontend/src/app/workspace/types/workflow-common.interface.ts index 3fb3aaa3d4b..3e711776839 100644 --- a/frontend/src/app/workspace/types/workflow-common.interface.ts +++ b/frontend/src/app/workspace/types/workflow-common.interface.ts @@ -76,6 +76,7 @@ export interface OperatorPredicate viewResult?: boolean; markedForReuse?: boolean; customDisplayName?: string; + macroIdParent?: string; }> {} export interface Comment @@ -92,6 +93,16 @@ export interface CommentBox { commentBoxPosition: Point; } +export interface WorkflowMacro + extends Readonly<{ + macroID: string; + name: string; + position: Point; + workflowId?: number; + workflowName?: string; + collapsed?: boolean; + }> {} + export interface OperatorLink extends Readonly<{ linkID: string; diff --git a/frontend/src/assets/operator_images/WorkflowMacro.png b/frontend/src/assets/operator_images/WorkflowMacro.png new file mode 100644 index 0000000000000000000000000000000000000000..6ca97e62b0513b6ebc66ec87640e0d4f2c40497c GIT binary patch literal 8218 zcmeHsc~DbXx9X|z%mo8IsIYDS6u9}9>nYYn?&!LEvu52?o_~9D#1` zfkRN6E9`ZmH3VJLhKCF_azP=qOVF&02m~dOxg3$j5cDlb1gd%nT!LB(eKkG;K_>`2 z^*%pASY3xLdTdpUk7iVkm0y+&`<9Ve>=S_txo%Mc&gzT=J z@-V7_P2PcDu$dOn6}64&zkr4+zuoD69?0F!BbxE18xf-P_F#GDfAg#1DqFZTOV86a zFt8?FP070pJNzyyObc+7GhJ0#8TgytdZt;rS~`WJjnvxR4ZH3!^2O=A9G&>t+1du# z=NL!jAUUeZQ6Q~XnkDW#kLIYrJwJ$6uL7l z1s%}P{5!5xiCYjf`J6}BE`>B%F7DA=HwP}q zEQh6j02JL5v>&R}Ds;Qi|Lj`1xW$03Meyh%riKE5Kc$(dElc5X(E0nuED2fWZV0)l*A`JXC*ha%DElpk3h~%Ioi@0h_&qG>lmLWAlZ%R&WLdNoG%KOg!I$?vu9{Ei*7JV2X#o%Gm-b>F0wuy;a0W_0{~=n?Vd(eKLGjIYfT zqXD!!g&WCEb!prn6t;x&_0fy-q_ATy2QaedMQ)S~zg7XUbL>>;pPQewO1?IC8V1ix z8A?X5?h6n>EM1duX~W0dnrPF$df=hZ4cEHAK!mJn=?l#tO~+*}B=t0(G?cp3HjVVE z2N74@Ge#{qNI_hThPk_~3n?1rP@2a>njyJg9>U5y$)wq*+ zCAp7vp9GRXY;0)$(%jcLr1F-bhI8uTnoD&OonH8gr4W4Jc4S;EW)FfB_`|`h`}qsB zUehf1K+E;2WA-UjZ<{Bzu)@XRkJrtU5+`+m;n5xn*1$4byR7tZ-S;QkNCaW}IuQWZ}=rMsQ&|?}%Xb zBzH`D_Y`8A@H(dbR*Pr0HWl61OIyMX!%*Yi#QS2S4kleI*-W}TDXA>=`?|%Nz5B`= zk1tgt9kmOcnhYkk>Ql95P#zY)9CCLz#oo?Nt7YJZVifXKLaa!4QsQ#&%RT#Q3NF(H zC7F`!(sKcrwyUqrJEb2Mv`NPpP$}2|iI*eJxqxx`gpA+93luufsm5lE?yR)cJCV&5 z6c*HM7^dD6L?sxpa^DR}0?y%>*vhqY=@l1?DHky`x8ZVIkX2H)hM46pT^X7uvD!W7 z^`{-4ZHB75pK2l+4&BP^7|69=@Mwu%wHsj0D{B#BJfsna=GsjwzSG{gd)I2NG*|B` z6avsh1m2W6veVBu=is@Ixa0M+_^ESOm}G4*=><(7f$wzdgPu!wHm^^Ss|){ zHo`MCH_Hwr=A1ySOz}(y>w@Km)9n>)ytzU%zKf~IDw?fvcTRd=%wJ2#5CGuwp8=zd zWAz=rDG4_}P?Yw?rkr2!GpK&a$Bx?9|Apn+ zxu|s9Ujjdal1=8)aU&}7kAB5;a{1{T++HLBVX)a*{xclldXu(I;R4$FYNVI` z_S{U$>sGP8``cAH}1l-%bmDXO@`#`0ne$W@V5+Ku6SXh|SC^jt0Slg$95_yp*2s|;9a_w^D z(#14%k+8GNZQ>Ni5vK*NfD@DXWUnVl?2@C81~4#Kmv>xT9O3?Mk0D0gqaRSCwQ!6T ztmIt>I4*EbOd8&5%rVym^tyHA(xx81@Yf$;*$$y(7WzlfFgV+abBtISP56P*{vK6h zNz%I1-NiTUc$>!mB!N){#K6fg?VS?4$|>^5QKUHVbE6vdaQB3iCNaGHN>9O$!*|3+ zKj|G9(ja}{!gO)GtvgZr1!0a|2=;8`--{4NXeCc2Z+UN=C~wn1ewZ|2h&jGD#s@?` z=Akj4_IgF-+$xekURf4(3KiEm&)(gjBf{{S0SlG4hj-Ul2_x+KNe9zyl4pN8iW0G? z7WrX%mOG=Q-Jt^5AoY8>?$E7^#T}X6JC9XjI0Xx7y^!+^XZWdDXUc8jJ{j>RDdH3g zD{pV%lJp`m@Cv96#9Vwcmqb}->r{~Rk03#*e3&GSkB?`I?`J9C8V9=-ICaCpbT~~? zw@!qSv;U8YEPGA~!MoS)fl9E$B;m&3e;|4JY;mPlqdyiq`%GVC1kdY*w)m@j_}U%x z-9fo|>zob{Ax~PbZg)IpZx6~BMZeXax=zz0P~iCUA)n=wXdAaH30Xxua2F1N^PUy$ zkzMr6&J=LuZ5a@`d=^LEF9k1#CgW|e@2P&2taeU;rAH7VKH||O12wen2%db)BFmgS zbI7%ED}%E4gR{)tMQ6(M_J*dWCTalQR$B6|&IDdYzZvnAl34>yS>8xn%IL~4DD12# z82A3A{4=d7vD#Go_>{?5Pfrh_N}5RcN>F{U!SpfkfAn}oIL~3sn_sa}%t3@PmL6NH z>$bHvXVh;nYr3z6X;F{8cmJsg@S8qO{F`IOU2y>f+rvdZ)%S6Rn-2!q(&q>ZeM19w zlniPC?adv_&IU~F=F_#$2AM0@pW_Ujo3}E=&k}|k#p+V{(!FAQZB%h5V=Dy8o>MZZ zdszDnvBJx}CCcuDPepv8o$`vOX%B-lt6Lpr5^#q4q^(0CH7}GK)wE;rv6Lxl5jGZQ zINAtC#p9Lh^XlNH@bI!;Ied-ztkJZ_sw`~Q80?#dyLBiD-_!XxLl$RhaxUFFZ7Mlv z>oTOvm*u|09f=KI3ij$Oz-PaQ>1KN*lc=Rm(4S*qx~+V^W>oNFzI(Z_Ww*Yz0I zl6Wmh@W|XD6GYntHl^Yw!YDZ-knG2vtyJ{T@`$sij#~14i_zTBM%%1;XY?@@5|S}l(;nv!iZc5<@**2>bH$EdaH@<3AL@85X3hpJ za^lC#-=6)TO(RL;p`aKaK2as1lF|%=EhQx-Z4QIbbJ2IkMd5u{YCjK1gm5R4lgY{= zCFvOpyFLtvhlGL?61KB|y&`ZYpo;t2H4$jjgn@cj{TT+1bj_eZdQpLdf*fJ5$r+{M zoxel}+~Th11Cv@+jmgVzctgPXd-m8cb#5ngpcwjUXRkT8D?}4)BH5|Ob)PHBUas3Csv2wn$Rt7e&y6ORs1vue|JmHNW_bqG@87={ z(R#TDKQ=hwlzvI`pBd+} zmxV$3`OlppbxCykf>y8-sE*O`;t0HIRD|5`ktCvZi8rqnXccd;kNwU|@$N^Fq#b5m7|9=4huh&ZpR|T!mki;7giAj%^66Y7vTPK>um2W@V2MS;N153`g zc}3YmoQ_YgKP+Zh#%?!_;7s0U%QWZobcT81A?##O7c;3}^EMJ;=$=%)?Qs!!$jW)$ zEzIo2$fYYIYem0u5Gp#j4vt4KUHcNth9&PV)-G1{KHzkS20UOfwNgm-rPZsTb+)l! zr?TA(de2iozXk(VX)h=cfehX{s@X$KZ9L9#Hz%EE{USZIFIo1AdyK^bah@>lT_Q_c zSeH~GiT2jv`A4GfMHoiRZu!>VH-&=E@{d$$X*;?549Xb9BapEsD5KEhT8QLbSGNo8 zGS6G4VLcfwr6Z^^49zE6H229$qds8$e0M{UsHiA`s?+S&8il(OjTz1ARGK8_RD^R1 z?UduMGRXt*y3wm(ug0(=ZvoY6uvs=PV^y#TQekNxv6xs^;=S(m`lQsbDJYg%wSuKGC)nRfTZ$*)Mx50g>4G?w{_SfvQeyZOgq$jj8kpEAdWSrE^27 z#ssebgNC$@YECKgsZ^e~vSkcxKlc z)$=*=@^KuW%r2u!|ES;r4ZD!ea4q)GzHwu+D8X`~>ZpyD-;$&jMc^p%QKD9BlyOFs zX~2?84)bZnl-xvfHbpo%TO=S}W}WG(k-PD0UNy?NYhVb%j4tlI>ElJ(ft_t7IPCJ1 z9Pl^4m>W0L5HBGD^-hiZgb=UKMxH<(?_ucfeih4!oC9yHro<`F3F^&aY#Da^q$ez< zgw8?IKGr$-)mGlb{^(xi^dyJjmNQq^3|QKyWxX*9Hba2eymWeNRN~~^L6LxKm9;tE z)g8*Fz2*Jqia>8eah~@D0=s9F^)!XAW>8`1=5iF0@vPIk;Eig7ZTqrMJwH9B)Tx^A zGIl{`$dB_-F|+D2-SH_gwPz^g{In#NERZAAexz~4j$R*ZH8L_*{ zI!qZYI^IN`v9xV`!&u_=v%;Fh+H`yIL!(tgK8yCs}l>?yCBlk1pd< z*V+g2DE&)asa|W;_)blvpr=xh{cE+ClRVO222eTyF2OU2Hp)_DB4W4Rx(Y6u`M}xF zWx57-2JKlEh$eO@-(5VgL1>BD$U(0LArF$)nWW(joeg}&h1|%Vj;Q0v`8zKw)GpL0 zY~@qO7;hZ6sK@NXaitYiFj~XdTojwVYXe_;A)J1qGJMpo#`OHk8&u933BjV+ntl}a z{M{Q(zN{hr=nB3v|l5UEim&;cpCA(7lvnitkm% zu6~Z%w5$BecHMFn#<*%d%2^{`j}xjXu%D?}Le*iXxMJ9UB6XslrO~9&15YI8c~w8A zlcw$I8p!#CmlZwEHAu?jni1oJBs=~IIGB&)9qzZ%s=ed9aLCkC5+^O!@|%g**8m`8 z+HQ%eE*qgkV-jaK8}PE+6)bsMhB{{3N&Bis~~-~m?Gn_h|{Q_*XNhEl$8is1E^)h_ac3XLGauMt_+)&jXDLEjCf8g zg-G?@u+}6ZUZ1dx_Q)eHwF8lgfV>*HrG+q~-5FP!5j(JhUAq#uqQr`*7U)%79}2lh zYNS{sWp{51ok8basWIrA#1?M63R&1u^n3E58qK1mwmO$@Rj>+@yf+nf6&NEg5X}^v zvPZDy6%>ygxBYrn9W7DWJX(BwerDH0VsUEE;0FOPZ7wjnvSUHAQ~Z(GXfMY2N+?*e zF~eU5o%n^969|vI*tD?EQ+r9tmJ75|o6%ZE=~%x%$Z={OH7bc&{^BsF*}y8gr{gK? zZH(l7i*T$KvPeSoY!h{=srWFBU^yy^4}-WN^24}I_B02n+ThSal}+=Ywb^waiv@F_ ziEz5lvn&MbTiQf47e$1z_(kQX_wt%iI;ba)q}n#G$^QJr)8N*z$d%tu-_ss6ip&HZ zne;%yOHPXeTkv(V*-QgCtvJRRbYISI+eZFN70PSH`Gh;{sBu^6Q?b#AfL7M?jr7}A zn9emL=BxHKPts4c?Eg5A%I361=PcuevC`ts{#KA-vVPKsGI~q{@e!XDO?NU4GfIh6`wVQNMwH7@`%2u zokc;IfzlR1a?F2|`Twu~cY4UNUa_LpDISpR`Ms!TUq{PdXK4PPoBQ^uAfG$?czJ_} z4HBn@dj5yOw}~$hrX4NVJ<0nmT|hc%jmn-Jt9oc6sBjE9FEc?h2Px;YY?yF7$e5VX z!ytLQjC~&t^3g-$l2CFGl7l@#8fqN$=OnhtvDZ?VQ6DFtlbrnMd&BxMNJT2h6izDr zKZyfHtk4db(KG)k1bkSycc;Za;BkfG;*`|aoC2%G* zQ{5{E8YNHuo(?(%{$XOHrU4x;In`|gLjEVg$wLK60rfmN-#6@$jjh>j;`Q|x2=d6| zI2G_wO9gG@s5L=SarUVWTM4kLw0k0Yo?*;CG6yQ}*lNYw^cRTc2;ve9biQ>2z!xY{ z^UBsLu~E?g1lP=_|_-|;@h|DC+{vA**o^B zqCt@%k5)(p`#Vkr`6VVLEm9>|3w`&o04UIrQz9;u`TyrWMgPA_u3_L$ X#-*>P&aa-~Kp37eIbEpdc;|ls1&!7t literal 0 HcmV?d00001 From 1bb00da859cbb338c3eb04024d63a83974ccaca3 Mon Sep 17 00:00:00 2001 From: carloea2 Date: Sun, 24 May 2026 04:59:37 -0600 Subject: [PATCH 2/3] refactor: drop create-macro-from-selection path Remove the context-menu "create macro" action together with its supporting service methods (canCreateMacroFromHighlightedOperators, createMacroFromHighlightedOperators, createMacroForOperators, and the three private position helpers it pulled in). Empty macros can still be created via toolbox drag-drop or operator search; "remove from macro" stays in the context menu. Test rewritten to set up macro membership via createMacroAt + setOperatorsMacroParent. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../context-menu/context-menu.component.html | 14 ------- .../context-menu.component.spec.ts | 2 - .../operator-menu.service.spec.ts | 23 ++++------- .../operator-menu/operator-menu.service.ts | 11 ------ .../model/workflow-action.service.ts | 38 ------------------- 5 files changed, 8 insertions(+), 80 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html index ce9632d643c..6d50e8232d8 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html @@ -36,20 +36,6 @@ highlightedCommentBoxIds.length === 0 && !hasHighlightedLinks() && isWorkflowModifiable && - operatorMenuService.canCreateMacroFromHighlightedOperators()" - (click)="operatorMenuService.createMacroFromHighlightedOperators()"> - - create macro -
    • -
  • { viewResultHighlightedOperators: vi.fn(), reuseResultHighlightedOperator: vi.fn(), executeUpToOperator: vi.fn(), - canCreateMacroFromHighlightedOperators: vi.fn().mockReturnValue(false), canRemoveHighlightedOperatorsFromMacro: vi.fn().mockReturnValue(false), - createMacroFromHighlightedOperators: vi.fn(), removeHighlightedOperatorsFromMacro: vi.fn(), } as unknown as Mocked; diff --git a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts index da4b80fe856..b9e027da914 100644 --- a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts +++ b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.spec.ts @@ -185,7 +185,7 @@ describe("OperatorMenuService", () => { }); }); - it("groups highlighted operators under a macro parent and can remove them", () => { + it("clears macroIdParent on highlighted operators via removeHighlightedOperatorsFromMacro", () => { workflowActionService.addOperatorsAndLinks( [ { op: mockScanPredicate, pos: mockPoint }, @@ -193,21 +193,16 @@ describe("OperatorMenuService", () => { ], [] ); + const macroID = workflowActionService.createMacroAt(mockPoint); + workflowActionService.setOperatorsMacroParent( + [mockScanPredicate.operatorID, mockSentimentPredicate.operatorID], + macroID + ); + const wrapper = workflowActionService.getJointGraphWrapper(); wrapper.unhighlightOperators(...wrapper.getCurrentHighlightedOperatorIDs()); wrapper.highlightOperators(mockScanPredicate.operatorID, mockSentimentPredicate.operatorID); - service.createMacroFromHighlightedOperators(); - - const scanMacroId = workflowActionService.getTexeraGraph().getOperator(mockScanPredicate.operatorID).macroIdParent; - const sentimentMacroId = workflowActionService - .getTexeraGraph() - .getOperator(mockSentimentPredicate.operatorID).macroIdParent; - expect(scanMacroId).toBeTruthy(); - expect(sentimentMacroId).toBe(scanMacroId); - expect(workflowActionService.getWorkflowContent().macros).toEqual([ - expect.objectContaining({ macroID: scanMacroId, name: "Macro" }), - ]); expect(service.canRemoveHighlightedOperatorsFromMacro()).toBe(true); service.removeHighlightedOperatorsFromMacro(); @@ -218,8 +213,6 @@ describe("OperatorMenuService", () => { expect( workflowActionService.getTexeraGraph().getOperator(mockSentimentPredicate.operatorID).macroIdParent ).toBeUndefined(); - expect(workflowActionService.getWorkflowContent().macros).toEqual([ - expect.objectContaining({ macroID: scanMacroId, name: "Macro" }), - ]); + expect(service.canRemoveHighlightedOperatorsFromMacro()).toBe(false); }); }); diff --git a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts index 1877b6ae171..37539fb1c98 100644 --- a/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts +++ b/frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts @@ -266,10 +266,6 @@ export class OperatorMenuService { this.executeWorkflowService.executeWorkflow("", targetOperatorId); } - public canCreateMacroFromHighlightedOperators(): boolean { - return this._highlightedOperators$.value.length > 0; - } - public canRemoveHighlightedOperatorsFromMacro(): boolean { const texeraGraph = this.workflowActionService.getTexeraGraph(); return this._highlightedOperators$.value.some(operatorID => @@ -277,13 +273,6 @@ export class OperatorMenuService { ); } - public createMacroFromHighlightedOperators(): void { - const operatorIDs = this._highlightedOperators$.value; - if (!operatorIDs.length) return; - this.workflowActionService.createMacroForOperators(operatorIDs); - this.notificationService.info("Created macro for selected operators."); - } - public removeHighlightedOperatorsFromMacro(): void { const operatorIDs = this._highlightedOperators$.value; if (!operatorIDs.length) return; diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts index af7ad89d0a3..ae125c3dcc5 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts @@ -469,16 +469,6 @@ export class WorkflowActionService { }); } - public createMacroForOperators(operatorIDs: readonly string[], name = "Macro"): string { - const macroID = this.workflowUtilService.getGroupRandomUUID().replace(/^group-/, "macro-"); - this.workflowMacros = [ - ...this.workflowMacros.filter(macro => macro.macroID !== macroID), - { macroID, name, position: this.getMacroNodePositionForOperators(operatorIDs) }, - ]; - this.setOperatorsMacroParent(operatorIDs, macroID); - return macroID; - } - public setOperatorsMacroParent(operatorIDs: readonly string[], macroIdParent?: string): void { this.texeraGraph.bundleActions(() => { operatorIDs.forEach(operatorID => { @@ -1186,34 +1176,6 @@ export class WorkflowActionService { } } - private getMacroNodePositionForOperators(operatorIDs: readonly string[]): Point { - return this.getMacroNodePositionFromPositions( - operatorIDs.map(operatorID => this.jointGraphWrapper.getElementPosition(operatorID)) - ); - } - - private getMacroNodePositionFromPositions(positions: readonly Point[]): Point { - if (positions.length === 0) { - return this.getNextMacroImportOrigin(); - } - return { - x: Math.min(...positions.map(position => position.x)) - 70, - y: Math.min(...positions.map(position => position.y)) - 115, - }; - } - - private getNextMacroImportOrigin(): Point { - const operators = this.texeraGraph.getAllOperators(); - if (operators.length === 0) { - return this.centerPoint; - } - const positions = operators.map(operator => this.jointGraphWrapper.getElementPosition(operator.operatorID)); - return { - x: Math.max(...positions.map(position => position.x)) + JointUIService.DEFAULT_OPERATOR_WIDTH + 180, - y: Math.min(...positions.map(position => position.y)) - 80, - }; - } - private updateOperatorVersions(operatorsAndPositions: { op: OperatorPredicate; pos: Point }[]) { const updatedOperators: { op: OperatorPredicate; pos: Point }[] = []; for (const operatorsAndPosition of operatorsAndPositions) { From c538fdd5cd4ee3ad648eb1e460b3501ec6c470af Mon Sep 17 00:00:00 2001 From: carloea2 Date: Sun, 24 May 2026 05:07:28 -0600 Subject: [PATCH 3/3] v2 --- .../macro-property-edit-frame.component.scss | 19 +++++++++++++++++++ .../workflow-editor.component.ts | 2 +- .../model/joint-graph-wrapper.spec.ts | 10 ++++++---- .../model/joint-graph-wrapper.ts | 18 +++++++++--------- .../model/workflow-action.service.ts | 9 ++++++--- 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss index 686c6353f1c..5684286b239 100644 --- a/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss +++ b/frontend/src/app/workspace/component/property-editor/macro-property-edit-frame/macro-property-edit-frame.component.scss @@ -1,3 +1,22 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + .macro-field-label { display: block; margin-bottom: 6px; diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts index a9ea00d84ab..92d98d95d05 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts @@ -357,7 +357,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy .getAllOperators() .forEach(op => { this.jointUIService.changeOperatorState(this.paper, op.operatorID, operatorState); - }); + }); } }); diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts index 2804f166f5e..783103ccf07 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts @@ -672,10 +672,12 @@ describe("JointGraphWrapperService", () => { id: JointGraphWrapper.getMacroNodeID(targetMacroID), port: "macro-in-macro-cross-link", }); - expect((jointGraph.getCell(JointGraphWrapper.getMacroNodeID(sourceMacroID)) as joint.dia.Element).getPorts().length) - .toBe(1); - expect((jointGraph.getCell(JointGraphWrapper.getMacroNodeID(targetMacroID)) as joint.dia.Element).getPorts().length) - .toBe(1); + expect( + (jointGraph.getCell(JointGraphWrapper.getMacroNodeID(sourceMacroID)) as joint.dia.Element).getPorts().length + ).toBe(1); + expect( + (jointGraph.getCell(JointGraphWrapper.getMacroNodeID(targetMacroID)) as joint.dia.Element).getPorts().length + ).toBe(1); expect(jointGraph.getCell(crossMacroLink.linkID).attr("root/display")).toBe("none"); }); diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts index 2756cbe4788..73c61ad4312 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts @@ -698,7 +698,9 @@ export class JointGraphWrapper { if (!internalElements.length) return []; const internalLinks = links - .filter(link => internalOperatorIDs.has(link.source.operatorID) && internalOperatorIDs.has(link.target.operatorID)) + .filter( + link => internalOperatorIDs.has(link.source.operatorID) && internalOperatorIDs.has(link.target.operatorID) + ) .map(link => this.jointGraph.getCell(link.linkID)) .filter((cell): cell is joint.dia.Link => Boolean(cell?.isLink())); @@ -840,11 +842,7 @@ export class JointGraphWrapper { x: minX - JointGraphWrapper.MACRO_FRAME_PADDING_X, y: minY - JointGraphWrapper.MACRO_FRAME_PADDING_TOP, width: maxX - minX + JointGraphWrapper.MACRO_FRAME_PADDING_X * 2, - height: - maxY - - minY + - JointGraphWrapper.MACRO_FRAME_PADDING_TOP + - JointGraphWrapper.MACRO_FRAME_PADDING_BOTTOM, + height: maxY - minY + JointGraphWrapper.MACRO_FRAME_PADDING_TOP + JointGraphWrapper.MACRO_FRAME_PADDING_BOTTOM, }; expandedMacroBounds.set(macroId, bounds); const existingFrame = this.jointGraph.getCell(frameID); @@ -1033,9 +1031,11 @@ export class JointGraphWrapper { this.clearMacroSelectionChrome(); this.mainPaper.updateViews(); this.jointGraph.getLinks().forEach(link => { - const linkView = this.mainPaper.findViewByModel(link) as (joint.dia.LinkView & { - requestConnectionUpdate?: () => void; - }) | null; + const linkView = this.mainPaper.findViewByModel(link) as + | (joint.dia.LinkView & { + requestConnectionUpdate?: () => void; + }) + | null; linkView?.requestConnectionUpdate?.(); }); }; diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts index ae125c3dcc5..8416b933886 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts @@ -1130,7 +1130,8 @@ export class WorkflowActionService { private adoptFullyInternalMacroOperators(link: OperatorLink): boolean { const adoptedOperatorIDs = [link.source.operatorID, link.target.operatorID].filter( - (operatorID, index, ids) => ids.indexOf(operatorID) === index && this.getMacroIDForFullyInternalOperator(operatorID) + (operatorID, index, ids) => + ids.indexOf(operatorID) === index && this.getMacroIDForFullyInternalOperator(operatorID) ); adoptedOperatorIDs.forEach(operatorID => { const macroID = this.getMacroIDForFullyInternalOperator(operatorID); @@ -1160,10 +1161,12 @@ export class WorkflowActionService { const macroID = macroIDs[0]; const hasInternalInput = incidentLinks.some( - link => link.target.operatorID === operatorID && operatorsByID.get(link.source.operatorID)?.macroIdParent === macroID + link => + link.target.operatorID === operatorID && operatorsByID.get(link.source.operatorID)?.macroIdParent === macroID ); const hasInternalOutput = incidentLinks.some( - link => link.source.operatorID === operatorID && operatorsByID.get(link.target.operatorID)?.macroIdParent === macroID + link => + link.source.operatorID === operatorID && operatorsByID.get(link.target.operatorID)?.macroIdParent === macroID ); return hasInternalInput && hasInternalOutput ? macroID : undefined; }