-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Prevent empty form steps #12560
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
Prevent empty form steps #12560
Conversation
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.
PR Summary
Added validation to prevent empty form steps in workflow triggers, requiring at least one field in form action inputs.
- Modified
assertFormStepIsValid
inpackages/twenty-server/src/modules/workflow/workflow-trigger/utils/assert-version-can-be-activated.util.ts
to require non-empty input arrays in form steps - Added new test file with comprehensive coverage for form step validation scenarios
- Made
assertFormStepIsValid
function exportable for better modularity and testability - Added error handling for empty form inputs using
WorkflowTriggerException
2 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
if (settings.input.length === 0) { | ||
throw new WorkflowTriggerException( | ||
'Form action must have at least one field', | ||
WorkflowTriggerExceptionCode.INVALID_WORKFLOW_VERSION, | ||
); | ||
} |
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.
logic: new validation uses INVALID_WORKFLOW_VERSION error code while similar empty check above uses INVALID_WORKFLOW_TRIGGER - consider using consistent error codes for similar validation types
type: 'RECORD', | ||
label: 'Record', |
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.
style: Use FieldMetadataType.RECORD instead of string literal 'RECORD' to maintain consistency with line 12 and ensure type safety
type: 'RECORD', | |
label: 'Record', | |
type: FieldMetadataType.RECORD, | |
label: 'Record', |
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:44787 This environment will automatically shut down when the PR is closed or after 5 hours. |
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.
Nice job
CleanShot.2025-06-12.at.11.17.09.mp4
Related to twentyhq/core-team-issues#1091