PG1204 opened a new pull request, #6075:
URL: https://github.com/apache/texera/pull/6075

   ### What changes were proposed in this PR?
   
   Follow-up to #5702. `WorkflowEditorComponent.applyOperatorBorder` decides an 
operator's border color and has two callers:
   
   - `handleOperatorValidation`: the validation-stream subscriber, which 
already had the `Validation` and passed it in.
   - The operator-add stream subscriber: which passed no `Validation` and 
relied on an optional-parameter fallback that recomputed it inside the helper.
   
   This PR unifies the two paths so both callers obtain the `Validation` 
themselves and pass it in:
   
   - The operator-add subscriber now computes it via 
`validationWorkflowService.validateOperator(...)` and passes it, mirroring the 
validation-stream caller.
   - `applyOperatorBorder`'s `validation` parameter becomes **required**, and 
the `?? validateOperator(...)` fallback is removed. The color decision no 
longer silently depends on a recompute hidden inside the helper.
   
   This is a non-functional cleanup and no behavioral change. In particular, 
the operator-add subscriber still paints the border immediately (and restores 
cached run statistics), preserving the navigation-reset behavior from #3614.
   
   Note on scope: the issue's summary mentions "painted exactly once." Truly 
collapsing to a single paint conflicts with the #3614 requirement that borders 
render immediately on reload without waiting for validation events, and would 
need fragile "just-added" state tracking in the validation handler. This PR 
implements the safe, behavior-preserving unification (consistent validation 
acquisition, fallback removed) rather than suppressing either paint.
   
   ### Any related issues, documentation, discussions?
   
   Closes #5726. 
   Related: #5702 (added the optional parameter; this issue arose from its 
review), #5146 (introduced `applyOperatorBorder` and the #3614 navigation fix).
   
   ### How was this PR tested?
   
   Updated `workflow-editor.component.spec.ts`:
   - Renamed the "uses the Validation passed in instead of recomputing it" test 
to "relies solely on the passed-in Validation (never recomputes inside the 
helper)": the old name implied a recompute path that no longer exists.
   - Added "supplies a computed Validation from the operator-add path", 
asserting the operator-add subscriber calls `applyOperatorBorder` with a 
`Validation` object.
   - Existing border-outcome tests (green / gray / red / invalid-over-cached 
priority) remain and continue to pass.
   
   Verified locally:
   - `tsc --noEmit`: clean
   - `prettier --check` / `eslint`: clean
   - `ng test` (jsdom): 30/30 pass in the editor spec
   - `ng run gui:test-browser`: 13/13 pass
   
   ### Was this PR authored or co-authored using generative AI tooling?
   Co-authored with Claude Opus 4.8 in compliance with ASF


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to