This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 43ca4b2c70 refactor(frontend): avoid redundant validateOperator call
in applyOperatorBorder (#5702)
43ca4b2c70 is described below
commit 43ca4b2c70849af57656bc9a777d0e53f617bcc4
Author: Prateek Ganigi <[email protected]>
AuthorDate: Wed Jun 24 22:55:06 2026 -0700
refactor(frontend): avoid redundant validateOperator call in
applyOperatorBorder (#5702)
### What changes were proposed in this PR?
WorkflowEditorComponent.applyOperatorBorder(operatorID)` always
recomputes validation as its first step:
```typescript
const validation =
this.validationWorkflowService.validateOperator(operatorID);
```
This is redundant when the helper is called from the validation-stream
subscriber in handleOperatorValidation, the stream's emitted event
already carries the Validation result that was just computed by
updateValidationState.
This PR:
- Adds an optional validation?: Validation parameter to
applyOperatorBorder. The helper uses it when provided, and falls back to
validateOperator(operatorID) otherwise.
- Updates handleOperatorValidation to pass value.validation from the
stream into the helper.
- Leaves the operator-add stream subscriber unchanged and hence it
doesn't have a Validation in hand at that point, so it correctly falls
through to the lazy-fetch path.
- Functionally identical (the stream emits the same Validation that
would have been recomputed), purely avoids the duplicate call.
Originally flagged as an optional nit during the PR #5146 review.
Attempted in PR #5626 but split out as per the reviewer's request so
that PR could stay scoped to test restructuring; this PR completes the
follow-up.
### Any related issues, documentation, discussions?
Closes #5683
Related: PR #5146 (where the nit was raised), PR #5626 (where this was
attempted and split out).
### How was this PR tested?
Added one focused unit test in `workflow-editor.component.spec.ts`
inside the existing `operator border restoration after navigation`
describe block: `it("uses the Validation passed in instead of
recomputing it", ...)`. The test clears the `validateOperator` spy after
the operator-add validation chain settles, then calls
`applyOperatorBorder` directly with a `Validation` argument and asserts
`validateOperator` is not called.
Verified locally:
- `tsc --noEmit`: clean
- `eslint`: clean
- `ng test` (jsdom): 26/26 pass in the editor spec (was 25, new test
adds one)
- `ng run gui:test-browser`: 13/13 pass
### Was this PR authored or co-authored using generative AI tooling?
This PR was co-authored using Claude Code (Anthropic Claude Opus 4.7)
---
.../workflow-editor.component.spec.ts | 28 ++++++++++++++++++++++
.../workflow-editor/workflow-editor.component.ts | 14 +++++++----
2 files changed, 37 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 c87ea058e7..81a84081a5 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,34 @@ 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 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");
+
+ // 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();
+ });
+
+ 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");
+ });
});
});
});
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 5411ea995a..1a9eb89e05 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));
}
/**
