feat(ui-v2): add WorkPoolEditForm component for editing work pools#39
Conversation
- 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>
WalkthroughAdds WorkPoolEditForm component for editing work pools with react-hook-form and zodResolver. Includes test suite with mocks and Storybook stories featuring MSW handlers. Exports API hooks, types, and form components. Updates the work pool edit route to use the new component. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Form as WorkPoolEditForm
participant Hook as useUpdateWorkPool
participant API as Backend API
participant Nav as Router/Toast
User->>Form: Fill and submit form
Form->>Form: Validate using zodResolver
alt Validation Failed
Form->>User: Show validation errors
else Validation Passed
Form->>Hook: Call updateWorkPool(payload)
Hook->>API: PATCH /work_pools/{name}
alt Success (204)
API-->>Hook: Success response
Hook->>Nav: Show success toast
Hook->>Nav: Navigate to work pool page
Nav-->>User: Display updated pool
else Error (404)
API-->>Hook: Error response
Hook->>Nav: Show error toast
Nav-->>User: Display error message
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx`:
- Line 5: Remove the unused type import WorkPoolUpdate from the import statement
that currently reads import { useUpdateWorkPool, type WorkPool, type
WorkPoolUpdate } from "@/api/work-pools"; — keep useUpdateWorkPool and type
WorkPool only (or add any other used symbols) so the file no longer imports
WorkPoolUpdate which is never referenced.
🧹 Nitpick comments (3)
ui-v2/src/components/work-pools/edit/work-pool-edit-form.test.tsx (1)
197-233: Consider importingtoastat the top level.The dynamic import of
toastinside test functions works but is unconventional. Since the mock is hoisted, you can import the mockedtoastat the top of the file.♻️ Suggested improvement
Add at the top of the file with other imports:
import { toast } from "sonner";Then update the tests to use it directly:
it("shows success toast on successful update", async () => { - const { toast } = await import("sonner"); const user = userEvent.setup(); // ... rest of test });ui-v2/src/components/work-pools/edit/work-pool-edit-form.stories.tsx (1)
32-40: Consider extracting the common MSW handler to reduce duplication.The same MSW handler for successful PATCH responses is repeated in 7 of 8 stories. Extracting it to a shared constant would improve maintainability.
♻️ Suggested improvement
const successHandler = http.patch( "http://localhost:4200/api/work_pools/:name", () => new HttpResponse(null, { status: 204 }) ); // Then in each story: export const Default: Story = { args: { /* ... */ }, parameters: { msw: { handlers: [successHandler] }, }, };ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx (1)
107-116: Addmin="0"to the number input for better UX feedback.The schema already validates non-negative concurrency limits via
z.number().min(0), but adding the HTMLminattribute provides immediate client-side feedback without waiting for form submission.♻️ Proposed fix
<Input {...field} type="number" + min="0" placeholder="Unlimited" value={field.value ?? ""} onChange={(e) => { const value = e.target.value; field.onChange(value === "" ? null : Number(value)); }} />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ui-v2/src/api/work-pools/index.tsui-v2/src/components/work-pools/edit/index.tsui-v2/src/components/work-pools/edit/work-pool-edit-form.stories.tsxui-v2/src/components/work-pools/edit/work-pool-edit-form.test.tsxui-v2/src/components/work-pools/edit/work-pool-edit-form.tsxui-v2/src/routes/work-pools/work-pool_.$workPoolName.edit.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx (7)
ui-v2/src/api/work-pools/index.ts (2)
WorkPool(12-12)useUpdateWorkPool(11-11)ui-v2/src/components/work-pools/edit/index.ts (1)
WorkPoolEditForm(2-2)ui-v2/src/components/ui/card.tsx (2)
Card(69-69)CardContent(74-74)ui-v2/src/components/ui/form/form.tsx (6)
Form(132-132)FormField(138-138)FormItem(133-133)FormLabel(134-134)FormControl(135-135)FormMessage(137-137)ui-v2/src/components/ui/input.tsx (1)
Input(81-81)ui-v2/src/components/ui/textarea.tsx (1)
Textarea(18-18)ui-v2/src/components/ui/button/button.tsx (1)
Button(49-49)
ui-v2/src/components/work-pools/edit/work-pool-edit-form.test.tsx (3)
ui-v2/src/router.tsx (1)
queryClient(6-6)ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx (1)
WorkPoolEditForm(25-146)ui-v2/src/components/work-pools/edit/index.ts (1)
WorkPoolEditForm(2-2)
ui-v2/src/components/work-pools/edit/work-pool-edit-form.stories.tsx (2)
ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx (1)
WorkPoolEditForm(25-146)ui-v2/src/components/work-pools/edit/index.ts (1)
WorkPoolEditForm(2-2)
🪛 GitHub Actions: UI v2 Checks
ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx
[warning] 1-1: Unused imports might be the result of an incomplete refactoring.
[warning] 1-1: Unsafe fix: Remove the unused imports.
[error] 1-1: File content differs from formatting output. Run the formatter to align the file with the project's formatting rules.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit checks
🔇 Additional comments (12)
ui-v2/src/api/work-pools/index.ts (1)
11-17: LGTM!The new exports for
useUpdateWorkPoolandWorkPoolUpdateare correctly added and maintain alphabetical ordering consistent with the existing export structure.ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx (3)
29-35: LGTM!The form initialization correctly handles nullable fields with appropriate fallback values.
41-66: LGTM!The submit handler properly trims the description, converts empty strings to null, and has comprehensive success/error handling with appropriate user feedback.
68-145: LGTM!The form JSX structure is well-organized with proper accessibility labels, disabled states during submission, and clear visual feedback for the pending state.
ui-v2/src/components/work-pools/edit/work-pool-edit-form.test.tsx (3)
8-44: LGTM!The mock setup is comprehensive, correctly preserving actual exports while overriding only the necessary functions for testing.
46-73: LGTM!Good test isolation with fresh QueryClient per test and a clean helper function for rendering.
75-298: LGTM!Excellent test coverage including edge cases for null values, user interactions, API calls, navigation, and toast notifications.
ui-v2/src/components/work-pools/edit/index.ts (1)
1-3: LGTM!Clean barrel exports following the established pattern, making the form component and its types easily importable from a single location.
ui-v2/src/routes/work-pools/work-pool_.$workPoolName.edit.tsx (2)
4-7: LGTM!Clean import of the new edit components from the barrel file.
28-33: LGTM!The route component properly composes the header and form, passing the work pool data to both components with a clean flex layout.
ui-v2/src/components/work-pools/edit/work-pool-edit-form.stories.tsx (2)
11-21: LGTM!Proper Storybook meta configuration with all necessary decorators for the component's dependencies.
124-145: LGTM!Good inclusion of an error scenario story to demonstrate error handling behavior in Storybook.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| import { useRouter } from "@tanstack/react-router"; | ||
| import { useForm } from "react-hook-form"; | ||
| import { toast } from "sonner"; | ||
| import { useUpdateWorkPool, type WorkPool, type WorkPoolUpdate } from "@/api/work-pools"; |
There was a problem hiding this comment.
Remove unused import WorkPoolUpdate.
The pipeline indicates an unused import warning. The WorkPoolUpdate type is imported but never used in this file.
🔧 Proposed fix
-import { useUpdateWorkPool, type WorkPool, type WorkPoolUpdate } from "@/api/work-pools";
+import { useUpdateWorkPool, type WorkPool } from "@/api/work-pools";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { useUpdateWorkPool, type WorkPool, type WorkPoolUpdate } from "@/api/work-pools"; | |
| import { useUpdateWorkPool, type WorkPool } from "@/api/work-pools"; |
🤖 Prompt for AI Agents
In `@ui-v2/src/components/work-pools/edit/work-pool-edit-form.tsx` at line 5,
Remove the unused type import WorkPoolUpdate from the import statement that
currently reads import { useUpdateWorkPool, type WorkPool, type WorkPoolUpdate }
from "@/api/work-pools"; — keep useUpdateWorkPool and type WorkPool only (or add
any other used symbols) so the file no longer imports WorkPoolUpdate which is
never referenced.
Benchmark PR from agentic-review-benchmarks#13
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.