Yicong-Huang opened a new pull request, #4530:
URL: https://github.com/apache/texera/pull/4530

   ### What changes were proposed in this PR?
   
   Refactor 
`frontend/src/app/workspace/service/operator-menu/operator-menu.service.ts` to 
address two coupled code-quality issues:
   
   1. **Encapsulate state.** `highlightedOperators` and 
`highlightedCommentBoxes` are now private `BehaviorSubject`s, exposed only as 
readonly `Observable`s (`highlightedOperators$`, `highlightedCommentBoxes$`). 
External callers can no longer call `.next()` and corrupt service state.
   2. **Add subscription cleanup.** All five constructor subscriptions now use 
`untilDestroyed(this)` (consistent with the rest of the codebase). HMR no 
longer leaves ghost subscriptions on the old `JointGraphWrapper` / 
`TexeraGraph` instances.
   3. **Eliminate fan-out.** Previously, three downstream handlers 
(`handleDisableOperatorStatusChange`, `handleViewResultOperatorStatusChange`, 
`handleReuseOperatorResultStatusChange`) all subscribed to the public 
`highlightedOperators` BehaviorSubject. A single user highlight action 
triggered the upstream handler that called `.next()`, which then re-fired all 
three downstream handlers — a 4× cascade per highlight event. The refactored 
code uses one private `recomputeMenuState()` method invoked directly from the 
upstream handler, plus a single `merge(...)` over the texera-graph state-change 
streams. One highlight = one recompute.
   
   ### Consumer migration
   
   - `ContextMenuComponent`: subscribes to the two Observables in its 
constructor (with `untilDestroyed`) and stores `highlightedOperatorIds` / 
`highlightedCommentBoxIds` as local fields for the template.
   - `context-menu.component.html`: template `*ngIf` checks now reference the 
component's local fields instead of 
`operatorMenuService.highlightedOperators.value`.
   - `WorkflowEditorComponent` copy/cut handlers: use 
`withLatestFrom(operatorMenu.highlightedOperators$, 
operatorMenu.highlightedCommentBoxes$)` instead of reading `.value`.
   
   ### Any related issues, documentation, discussions?
   
   Closes #4529
   
   ### How was this PR tested?
   
   - Replaced the placeholder `OperatorMenuService` spec with 9 new tests 
covering: empty initial state, encapsulation (BehaviorSubjects no longer 
publicly accessible), highlight/unhighlight propagation to both observables, 
the no-fan-out invariant (a single highlight produces exactly one emission on 
`highlightedOperators$`), sink exclusion in view-result/reuse-result targets, 
and recomputation triggered by `getViewResultOperatorsChangedStream` and the 
workflow-modification-enabled stream.
   - Existing `ContextMenuComponent`, `WorkflowEditorComponent`, 
`PropertyEditorComponent`, `ResultPanelComponent`, and `JointGraphWrapper` 
specs continue to pass.
   - `yarn ng build` is clean (no new TS errors).
   - `yarn format:fix` reports no changes.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (claude-opus-4-7)


-- 
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