A campaign.md is meant to stay around after it's sent as a record of what went out and when. Treating any non-draft campaign as a hard rejection forced deleting it just to keep CI green, and there was no way to fail on resolving a stale list/attachment reference either -- an archived campaign has nothing left to resolve, so check status before doing any of that work. Non-draft campaigns now report as a distinct, non-error "skipped" bucket instead of "rejected".
This commit is contained in:
+24
-12
@@ -18,7 +18,8 @@ import (
|
||||
type SyncResult struct {
|
||||
Synced []string // created or updated (content changed) in listmonk
|
||||
Unchanged []string // existing draft, content identical — no API write, no preview
|
||||
Rejected []string // validation/segmentation/list-lookup failure, or non-draft in listmonk — nothing written
|
||||
Skipped []string // already sent/running/paused/etc. in listmonk — a historical record, left untouched, not an error
|
||||
Rejected []string // validation/segmentation/list-lookup failure — nothing written
|
||||
PreviewFailed []string // content synced fine, but the automatic preview send itself failed
|
||||
}
|
||||
|
||||
@@ -45,6 +46,10 @@ func SyncDir(lm *listmonk.Client, root string, defaultPreviewEmails []string) (*
|
||||
result.Rejected = append(result.Rejected, fmt.Sprintf("%s: %v", slug, err))
|
||||
continue
|
||||
}
|
||||
if res.Skipped != "" {
|
||||
result.Skipped = append(result.Skipped, fmt.Sprintf("%s: %s", slug, res.Skipped))
|
||||
continue
|
||||
}
|
||||
if !res.Changed {
|
||||
result.Unchanged = append(result.Unchanged, slug)
|
||||
continue
|
||||
@@ -59,7 +64,8 @@ func SyncDir(lm *listmonk.Client, root string, defaultPreviewEmails []string) (*
|
||||
|
||||
type syncOneResult struct {
|
||||
Changed bool
|
||||
PreviewErr error // set only when Changed and the automatic preview send failed
|
||||
Skipped string // non-empty (the listmonk status) when left untouched because it's no longer a draft
|
||||
PreviewErr error // set only when Changed and the automatic preview send failed
|
||||
}
|
||||
|
||||
func syncOne(lm *listmonk.Client, slug, dir, path string, defaultPreviewEmails []string) (syncOneResult, error) {
|
||||
@@ -68,6 +74,22 @@ func syncOne(lm *listmonk.Client, slug, dir, path string, defaultPreviewEmails [
|
||||
return syncOneResult{}, err
|
||||
}
|
||||
|
||||
existing, err := lm.FindCampaignByName(slug)
|
||||
if err != nil {
|
||||
return syncOneResult{}, err
|
||||
}
|
||||
if existing != nil && existing.Status != "draft" {
|
||||
// Once a campaign has actually gone out (or is running/paused/etc.),
|
||||
// its campaign.md is a historical record of what was sent and when —
|
||||
// not something sync should touch or fail the build over. Forcing it
|
||||
// to be deleted just to keep CI green would throw away exactly the
|
||||
// record someone would want to look back at later. Check this before
|
||||
// resolving anything else below, so a since-renamed list or a
|
||||
// since-changed attachment on an old campaign can never break the
|
||||
// build either — an archived campaign has nothing left to resolve.
|
||||
return syncOneResult{Skipped: existing.Status}, nil
|
||||
}
|
||||
|
||||
listIDs, err := resolveLists(lm, fm.Lists)
|
||||
if err != nil {
|
||||
return syncOneResult{}, err
|
||||
@@ -97,11 +119,6 @@ func syncOne(lm *listmonk.Client, slug, dir, path string, defaultPreviewEmails [
|
||||
MediaIDs: mediaIDs,
|
||||
}
|
||||
|
||||
existing, err := lm.FindCampaignByName(slug)
|
||||
if err != nil {
|
||||
return syncOneResult{}, err
|
||||
}
|
||||
|
||||
var campaignID int
|
||||
var changed bool
|
||||
switch {
|
||||
@@ -112,11 +129,6 @@ func syncOne(lm *listmonk.Client, slug, dir, path string, defaultPreviewEmails [
|
||||
}
|
||||
campaignID, changed = created.ID, true
|
||||
|
||||
case existing.Status != "draft":
|
||||
// The core safety invariant: never silently skip or overwrite a
|
||||
// live/sent campaign. This is a hard rejection, not a warning.
|
||||
return syncOneResult{}, fmt.Errorf("campaign is %s in listmonk; sync refuses to modify a non-draft campaign", existing.Status)
|
||||
|
||||
case campaignUnchanged(existing, input):
|
||||
campaignID, changed = existing.ID, false
|
||||
|
||||
|
||||
@@ -479,7 +479,7 @@ func TestSyncDir_UpdatesWhenContentChanges(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncDir_RejectsNonDraftCampaign(t *testing.T) {
|
||||
func TestSyncDir_SkipsNonDraftCampaignWithoutFailing(t *testing.T) {
|
||||
f := newFakeListmonk(t)
|
||||
f.lists = []fakeList{{ID: 3, Name: "Newsletter"}}
|
||||
f.campaigns = []fakeCampaign{{ID: 1, Name: "launch", Status: "running", Subject: "Original"}}
|
||||
@@ -492,14 +492,44 @@ func TestSyncDir_RejectsNonDraftCampaign(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("SyncDir: %v", err)
|
||||
}
|
||||
if len(result.Rejected) != 1 || !strings.Contains(result.Rejected[0], "running") {
|
||||
t.Fatalf("expected a rejection mentioning 'running', got %v", result.Rejected)
|
||||
// A sent/running campaign is a historical record, not an error -- it
|
||||
// must never land in Rejected (which fails the whole CI job) just
|
||||
// because its campaign.md is still around.
|
||||
if len(result.Rejected) != 0 {
|
||||
t.Fatalf("expected no rejection for an already-sent campaign, got %v", result.Rejected)
|
||||
}
|
||||
if len(result.Skipped) != 1 || !strings.Contains(result.Skipped[0], "running") {
|
||||
t.Fatalf("expected launch in Skipped mentioning 'running', got %v", result.Skipped)
|
||||
}
|
||||
if f.campaigns[0].Subject != "Original" {
|
||||
t.Errorf("expected the live campaign's content to be untouched, got %+v", f.campaigns[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncDir_NonDraftCampaignSkipsEvenWithAStaleListReference(t *testing.T) {
|
||||
// A finished campaign's campaign.md might reference a list that's since
|
||||
// been renamed or removed -- that must never break the build, since an
|
||||
// archived campaign has nothing left to resolve.
|
||||
f := newFakeListmonk(t)
|
||||
f.campaigns = []fakeCampaign{{ID: 1, Name: "launch", Status: "finished", Subject: "Original"}}
|
||||
lm := f.client()
|
||||
|
||||
root := t.TempDir()
|
||||
fm := "subject: \"Big Announcement\"\nlists: [\"No Longer Exists\"]\nfrom_email: hello@example.com\n"
|
||||
writeCampaignDir(t, root, "launch", fm, "Hello world.")
|
||||
|
||||
result, err := SyncDir(lm, root, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("SyncDir: %v", err)
|
||||
}
|
||||
if len(result.Rejected) != 0 {
|
||||
t.Fatalf("expected no rejection despite the stale list reference, got %v", result.Rejected)
|
||||
}
|
||||
if len(result.Skipped) != 1 {
|
||||
t.Fatalf("expected launch in Skipped, got %v", result.Skipped)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncDir_AmbiguousListNameRejects(t *testing.T) {
|
||||
f := newFakeListmonk(t)
|
||||
f.lists = []fakeList{{ID: 3, Name: "Newsletter"}, {ID: 4, Name: "Newsletter"}}
|
||||
|
||||
Reference in New Issue
Block a user