Pārlūkot izejas kodu

Fixes campaign test messages not including unsub headers.

Campaign messages are handled by `manager` whereas test messages
were being pushed directly into a messenger skipping some campaign
related routines such as the addition of list unsub headers.

This commit exposes a new function `manager.PushCampaignMessage()`
that accepts arbitrary campaign messages that then pass through
the standard campaign message workers, thus getting the missing unsub
headers. This closes #360.

In addition, this removes the superfluous `CampaignMessage.Render()`
function which had to be mandatorily called always and makes it
implicit in `manager.NewCampaignMessage()`.
Kailash Nadh 4 gadi atpakaļ
vecāks
revīzija
931e467b25
7 mainītis faili ar 45 papildinājumiem un 44 dzēšanām
  1. 7 17
      cmd/campaigns.go
  2. 3 3
      cmd/public.go
  3. 3 3
      cmd/templates.go
  4. 1 1
      frontend/cypress.json
  5. 1 4
      go.mod
  6. 0 8
      go.sum
  7. 30 8
      internal/manager/manager.go

+ 7 - 17
cmd/campaigns.go

@@ -14,7 +14,6 @@ import (
 	"time"
 
 	"github.com/gofrs/uuid"
-	"github.com/knadh/listmonk/internal/messenger"
 	"github.com/knadh/listmonk/internal/subimporter"
 	"github.com/knadh/listmonk/models"
 	"github.com/labstack/echo"
@@ -199,14 +198,14 @@ func handlePreviewCampaign(c echo.Context) error {
 	}
 
 	// Render the message body.
-	m := app.manager.NewCampaignMessage(&camp, dummySubscriber)
-	if err := m.Render(); err != nil {
+	msg, err := app.manager.NewCampaignMessage(&camp, dummySubscriber)
+	if err != nil {
 		app.log.Printf("error rendering message: %v", err)
 		return echo.NewHTTPError(http.StatusBadRequest,
 			app.i18n.Ts("templates.errorRendering", "error", err.Error()))
 	}
 
-	return c.HTML(http.StatusOK, string(m.Body()))
+	return c.HTML(http.StatusOK, string(msg.Body()))
 }
 
 // handleCampaignContent handles campaign content (body) format conversions.
@@ -613,24 +612,15 @@ func sendTestMessage(sub models.Subscriber, camp *models.Campaign, app *App) err
 			app.i18n.Ts("templates.errorCompiling", "error", err.Error()))
 	}
 
-	// Render the message body.
-	m := app.manager.NewCampaignMessage(camp, sub)
-	if err := m.Render(); err != nil {
+	// Create a sample campaign message.
+	msg, err := app.manager.NewCampaignMessage(camp, sub)
+	if err != nil {
 		app.log.Printf("error rendering message: %v", err)
 		return echo.NewHTTPError(http.StatusNotFound,
 			app.i18n.Ts("templates.errorRendering", "error", err.Error()))
 	}
 
-	return app.messengers[camp.Messenger].Push(messenger.Message{
-		From:        camp.FromEmail,
-		To:          []string{sub.Email},
-		Subject:     m.Subject(),
-		ContentType: camp.ContentType,
-		Body:        m.Body(),
-		AltBody:     m.AltBody(),
-		Subscriber:  sub,
-		Campaign:    camp,
-	})
+	return app.manager.PushCampaignMessage(msg)
 }
 
 // validateCampaignFields validates incoming campaign field values.

+ 3 - 3
cmd/public.go

@@ -140,15 +140,15 @@ func handleViewCampaignMessage(c echo.Context) error {
 	}
 
 	// Render the message body.
-	m := app.manager.NewCampaignMessage(&camp, sub)
-	if err := m.Render(); err != nil {
+	msg, err := app.manager.NewCampaignMessage(&camp, sub)
+	if err != nil {
 		app.log.Printf("error rendering message: %v", err)
 		return c.Render(http.StatusInternalServerError, tplMessage,
 			makeMsgTpl(app.i18n.T("public.errorTitle"), "",
 				app.i18n.Ts("public.errorFetchingCampaign")))
 	}
 
-	return c.HTML(http.StatusOK, string(m.Body()))
+	return c.HTML(http.StatusOK, string(msg.Body()))
 }
 
 // handleSubscriptionPage renders the subscription management page and

+ 3 - 3
cmd/templates.go

@@ -116,13 +116,13 @@ func handlePreviewTemplate(c echo.Context) error {
 	}
 
 	// Render the message body.
-	m := app.manager.NewCampaignMessage(&camp, dummySubscriber)
-	if err := m.Render(); err != nil {
+	msg, err := app.manager.NewCampaignMessage(&camp, dummySubscriber)
+	if err != nil {
 		return echo.NewHTTPError(http.StatusBadRequest,
 			app.i18n.Ts("templates.errorRendering", "error", err.Error()))
 	}
 
-	return c.HTML(http.StatusOK, string(m.Body()))
+	return c.HTML(http.StatusOK, string(msg.Body()))
 }
 
 // handleCreateTemplate handles template creation.

+ 1 - 1
frontend/cypress.json

@@ -1,5 +1,5 @@
 {
-	"baseUrl": "http://localhost:8080",
+	"baseUrl": "http://localhost:9000",
 	"env": {
 		"server_init_command": "pkill -9 listmonk | cd ../ && ./listmonk --install --yes && ./listmonk > /dev/null 2>/dev/null &",
 		"username": "listmonk",

+ 1 - 4
go.mod

@@ -7,7 +7,6 @@ require (
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect
 	github.com/disintegration/imaging v1.6.2
 	github.com/gofrs/uuid v3.2.0+incompatible
-	github.com/jaytaylor/html2text v0.0.0-20200220170450-61d9dc4d7195
 	github.com/jmoiron/sqlx v1.2.0
 	github.com/knadh/goyesql/v2 v2.1.1
 	github.com/knadh/koanf v0.12.0
@@ -19,11 +18,9 @@ require (
 	github.com/mailru/easyjson v0.7.6
 	github.com/mitchellh/copystructure v1.1.2 // indirect
 	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
-	github.com/olekukonko/tablewriter v0.0.4 // indirect
 	github.com/rhnvrm/simples3 v0.5.0
 	github.com/spf13/pflag v1.0.5
-	github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect
-	github.com/yuin/goldmark v1.3.4 // indirect
+	github.com/yuin/goldmark v1.3.4
 	golang.org/x/mod v0.3.0
 	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
 	gopkg.in/volatiletech/null.v6 v6.0.0-20170828023728-0bef4e07ae1b

+ 0 - 8
go.sum

@@ -29,8 +29,6 @@ github.com/huandu/xstrings v1.3.1 h1:4jgBlKK6tLKFvO8u5pmYjG91cqytmDCDvGh7ECVFfFs
 github.com/huandu/xstrings v1.3.1/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
 github.com/imdario/mergo v0.3.8 h1:CGgOkSJeqMRmt0D9XLWExdT4m4F1vd3FV3VPt+0VxkQ=
 github.com/imdario/mergo v0.3.8/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
-github.com/jaytaylor/html2text v0.0.0-20200220170450-61d9dc4d7195 h1:j0UEFmS7wSjAwKEIkgKBn8PRDfjcuggzr93R9wk53nQ=
-github.com/jaytaylor/html2text v0.0.0-20200220170450-61d9dc4d7195/go.mod h1:CVKlgaMiht+LXvHG173ujK6JUhZXKb2u/BQtjPDIvyk=
 github.com/jmoiron/sqlx v1.2.0 h1:41Ip0zITnmWNR/vHV+S4m+VoUivnWY5E4OJfLZjCJMA=
 github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
@@ -62,8 +60,6 @@ github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVc
 github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
 github.com/mattn/go-isatty v0.0.9 h1:d5US/mDsogSGW37IV293h//ZFaeajb69h+EHFsv2xGg=
 github.com/mattn/go-isatty v0.0.9/go.mod h1:YNRxwqDuOph6SZLI9vUUz6OYw3QyUt7WiY2yME+cCiQ=
-github.com/mattn/go-runewidth v0.0.7 h1:Ei8KR0497xHyKJPAv59M1dkC+rOZCMBJ+t3fZ+twI54=
-github.com/mattn/go-runewidth v0.0.7/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
 github.com/mattn/go-sqlite3 v1.9.0 h1:pDRiWfl+++eC2FEFRy6jXmQlvp4Yh3z1MJKg4UeYM/4=
 github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
 github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
@@ -76,8 +72,6 @@ github.com/mitchellh/reflectwalk v1.0.1 h1:FVzMWA5RllMAKIdUSC8mdWo3XtwoecrH79BY7
 github.com/mitchellh/reflectwalk v1.0.1/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
-github.com/olekukonko/tablewriter v0.0.4 h1:vHD/YYe1Wolo78koG299f7V/VAS08c6IpCLn+Ejf/w8=
-github.com/olekukonko/tablewriter v0.0.4/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA=
 github.com/pelletier/go-toml v1.7.0 h1:7utD74fnzVc/cpcyy8sjrlFr5vYpypUixARcHIMIGuI=
 github.com/pelletier/go-toml v1.7.0/go.mod h1:vwGMzjaWMwyfHwgIBhI2YUM4fB6nL6lVAvS1LBMMhTE=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -90,8 +84,6 @@ github.com/spf13/cast v1.3.1 h1:nFm6S0SMdyzrzcmThSipiEubIDy8WEXKNZ0UOgiRpng=
 github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
-github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf h1:pvbZ0lM0XWPBqUKqFU8cmavspvIl9nulOYwdy6IFRRo=
-github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf/go.mod h1:RJID2RhlZKId02nZ62WenDCkgHFerpIOmW0iT7GKmXM=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=

+ 30 - 8
internal/manager/manager.go

@@ -153,8 +153,8 @@ func New(cfg Config, src DataSource, notifCB models.AdminNotifCallback, i *i18n.
 // NewCampaignMessage creates and returns a CampaignMessage that is made available
 // to message templates while they're compiled. It represents a message from
 // a campaign that's bound to a single Subscriber.
-func (m *Manager) NewCampaignMessage(c *models.Campaign, s models.Subscriber) CampaignMessage {
-	return CampaignMessage{
+func (m *Manager) NewCampaignMessage(c *models.Campaign, s models.Subscriber) (CampaignMessage, error) {
+	msg := CampaignMessage{
 		Campaign:   c,
 		Subscriber: s,
 
@@ -163,6 +163,12 @@ func (m *Manager) NewCampaignMessage(c *models.Campaign, s models.Subscriber) Ca
 		to:       s.Email,
 		unsubURL: fmt.Sprintf(m.cfg.UnsubURL, c.UUID, s.UUID),
 	}
+
+	if err := msg.render(); err != nil {
+		return msg, err
+	}
+
+	return msg, nil
 }
 
 // AddMessenger adds a Messenger messaging backend to the manager.
@@ -175,7 +181,8 @@ func (m *Manager) AddMessenger(msg messenger.Messenger) error {
 	return nil
 }
 
-// PushMessage pushes a Message to be sent out by the workers.
+// PushMessage pushes an arbitrary non-campaign Message to be sent out by the workers.
+// It times out if the queue is busy.
 func (m *Manager) PushMessage(msg Message) error {
 	t := time.NewTicker(time.Second * 3)
 	defer t.Stop()
@@ -183,7 +190,22 @@ func (m *Manager) PushMessage(msg Message) error {
 	select {
 	case m.msgQueue <- msg:
 	case <-t.C:
-		m.logger.Println("message push timed out: %'s'", msg.Subject)
+		m.logger.Printf("message push timed out: '%s'", msg.Subject)
+		return errors.New("message push timed out")
+	}
+	return nil
+}
+
+// PushCampaignMessage pushes a campaign messages to be sent out by the workers.
+// It times out if the queue is busy.
+func (m *Manager) PushCampaignMessage(msg CampaignMessage) error {
+	t := time.NewTicker(time.Second * 3)
+	defer t.Stop()
+
+	select {
+	case m.campMsgQueue <- msg:
+	case <-t.C:
+		m.logger.Printf("message push timed out: '%s'", msg.Subject())
 		return errors.New("message push timed out")
 	}
 	return nil
@@ -487,8 +509,8 @@ func (m *Manager) nextSubscribers(c *models.Campaign, batchSize int) (bool, erro
 	// Push messages.
 	for _, s := range subs {
 		// Send the message.
-		msg := m.NewCampaignMessage(c, s)
-		if err := msg.Render(); err != nil {
+		msg, err := m.NewCampaignMessage(c, s)
+		if err != nil {
 			m.logger.Printf("error rendering message (%s) (%s): %v", c.Name, s.Email, err)
 			continue
 		}
@@ -615,9 +637,9 @@ func (m *Manager) sendNotif(c *models.Campaign, status, reason string) error {
 	return m.notifCB(subject, data)
 }
 
-// Render takes a Message, executes its pre-compiled Campaign.Tpl
+// render takes a Message, executes its pre-compiled Campaign.Tpl
 // and applies the resultant bytes to Message.body to be used in messages.
-func (m *CampaignMessage) Render() error {
+func (m *CampaignMessage) render() error {
 	out := bytes.Buffer{}
 
 	// Render the subject if it's a template.