Fix spurious re-sync/preview for campaigns without an explicit template_id
CI / test (push) Successful in 21s
CI / test (push) Successful in 21s
campaignUnchanged compared input.TemplateID (0 when unset) literally against existing.TemplateID, but listmonk always assigns a real, nonzero template_id server-side even when create/update omits it. Every campaign without an explicit template_id was therefore flagged "changed" on every sync, forever, firing an unwanted extra preview email each time — caught via reground-campaigns' welcome/campaign.md re-previewing on an unrelated push. Treat TemplateID 0 as "don't compare" instead. Test fake now mirrors listmonk's real default-assignment behavior so this class of bug is caught.
This commit is contained in:
@@ -222,7 +222,13 @@ func campaignUnchanged(existing *listmonk.Campaign, input listmonk.CampaignInput
|
||||
// stored altbody is still null even though its body hasn't changed.
|
||||
existing.AltBody == input.Body &&
|
||||
existing.FromEmail == input.FromEmail &&
|
||||
existing.TemplateID == input.TemplateID &&
|
||||
// input.TemplateID == 0 means "unspecified, use whatever's assigned"
|
||||
// (see CampaignInput.TemplateID) -- listmonk always assigns some real,
|
||||
// nonzero template_id server-side even when it's omitted from the
|
||||
// create/update payload, so comparing 0 against that would flag every
|
||||
// campaign.md without an explicit template_id as changed on every
|
||||
// sync, forever.
|
||||
(input.TemplateID == 0 || existing.TemplateID == input.TemplateID) &&
|
||||
existing.Type == input.Type &&
|
||||
equalStringSets(existing.Tags, input.Tags) &&
|
||||
equalIntSets(existing.ListIDs, input.ListIDs) &&
|
||||
|
||||
@@ -152,12 +152,25 @@ func (f *fakeListmonk) writeCampaigns(w http.ResponseWriter) {
|
||||
json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"results": results}})
|
||||
}
|
||||
|
||||
// fakeDefaultTemplateID is the nonzero template_id real listmonk assigns
|
||||
// server-side to a campaign whose create/update request omitted template_id
|
||||
// entirely -- never 0, which is what makes the zero value ambiguous between
|
||||
// "unspecified" and "really is 0" in campaignUnchanged.
|
||||
const fakeDefaultTemplateID = 1
|
||||
|
||||
func (f *fakeListmonk) resolveTemplateID(body map[string]any) int {
|
||||
if v, ok := body["template_id"]; ok {
|
||||
return toInt(v)
|
||||
}
|
||||
return fakeDefaultTemplateID
|
||||
}
|
||||
|
||||
func (f *fakeListmonk) createCampaign(w http.ResponseWriter, r *http.Request) {
|
||||
body := decodeBody(r)
|
||||
c := fakeCampaign{
|
||||
ID: f.id(), Name: str(body["name"]), Status: "draft",
|
||||
Subject: str(body["subject"]), Body: str(body["body"]), AltBody: str(body["altbody"]),
|
||||
FromEmail: str(body["from_email"]), TemplateID: toInt(body["template_id"]),
|
||||
FromEmail: str(body["from_email"]), TemplateID: f.resolveTemplateID(body),
|
||||
Type: str(body["type"]), Tags: toStrings(body["tags"]),
|
||||
ListIDs: toInts(body["lists"]), MediaIDs: toInts(body["media"]),
|
||||
}
|
||||
@@ -174,7 +187,7 @@ func (f *fakeListmonk) updateCampaign(w http.ResponseWriter, r *http.Request) {
|
||||
f.campaigns[i].Body = str(body["body"])
|
||||
f.campaigns[i].AltBody = str(body["altbody"])
|
||||
f.campaigns[i].FromEmail = str(body["from_email"])
|
||||
f.campaigns[i].TemplateID = toInt(body["template_id"])
|
||||
f.campaigns[i].TemplateID = f.resolveTemplateID(body)
|
||||
f.campaigns[i].Type = str(body["type"])
|
||||
f.campaigns[i].Tags = toStrings(body["tags"])
|
||||
f.campaigns[i].ListIDs = toInts(body["lists"])
|
||||
@@ -409,6 +422,39 @@ func TestSyncDir_SecondSyncWithUnchangedContentIsNoop(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncDir_UnspecifiedTemplateIDDoesNotTriggerSpuriousResync(t *testing.T) {
|
||||
f := newFakeListmonk(t)
|
||||
f.lists = []fakeList{{ID: 3, Name: "Newsletter"}}
|
||||
lm := f.client()
|
||||
|
||||
root := t.TempDir()
|
||||
// baseFrontmatter never sets template_id, matching real campaign.md files
|
||||
// like reground-campaigns' welcome/campaign.md.
|
||||
writeCampaignDir(t, root, "launch", baseFrontmatter, "Hello world.")
|
||||
|
||||
if _, err := SyncDir(lm, root, nil); err != nil {
|
||||
t.Fatalf("first SyncDir: %v", err)
|
||||
}
|
||||
if f.campaigns[0].TemplateID != fakeDefaultTemplateID {
|
||||
t.Fatalf("expected the fake to assign its default template_id like real listmonk does, got %d", f.campaigns[0].TemplateID)
|
||||
}
|
||||
|
||||
// Re-sync with the exact same, still-unspecified template_id: this must
|
||||
// not be flagged as changed just because listmonk's stored, server-
|
||||
// assigned template_id (nonzero) doesn't literally equal the frontmatter's
|
||||
// unset value (0).
|
||||
result, err := SyncDir(lm, root, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("second SyncDir: %v", err)
|
||||
}
|
||||
if len(result.Synced) != 0 {
|
||||
t.Errorf("expected no spurious re-sync from an unspecified template_id, got Synced=%v", result.Synced)
|
||||
}
|
||||
if len(result.Unchanged) != 1 || result.Unchanged[0] != "launch" {
|
||||
t.Errorf("expected launch in Unchanged, got %v", result.Unchanged)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyncDir_UpdatesWhenContentChanges(t *testing.T) {
|
||||
f := newFakeListmonk(t)
|
||||
f.lists = []fakeList{{ID: 3, Name: "Newsletter"}}
|
||||
|
||||
Reference in New Issue
Block a user