diff --git a/README.md b/README.md index 81f34cd..ee2e48e 100644 --- a/README.md +++ b/README.md @@ -101,11 +101,14 @@ Each file in `attachments:` is uploaded to listmonk's media library under a synt Every `sync` that actually creates or changes a campaign auto-sends a preview via listmonk's test-send endpoint, to `preview_emails` if set, otherwise `CAMPAIGNS_PREVIEW_EMAIL`. `campaigns test` is for an on-demand re-preview without touching content. +## Sent campaigns stay in the repo + +A campaign's `campaign.md` is meant to stick around after it's sent — a record of what went out and when, not something to delete once it's live. Once a campaign is anything other than `draft` in listmonk (`scheduled`/`running`/`paused`/`cancelled`/`finished`), sync leaves it alone entirely — no write, no re-preview, and (as of v0.1.6) no failure either. It's reported as **skipped**, a distinct, non-error bucket from rejected. + ## Sync guardrails A rejected campaign is usually one of these, all deliberate: -- **Anything other than `draft` in listmonk** (`scheduled`/`running`/`paused`/`cancelled`/`finished`) — sync refuses to touch it once it's live or sent. - **A `lists:` name matches zero or more than one listmonk list** — sync never guesses. - **An invalid `segment_query`** — listmonk's own error is surfaced verbatim. - **Missing `subject`/`from_email`/a target audience, or `type: optin`.** diff --git a/internal/campaign/sync.go b/internal/campaign/sync.go index fb93dac..08d377c 100644 --- a/internal/campaign/sync.go +++ b/internal/campaign/sync.go @@ -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 diff --git a/internal/campaign/sync_test.go b/internal/campaign/sync_test.go index b3c9095..2f22a9f 100644 --- a/internal/campaign/sync_test.go +++ b/internal/campaign/sync_test.go @@ -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"}} diff --git a/main.go b/main.go index 68162a4..d458ee1 100644 --- a/main.go +++ b/main.go @@ -81,6 +81,9 @@ func runSync(lm *listmonk.Client, path string) { } log.Printf("synced: %v", result.Synced) log.Printf("unchanged: %v", result.Unchanged) + if len(result.Skipped) > 0 { + log.Printf("skipped (already sent): %v", result.Skipped) + } if len(result.Rejected) > 0 { log.Printf("rejected: %v", result.Rejected) }