Copilot commented on code in PR #8351:
URL: https://github.com/apache/texera/pull/8351#discussion_r4127082541
##########
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:
When a form-binding edit is queued before the first sync, `writeContentMeta`
has already announced it. If the room's initial value then arrives, this branch
calls `getFormBinding()`, which intentionally returns that pending edit, so the
same edit is announced again. By then `WorkflowFormComponent.reflectingLocally`
is false, causing an unnecessary form rebuild/defer and another autosave.
Suppress map notifications while a pending edit owns this key; the queued edit
is already announced and its eventual landing is deliberately silent.
This issue also appears on line 208 of the same file.
##########
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:
A local settings edit now reaches both persistence paths:
`SettingsComponent` still calls `persistWorkflow()` immediately
(`settings.component.ts:88-109`), while this new stream causes Workspace's
debounced autosave to call it again. The backend inserts a workflow version on
every persist (`WorkflowResource.scala:461-480`), so each settings change
creates duplicate requests/versions and hits the endpoint's documented
parallel-persist race. Remove the settings panel's obsolete direct persistence
(and update its tests) so this shared stream is the single save path.
--
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]
