mengw15 commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3996589016
##########
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:
Right — the queue only counts enqueued saves, so an edit still sitting in
the autosave debounce when the drain reached zero was lost to the full-page
load. Fixed in d5bf9af59: workflowChanged marks the page dirty before the
debounce, an enqueued snapshot clears the mark, and the drain flushes one more
save instead of handing over while the mark is set. Spec: an edit made after
the switch click, still in the debounce, goes out before navigation; disabling
the flush turns it red.
--
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]
