diff --git a/core/gui/src/app/workspace/component/menu/menu.component.html b/core/gui/src/app/workspace/component/menu/menu.component.html
index d75d918d43e..fc691f18b73 100644
--- a/core/gui/src/app/workspace/component/menu/menu.component.html
+++ b/core/gui/src/app/workspace/component/menu/menu.component.html
@@ -276,22 +276,22 @@
nzType="database"
nzTheme="twotone">
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
diff --git a/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts b/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts
index 40574b0f7ef..161d217fd86 100644
--- a/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts
+++ b/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts
@@ -897,56 +897,54 @@ describe("WorkflowEditorComponent", () => {
expect(jointGraphWrapper.getCurrentHighlightedOperatorIDs()).toContain(mockResultPredicate.operatorID);
});
- // Temporarily disabling undo-redo because of a bug that can cause invalid workflow structures.
- // TODO: enable after fixing the bug.
- // //undo
- // it("should undo action when user presses command + Z or control + Z", () => {
- // spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
- // spyOn(undoRedoService, "canUndo").and.returnValue(true);
- // let undoSpy = spyOn(undoRedoService, "undoAction");
- // fixture.detectChanges();
- // const commandZEvent = new KeyboardEvent("keydown", { key: "Z", metaKey: true, shiftKey: false });
- // (document.activeElement as HTMLElement)?.blur();
- // document.dispatchEvent(commandZEvent);
- // fixture.detectChanges();
- // expect(undoSpy).toHaveBeenCalledTimes(1);
- //
- // const controlZEvent = new KeyboardEvent("keydown", { key: "Z", ctrlKey: true, shiftKey: false });
- // (document.activeElement as HTMLElement)?.blur();
- // document.dispatchEvent(controlZEvent);
- // fixture.detectChanges();
- // expect(undoSpy).toHaveBeenCalledTimes(2);
- // });
- //
- // //redo
- // it("should redo action when user presses command/control + Y or command/control + shift + Z", () => {
- // spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
- // spyOn(undoRedoService, "canRedo").and.returnValue(true);
- // let redoSpy = spyOn(undoRedoService, "redoAction");
- // fixture.detectChanges();
- // const commandYEvent = new KeyboardEvent("keydown", { key: "y", metaKey: true, shiftKey: false });
- // (document.activeElement as HTMLElement)?.blur();
- // document.dispatchEvent(commandYEvent);
- // fixture.detectChanges();
- // expect(redoSpy).toHaveBeenCalledTimes(1);
- //
- // const controlYEvent = new KeyboardEvent("keydown", { key: "y", ctrlKey: true, shiftKey: false });
- // (document.activeElement as HTMLElement)?.blur();
- // document.dispatchEvent(controlYEvent);
- // fixture.detectChanges();
- // expect(redoSpy).toHaveBeenCalledTimes(2);
- //
- // const commandShitZEvent = new KeyboardEvent("keydown", { key: "z", metaKey: true, shiftKey: true });
- // (document.activeElement as HTMLElement)?.blur();
- // document.dispatchEvent(commandShitZEvent);
- // fixture.detectChanges();
- // expect(redoSpy).toHaveBeenCalledTimes(3);
- //
- // const controlShitZEvent = new KeyboardEvent("keydown", { key: "z", ctrlKey: true, shiftKey: true });
- // (document.activeElement as HTMLElement)?.blur();
- // document.dispatchEvent(controlShitZEvent);
- // fixture.detectChanges();
- // expect(redoSpy).toHaveBeenCalledTimes(4);
- // });
+ //undo
+ it("should undo action when user presses command + Z or control + Z", () => {
+ spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
+ spyOn(undoRedoService, "canUndo").and.returnValue(true);
+ let undoSpy = spyOn(undoRedoService, "undoAction");
+ fixture.detectChanges();
+ const commandZEvent = new KeyboardEvent("keydown", { key: "Z", metaKey: true, shiftKey: false });
+ (document.activeElement as HTMLElement)?.blur();
+ document.dispatchEvent(commandZEvent);
+ fixture.detectChanges();
+ expect(undoSpy).toHaveBeenCalledTimes(1);
+
+ const controlZEvent = new KeyboardEvent("keydown", { key: "Z", ctrlKey: true, shiftKey: false });
+ (document.activeElement as HTMLElement)?.blur();
+ document.dispatchEvent(controlZEvent);
+ fixture.detectChanges();
+ expect(undoSpy).toHaveBeenCalledTimes(2);
+ });
+
+ //redo
+ it("should redo action when user presses command/control + Y or command/control + shift + Z", () => {
+ spyOn(workflowVersionService, "getDisplayParticularVersionStream").and.returnValue(of(false));
+ spyOn(undoRedoService, "canRedo").and.returnValue(true);
+ let redoSpy = spyOn(undoRedoService, "redoAction");
+ fixture.detectChanges();
+ const commandYEvent = new KeyboardEvent("keydown", { key: "y", metaKey: true, shiftKey: false });
+ (document.activeElement as HTMLElement)?.blur();
+ document.dispatchEvent(commandYEvent);
+ fixture.detectChanges();
+ expect(redoSpy).toHaveBeenCalledTimes(1);
+
+ const controlYEvent = new KeyboardEvent("keydown", { key: "y", ctrlKey: true, shiftKey: false });
+ (document.activeElement as HTMLElement)?.blur();
+ document.dispatchEvent(controlYEvent);
+ fixture.detectChanges();
+ expect(redoSpy).toHaveBeenCalledTimes(2);
+
+ const commandShitZEvent = new KeyboardEvent("keydown", { key: "z", metaKey: true, shiftKey: true });
+ (document.activeElement as HTMLElement)?.blur();
+ document.dispatchEvent(commandShitZEvent);
+ fixture.detectChanges();
+ expect(redoSpy).toHaveBeenCalledTimes(3);
+
+ const controlShitZEvent = new KeyboardEvent("keydown", { key: "z", ctrlKey: true, shiftKey: true });
+ (document.activeElement as HTMLElement)?.blur();
+ document.dispatchEvent(controlShitZEvent);
+ fixture.detectChanges();
+ expect(redoSpy).toHaveBeenCalledTimes(4);
+ });
});
});
diff --git a/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts b/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts
index 3f419d05611..6c2cd8c8d76 100644
--- a/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts
+++ b/core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts
@@ -190,23 +190,21 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy
.pipe(takeUntil(this._onProcessKeyboardActionObservable))
.subscribe(displayParticularWorkflowVersion => {
if (!displayParticularWorkflowVersion) {
- // Temporarily disabling undo-redo because of a bug that can cause invalid workflow structures.
- // TODO: enable after fixing the bug.
- // // cmd/ctrl+z undo ; ctrl+y or cmd/ctrl + shift+z for redo
- // if ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "z") {
- // // UNDO
- // if (this.undoRedoService.canUndo()) {
- // this.undoRedoService.undoAction();
- // }
- // } else if (
- // ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "y") ||
- // ((event.metaKey || event.ctrlKey) && event.shiftKey && event.key.toLowerCase() === "z")
- // ) {
- // // redo
- // if (this.undoRedoService.canRedo()) {
- // this.undoRedoService.redoAction();
- // }
- // }
+ // cmd/ctrl+z undo ; ctrl+y or cmd/ctrl + shift+z for redo
+ if ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "z") {
+ // UNDO
+ if (this.undoRedoService.canUndo()) {
+ this.undoRedoService.undoAction();
+ }
+ } else if (
+ ((event.metaKey || event.ctrlKey) && !event.shiftKey && event.key.toLowerCase() === "y") ||
+ ((event.metaKey || event.ctrlKey) && event.shiftKey && event.key.toLowerCase() === "z")
+ ) {
+ // redo
+ if (this.undoRedoService.canRedo()) {
+ this.undoRedoService.redoAction();
+ }
+ }
// below for future hotkeys
}
this._onProcessKeyboardActionObservable.complete();
diff --git a/core/gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts b/core/gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts
index 4ad5bda5c8d..e5eab7a812e 100644
--- a/core/gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts
+++ b/core/gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts
@@ -153,9 +153,12 @@ export class SharedModelChangeHandler {
event.changes.keys.forEach((change, key) => {
if (change.action === "add") {
const newLink = this.texeraGraph.sharedModel.operatorLinkMap.get(key) as OperatorLink;
- const jointLinkCell = JointUIService.getJointLinkCell(newLink);
- jointElementsToAdd.push(jointLinkCell);
- linksToAdd.push(newLink);
+ // Validate the link first
+ if (this.validateAndRepairNewLink(newLink)) {
+ const jointLinkCell = JointUIService.getJointLinkCell(newLink);
+ jointElementsToAdd.push(jointLinkCell);
+ linksToAdd.push(newLink);
+ }
}
if (change.action === "delete") {
keysToDelete.push(key);
@@ -200,6 +203,33 @@ export class SharedModelChangeHandler {
});
}
+ /**
+ * Check the sanity of a newly added link. We have constraints on a new link (it should connect to operators and
+ * ports that exist, and it should not be duplicated with another link connecting to the same operator ports.) Such
+ * constraints are enforced if the change to the shared model comes from local UI (`WorkflowGraph.addLink()`). If
+ * the change is initiated by the `UndoManager` or from remote collaborators, however, due to the limitations of Yjs,
+ * it is not possible to check the sanity of this operation before it is applied to the shared model. To ensure the
+ * integrity of the shared model, we validate the link add operation here instead, and repair the shared model if it
+ * violates the constraints.
+ * @param newLink A new link that has already been added to the shared model
+ * @returns Whether this new link passes the sanity check. If it does, this change can be applied to the UI. Otherwise
+ * this link is already deleted from the shared model.
+ */
+ private validateAndRepairNewLink(newLink: OperatorLink): boolean {
+ try {
+ this.texeraGraph.assertLinkNotDuplicated(newLink);
+ // Verify the link connects to operators and ports that exist.
+ this.texeraGraph.assertLinkIsValid(newLink);
+ return true;
+ } catch (error) {
+ // Invalid link, repair the shared model
+ this.texeraGraph.sharedModel.operatorLinkMap.delete(newLink.linkID);
+ // This is treated as a normal repair step and not an error.
+ console.log("failed to add link. cause: ", (error as Error).message);
+ return false;
+ }
+ }
+
/**
* Syncs element positions. Will temporarily block local updates.
* @private
diff --git a/core/gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts b/core/gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts
index 8a3ff7a51c5..0a8f85ae6c6 100644
--- a/core/gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts
+++ b/core/gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts
@@ -1111,6 +1111,19 @@ export class WorkflowGraph {
}
}
+ /**
+ * Checks if a link is unique in the graph. Throws an error if more than one link with the same source and target
+ * as the given link exists.
+ */
+ public assertLinkNotDuplicated(link: OperatorLink): void {
+ const links = this.getAllLinks().filter(
+ value => isEqual(value.source, link.source) && isEqual(value.target, link.target)
+ );
+ if (links.length > 1) {
+ throw new Error(`duplicate link found with same source and target as link ${link}`);
+ }
+ }
+
/**
* Retrieves a subgraph (subDAG) from the workflow graph. This method excludes disabled operators and links.
*