feat: Use cron input in start workflow form#1040
feat: Use cron input in start workflow form#1040Assem-Uber merged 15 commits intocadence-workflow:masterfrom
Conversation
Signed-off-by: Assem Hafez <assem.hafez@uber.com>
Signed-off-by: Assem Hafez <assem.hafez@uber.com>
Signed-off-by: Assem Hafez <assem.hafez@uber.com>
Assem-Uber
changed the title
Signed-off-by: Assem Hafez <assem.hafez@uber.com>
…15436/use-cron-input-in-start-form
There was a problem hiding this comment.
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.
If we're not returning a string in L32, this function is not an error message
There was a problem hiding this comment.
You can also add a return type annotation to make things clearer
There was a problem hiding this comment.
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.
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 <assem.hafez@uber.com>
| }} | ||
| onBlur={field.onBlur} | ||
| error={Boolean(getErrorMessage('taskList.name'))} | ||
| error={Boolean(getFieldErrorMessages('taskList.name'))} |
There was a problem hiding this comment.
Will this work for empty arrays?
There was a problem hiding this comment.
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.
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.
src/views/workflow-actions/workflow-action-start-form/schemas/start-workflow-form-schema.ts
Show resolved
Hide resolved
src/views/workflow-actions/workflow-actions-modal/workflow-actions-modal.styles.ts
Show resolved
Hide resolved
src/views/workflow-actions/workflow-action-start-form/workflow-action-start-form.tsx
Show resolved
Hide resolved
Assem-Uber
changed the title
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