yangzhang75 commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3995169678
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -1301,40 +1545,47 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
* workflow when the payload has no id, so saving whatever the graph holds
would spawn
* stray "Untitled workflow" rows when the page is left before its workflow
loaded.
*/
- private save(): void {
+ private save(afterwards?: () => void): void {
// A read-only viewer can open and run the form (execution is gated on
computing-unit access,
// not workflow access) but must never persist: every such save is a
guaranteed 403 that would
// spam "Could not save" on each debounce. Their inputs are non-editable,
so nothing is lost.
+ // `afterwards` runs once the save has completed, or at once when there is
nothing to save;
+ // it does not run when the save fails, so a caller that navigates on it
stays put instead.
if (!this.canEdit) {
+ afterwards?.();
return;
}
if (!this.userService.isLogin() ||
!this.workflowPersistService.isWorkflowPersistEnabled()) {
+ afterwards?.();
return;
}
const workflow = this.workflowActionService.getWorkflow();
if (workflow.wid === undefined || workflow.wid !== this.wid) {
+ afterwards?.();
return;
}
+ // Snapshot now, not when the request's turn comes: on the way out
ngOnDestroy clears the graph
+ // right after asking for this save, and the queue may only reach it after
that.
const preserved: Workflow = {
...workflow,
content: { ...workflow.content, operatorPositions:
this.positionsToSave(workflow.content) },
};
- // On the way out the subscription must NOT be tied to this component:
ngOnDestroy
- // calls save(), and untilDestroyed would tear the subscription down as
part of the
- // very same destroy sequence, aborting the request that was the point of
the call.
- const persist = this.workflowPersistService.persistWorkflow(preserved);
- // The `destroyed` branch deliberately omits untilDestroyed (see above);
the persist call
- // is a one-shot HTTP request that completes on its own, so it needs no
teardown operator.
- // eslint-disable-next-line rxjs-angular/prefer-takeuntil
- (this.destroyed ? persist : persist.pipe(untilDestroyed(this))).subscribe({
- // Feed the saved workflow back, exactly as the operator canvas does:
this advances
- // lastModifiedTime (and the normalised name), and the metadata
subscription then repaints
- // the title bar -- so "Saved at ..." moves past the moment the page
opened.
- next: updatedWorkflow =>
this.workflowActionService.setWorkflowMetadata(updatedWorkflow),
- // A save that fails silently is the worst thing this page can do: the
author walks
- // away believing the form they just built is stored.
- error: () => this.notificationService.error("Could not save. Your latest
changes are not stored yet."),
- });
+ this.persistQueue.next({ workflow: preserved, afterwards });
+ }
+
+ /** One persist of the given snapshot, reporting its outcome; the queue
orders them. */
+ private persistNow(preserved: Workflow): Observable<Workflow> {
+ return this.workflowPersistService.persistWorkflow(preserved).pipe(
+ tap({
+ // Feed the saved workflow back, exactly as the operator canvas does:
this advances
+ // lastModifiedTime (and the normalised name), and the metadata
subscription then repaints
+ // the title bar -- so "Saved at ..." moves past the moment the page
opened.
+ next: updatedWorkflow =>
this.workflowActionService.setWorkflowMetadata(updatedWorkflow),
Review Comment:
Right on both counts. c77e15748: the response feeds back the server-owned
metadata (the timestamp, and the normalised name when nothing changed), but if
the name differs from the snapshot that was sent (a rename since, whose own
save is already queued) the current name is kept; and once the page is gone
nothing is fed back, so the cleared singleton is not refilled. Specs: an older
save's response does not undo a rename made meanwhile; no metadata repaint
after destroy. Deleting either guard turns its spec red.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -316,7 +346,11 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
.getViewResultOperatorsChangedStream()
.pipe(untilDestroyed(this))
.subscribe(() => {
+ // An eye toggled on the canvas changes both what shows (a
newly-viewed step) and what the
+ // author can pick, so rebuild the picker here too -- otherwise a
just-eyed step would not
+ // appear as an option (and an un-eyed one would linger) until the
next full re-read.
this.refreshShownResults();
+ this.rebuildResultChoices();
Review Comment:
Agreed. Compilation does re-read the config after those edits (it merges the
same add/delete/link/disabled streams), but debounced and held while the reader
is typing, so the card and the pill could lag. c77e15748 subscribes to the
graph's operator add/delete, link add/delete and disabled-changed streams
directly and refreshes the two derived collections at once (nothing rebuilds
the inputs). Spec: a shown final step disabled by a co-editor drops from the
cards and the picker on that event alone; deleting the subscription 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]
