From c3066f9cc96879c910cecb6155b02d5177c8a89b Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Mon, 24 Aug 2026 05:20:26 -0700 Subject: [PATCH] feat: unbox campaign start dates and make follow-up pacing real: accept today as "start now" and let an explicit null clear start/end dates on PATCH /campaigns (models.NullableTime distinguishes absent from null, which used to silently no-op while the error message told users to send null), reschedule an active campaign's parked wakeup when any schedule field changes so clearing a future start date takes effect immediately instead of at the old slot, let a completed campaign be started again and turn the past-end-date start 500 into a clear 400, gate the campaign task on the step's hard-constraint floor (wait_after, start date, windows, day capacity, mailbox min-gap) via ErrCampaignDeferred so an early successor tick can no longer send a wait-3-days follow-up seconds after step one (live-tested in TestLiveFollowUpWaitIsHonored), disable past days in the schedule date picker, and fix the sandbox seed leaving worker 1a01 free-tier after make seed which unassigned the paid org's mailboxes and failed every send --- AGENTS.md | 2 + docs/content/docs/api/reference/campaigns.mdx | 8 +- docs/content/docs/guides/campaigns.mdx | 6 +- internal/app/campaign/handlers.go | 40 ++++++- internal/config/constants.go | 7 ++ internal/errx/common.go | 2 +- internal/models/campaign.go | 21 +++- internal/models/nullable.go | 29 +++++ internal/models/nullable_test.go | 52 ++++++++ internal/repository/pg_campaign.go | 20 ++-- internal/sandbox/seed.go | 6 +- internal/scheduler/campaign_scheduler.go | 20 ++++ internal/scheduler/live_integration_test.go | 113 +++++++++++++----- internal/utils/validate/campaign.go | 6 +- internal/utils/validate/campaign_test.go | 32 +++++ .../app/app/campaigns/[id]/schedule/page.tsx | 4 +- web/src/components/app/Calendar.tsx | 23 ++-- .../campaigns/schedule/ScheduleDateSelect.tsx | 4 +- 18 files changed, 327 insertions(+), 68 deletions(-) create mode 100644 internal/models/nullable.go create mode 100644 internal/models/nullable_test.go create mode 100644 internal/utils/validate/campaign_test.go diff --git a/AGENTS.md b/AGENTS.md index cd81d83a..035d90d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -748,6 +748,8 @@ Rules that follow from this: - the per-task result is always `EMAIL_FAILED`; the typed account events (`EMAIL_AUTH_ERROR` and friends) are raised in addition and carry an `EmailErrorEvent`, never a `SendEmailResult` - the backend refuses to publish a send to a worker that is not heartbeating (`tasks.NewWorkerLiveness`), because a command queued for a dead worker is never executed and never answered - the campaign wizard (and `POST /campaigns` with `steps`) connects steps in order at creation. Routing has no implicit "next position": a step with no outgoing connection ends the flow +- the campaign task handler sends whatever pair `CalculateNextCampaignTime` returns, so the scheduler is the timing gate: when the step's hard constraints (wait_after, start date, sending windows, day capacity, mailbox min-gap) sit beyond `config.CampaignNotDueGraceSeconds`, it returns `ErrCampaignDeferred` with the slot instead of a pair, and the task reschedules without sending. Without this, any early tick (the successor task after a send, a duplicate chain, a moved slot) sends a "wait 3 days" follow-up seconds after step one +- schedule edits on an active campaign reschedule the parked wakeup (`rescheduleCampaignWakeup` in the campaign service): clearing or shortening a future start date takes effect immediately instead of when the old slot fires. PATCH `start_date`/`end_date` accept explicit `null` to clear (`models.NullableTime` distinguishes absent from null) ## Control Plane vs Execution Plane diff --git a/docs/content/docs/api/reference/campaigns.mdx b/docs/content/docs/api/reference/campaigns.mdx index 1b0f87a4..82f0d52f 100644 --- a/docs/content/docs/api/reference/campaigns.mdx +++ b/docs/content/docs/api/reference/campaigns.mdx @@ -121,8 +121,8 @@ Create a campaign. Only `name` is required, every other field is optional and ap | `risky_emails` | boolean | no | Allow sending to risky/unverified addresses. | | `cc` | string[] | no | Static CC list. | | `bcc` | string[] | no | Static BCC list. | -| `start_date` | string (RFC 3339) | no | Earliest send time. | -| `end_date` | string (RFC 3339) | no | Latest send time. | +| `start_date` | string (RFC 3339), nullable | no | Earliest send time. Today or later; omit or send `null` to start as soon as the campaign is active. | +| `end_date` | string (RFC 3339), nullable | no | Latest send time. Must be in the future; omit or send `null` for an open-ended campaign. | | `timezone` | string | no | IANA timezone for the schedule. | | `days` | integer (0-127) | no | Legacy weekday bitmask (superseded by `schedule_windows`). | | `start_time` | string | no | Legacy daily start (`HH:MM`). | @@ -200,7 +200,7 @@ Patch any subset of campaign fields. Omitted fields are left unchanged. The expl ### Request body -Every field is optional. Scalar fields use nullable pointers, so any field you send is applied. Notable fields: `name`, `description`, `status`, `stop_on_reply`, `open_tracking`, `link_tracking`, `text_only`, `daily_limit`, `unsubscribe_header`, `risky_emails`, `cc`, `bcc`, `start_date`, `end_date`, `timezone`, `days`, `start_time`, `end_time`, `schedule_windows`, `email_tags`, `folders`, `contact_order_by`, `contact_order_dir`, `contact_order_field`, `sender_strategy`, `rotation_mode`, `ramp_enabled`, `ramp_start`, `ramp_increment`, `ramp_ceiling`, `esp_match_mode`, `max_new_leads_per_day`, `prioritize_new_leads`, `tracking_domain`. +Every field is optional. Scalar fields use nullable pointers, so any field you send is applied. `start_date` and `end_date` additionally accept an explicit `null` to clear the stored date: a null `start_date` means "start now" and a null `end_date` means "run open-ended". Changing any schedule field (`start_date`, `end_date`, `timezone`, `days`, `start_time`, `end_time`, `schedule_windows`) on an active campaign reschedules its next send immediately, so clearing a future start date takes effect right away. Notable fields: `name`, `description`, `status`, `stop_on_reply`, `open_tracking`, `link_tracking`, `text_only`, `daily_limit`, `unsubscribe_header`, `risky_emails`, `cc`, `bcc`, `start_date`, `end_date`, `timezone`, `days`, `start_time`, `end_time`, `schedule_windows`, `email_tags`, `folders`, `contact_order_by`, `contact_order_dir`, `contact_order_field`, `sender_strategy`, `rotation_mode`, `ramp_enabled`, `ramp_start`, `ramp_increment`, `ramp_ceiling`, `esp_match_mode`, `max_new_leads_per_day`, `prioritize_new_leads`, `tracking_domain`. ```json { @@ -617,7 +617,7 @@ Send a one-off preview of a sequence step to a chosen recipient through a chosen `POST /campaigns/:id/start` -Start (activate) the campaign so it begins sending real mail. **Scope** `SEND_CAMPAIGNS` · **Org permission** `send_campaigns`. +Start (activate) the campaign so it begins sending real mail. Works from `draft`, any paused status, or `completed` (a campaign closed by a passed end date resumes once the date is extended or cleared; one with nothing left to send re-completes with a `400` explaining why). **Scope** `SEND_CAMPAIGNS` · **Org permission** `send_campaigns`. | Parameter | In | Type | Description | | --- | --- | --- | --- | diff --git a/docs/content/docs/guides/campaigns.mdx b/docs/content/docs/guides/campaigns.mdx index c49b0ca1..f58fbcc8 100644 --- a/docs/content/docs/guides/campaigns.mdx +++ b/docs/content/docs/guides/campaigns.mdx @@ -81,13 +81,15 @@ Campaigns send only inside the **weekly sending windows** you define, in the cam Within a window Warmbly spreads the day's emails evenly, adds random jitter, and rounds to natural send times so nothing arrives in obvious bursts. A mailbox with its own timezone is also kept inside its local business hours (`8am` to `8pm`). -Optional **campaign dates** bound when it may send. Leave both blank to run open-ended. +Optional **campaign dates** bound when it may send. Leave both blank to run open-ended. Picking today as the start date means "start now", and clearing a start date on a running campaign removes the wait entirely. Schedule changes on an active campaign take effect immediately: the next send is recomputed on save, so a campaign waiting on a future start date starts sending as soon as you clear or shorten it. ## Start and stop The play and pause buttons work from the list row or the detail view. Starting moves the campaign to **active** and begins scheduling inside your windows and limits; pausing stops new scheduling immediately and resumes from where it left off. -A campaign can pause itself: **paused, no accounts** when it loses every sender, **paused, trial expired** when a trial ends, and **auto-paused** when a guardrail trips. It moves to **finished** once every contact completes the sequence. Configured to do so, it also stops following up with a contact the moment they reply. +A campaign can pause itself: **paused, no accounts** when it loses every sender, **paused, trial expired** when a trial ends, and **auto-paused** when a guardrail trips. It moves to **finished** once every contact completes the sequence or its end date passes. Configured to do so, it also stops following up with a contact the moment they reply. + +A finished campaign can be started again: after extending or clearing its end date, or adding new leads, pressing play resumes it. If there is genuinely nothing left to send it finishes again immediately with a message saying so. ## Auto-pause guardrails diff --git a/internal/app/campaign/handlers.go b/internal/app/campaign/handlers.go index 2f536630..11ee37de 100644 --- a/internal/app/campaign/handlers.go +++ b/internal/app/campaign/handlers.go @@ -111,7 +111,32 @@ func (s *campaignService) Overview(ctx context.Context, orgID string) (*models.C } func (s *campaignService) Update(ctx context.Context, userID, query string, data *models.UpdateCampaign) (*models.Campaign, *errx.Error) { - return s.campaignRepository.Update(ctx, userID, query, data) + resp, err := s.campaignRepository.Update(ctx, userID, query, data) + if err != nil { + return nil, err + } + // A schedule edit on a running campaign must move its parked wakeup task: + // otherwise a cleared or earlier start date only takes effect when the old + // slot fires, and the campaign looks stuck until then (issue #171). + if data.TouchesSchedule() && resp != nil && resp.Status == "active" { + s.rescheduleCampaignWakeup(ctx, resp.ID) + } + return resp, nil +} + +// rescheduleCampaignWakeup drops the campaign's parked pending tasks and seeds +// a fresh wakeup from the just-saved schedule. Best-effort: enqueue handles the +// no-mailbox/completed cases itself, and the campaign reconciler re-seeds any +// chain left without a task. +func (s *campaignService) rescheduleCampaignWakeup(ctx context.Context, campaignID uuid.UUID) { + if s.taskRepo != nil { + if pending, err := s.campaignRepository.GetPendingCampaignTasks(ctx, campaignID); err == nil { + for _, t := range pending { + _ = s.taskRepo.DeleteTask(ctx, t.ID) + } + } + } + _ = s.enqueueCampaignWakeup(ctx, campaignID) } func (s *campaignService) Delete(ctx context.Context, userID, id string) *errx.Error { @@ -147,12 +172,16 @@ func (s *campaignService) StartCampaign(ctx context.Context, orgID uuid.UUID, ca // Verify status allows starting. paused_guardrail is included: an // auto-pause is meant to be reviewed and then explicitly restarted, not to - // become a dead end the owner cannot recover from. + // become a dead end the owner cannot recover from. completed is included + // for the same reason: a campaign closed by a passed end date must be + // restartable once the date is extended or cleared, and a truly finished + // one just re-completes in enqueueCampaignWakeup with a clear message. startable := map[string]bool{ - "draft": true, "paused": true, "paused_no_accounts": true, "paused_guardrail": true, + "draft": true, "paused": true, "paused_no_accounts": true, + "paused_guardrail": true, "completed": true, } if !startable[campaign.Status] { - return errx.New(errx.BadRequest, "campaign must be in draft, paused, paused_no_accounts, or paused_guardrail status to start") + return errx.New(errx.BadRequest, "campaign must be in draft, paused, or completed status to start") } // Check cooldown @@ -276,6 +305,9 @@ func (s *campaignService) enqueueCampaignWakeup(ctx context.Context, campaignID case errors.Is(err, scheduler.ErrCampaignCompleted): _ = s.campaignRepository.UpdateStatusWithLock(ctx, campaignID, "completed") return errx.New(errx.BadRequest, "campaign has no remaining contacts to send") + case errors.Is(err, scheduler.ErrCampaignEnded): + _ = s.campaignRepository.UpdateStatusWithLock(ctx, campaignID, "completed") + return errx.New(errx.BadRequest, "campaign is past its end date; extend or clear the end date to keep sending") default: sentry.CaptureException(err) return errx.InternalError() diff --git a/internal/config/constants.go b/internal/config/constants.go index 66e8db6a..461680ea 100644 --- a/internal/config/constants.go +++ b/internal/config/constants.go @@ -79,6 +79,13 @@ const ( // tick retries it; this bounds that loop for a mailbox that can never send. CampaignSendMaxAttempts = 5 + // CampaignNotDueGraceSeconds is how far in the future a step's hard + // constraints (wait_after, start date, sending window, mailbox min-gap) + // may sit while a firing task still sends it. Beyond this the scheduler + // reports the step deferred so the task reschedules instead of sending a + // follow-up early; a task that fired on time always passes. + CampaignNotDueGraceSeconds = 60 + // Webhook/integration fan-out throttle. Caps how many events of a single // type one org can fan out to its webhooks + integration sinks // (Slack/Discord/CRM) per minute — the backstop against a campaign "notify" diff --git a/internal/errx/common.go b/internal/errx/common.go index 78376e6c..80c8f4b1 100644 --- a/internal/errx/common.go +++ b/internal/errx/common.go @@ -129,7 +129,7 @@ var ( ErrCampaignName = New(BadRequest, "Campaign name length must be between 3 and 50 characters.") ErrCampaignDescription = New(BadRequest, "Campaign description length must be below 300 characters.") ErrCampaignDailyLimit = New(BadRequest, "Daily limit must be between 3 and 10000000.") - ErrCampaignStartDate = New(BadRequest, "Start date must be in the future, use null if you want to start now.") + ErrCampaignStartDate = New(BadRequest, "Start date cannot be in the past. Pick today or later, or clear it (null) to start right away.") ErrCampaignEndDate = New(BadRequest, "End date must be in the future.") ErrCampaignLimit = New(BadRequest, "You reached your limit for campaigns, please try again later.") diff --git a/internal/models/campaign.go b/internal/models/campaign.go index 732cde21..39152452 100644 --- a/internal/models/campaign.go +++ b/internal/models/campaign.go @@ -235,12 +235,14 @@ type UpdateCampaign struct { CC []string `json:"cc"` BCC []string `json:"bcc"` - StartDate *time.Time `json:"start_date"` - EndDate *time.Time `json:"end_date"` - Timezone *string `json:"timezone"` - Days *uint8 `json:"days"` - StartTime *string `json:"start_time"` - EndTime *string `json:"end_time"` + // Absent leaves the stored date untouched; an explicit null clears it + // ("start now" / "no end date"), matching the validation error's promise. + StartDate NullableTime `json:"start_date"` + EndDate NullableTime `json:"end_date"` + Timezone *string `json:"timezone"` + Days *uint8 `json:"days"` + StartTime *string `json:"start_time"` + EndTime *string `json:"end_time"` // Authoritative per-day schedule. When sent, supersedes Days/StartTime/EndTime. ScheduleWindows *ScheduleWindows `json:"schedule_windows,omitempty"` @@ -278,6 +280,13 @@ type UpdateCampaign struct { GuardrailWindowDays *int `json:"guardrail_window_days,omitempty"` } +// TouchesSchedule reports whether the patch changes anything the campaign's +// next wakeup time is computed from. +func (u *UpdateCampaign) TouchesSchedule() bool { + return u.StartDate.Set || u.EndDate.Set || u.Timezone != nil || u.Days != nil || + u.StartTime != nil || u.EndTime != nil || u.ScheduleWindows != nil +} + // CreateCampaign is the payload accepted by POST /campaigns. Name is required; // every other field is optional and only applied if the caller sent a non-nil // value. The wizard sends everything at once; the simple modal can still send diff --git a/internal/models/nullable.go b/internal/models/nullable.go new file mode 100644 index 00000000..ebfbc76c --- /dev/null +++ b/internal/models/nullable.go @@ -0,0 +1,29 @@ +package models + +import ( + "encoding/json" + "time" +) + +// NullableTime distinguishes an absent JSON field from an explicit null in +// PATCH payloads. Absent leaves the column untouched; null clears it. +type NullableTime struct { + Set bool `json:"-"` + Value *time.Time `json:"-"` +} + +func (n *NullableTime) UnmarshalJSON(b []byte) error { + n.Set = true + if string(b) == "null" { + n.Value = nil + return nil + } + return json.Unmarshal(b, &n.Value) +} + +func (n NullableTime) MarshalJSON() ([]byte, error) { + if n.Value == nil { + return []byte("null"), nil + } + return json.Marshal(n.Value) +} diff --git a/internal/models/nullable_test.go b/internal/models/nullable_test.go new file mode 100644 index 00000000..64eb5f10 --- /dev/null +++ b/internal/models/nullable_test.go @@ -0,0 +1,52 @@ +package models + +import ( + "encoding/json" + "testing" + "time" +) + +// Issue #171: PATCH /campaigns must distinguish an absent start_date (leave +// untouched) from an explicit null (clear it / start now). +func TestNullableTimeAbsentVsNullVsValue(t *testing.T) { + var u UpdateCampaign + if err := json.Unmarshal([]byte(`{}`), &u); err != nil { + t.Fatal(err) + } + if u.StartDate.Set { + t.Fatal("absent field must not be marked Set") + } + + u = UpdateCampaign{} + if err := json.Unmarshal([]byte(`{"start_date":null}`), &u); err != nil { + t.Fatal(err) + } + if !u.StartDate.Set || u.StartDate.Value != nil { + t.Fatalf("explicit null must be Set with nil Value, got Set=%v Value=%v", u.StartDate.Set, u.StartDate.Value) + } + + u = UpdateCampaign{} + if err := json.Unmarshal([]byte(`{"start_date":"2030-01-02T00:00:00Z"}`), &u); err != nil { + t.Fatal(err) + } + want := time.Date(2030, 1, 2, 0, 0, 0, 0, time.UTC) + if !u.StartDate.Set || u.StartDate.Value == nil || !u.StartDate.Value.Equal(want) { + t.Fatalf("value must round-trip, got Set=%v Value=%v", u.StartDate.Set, u.StartDate.Value) + } +} + +func TestUpdateCampaignTouchesSchedule(t *testing.T) { + var u UpdateCampaign + if err := json.Unmarshal([]byte(`{"name":"x"}`), &u); err != nil { + t.Fatal(err) + } + if u.TouchesSchedule() { + t.Fatal("a name-only patch must not touch the schedule") + } + if err := json.Unmarshal([]byte(`{"start_date":null}`), &u); err != nil { + t.Fatal(err) + } + if !u.TouchesSchedule() { + t.Fatal("clearing start_date must count as a schedule change") + } +} diff --git a/internal/repository/pg_campaign.go b/internal/repository/pg_campaign.go index 712f7682..626f04dc 100644 --- a/internal/repository/pg_campaign.go +++ b/internal/repository/pg_campaign.go @@ -916,20 +916,24 @@ func (r *campaignRepository) Update(ctx context.Context, userID, campaignID stri args = append(args, data.BCC) argPos++ } - if data.StartDate != nil { - if err := validate.CampaignStartDate(*data.StartDate); err != nil { - return nil, err + if data.StartDate.Set { + if data.StartDate.Value != nil { + if err := validate.CampaignStartDate(*data.StartDate.Value); err != nil { + return nil, err + } } setClauses = append(setClauses, fmt.Sprintf("%s = $%d", "start_date", argPos)) - args = append(args, *data.StartDate) + args = append(args, data.StartDate.Value) // nil clears (start now) argPos++ } - if data.EndDate != nil { - if err := validate.CampaignEndDate(*data.EndDate); err != nil { - return nil, err + if data.EndDate.Set { + if data.EndDate.Value != nil { + if err := validate.CampaignEndDate(*data.EndDate.Value); err != nil { + return nil, err + } } setClauses = append(setClauses, fmt.Sprintf("%s = $%d", "end_date", argPos)) - args = append(args, *data.EndDate) + args = append(args, data.EndDate.Value) // nil clears (open-ended) argPos++ } if data.Timezone != nil { diff --git a/internal/sandbox/seed.go b/internal/sandbox/seed.go index 07265cb1..dc8337cc 100644 --- a/internal/sandbox/seed.go +++ b/internal/sandbox/seed.go @@ -406,10 +406,14 @@ func seedIdentity(ctx context.Context, pool *pgxpool.Pool) error { // placement and the reconciler treat it as live; the real worker process adopts // the row on its first heartbeat. func seedWorker(ctx context.Context, pool *pgxpool.Pool) error { + // The conflict branch must also assert the tier tuple: `make seed` upserts + // this same UUID as free-tier, and the sandbox org is on a paid plan, so a + // leftover free-tier row gets its mailboxes unassigned by placement and + // every send fails with "no available workers". _, err := pool.Exec(ctx, ` INSERT INTO workers (id, name, notes, ip_addr, active, worker_type, account_count, free_tier) VALUES ($1, 'worker-sandbox-1', 'Sandbox worker (make sandbox / make worker)', '127.0.0.1', TRUE, 'shared', 0, FALSE) - ON CONFLICT (id) DO UPDATE SET active = TRUE, updated_at = NOW()`, + ON CONFLICT (id) DO UPDATE SET active = TRUE, worker_type = 'shared', free_tier = FALSE, updated_at = NOW()`, sandboxWorker) return err } diff --git a/internal/scheduler/campaign_scheduler.go b/internal/scheduler/campaign_scheduler.go index 5a0a9cbd..6406c60a 100644 --- a/internal/scheduler/campaign_scheduler.go +++ b/internal/scheduler/campaign_scheduler.go @@ -6,6 +6,7 @@ import ( "time" "github.com/google/uuid" + "github.com/warmbly/warmbly/internal/config" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/repository" ) @@ -516,6 +517,12 @@ func (s *schedulerService) CalculateNextCampaignTime(ctx context.Context, campai candidateTime = *selected.BehaviorOpenAt } + // hardFloor is the earliest moment this send is ALLOWED: wait_after, + // start date, sending windows, day capacity and workday placement so far, + // with the mailbox min-gap folded in below. Pacing added after this point + // (distribution, jitter, curve) shapes the slot but never gates a send. + hardFloor := candidateTime + // STEP 9: Even distribution across the candidate day's sending window. With // a behaviour profile that is the mailbox's own rolled workday (lunch // excluded); otherwise it is the span of the campaign's intervals for that @@ -554,6 +561,9 @@ func (s *schedulerService) CalculateNextCampaignTime(ctx context.Context, campai // Re-snap into a sending window after adjusting for min wait. candidateTime = nextScheduleSlot(candidateTime, windows, campaignTZ) } + if hardFloor.Before(earliestNext) { + hardFloor = nextScheduleSlot(earliestNext, windows, campaignTZ) + } } // STEP 11: Add jitter. Deliberately NOT rounded to a 5-minute grid — a @@ -583,6 +593,16 @@ func (s *schedulerService) CalculateNextCampaignTime(ctx context.Context, campai // out of the mailbox's workday). Satisfies both calendars. candidateTime = s.intersectWindows(ctx, selected.Behavior, notBefore(candidateTime), windows, campaignTZ) + // STEP 14.5: A task can fire ahead of the step's hard constraints — an + // early successor tick, a duplicate chain, a moved slot. The task handler + // sends whatever pair this function returns, so returning a not-yet-due + // pair here is what sends a "wait 3 days" follow-up seconds after step + // one. Report it deferred instead: the caller reschedules at the computed + // slot without sending. A task that fired at its own slot always passes. + if time.Until(hardFloor) > config.CampaignNotDueGraceSeconds*time.Second { + return finalSlot(candidateTime), nil, account.ID, ErrCampaignDeferred + } + // STEP 15: Randomise the sub-minute component so sends never land on :00. return finalSlot(candidateTime), nextPair, account.ID, nil } diff --git a/internal/scheduler/live_integration_test.go b/internal/scheduler/live_integration_test.go index 728e6118..631c58b7 100644 --- a/internal/scheduler/live_integration_test.go +++ b/internal/scheduler/live_integration_test.go @@ -2,6 +2,7 @@ package scheduler import ( "context" + "errors" "os" "testing" "time" @@ -172,6 +173,19 @@ func liveScheduler(t *testing.T, handle *db.DB, pool *pgxpool.Pool) SchedulerSer return s } +// scheduleSlot runs a scheduling pass and returns the computed slot. A pass +// whose slot sits beyond the not-due grace now (correctly) reports +// ErrCampaignDeferred instead of handing back a sendable pair; for these +// placement assertions the slot is what matters, so both outcomes pass. +func scheduleSlot(t *testing.T, s SchedulerService, campaign uuid.UUID) (time.Time, uuid.UUID) { + t.Helper() + at, _, accountID, err := s.CalculateNextCampaignTime(context.Background(), campaign) + if err != nil && !errors.Is(err, ErrCampaignDeferred) { + t.Fatalf("schedule: %v", err) + } + return at, accountID +} + func liveProfile() models.SendingBehavior { b := models.DefaultSendingBehavior(uuid.Nil) b.Enabled = true @@ -193,10 +207,7 @@ func TestLiveCampaignSendLandsInsideTheRolledWorkday(t *testing.T) { b.Weekdays = models.BehaviorWeekdaysAll f.setProfile(t, b) - at, _, accountID, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, accountID := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) if accountID != f.mailbox { t.Fatalf("picked mailbox %s, want the fixture's %s", accountID, f.mailbox) @@ -236,10 +247,7 @@ func TestLiveSendSkipsTheLunchBreak(t *testing.T) { b.Weekdays = models.BehaviorWeekdaysAll f.setProfile(t, b) - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) @@ -269,10 +277,7 @@ func TestLiveNonWorkingDayRollsForward(t *testing.T) { b.Weekdays = 1 << ((int(target) + 6) % 7) f.setProfile(t, b) - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) if at.UTC().Weekday() != target { t.Fatalf("scheduled on %s, want the only working day %s (%s)", @@ -298,10 +303,7 @@ func TestLiveTimezoneIsTheMailboxOwn(t *testing.T) { b.Weekdays = models.BehaviorWeekdaysAll f.setProfile(t, b) - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) @@ -328,10 +330,7 @@ func TestLiveDisabledProfileChangesNothing(t *testing.T) { b.LunchEnabled = false f.setProfile(t, b) - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) // With behaviour off the send is placed by the legacy path, which is free to @@ -409,10 +408,7 @@ func TestLiveHourlyCeilingPushesToALaterHour(t *testing.T) { now := time.Now().UTC() f.bookTask(t, now) - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) if at.UTC().Truncate(time.Hour).Equal(now.Truncate(time.Hour)) { @@ -436,10 +432,7 @@ func TestLiveDailyCeilingPushesToTheNextDay(t *testing.T) { now := time.Now().UTC() f.bookTask(t, now) // spends the whole day - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) if at.UTC().Format("2006-01-02") == now.Format("2006-01-02") { @@ -474,10 +467,7 @@ func TestLiveGapIsDrawnFromTheProfile(t *testing.T) { t.Fatalf("seed completed task: %v", err) } - at, _, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(context.Background(), f.campaign) - if err != nil { - t.Fatalf("schedule: %v", err) - } + at, _ := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign) assertFuture(t, at) gap := time.Until(at) @@ -487,3 +477,62 @@ func TestLiveGapIsDrawnFromTheProfile(t *testing.T) { } t.Logf("next send %s away — the profile's one-hour band, not the mailbox's 600s gap", gap.Round(time.Second)) } + +// TestLiveFollowUpWaitIsHonored is the regression check for follow-ups riding +// an early tick (issue #171 follow-up): once step 1 is sent, a scheduling pass +// that runs immediately afterwards must NOT hand back the "wait 3 days" step as +// sendable. It must report the campaign deferred, with the slot parked at the +// follow-up's real time, so the task handler reschedules instead of sending a +// three-day follow-up seconds after step one. +func TestLiveFollowUpWaitIsHonored(t *testing.T) { + handle, pool := liveDB(t) + f := newLiveFixture(t, pool, "UTC") + ctx := context.Background() + + step2 := uuid.New() + if _, err := pool.Exec(ctx, ` + INSERT INTO sequences (id, campaign_id, organization_id, name, subject, + body_plain, body_html, wait_after, position, kind) + VALUES ($1, $2, $3, 'Step 2', 'Bump', 'Bump', '

Bump

', 3, 1, 'email')`, + step2, f.campaign, f.org); err != nil { + t.Fatalf("insert step 2: %v", err) + } + + var step1, contact uuid.UUID + if err := pool.QueryRow(ctx, + `SELECT id FROM sequences WHERE campaign_id = $1 AND position = 0`, f.campaign).Scan(&step1); err != nil { + t.Fatalf("load step 1: %v", err) + } + if err := pool.QueryRow(ctx, + `SELECT contact_id FROM campaign_leads WHERE campaign_id = $1`, f.campaign).Scan(&contact); err != nil { + t.Fatalf("load contact: %v", err) + } + + // Connect step 1 -> step 2 (routing has no implicit next step; the wizard + // writes this catch-all branch at creation). + if _, err := pool.Exec(ctx, ` + UPDATE sequences SET conditions = jsonb_build_object('branches', jsonb_build_array( + jsonb_build_object('branch_id', 'live-else', 'target_step_id', $1::text))) + WHERE campaign_id = $2 AND position = 0`, step2.String(), f.campaign); err != nil { + t.Fatalf("connect steps: %v", err) + } + + // Step 1 went out moments ago; routing now points at the wait-gated step 2. + if _, err := pool.Exec(ctx, ` + INSERT INTO campaign_contact_progress (campaign_id, contact_id, sequence_id, sent_at) + VALUES ($1, $2, $3, NOW())`, f.campaign, contact, step1); err != nil { + t.Fatalf("stamp step 1 sent: %v", err) + } + + at, pair, _, err := liveScheduler(t, handle, pool).CalculateNextCampaignTime(ctx, f.campaign) + if !errors.Is(err, ErrCampaignDeferred) { + t.Fatalf("want ErrCampaignDeferred for a not-yet-due follow-up, got pair=%v err=%v at=%s", pair, err, at) + } + if pair != nil { + t.Fatal("a deferred result must not carry a sendable pair") + } + if time.Until(at) < 47*time.Hour { + t.Fatalf("deferred slot %s is too soon for a 3-day wait", at) + } + t.Logf("follow-up correctly deferred to %s", at.UTC().Format("2006-01-02 15:04:05")) +} diff --git a/internal/utils/validate/campaign.go b/internal/utils/validate/campaign.go index 179eb691..51510a1a 100644 --- a/internal/utils/validate/campaign.go +++ b/internal/utils/validate/campaign.go @@ -32,8 +32,12 @@ func CampaignDailyLimit(val int) *errx.Error { return nil } +// CampaignStartDate accepts today and future dates. The dashboard's date +// picker sends midnight in the user's timezone, so "today" always sits a few +// hours in the past at submit time; a 24h grace keeps "today = start now" +// working from any timezone while still rejecting genuinely past dates. func CampaignStartDate(date time.Time) *errx.Error { - if !date.After(time.Now()) { + if date.Before(time.Now().Add(-24 * time.Hour)) { return errx.ErrCampaignStartDate } return nil diff --git a/internal/utils/validate/campaign_test.go b/internal/utils/validate/campaign_test.go new file mode 100644 index 00000000..d919006b --- /dev/null +++ b/internal/utils/validate/campaign_test.go @@ -0,0 +1,32 @@ +package validate + +import ( + "testing" + "time" +) + +// Issue #171: the dashboard's date picker sends midnight in the user's +// timezone, so "today" is always a few hours in the past at submit time and +// must be accepted as "start now". +func TestCampaignStartDateAcceptsToday(t *testing.T) { + localMidnight := time.Now().Truncate(24 * time.Hour) + if err := CampaignStartDate(localMidnight); err != nil { + t.Fatalf("today at midnight should be accepted, got %v", err) + } + // Worst-case timezone offset: a "today" pick is never more than 24h old. + if err := CampaignStartDate(time.Now().Add(-23 * time.Hour)); err != nil { + t.Fatalf("a date within the last 24h should be accepted, got %v", err) + } +} + +func TestCampaignStartDateRejectsPast(t *testing.T) { + if err := CampaignStartDate(time.Now().Add(-48 * time.Hour)); err == nil { + t.Fatal("a date two days in the past should be rejected") + } +} + +func TestCampaignStartDateAcceptsFuture(t *testing.T) { + if err := CampaignStartDate(time.Now().Add(72 * time.Hour)); err != nil { + t.Fatalf("a future date should be accepted, got %v", err) + } +} diff --git a/web/src/app/app/campaigns/[id]/schedule/page.tsx b/web/src/app/app/campaigns/[id]/schedule/page.tsx index 7e3b22d3..4c99aaa1 100644 --- a/web/src/app/app/campaigns/[id]/schedule/page.tsx +++ b/web/src/app/app/campaigns/[id]/schedule/page.tsx @@ -1,7 +1,7 @@ import PermissionButton from "@/components/ui/PermissionButton"; import React from "react"; import { ArrowRightIcon, CalendarClockIcon, CalendarRangeIcon, GlobeIcon } from "lucide-react"; -import { differenceInCalendarDays, format } from "date-fns"; +import { addDays, differenceInCalendarDays, format } from "date-fns"; import DateSelect from "@/components/app/campaigns/schedule/ScheduleDateSelect"; import WeekScheduleGrid, { type Interval } from "@/components/app/campaigns/schedule/WeekScheduleGrid"; import { Loading } from "@/components/loader"; @@ -246,6 +246,7 @@ export default function CampaignSchedule() { title="Start date" value={newData.start_date ?? null} onChange={(v) => setNewData((b) => ({ ...b, start_date: v }))} + minDate={new Date()} /> @@ -254,6 +255,7 @@ export default function CampaignSchedule() { title="End date" value={newData.end_date ?? null} onChange={(v) => setNewData((b) => ({ ...b, end_date: v }))} + minDate={addDays(new Date(), 1)} />

diff --git a/web/src/components/app/Calendar.tsx b/web/src/components/app/Calendar.tsx index bb6ba399..9e5ea1a6 100644 --- a/web/src/components/app/Calendar.tsx +++ b/web/src/components/app/Calendar.tsx @@ -13,6 +13,8 @@ import { isSameMonth, isSameDay, isToday, + isBefore, + startOfDay, } from "date-fns"; import { ChevronLeftIcon, ChevronRightIcon } from "lucide-react"; @@ -21,11 +23,14 @@ export default function Calendar({ active, close, onSubmit, + minDate, }: { date: null | Date, active: boolean, close: () => void; onSubmit: (d: Date | null) => void, + /** Days before this date render disabled (compared at day granularity). */ + minDate?: Date, }) { const [currentMonth, setCurrentMonth] = React.useState(date || new Date()); @@ -171,23 +176,27 @@ export default function Calendar({ const isSelected = date && isSameDay(day, date); const isCurrentDay = isToday(day); const isCurrentMonth = isSameMonth(day, currentMonth); + const isDisabled = !!minDate && isBefore(day, startOfDay(minDate)); return ( - setOpen(false)} onSubmit={onChange} /> + setOpen(false)} onSubmit={onChange} minDate={minDate} /> );