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]
