yangzhang75 commented on code in PR #8351:
URL: https://github.com/apache/texera/pull/8351#discussion_r4127219671
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts:
##########
@@ -136,12 +158,61 @@ export class WorkflowActionService {
);
this.sharedModelChangeHandler.setConfigService(this.config);
this.workflowMetadata = DEFAULT_WORKFLOW;
- this.workflowSettings = this.getDefaultSettings();
this.undoRedoService.setUndoManager(this.texeraGraph.sharedModel.undoManager);
+ // Watch the shared content map, re-attaching whenever the shared model is
recreated
+ // (opening another workflow), the same way SharedModelChangeHandler
re-attaches its
+ // graph observers. A formBinding change from a local edit or a co-editor
is republished
+ // on formBindingChanged$ so the Form View re-renders and the existing
autosave picks it
+ // up. The reload seed is skipped -- like the graph seed -- so opening a
workflow is not
+ // announced as an edit and does not save on every open.
+ this.observeContentMeta();
+ this.texeraGraph.newYDocLoadedSubject.subscribe(() =>
this.observeContentMeta());
+
this.handleJointElementDrag();
}
+ private observeContentMeta(): void {
+ // Detach the observer from the previous shared model before attaching to
the new one, so
+ // re-attaching on each opened workflow does not stack listeners on the
same map.
+ this.observedContentMetaMap?.unobserve(this.contentMetaObserver!);
+ // A new document starts with no seed state: nothing in it came from the
previous room, no key
+ // has been seeded on it yet, and whatever copy the previously open
workflow left waiting is
+ // not this one's -- dropped here, so it cannot leak into the next save of
a workflow opened
+ // (or a blank one started) on this document.
+ for (const key of [...this.contentMetaSeeds.keys()]) {
+ this.cancelContentMetaSeed(key); // drops the copy it holds, and stops
its listening
+ }
+ this.roomOwnedContentMeta.clear();
+ this.seededContentMetaKeys.clear();
+ const contentMetaMap = this.texeraGraph.sharedModel.contentMetaMap;
+ this.contentMetaObserver = event => {
+ if (!event.transaction.local) {
+ // An entry that arrived from the room belongs to the room, a
co-editor's value or its
+ // cleared mark alike: the seed a workflow is opened with yields to it
(see seedContentMeta).
+ // The first sync delivers the room's state as a remote transaction,
which is how an
+ // opening client learns what the room already holds.
+ for (const key of event.changes.keys.keys()) {
+ this.roomOwnedContentMeta.add(key as ContentMetaKey);
+ }
+ }
+ if (this.jointGraphWrapper.getReloadingWorkflow()) {
+ return;
+ }
+ if (event.changes.keys.has("formBinding") &&
!this.seedingContentMetaKeys.has("formBinding")) {
+ this.formBindingChangeSubject.next(this.getFormBinding());
+ }
Review Comment:
Right, the room's value arriving re-announced the held edit. Fixed in
7255e791d: a change to a key is announced only when it is neither a seed's own
write nor made while an edit held for the first sync owns the key
(`announces`); that edit was announced when it was made, reads answer it, and
its landing is a seed's write. Spec: the held edit is announced exactly once
across the room's value arriving and the landing.
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts:
##########
@@ -774,6 +845,7 @@ export class WorkflowActionService {
this.getTexeraGraph().getPortDisplayNameChangedSubject(),
this.getTexeraGraph().getPortPropertyChangedStream(),
this.formBindingChanged$,
+ this.workflowSettingsChanged$,
Review Comment:
Agreed, and taken the other way round in 7255e791d: a settings change stays
on `workflowSettingsChanged$` and is no longer merged into `workflowChanged()`,
so the panel's own persist remains the single save path for settings, with its
error notification, and the autosave does not cut a second version. The
settings panel refreshes from `workflowSettingsChanged$` alongside
`workflowChanged()`, so a co-editor's change still reaches it live; any later
autosave carries the current settings anyway, since the content reads them from
the shared map. (Removing the panel's persist instead would have left a failed
settings save silent, as the autosave has no error path.) Spec: a shared-map
settings change is announced on `workflowSettingsChanged$` and not on
`workflowChanged()`; the panel re-syncs from the settings stream without
persisting.
--
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]
