From c58eb240b1db05761bafc35eebe42776d9008f55 Mon Sep 17 00:00:00 2001
From: Lukas <lukas.pupkalipinski@lpl-mind.de>
Date: Sun, 19 Apr 2026 19:59:13 +0200
Subject: [PATCH] Fix KVEditor duplicate key rename validation

Prevent users from renaming a KVEditor entry to an existing key and show inline validation errors.
---
 Docs/Tasks.md                                 |  4 +-
 frontend/src/components/config/KVEditor.tsx   | 75 ++++++++++++++++---
 .../config/__tests__/KVEditor.test.tsx        | 37 +++++++++
 .../src/components/config/configStyles.ts     |  5 ++
 4 files changed, 110 insertions(+), 11 deletions(-)
 create mode 100644 frontend/src/components/config/__tests__/KVEditor.test.tsx

diff --git a/Docs/Tasks.md b/Docs/Tasks.md
index 2a4d2c7..c617c66 100644
--- a/Docs/Tasks.md
+++ b/Docs/Tasks.md
@@ -73,9 +73,9 @@ Issues are grouped by category and ordered roughly by severity. Each entry descr
 
 ---
 
-### TASK-004 — `KVEditor` key rename silently overwrites duplicate keys
+### TASK-004 — `KVEditor` key rename silently overwrites duplicate keys (done)
 
-**Where found:** `frontend/src/components/config/KVEditor.tsx`, `handleKeyChange` function (lines 14–20).
+**Where fixed:** `frontend/src/components/config/KVEditor.tsx`, `frontend/src/components/config/configStyles.ts`, `frontend/src/components/config/__tests__/KVEditor.test.tsx`
 
 **Goal:** Before applying a key rename, check whether `newKey` already exists in `entries`. If it does, show a validation error inline (a `MessageBar` beneath the affected input row or a red border via `validationState="error"` on the Fluent UI `Input`). Block the `onChange` call until the conflict is resolved.
 
diff --git a/frontend/src/components/config/KVEditor.tsx b/frontend/src/components/config/KVEditor.tsx
index 59cfd66..8870a33 100644
--- a/frontend/src/components/config/KVEditor.tsx
+++ b/frontend/src/components/config/KVEditor.tsx
@@ -1,5 +1,6 @@
-import { Button, Input, tokens } from "@fluentui/react-components";
+import { Button, Input, MessageBar, MessageBarBody, tokens } from "@fluentui/react-components";
 import { Add24Regular, Delete24Regular } from "@fluentui/react-icons";
+import { useEffect, useMemo, useState } from "react";
 import { useConfigStyles } from "./configStyles";
 
 interface KVEditorProps {
@@ -9,14 +10,63 @@ interface KVEditorProps {
 
 export function KVEditor({ entries, onChange }: KVEditorProps): React.JSX.Element {
   const styles = useConfigStyles();
-  const rows = Object.entries(entries);
+  const rows = useMemo(() => Object.entries(entries), [entries]);
+  const entryKeys = useMemo(() => Object.keys(entries), [entries]);
+  const entryKeyList = entryKeys.join(",");
+  const [editedKeys, setEditedKeys] = useState<Record<string, string>>(
+    Object.fromEntries(rows.map(([key]) => [key, key])),
+  );
+  const [errors, setErrors] = useState<Record<string, string>>({});
 
-  const handleKeyChange = (oldKey: string, newKey: string): void => {
-    const next: Record<string, string> = {};
-    for (const [k, v] of Object.entries(entries)) {
-      next[k === oldKey ? newKey : k] = v;
+  useEffect(() => {
+    setEditedKeys(Object.fromEntries(rows.map(([key]) => [key, key])));
+    setErrors((previousErrors) =>
+      Object.fromEntries(
+        entryKeys
+          .filter((key) => previousErrors[key])
+          .map((key) => [key, previousErrors[key] ?? ""]),
+      ),
+    );
+  }, [entryKeyList, rows, entryKeys]);
+
+  const validateKey = (oldKey: string, newKey: string): string | null => {
+    const trimmedKey = newKey.trim();
+    if (trimmedKey.length === 0) {
+      return "Key may not be blank.";
+    }
+
+    const conflict = Object.entries(editedKeys).some(
+      ([otherKey, otherValue]) => otherKey !== oldKey && otherValue === trimmedKey,
+    );
+    if (conflict) {
+      return "A setting with this key already exists.";
+    }
+
+    return null;
+  };
+
+  const updateKeyValue = (oldKey: string, newKey: string): void => {
+    setEditedKeys((previous) => ({ ...previous, [oldKey]: newKey }));
+    setErrors((previous) =>
+      Object.fromEntries(Object.entries(previous).filter(([key]) => key !== oldKey)),
+    );
+  };
+
+  const commitKeyChange = (oldKey: string): void => {
+    const newKey = editedKeys[oldKey] ?? oldKey;
+    const error = validateKey(oldKey, newKey);
+    if (error) {
+      setErrors((previous) => ({ ...previous, [oldKey]: error }));
+      return;
+    }
+
+    if (newKey !== oldKey) {
+      const next: Record<string, string> = {};
+      for (const [key, value] of Object.entries(entries)) {
+        next[key === oldKey ? newKey : key] = value;
+      }
+      onChange(next);
     }
-    onChange(next);
   };
 
   const handleValueChange = (key: string, value: string): void => {
@@ -43,11 +93,13 @@ export function KVEditor({ entries, onChange }: KVEditorProps): React.JSX.Elemen
       {rows.map(([key, value]) => (
         <div key={key} className={styles.fieldRow}>
           <Input
-            value={key}
+            value={editedKeys[key] ?? key}
             size="small"
             style={{ width: 160, fontFamily: tokens.fontFamilyMonospace }}
             aria-label={`Setting name: ${key}`}
-            onChange={(_e, d) => { handleKeyChange(key, d.value); }}
+            aria-invalid={Boolean(errors[key])}
+            onBlur={() => { commitKeyChange(key); }}
+            onChange={(_e, d) => { updateKeyValue(key, d.value); }}
           />
           <Input
             value={value}
@@ -63,6 +115,11 @@ export function KVEditor({ entries, onChange }: KVEditorProps): React.JSX.Elemen
             onClick={() => { handleDelete(key); }}
             aria-label={`Delete key ${key}`}
           />
+          {errors[key] ? (
+            <MessageBar intent="error" className={styles.fieldError}>
+              <MessageBarBody>{errors[key]}</MessageBarBody>
+            </MessageBar>
+          ) : null}
         </div>
       ))}
       <Button
diff --git a/frontend/src/components/config/__tests__/KVEditor.test.tsx b/frontend/src/components/config/__tests__/KVEditor.test.tsx
new file mode 100644
index 0000000..ee7b9f0
--- /dev/null
+++ b/frontend/src/components/config/__tests__/KVEditor.test.tsx
@@ -0,0 +1,37 @@
+import { describe, expect, it, vi } from "vitest";
+import { fireEvent, render, screen } from "@testing-library/react";
+import { FluentProvider, webLightTheme } from "@fluentui/react-components";
+import { KVEditor } from "../KVEditor";
+
+describe("KVEditor", () => {
+  it("prevents renaming a key to a duplicate and shows an inline error", () => {
+    const handleChange = vi.fn();
+    render(
+      <FluentProvider theme={webLightTheme}>
+        <KVEditor entries={{ first: "1", second: "2" }} onChange={handleChange} />
+      </FluentProvider>,
+    );
+
+    const firstKeyInput = screen.getByLabelText(/Setting name: first/i);
+    fireEvent.change(firstKeyInput, { target: { value: "second" } });
+    fireEvent.blur(firstKeyInput);
+
+    expect(handleChange).not.toHaveBeenCalled();
+    expect(screen.getByText(/already exists/i)).toBeInTheDocument();
+  });
+
+  it("commits a valid key rename on blur", () => {
+    const handleChange = vi.fn();
+    render(
+      <FluentProvider theme={webLightTheme}>
+        <KVEditor entries={{ first: "1", second: "2" }} onChange={handleChange} />
+      </FluentProvider>,
+    );
+
+    const firstKeyInput = screen.getByLabelText(/Setting name: first/i);
+    fireEvent.change(firstKeyInput, { target: { value: "primary" } });
+    fireEvent.blur(firstKeyInput);
+
+    expect(handleChange).toHaveBeenCalledWith({ primary: "1", second: "2" });
+  });
+});
diff --git a/frontend/src/components/config/configStyles.ts b/frontend/src/components/config/configStyles.ts
index adb1581..9b5caf9 100644
--- a/frontend/src/components/config/configStyles.ts
+++ b/frontend/src/components/config/configStyles.ts
@@ -167,6 +167,11 @@ export const useConfigStyles = makeStyles({
     fontFamily: "monospace",
     borderLeft: `3px solid ${tokens.colorBrandStroke1}`,
   },
+  fieldError: {
+    gridColumn: "1 / -1",
+    marginTop: tokens.spacingVerticalXXS,
+    color: tokens.colorPaletteRedForeground1,
+  },
   logLine: {
     padding: `${tokens.spacingVerticalXS} ${tokens.spacingHorizontalS}`,
     borderRadius: tokens.borderRadiusSmall,