apache / apache/texera

Unify operator border-coloring so a newly-added operator is validated and painted once

Open
#5,726 11 comments 0 reactions 0 assignees View on GitHub
Dominant language
Scala
Stars
314
Forks
187
Avg merge
1d 21h
Merged PRs (30d)
214

Description

### Task Summary
When an operator is newly added, its border is currently validated and painted twice, once by the operator-add stream subscriber and once by the validation stream that fires shortly after. This task unifies the two border-coloring paths in WorkflowEditorComponent so a newly-added operator is validated and painted exactly once, while preserving the navigation-reset restore behavior from #3614.

### Context

`WorkflowEditorComponent.applyOperatorBorder` decides an operator's border color (invalid -> red, valid + cached run status -> state color, valid + no status -> default gray). It has two callers:

- `handleOperatorValidation`: the validation-stream subscriber, which already has the `Validation` result and passes it in.
- The operator-add stream subscriber: which has no `Validation` in hand and relies on the helper recomputing it.

These run off two separate event streams. As a result, when an operator is **newly added**, it ends up validated and its border painted **twice**:

1. The operator-add subscriber fires (`getOperatorAddStream`) -> paints the border using a freshly-computed validation.
2. A moment later, validation for the new operator runs via `getOperatorDynamicSchemaChangedStream` -> `operatorValidationStream` fires → `handleOperatorValidation` paints the border again.

Raised as a follow-up suggestion during review of the `applyOperatorBorder` optimization (PR #).

### Proposed change

Unify the two paths so a newly-added operator is validated and painted exactly once, ideally with both callers obtaining the `Validation` the same way, allowing the optional-parameter fallback in `applyOperatorBorder` to be removed.

In `frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts`.

### Considerations

- The operator-add subscriber was added for the navigation-reset fix (#3614): when the user navigates back to a workflow, `reloadWorkflow` re-adds every operator and the border (plus cached run status) must be restored immediately. Any unification must preserve that the border can't be left blank waiting on a later validation event.
- This is a non-functional cleanup; behavior should be unchanged. Existing tests in `workflow-editor.component.spec.ts` (the "operator border restoration after navigation" block) should continue to pass, with coverage added for the single-paint behavior.

### Related

- PR #5146 (introduced `applyOperatorBorder` and the #3614 navigation fix)
- PR #5702 (added the optional `Validation` parameter; this issue was raised in its review)

### Task Type

- [x] Refactor / Cleanup
- [ ] DevOps / Deployment / CI
- [ ] Testing / QA
- [ ] Documentation
- [ ] Performance
- [ ] Other

Contributor guide

Open the contributing guide

Research direction

Start in frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts and trace applyOperatorBorder, handleOperatorValidation, and the operator-add and validation stream subscribers. Then run the operator border restoration tests in workflow-editor.component.spec.ts. Done means a newly added operator is validated and painted once while navigation-reset border restoration still passes, with coverage for the single-paint behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.