mirror of
https://github.com/warmbly/warmbly.git
synced 2026-09-06 08:01:24 +00:00
Merge pull request #172 from warmbly/fix/171-campaign-stuck-processing-recurrence
Fix campaign start-date dead ends behind the #161 recurrence, and stop early ticks from sending follow-ups instantly
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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 |
|
||||
| --- | --- | --- | --- |
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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.")
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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', '<p>Bump</p>', 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"))
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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()}
|
||||
/>
|
||||
</div>
|
||||
<ArrowRightIcon className="w-4 h-4 text-slate-300 shrink-0 self-end mb-1.5 hidden sm:block" />
|
||||
@@ -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)}
|
||||
/>
|
||||
</div>
|
||||
<p className="text-[11px] text-slate-400 self-end mb-1 flex-1 sm:min-w-[180px]">
|
||||
|
||||
@@ -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 (
|
||||
<button
|
||||
key={day.toString()}
|
||||
type="button"
|
||||
disabled={isDisabled}
|
||||
onClick={() => {
|
||||
handleDateSelect(day);
|
||||
if (!isCurrentMonth) setCurrentMonth(day);
|
||||
}}
|
||||
className={`aspect-square w-full inline-flex items-center justify-center text-[12px] rounded-md transition-colors ${
|
||||
isSelected
|
||||
? "bg-sky-600 text-white font-medium"
|
||||
: isCurrentDay
|
||||
? "bg-sky-50 text-sky-700 font-medium hover:bg-sky-100"
|
||||
: isCurrentMonth
|
||||
? "text-slate-700 hover:bg-slate-100"
|
||||
: "text-slate-300 hover:bg-slate-50"
|
||||
isDisabled
|
||||
? "text-slate-300 cursor-not-allowed"
|
||||
: isSelected
|
||||
? "bg-sky-600 text-white font-medium"
|
||||
: isCurrentDay
|
||||
? "bg-sky-50 text-sky-700 font-medium hover:bg-sky-100"
|
||||
: isCurrentMonth
|
||||
? "text-slate-700 hover:bg-slate-100"
|
||||
: "text-slate-300 hover:bg-slate-50"
|
||||
}`}
|
||||
>
|
||||
{format(day, "d")}
|
||||
|
||||
@@ -8,10 +8,12 @@ export default function DateSelect({
|
||||
title,
|
||||
value,
|
||||
onChange,
|
||||
minDate,
|
||||
}: {
|
||||
title: string;
|
||||
value: Date | null;
|
||||
onChange: (v: Date | null) => void;
|
||||
minDate?: Date;
|
||||
}) {
|
||||
const [open, setOpen] = React.useState(false);
|
||||
const modalRef = React.useRef<HTMLDivElement>(null);
|
||||
@@ -62,7 +64,7 @@ export default function DateSelect({
|
||||
<span className="text-slate-400">Not set</span>
|
||||
)}
|
||||
</button>
|
||||
<Calendar date={value} active={open} close={() => setOpen(false)} onSubmit={onChange} />
|
||||
<Calendar date={value} active={open} close={() => setOpen(false)} onSubmit={onChange} minDate={minDate} />
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user