From ab68def82adbf6a335a58b6847b36bee039202fb Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:58:25 +0200 Subject: [PATCH] Replace only the selected config field in find and replace Replacing a single config match reduced it to its node and then ran the replacement over every string in that node's config. Picking only workingDirectory therefore also changed the script and any other field containing the search text. Matches now carry their config path as segments, and the replacement touches exactly the strings at the selected paths. A label match no longer changes config either. Replace All covers the same strings as before, because it passes every match. Fixes #597 Co-Authored-By: Claude Opus 5.5 --- .../overlays/FindReplaceOverlay.test.tsx | 23 +++++++++++ .../src/__tests__/lib/findReplace.test.ts | 36 ++++++++++++++++ src/nodepilot-ui/src/lib/findReplace.ts | 41 +++++++++++-------- 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/nodepilot-ui/src/__tests__/components/overlays/FindReplaceOverlay.test.tsx b/src/nodepilot-ui/src/__tests__/components/overlays/FindReplaceOverlay.test.tsx index 928c3090..033e3059 100644 --- a/src/nodepilot-ui/src/__tests__/components/overlays/FindReplaceOverlay.test.tsx +++ b/src/nodepilot-ui/src/__tests__/components/overlays/FindReplaceOverlay.test.tsx @@ -114,6 +114,29 @@ describe('FindReplaceOverlay', () => { expect(onClose).not.toHaveBeenCalled(); }); + it('replacingTheSelectedConfigMatch_changesOnlyThatField', () => { + const onApply = vi.fn(); + render( + + ); + + fireEvent.change(screen.getByPlaceholderText(/Find/i), { target: { value: 'prod' } }); + fireEvent.change(screen.getByPlaceholderText(/Replace with/i), { target: { value: 'test' } }); + fireEvent.click(screen.getByText('Deploy › workingDirectory')); + fireEvent.click(screen.getByTitle(/Replace current match/i)); + + const [appliedNodes] = onApply.mock.calls[0] as [Node[], Edge[]]; + expect((appliedNodes[0].data as { config: Record }).config).toEqual({ + script: 'Restart-Service prod-api', + workingDirectory: '/srv/test', + }); + }); + it('replaceButtonsDisabledWhenNoMatches', () => { render( diff --git a/src/nodepilot-ui/src/__tests__/lib/findReplace.test.ts b/src/nodepilot-ui/src/__tests__/lib/findReplace.test.ts index 7c427d6d..e200723b 100644 --- a/src/nodepilot-ui/src/__tests__/lib/findReplace.test.ts +++ b/src/nodepilot-ui/src/__tests__/lib/findReplace.test.ts @@ -125,6 +125,42 @@ describe('applyReplaceAll', () => { expect(cfg.headers[0].value).toBe('still-New'); }); + it('replacingOneConfigMatch_leavesOtherFieldsOfTheSameNodeUnchanged', () => { + const nodes = [node('n1', 'Deploy prod', { workingDirectory: '/srv/prod/app', script:'Restart-Service prod-api' })]; + const selected = findMatches('prod', nodes, [], ALL_SCOPES) + .filter((m) => m.configKeyPath === 'workingDirectory'); + + const { nodes: out } = applyReplaceAll(selected, 'prod', 'test', nodes, []); + + const data = out[0].data as { label: string; config: Record }; + expect(data.config.workingDirectory).toBe('/srv/test/app'); + expect(data.config.script).toBe('Restart-Service prod-api'); + expect(data.label).toBe('Deploy prod'); + }); + + it('replacingALabelMatch_leavesConfigUnchanged', () => { + const nodes = [node('n1', 'Old step', { script: 'Old-Command' })]; + const labelOnly = findMatches('Old', nodes, [], ALL_SCOPES).filter((m) => m.kind === 'node-label'); + + const { nodes: out } = applyReplaceAll(labelOnly, 'Old', 'New', nodes, []); + + const data = out[0].data as { label: string; config: Record }; + expect(data.label).toBe('New step'); + expect(data.config.script).toBe('Old-Command'); + }); + + it('replacesAtAConfigKeyThatContainsADot', () => { + const nodes = [node('n1', 'Call', { headers: { 'X.Old': 'Old-value', X: { Old: 'Old-nested' } } })]; + const selected = findMatches('Old', nodes, [], ALL_SCOPES) + .filter((m) => m.configPath?.join('/') === 'headers/X.Old'); + + const { nodes: out } = applyReplaceAll(selected, 'Old', 'New', nodes, []); + + const cfg = (out[0].data as { config: { headers: Record } }).config; + expect(cfg.headers['X.Old']).toBe('New-value'); + expect(cfg.headers.X).toEqual({ Old: 'Old-nested' }); + }); + it('doesNotMutateOriginalArrays', () => { // applyReplaceAll must produce new arrays instead of splicing the inputs in place. const nodes = [node('n1', 'OldText')]; diff --git a/src/nodepilot-ui/src/lib/findReplace.ts b/src/nodepilot-ui/src/lib/findReplace.ts index fa77f351..32a13e7a 100644 --- a/src/nodepilot-ui/src/lib/findReplace.ts +++ b/src/nodepilot-ui/src/lib/findReplace.ts @@ -6,8 +6,10 @@ export interface MatchLocation { kind: MatchKind; nodeId?: string; edgeId?: string; - /** Dot-path into config, e.g. "script" or "headers.0.value" */ + /** Dot-path into config for display, e.g. "script" or "headers.0.value" */ configKeyPath?: string; + /** The same path as segments; replacement uses it, because a key may itself contain a dot. */ + configPath?: string[]; /** The matched substring */ matchedText: string; /** Short context around the match for display */ @@ -35,11 +37,11 @@ function traverseObject( obj: unknown, query: string, pathParts: string[], - results: Array<{ path: string; text: string; context: string }>, + results: Array<{ path: string[]; text: string; context: string }>, ): void { if (typeof obj === 'string') { if (obj.toLowerCase().includes(query.toLowerCase())) { - results.push({ path: pathParts.join('.'), text: obj, context: snippet(obj, query) }); + results.push({ path: pathParts, text: obj, context: snippet(obj, query) }); } return; } @@ -98,17 +100,19 @@ export function findMatches( for (const n of nodes) { const config = (n.data as Record).config; if (!config) continue; - const hits: Array<{ path: string; text: string; context: string }> = []; + const hits: Array<{ path: string[]; text: string; context: string }> = []; traverseObject(config, query, [], hits); const nodeLabel = (n.data as Record).label as string | undefined; for (const h of hits) { + const keyPath = h.path.join('.'); results.push({ kind: 'config', nodeId: n.id, - configKeyPath: h.path, + configKeyPath: keyPath, + configPath: h.path, matchedText: h.text, contextSnippet: h.context, - displayName: `${nodeLabel || n.id} › ${h.path || 'config'}`, + displayName: `${nodeLabel || n.id} › ${keyPath || 'config'}`, }); } } @@ -117,17 +121,18 @@ export function findMatches( return results.slice(0, 50); } -function replaceInValue(value: unknown, search: string, replacement: string): unknown { - if (typeof value === 'string') { - return value.replaceAll(search, replacement); - } +/** Replaces inside the one string at `path`; every other config value is left untouched. */ +function replaceAtPath(value: unknown, path: string[], search: string, replacement: string): unknown { + if (path.length === 0) return typeof value === 'string' ? value.replaceAll(search, replacement) : value; + const [head, ...rest] = path; if (Array.isArray(value)) { - return value.map((v) => replaceInValue(v, search, replacement)); + const index = Number(head); + if (!Number.isInteger(index) || index < 0 || index >= value.length) return value; + return value.map((item, i) => (i === index ? replaceAtPath(item, rest, search, replacement) : item)); } - if (value && typeof value === 'object') { - return Object.fromEntries( - Object.entries(value as Record).map(([k, v]) => [k, replaceInValue(v, search, replacement)]) - ); + if (value && typeof value === 'object' && Object.hasOwn(value, head)) { + const record = value as Record; + return { ...record, [head]: replaceAtPath(record[head], rest, search, replacement) }; } return value; } @@ -145,13 +150,13 @@ export function applyReplaceAll( const newNodes = nodes.map((n) => { if (!nodeIds.has(n.id)) return n; const hasLabelMatch = matches.some((m) => m.nodeId === n.id && m.kind === 'node-label'); - const hasConfigMatch = matches.some((m) => m.nodeId === n.id && m.kind === 'config'); let data = n.data as Record; if (hasLabelMatch && typeof data.label === 'string') { data = { ...data, label: data.label.replaceAll(search, replacement) }; } - if (hasConfigMatch && data.config) { - data = { ...data, config: replaceInValue(data.config, search, replacement) }; + for (const m of matches) { + if (m.nodeId !== n.id || m.kind !== 'config' || !m.configPath) continue; + data = { ...data, config: replaceAtPath(data.config, m.configPath, search, replacement) }; } return { ...n, data }; });