fix(ui): clear pass-through header rows when the create modal is reopened (#37549)

KeyValueInput and QueryParamInput each seeded a private copy of their rows
from the value prop with a one-time useState initializer. The antd form they
were written for hid that: rc-field-form bumps an internal resetCount key on
resetFields, which remounts a Field's children, so the private copy was thrown
away on every reset. react-hook-form's reset does not remount, and the modal is
hidden rather than unmounted, so after Cancel the rows stayed on screen holding
the old values while the form value went back to empty.

The visible cost was a blocked create flow. A leaked header row made the modal
look configured, but the form value behind it was gone, so submitting a fresh
path and target was refused with "Please configure the headers" and no request
was sent. Typing one character into the leaked row put a value back and the
submit went through, which is not something a user can guess.

Both inputs are now controlled off the value prop, which is an array of pairs
rather than a record. A record cannot represent a row whose name is still empty,
which is the reason the private copy existed: two blank rows collapse into one
and a half-typed row disappears as it is typed. With pairs the field value is
the editable shape, the second source of truth is gone, and a form reset clears
the rows like every other field. add_pass_through converts to a record at submit,
so the request payload is unchanged.

Headers now require at least one row with a non-empty name. Previously that was
enforced by accident, because adding a row did not notify the form at all.
This commit is contained in:
yuneng-jiang 2026-08-19 18:44:01 -07:00 committed by GitHub
parent 8922aaab95
commit 2672b36dc3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 215 additions and 64 deletions

View file

@ -34,20 +34,59 @@ const openModal = async (user: User) => {
await screen.findByText("Route Configuration");
};
const fillRequiredFields = async (user: User) => {
fireEvent.change(screen.getByPlaceholderText("bria"), { target: { value: "bria" } });
interface RequiredFieldValues {
path?: string;
target?: string;
headerName?: string;
headerValue?: string;
}
const fillRequiredFields = async (user: User, values: RequiredFieldValues = {}) => {
const {
path = "bria",
target = "https://example.com",
headerName = "Authorization",
headerValue = "Bearer abc",
} = values;
fireEvent.change(screen.getByPlaceholderText("bria"), { target: { value: path } });
fireEvent.change(screen.getByPlaceholderText("https://engine.prod.bria-api.com"), {
target: { value: "https://example.com" },
target: { value: target },
});
await user.click(screen.getByRole("button", { name: /add header/i }));
fireEvent.change(screen.getByPlaceholderText("Header Name"), { target: { value: "Authorization" } });
fireEvent.change(screen.getByPlaceholderText("Header Value"), { target: { value: "Bearer abc" } });
fireEvent.change(screen.getByPlaceholderText("Header Name"), { target: { value: headerName } });
fireEvent.change(screen.getByPlaceholderText("Header Value"), { target: { value: headerValue } });
};
const addQueryParam = async (user: User, name: string, value: string) => {
await user.click(screen.getByRole("button", { name: /add query parameter/i }));
fireEvent.change(screen.getByPlaceholderText("Parameter Name (e.g., version)"), { target: { value: name } });
fireEvent.change(screen.getByPlaceholderText("Parameter Value (e.g., v1)"), { target: { value } });
};
const submit = async (user: User) => user.click(screen.getByRole("button", { name: "Add Pass-Through Endpoint" }));
const lastPayload = () => createPassThroughEndpoint.mock.calls.at(-1)?.[1] as Record<string, unknown>;
const reopenedFields: RequiredFieldValues = {
path: "adobe",
target: "https://adobe.example.com",
headerName: "x-api-key",
headerValue: "second-secret",
};
const reopenedPayload = {
path: "/adobe",
target: "https://adobe.example.com",
headers: { "x-api-key": "second-secret" },
include_subpath: true,
methods: undefined,
default_query_params: undefined,
auth: undefined,
timeout: undefined,
cost_per_request: undefined,
};
describe("add_pass_through submit payload", () => {
beforeEach(() => {
vi.clearAllMocks();
@ -218,4 +257,35 @@ describe("add_pass_through submit payload", () => {
expect(createPassThroughEndpoint).not.toHaveBeenCalled();
});
it("leaves no header or query parameter rows behind when the modal is cancelled and reopened", async () => {
const user = setup();
renderForm();
await openModal(user);
await fillRequiredFields(user);
await addQueryParam(user, "version", "v1");
await user.click(screen.getByRole("button", { name: "Cancel" }));
await openModal(user);
expect(screen.queryByPlaceholderText("Header Name")).not.toBeInTheDocument();
expect(screen.queryByPlaceholderText("Parameter Name (e.g., version)")).not.toBeInTheDocument();
});
it("submits the reopened endpoint instead of rejecting it as missing headers", async () => {
const user = setup();
renderForm();
await openModal(user);
await fillRequiredFields(user);
await user.click(screen.getByRole("button", { name: "Cancel" }));
await openModal(user);
await fillRequiredFields(user, reopenedFields);
await submit(user);
await waitFor(() => expect(createPassThroughEndpoint).toHaveBeenCalled());
expect(screen.queryByText("Please configure the headers")).not.toBeInTheDocument();
expect(lastPayload()).toStrictEqual(reopenedPayload);
});
});

View file

@ -8,7 +8,7 @@ import { z } from "zod/v4";
import { createPassThroughEndpoint } from "./networking";
import NumericalInput from "./shared/numerical_input";
import KeyValueInput from "./key_value_input";
import KeyValueInput, { type KeyValuePair } from "./key_value_input";
import QueryParamInput from "./query_param_input";
import { passThroughItem } from "./PassThroughSettings/PassThroughSettings";
import RoutePreview from "./route_preview";
@ -31,6 +31,8 @@ const HTTP_METHOD_OPTIONS = HTTP_METHODS.map((method) => ({ label: method, value
type GuardrailSettings = Record<string, { request_fields?: string[]; response_fields?: string[] } | null>;
const keyValuePairsSchema = z.array(z.tuple([z.string(), z.string()]));
const passThroughFormSchema = z.object({
path: z.string().min(1, "Path is required").regex(/^\//, "Path is required"),
target: z
@ -39,8 +41,10 @@ const passThroughFormSchema = z.object({
.pipe(z.url({ error: "Please enter a valid URL" })),
methods: z.array(z.string()).optional(),
include_subpath: z.boolean(),
headers: z.record(z.string(), z.string(), { error: "Please configure the headers" }),
default_query_params: z.record(z.string(), z.string()).optional(),
headers: keyValuePairsSchema.refine((pairs) => pairs.some(([name]) => name !== ""), {
error: "Please configure the headers",
}),
default_query_params: keyValuePairsSchema.optional(),
auth: z.boolean().optional(),
timeout: z.string().optional(),
cost_per_request: z.string().optional(),
@ -53,7 +57,7 @@ const emptyFormValues = {
target: "",
methods: undefined,
include_subpath: true,
headers: undefined,
headers: [],
default_query_params: undefined,
auth: undefined,
timeout: undefined,
@ -72,6 +76,14 @@ const labelWithHint = (label: React.ReactNode, hint: string): React.ReactNode =>
const optionalText = (raw: string): string | undefined => (raw === "" ? undefined : raw);
const toRecord = (pairs: readonly KeyValuePair[]): Record<string, string> =>
Object.fromEntries(pairs.filter(([name]) => name !== ""));
const optionalRecord = (pairs: readonly KeyValuePair[] | undefined): Record<string, string> | undefined => {
const record = toRecord(pairs ?? []);
return Object.keys(record).length > 0 ? record : undefined;
};
interface AddFallbacksProps {
accessToken: string;
passThroughItems: passThroughItem[];
@ -109,8 +121,8 @@ const AddPassThroughEndpoint: React.FC<AddFallbacksProps> = ({
target: values.target,
methods: values.methods,
include_subpath: values.include_subpath,
headers: values.headers,
default_query_params: values.default_query_params,
headers: toRecord(values.headers),
default_query_params: optionalRecord(values.default_query_params),
...(premiumUser ? { auth: values.auth } : {}),
timeout: values.timeout,
cost_per_request: values.cost_per_request,

View file

@ -1,12 +1,26 @@
import { useState } from "react";
import { fireEvent, render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import KeyValueInput from "./key_value_input";
import KeyValueInput, { type KeyValuePair } from "./key_value_input";
const ControlledKeyValueInput = ({ onChange }: { onChange?: (value: readonly KeyValuePair[]) => void }) => {
const [value, setValue] = useState<readonly KeyValuePair[]>([]);
return (
<KeyValueInput
value={value}
onChange={(next) => {
setValue(next);
onChange?.(next);
}}
/>
);
};
describe("KeyValueInput", () => {
it("renders existing header pairs", () => {
render(<KeyValueInput value={{ Authorization: "Bearer token" }} />);
render(<KeyValueInput value={[["Authorization", "Bearer token"]]} />);
expect(screen.getByPlaceholderText("Header Name")).toHaveValue("Authorization");
expect(screen.getByPlaceholderText("Header Value")).toHaveValue("Bearer token");
@ -15,12 +29,54 @@ describe("KeyValueInput", () => {
it("adds a header row and emits edits", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(<KeyValueInput onChange={onChange} />);
render(<ControlledKeyValueInput onChange={onChange} />);
await user.click(screen.getByRole("button", { name: /Add Header$/ }));
fireEvent.change(screen.getByPlaceholderText("Header Name"), { target: { value: "X-Trace" } });
fireEvent.change(screen.getByPlaceholderText("Header Value"), { target: { value: "enabled" } });
expect(onChange).toHaveBeenLastCalledWith({ "X-Trace": "enabled" });
expect(onChange).toHaveBeenLastCalledWith([["X-Trace", "enabled"]]);
});
it("renders no rows once the value prop goes back to empty", () => {
const { rerender } = render(<KeyValueInput value={[["Authorization", "Bearer token"]]} />);
expect(screen.getByPlaceholderText("Header Name")).toHaveValue("Authorization");
rerender(<KeyValueInput value={[]} />);
expect(screen.queryByPlaceholderText("Header Name")).not.toBeInTheDocument();
expect(screen.queryByPlaceholderText("Header Value")).not.toBeInTheDocument();
});
it("keeps a half-typed row editable while its name is still empty", async () => {
const user = userEvent.setup();
render(<ControlledKeyValueInput />);
await user.click(screen.getByRole("button", { name: /Add Header$/ }));
await user.click(screen.getByRole("button", { name: /Add Header$/ }));
expect(screen.getAllByPlaceholderText("Header Name")).toHaveLength(2);
fireEvent.change(screen.getAllByPlaceholderText("Header Value")[1], { target: { value: "pending" } });
expect(screen.getAllByPlaceholderText("Header Name")).toHaveLength(2);
expect(screen.getAllByPlaceholderText("Header Value")[1]).toHaveValue("pending");
});
it("removes only the row whose remove button is clicked", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(
<KeyValueInput
value={[
["a", "1"],
["b", "2"],
]}
onChange={onChange}
/>,
);
await user.click(screen.getByRole("button", { name: "Remove header 1" }));
expect(onChange).toHaveBeenLastCalledWith([["b", "2"]]);
});
});

View file

@ -1,40 +1,30 @@
import React, { useState } from "react";
import React from "react";
import { Minus, Plus } from "lucide-react";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
export type KeyValuePair = readonly [string, string];
interface KeyValueInputProps {
value?: Record<string, string>;
onChange?: (value: Record<string, string>) => void;
value?: readonly KeyValuePair[];
onChange?: (value: readonly KeyValuePair[]) => void;
}
const KeyValueInput: React.FC<KeyValueInputProps> = ({ value = {}, onChange }) => {
const [pairs, setPairs] = useState<[string, string][]>(Object.entries(value));
const KeyValueInput: React.FC<KeyValueInputProps> = ({ value = [], onChange }) => {
const handleAdd = () => onChange?.([...value, ["", ""]]);
const handleAdd = () => {
setPairs([...pairs, ["", ""]]);
};
const handleRemove = (index: number) => onChange?.(value.filter((_, i) => i !== index));
const handleRemove = (index: number) => {
const newPairs = pairs.filter((_, i) => i !== index);
setPairs(newPairs);
onChange?.(Object.fromEntries(newPairs));
};
const handleChange = (index: number, key: string, val: string) => {
const newPairs = [...pairs];
newPairs[index] = [key, val];
setPairs(newPairs);
onChange?.(Object.fromEntries(newPairs));
};
const handleChange = (index: number, pair: KeyValuePair) =>
onChange?.(value.map((existing, i) => (i === index ? pair : existing)));
return (
<div className="space-y-2">
{pairs.map(([key, val], index) => (
{value.map(([key, val], index) => (
<div key={index} className="flex items-center gap-2">
<Input placeholder="Header Name" value={key} onChange={(e) => handleChange(index, e.target.value, val)} />
<Input placeholder="Header Value" value={val} onChange={(e) => handleChange(index, key, e.target.value)} />
<Input placeholder="Header Name" value={key} onChange={(e) => handleChange(index, [e.target.value, val])} />
<Input placeholder="Header Value" value={val} onChange={(e) => handleChange(index, [key, e.target.value])} />
<Button
type="button"
variant="ghost"

View file

@ -1,12 +1,27 @@
import { useState } from "react";
import { fireEvent, render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import QueryParamInput from "./query_param_input";
import type { KeyValuePair } from "./key_value_input";
const ControlledQueryParamInput = ({ onChange }: { onChange?: (value: readonly KeyValuePair[]) => void }) => {
const [value, setValue] = useState<readonly KeyValuePair[]>([]);
return (
<QueryParamInput
value={value}
onChange={(next) => {
setValue(next);
onChange?.(next);
}}
/>
);
};
describe("QueryParamInput", () => {
it("renders existing query parameter pairs", () => {
render(<QueryParamInput value={{ version: "v1" }} />);
render(<QueryParamInput value={[["version", "v1"]]} />);
expect(screen.getByPlaceholderText("Parameter Name (e.g., version)")).toHaveValue("version");
expect(screen.getByPlaceholderText("Parameter Value (e.g., v1)")).toHaveValue("v1");
@ -15,12 +30,31 @@ describe("QueryParamInput", () => {
it("adds a query parameter row and emits edits", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(<QueryParamInput onChange={onChange} />);
render(<ControlledQueryParamInput onChange={onChange} />);
await user.click(screen.getByRole("button", { name: /Add Query Parameter$/ }));
fireEvent.change(screen.getByPlaceholderText("Parameter Name (e.g., version)"), { target: { value: "region" } });
fireEvent.change(screen.getByPlaceholderText("Parameter Value (e.g., v1)"), { target: { value: "us-west" } });
expect(onChange).toHaveBeenLastCalledWith({ region: "us-west" });
expect(onChange).toHaveBeenLastCalledWith([["region", "us-west"]]);
});
it("renders no rows once the value prop goes back to empty", () => {
const { rerender } = render(<QueryParamInput value={[["version", "v1"]]} />);
expect(screen.getByPlaceholderText("Parameter Name (e.g., version)")).toHaveValue("version");
rerender(<QueryParamInput value={[]} />);
expect(screen.queryByPlaceholderText("Parameter Name (e.g., version)")).not.toBeInTheDocument();
});
it("keeps a half-typed row editable while its name is still empty", async () => {
const user = userEvent.setup();
render(<ControlledQueryParamInput />);
await user.click(screen.getByRole("button", { name: /Add Query Parameter$/ }));
await user.click(screen.getByRole("button", { name: /Add Query Parameter$/ }));
expect(screen.getAllByPlaceholderText("Parameter Name (e.g., version)")).toHaveLength(2);
});
});

View file

@ -1,47 +1,36 @@
import React, { useState } from "react";
import React from "react";
import { Minus, Plus } from "lucide-react";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import type { KeyValuePair } from "./key_value_input";
interface QueryParamInputProps {
value?: Record<string, string>;
onChange?: (value: Record<string, string>) => void;
value?: readonly KeyValuePair[];
onChange?: (value: readonly KeyValuePair[]) => void;
}
const QueryParamInput: React.FC<QueryParamInputProps> = ({ value = {}, onChange }) => {
const [pairs, setPairs] = useState<[string, string][]>(Object.entries(value));
const QueryParamInput: React.FC<QueryParamInputProps> = ({ value = [], onChange }) => {
const handleAdd = () => onChange?.([...value, ["", ""]]);
const handleAdd = () => {
setPairs([...pairs, ["", ""]]);
};
const handleRemove = (index: number) => onChange?.(value.filter((_, i) => i !== index));
const handleRemove = (index: number) => {
const newPairs = pairs.filter((_, i) => i !== index);
setPairs(newPairs);
onChange?.(Object.fromEntries(newPairs));
};
const handleChange = (index: number, key: string, val: string) => {
const newPairs = [...pairs];
newPairs[index] = [key, val];
setPairs(newPairs);
onChange?.(Object.fromEntries(newPairs));
};
const handleChange = (index: number, pair: KeyValuePair) =>
onChange?.(value.map((existing, i) => (i === index ? pair : existing)));
return (
<div className="space-y-2">
{pairs.map(([key, val], index) => (
{value.map(([key, val], index) => (
<div key={index} className="flex items-center gap-2">
<Input
placeholder="Parameter Name (e.g., version)"
value={key}
onChange={(e) => handleChange(index, e.target.value, val)}
onChange={(e) => handleChange(index, [e.target.value, val])}
/>
<Input
placeholder="Parameter Value (e.g., v1)"
value={val}
onChange={(e) => handleChange(index, key, e.target.value)}
onChange={(e) => handleChange(index, [key, e.target.value])}
/>
<Button
type="button"