diff --git a/docs/content/docs/api/reference/campaigns.mdx b/docs/content/docs/api/reference/campaigns.mdx index c9a76054..3002eb44 100644 --- a/docs/content/docs/api/reference/campaigns.mdx +++ b/docs/content/docs/api/reference/campaigns.mdx @@ -171,6 +171,7 @@ Create a campaign. Only `name` is required, every other field is optional and ap | `days` | integer (0-127) | no | Legacy weekday bitmask (superseded by `schedule_windows`). | | `start_time` | string | no | Legacy daily start (`HH:MM`). | | `end_time` | string | no | Legacy daily end (`HH:MM`). | +| `schedule_windows` | array | no | Per-day sending windows, 7 arrays indexed by weekday (Sunday = 0) of `{start, end}` minute-of-day intervals. When non-empty it supersedes `days`, `start_time` and `end_time`. | | `email_tag_ids` | string[] | no | Mailbox tag ids that resolve the sender pool (tags strategy). | | `folder_ids` | string[] | no | Folder ids to file the campaign under. | | `sender_strategy` | string | no | `tags` (default) or `explicit`. | diff --git a/internal/models/campaign.go b/internal/models/campaign.go index e65a1ecb..f1b4603e 100644 --- a/internal/models/campaign.go +++ b/internal/models/campaign.go @@ -376,6 +376,9 @@ type CreateCampaign struct { StartTime *string `json:"start_time,omitempty"` EndTime *string `json:"end_time,omitempty"` + // Authoritative per-day schedule. When sent, supersedes Days/StartTime/EndTime. + ScheduleWindows *ScheduleWindows `json:"schedule_windows,omitempty"` + // Sender pool — accepts UUIDs already created by the user. EmailTagIDs []string `json:"email_tag_ids,omitempty"` FolderIDs []string `json:"folder_ids,omitempty"` diff --git a/internal/repository/campaign_create_schedule_windows_live_test.go b/internal/repository/campaign_create_schedule_windows_live_test.go new file mode 100644 index 00000000..811f9681 --- /dev/null +++ b/internal/repository/campaign_create_schedule_windows_live_test.go @@ -0,0 +1,108 @@ +package repository + +import ( + "context" + "reflect" + "testing" + + "github.com/warmbly/warmbly/internal/models" +) + +// Regression cover for issue #307 item 5: POST /campaigns accepted +// schedule_windows but Create did not persist the authoritative schedule. +// +// Run against the dev stack: +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \ +// go test ./internal/repository/ -run LiveCreateCampaignPersistsScheduleWindows -v +func TestLiveCreateCampaignPersistsScheduleWindows(t *testing.T) { + handle, pool := liveContactDB(t) + f := newSharedOrgFixture(t, pool) + repo := NewCampaignRepostory(handle) + ctx := context.Background() + + t.Run("persists supplied windows", func(t *testing.T) { + want := models.ScheduleWindows{ + 1: []models.TimeInterval{{Start: 540, End: 1020}}, + 5: []models.TimeInterval{{Start: 540, End: 840}}, + } + campaign, xerr := repo.Create(ctx, f.owner.String(), &f.org, &models.CreateCampaign{ + Name: "Issue 307 schedule windows", + ScheduleWindows: &want, + }) + if xerr != nil { + t.Fatalf("Create: %v", xerr) + } + if !reflect.DeepEqual(campaign.ScheduleWindows, want) { + t.Fatalf("Create schedule_windows = %#v, want %#v", campaign.ScheduleWindows, want) + } + + var notNull bool + var stored models.ScheduleWindows + if err := pool.QueryRow(ctx, + `SELECT schedule_windows IS NOT NULL, schedule_windows FROM campaigns WHERE id = $1`, + campaign.ID).Scan(¬Null, &stored); err != nil { + t.Fatalf("select schedule_windows: %v", err) + } + if !notNull { + t.Fatal("database schedule_windows is NULL, want supplied windows") + } + if !reflect.DeepEqual(stored, want) { + t.Fatalf("database schedule_windows = %#v, want %#v", stored, want) + } + + got, err := repo.Get(ctx, f.org.String(), campaign.ID.String()) + if err != nil { + t.Fatalf("Get: %v", err) + } + if !reflect.DeepEqual(got.ScheduleWindows, want) { + t.Fatalf("Get schedule_windows = %#v, want %#v", got.ScheduleWindows, want) + } + }) + + t.Run("keeps legacy schedule null when omitted", func(t *testing.T) { + campaign, xerr := repo.Create(ctx, f.owner.String(), &f.org, &models.CreateCampaign{ + Name: "Issue 307 legacy schedule", + }) + if xerr != nil { + t.Fatalf("Create: %v", xerr) + } + if !campaign.ScheduleWindows.IsEmpty() { + t.Fatalf("Create schedule_windows = %#v, want empty", campaign.ScheduleWindows) + } + + var isNull bool + if err := pool.QueryRow(ctx, + `SELECT schedule_windows IS NULL FROM campaigns WHERE id = $1`, + campaign.ID).Scan(&isNull); err != nil { + t.Fatalf("select schedule_windows null state: %v", err) + } + if !isNull { + t.Fatal("database schedule_windows is not NULL, want legacy NULL") + } + }) + + t.Run("rejects invalid windows without inserting", func(t *testing.T) { + const name = "Issue 307 invalid schedule" + bad := models.ScheduleWindows{ + 1: []models.TimeInterval{{Start: 600, End: 540}}, + } + campaign, xerr := repo.Create(ctx, f.owner.String(), &f.org, &models.CreateCampaign{ + Name: name, + ScheduleWindows: &bad, + }) + if xerr == nil { + t.Fatalf("Create = %#v, nil error; want validation error", campaign) + } + + var count int + if err := pool.QueryRow(ctx, + `SELECT count(*) FROM campaigns WHERE organization_id = $1 AND name = $2`, + f.org, name).Scan(&count); err != nil { + t.Fatalf("count invalid campaign rows: %v", err) + } + if count != 0 { + t.Fatalf("invalid campaign rows = %d, want 0", count) + } + }) +} diff --git a/internal/repository/campaign_create_schedule_windows_test.go b/internal/repository/campaign_create_schedule_windows_test.go new file mode 100644 index 00000000..6bb04d09 --- /dev/null +++ b/internal/repository/campaign_create_schedule_windows_test.go @@ -0,0 +1,57 @@ +package repository + +import ( + "context" + "testing" + + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" +) + +func TestCreateCampaignRejectsInvalidScheduleWindows(t *testing.T) { + requireBadRequest := func(t *testing.T, got *errx.Error) { + t.Helper() + if got == nil { + t.Fatal("Create() error = nil, want *errx.Error") + } + if got.Code != errx.BadRequest { + t.Fatalf("Create() error code = %v, want %v", got.Code, errx.BadRequest) + } + } + + nineIntervals := make([]models.TimeInterval, 9) + for i := range nineIntervals { + nineIntervals[i] = models.TimeInterval{Start: i * 10, End: i*10 + 5} + } + + tests := []struct { + name string + windows models.ScheduleWindows + }{ + { + name: "end before start", + windows: models.ScheduleWindows{ + 1: []models.TimeInterval{{Start: 600, End: 540}}, + }, + }, + { + name: "more than eight intervals", + windows: models.ScheduleWindows{ + 1: nineIntervals, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + orgID := uuid.New() + _, got := (&campaignRepository{}).Create(context.Background(), "u", &orgID, &models.CreateCampaign{ + Name: "x", + ScheduleWindows: &tt.windows, + }) + requireBadRequest(t, got) + }) + } +} diff --git a/internal/repository/pg_campaign.go b/internal/repository/pg_campaign.go index 347875fc..4ca7cf17 100644 --- a/internal/repository/pg_campaign.go +++ b/internal/repository/pg_campaign.go @@ -239,6 +239,13 @@ func (r *campaignRepository) Create(ctx context.Context, userID string, orgID *u } endTime = *data.EndTime } + var scheduleWindows models.ScheduleWindows + if data.ScheduleWindows != nil { + if err := validate.CampaignScheduleWindows(data.ScheduleWindows); err != nil { + return nil, err + } + scheduleWindows = *data.ScheduleWindows + } if data.StartDate != nil { if err := validate.CampaignStartDate(*data.StartDate); err != nil { return nil, err @@ -454,7 +461,7 @@ func (r *campaignRepository) Create(ctx context.Context, userID string, orgID *u stop_on_reply, open_tracking, link_tracking, text_only, daily_limit, unsubscribe_header, risky_emails, cc_addr, bcc_addr, - start_date, end_date, timezone, days, start_time, end_time, + start_date, end_date, timezone, days, start_time, end_time, schedule_windows, sender_strategy, rotation_mode, ramp_enabled, ramp_start, ramp_increment, ramp_ceiling, esp_match_mode, max_new_leads_per_day, prioritize_new_leads, @@ -467,13 +474,13 @@ func (r *campaignRepository) Create(ctx context.Context, userID string, orgID *u $5, $6, $7, $8, $9, $10, $11, $12, $13, - $14, $15, $16, $17, $18, $19, - $20, $21, - $22, $23, $24, $25, - $26, $27, $28, - $29, $30, - $31, $32, $33, $34, - $35, + $14, $15, $16, $17, $18, $19, $20, + $21, $22, + $23, $24, $25, $26, + $27, $28, $29, + $30, $31, + $32, $33, $34, $35, + $36, NOW(), NOW() ) RETURNING %s @@ -499,22 +506,23 @@ func (r *campaignRepository) Create(ctx context.Context, userID string, orgID *u days, // $17 startTime, // $18 endTime, // $19 - senderStrategy, // $20 - rotationMode, // $21 - rampEnabled, // $22 - rampStart, // $23 - rampIncrement, // $24 - rampCeiling, // $25 - espMatchMode, // $26 - maxNewLeads, // $27 - prioritizeNewLeads, // $28 - trackingDomain, // $29 - kind, // $30 - utmTracking, // $31 - utmSource, // $32 - utmMedium, // $33 - utmCampaign, // $34 - unsubMode, // $35 + scheduleWindows, // $20 + senderStrategy, // $21 + rotationMode, // $22 + rampEnabled, // $23 + rampStart, // $24 + rampIncrement, // $25 + rampCeiling, // $26 + espMatchMode, // $27 + maxNewLeads, // $28 + prioritizeNewLeads, // $29 + trackingDomain, // $30 + kind, // $31 + utmTracking, // $32 + utmSource, // $33 + utmMedium, // $34 + utmCampaign, // $35 + unsubMode, // $36 } row := tx.QueryRow(ctx, insertSQL, params...)