Răsfoiți Sursa

refactor(ui): replace TagInput with checkbox-based preset selection in ClientRestrictionsEditor

Replace free-text TagInput with checkbox rows for preset clients (Allow/Block)
in the provider edit form, matching the existing user edit form pattern.

- Rewrite ClientRestrictionsEditor to render preset clients as checkbox rows
  with Allow/Block options, and custom patterns via ArrayTagInputField
- Update routing-section.tsx to pass translations object instead of individual
  label/placeholder/getPresetLabel props
- Add 12 unit tests covering preset toggles, mutual exclusion, custom value
  splitting, disabled state, and mixed preset+custom scenarios
ding113 1 lună în urmă
părinte
comite
dc6b6d0fc9

+ 26 - 11
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx

@@ -3,7 +3,7 @@
 import { motion } from "framer-motion";
 import { Info, Layers, Route, Scale, Settings, Timer } from "lucide-react";
 import { useTranslations } from "next-intl";
-import { useCallback, useEffect, useState } from "react";
+import { useEffect, useState } from "react";
 import { toast } from "sonner";
 import { ClientRestrictionsEditor } from "@/components/form/client-restrictions-editor";
 import { Badge } from "@/components/ui/badge";
@@ -91,11 +91,6 @@ export function RoutingSection() {
     }
   };
 
-  const getClientRestrictionPresetLabel = useCallback(
-    (presetValue: string) => t(`sections.routing.clientRestrictions.presetClients.${presetValue}`),
-    [t]
-  );
-
   return (
     <TooltipProvider>
       <motion.div
@@ -279,12 +274,32 @@ export function RoutingSection() {
                   onBlockedChange={(next) =>
                     dispatch({ type: "SET_BLOCKED_CLIENTS", payload: next })
                   }
-                  allowedLabel={t("sections.routing.clientRestrictions.allowedLabel")}
-                  blockedLabel={t("sections.routing.clientRestrictions.blockedLabel")}
-                  allowedPlaceholder={t("sections.routing.clientRestrictions.allowedPlaceholder")}
-                  blockedPlaceholder={t("sections.routing.clientRestrictions.blockedPlaceholder")}
                   disabled={state.ui.isPending}
-                  getPresetLabel={getClientRestrictionPresetLabel}
+                  translations={{
+                    allowAction: t("sections.routing.clientRestrictions.allowAction"),
+                    blockAction: t("sections.routing.clientRestrictions.blockAction"),
+                    customAllowedLabel: t("sections.routing.clientRestrictions.customAllowedLabel"),
+                    customAllowedPlaceholder: t(
+                      "sections.routing.clientRestrictions.customAllowedPlaceholder"
+                    ),
+                    customBlockedLabel: t("sections.routing.clientRestrictions.customBlockedLabel"),
+                    customBlockedPlaceholder: t(
+                      "sections.routing.clientRestrictions.customBlockedPlaceholder"
+                    ),
+                    customHelp: t("sections.routing.clientRestrictions.customHelp"),
+                    presetClients: {
+                      "claude-code": t(
+                        "sections.routing.clientRestrictions.presetClients.claude-code"
+                      ),
+                      "gemini-cli": t(
+                        "sections.routing.clientRestrictions.presetClients.gemini-cli"
+                      ),
+                      "factory-cli": t(
+                        "sections.routing.clientRestrictions.presetClients.factory-cli"
+                      ),
+                      "codex-cli": t("sections.routing.clientRestrictions.presetClients.codex-cli"),
+                    },
+                  }}
                   onInvalidTag={(_tag, reason) => {
                     const messages: Record<string, string> = {
                       empty: tUI("emptyTag"),

+ 370 - 0
src/components/form/client-restrictions-editor.test.tsx

@@ -0,0 +1,370 @@
+/**
+ * @vitest-environment happy-dom
+ */
+
+import type { ReactNode } from "react";
+import { useState } from "react";
+import { act } from "react";
+import { createRoot } from "react-dom/client";
+import { describe, expect, test, afterEach, vi } from "vitest";
+import { ClientRestrictionsEditor } from "@/components/form/client-restrictions-editor";
+
+const TEST_TRANSLATIONS = {
+  allowAction: "Allow",
+  blockAction: "Block",
+  customAllowedLabel: "Custom Allowed",
+  customAllowedPlaceholder: "e.g. my-tool",
+  customBlockedLabel: "Custom Blocked",
+  customBlockedPlaceholder: "e.g. legacy",
+  customHelp: "Custom patterns help text",
+  presetClients: {
+    "claude-code": "Claude Code (all)",
+    "gemini-cli": "Gemini CLI",
+    "factory-cli": "Droid CLI",
+    "codex-cli": "Codex CLI",
+  },
+};
+
+function render(node: ReactNode) {
+  const container = document.createElement("div");
+  document.body.appendChild(container);
+  const root = createRoot(container);
+
+  act(() => {
+    root.render(node);
+  });
+
+  return {
+    container,
+    unmount: () => {
+      act(() => root.unmount());
+      container.remove();
+    },
+  };
+}
+
+afterEach(() => {
+  while (document.body.firstChild) {
+    document.body.removeChild(document.body.firstChild);
+  }
+});
+
+describe("ClientRestrictionsEditor", () => {
+  test("renders all 4 preset client rows with correct display labels", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    const { container, unmount } = render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    expect(container.textContent).toContain("Claude Code (all)");
+    expect(container.textContent).toContain("Gemini CLI");
+    expect(container.textContent).toContain("Droid CLI");
+    expect(container.textContent).toContain("Codex CLI");
+
+    unmount();
+  });
+
+  test("renders Allow and Block checkboxes for each preset", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    const { container, unmount } = render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    expect(document.getElementById("allow-claude-code")).not.toBeNull();
+    expect(document.getElementById("block-claude-code")).not.toBeNull();
+    expect(document.getElementById("allow-gemini-cli")).not.toBeNull();
+    expect(document.getElementById("block-gemini-cli")).not.toBeNull();
+    expect(document.getElementById("allow-factory-cli")).not.toBeNull();
+    expect(document.getElementById("block-factory-cli")).not.toBeNull();
+    expect(document.getElementById("allow-codex-cli")).not.toBeNull();
+    expect(document.getElementById("block-codex-cli")).not.toBeNull();
+
+    unmount();
+  });
+
+  test("clicking Allow checkbox calls onAllowedChange with preset value added", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const allowCheckbox = document.getElementById("allow-claude-code");
+    expect(allowCheckbox).not.toBeNull();
+
+    act(() => {
+      allowCheckbox?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(onAllowedChange).toHaveBeenCalledWith(["claude-code"]);
+
+    while (document.body.firstChild) {
+      document.body.removeChild(document.body.firstChild);
+    }
+  });
+
+  test("clicking Allow checkbox also removes preset from blocked list", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={["claude-code"]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const allowCheckbox = document.getElementById("allow-claude-code");
+    expect(allowCheckbox).not.toBeNull();
+
+    act(() => {
+      allowCheckbox?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(onAllowedChange).toHaveBeenCalledWith(["claude-code"]);
+    expect(onBlockedChange).toHaveBeenCalledWith([]);
+
+    while (document.body.firstChild) {
+      document.body.removeChild(document.body.firstChild);
+    }
+  });
+
+  test("clicking Block checkbox calls onBlockedChange with preset value added", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const blockCheckbox = document.getElementById("block-gemini-cli");
+    expect(blockCheckbox).not.toBeNull();
+
+    act(() => {
+      blockCheckbox?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(onBlockedChange).toHaveBeenCalledWith(["gemini-cli"]);
+
+    while (document.body.firstChild) {
+      document.body.removeChild(document.body.firstChild);
+    }
+  });
+
+  test("clicking Block checkbox also removes preset from allowed list", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    render(
+      <ClientRestrictionsEditor
+        allowed={["gemini-cli"]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const blockCheckbox = document.getElementById("block-gemini-cli");
+    expect(blockCheckbox).not.toBeNull();
+
+    act(() => {
+      blockCheckbox?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(onBlockedChange).toHaveBeenCalledWith(["gemini-cli"]);
+    expect(onAllowedChange).toHaveBeenCalledWith([]);
+
+    while (document.body.firstChild) {
+      document.body.removeChild(document.body.firstChild);
+    }
+  });
+
+  test("unchecking an already-checked Allow removes preset from allowed list", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    render(
+      <ClientRestrictionsEditor
+        allowed={["claude-code"]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const allowCheckbox = document.getElementById("allow-claude-code");
+    expect(allowCheckbox).not.toBeNull();
+
+    act(() => {
+      allowCheckbox?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(onAllowedChange).toHaveBeenCalledWith([]);
+
+    while (document.body.firstChild) {
+      document.body.removeChild(document.body.firstChild);
+    }
+  });
+
+  test("custom values are split correctly and shown in custom TagInput fields", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    const { container, unmount } = render(
+      <ClientRestrictionsEditor
+        allowed={["my-custom-tool", "another-one"]}
+        blocked={["legacy-client"]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    // Verify that custom allowed and custom blocked labels are rendered
+    const labels = Array.from(container.querySelectorAll("label"));
+    const labelTexts = labels.map((label) => label.textContent);
+
+    expect(labelTexts.some((text) => text?.includes("Custom Allowed"))).toBe(true);
+    expect(labelTexts.some((text) => text?.includes("Custom Blocked"))).toBe(true);
+
+    // Verify checkboxes exist (should have 8: 4 presets * 2 checkboxes)
+    const allowCheckboxes = container.querySelectorAll('[id^="allow-"]');
+    const blockCheckboxes = container.querySelectorAll('[id^="block-"]');
+    expect(allowCheckboxes.length).toBe(4);
+    expect(blockCheckboxes.length).toBe(4);
+
+    unmount();
+  });
+  test("mixed state: allowed with preset and custom renders correctly", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    const { container, unmount } = render(
+      <ClientRestrictionsEditor
+        allowed={["claude-code", "my-custom"]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const allowCheckbox = document.getElementById("allow-claude-code");
+    expect(allowCheckbox).not.toBeNull();
+
+    const checkedAttr = allowCheckbox?.getAttribute("data-state");
+    expect(checkedAttr).toBe("checked");
+
+    const customAllowedLabel = Array.from(container.querySelectorAll("label")).find((label) =>
+      label.textContent?.includes("Custom Allowed")
+    );
+    expect(customAllowedLabel).not.toBeNull();
+
+    unmount();
+  });
+  test("disabled prop disables all checkboxes", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    const { container, unmount } = render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        disabled
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const checkboxes = container.querySelectorAll('input[type="checkbox"]');
+    checkboxes.forEach((checkbox) => {
+      expect(checkbox.hasAttribute("disabled")).toBe(true);
+    });
+
+    unmount();
+  });
+
+  test("unchecking Block removes preset from blocked list", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+
+    render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={["factory-cli"]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    const blockCheckbox = document.getElementById("block-factory-cli");
+    expect(blockCheckbox).not.toBeNull();
+
+    act(() => {
+      blockCheckbox?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(onBlockedChange).toHaveBeenCalledWith([]);
+
+    while (document.body.firstChild) {
+      document.body.removeChild(document.body.firstChild);
+    }
+  });
+
+  test("onInvalidTag callback is passed to ArrayTagInputField", () => {
+    const onAllowedChange = vi.fn();
+    const onBlockedChange = vi.fn();
+    const onInvalidTag = vi.fn();
+
+    const { unmount } = render(
+      <ClientRestrictionsEditor
+        allowed={[]}
+        blocked={[]}
+        onAllowedChange={onAllowedChange}
+        onBlockedChange={onBlockedChange}
+        onInvalidTag={onInvalidTag}
+        translations={TEST_TRANSLATIONS}
+      />
+    );
+
+    expect(onInvalidTag).not.toHaveBeenCalled();
+
+    unmount();
+  });
+});

+ 112 - 102
src/components/form/client-restrictions-editor.tsx

@@ -1,77 +1,37 @@
 "use client";
 
 import { useMemo } from "react";
-import { TagInput, type TagInputSuggestion } from "@/components/ui/tag-input";
-import { CLIENT_RESTRICTION_PRESET_OPTIONS } from "@/lib/client-restrictions/client-presets";
+import { ArrayTagInputField } from "@/components/form/form-field";
+import { Checkbox } from "@/components/ui/checkbox";
+import { Label } from "@/components/ui/label";
+import {
+  CLIENT_RESTRICTION_PRESET_OPTIONS,
+  isPresetSelected,
+  mergePresetAndCustomClients,
+  removePresetValues,
+  splitPresetAndCustomClients,
+  togglePresetSelection,
+} from "@/lib/client-restrictions/client-presets";
 import { cn } from "@/lib/utils";
 
-function uniqueOrdered(values: string[]): string[] {
-  const seen = new Set<string>();
-  const result: string[] = [];
-
-  for (const value of values) {
-    const trimmed = value.trim();
-    if (!trimmed) continue;
-    if (seen.has(trimmed)) continue;
-    seen.add(trimmed);
-    result.push(trimmed);
-  }
-
-  return result;
-}
-
-interface ClientRestrictionListEditorProps {
-  label: string;
-  values: string[];
-  placeholder?: string;
-  disabled?: boolean;
-  suggestions: TagInputSuggestion[];
-  onChange: (next: string[]) => void;
-  onInvalidTag?: (tag: string, reason: string) => void;
-  className?: string;
-}
-
-function ClientRestrictionListEditor({
-  label,
-  values,
-  placeholder,
-  disabled,
-  suggestions,
-  onChange,
-  onInvalidTag,
-  className,
-}: ClientRestrictionListEditorProps) {
-  return (
-    <div className={cn("space-y-3", className)}>
-      <div className="text-sm font-medium text-foreground">{label}</div>
-      <TagInput
-        value={values}
-        onChange={onChange}
-        placeholder={placeholder}
-        maxTagLength={64}
-        maxTags={50}
-        disabled={disabled}
-        validateTag={() => true}
-        onInvalidTag={onInvalidTag}
-        suggestions={suggestions}
-      />
-    </div>
-  );
-}
-
 export interface ClientRestrictionsEditorProps {
   allowed: string[];
   blocked: string[];
   onAllowedChange: (next: string[]) => void;
   onBlockedChange: (next: string[]) => void;
-  allowedLabel: string;
-  blockedLabel: string;
-  allowedPlaceholder?: string;
-  blockedPlaceholder?: string;
   disabled?: boolean;
-  getPresetLabel: (presetValue: string) => string;
   onInvalidTag?: (tag: string, reason: string) => void;
   className?: string;
+  translations: {
+    allowAction: string;
+    blockAction: string;
+    customAllowedLabel: string;
+    customAllowedPlaceholder: string;
+    customBlockedLabel: string;
+    customBlockedPlaceholder: string;
+    customHelp: string;
+    presetClients: Record<string, string>;
+  };
 }
 
 export function ClientRestrictionsEditor({
@@ -79,65 +39,115 @@ export function ClientRestrictionsEditor({
   blocked,
   onAllowedChange,
   onBlockedChange,
-  allowedLabel,
-  blockedLabel,
-  allowedPlaceholder,
-  blockedPlaceholder,
   disabled,
-  getPresetLabel,
   onInvalidTag,
   className,
+  translations,
 }: ClientRestrictionsEditorProps) {
-  const suggestions: TagInputSuggestion[] = useMemo(
-    () =>
-      CLIENT_RESTRICTION_PRESET_OPTIONS.map((option) => ({
-        value: option.value,
-        label: getPresetLabel(option.value),
-        keywords: [...option.aliases],
-      })),
-    [getPresetLabel]
+  const { customValues: customAllowed } = useMemo(
+    () => splitPresetAndCustomClients(allowed),
+    [allowed]
+  );
+
+  const { customValues: customBlocked } = useMemo(
+    () => splitPresetAndCustomClients(blocked),
+    [blocked]
   );
 
-  const handleAllowedChange = (next: string[]) => {
-    const nextAllowed = uniqueOrdered(next);
-    onAllowedChange(nextAllowed);
+  const handleAllowToggle = (presetValue: string, checked: boolean) => {
+    onAllowedChange(togglePresetSelection(allowed, presetValue, checked));
+    if (checked) {
+      onBlockedChange(removePresetValues(blocked, presetValue));
+    }
+  };
 
-    const allowedSet = new Set(nextAllowed);
-    const nextBlocked = blocked.filter((value) => !allowedSet.has(value));
-    if (nextBlocked.length !== blocked.length) {
-      onBlockedChange(nextBlocked);
+  const handleBlockToggle = (presetValue: string, checked: boolean) => {
+    onBlockedChange(togglePresetSelection(blocked, presetValue, checked));
+    if (checked) {
+      onAllowedChange(removePresetValues(allowed, presetValue));
     }
   };
 
-  const handleBlockedChange = (next: string[]) => {
-    const nextBlocked = uniqueOrdered(next);
-    onBlockedChange(nextBlocked);
+  const handleCustomAllowedChange = (newCustom: string[]) => {
+    onAllowedChange(mergePresetAndCustomClients(allowed, newCustom));
+  };
 
-    const blockedSet = new Set(nextBlocked);
-    const nextAllowed = allowed.filter((value) => !blockedSet.has(value));
-    if (nextAllowed.length !== allowed.length) {
-      onAllowedChange(nextAllowed);
-    }
+  const handleCustomBlockedChange = (newCustom: string[]) => {
+    onBlockedChange(mergePresetAndCustomClients(blocked, newCustom));
+  };
+
+  const renderPresetRow = (value: string) => {
+    const isAllowed = isPresetSelected(allowed, value);
+    const isBlocked = isPresetSelected(blocked, value);
+    const displayLabel = translations.presetClients[value] ?? value;
+
+    return (
+      <div key={value} className="flex items-center gap-4 py-1">
+        <span className="text-sm flex-1 text-foreground">{displayLabel}</span>
+        <div className="flex items-center gap-3">
+          <div className="flex items-center gap-1.5">
+            <Checkbox
+              id={`allow-${value}`}
+              checked={isAllowed}
+              onCheckedChange={(checked) => handleAllowToggle(value, checked === true)}
+              disabled={disabled}
+            />
+            <Label
+              htmlFor={`allow-${value}`}
+              className="text-xs font-normal cursor-pointer text-muted-foreground"
+            >
+              {translations.allowAction}
+            </Label>
+          </div>
+          <div className="flex items-center gap-1.5">
+            <Checkbox
+              id={`block-${value}`}
+              checked={isBlocked}
+              onCheckedChange={(checked) => handleBlockToggle(value, checked === true)}
+              disabled={disabled}
+            />
+            <Label
+              htmlFor={`block-${value}`}
+              className="text-xs font-normal cursor-pointer text-muted-foreground"
+            >
+              {translations.blockAction}
+            </Label>
+          </div>
+        </div>
+      </div>
+    );
   };
 
   return (
-    <div className={cn("grid grid-cols-1 gap-4 sm:grid-cols-2", className)}>
-      <ClientRestrictionListEditor
-        label={allowedLabel}
-        values={allowed}
-        placeholder={allowedPlaceholder}
+    <div className={cn("space-y-3", className)}>
+      {/* Preset client checkbox rows */}
+      <div className="space-y-0.5 border rounded-md p-2">
+        {CLIENT_RESTRICTION_PRESET_OPTIONS.map((client) => renderPresetRow(client.value))}
+      </div>
+
+      {/* Custom allowed patterns */}
+      <ArrayTagInputField
+        label={translations.customAllowedLabel}
+        description={translations.customHelp}
+        maxTagLength={64}
+        maxTags={50}
+        placeholder={translations.customAllowedPlaceholder}
+        value={customAllowed}
+        onChange={handleCustomAllowedChange}
         disabled={disabled}
-        suggestions={suggestions}
-        onChange={handleAllowedChange}
         onInvalidTag={onInvalidTag}
       />
-      <ClientRestrictionListEditor
-        label={blockedLabel}
-        values={blocked}
-        placeholder={blockedPlaceholder}
+
+      {/* Custom blocked patterns */}
+      <ArrayTagInputField
+        label={translations.customBlockedLabel}
+        description={translations.customHelp}
+        maxTagLength={64}
+        maxTags={50}
+        placeholder={translations.customBlockedPlaceholder}
+        value={customBlocked}
+        onChange={handleCustomBlockedChange}
         disabled={disabled}
-        suggestions={suggestions}
-        onChange={handleBlockedChange}
         onInvalidTag={onInvalidTag}
       />
     </div>