-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(form): added a deployed form that can be public facing and trigger the workflow #1438
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
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.
Greptile Overview
Summary
This PR implements a comprehensive deployed form feature that allows workflows to be triggered via public-facing forms. The implementation includes form configuration UI, public form rendering, robust API validation, and background workflow execution.
Key Changes:
- Public Form Pages: New
/form/[formId]route with form rendering component - Form APIs: Secure form retrieval and submission endpoints with validation, rate limiting, and usage controls
- Background Execution: Integration with workflow executor via Trigger.dev or direct execution
- Database Schema: New
workflow_formtable with proper indexing and constraints - Form Configuration: Rich UI for configuring form fields, validation, styling, and behavior
- Block Integration: New
form_triggerblock type with proper TypeScript definitions
Technical Highlights:
- Comprehensive form validation on both client and server sides
- Rate limiting and usage limit enforcement
- Idempotency handling for duplicate submissions
- Proper error handling and logging throughout the flow
- Integration with existing workflow execution infrastructure
Minor Issues Found:
- Some type assertions to
anyshould be replaced with proper interfaces - UUID generation logic in form management API needs consistency fix
- Hardcoded image path in form page could be improved
The feature is well-implemented with proper security considerations, error handling, and follows the established patterns in the codebase.
Confidence Score: 4/5
- This PR is safe to merge with minor improvements recommended
- Score reflects well-implemented core functionality with comprehensive security measures, proper error handling, and good integration patterns. Only minor style improvements and one logical fix needed.
- apps/sim/app/api/workflows/[id]/forms/route.ts needs the UUID generation logic fix
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/api/forms/[formId]/submit/route.ts | 4/5 | Form submission API with comprehensive validation, rate limiting, and idempotency handling - well implemented |
| apps/sim/background/form-execution.ts | 4/5 | Form execution with proper logging, error handling, and idempotency - solid implementation |
| apps/sim/app/form/[formId]/components/form-renderer.tsx | 4/5 | React form renderer with comprehensive validation and error handling - well-structured component |
| apps/sim/app/form/[formId]/page.tsx | 3/5 | Form page component with potential hardcoded URL issue - needs attention |
| apps/sim/app/api/workflows/[id]/forms/route.ts | 3/5 | Workflow forms API with potential UUID generation issue in POST vs PUT logic |
Sequence Diagram
sequenceDiagram
participant User
participant FormPage as Form Page<br/>(sim.ai/form/[id])
participant FormAPI as Form Submit API<br/>(/api/forms/[id]/submit)
participant BackgroundJob as Background Execution<br/>(form-execution.ts)
participant Executor as Workflow Executor
participant Database as Database
User->>FormPage: Access form URL
FormPage->>FormAPI: GET /api/forms/[id] (fetch form config)
FormAPI->>Database: Query workflow_form table
Database-->>FormAPI: Return form configuration
FormAPI-->>FormPage: Form config & styling
FormPage-->>User: Render form with fields
User->>FormPage: Fill form and submit
FormPage->>FormAPI: POST /api/forms/[id]/submit (form data)
FormAPI->>FormAPI: Validate form fields
FormAPI->>FormAPI: Check rate limits
FormAPI->>FormAPI: Check usage limits
alt Trigger.dev enabled
FormAPI->>BackgroundJob: Queue via trigger.dev
else Direct execution
FormAPI->>BackgroundJob: Execute directly
end
FormAPI-->>User: Success response with message
BackgroundJob->>Database: Load workflow configuration
BackgroundJob->>BackgroundJob: Process form data as workflow input
BackgroundJob->>Executor: Execute workflow with form data
Executor->>Executor: Run workflow blocks
BackgroundJob->>Database: Update execution stats
BackgroundJob->>Database: Save execution logs
22 files reviewed, 4 comments
| foundWorkflow = forms[0].workflow | ||
|
|
||
| try { | ||
| const formConfig = foundForm.formConfig as any |
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: avoid type assertion to any - should define proper interface for formConfig structure
| const formConfig = foundForm.formConfig as any | |
| const formConfig = foundForm.formConfig as { fields: Array<{ name: string; label: string; type: string; required: boolean }> } |
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| let foundWorkflow: any = null | ||
| let foundForm: any = null | ||
|
|
||
| let body: any |
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: avoid type assertion to any - define proper types for foundWorkflow and foundForm
| let foundWorkflow: any = null | |
| let foundForm: any = null | |
| let body: any | |
| let foundWorkflow: { id: string; userId: string; name: string } | null = null | |
| let foundForm: { id: string; formConfig: any; settings: any; blockId?: string } | null = null | |
| let body: Record<string, any> |
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| className='inline-flex items-center gap-2 text-muted-foreground text-sm transition-colors hover:text-foreground' | ||
| > | ||
| <span>Powered by</span> | ||
| <img src='/logo/reverse/text/medium.png' alt='Sim' className='h-4' /> |
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: hardcoded image path may not work in all environments - consider using Next.js Image component or dynamic imports
| const formPath = uuidv4() | ||
| const formData = { | ||
| workflowId, | ||
| blockId, | ||
| path: formPath, | ||
| title, | ||
| description, | ||
| formConfig, | ||
| styling: styling || {}, | ||
| settings: settings || {}, | ||
| isActive: true, | ||
| } | ||
|
|
||
| let result | ||
| if (existingForm.length > 0) { | ||
| result = await db | ||
| .update(workflowForm) | ||
| .set({ | ||
| ...formData, | ||
| path: existingForm[0].path, | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(eq(workflowForm.id, existingForm[0].id)) | ||
| .returning() | ||
|
|
||
| result[0] = { ...result[0], path: existingForm[0].path } |
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 UUID generated for every form update, but existing path is preserved - this creates inconsistent behavior between POST and PUT
| const formPath = uuidv4() | |
| const formData = { | |
| workflowId, | |
| blockId, | |
| path: formPath, | |
| title, | |
| description, | |
| formConfig, | |
| styling: styling || {}, | |
| settings: settings || {}, | |
| isActive: true, | |
| } | |
| let result | |
| if (existingForm.length > 0) { | |
| result = await db | |
| .update(workflowForm) | |
| .set({ | |
| ...formData, | |
| path: existingForm[0].path, | |
| updatedAt: new Date(), | |
| }) | |
| .where(eq(workflowForm.id, existingForm[0].id)) | |
| .returning() | |
| result[0] = { ...result[0], path: existingForm[0].path } | |
| const formPath = existingForm.length > 0 ? existingForm[0].path : uuidv4() | |
| const formData = { | |
| workflowId, | |
| blockId, | |
| path: formPath, | |
| title, | |
| description, | |
| formConfig, | |
| styling: styling || {}, | |
| settings: settings || {}, | |
| isActive: true, | |
| } | |
| let result | |
| if (existingForm.length > 0) { | |
| result = await db | |
| .update(workflowForm) | |
| .set({ | |
| ...formData, | |
| updatedAt: new Date(), | |
| }) | |
| .where(eq(workflowForm.id, existingForm[0].id)) | |
| .returning() |
f0c20a9 to
010753d
Compare
Summary
added a deployed form that can be public facing and trigger the workflow, when a user uses the
deployed forminput type, they can configure the form and it becomes public-facing at sim.ai/forms/[formId] and when someone submits the form, it will trigger the workflow and make those fields accessible via the tag dropdown.Type of Change
Testing
Tested manually.
Checklist