Fix campaign/listmonk API shapes against confirmed source, not guesses

Read knadh/listmonk's actual Go source (cmd/campaigns.go, cmd/subscribers.go) instead of relying on incomplete public docs. Two corrections: the media attach field is "media" (plain IDs), not "media_ids" as originally guessed; and there's a single query-based bulk list action (PUT /api/subscribers/query/lists) that applies a segment_query server-side, so resolveSegment no longer fetches subscriber IDs client-side before bulk-adding them. Create-status-defaults-to-draft and the test-send "subscribers" field are now confirmed rather than flagged TODO(verify).
2026-07-10 08:37:07 -04:00
parent b16d2b6c5f
commit bcf8df5579
5 changed files with 109 additions and 166 deletions
@@ -16,7 +16,6 @@ import (
 	"io"
 	"mime/multipart"
 	"net/http"
-	"net/url"
 )

 type Client struct {
@@ -160,60 +159,27 @@ func (c *Client) FindOrCreateListByName(name string) (int, error) {

 // ---- Subscribers / segmentation ----

-// QuerySubscriberIDs runs a raw SQL boolean expression against the
-// subscribers table (the same segmentation mechanism listmonk's own admin
-// UI search box uses) and returns the IDs of every match. per_page=all
-// deliberately skips pagination, matching drip's existing Query method.
-func (c *Client) QuerySubscriberIDs(query string) ([]int, error) {
-	path := "/api/subscribers?per_page=all&query=" + url.QueryEscape(query)
-	respBody, status, err := c.do(http.MethodGet, path, nil)
-	if err != nil {
-		return nil, fmt.Errorf("querying subscribers: %w", err)
-	}
-	if status != http.StatusOK {
-		return nil, fmt.Errorf("segment_query failed (%d): %s", status, string(respBody))
-	}
-	var parsed struct {
-		Data struct {
-			Results []struct {
-				ID int `json:"id"`
-			} `json:"results"`
-		} `json:"data"`
-	}
-	if err := json.Unmarshal(respBody, &parsed); err != nil {
-		return nil, fmt.Errorf("parsing subscriber query response: %w", err)
-	}
-	ids := make([]int, 0, len(parsed.Data.Results))
-	for _, r := range parsed.Data.Results {
-		ids = append(ids, r.ID)
-	}
-	return ids, nil
-}
-
-// BulkAddToList adds the given subscriber IDs to listID.
-//
-// TODO(verify): the bulk list-membership action's exact endpoint/body shape
-// isn't confirmed against a live listmonk instance — this targets the
-// documented bulk subscriber-action endpoint (PUT /api/subscribers/lists
-// with explicit ids+action+target_list_ids), which is the same mechanism
-// listmonk's admin UI uses for "add selected subscribers to list". Confirm
-// against the real instance/version before relying on this in production.
-func (c *Client) BulkAddToList(ids []int, listID int) error {
-	if len(ids) == 0 {
-		return nil
-	}
+// QueryAddToList bulk-adds every subscriber matching the given raw SQL
+// boolean expression to listID, in one call — listmonk's query-based bulk
+// subscriber-list action, confirmed against the actual handler source
+// (cmd/subscribers.go's ManageSubscriberListsByQuery, routed at
+// PUT /api/subscribers/query/lists, request fields query/target_list_ids/
+// action/status). This is the same segmentation mechanism listmonk's own
+// admin UI search box uses, applied server-side without ever fetching
+// individual subscriber IDs client-side.
+func (c *Client) QueryAddToList(query string, listID int) error {
 	payload := map[string]any{
-		"ids":             ids,
-		"action":          "add",
+		"query":           query,
 		"target_list_ids": []int{listID},
+		"action":          "add",
 		"status":          "unconfirmed",
 	}
-	respBody, status, err := c.do(http.MethodPut, "/api/subscribers/lists", payload)
+	respBody, status, err := c.do(http.MethodPut, "/api/subscribers/query/lists", payload)
 	if err != nil {
-		return fmt.Errorf("bulk-adding %d subscriber(s) to list %d: %w", len(ids), listID, err)
+		return fmt.Errorf("segment_query bulk-add to list %d: %w", listID, err)
 	}
 	if status != http.StatusOK {
-		return fmt.Errorf("bulk-adding subscribers to list %d failed (%d): %s", listID, status, string(respBody))
+		return fmt.Errorf("segment_query bulk-add to list %d failed (%d): %s", listID, status, string(respBody))
 	}
 	return nil
 }
@@ -364,12 +330,13 @@ func (in CampaignInput) payload() map[string]any {
 		p["tags"] = in.Tags
 	}
 	if len(in.MediaIDs) > 0 {
-		// TODO(verify): field name for attaching media library items to a
-		// campaign isn't documented publicly — confirm against a live
-		// listmonk admin UI network trace (attach a file to any test
-		// campaign and inspect the PUT /api/campaigns/{id} request it
-		// sends) before relying on this in production.
-		p["media_ids"] = in.MediaIDs
+		// Field name confirmed against knadh/listmonk's actual request-binding
+		// struct (cmd/campaigns.go's campReq: `MediaIDs []int json:"media"`) —
+		// requests take plain media IDs under "media"; listmonk's own
+		// responses echo full media objects back under the same key (see
+		// parseCampaign below), a request/response asymmetry on listmonk's
+		// side, not a mistake here.
+		p["media"] = in.MediaIDs
 	}
 	return p
 }
@@ -388,7 +355,9 @@ func parseCampaign(data []byte) (*Campaign, error) {
 		Lists      []struct {
 			ID int `json:"id"`
 		} `json:"lists"`
-		MediaIDs []int `json:"media_ids"`
+		Media []struct {
+			ID int `json:"id"`
+		} `json:"media"`
 	}
 	if err := json.Unmarshal(data, &parsed); err != nil {
 		return nil, err
@@ -397,6 +366,10 @@ func parseCampaign(data []byte) (*Campaign, error) {
 	for _, l := range parsed.Lists {
 		listIDs = append(listIDs, l.ID)
 	}
+	mediaIDs := make([]int, 0, len(parsed.Media))
+	for _, m := range parsed.Media {
+		mediaIDs = append(mediaIDs, m.ID)
+	}
 	return &Campaign{
 		ID:         parsed.ID,
 		Name:       parsed.Name,
@@ -408,7 +381,7 @@ func parseCampaign(data []byte) (*Campaign, error) {
 		Type:       parsed.Type,
 		Tags:       parsed.Tags,
 		ListIDs:    listIDs,
-		MediaIDs:   parsed.MediaIDs,
+		MediaIDs:   mediaIDs,
 	}, nil
 }

@@ -452,13 +425,10 @@ func (c *Client) FindCampaignByName(name string) (*Campaign, error) {
 	}
 }

-// CreateCampaign creates a new campaign. listmonk's create-time default
-// status is draft, which is this tool's core safety invariant — see the
-// TODO below.
-//
-// TODO(verify): confirm that POST /api/campaigns' create-time default
-// status really is "draft" against a live instance (and whether an explicit
-// status field is even accepted on create) before depending on it.
+// CreateCampaign creates a new campaign. Confirmed against knadh/listmonk's
+// actual handler source: the create handler ignores any caller-supplied
+// status and always creates as draft — this tool's core safety invariant —
+// so in.payload() doesn't bother sending one.
 func (c *Client) CreateCampaign(in CampaignInput) (*Campaign, error) {
 	respBody, status, err := c.do(http.MethodPost, "/api/campaigns", in.payload())
 	if err != nil {
@@ -512,11 +482,9 @@ func (c *Client) SetCampaignStatus(id int, status string) error {
 }

 // TestCampaign sends a preview of the campaign's current content to the
-// given addresses without touching its status.
-//
-// TODO(verify): the request body key for the recipient address list isn't
-// confirmed against a live instance — using "subscribers" per listmonk's
-// documented shape for this endpoint; confirm before relying on it.
+// given addresses without touching its status. Request field confirmed
+// against knadh/listmonk's actual handler source (cmd/campaigns.go's
+// campReq.SubscriberEmails, json tag "subscribers").
 func (c *Client) TestCampaign(id int, emails []string) error {
 	if len(emails) == 0 {
 		return nil
@@ -98,28 +98,7 @@ func TestFindOrCreateListByName_CreatesWhenMissing(t *testing.T) {
 	}
 }

-func TestQuerySubscriberIDs_UsesPerPageAllAndParsesIDs(t *testing.T) {
-	var gotPath string
-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		gotPath = r.URL.RequestURI()
-		w.Write([]byte(`{"data":{"results":[{"id":1},{"id":2}]}}`))
-	}))
-	defer srv.Close()
-
-	c := New(srv.URL, "u", "t")
-	ids, err := c.QuerySubscriberIDs("subscribers.attribs->>'source' = 'workshop'")
-	if err != nil {
-		t.Fatalf("QuerySubscriberIDs: %v", err)
-	}
-	if !strings.Contains(gotPath, "per_page=all") {
-		t.Errorf("expected per_page=all, got %q", gotPath)
-	}
-	if len(ids) != 2 || ids[0] != 1 || ids[1] != 2 {
-		t.Errorf("expected [1 2], got %v", ids)
-	}
-}
-
-func TestBulkAddToList_SendsExpectedPayload(t *testing.T) {
+func TestQueryAddToList_SendsExpectedPayload(t *testing.T) {
 	var gotMethod, gotPath string
 	var gotBody map[string]any
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -132,34 +111,35 @@ func TestBulkAddToList_SendsExpectedPayload(t *testing.T) {
 	defer srv.Close()

 	c := New(srv.URL, "u", "t")
-	if err := c.BulkAddToList([]int{1, 2, 3}, 9); err != nil {
-		t.Fatalf("BulkAddToList: %v", err)
+	query := "subscribers.attribs->>'source' = 'workshop'"
+	if err := c.QueryAddToList(query, 9); err != nil {
+		t.Fatalf("QueryAddToList: %v", err)
 	}
 	if gotMethod != http.MethodPut {
 		t.Errorf("expected PUT, got %s", gotMethod)
 	}
-	if gotPath != "/api/subscribers/lists" {
-		t.Errorf("expected /api/subscribers/lists, got %q", gotPath)
+	if gotPath != "/api/subscribers/query/lists" {
+		t.Errorf("expected /api/subscribers/query/lists, got %q", gotPath)
+	}
+	if gotBody["query"] != query {
+		t.Errorf("expected query sent verbatim, got %+v", gotBody)
 	}
 	if gotBody["action"] != "add" {
 		t.Errorf("expected action=add, got %+v", gotBody)
 	}
 }

-func TestBulkAddToList_NoopOnEmptyIDs(t *testing.T) {
-	var calls int
+func TestQueryAddToList_SurfacesListmonkErrors(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		calls++
-		w.Write([]byte(`{}`))
+		w.WriteHeader(http.StatusBadRequest)
+		w.Write([]byte(`{"message":"invalid SQL expression"}`))
 	}))
 	defer srv.Close()

 	c := New(srv.URL, "u", "t")
-	if err := c.BulkAddToList(nil, 9); err != nil {
-		t.Fatalf("BulkAddToList: %v", err)
-	}
-	if calls != 0 {
-		t.Errorf("expected no request for empty id list, got %d", calls)
+	err := c.QueryAddToList("not valid sql", 9)
+	if err == nil || !strings.Contains(err.Error(), "invalid SQL expression") {
+		t.Fatalf("expected listmonk's error message to surface, got %v", err)
 	}
 }