feat(ui-v2): Add trigger form templates - #10
Conversation
…ool-status, work-queue-status, and custom triggers - Create DeploymentStatusTriggerFields component with status select - Create WorkPoolStatusTriggerFields component with status select - Create WorkQueueStatusTriggerFields component with status select - Create CustomTriggerFields component with textarea for expected events - Update TriggerStep to use new components instead of placeholders - Export all new components from index.ts All components follow the same pattern as FlowRunStateTriggerFields with: - PostureSelect for Reactive/Proactive toggle - Threshold input field - Conditional Within field (shown only for Proactive posture) Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
…mponents Update tests to check for actual component rendering instead of placeholder text now that the trigger form templates are implemented. Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
📝 WalkthroughWalkthroughThis PR introduces four new React components for automation trigger configuration fields (custom, deployment status, work pool status, work queue status) that replace placeholder text in the trigger wizard. These components are exported via module index, integrated into the trigger-step workflow, tested for visibility, and TypeScript strict mode is disabled in the build configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx`:
- Around line 37-40: The onChange handler for the textarea currently does const
lines = e.target.value.split("\n"); and then field.onChange(lines.length > 0 ?
lines : undefined), which treats an empty textarea as [""]; update the handler
to split and then filter out empty/whitespace-only lines (e.g., trim each line
and filter(Boolean) or filter(l => l.trim() !== "")), and call
field.onChange(filteredLines.length ? filteredLines : undefined) so clearing the
textarea sets the field to undefined (or an empty array if you prefer) rather
than [""]; reference the onChange prop, e.target.value, the local variable
lines, and field.onChange to locate the change.
In `@ui-v2/tsconfig.app.json`:
- Line 19: The tsconfig change turned off TypeScript strict mode by setting the
"strict" compiler flag to false, which weakens type safety; revert "strict" back
to true in the tsconfig and fix the resulting type errors in the new trigger
field components instead of relaxing compiler checks globally—locate uses of the
"strict" flag and the new trigger field components (the recently added
TriggerField* components) and address missing/any types, nullable checks, and
incorrect function signatures (e.g., add precise types, use optional chaining,
tighten return types) so the codebase compiles under strict mode.
🧹 Nitpick comments (3)
ui-v2/src/components/automations/automations-wizard/trigger-step/work-pool-status-trigger-fields.tsx (2)
78-78: Inconsistent number parsing:parseIntvsNumber.This component uses
parseInt(e.target.value)for the threshold field, while other components (deployment-status, work-queue-status, custom) useNumber(e.target.value). For consistency and to handle edge cases uniformly, consider usingNumber()here as well.Proposed fix
-onChange={(e) => field.onChange(parseInt(e.target.value))} +onChange={(e) => field.onChange(Number(e.target.value))}
26-109: Consider extracting shared logic to reduce duplication.
WorkPoolStatusTriggerFieldsandWorkQueueStatusTriggerFieldsare nearly identical, differing only in the status options and event prefixes. While acceptable for the initial implementation, consider extracting a sharedStatusTriggerFieldscomponent that accepts status options as a prop to reduce future maintenance burden.ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.test.tsx (1)
45-56: Use case-insensitive regex matchers for label assertions to improve test resilience.These assertions rely on exact string matching for labels. Any capitalization change (e.g., "Select posture" instead of "select posture") will break the tests. React Testing Library supports case-insensitive RegExp matchers, which reduces brittleness and is the recommended pattern for stable assertions. Apply this across all similar template tests in this file:
- expect(screen.getByLabelText("select posture")).toBeVisible(); - expect(screen.getByLabelText("Threshold")).toBeVisible(); + expect(screen.getByLabelText(/select posture/i)).toBeVisible(); + expect(screen.getByLabelText(/threshold/i)).toBeVisible();
| onChange={(e) => { | ||
| const lines = e.target.value.split("\n"); | ||
| field.onChange(lines.length > 0 ? lines : undefined); | ||
| }} |
There was a problem hiding this comment.
Empty textarea results in [""] instead of empty array or undefined.
"".split("\n") returns [""] (an array with one empty string), so the condition lines.length > 0 is always true. This means clearing the textarea sets the field to [""] rather than undefined or [].
Consider filtering out empty lines:
Proposed fix
onChange={(e) => {
- const lines = e.target.value.split("\n");
- field.onChange(lines.length > 0 ? lines : undefined);
+ const lines = e.target.value.split("\n").filter(line => line.trim());
+ field.onChange(lines.length > 0 ? lines : undefined);
}}📝 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.
| onChange={(e) => { | |
| const lines = e.target.value.split("\n"); | |
| field.onChange(lines.length > 0 ? lines : undefined); | |
| }} | |
| onChange={(e) => { | |
| const lines = e.target.value.split("\n").filter(line => line.trim()); | |
| field.onChange(lines.length > 0 ? lines : undefined); | |
| }} |
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx`
around lines 37 - 40, The onChange handler for the textarea currently does const
lines = e.target.value.split("\n"); and then field.onChange(lines.length > 0 ?
lines : undefined), which treats an empty textarea as [""]; update the handler
to split and then filter out empty/whitespace-only lines (e.g., trim each line
and filter(Boolean) or filter(l => l.trim() !== "")), and call
field.onChange(filteredLines.length ? filteredLines : undefined) so clearing the
textarea sets the field to undefined (or an empty array if you prefer) rather
than [""]; reference the onChange prop, e.target.value, the local variable
lines, and field.onChange to locate the change.
| "noEmit": true, | ||
| "jsx": "react-jsx", | ||
| "strict": true, | ||
| "strict": false, |
There was a problem hiding this comment.
Disabling TypeScript strict mode weakens type safety for the entire application.
Setting "strict": false disables critical type-checking features like noImplicitAny, strictNullChecks, and strictFunctionTypes. This is a significant regression that can mask bugs and reduce code quality across the entire ui-v2 codebase.
The new trigger field components should be written to comply with strict mode rather than relaxing compiler settings globally. Consider keeping "strict": true and addressing any type errors in the new components directly.
🤖 Prompt for AI Agents
In `@ui-v2/tsconfig.app.json` at line 19, The tsconfig change turned off
TypeScript strict mode by setting the "strict" compiler flag to false, which
weakens type safety; revert "strict" back to true in the tsconfig and fix the
resulting type errors in the new trigger field components instead of relaxing
compiler checks globally—locate uses of the "strict" flag and the new trigger
field components (the recently added TriggerField* components) and address
missing/any types, nullable checks, and incorrect function signatures (e.g., add
precise types, use optional chaining, tighten return types) so the codebase
compiles under strict mode.
1 participant
Benchmark PR from agentic-review-benchmarks#10
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.