feat(ui-v2): add CustomPostureSelect component with radio buttons for custom triggers#129
Open
tomerqodo wants to merge 3 commits into
Open
feat(ui-v2): add CustomPostureSelect component with radio buttons for custom triggers#129tomerqodo wants to merge 3 commits into
tomerqodo wants to merge 3 commits into
Conversation
… custom triggers - Create CustomPostureSelect component with 'Observe' and 'Don't observe' radio buttons - Update CustomTriggerFields to use the new component instead of PostureSelect dropdown - Export CustomPostureSelect from trigger-step index - Update tests to verify radio button labels are visible Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
- Move component to custom-posture-select/ directory with colocated files - Add comprehensive unit tests for radio button rendering and posture change logic - Add Storybook stories showing Reactive and Proactive states - Tests verify value migration between expect/after fields and within default Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>
Code Review by Qodo (Alpha)
1. `httpcore` version mismatch
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| "graphviz>=0.20.1", | ||
| "griffe>=0.49.0,<2.0.0", | ||
| "httpcore>=1.0.5,<2.0.0", | ||
| "httpcore>=1.0.6,<2.0.0", |
There was a problem hiding this comment.
1. httpcore version mismatch 📘 Rule Violation
• The root pyproject.toml updates httpcore to >=1.0.6,<2.0.0 while client/pyproject.toml still pins httpcore at >=1.0.5,<2.0.0. • This creates dependency drift between the main project and the client, violating the requirement that client dependency updates be mirrored in both files. • The mismatch can lead to inconsistent environments and hard-to-debug runtime/test differences depending on which lock/install path is used.
Agent Prompt
## Issue description
The `httpcore` dependency was updated in the root `pyproject.toml` but not in `client/pyproject.toml`, causing dependency drift.
## Issue Context
Compliance requires that client dependency changes be applied consistently in both `pyproject.toml` files.
## Fix Focus Areas
- client/pyproject.toml[37-41]
- pyproject.toml[52-55]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+22
to
+39
| const handlePostureChange = (value: "Reactive" | "Proactive") => { | ||
| const previousPosture = form.getValues("trigger.posture"); | ||
| form.setValue("trigger.posture", value); | ||
|
|
||
| // Move state values between expect and after when posture changes | ||
| // Reactive uses "expect", Proactive uses "after" | ||
| if (previousPosture === "Reactive" && value === "Proactive") { | ||
| const expectValues = form.getValues("trigger.expect") ?? []; | ||
| form.setValue("trigger.after", expectValues); | ||
| const currentWithin = form.getValues("trigger.within"); | ||
| if (currentWithin === 0) { | ||
| form.setValue("trigger.within", DEFAULT_PROACTIVE_WITHIN); | ||
| } | ||
| } else if (previousPosture === "Proactive" && value === "Reactive") { | ||
| const afterValues = form.getValues("trigger.after") ?? []; | ||
| form.setValue("trigger.expect", afterValues); | ||
| form.setValue("trigger.after", []); | ||
| } |
There was a problem hiding this comment.
2. Expect not cleared 🐞 Bug
• Switching posture Reactive→Proactive copies trigger.expect into trigger.after but never clears trigger.expect, leaving both fields populated. • This diverges from the existing PostureSelect logic and will produce contradictory/duplicated trigger configuration in the form state. • It also likely breaks the intended UX (the “expected events” textarea would still show values after selecting Proactive).
Agent Prompt
## Issue description
Reactive→Proactive posture toggle in `CustomPostureSelect` moves `trigger.expect` into `trigger.after` but fails to clear `trigger.expect`, leaving both populated.
## Issue Context
The existing `PostureSelect` clears `trigger.expect` on Reactive→Proactive. `CustomPostureSelect` should match this behavior.
## Fix Focus Areas
- ui-v2/src/components/automations/automations-wizard/trigger-step/custom-posture-select/index.tsx[22-39]
- ui-v2/src/components/automations/automations-wizard/trigger-step/posture-select.tsx[31-55]
## Expected change
- In the Reactive→Proactive branch, add `form.setValue("trigger.expect", [])` after setting `trigger.after`.
- (Recommended) Ensure posture updates go through RHF field change semantics (either `field.onChange(value)` or `setValue` with `{shouldDirty:true, shouldTouch:true, shouldValidate:true}`) so the form state behaves consistently.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
122
to
126
| <DurationInput | ||
| value={field.value ?? 0} | ||
| onChange={field.onChange} | ||
| min={posture === "Proactive" ? 10 : 0} | ||
| min={0} | ||
| /> |
There was a problem hiding this comment.
3. Proactive within invalid 🐞 Bug
• CustomTriggerFields now allows within values below 10 seconds even when posture is Proactive (min=0), but the backend rejects Proactive within values <10s. • Because the UI Zod schema only enforces within >= 0, users can progress through the wizard and only fail at API validation time (or worse, save inconsistent JSON mode values). • Separately, removing the conditional min makes posture unused in this file, and with noUnusedLocals=true this can break the UI build.
Agent Prompt
## Issue description
Custom trigger `within` input no longer enforces the backend constraint for Proactive triggers (>=10 seconds). The UI can produce values that the server rejects. Additionally, `posture` is now an unused local, which is a TypeScript build error due to `noUnusedLocals: true`.
## Issue Context
Backend enforces `within >= 10s` for Proactive triggers and raises a validation error for values below 10 seconds.
## Fix Focus Areas
- ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx[21-24]
- ui-v2/src/components/automations/automations-wizard/trigger-step/custom-trigger-fields.tsx[115-126]
- src/prefect/events/schemas/automations.py[171-205]
## Expected change
- Restore conditional minimum in `DurationInput`:
- `min={posture === "Proactive" ? 10 : 0}`
- Optionally (recommended for JSON mode parity), add a Zod object-level validation to the UI `EventTriggerSchema` to prevent Proactive `within` in the range 1..9.
- Ensure `posture` is actually used (by restoring the conditional min) or remove the `useWatch`/`posture` variable entirely if no longer needed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+22
to
+25
| const handlePostureChange = (value: "Reactive" | "Proactive") => { | ||
| const previousPosture = form.getValues("trigger.posture"); | ||
| form.setValue("trigger.posture", value); | ||
|
|
There was a problem hiding this comment.
4. Rhf change not tracked 🐞 Bug
• CustomPostureSelect updates trigger.posture via form.setValue instead of the provided RHF field onChange, which can skip touched/dirty semantics. • This can lead to posture changes not being reflected in formState (e.g., dirty tracking, validation timing) even though the underlying value changes. • The existing PostureSelect uses field.onChange, so this is an inconsistency that can cause subtle wizard UX bugs.
Agent Prompt
## Issue description
`CustomPostureSelect` updates `trigger.posture` via `form.setValue` instead of the RHF field handler. This can skip dirty/touched semantics.
## Issue Context
Existing `PostureSelect` uses `field.onChange` for posture changes.
## Fix Focus Areas
- ui-v2/src/components/automations/automations-wizard/trigger-step/custom-posture-select/index.tsx[22-25]
- ui-v2/src/components/automations/automations-wizard/trigger-step/posture-select.tsx[31-34]
## Expected change
- Prefer calling `field.onChange(value)` inside the `FormField` render (pass the `field` into your handler), then do the `expect/after/within` migration.
- Alternatively, keep `setValue` but add options: `{ shouldDirty: true, shouldTouch: true, shouldValidate: true }`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from qodo-benchmark#401