Skip to content
/ qp-n8n Public
forked from n8n-io/n8n

Commit

Permalink
feat(editor): Do not automatically add manual trigger on node plus (n…
Browse files Browse the repository at this point in the history
…8n-io#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
  • Loading branch information
OlegIvaniv authored and sunilrr committed Apr 24, 2023
1 parent 8c554e9 commit b96d654
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 18 deletions.
47 changes: 47 additions & 0 deletions cypress/e2e/4-node-creator.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
});
});
4 changes: 4 additions & 0 deletions cypress/pages/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
11 changes: 11 additions & 0 deletions packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions packages/editor-ui/src/components/Node/NodeCreation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
16 changes: 16 additions & 0 deletions packages/editor-ui/src/constants.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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';
Expand Down
27 changes: 23 additions & 4 deletions packages/editor-ui/src/stores/nodeCreator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -245,6 +246,7 @@ export const useNodeCreatorStore = defineStore(STORES.NODE_CREATOR, {
showScrim: false,
selectedView: TRIGGER_NODE_FILTER,
rootViewHistory: [],
openSource: '',
}),
actions: {
setShowScrim(isVisible: boolean) {
Expand Down Expand Up @@ -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,
];

// 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 =
!isTrigger && !workflowContainsTrigger && isTriggerPanel && !isStickyNode
? [MANUAL_TRIGGER_NODE_TYPE, nodeType]
: [nodeType];
const nodeTypes = shouldAppendManualTrigger
? [MANUAL_TRIGGER_NODE_TYPE, nodeType]
: [nodeType];

return nodeTypes;
},
Expand Down
30 changes: 19 additions & 11 deletions packages/editor-ui/src/views/NodeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -256,6 +257,7 @@ import {
IWorkflowToShare,
IUser,
INodeUpdatePropertiesInformation,
NodeCreatorOpenSource,
} from '@/Interface';
import { debounceHelper } from '@/mixins/debounce';
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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,
});
},
);
Expand Down Expand Up @@ -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,
});
}
},
Expand Down Expand Up @@ -3718,7 +3724,7 @@ export default mixins(
source,
createNodeActive,
}: {
source?: string;
source?: NodeCreatorOpenSource;
createNodeActive: boolean;
}) {
if (createNodeActive === this.createNodeActive) return;
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit b96d654

Please sign in to comment.