feat(schedules): move schedule configuration out of modals into subblocks#1805
Merged
Conversation
added 3 commits
November 4, 2025 12:39
…n numeric values for date and time, fix update schedule behavior
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR refactors schedule configuration from a modal-based approach to an inline subblock system, matching the pattern used by webhook triggers and reducing code surface area.
Key Changes
- Replaced
ScheduleConfigmodal component with newScheduleSaveinline component - Moved schedule configuration UI directly into the schedule block using existing subblock infrastructure
- Added in-memory rate limiting to schedule save endpoint (10 saves per minute per user)
- Improved numeric validation for schedule time values
- Wrapped schedule database operations in transactions
- Removed redundant logging throttle mechanism
- Updated documentation to reflect "schedule block" instead of "schedule modal"
Issues Found
- Critical validation bug: Hourly schedule validation incorrectly rejects
0as a minute value (line 94-102 in schedule-save.tsx) - Architecture concern: In-memory rate limiting won't persist across restarts or scale horizontally in multi-instance deployments
- Minor: Redundant state update in handleSave (lines 300-305)
Confidence Score: 3/5
- This PR has a critical validation bug that will break hourly schedules set to run at minute 0
- The refactoring is well-structured and follows existing patterns, but contains a logic error in validation that will cause hourly schedules with minute=0 to fail. The in-memory rate limiting is a temporary solution that won't scale. Once the validation bug is fixed, this would be safe to merge.
- Pay close attention to
schedule-save.tsxvalidation logic at lines 94-102 - the hourly minute validation must be fixed before merge
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/schedule-save/schedule-save.tsx | 3/5 | New schedule save component with validation logic issue for hourly schedules (0 minute validation fails), and redundant state updates |
| apps/sim/app/api/schedules/route.ts | 4/5 | Added in-memory rate limiting and improved validation, wrapped DB operations in transaction, simplified logging |
| apps/sim/hooks/use-schedule-management.ts | 5/5 | Hook handles schedule lifecycle - loading, saving, and deleting schedules via API calls |
Sequence Diagram
sequenceDiagram
participant User
participant ScheduleSave as ScheduleSave Component
participant Hook as useScheduleManagement
participant API as /api/schedules
participant DB as Database
participant Store as SubBlockStore
User->>ScheduleSave: Configure schedule settings
ScheduleSave->>Store: getValue(blockId, scheduleType)
Store-->>ScheduleSave: Return schedule values
User->>ScheduleSave: Click "Save Schedule"
ScheduleSave->>ScheduleSave: validateRequiredFields()
alt Validation Fails
ScheduleSave->>User: Show error message
else Validation Passes
ScheduleSave->>Hook: saveConfig()
Hook->>Store: Get workflow state
Hook->>API: POST /api/schedules
API->>API: Check rate limit (in-memory)
alt Rate Limit Exceeded
API-->>Hook: 429 Rate Limited
Hook-->>ScheduleSave: Error
ScheduleSave->>User: Show error
else Rate Limit OK
API->>API: Validate authorization
API->>API: Generate cron expression
API->>API: Validate cron expression
API->>DB: Insert/Update schedule (transaction)
DB-->>API: Success
API-->>Hook: Return schedule data
Hook->>Store: setValue(blockId, scheduleId)
Hook-->>ScheduleSave: Success result
ScheduleSave->>API: GET /api/schedules (fetch status)
API->>DB: Query schedule
DB-->>API: Schedule data
API-->>ScheduleSave: Schedule status
ScheduleSave->>User: Show success & next run time
end
end
Additional Comments (1)
-
apps/sim/app/api/schedules/route.ts, line 149-182 (link)style: in-memory rate limiting will not persist across server restarts or scale horizontally
The
saveAttemptsMap is stored in memory and will be reset on server restart. In multi-instance deployments, each instance maintains separate counters, allowing users to bypass limits.Consider using Redis or database-backed rate limiting for production environments.
14 files reviewed, 3 comments
Collaborator
Author
|
@greptile |
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR refactors schedule configuration from a modal-based UI to an inline subblock system, reducing code complexity and improving consistency with the trigger configuration pattern.
Key changes:
- Removed
schedule-modal.tsx(634 lines) andschedule-config.tsx(440 lines) - Added new
schedule-save.tsxcomponent (554 lines) that consolidates save/delete/status logic - Created
use-schedule-management.tshook to extract schedule lifecycle logic - Made schedule configuration fields visible as subblocks (previously hidden) with conditional rendering
- Cleaned up test file by removing commented code and improving mock structure
Benefits:
- Reduced total code by ~378 lines
- Unified UX pattern between triggers and schedules
- Better separation of concerns with dedicated hook
- Inline editing eliminates modal management complexity
Issues addressed in commits:
The head commit acknowledges and fixes two issues from previous review:
- Validation logic incorrectly rejecting 0 as valid minute value (line 94)
- Redundant
savedCronExpressionassignment (line 300)
Confidence Score: 4/5
- This PR is safe to merge with minor issues already acknowledged
- The refactoring follows established patterns and consolidates existing functionality. Two issues were identified and acknowledged in PR comments. The changes are well-tested with updated test coverage. Main risk is the complexity of the validation logic and state management, but these follow existing patterns in the codebase.
- The
schedule-save.tsxcomponent has two acknowledged issues that should be resolved before merge
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/schedule-save/schedule-save.tsx | 4/5 | New component that replaces modal-based schedule configuration with inline subblock UI. Includes validation, save/delete/toggle actions, and status display. Two acknowledged issues: validation logic for 0 minutes and redundant cron expression assignment. |
| apps/sim/app/api/schedules/route.test.ts | 5/5 | Cleaned up test file with improved mocks and removed commented-out test. Added transaction mock support and validateCronExpression mock. |
Sequence Diagram
sequenceDiagram
participant User
participant ScheduleSave as ScheduleSave Component
participant Hook as useScheduleManagement
participant SubBlockStore as SubBlock Store
participant API as /api/schedules
participant DB as Database
User->>ScheduleSave: Configure schedule fields
ScheduleSave->>SubBlockStore: Update field values
SubBlockStore-->>ScheduleSave: Values updated
User->>ScheduleSave: Click Save
ScheduleSave->>ScheduleSave: validateRequiredFields()
alt Validation fails
ScheduleSave->>User: Show error message
else Validation passes
ScheduleSave->>Hook: saveConfig()
Hook->>SubBlockStore: Get workflow state
SubBlockStore-->>Hook: Return state
Hook->>API: POST /api/schedules
API->>API: Validate & generate cron
API->>DB: Insert/update schedule
DB-->>API: Schedule saved
API-->>Hook: Return schedule ID & nextRunAt
Hook->>SubBlockStore: Set scheduleId
Hook-->>ScheduleSave: Return success
ScheduleSave->>API: GET /api/schedules (status)
API->>DB: Fetch schedule details
DB-->>API: Return schedule
API-->>ScheduleSave: Return status info
ScheduleSave->>ScheduleSave: Update UI with cron & nextRunAt
ScheduleSave->>User: Show success state
end
User->>ScheduleSave: Toggle status (enable/disable)
ScheduleSave->>API: PUT /api/schedules/:id
API->>DB: Update status
DB-->>API: Status updated
API-->>ScheduleSave: Success
ScheduleSave->>API: GET /api/schedules (refresh)
API-->>ScheduleSave: Updated status
ScheduleSave->>User: Show updated status
User->>ScheduleSave: Delete schedule
ScheduleSave->>Hook: deleteConfig()
Hook->>API: DELETE /api/schedules/:id
API->>DB: Delete schedule
DB-->>API: Deleted
API-->>Hook: Success
Hook->>SubBlockStore: Clear scheduleId
Hook-->>ScheduleSave: Success
ScheduleSave->>User: Clear status UI
2 files reviewed, no comments
10 tasks
waleedlatif1
added a commit
that referenced
this pull request
Nov 12, 2025
…ocks (#1805) * feat(schedules): move schedule configuration out of modals into subblocks * added more timezones * added simple in-memory rate limiting to update schedule, validation on numeric values for date and time, fix update schedule behavior * fix failing tests, ack PR comments * surface better errors
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.
Summary
Type of Change
Testing
Tested manually
Checklist