Skip to content

feat(schedules): move schedule configuration out of modals into subblocks - #1805

Merged
waleedlatif1 merged 5 commits into
stagingfrom
sim-340
Nov 4, 2025
Merged

feat(schedules): move schedule configuration out of modals into subblocks#1805
waleedlatif1 merged 5 commits into
stagingfrom
sim-340

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • move schedule configuration out of modals into subblocks
  • reduced code surface area, use existing subblocks system and the same pattern from the triggers

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Nov 4, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentPreviewCommentsUpdated (UTC)
docsSkippedSkippedNov 4, 2025 9:34pm

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ScheduleConfig modal component with new ScheduleSave inline 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 0 as 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.tsx validation logic at lines 94-102 - the hourly minute validation must be fixed before merge

Important Files Changed

File Analysis

FilenameScoreOverview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/schedule-save/schedule-save.tsx3/5New schedule save component with validation logic issue for hourly schedules (0 minute validation fails), and redundant state updates
apps/sim/app/api/schedules/route.ts4/5Added in-memory rate limiting and improved validation, wrapped DB operations in transaction, simplified logging
apps/sim/hooks/use-schedule-management.ts5/5Hook 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
Loading

Additional Comments (1)

  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 saveAttempts Map 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

Edit Code Review Agent Settings | Greptile

@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) and schedule-config.tsx (440 lines)
  • Added new schedule-save.tsx component (554 lines) that consolidates save/delete/status logic
  • Created use-schedule-management.ts hook 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:

  1. Validation logic incorrectly rejecting 0 as valid minute value (line 94)
  2. Redundant savedCronExpression assignment (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.tsx component has two acknowledged issues that should be resolved before merge

Important Files Changed

File Analysis

FilenameScoreOverview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/components/sub-block/components/schedule-save/schedule-save.tsx4/5New 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.ts5/5Cleaned 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
Loading

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
waleedlatif1 merged commit a29f9fd into stagingNov 4, 2025
9 checks passed
@waleedlatif1
waleedlatif1 deleted the sim-340 branch November 4, 2025 21:36
@waleedlatif1waleedlatif1 mentioned this pull request Nov 4, 2025
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
@waleedlatif1waleedlatif1 mentioned this pull request Nov 12, 2025
10 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1