Skip to content

Commit

Permalink
fix(editor): Fix empty node name handling (#9548)
Browse files Browse the repository at this point in the history
Co-authored-by: oleg <me@olegivaniv.com>
  • Loading branch information
MiloradFilipovic and OlegIvaniv authored May 30, 2024
1 parent 596990b commit da41d31
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 1 deletion.
11 changes: 11 additions & 0 deletions cypress/e2e/12-canvas.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,17 @@ describe('Canvas Node Manipulation and Navigation', () => {
WorkflowPage.getters.canvasNodeByName(RENAME_NODE_NAME2).should('exist');
});

it('should not allow empty strings for node names', () => {
WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME);
WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME);
WorkflowPage.getters.canvasNodes().last().click();
cy.get('body').trigger('keydown', { key: 'F2' });
cy.get('.rename-prompt').should('be.visible');
cy.get('body').type('{backspace}');
cy.get('body').type('{enter}');
cy.get('.rename-prompt').should('contain', 'Invalid Name');
});

it('should duplicate nodes (context menu or shortcut)', () => {
WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME);
WorkflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click();
Expand Down
13 changes: 13 additions & 0 deletions cypress/e2e/7-workflow-actions.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,19 @@ describe('Workflow Actions', () => {
});
});

it('should allow importing nodes without names', () => {
cy.fixture('Test_workflow-actions_import_nodes_empty_name.json').then((data) => {
cy.get('body').paste(JSON.stringify(data));
WorkflowPage.actions.zoomToFit();
WorkflowPage.getters.canvasNodes().should('have.length', 3);
WorkflowPage.getters.nodeConnections().should('have.length', 2);
// Check if all nodes have names
WorkflowPage.getters.canvasNodes().each((node) => {
cy.wrap(node).should('have.attr', 'data-name');
});
});
});

it('should update workflow settings', () => {
cy.visit(WorkflowPages.url);
WorkflowPages.getters.workflowCards().then((cards) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
{
"meta": {
"instanceId": "4a31be0d29cfa6246ba62b359030d712af57b98c5dfe6a7ee8beee0a46c5b5a4"
},
"nodes": [
{
"parameters": {
"operation": "get"
},
"id": "5b084875-bd5e-4731-9591-18d2c8996945",
"name": "",
"type": "n8n-nodes-base.gmail",
"typeVersion": 2.1,
"position": [
900,
460
]
},
{
"parameters": {},
"id": "449ab540-d9d7-480d-b131-05e9989a69cd",
"name": "When clicking \"Test workflow\"",
"type": "n8n-nodes-base.manualTrigger",
"typeVersion": 1,
"position": [
460,
460
]
},
{
"parameters": {
"operation": "get"
},
"id": "3a791321-6f0c-4f92-91e5-20e1be0d4964",
"name": "Gmail",
"type": "n8n-nodes-base.gmail",
"typeVersion": 2.1,
"position": [
680,
460
]
}
],
"connections": {
"When clicking \"Test workflow\"": {
"main": [
[
{
"node": "Gmail",
"type": "main",
"index": 0
}
]
]
},
"Gmail": {
"main": [
[
{
"node": "Gmail1",
"type": "main",
"index": 0
}
]
]
}
},
"pinData": {}
}
16 changes: 15 additions & 1 deletion packages/editor-ui/src/views/NodeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,15 @@ export default defineComponent({
try {
const nodeIdMap: { [prev: string]: string } = {};
if (workflowData.nodes) {
const nodeNames = workflowData.nodes.map((node) => node.name);
workflowData.nodes.forEach((node: INode) => {
// Provide a new name for nodes that don't have one
if (!node.name) {
const nodeType = this.nodeTypesStore.getNodeType(node.type);
const newName = this.uniqueNodeName(nodeType?.displayName ?? node.type, nodeNames);
node.name = newName;
nodeNames.push(newName);
}
//generate new webhookId if workflow already contains a node with the same webhookId
if (node.webhookId && UPDATE_WEBHOOK_ID_NODE_TYPES.includes(node.type)) {
const isDuplicate = Object.values(
Expand Down Expand Up @@ -4137,6 +4145,12 @@ export default defineComponent({
cancelButtonText: this.$locale.baseText('nodeView.prompt.cancel'),
inputErrorMessage: this.$locale.baseText('nodeView.prompt.invalidName'),
inputValue: currentName,
inputValidator: (value: string) => {
if (!value.trim()) {
return this.$locale.baseText('nodeView.prompt.invalidName');
}
return true;
},
},
);
Expand Down Expand Up @@ -4402,7 +4416,7 @@ export default defineComponent({
async addNodesToWorkflow(data: IWorkflowDataUpdate): Promise<IWorkflowDataUpdate> {
// Because nodes with the same name maybe already exist, it could
// be needed that they have to be renamed. Also could it be possible
// that nodes are not allowd to be created because they have a create
// that nodes are not allowed to be created because they have a create
// limit set. So we would then link the new nodes with the already existing ones.
// In this object all that nodes get saved in the format:
// old-name -> new-name
Expand Down

0 comments on commit da41d31

Please sign in to comment.