diff --git a/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts b/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts index f9546a64a89..3837de52493 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/shared-model.ts @@ -45,6 +45,11 @@ export class SharedModel { public operatorLinkMap: Y.Map; public elementPositionMap: Y.Map; public debugState: Y.Map>; + // Non-graph workflow content that used to live as a per-client private field and so was + // silently overwritten by another editor's whole-content autosave (workflowSettings, and + // the Form View definition). Kept in the shared doc, keyed "settings"/"formBinding", so it + // syncs live like the graph and every autosave writes the current value, not a stale copy. + public contentMetaMap: Y.Map; public undoManager: Y.UndoManager; public clientId: string; @@ -68,6 +73,7 @@ export class SharedModel { this.commentBoxMap = this.yDoc.getMap("commentBoxMap"); this.operatorLinkMap = this.yDoc.getMap("operatorLinkMap"); this.elementPositionMap = this.yDoc.getMap("elementPositionMap"); + this.contentMetaMap = this.yDoc.getMap("contentMeta"); // Initialize Y-undo manager by aggregating intended Y-structures. Only structures included here will be undoable. this.undoManager = new Y.UndoManager( diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts index 52908fdb368..4e905bc3f5f 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts @@ -891,12 +891,41 @@ describe("WorkflowActionService", () => { sub.unsubscribe(); }); - // Opening a workflow is not an edit; announcing it would save on every open. + // The definition lives in the shared model (#8315), so a co-editor's change -- a write to + // the shared map from a remote transaction -- is picked up locally and re-rendered, and + // this client's next autosave carries the current value instead of a stale private copy. + it("should pick up a co-editor's change from the shared model", () => { + const seen: unknown[] = []; + const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); + + texeraGraph.sharedModel.contentMetaMap.set("formBinding", config); + + expect(seen).toEqual([config]); + expect(service.getFormBinding()).toEqual(config); + sub.unsubscribe(); + }); + + // Opening a workflow is not an edit; announcing it would save on every open. The seed + // runs under the reloading flag, so the shared-map observer skips it. it("should stay silent while a workflow is being opened", () => { const seen: unknown[] = []; const sub = service.formBindingChanged$.subscribe(v => seen.push(v)); - service.hydrateFormBinding(config); + service.reloadWorkflow( + { + ...DEFAULT_WORKFLOW, + content: { + operators: [mockScanPredicate], + operatorPositions: { [mockScanPredicate.operatorID]: mockPoint }, + links: [], + commentBoxes: [], + settings: undefined as any, + formBinding: config, + }, + }, + false, + false + ); expect(seen.length).toEqual(0); expect(service.getFormBinding()).toEqual(config); diff --git a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts index f5313365936..135cdbb5e1b 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts @@ -102,17 +102,12 @@ export class WorkflowActionService { private resultPanelOpenSubject = new Subject(); public readonly resultPanelOpen$: Observable = this.resultPanelOpenSubject.asObservable(); - private workflowSettings: WorkflowSettings; private workflowResetSubject = new Subject(); - // The Form View definition. Presentation, not structure, so it stays out of the shared - // graph (no collaborative merge) and is handled like workflowSettings -- hydrated by - // reloadWorkflow, emitted by getWorkflowContent. - private formBinding: FormBindingConfig = getDefaultFormBinding(); - // Whether the opened workflow's content carried a formBinding. Kept so getWorkflowContent - // re-emits the key only when it was there (or an author has since populated it), leaving a - // plain workflow's content byte-identical -- the same rule agent-service follows. - private formBindingLoaded = false; + // The Form View definition. Presentation, not structure, but it still lives in the shared + // doc (shared-model contentMetaMap, key "formBinding") so a co-editor sees it live and no + // collaborator's whole-content autosave overwrites it with a stale copy. workflowSettings + // is kept there too (key "settings") for the same reason. private formBindingChangeSubject = new Subject(); public readonly formBindingChanged$: Observable = this.formBindingChangeSubject.asObservable(); @@ -136,12 +131,28 @@ 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 { + this.texeraGraph.sharedModel.contentMetaMap.observe(event => { + if (event.changes.keys.has("formBinding") && !this.jointGraphWrapper.getReloadingWorkflow()) { + this.formBindingChangeSubject.next(this.getFormBinding()); + } + }); + } + private getDefaultSettings(): WorkflowSettings { return { dataTransferBatchSize: this.config.env.defaultDataTransferBatchSize, @@ -667,7 +678,7 @@ export class WorkflowActionService { } const workflowContent: WorkflowContent = workflow.content; - this.workflowSettings = workflowContent.settings || this.getDefaultSettings(); + this.setWorkflowSettings(workflowContent.settings); this.hydrateFormBinding(workflowContent.formBinding); let operatorsAndPositions: { op: OperatorPredicate; pos: Point }[] = []; @@ -747,25 +758,29 @@ export class WorkflowActionService { } public setWorkflowSettings(workflowSettings: WorkflowSettings | undefined): void { - if (this.workflowSettings === workflowSettings) { - 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. + * Load a definition into the shared model while opening a workflow. `undefined` clears the + * key so a plain workflow carries none and the one left by a previously open workflow cannot + * leak in. Called under the reloading flag, so the shared-map observer skips this seed and + * opening a workflow is not announced as an edit. */ public hydrateFormBinding(formBinding: FormBindingConfig | undefined): void { - this.formBindingLoaded = formBinding !== undefined; - this.formBinding = formBinding ?? getDefaultFormBinding(); + const contentMeta = this.texeraGraph.sharedModel.contentMetaMap; + if (formBinding === undefined) { + contentMeta.delete("formBinding"); + } else { + contentMeta.set("formBinding", formBinding); + } } /** A form binding worth persisting: an author populated it (fields, results, or an @@ -775,16 +790,17 @@ export class WorkflowActionService { } /** - * Replace the definition as an edit: announced on `formBindingChanged$`, which - * feeds workflowChanged() and so reaches the existing autosave. + * Replace the definition as an edit. The shared map's observer republishes it on + * `formBindingChanged$`, which feeds workflowChanged() and so reaches the existing autosave. */ public setFormBinding(formBinding: FormBindingConfig): void { - this.formBinding = formBinding; - this.formBindingChangeSubject.next(this.formBinding); + this.texeraGraph.sharedModel.contentMetaMap.set("formBinding", formBinding); } public getFormBinding(): FormBindingConfig { - return this.formBinding; + return ( + (this.texeraGraph.sharedModel.contentMetaMap.get("formBinding") as FormBindingConfig) ?? getDefaultFormBinding() + ); } public getWorkflowMetadata(): WorkflowMetadata { @@ -798,7 +814,7 @@ export class WorkflowActionService { const links = texeraGraph.getAllLinks(); const operatorPositions: { [key: string]: Point } = {}; const commentBoxes = texeraGraph.getAllCommentBoxes(); - const settings = this.workflowSettings; + const settings = this.getWorkflowSettings(); texeraGraph .getAllOperators() @@ -814,11 +830,13 @@ export class WorkflowActionService { links, commentBoxes, settings, - // Carry formBinding only when the workflow has one (loaded with it, or an author - // populated it), so a plain workflow's content is unchanged and its save cuts no - // needless version. - ...(this.formBindingLoaded || this.isFormBindingNonEmpty(this.formBinding) - ? { formBinding: this.formBinding } + // Carry formBinding only when the workflow has one (opened with it, or an author + // populated it since), so a plain workflow's content is unchanged and its save cuts no + // needless version. `has` stands in for the old "loaded" flag: hydrate sets the key for + // a workflow opened with a binding and deletes it for one without. + ...(this.texeraGraph.sharedModel.contentMetaMap.has("formBinding") || + this.isFormBindingNonEmpty(this.getFormBinding()) + ? { formBinding: this.getFormBinding() } : {}), }; } @@ -864,12 +882,12 @@ export class WorkflowActionService { public setWorkflowDataTransferBatchSize(size: number): void { if (size > 0 && size != null) { - this.setWorkflowSettings({ ...this.workflowSettings, dataTransferBatchSize: size }); + this.setWorkflowSettings({ ...this.getWorkflowSettings(), dataTransferBatchSize: size }); } } public updateExecutionMode(mode: ExecutionMode): void { - this.setWorkflowSettings({ ...this.workflowSettings, executionMode: mode }); + this.setWorkflowSettings({ ...this.getWorkflowSettings(), executionMode: mode }); } public clearWorkflow(): void {