The GitHub Actions job "Required Checks" on texera.git/gh-readonly-queue/main/pr-8600-e7d1676e1662879eb210a21acab8d30779b01024 has succeeded. Run started by GitHub user mengw15 (triggered by mengw15).
Head commit for run: 957b6c965640c07ace034eb9edcae53ef974324a / yangzhang75 <[email protected]> fix(workflow): stop the workspace destroying itself on beforeunload (#8600) ### What changes were proposed in this PR? Switching between the operator canvas and the Form View is a full-page navigation, and both pages ran their entire teardown from a `beforeunload` host binding: clear the graph, destroy the Yjs shared document, disconnect the computing unit, reset the execution state and the results. A browser does not always discard a document it navigates away from. Chrome may keep it in the back/forward cache, and going back restores the JavaScript state exactly as it was left, re-running nothing. What came back was the page these methods had already gutted: - an empty canvas that answered no clicks, because the graph had been cleared; - a workflow id reset to `DEFAULT_WORKFLOW`'s, which the share dialog then asked the backend about; - an autosave still subscribed to that reset metadata, which wrote the default out as a brand-new blank workflow, so the workflow list grew by one on every trip. There was never anything to tear down there. A document that really is discarded takes its websockets and its graph with it, and a document that comes back needs them. So `beforeunload` now only saves, and the teardown stays in `ngOnDestroy`, which runs when the page is genuinely replaced. Both views change the same way. Three more defects turned this into a silent failure, and all are fixed here: - `WorkflowResource.getWorkflowType` read `workflowDao.fetchOneByWid(wid)` and dereferenced it, so an id matching no row answered 500 with a stack trace rather than 404. - The share dialog's publish-state subscription had no error handler. A failed request left `isPublic` null, and the template hides the Private/Public choice on exactly that (`*ngIf="isPublic !== null"`), so the dialog looked complete while offering one control fewer and the only way to find out was the network tab. `ngOnInit` also doubles as a refresh after an access change, so a failed second read used to keep the value from the first: the buttons stayed on screen showing a state nothing had confirmed, while the toast said the choice was not shown. It is dropped now. - The Share button had no gate. `ngAfterViewInit` calls `resetAsNewWorkflow()`, so the metadata sits at `DEFAULT_WORKFLOW` (wid 0) on every canvas load and the real id only arrives with the workflow; the menu renders outside the loading spinner's container, so a click in that window opened a dialog that asked `GET /workflow/type/0` and came back without the Private/Public choice. The button is disabled until the id arrives and the handler refuses the same window. This is the second route to the symptom, and it is the gesture the issue reports. #### Before <!-- Drop the recording here: open a saved workflow, switch to the Form View, press the browser's Back button. The canvas comes back blank and unclickable, the share dialog has no Private/Public choice, and the workflow list has gained a blank workflow. --> #### After <!-- Drop the recording here: the same steps on this branch. The canvas comes back live, the share dialog keeps both buttons, and no blank workflow is created. --> **One behaviour changes deliberately.** The shared document is no longer destroyed on unload, so a co-editor is no longer told explicitly that you left; the room notices when the socket closes with the document. Destroying it on unload is what made a restored page unusable, and a restored page needs its room. ### Any related issues, documentation, discussions? Closes #8599. ### How was this PR tested? Reproduced first, on a local instance running plain `main` (`e7d1676e1`): open a saved workflow, switch to the Form View, press the browser's Back button, and the canvas comes back blank and unclickable, the share dialog has no Private/Public choice, and the workflow list has gained a blank workflow. With this branch deployed to the same instance, none of the three happens. The endpoint was checked against that instance directly: `GET /api/workflow/type/0` and `/type/999999` answered 500 before and answer 404 after. Unit tests: - `workspace.component.spec` and `workflow-form.component.spec`: `beforeunload` saves and tears nothing down; the existing tests that `ngOnDestroy` still tears everything down are unchanged and still pass. The workspace test dispatches a real `beforeunload` DOM event rather than calling the handler, so the host binding being removed or miswired is caught too; `workflow-form.rendered.spec` already did this for the form. - `menu.component.spec`: the share dialog is not opened for wid 0 or for no id at all, and the Share button is disabled until the id arrives. - `share-access.component.spec`: a failed publish-state request reports itself instead of hiding the choice silently, and a failed refresh drops the value the previous read left behind. - `WorkflowResourceCoverSpec`: `getWorkflowType` reports the publish state, and throws `NotFoundException` for an id that matches no workflow. Each new guard was deletion-checked: restoring the teardown on `beforeunload` in either view, dropping the error handler or the `isPublic` reset, removing the `[disabled]` on the Share button, or removing the handler's own guard each turns exactly the intended tests red. Removing the `@HostListener` itself now turns a test red, which it would not have before. Full frontend suite: 224 files, 6126 passed, 1 skipped (pre-existing), 0 failed. `WorkflowExecutionService/testOnly ... WorkflowResourceCoverSpec`: 16 passed. `ng build --configuration=production` (AOT), `eslint`, `prettier --check`, `scalafmtCheck` on main and test sources: all clean. **Deliberately out of scope.** Grepping for the same shape found two more pages that act on `beforeunload`, and neither is on the path this issue reports, so both are left alone and filed instead of widened into here: the Hub's workflow detail page clears the graph there, and `AgentPanelComponent` deactivates the current agent there. The app's other four `beforeunload` bindings only write panel geometry to `localStorage` and are unaffected. **The in-page button, the gesture the issue reports,** reaches the same symptom by the second route above rather than through the cache, which is why it did not reproduce for me on the Back button's steps: it needs the click to land before the workflow does. Found by @mengw15 in review. The Back-button path is the one I reproduced in a running instance; the button gate that closes this one is covered by tests rather than by a manual repro, since it is a race against the workflow fetch. ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Opus 5, Anthropic). Co-authored with Claude; the author reviewed the change line by line and reproduced both the failure and the fix in a running instance before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]> Report URL: https://github.com/apache/texera/actions/runs/35409391447 With regards, GitHub Actions via GitBox
