yangzhang75 opened a new pull request, #8600:
URL: https://github.com/apache/texera/pull/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.
   
   Two more defects turned this into a silent failure, and both 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.
   
   **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.
   - `workflow-form.rendered.spec`: the existing test that dispatches a real 
`beforeunload` DOM event now asserts the new contract, so the host binding 
being removed or miswired is still caught.
   - `share-access.component.spec`: a failed publish-state request reports 
itself instead of hiding the choice silently.
   - `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, or dropping the error handler, turns exactly one 
named test red.
   
   Full frontend suite: 224 files, 6129 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.
   
   **Not covered:** the issue reports reaching this by switching with the 
in-page button rather than the Back button. That path is a normal navigation, 
which does not use the back/forward cache, and it does not reproduce for me on 
plain `main`. If it turns out to have a separate cause, the two fixes above 
mean it will now announce itself rather than silently dropping a control.
   
   ### 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
   


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