Forráskód Böngészése

feat: node names stay unique, and new nodes stop landing on each other

A name is not decoration: an expression refers to another node by name, as
{{$node["Fetch Image"].url}}. Two nodes called the same thing make that
ambiguous, and it resolves to one of them silently - not necessarily the one
meant. 35photo2anime had two feeds both called "RSS Reader", so nothing in it
could say which feed it wanted. They are now "35photo pro" and "35photo ru".

Adding or pasting a second node of a kind now numbers it, and it reads like
something a person would write: "RSS Reader 2", not n8n's "RSS Reader1". A
name that already ends in a number has it replaced rather than appended, so
copying "RSS Reader 2" gives "RSS Reader 3" and not "RSS Reader 2 2".

Typing a name is the opposite case and is refused rather than renumbered,
with the reason and a disabled save. Nobody chose the name of a node they
just added, so numbering it is helpful; silently changing a name somebody
typed on purpose is not.

Also fixed, because it turned up while testing the above: three nodes added
in a row all landed on the same point, stacked, and only the top one could be
clicked at all. Each now steps clear of whatever is already there.

Verified in the editor: adding three gives "RSS Reader", "RSS Reader 2",
"RSS Reader 3" at three distinct positions with all three reachable; two
pastes continue the sequence to 4 and 5; renaming one to a taken name shows
"Another node is already called ..." and disables the save, and a free name
clears both. 65 passed.
fszontagh 1 hónapja
szülő
commit
3e1d348197

+ 31 - 6
webui/src/components/workflow/NodeConfigModal.tsx

@@ -1,3 +1,4 @@
+import { Node } from 'reactflow'
 import { useEffect, useState } from 'react'
 import { X, Database, Maximize2, Minimize2, ExternalLink, Download, Loader2 } from 'lucide-react'
 import { useQuery } from '@tanstack/react-query'
@@ -8,6 +9,7 @@ import { NodeOptionsSelect } from './NodeOptionsSelect'
 import { groupFields, isCredentialKey } from './fieldGroups'
 import { CredentialSelect } from './CredentialSelect'
 import { CollectionSelect } from './CollectionSelect'
+import { nodeNameProblem } from '../../utils/nodeNames'
 import { EnumSelect } from './EnumSelect'
 import { KeyValueEditor } from './KeyValueEditor'
 import { ArrayFieldEditor } from './ArrayFieldEditor'
@@ -44,6 +46,8 @@ interface NodeConfigModalProps {
   // question may use.
   workflowId: string
   editingConfig: Record<string, any>
+  // Every node on the canvas, so a rename can tell whether the name is taken.
+  allNodes: Node[]
   nodeDefs: NodeDefinition[]
   storageCollections: StorageCollection[]
   // Give this workflow access to a collection without leaving the node. The
@@ -64,6 +68,7 @@ export function NodeConfigModal({
   suppliedConfig,
   workflowId,
   editingConfig,
+  allNodes,
   nodeDefs,
   storageCollections,
   onGrantCollection,
@@ -83,6 +88,16 @@ export function NodeConfigModal({
 
   const credentials: CredentialInfo[] = credentialsData?.credentials || []
 
+  // A rename is deliberate, so a clash is refused and explained rather than
+  // quietly numbered - silently changing what somebody typed is worse than
+  // telling them the name is taken. Adding and pasting are the opposite case
+  // and get numbered automatically, because nobody chose those names.
+  const nameProblem = nodeNameProblem(
+    editingConfig._customLabel || '',
+    allNodes,
+    selectedNodeData.id
+  )
+
   // Names for the credential types nodes registered, so a credential shows as
   // "SD.cpp Server" rather than the "basic" it is stored as.
   const credentialTypeLabels: Record<string, string> = {}
@@ -298,14 +313,22 @@ export function NodeConfigModal({
                 value={editingConfig._customLabel || ''}
                 onChange={(e) => onConfigChange({ ...editingConfig, _customLabel: e.target.value })}
                 placeholder={selectedNodeData.name}
-                className="w-full px-3 py-2 border border-gray-300 dark:border-slate-600 rounded-lg text-sm
+                className={`w-full px-3 py-2 border rounded-lg text-sm
                   bg-white dark:bg-slate-900 text-gray-900 dark:text-gray-100
                   placeholder-gray-400 dark:placeholder-gray-500
-                  focus:outline-none focus:ring-2 focus:ring-primary-500 focus:border-transparent"
+                  focus:outline-none focus:ring-2 focus:border-transparent ${
+                    nameProblem
+                      ? 'border-red-400 dark:border-red-500 focus:ring-red-500'
+                      : 'border-gray-300 dark:border-slate-600 focus:ring-primary-500'
+                  }`}
               />
-              <p className="mt-1 text-xs text-gray-500 dark:text-gray-400">
-                Custom name for this node. Leave empty to use default: {selectedNodeData.name}
-              </p>
+              {nameProblem ? (
+                <p className="mt-1 text-xs text-red-600 dark:text-red-400">{nameProblem}</p>
+              ) : (
+                <p className="mt-1 text-xs text-gray-500 dark:text-gray-400">
+                  Custom name for this node. Leave empty to use default: {selectedNodeData.name}
+                </p>
+              )}
             </div>
 
             <div>
@@ -978,7 +1001,9 @@ export function NodeConfigModal({
             </button>
             <button
               onClick={onSave}
-              className="px-4 py-2 bg-primary-600 text-white rounded-lg hover:bg-primary-700"
+              disabled={!!nameProblem}
+              title={nameProblem || undefined}
+              className="px-4 py-2 bg-primary-600 text-white rounded-lg hover:bg-primary-700 disabled:opacity-50 disabled:cursor-not-allowed"
             >
               Save Configuration
             </button>

+ 34 - 3
webui/src/pages/WorkflowEditorPage.tsx

@@ -47,6 +47,7 @@ import { EditorHeader } from '../components/workflow/EditorHeader'
 import { WorkflowSettingsModal } from '../components/workflow/WorkflowSettingsModal'
 import { NodeConfigModal } from '../components/workflow/NodeConfigModal'
 import { copyNodesToClipboard, readNodeClipboard, offsetFor } from '../utils/nodeClipboard'
+import { takenNodeNames, uniqueNodeName } from '../utils/nodeNames'
 
 interface SelectedNodeData {
   id: string
@@ -1582,8 +1583,18 @@ function WorkflowEditorInner() {
       })) as Edge[]
 
     // The originals lose their selection, so the pasted group is what a
-    // follow-up drag or delete acts on.
-    setNodes((nds) => [...nds.map((n) => (n.selected ? { ...n, selected: false } : n)), ...pasted])
+    // follow-up drag or delete acts on. Names are settled here too: pasting
+    // into the workflow the nodes came from would otherwise produce two of
+    // every name.
+    setNodes((nds) => {
+      const taken = takenNodeNames(nds)
+      const renamed = pasted.map((node) => {
+        const label = uniqueNodeName(node.data.label, taken)
+        taken.add(label)
+        return { ...node, data: { ...node.data, label } }
+      })
+      return [...nds.map((n) => (n.selected ? { ...n, selected: false } : n)), ...renamed]
+    })
     if (pastedEdges.length > 0) setEdges((eds) => [...eds, ...pastedEdges])
     setHasChanges(true)
 
@@ -1758,7 +1769,26 @@ function WorkflowEditorInner() {
     // Placed by its middle rather than its corner, so it lands where the eye is.
     const position = { x: Math.round(centre.x - 110), y: Math.round(centre.y - 40) }
     const newNode = makeNode(nodeType, nodeDef, position)
-    setNodes((nds) => [...nds, newNode])
+    // Named inside the update, where the current list is the one being added
+    // to. Two nodes with the same name make {{$node["..."]}} ambiguous.
+    setNodes((nds) => {
+      // Adding several in a row puts them all at the same point, stacked, and
+      // the ones underneath cannot be clicked at all. Each one steps down and
+      // right until it is on free ground.
+      let { x, y } = newNode.position
+      while (nds.some((n) => Math.abs(n.position.x - x) < 30 && Math.abs(n.position.y - y) < 30)) {
+        x += 40
+        y += 40
+      }
+      return [
+        ...nds,
+        {
+          ...newNode,
+          position: { x, y },
+          data: { ...newNode.data, label: uniqueNodeName(newNode.data.label, takenNodeNames(nds)) },
+        },
+      ]
+    })
 
     // Chosen because a link was let go here: connect it to the port that link
     // came from, which is the whole reason the list opened.
@@ -2809,6 +2839,7 @@ function WorkflowEditorInner() {
       {showNodeConfig && selectedNodeData && (
         <NodeConfigModal
           selectedNodeData={selectedNodeData}
+          allNodes={nodes}
           workflowId={id!}
           suppliedConfig={
             // What a connected Configurator will supply to the node being

+ 58 - 0
webui/src/utils/nodeNames.ts

@@ -0,0 +1,58 @@
+import { Node } from 'reactflow'
+
+/**
+ * Keeping node names unique within a workflow.
+ *
+ * Names are not decoration: an expression refers to another node by name, as
+ * {{$node["Fetch Image"].url}}. Two nodes called the same thing make that
+ * reference ambiguous, and it resolves to one of them - silently, and not
+ * necessarily the one meant. A workflow with two feeds both called "RSS Reader"
+ * cannot say which feed it wants.
+ *
+ * So a second node of a kind gets a number, and it reads like something a
+ * person would write: "RSS Reader 2", not "RSS Reader1".
+ */
+
+export function takenNodeNames(nodes: Node[], exceptId?: string): Set<string> {
+  const taken = new Set<string>()
+  for (const node of nodes) {
+    if (exceptId && node.id === exceptId) continue
+    const name = node.data?.label
+    if (typeof name === 'string' && name) taken.add(name)
+  }
+  return taken
+}
+
+export function uniqueNodeName(desired: string, taken: Set<string>): string {
+  const wanted = (desired || 'Node').trim() || 'Node'
+  if (!taken.has(wanted)) return wanted
+
+  // A trailing number is stripped first, so copying "RSS Reader 2" gives
+  // "RSS Reader 3" rather than "RSS Reader 2 2".
+  const base = wanted.replace(/\s+\d+$/, '')
+
+  // Starts at 2 because the one already there is the first. Bounded so a
+  // pathological workflow cannot hang the editor; past that the name is made
+  // unique by other means rather than looping for ever.
+  for (let n = 2; n < 1000; n++) {
+    const candidate = `${base} ${n}`
+    if (!taken.has(candidate)) return candidate
+  }
+  return `${base} ${Date.now()}`
+}
+
+/**
+ * Why a name cannot be used, or null if it can.
+ *
+ * Used when someone types a name themselves. Renaming is deliberate, so a
+ * clash is refused and explained rather than quietly numbered - silently
+ * changing what somebody typed is worse than telling them it is taken.
+ */
+export function nodeNameProblem(name: string, nodes: Node[], nodeId: string): string | null {
+  const wanted = (name || '').trim()
+  if (!wanted) return null  // empty means "use the node's own name", handled elsewhere
+  if (takenNodeNames(nodes, nodeId).has(wanted)) {
+    return `Another node is already called "${wanted}". Names have to be unique, because an expression refers to a node by name.`
+  }
+  return null
+}