yangzhang75 commented on code in PR #8351:
URL: https://github.com/apache/texera/pull/8351#discussion_r4030024783
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts:
##########
@@ -747,25 +770,49 @@ export class WorkflowActionService {
}
public setWorkflowSettings(workflowSettings: WorkflowSettings | undefined):
void {
- if (this.workflowSettings === workflowSettings) {
+ const newSettings = workflowSettings === undefined ?
this.getDefaultSettings() : workflowSettings;
+ // Skip a redundant write: setting the same value would still cut a Yjs
update and observer
+ // churn for every collaborator.
+ if (isEqual(this.getWorkflowSettings(), newSettings)) {
return;
}
-
- const newSettings = workflowSettings === undefined ?
this.getDefaultSettings() : workflowSettings;
- this.workflowSettings = newSettings;
+ this.texeraGraph.sharedModel.contentMetaMap.set("settings", newSettings);
}
public getWorkflowSettings(): WorkflowSettings {
- return this.workflowSettings;
+ return (
+ (this.texeraGraph.sharedModel.contentMetaMap.get("settings") as
WorkflowSettings) ?? this.getDefaultSettings()
+ );
}
/**
- * Load a definition without announcing an edit. Used while opening a
workflow, so
- * that merely reading one does not look like a change and trigger a save.
+ * Seed workflowSettings into the shared model while opening a workflow,
mirroring
+ * hydrateFormBinding: `undefined` deletes the key rather than writing
defaults, so opening a
+ * workflow that never saved settings does not overwrite a co-editor's live
ones (getWorkflow-
+ * Settings defaults on an absent key). Called under the reloading flag.
+ */
+ public hydrateSettings(workflowSettings: WorkflowSettings | undefined): void
{
+ const contentMeta = this.texeraGraph.sharedModel.contentMetaMap;
+ if (workflowSettings === undefined) {
+ contentMeta.delete("settings");
+ } else {
+ contentMeta.set("settings", workflowSettings);
Review Comment:
Fixed. The seed now waits for the provider's first sync (immediate when the
document is offline or already synced) and writes only when the key is still
absent, and a value that arrived from the room is left alone -- neither
overwritten nor deleted -- which also covers opening a workflow that carries
none over a co-editor who has one. Meanwhile the database copy is held and read
from, so the page shows it and an autosave in that window carries it instead of
an empty one; the wait is bounded (5s) so an unreachable y-websocket still ends
up with the value. The deferred write is not announced as an edit. Tests: a
co-editor's value survives both a seed and an empty open, reads fall back while
waiting, the seed lands after the timeout, and the deferred seed stays silent;
each is deletion-checked.
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts:
##########
@@ -136,12 +137,34 @@ 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!);
+ const contentMetaMap = this.texeraGraph.sharedModel.contentMetaMap;
+ this.contentMetaObserver = event => {
+ if (event.changes.keys.has("formBinding") &&
!this.jointGraphWrapper.getReloadingWorkflow()) {
+ this.formBindingChangeSubject.next(this.getFormBinding());
+ }
Review Comment:
Fixed. The observer now publishes settings changes too, on a new
`workflowSettingsChanged$` that is merged into `workflowChanged()`, so the
settings panel (which refreshes from that stream) picks up a co-editor's change
and this client's autosave carries it. Covered by a test that writes to the
shared map and asserts both streams fire.
--
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]
