fix(editor): Add stickies to node insert position conflict check allo…

…wlist (#11624)
n8n-io · Nov 7, 2024 · fc39e3c · fc39e3c
1 parent 6439291
commit fc39e3c
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 2 deletions.
diff --git a/packages/editor-ui/src/constants.ts b/packages/editor-ui/src/constants.ts
@@ -221,6 +221,8 @@ export const NODES_USING_CODE_NODE_EDITOR = [
 	AI_TRANSFORM_NODE_TYPE,
 ];
 
+export const NODE_POSITION_CONFLICT_ALLOWLIST = [STICKY_NODE_TYPE];
+
 export const PIN_DATA_NODE_TYPES_DENYLIST = [SPLIT_IN_BATCHES_NODE_TYPE, STICKY_NODE_TYPE];
 
 export const OPEN_URL_PANEL_TRIGGER_NODE_TYPES = [

diff --git a/packages/editor-ui/src/utils/nodeViewUtils.test.ts b/packages/editor-ui/src/utils/nodeViewUtils.test.ts
@@ -1,10 +1,12 @@
-import { generateOffsets, getGenericHints } from './nodeViewUtils';
+import { generateOffsets, getGenericHints, getNewNodePosition } from './nodeViewUtils';
 import type { INode, INodeTypeDescription, INodeExecutionData, Workflow } from 'n8n-workflow';
-import type { INodeUi } from '@/Interface';
+import type { INodeUi, XYPosition } from '@/Interface';
 import { NodeHelpers } from 'n8n-workflow';
 
 import { describe, it, expect, beforeEach } from 'vitest';
 import { mock, type MockProxy } from 'vitest-mock-extended';
+import { SET_NODE_TYPE, STICKY_NODE_TYPE } from '@/constants';
+import { createTestNode } from '@/__tests__/mocks';
 
 describe('getGenericHints', () => {
 	let mockWorkflowNode: MockProxy<INode>;
@@ -169,3 +171,57 @@ describe('generateOffsets', () => {
 		expect(result).toEqual([-580, -460, -340, -220, -100, 100, 220, 340, 460, 580]);
 	});
 });
+
+describe('getNewNodePosition', () => {
+	it('should return the new position when there are no conflicts', () => {
+		const nodes: INodeUi[] = [];
+		const newPosition: XYPosition = [100, 100];
+		const result = getNewNodePosition(nodes, newPosition);
+		expect(result).toEqual([100, 100]);
+	});
+
+	it('should adjust the position to the closest grid size', () => {
+		const nodes: INodeUi[] = [];
+		const newPosition: XYPosition = [105, 115];
+		const result = getNewNodePosition(nodes, newPosition);
+		expect(result).toEqual([120, 120]);
+	});
+
+	it('should move the position to avoid conflicts', () => {
+		const nodes: INodeUi[] = [
+			createTestNode({ id: '1', position: [100, 100], type: SET_NODE_TYPE }),
+		];
+		const newPosition: XYPosition = [100, 100];
+		const result = getNewNodePosition(nodes, newPosition);
+		expect(result).toEqual([180, 180]);
+	});
+
+	it('should skip nodes in the conflict allowlist', () => {
+		const nodes: INodeUi[] = [
+			createTestNode({ id: '1', position: [100, 100], type: STICKY_NODE_TYPE }),
+		];
+		const newPosition: XYPosition = [100, 100];
+		const result = getNewNodePosition(nodes, newPosition);
+		expect(result).toEqual([100, 100]);
+	});
+
+	it('should use the provided move position to resolve conflicts', () => {
+		const nodes: INodeUi[] = [
+			createTestNode({ id: '1', position: [100, 100], type: SET_NODE_TYPE }),
+		];
+		const newPosition: XYPosition = [100, 100];
+		const movePosition: XYPosition = [50, 50];
+		const result = getNewNodePosition(nodes, newPosition, movePosition);
+		expect(result).toEqual([200, 200]);
+	});
+
+	it('should handle multiple conflicts correctly', () => {
+		const nodes: INodeUi[] = [
+			createTestNode({ id: '1', position: [100, 100], type: SET_NODE_TYPE }),
+			createTestNode({ id: '2', position: [140, 140], type: SET_NODE_TYPE }),
+		];
+		const newPosition: XYPosition = [100, 100];
+		const result = getNewNodePosition(nodes, newPosition);
+		expect(result).toEqual([220, 220]);
+	});
+});
diff --git a/packages/editor-ui/src/utils/nodeViewUtils.ts b/packages/editor-ui/src/utils/nodeViewUtils.ts
@@ -2,6 +2,7 @@ import { isNumber, isValidNodeConnectionType } from '@/utils/typeGuards';
 import {
 	LIST_LIKE_NODE_OPERATIONS,
 	NODE_OUTPUT_DEFAULT_KEY,
+	NODE_POSITION_CONFLICT_ALLOWLIST,
 	SET_NODE_TYPE,
 	SPLIT_IN_BATCHES_NODE_TYPE,
 	STICKY_NODE_TYPE,
@@ -582,6 +583,11 @@ export const getNewNodePosition = (
 		conflictFound = false;
 		for (i = 0; i < nodes.length; i++) {
 			node = nodes[i];
+
+			if (NODE_POSITION_CONFLICT_ALLOWLIST.includes(node.type)) {
+				continue;
+			}
+
 			if (!canUsePosition(node.position, targetPosition)) {
 				conflictFound = true;
 				break;