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

   ### What changes were proposed in this PR?
   
   **Root cause.** Two items in the operator context menu 
(`context-menu.component.html`) grey themselves out with `nzDisabled` but keep 
a `(click)` binding on the same `<li nz-menu-item>`. In ng-zorro-antd 21.3.2, 
`nzDisabled` does two things only: it adds the 
`ant-dropdown-menu-item-disabled` class, and the component's own host listener 
`clickMenuItem($event)` calls `preventDefault()` and `stopPropagation()` and 
returns. The template `(click)` is a second listener on the same element, so 
stopping propagation does not reach it, and the disabled styling puts 
`pointer-events: none` only on a nested `<a>`, which these items do not have. 
The item looks disabled and still runs its action.
   
   - "execute to this operator": `[nzDisabled]="!canExecuteOperator()"` with 
`(click)="operatorMenuService.executeUpToOperator()"`. Clicking it while greyed 
out, for example during a run, sends a new execution request. The backend then 
shuts down the in-flight execution and clears its results 
(`WorkflowService.initExecutionService`), and that execution's row stays 
RUNNING because nothing writes its final status.
   - "reuse result": hardcoded `[nzDisabled]="true"` with 
`(click)="operatorMenuService.reuseResultHighlightedOperator()"`, so it is 
always clickable through a greyed row.
   
   No other item in this template combines `nzDisabled` with `(click)`.
   
   ```
   Before:  greyed "execute to this operator" clicked mid-run -> 
executeUpToOperator() -> new execution; the running one is shut down, its 
results cleared, its row left RUNNING
   After:   greyed "execute to this operator" clicked -> 
onExecuteUpToOperator() returns early; nothing is sent
   ```
   
   Reproduced on adc6a4ac4 with a local computing unit: workflow Text Input -> 
Python UDF that sleeps -> Python Lambda; Run; right-click Python Lambda while 
running; "execute to this operator" is greyed; click it.
   
   <img width="1568" height="287" 
alt="01-greyed-execute-to-operator-during-run" 
src="https://github.com/user-attachments/assets/aa0193fb-3df8-4fc4-b6e7-588ac8b62c8c";
 />
   
   | eid | status | starting_time | note |
   | --- | --- | --- | --- |
   | 9 | 1 (RUNNING) | 19:01:56 | started by Run; shut down by the second 
request, row never updated |
   | 10 | 1 (RUNNING) | 19:02:30 | started by the click on the greyed item |
   
   With this branch, the same steps: the item is still greyed, the click does 
nothing, the run started by Run keeps going (the UDF's output count keeps 
climbing, the clock keeps counting), and `workflow_executions` holds only that 
one new row.
   
   | After: running, item greyed | After: greyed item clicked, same run 
continues |
   | --- | --- |
   | 
   <img width="1312" height="773" alt="02-after-fix-running-greyed-item" 
src="https://github.com/user-attachments/assets/3a37c210-c764-4360-80c9-08512928ad7b";
 /> | 
   <img width="1312" height="773" 
alt="03-after-fix-greyed-item-clicked-same-run-continues" 
src="https://github.com/user-attachments/assets/660049f0-8925-40a8-b83c-50d314a96ea2";
 /> |
   
   **Changes.**
   
   1. `context-menu.component.ts`: new `onExecuteUpToOperator()` that returns 
early unless `canExecuteOperator()` holds and otherwise calls 
`operatorMenuService.executeUpToOperator()`, in the same style as 
`onCopy`/`onPaste`/`onCut`/`onDelete`. The template binds 
`(click)="onExecuteUpToOperator()"`.
   2. `context-menu.component.html`: the `(click)` on "reuse result" is 
removed. The item is hardcoded `nzDisabled`, so there is no enabled state in 
which that handler could be meant to run; "remove reusing result" keeps its own 
binding to the same service method. The one behaviour this removes is the 
unintended one: on main a click on the greyed "reuse result" row still called 
`reuseResultHighlightedOperator()`, which marked the highlighted operators for 
result reuse. The guard-in-handler shape follows the existing precedent in 
`computing-unit-selection.component.html`, whose 
`[nzDisabled]="cannotSelectUnit(unit)"` row routes its `(click)` through 
`onClickComputingUnitRow`, which returns early on the same condition.
   3. `context-menu.component.spec.ts`: `clickItem` now dispatches a real DOM 
`MouseEvent("click")` instead of `triggerEventHandler("click", null)`, so the 
nz-menu-item host listener and the template binding both run as in the browser 
(with `null` the host listener of a disabled item would throw on 
`preventDefault`). The old "renders the reuse result item (disabled)" test, 
whose comment claimed the item "can't be clicked", is replaced by a click test. 
A small `isItemDisabled(label)` helper reads `nzDisabled` from the rendered 
`NzMenuItemComponent`.
   
   ### Any related issues, documentation, discussions?
   
   Closes #8966
   
   ### How was this PR tested?
   
   New tests in 
`frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts`:
   
   - execute to this operator does nothing while the item is greyed out 
(operator invalid)
   - execute to this operator does nothing while the workflow is not modifiable
   - execute to this operator does nothing while the operator is disabled
   - execute to this operator is not offered while more than one operator is 
highlighted
   - execute to this operator checks the state at click time, not at the last 
render: the item is enabled and clicked once, then the workflow locks without a 
change-detection pass so the row still looks enabled; a second click does 
nothing
   - reuse result renders greyed out and does nothing when clicked
   
   The existing "execute to this operator invokes executeUpToOperator" test 
still passes through the real click, so the enabled path is unchanged.
   
   ```
   cd frontend
   NODE_OPTIONS="--no-experimental-webstorage" npx ng test --watch=false 
--include 
src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.spec.ts
   # 50 passed (50)
   NODE_OPTIONS="--no-experimental-webstorage" npx ng test --watch=false
   # 235 files, 6617 passed | 1 skipped
   npx tsc --noEmit -p tsconfig.json               # clean
   npx eslint <changed files>                      # clean
   npx prettier --check <changed files>            # clean
   ```
   
   Mutation check: removing the guard in `onExecuteUpToOperator()` fails the 
three "does nothing" tests and the click-time test; restoring the `(click)` on 
"reuse result" fails its test.
   
   The `NODE_OPTIONS` setting is only needed locally on Node 25, whose built-in 
`localStorage` shadows jsdom's. CI runs Node 24 and needs no flag.
   
   Manual check in Chrome against a build of this branch on the local stack 
with a local computing unit:
   
   1. Workflow: Text Input -> Python UDF whose `process_tuple` yields the input 
once per second for 900 seconds -> Python Lambda Function.
   2. Click Run and wait until the UDF is producing output.
   3. Right-click the Python Lambda Function: "execute to this operator" is 
greyed out. Click it.
   4. The run keeps going with the same execution id, the output count keeps 
climbing, and no second `workflow_executions` row appears (screenshots above). 
On main the same click started a second execution and shut down the first.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Yes. Generated-by: Claude Code (claude-fable-5-1). Co-authored with Claude; 
the author reviewed the change before submission.


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