-
Notifications
You must be signed in to change notification settings - Fork 123
feat: Use cron input in start workflow form #1040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Use cron input in start workflow form #1040
Conversation
Signed-off-by: Assem Hafez <[email protected]>
Signed-off-by: Assem Hafez <[email protected]>
Signed-off-by: Assem Hafez <[email protected]>
Signed-off-by: Assem Hafez <[email protected]>
…15436/use-cron-input-in-start-form
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the plain text cron input with a structured CronScheduleInput in the start workflow form, updates validation to map errors to sub-fields, adds a trigger prop to forms to validate parent fields from sub-field changes, and adjusts the modal width to accommodate the new input.
- Replace cron string input with CronScheduleInput component and object schema
- Add zod superRefine with granular error mapping via getCronFieldsError
- Pass react-hook-form trigger through form props; widen modal Dialog to 600px
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/views/workflow-actions/workflow-actions.types.ts | Adds trigger to form props so sub-field changes can validate the parent field. |
| src/views/workflow-actions/workflow-actions-modal/workflow-actions-modal.styles.ts | Sets modal Dialog width to 600px to fit the cron input. |
| src/views/workflow-actions/workflow-actions-modal-content/workflow-actions-modal-content.tsx | Plumbs trigger from useForm into form components. |
| src/views/workflow-actions/workflow-action-start-form/workflow-action-start-form.tsx | Integrates CronScheduleInput, maps nested cron errors, and triggers validation on change. |
| src/views/workflow-actions/workflow-action-start-form/schemas/start-workflow-form-schema.ts | Changes cronSchedule schema to an object and adds superRefine to validate cron and map field-specific errors. |
| src/views/workflow-actions/workflow-action-start-form/helpers/transform-start-workflow-form-to-submission.ts | Transforms cron object to a cron string for submission using CRON_FIELD_ORDER. |
| src/views/workflow-actions/workflow-action-start-form/helpers/get-cron-fields-error.ts | Adds helper to translate cron-validate errors into per-field or general errors. |
| src/views/workflow-actions/workflow-action-start-form/helpers/tests/transform-start-workflow-form-to-submission.test.ts | Updates tests to use cron object and validate string transformation. |
| src/views/workflow-actions/workflow-action-start-form/helpers/tests/get-cron-fields-error.test.ts | Adds tests for mapping cron-validate errors to field/general errors. |
| src/views/workflow-actions/workflow-action-start-form/tests/workflow-action-start-form.test.tsx | Updates assertions to target the cron sub-inputs and passes trigger in the test wrapper. |
| src/views/workflow-actions/workflow-action-signal-form/tests/workflow-action-signal-form.test.tsx | Passes trigger to the form in tests. |
| src/views/workflow-actions/workflow-action-reset-form/tests/workflow-action-reset-form.test.tsx | Passes trigger to the form in tests. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ow-actions/workflow-action-start-form/helpers/transform-start-workflow-form-to-submission.ts
Show resolved
Hide resolved
.../workflow-actions/workflow-action-start-form/helpers/__tests__/get-cron-fields-error.test.ts
Outdated
Show resolved
Hide resolved
src/views/workflow-actions/workflow-actions-modal/workflow-actions-modal.styles.ts
Show resolved
Hide resolved
| }: Props) { | ||
| const now = useMemo(() => new Date(), []); | ||
|
|
||
| const getErrorMessage = (field: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're not returning a string in L32, this function is not an error message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also add a return type annotation to make things clearer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sometimes it is more than one message (Array/object of error messages). So adding an s to the name and type should make it clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typing the error turned out to be tricky as the errors are generated using superRefine which makes them almost dynamic. Changed its name and fixed one issue in it.
src/views/workflow-actions/workflow-action-start-form/workflow-action-start-form.tsx
Outdated
Show resolved
Hide resolved
src/views/workflow-actions/workflow-action-start-form/helpers/get-cron-fields-error.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Assem Hafez <[email protected]>
| }} | ||
| onBlur={field.onBlur} | ||
| error={Boolean(getErrorMessage('taskList.name'))} | ||
| error={Boolean(getFieldErrorMessages('taskList.name'))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work for empty arrays?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but booleans shouldn't be used for errors of type array or object.
Sinc errors are not typed, it is currently manual effort to process it correctly. If you know a good way to type the errors, it would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary
Replace usage of text input in start workflow form with the new cron input.
Changes
Screen Recording
Screen.Recording.2025-09-26.at.18.19.03.mov