feat(ui-v2): add WorkPoolEditForm component for editing work pools by tomerqodo · Pull Request #13 · agentic-review-benchmarks/prefect-combined-copilot

tomerqodo · 2026-01-26T01:30:08Z

Benchmark PR from agentic-review-benchmarks#13

- Create WorkPoolEditForm component with name, description, concurrency limit, and type fields - Name and type fields are disabled/read-only - Description and concurrency limit fields are editable - Add comprehensive test file with 16 test cases - Add Storybook stories with MSW handlers - Update index.ts exports to include new component and useUpdateWorkPool hook - Update route component to use new form Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>

Copilot

Pull request overview

This PR adds a new WorkPoolEditForm component that enables users to edit existing work pools, allowing them to modify the description and concurrency limit fields. The implementation includes comprehensive tests and Storybook stories, following established patterns in the codebase.

Changes:

New WorkPoolEditForm component with form validation and API integration
Integration of the form into the edit route with proper header navigation
Comprehensive test suite covering form interactions, validation, and error scenarios
Storybook stories demonstrating various use cases and work pool types

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ui-v2/src/routes/work-pools/work-pool_.$workPoolName.edit.tsx	Integrates WorkPoolEditForm and WorkPoolEditPageHeader components into the edit route
ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx	Main form component implementing description and concurrency limit editing with validation
ui-v2/src/components/work-pools/edit/work-pool-edit-form.test.tsx	Comprehensive test suite covering form behavior, validation, and API interactions
ui-v2/src/components/work-pools/edit/work-pool-edit-form.stories.tsx	Storybook stories demonstrating various form states and work pool configurations
ui-v2/src/components/work-pools/edit/index.ts	Exports WorkPoolEditForm and related types for external use
ui-v2/src/api/work-pools/index.ts	Adds exports for useUpdateWorkPool hook and WorkPoolUpdate type

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T01:33:55Z

+export const WorkPoolEditForm = ({ workPool }: WorkPoolEditFormProps) => {
+	const router = useRouter();
+	const { updateWorkPool, isPending } = useUpdateWorkPool();
+
+	const form = useForm<WorkPoolEditFormValues>({
+		resolver: zodResolver(workPoolEditSchema),
+		defaultValues: {
+			description: workPool.description ?? "",
+			concurrencyLimit: workPool.concurrency_limit ?? null,
+		},
+	});
+
+	const handleCancel = () => {
+		router.history.back();
+	};
+
+	const handleSubmit = (data: WorkPoolEditFormValues) => {
+		const trimmedDescription = data.description?.trim();
+		updateWorkPool(
+			{
+				name: workPool.name,
+				workPool: {
+					description: trimmedDescription === "" ? null : trimmedDescription,
+					concurrency_limit: data.concurrencyLimit,
+				},
+			},
+			{
+				onSuccess: () => {
+					toast.success("Work pool updated");
+					void router.navigate({
+						to: "/work-pools/work-pool/$workPoolName",
+						params: { workPoolName: workPool.name },
+					});
+				},
+				onError: (error) => {
+					toast.error(
+						`Failed to update work pool: ${error instanceof Error ? error.message : "Unknown error"}`,
+					);
+				},
+			},
+		);
+	};
+
+	return (
+		<Card>
+			<CardContent className="pt-6">
+				<Form {...form}>
+					<form
+						onSubmit={(e) => void form.handleSubmit(handleSubmit)(e)}
+						className="space-y-6"
+					>
+						<div className="space-y-2">
+							<Label htmlFor="work-pool-name">Name</Label>
+							<Input id="work-pool-name" value={workPool.name} disabled />
+						</div>
+
+						<FormField
+							control={form.control}
+							name="description"
+							render={({ field }) => (
+								<FormItem>
+									<FormLabel>Description (Optional)</FormLabel>
+									<FormControl>
+										<Textarea
+											{...field}
+											value={field.value ?? ""}
+											rows={7}
+											placeholder="Enter a description for your work pool"
+										/>
+									</FormControl>
+									<FormMessage />
+								</FormItem>
+							)}
+						/>
+
+						<FormField
+							control={form.control}
+							name="concurrencyLimit"
+							render={({ field }) => (
+								<FormItem>
+									<FormLabel>Flow Run Concurrency (Optional)</FormLabel>
+									<FormControl>
+										<Input
+											{...field}
+											type="number"
+											placeholder="Unlimited"
+											value={field.value ?? ""}
+											onChange={(e) => {
+												const value = e.target.value;
+												field.onChange(value === "" ? null : Number(value));
+											}}
+										/>
+									</FormControl>
+									<FormMessage />
+								</FormItem>
+							)}
+						/>
+
+						<div className="space-y-2">
+							<Label htmlFor="work-pool-type">Type</Label>
+							<Input id="work-pool-type" value={workPool.type} disabled />
+						</div>
+
+						<div className="flex justify-end gap-2">
+							<Button
+								type="button"
+								variant="outline"
+								onClick={handleCancel}
+								disabled={isPending}
+							>
+								Cancel
+							</Button>
+							<Button type="submit" disabled={isPending}>
+								{isPending ? "Saving..." : "Save"}
+							</Button>
+						</div>
+					</form>
+				</Form>
+			</CardContent>
+		</Card>
+	);
+};


The schema includes a baseJobTemplate field that is not used in the form implementation. If this field is intended for future use, consider removing it from the schema until it's actually implemented to avoid confusion. If it's meant to be editable but was overlooked, the form needs to be updated to include this field.

Copilot · 2026-01-26T01:33:56Z

+describe("WorkPoolEditForm", () => {
+	let queryClient: QueryClient;
+
+	beforeEach(() => {
+		queryClient = new QueryClient({
+			defaultOptions: {
+				queries: { retry: false },
+				mutations: { retry: false },
+			},
+		});
+
+		vi.clearAllMocks();
+	});
+
+	const renderWorkPoolEditForm = (
+		workPool = createFakeWorkPool({
+			name: "test-work-pool",
+			description: "Test description",
+			concurrency_limit: 10,
+			type: "process",
+		}),
+	) => {
+		return render(
+			<QueryClientProvider client={queryClient}>
+				<WorkPoolEditForm workPool={workPool} />
+			</QueryClientProvider>,
+		);
+	};
+
+	it("renders all form fields", () => {
+		renderWorkPoolEditForm();
+
+		expect(screen.getByLabelText("Name")).toBeInTheDocument();
+		expect(screen.getByLabelText("Description (Optional)")).toBeInTheDocument();
+		expect(
+			screen.getByLabelText("Flow Run Concurrency (Optional)"),
+		).toBeInTheDocument();
+		expect(screen.getByLabelText("Type")).toBeInTheDocument();
+	});
+
+	it("displays work pool name in disabled field", () => {
+		renderWorkPoolEditForm();
+
+		const nameInput = screen.getByLabelText("Name");
+		expect(nameInput).toHaveValue("test-work-pool");
+		expect(nameInput).toBeDisabled();
+	});
+
+	it("displays work pool type in disabled field", () => {
+		renderWorkPoolEditForm();
+
+		const typeInput = screen.getByLabelText("Type");
+		expect(typeInput).toHaveValue("process");
+		expect(typeInput).toBeDisabled();
+	});
+
+	it("displays description in editable field", () => {
+		renderWorkPoolEditForm();
+
+		const descriptionInput = screen.getByLabelText("Description (Optional)");
+		expect(descriptionInput).toHaveValue("Test description");
+		expect(descriptionInput).not.toBeDisabled();
+	});
+
+	it("displays concurrency limit in editable field", () => {
+		renderWorkPoolEditForm();
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		expect(concurrencyInput).toHaveValue(10);
+		expect(concurrencyInput).not.toBeDisabled();
+	});
+
+	it("allows editing description", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const descriptionInput = screen.getByLabelText("Description (Optional)");
+		await user.clear(descriptionInput);
+		await user.type(descriptionInput, "New description");
+
+		expect(descriptionInput).toHaveValue("New description");
+	});
+
+	it("allows editing concurrency limit", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		await user.clear(concurrencyInput);
+		await user.type(concurrencyInput, "20");
+
+		expect(concurrencyInput).toHaveValue(20);
+	});
+
+	it("handles empty concurrency limit as null", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		await user.clear(concurrencyInput);
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(mockUpdateWorkPool).toHaveBeenCalled();
+			const callArgs = mockUpdateWorkPool.mock.calls[0] as [
+				{ name: string; workPool: { concurrency_limit: number | null } },
+				UpdateWorkPoolOptions,
+			];
+			expect(callArgs[0].workPool.concurrency_limit).toBeNull();
+		});
+	});
+
+	it("calls updateWorkPool on form submission", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(mockUpdateWorkPool).toHaveBeenCalledWith(
+				{
+					name: "test-work-pool",
+					workPool: {
+						description: "Test description",
+						concurrency_limit: 10,
+					},
+				},
+				expect.any(Object),
+			);
+		});
+	});
+
+	it("calls router.history.back on cancel", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const cancelButton = screen.getByRole("button", { name: "Cancel" });
+		await user.click(cancelButton);
+
+		expect(mockHistoryBack).toHaveBeenCalled();
+	});
+
+	it("shows success toast on successful update", async () => {
+		const { toast } = await import("sonner");
+		const user = userEvent.setup();
+
+		mockUpdateWorkPool.mockImplementation((_data, options) => {
+			options.onSuccess?.();
+		});
+
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(toast.success).toHaveBeenCalledWith("Work pool updated");
+		});
+	});
+
+	it("shows error toast on failed update", async () => {
+		const { toast } = await import("sonner");
+		const user = userEvent.setup();
+
+		mockUpdateWorkPool.mockImplementation((_data, options) => {
+			options.onError?.(new Error("Network error"));
+		});
+
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(toast.error).toHaveBeenCalledWith(
+				"Failed to update work pool: Network error",
+			);
+		});
+	});
+
+	it("navigates to work pool page on successful update", async () => {
+		const user = userEvent.setup();
+
+		mockUpdateWorkPool.mockImplementation((_data, options) => {
+			options.onSuccess?.();
+		});
+
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(mockNavigate).toHaveBeenCalledWith({
+				to: "/work-pools/work-pool/$workPoolName",
+				params: { workPoolName: "test-work-pool" },
+			});
+		});
+	});
+
+	it("renders with null description", () => {
+		const workPool = createFakeWorkPool({
+			name: "test-pool",
+			description: null,
+			concurrency_limit: 5,
+			type: "docker",
+		});
+
+		render(
+			<QueryClientProvider client={queryClient}>
+				<WorkPoolEditForm workPool={workPool} />
+			</QueryClientProvider>,
+		);
+
+		const descriptionInput = screen.getByLabelText("Description (Optional)");
+		expect(descriptionInput).toHaveValue("");
+	});
+
+	it("renders with null concurrency limit", () => {
+		const workPool = createFakeWorkPool({
+			name: "test-pool",
+			description: "Test",
+			concurrency_limit: null,
+			type: "kubernetes",
+		});
+
+		render(
+			<QueryClientProvider client={queryClient}>
+				<WorkPoolEditForm workPool={workPool} />
+			</QueryClientProvider>,
+		);
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		expect(concurrencyInput).toHaveValue(null);
+	});
+
+	it("renders Save and Cancel buttons", () => {
+		renderWorkPoolEditForm();
+
+		expect(screen.getByRole("button", { name: "Save" })).toBeInTheDocument();
+		expect(screen.getByRole("button", { name: "Cancel" })).toBeInTheDocument();
+	});
+});


Missing test coverage for the description trimming behavior implemented in lines 42-47 of work-pool-edit-form.tsx. A test should verify that when a user submits a description with leading/trailing whitespace (e.g., " Test description "), it gets trimmed before being sent to the API. Additionally, a test should verify that a description with only whitespace gets converted to null.

Copilot · 2026-01-26T01:33:56Z

+describe("WorkPoolEditForm", () => {
+	let queryClient: QueryClient;
+
+	beforeEach(() => {
+		queryClient = new QueryClient({
+			defaultOptions: {
+				queries: { retry: false },
+				mutations: { retry: false },
+			},
+		});
+
+		vi.clearAllMocks();
+	});
+
+	const renderWorkPoolEditForm = (
+		workPool = createFakeWorkPool({
+			name: "test-work-pool",
+			description: "Test description",
+			concurrency_limit: 10,
+			type: "process",
+		}),
+	) => {
+		return render(
+			<QueryClientProvider client={queryClient}>
+				<WorkPoolEditForm workPool={workPool} />
+			</QueryClientProvider>,
+		);
+	};
+
+	it("renders all form fields", () => {
+		renderWorkPoolEditForm();
+
+		expect(screen.getByLabelText("Name")).toBeInTheDocument();
+		expect(screen.getByLabelText("Description (Optional)")).toBeInTheDocument();
+		expect(
+			screen.getByLabelText("Flow Run Concurrency (Optional)"),
+		).toBeInTheDocument();
+		expect(screen.getByLabelText("Type")).toBeInTheDocument();
+	});
+
+	it("displays work pool name in disabled field", () => {
+		renderWorkPoolEditForm();
+
+		const nameInput = screen.getByLabelText("Name");
+		expect(nameInput).toHaveValue("test-work-pool");
+		expect(nameInput).toBeDisabled();
+	});
+
+	it("displays work pool type in disabled field", () => {
+		renderWorkPoolEditForm();
+
+		const typeInput = screen.getByLabelText("Type");
+		expect(typeInput).toHaveValue("process");
+		expect(typeInput).toBeDisabled();
+	});
+
+	it("displays description in editable field", () => {
+		renderWorkPoolEditForm();
+
+		const descriptionInput = screen.getByLabelText("Description (Optional)");
+		expect(descriptionInput).toHaveValue("Test description");
+		expect(descriptionInput).not.toBeDisabled();
+	});
+
+	it("displays concurrency limit in editable field", () => {
+		renderWorkPoolEditForm();
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		expect(concurrencyInput).toHaveValue(10);
+		expect(concurrencyInput).not.toBeDisabled();
+	});
+
+	it("allows editing description", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const descriptionInput = screen.getByLabelText("Description (Optional)");
+		await user.clear(descriptionInput);
+		await user.type(descriptionInput, "New description");
+
+		expect(descriptionInput).toHaveValue("New description");
+	});
+
+	it("allows editing concurrency limit", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		await user.clear(concurrencyInput);
+		await user.type(concurrencyInput, "20");
+
+		expect(concurrencyInput).toHaveValue(20);
+	});
+
+	it("handles empty concurrency limit as null", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		await user.clear(concurrencyInput);
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(mockUpdateWorkPool).toHaveBeenCalled();
+			const callArgs = mockUpdateWorkPool.mock.calls[0] as [
+				{ name: string; workPool: { concurrency_limit: number | null } },
+				UpdateWorkPoolOptions,
+			];
+			expect(callArgs[0].workPool.concurrency_limit).toBeNull();
+		});
+	});
+
+	it("calls updateWorkPool on form submission", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(mockUpdateWorkPool).toHaveBeenCalledWith(
+				{
+					name: "test-work-pool",
+					workPool: {
+						description: "Test description",
+						concurrency_limit: 10,
+					},
+				},
+				expect.any(Object),
+			);
+		});
+	});
+
+	it("calls router.history.back on cancel", async () => {
+		const user = userEvent.setup();
+		renderWorkPoolEditForm();
+
+		const cancelButton = screen.getByRole("button", { name: "Cancel" });
+		await user.click(cancelButton);
+
+		expect(mockHistoryBack).toHaveBeenCalled();
+	});
+
+	it("shows success toast on successful update", async () => {
+		const { toast } = await import("sonner");
+		const user = userEvent.setup();
+
+		mockUpdateWorkPool.mockImplementation((_data, options) => {
+			options.onSuccess?.();
+		});
+
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(toast.success).toHaveBeenCalledWith("Work pool updated");
+		});
+	});
+
+	it("shows error toast on failed update", async () => {
+		const { toast } = await import("sonner");
+		const user = userEvent.setup();
+
+		mockUpdateWorkPool.mockImplementation((_data, options) => {
+			options.onError?.(new Error("Network error"));
+		});
+
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(toast.error).toHaveBeenCalledWith(
+				"Failed to update work pool: Network error",
+			);
+		});
+	});
+
+	it("navigates to work pool page on successful update", async () => {
+		const user = userEvent.setup();
+
+		mockUpdateWorkPool.mockImplementation((_data, options) => {
+			options.onSuccess?.();
+		});
+
+		renderWorkPoolEditForm();
+
+		const saveButton = screen.getByRole("button", { name: "Save" });
+		await user.click(saveButton);
+
+		await waitFor(() => {
+			expect(mockNavigate).toHaveBeenCalledWith({
+				to: "/work-pools/work-pool/$workPoolName",
+				params: { workPoolName: "test-work-pool" },
+			});
+		});
+	});
+
+	it("renders with null description", () => {
+		const workPool = createFakeWorkPool({
+			name: "test-pool",
+			description: null,
+			concurrency_limit: 5,
+			type: "docker",
+		});
+
+		render(
+			<QueryClientProvider client={queryClient}>
+				<WorkPoolEditForm workPool={workPool} />
+			</QueryClientProvider>,
+		);
+
+		const descriptionInput = screen.getByLabelText("Description (Optional)");
+		expect(descriptionInput).toHaveValue("");
+	});
+
+	it("renders with null concurrency limit", () => {
+		const workPool = createFakeWorkPool({
+			name: "test-pool",
+			description: "Test",
+			concurrency_limit: null,
+			type: "kubernetes",
+		});
+
+		render(
+			<QueryClientProvider client={queryClient}>
+				<WorkPoolEditForm workPool={workPool} />
+			</QueryClientProvider>,
+		);
+
+		const concurrencyInput = screen.getByLabelText(
+			"Flow Run Concurrency (Optional)",
+		);
+		expect(concurrencyInput).toHaveValue(null);
+	});
+
+	it("renders Save and Cancel buttons", () => {
+		renderWorkPoolEditForm();
+
+		expect(screen.getByRole("button", { name: "Save" })).toBeInTheDocument();
+		expect(screen.getByRole("button", { name: "Cancel" })).toBeInTheDocument();
+	});
+});


Missing test coverage for negative concurrency limit values. The schema enforces min(0) validation (schema.ts:8), but there's no test verifying that negative values are rejected or that the appropriate validation error is displayed to the user.

Copilot · 2026-01-26T01:33:56Z

+										<Input
+											{...field}
+											type="number"
+											placeholder="Unlimited"
+											value={field.value ?? ""}
+											onChange={(e) => {
+												const value = e.target.value;
+												field.onChange(value === "" ? null : Number(value));
+											}}
+										/>


The concurrency limit input should include a min={0} attribute for consistency with the create form (ui-v2/src/components/work-pools/create/information-step/information-step.tsx:61) and to match the schema validation which requires min(0). This ensures the HTML5 validation aligns with the Zod schema validation and provides better user experience by preventing negative numbers from being entered.

devin-ai-integration Bot and others added 2 commits January 25, 2026 12:11

update pr

fabb667

Copilot AI review requested due to automatic review settings January 26, 2026 01:30

Copilot started reviewing on behalf of tomerqodo January 26, 2026 01:30 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(ui-v2): add WorkPoolEditForm component for editing work pools#13

feat(ui-v2): add WorkPoolEditForm component for editing work pools#13
tomerqodo wants to merge 2 commits into
copilot_full_base_featui-v2_add_workpooleditform_component_for_editing_work_pools_pr13from
copilot_full_head_featui-v2_add_workpooleditform_component_for_editing_work_pools_pr13

tomerqodo commented Jan 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tomerqodo commented Jan 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants