From ac2f89a18a4c25ef1b39bcacc624884de9197fdf Mon Sep 17 00:00:00 2001
From: OlegIvaniv <me@olegivaniv.com>
Date: Thu, 9 Mar 2023 15:22:12 +0100
Subject: [PATCH] feat(editor): Do not automatically add manual trigger on node
 plus (#5644)

* feat(editor): Do not add manual trigger node if node creator trigger via canvas actions

* Add e2e tests

* Install cypress-plugin-tab, do not use cy.realPress as it hangs the tests

* Exclude tab tests
---
 cypress/e2e/4-node-creator.cy.ts              | 47 +++++++++++++++++++
 cypress/pages/workflow.ts                     |  4 ++
 cypress/support/commands.ts                   |  2 +-
 packages/editor-ui/src/Interface.ts           | 11 +++++
 .../src/components/Node/NodeCreation.vue      | 12 ++++-
 packages/editor-ui/src/constants.ts           | 16 +++++++
 packages/editor-ui/src/stores/nodeCreator.ts  | 27 +++++++++--
 packages/editor-ui/src/views/NodeView.vue     | 30 +++++++-----
 8 files changed, 131 insertions(+), 18 deletions(-)

diff --git a/cypress/e2e/4-node-creator.cy.ts b/cypress/e2e/4-node-creator.cy.ts
index 2054209f4f..014e94aece 100644
--- a/cypress/e2e/4-node-creator.cy.ts
+++ b/cypress/e2e/4-node-creator.cy.ts
@@ -144,4 +144,51 @@ describe('Node Creator', () => {
 			.click();
 		secondParameter().find('input.el-input__inner').should('have.value', 'option4');
 	});
+
+	describe('should correctly append manual trigger for regular actions', () => {
+		// For these sources, manual node should be added
+		const sourcesWithAppend = [
+			{
+				name: 'canvas add button',
+				handler: () => nodeCreatorFeature.getters.canvasAddButton().click(),
+			}, {
+				name: 'plus button',
+				handler: () => nodeCreatorFeature.getters.plusButton().click(),
+			},
+			// We can't test this one because it's not possible to trigger tab key in Cypress
+			// only way is to use `realPress` which is hanging the tests in Electron for some reason
+			// {
+			// 	name: 'tab key',
+			// 	handler: () => cy.realPress('Tab'),
+			// },
+		]
+		sourcesWithAppend.forEach((source) => {
+			it(`should append manual trigger when source is ${source.name}`, () => {
+				source.handler()
+				nodeCreatorFeature.getters.searchBar().find('input').clear().type('n8n');
+				nodeCreatorFeature.getters.getCreatorItem('n8n').click();
+				nodeCreatorFeature.getters.getCreatorItem('Create a credential').click();
+				NDVModal.actions.close();
+				WorkflowPage.getters.canvasNodes().should('have.length', 2);
+			});
+		});
+
+		it('should not append manual trigger when source is canvas related', () => {
+			nodeCreatorFeature.getters.canvasAddButton().click();
+			nodeCreatorFeature.getters.searchBar().find('input').clear().type('n8n');
+			nodeCreatorFeature.getters.getCreatorItem('n8n').click();
+			nodeCreatorFeature.getters.getCreatorItem('Create a credential').click();
+			NDVModal.actions.close();
+			WorkflowPage.actions.deleteNode('When clicking "Execute Workflow"')
+			WorkflowPage.getters.canvasNodePlusEndpointByName('n8n').click()
+			nodeCreatorFeature.getters.searchBar().find('input').clear().type('n8n');
+			nodeCreatorFeature.getters.getCreatorItem('n8n').click();
+			nodeCreatorFeature.getters.getCreatorItem('Create a credential').click();
+			NDVModal.actions.close();
+			WorkflowPage.getters.canvasNodes().should('have.length', 2);
+			WorkflowPage.actions.zoomToFit();
+			WorkflowPage.actions.addNodeBetweenNodes('n8n', 'n8n1', 'Item Lists')
+			WorkflowPage.getters.canvasNodes().should('have.length', 3);
+		})
+	});
 });
diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts
index 2dc647e4bf..4ad4b753a9 100644
--- a/cypress/pages/workflow.ts
+++ b/cypress/pages/workflow.ts
@@ -174,6 +174,10 @@ export class WorkflowPage extends BasePage {
 		saveWorkflowUsingKeyboardShortcut: () => {
 			cy.get('body').type('{meta}', { release: false }).type('s');
 		},
+		deleteNode: (name: string) => {
+			this.getters.canvasNodeByName(name).first().click();
+			cy.get('body').type('{del}');
+		},
 		setWorkflowName: (name: string) => {
 			this.getters.workflowNameInput().should('be.disabled');
 			this.getters.workflowNameInput().parent().click();
diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts
index 233c74ee55..f700b3c6f3 100644
--- a/cypress/support/commands.ts
+++ b/cypress/support/commands.ts
@@ -24,7 +24,7 @@
 // -- This will overwrite an existing command --
 // Cypress.Commands.overwrite('visit', (originalFn, url, options) => { ... })
 import 'cypress-real-events';
-import { WorkflowsPage, SigninPage, SignupPage, SettingsUsersPage, WorkflowPage } from '../pages';
+import { SigninPage, SignupPage, SettingsUsersPage, WorkflowPage } from '../pages';
 import { N8N_AUTH_COOKIE } from '../constants';
 import { WorkflowPage as WorkflowPageClass } from '../pages/workflow';
 import { MessageBox } from '../pages/modals/message-box';
diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts
index 8503e38759..8f1021a22d 100644
--- a/packages/editor-ui/src/Interface.ts
+++ b/packages/editor-ui/src/Interface.ts
@@ -1179,11 +1179,22 @@ export type IFakeDoorLocation =
 
 export type INodeFilterType = typeof REGULAR_NODE_FILTER | typeof TRIGGER_NODE_FILTER;
 
+export type NodeCreatorOpenSource =
+	| ''
+	| 'no_trigger_execution_tooltip'
+	| 'plus_endpoint'
+	| 'trigger_placeholder_button'
+	| 'tab'
+	| 'node_connection_action'
+	| 'node_connection_drop'
+	| 'add_node_button';
+
 export interface INodeCreatorState {
 	itemsFilter: string;
 	showScrim: boolean;
 	rootViewHistory: INodeFilterType[];
 	selectedView: INodeFilterType;
+	openSource: NodeCreatorOpenSource;
 }
 
 export interface ISettingsState {
diff --git a/packages/editor-ui/src/components/Node/NodeCreation.vue b/packages/editor-ui/src/components/Node/NodeCreation.vue
index 58f4948226..edae0d921b 100644
--- a/packages/editor-ui/src/components/Node/NodeCreation.vue
+++ b/packages/editor-ui/src/components/Node/NodeCreation.vue
@@ -38,7 +38,12 @@
 <script lang="ts">
 import Vue from 'vue';
 import { getMidCanvasPosition } from '@/utils/nodeViewUtils';
-import { DEFAULT_STICKY_HEIGHT, DEFAULT_STICKY_WIDTH, STICKY_NODE_TYPE } from '@/constants';
+import {
+	DEFAULT_STICKY_HEIGHT,
+	DEFAULT_STICKY_WIDTH,
+	NODE_CREATOR_OPEN_SOURCES,
+	STICKY_NODE_TYPE,
+} from '@/constants';
 import { mapStores } from 'pinia';
 import { useUIStore } from '@/stores/ui';
 
@@ -94,7 +99,10 @@ export default Vue.extend({
 			document.addEventListener('mousemove', moveCallback, false);
 		},
 		openNodeCreator() {
-			this.$emit('toggleNodeCreator', { source: 'add_node_button', createNodeActive: true });
+			this.$emit('toggleNodeCreator', {
+				source: NODE_CREATOR_OPEN_SOURCES.ADD_NODE_BUTTON,
+				createNodeActive: true,
+			});
 		},
 		addStickyNote() {
 			if (document.activeElement) {
diff --git a/packages/editor-ui/src/constants.ts b/packages/editor-ui/src/constants.ts
index 8dc8124244..3607bb8f25 100644
--- a/packages/editor-ui/src/constants.ts
+++ b/packages/editor-ui/src/constants.ts
@@ -1,3 +1,5 @@
+import { NodeCreatorOpenSource } from './Interface';
+
 export const MAX_WORKFLOW_SIZE = 16777216; // Workflow size limit in bytes
 export const MAX_WORKFLOW_PINNED_DATA_SIZE = 12582912; // Workflow pinned data size limit in bytes
 export const MAX_DISPLAY_DATA_SIZE = 204800;
@@ -147,6 +149,20 @@ export const NON_ACTIVATABLE_TRIGGER_NODE_TYPES = [
 export const PIN_DATA_NODE_TYPES_DENYLIST = [SPLIT_IN_BATCHES_NODE_TYPE];
 
 // Node creator
+export const NODE_CREATOR_OPEN_SOURCES: Record<
+	Uppercase<NodeCreatorOpenSource>,
+	NodeCreatorOpenSource
+> = {
+	NO_TRIGGER_EXECUTION_TOOLTIP: 'no_trigger_execution_tooltip',
+	PLUS_ENDPOINT: 'plus_endpoint',
+	TRIGGER_PLACEHOLDER_BUTTON: 'trigger_placeholder_button',
+	ADD_NODE_BUTTON: 'add_node_button',
+	TAB: 'tab',
+	NODE_CONNECTION_ACTION: 'node_connection_action',
+	NODE_CONNECTION_DROP: 'node_connection_drop',
+	'': '',
+};
+
 export const CORE_NODES_CATEGORY = 'Core Nodes';
 export const COMMUNICATION_CATEGORY = 'Communication';
 export const CUSTOM_NODES_CATEGORY = 'Custom Nodes';
diff --git a/packages/editor-ui/src/stores/nodeCreator.ts b/packages/editor-ui/src/stores/nodeCreator.ts
index aea5f2af93..824f17754f 100644
--- a/packages/editor-ui/src/stores/nodeCreator.ts
+++ b/packages/editor-ui/src/stores/nodeCreator.ts
@@ -16,6 +16,7 @@ import {
 	CORE_NODES_CATEGORY,
 	TRIGGER_NODE_FILTER,
 	STICKY_NODE_TYPE,
+	NODE_CREATOR_OPEN_SOURCES,
 } from '@/constants';
 import { useNodeTypesStore } from '@/stores/nodeTypes';
 import { useWorkflowsStore } from './workflows';
@@ -245,6 +246,7 @@ export const useNodeCreatorStore = defineStore(STORES.NODE_CREATOR, {
 		showScrim: false,
 		selectedView: TRIGGER_NODE_FILTER,
 		rootViewHistory: [],
+		openSource: '',
 	}),
 	actions: {
 		setShowScrim(isVisible: boolean) {
@@ -350,11 +352,28 @@ export const useNodeCreatorStore = defineStore(STORES.NODE_CREATOR, {
 				const workflowContainsTrigger = workflowTriggerNodes.length > 0;
 				const isTriggerPanel = useNodeCreatorStore().selectedView === TRIGGER_NODE_FILTER;
 				const isStickyNode = nodeType === STICKY_NODE_TYPE;
+				const singleNodeOpenSources = [
+					NODE_CREATOR_OPEN_SOURCES.PLUS_ENDPOINT,
+					NODE_CREATOR_OPEN_SOURCES.NODE_CONNECTION_ACTION,
+					NODE_CREATOR_OPEN_SOURCES.NODE_CONNECTION_DROP,
+				];
 
-				const nodeTypes =
-					!isTrigger && !workflowContainsTrigger && isTriggerPanel && !isStickyNode
-						? [MANUAL_TRIGGER_NODE_TYPE, nodeType]
-						: [nodeType];
+				// If the node creator was opened from the plus endpoint, node connection action, or node connection drop
+				// then we do not want to append the manual trigger
+				const isSingleNodeOpenSource = singleNodeOpenSources.includes(
+					useNodeCreatorStore().openSource,
+				);
+
+				const shouldAppendManualTrigger =
+					!isSingleNodeOpenSource &&
+					!isTrigger &&
+					!workflowContainsTrigger &&
+					isTriggerPanel &&
+					!isStickyNode;
+
+				const nodeTypes = shouldAppendManualTrigger
+					? [MANUAL_TRIGGER_NODE_TYPE, nodeType]
+					: [nodeType];
 
 				return nodeTypes;
 			},
diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue
index 29c4ebc708..86461bc09b 100644
--- a/packages/editor-ui/src/views/NodeView.vue
+++ b/packages/editor-ui/src/views/NodeView.vue
@@ -33,7 +33,7 @@
 				>
 					<canvas-add-button
 						:style="canvasAddButtonStyle"
-						@click="showTriggerCreator('trigger_placeholder_button')"
+						@click="showTriggerCreator(NODE_CREATOR_OPEN_SOURCES.TRIGGER_PLACEHOLDER_BUTTON)"
 						v-show="showCanvasAddButton"
 						:showTooltip="!containsTrigger && showTriggerMissingTooltip"
 						:position="canvasStore.canvasAddButtonPosition"
@@ -198,6 +198,7 @@ import {
 	ASSUMPTION_EXPERIMENT,
 	REGULAR_NODE_FILTER,
 	MANUAL_TRIGGER_NODE_TYPE,
+	NODE_CREATOR_OPEN_SOURCES,
 } from '@/constants';
 import { copyPaste } from '@/mixins/copyPaste';
 import { externalHooks } from '@/mixins/externalHooks';
@@ -256,6 +257,7 @@ import {
 	IWorkflowToShare,
 	IUser,
 	INodeUpdatePropertiesInformation,
+	NodeCreatorOpenSource,
 } from '@/Interface';
 
 import { debounceHelper } from '@/mixins/debounce';
@@ -613,6 +615,7 @@ export default mixins(
 			// in undo history as a user action.
 			// This should prevent automatically removed connections from populating undo stack
 			suspendRecordingDetachedConnections: false,
+			NODE_CREATOR_OPEN_SOURCES,
 		};
 	},
 	beforeDestroy() {
@@ -662,7 +665,7 @@ export default mixins(
 					: this.$locale.baseText('nodeView.addATriggerNodeFirst');
 
 			this.registerCustomAction('showNodeCreator', () =>
-				this.showTriggerCreator('no_trigger_execution_tooltip'),
+				this.showTriggerCreator(NODE_CREATOR_OPEN_SOURCES.NO_TRIGGER_EXECUTION_TOOLTIP),
 			);
 			const notice = this.$showMessage({
 				type: 'info',
@@ -756,7 +759,7 @@ export default mixins(
 			const saved = await this.saveCurrentWorkflow();
 			if (saved) await this.settingsStore.fetchPromptsData();
 		},
-		showTriggerCreator(source: string) {
+		showTriggerCreator(source: NodeCreatorOpenSource) {
 			if (this.createNodeActive) return;
 			this.nodeCreatorStore.setSelectedView(TRIGGER_NODE_FILTER);
 			this.nodeCreatorStore.setShowScrim(true);
@@ -1029,7 +1032,7 @@ export default mixins(
 				this.callDebounced('deleteSelectedNodes', { debounceTime: 500 });
 			} else if (e.key === 'Tab') {
 				this.onToggleNodeCreator({
-					source: 'tab',
+					source: NODE_CREATOR_OPEN_SOURCES.TAB,
 					createNodeActive: !this.createNodeActive && !this.isReadOnly,
 				});
 			} else if (e.key === this.controlKeyCode) {
@@ -2059,7 +2062,7 @@ export default mixins(
 		insertNodeAfterSelected(info: {
 			sourceId: string;
 			index: number;
-			eventSource: string;
+			eventSource: NodeCreatorOpenSource;
 			connection?: Connection;
 		}) {
 			// Get the node and set it as active that new nodes
@@ -2078,7 +2081,10 @@ export default mixins(
 				this.lastSelectedConnection = info.connection;
 			}
 
-			this.onToggleNodeCreator({ source: info.eventSource, createNodeActive: true });
+			this.onToggleNodeCreator({
+				source: info.eventSource,
+				createNodeActive: true,
+			});
 		},
 		onEventConnectionAbort(connection: Connection) {
 			try {
@@ -2103,7 +2109,7 @@ export default mixins(
 				this.insertNodeAfterSelected({
 					sourceId: connection.parameters.nodeId,
 					index: connection.parameters.index,
-					eventSource: 'node_connection_drop',
+					eventSource: NODE_CREATOR_OPEN_SOURCES.NODE_CONNECTION_DROP,
 				});
 			} catch (e) {
 				console.error(e); // eslint-disable-line no-console
@@ -2183,7 +2189,7 @@ export default mixins(
 								sourceId: info.sourceEndpoint.parameters.nodeId,
 								index: sourceInfo.index,
 								connection: info.connection,
-								eventSource: 'node_connection_action',
+								eventSource: NODE_CREATOR_OPEN_SOURCES.NODE_CONNECTION_ACTION,
 							});
 						},
 					);
@@ -2421,7 +2427,7 @@ export default mixins(
 				this.insertNodeAfterSelected({
 					sourceId: endpoint.__meta.nodeId,
 					index: endpoint.__meta.index,
-					eventSource: 'plus_endpoint',
+					eventSource: NODE_CREATOR_OPEN_SOURCES.PLUS_ENDPOINT,
 				});
 			}
 		},
@@ -3718,7 +3724,7 @@ export default mixins(
 			source,
 			createNodeActive,
 		}: {
-			source?: string;
+			source?: NodeCreatorOpenSource;
 			createNodeActive: boolean;
 		}) {
 			if (createNodeActive === this.createNodeActive) return;
@@ -3732,6 +3738,8 @@ export default mixins(
 
 			const mode =
 				this.nodeCreatorStore.selectedView === TRIGGER_NODE_FILTER ? 'trigger' : 'regular';
+
+			this.nodeCreatorStore.openSource = source || '';
 			this.$externalHooks().run('nodeView.createNodeActiveChanged', {
 				source,
 				mode,
@@ -3927,7 +3935,7 @@ export default mixins(
 	activated() {
 		const openSideMenu = this.uiStore.addFirstStepOnLoad;
 		if (openSideMenu) {
-			this.showTriggerCreator('trigger_placeholder_button');
+			this.showTriggerCreator(NODE_CREATOR_OPEN_SOURCES.TRIGGER_PLACEHOLDER_BUTTON);
 		}
 		this.uiStore.addFirstStepOnLoad = false;
 		this.bindCanvasEvents();