From 0dfd935475a6142a8f0f3077b16ddf807c4f8947 Mon Sep 17 00:00:00 2001 From: PG1204 Date: Sat, 13 Jun 2026 16:53:37 -0700 Subject: [PATCH 1/2] refactor(frontend): reuse Validation from stream in applyOperatorBorder --- .../workflow-editor.component.spec.ts | 16 ++++++++++++++++ .../workflow-editor/workflow-editor.component.ts | 14 +++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts index c87ea058e76..fbc0d14e477 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts @@ -910,6 +910,22 @@ describe("WorkflowEditorComponent", () => { expect(getStroke(mockScanPredicate.operatorID)).toBe("red"); }); + + it("uses the Validation passed in instead of recomputing it", () => { + // Let the validation chain settle from the operator-add so subsequent + // calls to validateOperator are isolated to the helper itself. + workflowActionService.addOperator(mockScanPredicate, mockPoint); + fixture.detectChanges(); + + const validateSpy = vi.spyOn(validationWorkflowService, "validateOperator").mockClear(); + + // Call the helper directly with a Validation argument, mirroring what + // the validation-stream subscriber does at runtime + // (handleOperatorValidation passes value.validation through). + (component as any).applyOperatorBorder(mockScanPredicate.operatorID, { isValid: true }); + + expect(validateSpy).not.toHaveBeenCalled(); + }); }); }); }); 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 5411ea995a0..1a9eb89e05f 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 @@ -25,7 +25,7 @@ import { DragDropService } from "../../service/drag-drop/drag-drop.service"; import { DynamicSchemaService } from "../../service/dynamic-schema/dynamic-schema.service"; import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-workflow.service"; import { fromJointPaperEvent, JointUIService, linkPathStrokeColor } from "../../service/joint-ui/joint-ui.service"; -import { ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; +import { Validation, ValidationWorkflowService } from "../../service/validation/validation-workflow.service"; import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; import { WorkflowStatusService } from "../../service/workflow-status/workflow-status.service"; import { ExecutionState, OperatorState } from "../../types/execute-workflow.interface"; @@ -397,10 +397,14 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy * Centralizing this here avoids the race where the validation pass * overwrites a state-derived stroke (or vice versa) for an operator that * is both invalid and has a cached execution status. + * + * Callers that already have a Validation result (the validation stream + * subscriber) may pass it in to avoid recomputing it; callers without one + * (the operator-add stream subscriber) let the helper fetch it lazily. */ - private applyOperatorBorder(operatorID: string): void { - const validation = this.validationWorkflowService.validateOperator(operatorID); - if (!validation.isValid) { + private applyOperatorBorder(operatorID: string, validation?: Validation): void { + const resolved = validation ?? this.validationWorkflowService.validateOperator(operatorID); + if (!resolved.isValid) { this.jointUIService.changeOperatorColor(this.paper, operatorID, false); return; } @@ -1025,7 +1029,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy this.validationWorkflowService .getOperatorValidationStream() .pipe(untilDestroyed(this)) - .subscribe(value => this.applyOperatorBorder(value.operatorID)); + .subscribe(value => this.applyOperatorBorder(value.operatorID, value.validation)); } /** From cda9952a2783cbd59d951474f1d9e58f4a3097ef Mon Sep 17 00:00:00 2001 From: PG1204 Date: Mon, 22 Jun 2026 14:29:41 -0700 Subject: [PATCH 2/2] test(frontend): drop redundant mockClear and assert passed-in validation paints red --- .../workflow-editor.component.spec.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts index fbc0d14e477..81a84081a5d 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts @@ -912,12 +912,12 @@ describe("WorkflowEditorComponent", () => { }); it("uses the Validation passed in instead of recomputing it", () => { - // Let the validation chain settle from the operator-add so subsequent - // calls to validateOperator are isolated to the helper itself. + // Let the validation chain settle from the operator-add so the spy + // below is created after those calls and starts with a clean slate. workflowActionService.addOperator(mockScanPredicate, mockPoint); fixture.detectChanges(); - const validateSpy = vi.spyOn(validationWorkflowService, "validateOperator").mockClear(); + const validateSpy = vi.spyOn(validationWorkflowService, "validateOperator"); // Call the helper directly with a Validation argument, mirroring what // the validation-stream subscriber does at runtime @@ -926,6 +926,18 @@ describe("WorkflowEditorComponent", () => { expect(validateSpy).not.toHaveBeenCalled(); }); + + it("honors the passed-in Validation result (paints red when it is invalid)", () => { + // Proves the passed-in value actually drives the border, not just that + // the recompute is skipped: an invalid result must paint red. + vi.spyOn(workflowStatusService, "getCurrentStatus").mockReturnValue({}); + workflowActionService.addOperator(mockScanPredicate, mockPoint); + fixture.detectChanges(); + + (component as any).applyOperatorBorder(mockScanPredicate.operatorID, { isValid: false, messages: {} }); + + expect(getStroke(mockScanPredicate.operatorID)).toBe("red"); + }); }); }); });