feat(ui-v2): Add trigger form templates#36
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>
WalkthroughFour new React components for trigger field configuration in an Automation Wizard replace placeholder text with functional form fields (CustomTriggerFields, DeploymentStatusTriggerFields, WorkPoolStatusTriggerFields, WorkQueueStatusTriggerFields). These components use react-hook-form context for binding. The trigger-step component integrates them, and tests are updated accordingly. TypeScript strict mode is disabled in 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
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: 3
🤖 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 sets field.onChange(lines.length > 0 ?
lines : undefined), but "" splits to [""] so an empty textarea stores [""];
change the logic in the onChange callback (the handler that calls
field.onChange) to detect empty input or strip empty lines before setting the
field: e.g. check if e.target.value === "" (set undefined) or use
lines.filter(Boolean) and then call field.onChange(filtered.length ? filtered :
undefined) so blank textarea clears the field instead of storing [""].
In
`@ui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsx`:
- Around line 29-31: The component currently switches statusFieldName based on
posture (posture === "Proactive" ? "trigger.after" : "trigger.expect") but
leaves the alternate field populated; add logic to clear the unused field when
posture changes—locate the place where statusFieldName is computed in
deployment-status-trigger-fields (or the component render) and add an effect or
change handler that calls the form API (e.g., resetField/setValue) to clear the
opposite key ("trigger.expect" or "trigger.after") whenever posture changes;
alternatively, add cleanup in the form submission handler to drop the unused
field based on posture so only the relevant trigger field is submitted.
In `@ui-v2/tsconfig.app.json`:
- Line 19: Change the project-wide TypeScript setting back to strict mode by
setting "strict": true in tsconfig.app.json, then fix the type errors surfaced
in the new trigger field components (the trigger field components mentioned in
the PR) rather than disabling strictness; for localized workarounds use targeted
// `@ts-expect-error` comments or explicit type assertions in the specific
component files and adjust function/prop types (e.g., component props, event
handlers, and any helper functions used by those trigger field components) until
the project compiles cleanly under strict mode.
🧹 Nitpick comments (3)
ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx (1)
58-63: Consider handling empty/invalid numeric input.
Number("")returns0andNumber("abc")returnsNaN. If the user clears the input or enters invalid text, this could set unexpected values. Consider validating or falling back gracefully.Proposed defensive handling
<Input type="number" min={1} {...field} - onChange={(e) => field.onChange(Number(e.target.value))} + onChange={(e) => { + const val = e.target.value; + field.onChange(val === "" ? undefined : Number(val)); + }} />ui-v2/src/components/automations/automations-wizard/trigger-step/work-pool-status-trigger-fields.tsx (1)
74-79: Inconsistent numeric parsing:parseIntvsNumber.Line 78 uses
parseInt(e.target.value)for threshold, while Line 98 (and other trigger field components) useNumber(e.target.value). UseNumber()consistently for predictable behavior, especially with empty strings (parseInt("")→NaN,Number("")→0).Proposed fix
<Input type="number" min={1} {...field} - onChange={(e) => field.onChange(parseInt(e.target.value))} + onChange={(e) => field.onChange(Number(e.target.value))} />ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.test.tsx (1)
45-56: Consider adding test coverage for Proactive posture behavior.The test verifies Reactive posture fields are visible, but doesn't test the conditional "Within" field that appears when posture is set to "Proactive". Based on the component docstrings, this is a key behavioral difference worth testing.
💡 Example additional test case
it("shows Within field when Proactive posture is selected for deployment-status", async () => { const user = userEvent.setup(); render(<TriggerStepFormContainer />); await user.click(screen.getByLabelText("Trigger Template")); await user.click(screen.getByRole("option", { name: "Deployment status" })); // Select Proactive posture await user.click(screen.getByLabelText("select posture")); await user.click(screen.getByRole("option", { name: "Proactive" })); expect(screen.getByLabelText("Within")).toBeVisible(); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsxui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsxui-v2/src/components/automations/automations-wizard/trigger-step/index.tsui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.test.tsxui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.tsxui-v2/src/components/automations/automations-wizard/trigger-step/work-pool-status-trigger-fields.tsxui-v2/src/components/automations/automations-wizard/trigger-step/work-queue-status-trigger-fields.tsxui-v2/tsconfig.app.json
🧰 Additional context used
🧬 Code graph analysis (4)
ui-v2/src/components/automations/automations-wizard/trigger-step/work-queue-status-trigger-fields.tsx (4)
ui-v2/src/components/automations/automations-wizard/trigger-step/index.ts (2)
WorkQueueStatusTriggerFields(9-9)PostureSelect(4-4)ui-v2/src/components/ui/form/form.tsx (5)
FormField(138-138)FormItem(133-133)FormLabel(134-134)FormControl(135-135)FormMessage(137-137)ui-v2/src/components/ui/select.tsx (5)
Select(169-169)SelectTrigger(177-177)SelectValue(178-178)SelectContent(170-170)SelectItem(172-172)ui-v2/src/components/ui/input.tsx (1)
Input(84-84)
ui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsx (4)
ui-v2/src/components/automations/automations-wizard/trigger-step/index.ts (2)
DeploymentStatusTriggerFields(2-2)PostureSelect(4-4)ui-v2/src/components/ui/form/form.tsx (5)
FormField(138-138)FormItem(133-133)FormLabel(134-134)FormControl(135-135)FormMessage(137-137)ui-v2/src/components/ui/select.tsx (5)
Select(169-169)SelectTrigger(177-177)SelectValue(178-178)SelectContent(170-170)SelectItem(172-172)ui-v2/src/components/ui/input.tsx (1)
Input(84-84)
ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx (4)
ui-v2/src/components/automations/automations-wizard/trigger-step/index.ts (2)
CustomTriggerFields(1-1)PostureSelect(4-4)ui-v2/src/components/ui/form/form.tsx (5)
FormField(138-138)FormItem(133-133)FormLabel(134-134)FormControl(135-135)FormMessage(137-137)ui-v2/src/components/ui/textarea.tsx (1)
Textarea(18-18)ui-v2/src/components/ui/input.tsx (1)
Input(84-84)
ui-v2/src/components/automations/automations-wizard/trigger-step/work-pool-status-trigger-fields.tsx (4)
ui-v2/src/components/automations/automations-wizard/trigger-step/index.ts (1)
WorkPoolStatusTriggerFields(8-8)ui-v2/src/components/ui/form/form.tsx (5)
FormField(138-138)FormItem(133-133)FormLabel(134-134)FormControl(135-135)FormMessage(137-137)ui-v2/src/components/ui/select.tsx (5)
Select(169-169)SelectTrigger(177-177)SelectValue(178-178)SelectContent(170-170)SelectItem(172-172)ui-v2/src/components/ui/input.tsx (1)
Input(84-84)
⏰ 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: Test
🔇 Additional comments (9)
ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx (1)
14-23: Component structure follows established patterns.The use of
useFormContext,useWatch, and the conditional rendering based on posture aligns with the existingFlowRunStateTriggerFieldspattern mentioned in the PR objectives.ui-v2/src/components/automations/automations-wizard/trigger-step/work-queue-status-trigger-fields.tsx (2)
34-63: Verify: Status field always usestrigger.expectregardless of posture.
DeploymentStatusTriggerFieldsswitches betweentrigger.expectandtrigger.afterbased on posture, but this component always usestrigger.expect. Confirm this is intentional for work queue triggers, or align the behavior if needed.
26-109: Component structure is consistent with sibling trigger field components.The layout, form bindings, and conditional Within field rendering follow the established pattern.
ui-v2/src/components/automations/automations-wizard/trigger-step/work-pool-status-trigger-fields.tsx (1)
34-63: Same concern: Status field always usestrigger.expectregardless of posture.Like
WorkQueueStatusTriggerFields, this differs fromDeploymentStatusTriggerFieldswhich switches totrigger.afterfor Proactive posture. Verify this is the intended behavior for work pool triggers.ui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsx (1)
25-112: Overall component structure is well-organized.The conditional logic for posture-based field selection and the form bindings are implemented correctly.
ui-v2/src/components/automations/automations-wizard/trigger-step/index.ts (1)
1-9: New trigger field components are properly exported.All four new components are correctly re-exported, making them available from the trigger-step module.
ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.test.tsx (1)
97-111: LGTM!The custom template test correctly verifies the additional "Expected Events (one per line)" field that differentiates it from other trigger templates. Good coverage of the unique behavior.
ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.tsx (2)
8-13: LGTM!Clean imports following the established pattern with FlowRunStateTriggerFields. Relative imports from the same directory are appropriate.
39-54: LGTM!The switch statement is well-structured and covers all trigger templates with a sensible
nulldefault. The pattern aligns with the existing FlowRunStateTriggerFields implementation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| 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 incorrectly stores [""] instead of clearing the field.
"".split("\n") returns [""] (length 1), so the condition lines.length > 0 is always true. An empty textarea will store [""] rather than undefined or [].
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(Boolean);
+ 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 sets field.onChange(lines.length > 0 ?
lines : undefined), but "" splits to [""] so an empty textarea stores [""];
change the logic in the onChange callback (the handler that calls
field.onChange) to detect empty input or strip empty lines before setting the
field: e.g. check if e.target.value === "" (set undefined) or use
lines.filter(Boolean) and then call field.onChange(filtered.length ? filtered :
undefined) so blank textarea clears the field instead of storing [""].
| // Determine which field to use based on posture | ||
| const statusFieldName = | ||
| posture === "Proactive" ? "trigger.after" : "trigger.expect"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd ui-v2 && find . -type f -name "*.tsx" -o -name "*.ts" | head -20Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 972
🏁 Script executed:
rg -n "trigger\.(expect|after)" --type=ts --type=tsx -C 3Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 113
🏁 Script executed:
cat -n src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsxRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 213
🏁 Script executed:
# Search for form submission and schema validation logic
rg -n "onSubmit\|schema\|validation" src/components/automations/automations-wizard/trigger-step/ --type=ts --type=tsx -C 2Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 113
🏁 Script executed:
# Look for where the form data is processed/submitted
rg -n "trigger\." src/components/automations/automations-wizard/ --type=ts --type=tsx | grep -i "submit\|reset\|clear\|handle" | head -20Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 113
🏁 Script executed:
find . -name "deployment-status-trigger-fields.tsx" -o -name "deployment-status-trigger-fields.ts"Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 189
🏁 Script executed:
rg -n "trigger\.(expect|after)" -C 3Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 16318
🏁 Script executed:
find . -path "*/automations-wizard/trigger-step/*" -type f | head -20Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 1411
🏁 Script executed:
find . -name "*schema*" -o -name "*validation*" | grep -i automation | head -20Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 424
🏁 Script executed:
cat -n ui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsxRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 3768
🏁 Script executed:
cat -n ui-v2/src/components/automations/automations-wizard/automation-schema.tsRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 6118
🏁 Script executed:
# Look at how form submission is handled
rg -n "onSubmit\|handleSubmit" ui-v2/src/components/automations/automations-wizard/trigger-step/ -C 3Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
🏁 Script executed:
# Check flow-run-state-trigger-fields for comparison - similar dynamic field pattern
cat -n ui-v2/src/components/automations/automations-wizard/trigger-step/flow-run-state-trigger-fields.tsxRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 3796
🏁 Script executed:
# Check the trigger step parent component to see how form is submitted
cat -n ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.tsxRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 2335
🏁 Script executed:
# Search for any form submission or validation logic that might handle cleanup
rg -n "FormProvider\|useForm\|handleSubmit" ui-v2/src/components/automations/automations-wizard/ -A 5 -B 2Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
🏁 Script executed:
# Check if there are any tests for this component to understand expected behavior
cat -n ui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.test.tsx 2>/dev/null || echo "No test file found"Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 104
🏁 Script executed:
# Look at how the form data is sent to the backend
rg -n "JSON\|stringify\|submit\|mutation" ui-v2/src/components/automations/automations-wizard/ --type=ts -C 2Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
🏁 Script executed:
# Look for the main automation wizard form or submission logic
rg -n "useForm\|FormProvider" ui-v2/src/components/automations/automations-wizard/ --type=ts -C 5Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
🏁 Script executed:
# Check for any form reset or cleanup logic when template changes
cat -n ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step-utils.tsRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 1582
🏁 Script executed:
# Look at test files to understand expected behavior
cat -n ui-v2/src/components/automations/automations-wizard/trigger-step/trigger-step.test.tsx 2>/dev/null | head -100 || echo "Checking for test files..."Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 4041
🏁 Script executed:
# Find the main automation wizard component
find ui-v2 -name "*automation*wizard*.tsx" -type f | grep -v node_modulesRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 241
🏁 Script executed:
cat -n ui-v2/src/components/automations/automations-wizard/automation-wizard.tsxRepository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 3754
🏁 Script executed:
# Check if react-hook-form has any built-in handling for dynamic field names
rg -n "field.value\|trigger.after\|trigger.expect" ui-v2/src/components/automations/automations-wizard/ -C 2Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
🏁 Script executed:
# Look for any transform or cleanup logic in form submission
rg -n "formData\|transform\|filter\|omit" ui-v2/src/components/automations/automations-wizard/ --type=ts -C 3Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
🏁 Script executed:
# Check if there's any handling in the API call for automations
rg -n "trigger" ui-v2/src/api/automations/ --type=ts -C 3Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 1548
🏁 Script executed:
# Search for form submission transformation or API handling
rg -n "axios\|fetch\|mutation\|useMutation" ui-v2/src/components/automations/automations-wizard/ --type=ts -C 2Repository: qodo-benchmark/prefect-combined-coderabbit
Length of output: 68
Dynamic field switching may leave stale data when posture changes.
When posture toggles between Reactive and Proactive, the component switches between trigger.expect and trigger.after for the status selection. The previously used field retains its value in form state, potentially resulting in both fields being populated during form submission.
Consider clearing the alternate field when posture changes, or implement cleanup logic in the form submission to ensure only the relevant field is sent based on the current posture.
🤖 Prompt for AI Agents
In
`@ui-v2/src/components/automations/automations-wizard/trigger-step/deployment-status-trigger-fields.tsx`
around lines 29 - 31, The component currently switches statusFieldName based on
posture (posture === "Proactive" ? "trigger.after" : "trigger.expect") but
leaves the alternate field populated; add logic to clear the unused field when
posture changes—locate the place where statusFieldName is computed in
deployment-status-trigger-fields (or the component render) and add an effect or
change handler that calls the form API (e.g., resetField/setValue) to clear the
opposite key ("trigger.expect" or "trigger.after") whenever posture changes;
alternatively, add cleanup in the form submission handler to drop the unused
field based on posture so only the relevant trigger field is submitted.
| "noEmit": true, | ||
| "jsx": "react-jsx", | ||
| "strict": true, | ||
| "strict": false, |
There was a problem hiding this comment.
Avoid disabling TypeScript strict mode project-wide.
Disabling strict turns off critical type checks (strictNullChecks, noImplicitAny, etc.) for the entire ui-v2 codebase. This masks type errors rather than fixing them and degrades long-term maintainability.
Instead, fix the type issues in the new trigger field components. If temporary escape hatches are needed, use targeted // @ts-expect-error`` or explicit type assertions in the specific locations rather than weakening the entire project's type safety.
Proposed fix
- "strict": false,
+ "strict": true,Then address type errors in the new components individually.
🤖 Prompt for AI Agents
In `@ui-v2/tsconfig.app.json` at line 19, Change the project-wide TypeScript
setting back to strict mode by setting "strict": true in tsconfig.app.json, then
fix the type errors surfaced in the new trigger field components (the trigger
field components mentioned in the PR) rather than disabling strictness; for
localized workarounds use targeted // `@ts-expect-error` comments or explicit type
assertions in the specific component files and adjust function/prop types (e.g.,
component props, event handlers, and any helper functions used by those trigger
field components) until the project compiles cleanly under strict mode.
1 participant
Benchmark PR from agentic-review-benchmarks#10
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.