Copilot commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3996539836
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -1278,18 +1530,66 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
* of yourself, broken runs. A fresh document is the reliable handover.
*/
public openRegularCanvas(): void {
- this.save();
- /* v8 ignore start -- full-document navigation; jsdom cannot navigate */
+ // Save first and hand over only once the save has completed: the
full-page load unloads this
+ // document, and a request still in flight at that moment is aborted, so
navigating right after
+ // firing the save could lose the very edit the switch is meant to carry
across. A save that
+ // fails keeps the author here with the error shown, rather than leaving
with changes that were
+ // never stored. A reader, who has nothing to save, goes straight over.
+ this.save(() => this.openCanvasPage());
+ }
+
+ /**
+ * The full-page handover to the operator canvas, apart from the save so the
order is testable.
+ * Excluded from coverage as a whole: jsdom cannot navigate, so the specs
stub this method and
+ * assert when it is called rather than what it does.
+ */
+ /* v8 ignore start */
+ private openCanvasPage(): void {
window.location.href = `${USER_WORKSPACE}/${this.wid}`;
- /* v8 ignore stop */
}
+ /* v8 ignore stop */
/**
* Save the same way the operator canvas does. Both views edit one workflow,
so the
* form has to write through the same debounced persist -- otherwise an
author's
* setup, or a value someone filled in, would be gone on the next visit.
+ *
+ * Saves go out one at a time, in order (persistQueue). Two persists in
flight at once can reach
+ * the backend out of order, and then the older content wins; with the
queue, the save behind the
+ * Canvas switch is sent only after an autosave already on its way has
completed, and the last
+ * one enqueued carries the latest content. The drain is deliberately NOT
tied to this component's
+ * lifetime: the final save on the way out (ngOnDestroy) has to line up
behind an autosave still in
+ * flight too, or the older snapshot could commit after it. ngOnDestroy
completes the queue, so the
+ * drain ends by itself once the last save has gone out; every request is a
one-shot HTTP call.
*/
private registerAutoPersist(): void {
+ this.persistQueue
+ .pipe(
+ concatMap(workflow =>
+ this.persistNow(workflow).pipe(
+ // A failed save has reported itself; it must not let anyone
navigate away from changes
+ // that were never stored, so whatever was waiting for the drain
is dropped.
+ tap({ error: () => (this.afterDrain = []) }),
+ // A failed save must not take the queue down with it: the next
one still goes out.
+ catchError(() => EMPTY),
+ // Complete or failed, this save is done. Once nothing is left in
the queue, run what was
+ // waiting for the drain: by then every save asked for so far,
this one and any enqueued
+ // behind it while it was in flight, has gone out and come back.
+ finalize(() => {
+ this.queuedSaves--;
+ if (this.queuedSaves === 0) {
+ const waiting = this.afterDrain;
+ this.afterDrain = [];
+ waiting.forEach(run => run());
Review Comment:
The drain can navigate while a newer edit is still waiting in the 5-second
`workflowChanged()` debounce. If the user edits after clicking Canvas and the
switch save completes before that debounce fires, `queuedSaves` reaches zero
here, the full-page load cancels the pending autosave, and the post-click edit
was not in the switch snapshot. Track dirty changes immediately and
enqueue/flush the latest snapshot before running `afterDrain` (or make the page
non-editable as soon as handover starts).
--
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]
