From 278edd60104cc30d2a786ff7b0e096b15c5737e1 Mon Sep 17 00:00:00 2001
From: Alex Grozav <alex@grozav.com>
Date: Tue, 23 Jul 2024 10:16:56 +0300
Subject: [PATCH] feat(editor): Add undo/redo when moving nodes in new canvas
 (no-changelog) (#10137)

---
 .../src/components/canvas/Canvas.spec.ts      | 13 ++---
 .../src/components/canvas/Canvas.vue          | 14 +++--
 .../__tests__/useCanvasOperations.spec.ts     | 54 +++++++++++++++++++
 .../src/composables/useCanvasOperations.ts    | 45 ++++++++++++----
 packages/editor-ui/src/types/canvas.ts        |  2 +
 packages/editor-ui/src/views/NodeView.v2.vue  | 22 ++++++--
 6 files changed, 126 insertions(+), 24 deletions(-)

diff --git a/packages/editor-ui/src/components/canvas/Canvas.spec.ts b/packages/editor-ui/src/components/canvas/Canvas.spec.ts
index f9868e7a48..9a3293738e 100644
--- a/packages/editor-ui/src/components/canvas/Canvas.spec.ts
+++ b/packages/editor-ui/src/components/canvas/Canvas.spec.ts
@@ -8,8 +8,7 @@ import { createCanvasConnection, createCanvasNodeElement } from '@/__tests__/dat
 import { NodeConnectionType } from 'n8n-workflow';
 import type { useDeviceSupport } from 'n8n-design-system';
 
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-// @ts-expect-error
+// @ts-expect-error Initialize window object
 global.window = jsdom.window as unknown as Window & typeof globalThis;
 
 vi.mock('n8n-design-system', async (importOriginal) => {
@@ -100,12 +99,14 @@ describe('Canvas', () => {
 		await fireEvent.mouseDown(node, { view: window });
 		await fireEvent.mouseMove(node, {
 			view: window,
-			clientX: 100,
-			clientY: 100,
+			clientX: 96,
+			clientY: 96,
 		});
 		await fireEvent.mouseUp(node, { view: window });
 
-		// Snap to 16px grid: 100 -> 96
-		expect(emitted()['update:node:position']).toEqual([['1', { x: 96, y: 96 }]]);
+		// Snap to 20px grid: 96 -> 100
+		expect(emitted()['update:nodes:position']).toEqual([
+			[[{ id: '1', position: { x: 100, y: 100 } }]],
+		]);
 	});
 });
diff --git a/packages/editor-ui/src/components/canvas/Canvas.vue b/packages/editor-ui/src/components/canvas/Canvas.vue
index cc3f8490c9..4fdbc71503 100644
--- a/packages/editor-ui/src/components/canvas/Canvas.vue
+++ b/packages/editor-ui/src/components/canvas/Canvas.vue
@@ -1,5 +1,5 @@
 <script lang="ts" setup>
-import type { CanvasConnection, CanvasNode, ConnectStartEvent } from '@/types';
+import type { CanvasConnection, CanvasNode, CanvasNodeMoveEvent, ConnectStartEvent } from '@/types';
 import type { EdgeMouseEvent, NodeDragEvent, Connection, XYPosition } from '@vue-flow/core';
 import { useVueFlow, VueFlow, PanelPosition } from '@vue-flow/core';
 import { Background } from '@vue-flow/background';
@@ -16,12 +16,14 @@ import ContextMenu from '@/components/ContextMenu/ContextMenu.vue';
 import type { NodeCreatorOpenSource } from '@/Interface';
 import type { PinDataSource } from '@/composables/usePinnedData';
 import { isPresent } from '@/utils/typesUtils';
+import { GRID_SIZE } from '@/utils/nodeViewUtils';
 
 const $style = useCssModule();
 
 const emit = defineEmits<{
 	'update:modelValue': [elements: CanvasNode[]];
 	'update:node:position': [id: string, position: XYPosition];
+	'update:nodes:position': [events: CanvasNodeMoveEvent[]];
 	'update:node:active': [id: string];
 	'update:node:enabled': [id: string];
 	'update:node:selected': [id: string];
@@ -116,9 +118,11 @@ function onClickNodeAdd(id: string, handle: string) {
 }
 
 function onNodeDragStop(e: NodeDragEvent) {
-	e.nodes.forEach((node) => {
-		onUpdateNodePosition(node.id, node.position);
-	});
+	onUpdateNodesPosition(e.nodes.map((node) => ({ id: node.id, position: node.position })));
+}
+
+function onUpdateNodesPosition(events: CanvasNodeMoveEvent[]) {
+	emit('update:nodes:position', events);
 }
 
 function onUpdateNodePosition(id: string, position: XYPosition) {
@@ -343,7 +347,7 @@ onPaneReady(async () => {
 		:apply-changes="false"
 		pan-on-scroll
 		snap-to-grid
-		:snap-grid="[16, 16]"
+		:snap-grid="[GRID_SIZE, GRID_SIZE]"
 		:min-zoom="0.2"
 		:max-zoom="4"
 		:class="[$style.canvas, { [$style.visible]: paneReady }]"
diff --git a/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts b/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts
index 1d9be2b27d..181cd694ad 100644
--- a/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts
+++ b/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts
@@ -190,6 +190,60 @@ describe('useCanvasOperations', () => {
 		});
 	});
 
+	describe('updateNodesPosition', () => {
+		it('records history for multiple node position updates when tracking is enabled', () => {
+			const events = [
+				{ id: 'node1', position: { x: 100, y: 100 } },
+				{ id: 'node2', position: { x: 200, y: 200 } },
+			];
+			const startRecordingUndoSpy = vi.spyOn(historyStore, 'startRecordingUndo');
+			const stopRecordingUndoSpy = vi.spyOn(historyStore, 'stopRecordingUndo');
+
+			canvasOperations.updateNodesPosition(events, { trackHistory: true, trackBulk: true });
+
+			expect(startRecordingUndoSpy).toHaveBeenCalled();
+			expect(stopRecordingUndoSpy).toHaveBeenCalled();
+		});
+
+		it('updates positions for multiple nodes', () => {
+			const events = [
+				{ id: 'node1', position: { x: 100, y: 100 } },
+				{ id: 'node2', position: { x: 200, y: 200 } },
+			];
+			const setNodePositionByIdSpy = vi.spyOn(workflowsStore, 'setNodePositionById');
+			vi.spyOn(workflowsStore, 'getNodeById')
+				.mockReturnValueOnce(
+					createTestNode({
+						id: events[0].id,
+						position: [events[0].position.x, events[0].position.y],
+					}),
+				)
+				.mockReturnValueOnce(
+					createTestNode({
+						id: events[1].id,
+						position: [events[1].position.x, events[1].position.y],
+					}),
+				);
+
+			canvasOperations.updateNodesPosition(events);
+
+			expect(setNodePositionByIdSpy).toHaveBeenCalledTimes(2);
+			expect(setNodePositionByIdSpy).toHaveBeenCalledWith('node1', [100, 100]);
+			expect(setNodePositionByIdSpy).toHaveBeenCalledWith('node2', [200, 200]);
+		});
+
+		it('does not record history when trackHistory is false', () => {
+			const events = [{ id: 'node1', position: { x: 100, y: 100 } }];
+			const startRecordingUndoSpy = vi.spyOn(historyStore, 'startRecordingUndo');
+			const stopRecordingUndoSpy = vi.spyOn(historyStore, 'stopRecordingUndo');
+
+			canvasOperations.updateNodesPosition(events, { trackHistory: false, trackBulk: false });
+
+			expect(startRecordingUndoSpy).not.toHaveBeenCalled();
+			expect(stopRecordingUndoSpy).not.toHaveBeenCalled();
+		});
+	});
+
 	describe('updateNodePosition', () => {
 		it('should update node position', () => {
 			const setNodePositionByIdSpy = vi
diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts
index c2f87f8faa..1771e922a0 100644
--- a/packages/editor-ui/src/composables/useCanvasOperations.ts
+++ b/packages/editor-ui/src/composables/useCanvasOperations.ts
@@ -49,7 +49,12 @@ import { useSettingsStore } from '@/stores/settings.store';
 import { useTagsStore } from '@/stores/tags.store';
 import { useUIStore } from '@/stores/ui.store';
 import { useWorkflowsStore } from '@/stores/workflows.store';
-import type { CanvasConnection, CanvasConnectionCreateData, CanvasNode } from '@/types';
+import type {
+	CanvasConnection,
+	CanvasConnectionCreateData,
+	CanvasNode,
+	CanvasNodeMoveEvent,
+} from '@/types';
 import { CanvasConnectionMode } from '@/types';
 import {
 	createCanvasConnectionHandleString,
@@ -138,20 +143,33 @@ export function useCanvasOperations({
 	 * Node operations
 	 */
 
+	function updateNodesPosition(
+		events: CanvasNodeMoveEvent[],
+		{ trackHistory = false, trackBulk = true } = {},
+	) {
+		if (trackHistory && trackBulk) {
+			historyStore.startRecordingUndo();
+		}
+
+		events.forEach(({ id, position }) => {
+			updateNodePosition(id, position, { trackHistory });
+		});
+
+		if (trackBulk) {
+			historyStore.stopRecordingUndo();
+		}
+	}
+
 	function updateNodePosition(
 		id: string,
 		position: CanvasNode['position'],
-		{ trackHistory = false, trackBulk = true } = {},
+		{ trackHistory = false } = {},
 	) {
 		const node = workflowsStore.getNodeById(id);
 		if (!node) {
 			return;
 		}
 
-		if (trackHistory && trackBulk) {
-			historyStore.startRecordingUndo();
-		}
-
 		const oldPosition: XYPosition = [...node.position];
 		const newPosition: XYPosition = [position.x, position.y];
 
@@ -159,13 +177,18 @@ export function useCanvasOperations({
 
 		if (trackHistory) {
 			historyStore.pushCommandToUndo(new MoveNodeCommand(node.name, oldPosition, newPosition));
-
-			if (trackBulk) {
-				historyStore.stopRecordingUndo();
-			}
 		}
 	}
 
+	function revertUpdateNodePosition(nodeName: string, position: CanvasNode['position']) {
+		const node = workflowsStore.getNodeByName(nodeName);
+		if (!node) {
+			return;
+		}
+
+		updateNodePosition(node.id, position);
+	}
+
 	async function renameNode(currentName: string, newName: string, { trackHistory = false } = {}) {
 		if (currentName === newName) {
 			return;
@@ -1648,7 +1671,9 @@ export function useCanvasOperations({
 		addNodes,
 		addNode,
 		revertAddNode,
+		updateNodesPosition,
 		updateNodePosition,
+		revertUpdateNodePosition,
 		setNodeActive,
 		setNodeActiveByName,
 		setNodeSelected,
diff --git a/packages/editor-ui/src/types/canvas.ts b/packages/editor-ui/src/types/canvas.ts
index dc2b9032ae..6eddc500b6 100644
--- a/packages/editor-ui/src/types/canvas.ts
+++ b/packages/editor-ui/src/types/canvas.ts
@@ -132,3 +132,5 @@ export interface CanvasNodeHandleInjectionData {
 }
 
 export type ConnectStartEvent = { handleId: string; handleType: string; nodeId: string };
+
+export type CanvasNodeMoveEvent = { id: string; position: CanvasNode['position'] };
diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue
index 99c4330e31..3012a34ac2 100644
--- a/packages/editor-ui/src/views/NodeView.v2.vue
+++ b/packages/editor-ui/src/views/NodeView.v2.vue
@@ -29,7 +29,12 @@ import type {
 	XYPosition,
 } from '@/Interface';
 import type { Connection } from '@vue-flow/core';
-import type { CanvasConnectionCreateData, CanvasNode, ConnectStartEvent } from '@/types';
+import type {
+	CanvasConnectionCreateData,
+	CanvasNode,
+	CanvasNodeMoveEvent,
+	ConnectStartEvent,
+} from '@/types';
 import { CanvasNodeRenderType } from '@/types';
 import {
 	CHAT_TRIGGER_NODE_TYPE,
@@ -134,6 +139,8 @@ const lastClickPosition = ref<XYPosition>([450, 450]);
 const { runWorkflow, stopCurrentExecution, stopWaitingForWebhook } = useRunWorkflow({ router });
 const {
 	updateNodePosition,
+	updateNodesPosition,
+	revertUpdateNodePosition,
 	renameNode,
 	revertRenameNode,
 	setNodeActive,
@@ -444,10 +451,18 @@ const allTriggerNodesDisabled = computed(() => {
 	return disabledTriggerNodes.length === triggerNodes.value.length;
 });
 
+function onUpdateNodesPosition(events: CanvasNodeMoveEvent[]) {
+	updateNodesPosition(events, { trackHistory: true });
+}
+
 function onUpdateNodePosition(id: string, position: CanvasNode['position']) {
 	updateNodePosition(id, position, { trackHistory: true });
 }
 
+function onRevertNodePosition({ nodeName, position }: { nodeName: string; position: XYPosition }) {
+	revertUpdateNodePosition(nodeName, { x: position[0], y: position[1] });
+}
+
 function onDeleteNode(id: string) {
 	deleteNode(id, { trackHistory: true });
 }
@@ -986,7 +1001,7 @@ const chatTriggerNodePinnedData = computed(() => {
  */
 
 function addUndoRedoEventBindings() {
-	// historyBus.on('nodeMove', onMoveNode);
+	historyBus.on('nodeMove', onRevertNodePosition);
 	historyBus.on('revertAddNode', onRevertAddNode);
 	historyBus.on('revertRemoveNode', onRevertDeleteNode);
 	historyBus.on('revertAddConnection', onRevertCreateConnection);
@@ -996,7 +1011,7 @@ function addUndoRedoEventBindings() {
 }
 
 function removeUndoRedoEventBindings() {
-	// historyBus.off('nodeMove', onMoveNode);
+	historyBus.off('nodeMove', onRevertNodePosition);
 	historyBus.off('revertAddNode', onRevertAddNode);
 	historyBus.off('revertRemoveNode', onRevertDeleteNode);
 	historyBus.off('revertAddConnection', onRevertCreateConnection);
@@ -1362,6 +1377,7 @@ onBeforeUnmount(() => {
 		:workflow-object="editableWorkflowObject"
 		:fallback-nodes="fallbackNodes"
 		:event-bus="canvasEventBus"
+		@update:nodes:position="onUpdateNodesPosition"
 		@update:node:position="onUpdateNodePosition"
 		@update:node:active="onSetNodeActive"
 		@update:node:selected="onSetNodeSelected"