Feature/send sms based on schedule#857
Conversation
…le-review-fixes # Conflicts: # web/pages/settings/index.vue
…ixes feat: send schedules based on review feedback
PR Summary
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 2 minor |
| CodeStyle | 98 minor |
🟢 Metrics 192 complexity · 51 duplication
Metric Results Complexity 192 Duplication 51
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Greptile SummaryThis PR adds a
Confidence Score: 3/5Not safe to merge as-is — the missing ownership check on schedule_id allows cross-user FK references that can be silently mutated by the schedule owner's delete action. One P1 finding (cross-user schedule attachment enabling cross-user data mutation via FK cascade) caps the score at 4; the presence of a security-relevant gap lowers it further to 3. api/pkg/validators/phone_handler_validator.go — needs an ownership check before accepting schedule_id on a phone update.
|
| Filename | Overview |
|---|---|
| api/pkg/validators/phone_handler_validator.go | Adds UUID-format validation for schedule_id but lacks ownership verification — a user can attach another user's schedule to their phone, causing cross-user data mutation via FK cascade. |
| api/pkg/entities/send_schedule.go | New entity and ResolveScheduledAt algorithm; logic is sound for typical schedules but an active schedule with empty windows silently falls back to "send immediately". |
| api/pkg/repositories/gorm_phone_notification_repository.go | Schedule parameter wired into rate-limited notification scheduling; the two-step resolve (schedule window + rate limit) logic looks correct for both the transactional and non-transactional paths. |
| api/pkg/validators/send_schedule_handler_validator.go | Validates windows per day, start/end ranges, and overlap; logic is correct, though empty-windows case is not blocked here. |
| api/pkg/services/phone_notification_service.go | Schedule is loaded with the caller's userID before notification scheduling, providing correct ownership isolation at runtime; silently falls back to immediate send if schedule is not found. |
| web/store/index.ts | Adds CRUD Vuex actions for send schedules; error handlers use unnecessary Promise.all wrapping, and getSendSchedules passes an ignored limit parameter to the backend. |
| api/pkg/di/container.go | Wires new repository, service, validator, handler, and listener into the DI container; AutoMigrate for MessageSendSchedule is correctly ordered before Phone. |
| api/pkg/handlers/send_schedule_handler.go | Standard CRUD handler with proper auth middleware, not-found handling, and validation flow; no issues found. |
| web/pages/settings/send-schedules/index.vue | New dedicated page for managing send schedules with full CRUD UI; no significant issues found. |
Sequence Diagram
sequenceDiagram
participant Client
participant SendScheduleHandler
participant PhoneHandler
participant PhoneNotificationService
participant SendScheduleRepository
participant PhoneNotificationRepository
Client->>SendScheduleHandler: POST /v1/send-schedules
SendScheduleHandler->>SendScheduleRepository: Store(schedule)
Client->>PhoneHandler: PUT /v1/phones {schedule_id}
PhoneHandler->>PhoneHandler: ValidateUUID(schedule_id) [no ownership check]
PhoneHandler->>PhoneNotificationService: Upsert phone with ScheduleID
Note over PhoneNotificationService,SendScheduleRepository: On message send
PhoneNotificationService->>SendScheduleRepository: Load(userID, phone.ScheduleID)
SendScheduleRepository-->>PhoneNotificationService: schedule (or ErrNotFound → nil)
PhoneNotificationService->>PhoneNotificationRepository: Schedule(messagesPerMinute, schedule, notification)
PhoneNotificationRepository->>PhoneNotificationRepository: resolveScheduledAt(schedule, now)
Note right of PhoneNotificationRepository: Finds next window start, applies rate limit, re-resolves
PhoneNotificationRepository-->>PhoneNotificationService: notification.ScheduledAt set
Reviews (1): Last reviewed commit: "Merge pull request #841 from giresse19/f..." | Re-trigger Greptile
| resolve(response.data.data) | ||
| }) | ||
| .catch(async (error: AxiosError) => { | ||
| await Promise.all([ | ||
| context.dispatch('addNotification', { | ||
| message: | ||
| (error.response?.data as any)?.message ?? |
| axios | ||
| .get<ResponsesSendSchedulesResponse>(`/v1/send-schedules`, { | ||
| params: { | ||
| limit: 100, | ||
| }, |
There was a problem hiding this comment.
Unused
limit query param sent to unimplemented pagination
getSendSchedules sends { params: { limit: 100 } } but the backend GET /v1/send-schedules handler ignores all query params and returns every schedule for the user. The param is silently discarded, which may also mask the fact that if a user ever has more than 100 schedules the frontend won't show them all without backend pagination support.
Replace the separate /settings/send-schedules page with an inline dialog on the main settings page, matching the pattern used for phones, webhooks, and Discord integrations. - Add v-dialog for creating and editing send schedules with day/time windows - Add delete confirmation dialog - Fix edit button in schedules table (was incorrectly calling showEditPhone) - Remove the now-redundant /settings/send-schedules/ page Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Create EntitlementService with configurable entity limits per plan - Add ENTITLEMENT_ENABLED env var (defaults to false for self-hosted) - Free users limited to 1 send schedule, paid users unlimited - Add CountByUser to send schedule repository for efficient counting - Return 402 Payment Required when limit exceeded Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sages Converts PascalCase entity names to lowercase words with proper pluralization (e.g. MessageSendSchedule → 'message send schedules') Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace custom pluralization logic with github.com/gertd/go-pluralize for more accurate English pluralization rules. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ndMessage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Schedule Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…event Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ing and send rate Link to https://docs.httpsms.com/features/scheduling-sms-messages and https://docs.httpsms.com/features/control-sms-send-rate instead of repeating their explanations. Add detailed MessageSendSchedule (send windows) section since it's the only new feature without its own docs page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds recurring send schedules so outgoing SMS can respect per-phone availability windows, while also refactoring backend scheduling so explicit SendAt messages bypass queue adjustments and bulk sends are delayed by the configured send rate instead of the old fixed 1-second spacing. It touches the Go API, Nuxt settings UI, generated API models/docs, and several design docs describing the new scheduling/entitlement work.
Changes:
- Added send schedule CRUD support across the API, frontend settings page, and generated web models.
- Updated message dispatch flow to support exact-send semantics and per-phone rate-based bulk delays.
- Added entitlement plumbing and documentation/spec updates for schedules, queue behavior, and planned test infrastructure.
Reviewed changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| web/store/index.ts | Adds Vuex actions for send schedules and sends schedule_id when updating phones |
| web/pages/settings/index.vue | Adds UI for listing, editing, assigning, and deleting send schedules |
| web/pages/bulk-messages/index.vue | Adds help text linking bulk sends to send schedules |
| web/models/api.ts | Extends generated TS API types with send schedule models and phone schedule_id |
| outgoing-message-queue.md | New user-facing docs for queueing, explicit send times, rates, and schedule windows |
| docs/superpowers/specs/2026-05-03-scheduling-send-refactor-design.md | Design doc for exact-send and rate-based scheduling refactor |
| docs/superpowers/specs/2026-05-03-integration-test-setup-design.md | Design doc for future integration test infrastructure |
| docs/superpowers/specs/2026-05-03-entitlement-service-design.md | Design doc for plan-based feature limits |
| docs/superpowers/plans/2026-05-03-scheduling-send-refactor.md | Detailed implementation plan for backend scheduling refactor |
| api/pkg/validators/phone_handler_validator.go | Adds UUID validation for schedule_id on phone updates |
| api/pkg/validators/message_send_schedule_handler_validator.go | New validator for send schedule requests and window rules |
| api/pkg/services/phone_service.go | Persists ScheduleID on phones during create/update |
| api/pkg/services/phone_notification_service.go | Loads schedules during notification scheduling and adds exact-send bypass |
| api/pkg/services/message_service_test.go | Adds unit tests for new send-delay logic |
| api/pkg/services/message_service.go | Adds per-message index and rate-based delay calculation |
| api/pkg/services/message_send_schedule_service.go | New service for send schedule CRUD/count/delete-all operations |
| api/pkg/services/entitlement_service.go | New service for subscription-based entity limits |
| api/pkg/responses/message_send_schedule_responses.go | Adds wrapped API response types for send schedules |
| api/pkg/requests/phone_update_request.go | Parses optional schedule_id into phone upsert params |
| api/pkg/requests/message_send_schedule_store_request.go | New request DTO for send schedule create/update |
| api/pkg/requests/message_bulk_send_request.go | Replaces synthetic SendAt with per-item bulk index |
| api/pkg/requests/bulk_message_request.go | Adds per-phone index support for CSV bulk sends |
| api/pkg/repositories/repository.go | Removes generic retry helpers from repository package |
| api/pkg/repositories/phone_notification_repository.go | Extends interface with schedule-aware Schedule and ScheduleExact |
| api/pkg/repositories/message_send_schedule_repository.go | New repository interface for send schedules |
| api/pkg/repositories/gorm_phone_notification_repository.go | Implements schedule-aware scheduling and exact scheduling persistence |
| api/pkg/repositories/gorm_message_send_schedule_repository.go | New GORM repository for send schedules |
| api/pkg/repositories/gorm_heartbeat_repository.go | Removes retry wrapper usage from heartbeat queries |
| api/pkg/repositories/gorm_heartbeat_monitor_repository.go | Removes retry wrapper usage from heartbeat monitor queries |
| api/pkg/listeners/phone_notification_listener.go | Passes exact-send metadata into notification scheduling |
| api/pkg/listeners/message_send_schedule_listener.go | Cleans up schedules when a user account is deleted |
| api/pkg/handlers/message_send_schedule_handler.go | Adds REST endpoints for send schedule CRUD |
| api/pkg/handlers/message_handler.go | Removes old 1-second bulk SendAt hack |
| api/pkg/handlers/bulk_message_handler.go | Computes per-phone bulk index for CSV scheduling |
| api/pkg/events/message_api_sent_event.go | Adds exact_send_time to event payload |
| api/pkg/entities/phone.go | Adds optional schedule relationship to phones |
| api/pkg/entities/message_send_schedule_test.go | Adds initial tests for schedule resolution |
| api/pkg/entities/message_send_schedule.go | Adds recurring schedule entity and scheduling resolution logic |
| api/pkg/di/container.go | Wires schedule repository/service/handler/listener and entitlement service |
| api/main.go | Removes libsql side-effect import |
| api/go.sum | Dependency updates for removed libsql/sqlite and added pluralize |
| api/go.mod | Dependency updates for entitlement service and removed libsql/sqlite |
| api/docs/swagger.yaml | Generated OpenAPI updates for schedules and phone schedule_id |
| api/docs/swagger.json | Generated OpenAPI JSON updates |
| api/docs/docs.go | Generated swagger doc template updates |
| api/.env.docker | Adds entitlement feature flag for docker env |
| // @Success 200 {array} entities.MessageSendSchedule | ||
| // @Failure 401 {object} responses.Unauthorized | ||
| // @Failure 500 {object} responses.InternalServerError | ||
| // @Router /send-schedules [get] | ||
| func (h *MessageSendScheduleHandler) Index(c *fiber.Ctx) error { |
| scheduleWindowError(index: number): string | null { | ||
| const messages = this.errorMessages.has('windows') | ||
| ? this.errorMessages.get('windows') | ||
| : [] | ||
| if (messages.length == 0) { | ||
| return null | ||
| } | ||
|
|
||
| const message = messages.find((x: string) => | ||
| x.includes(`Day of week ${index}`), | ||
| ) | ||
| return message | ||
| ? message.replace(`Day of week ${index}`, this.getWeekday(index)) | ||
| : null |
| // @Failure 402 {object} responses.BadRequest | ||
| // @Failure 422 {object} responses.UnprocessableEntity | ||
| // @Failure 500 {object} responses.InternalServerError |
| - messages_per_minute | ||
| - missed_call_auto_reply | ||
| - phone_number | ||
| - schedule_id |
- Add schedule ownership check in phone handler to prevent cross-user schedule_id assignment (security fix) - Fix OpenAPI annotation for Index endpoint to use wrapper response type - Fix 402 response annotation to use proper PaymentRequired type - Add PaymentRequired response type to responses package - Mark schedule_id as optional in PhoneUpsert request struct - Fix scheduleWindowError matching to use backend error format (day_of_week) - Remove unnecessary Promise.all wrapping in store actions - Remove unused limit query param from getSendSchedules - Treat active schedule with empty windows as inactive (send immediately) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No description provided.