Skip to content

Commit

Permalink
Fixes campaign test messages not including unsub headers.
Browse files Browse the repository at this point in the history
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()`.
  • Loading branch information
knadh committed May 21, 2021
1 parent ea92e8b commit 931e467
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 44 deletions.
24 changes: 7 additions & 17 deletions cmd/campaigns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions cmd/public.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions cmd/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion frontend/cypress.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
5 changes: 1 addition & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 0 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand All @@ -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=
Expand All @@ -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=
Expand Down
38 changes: 30 additions & 8 deletions internal/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand All @@ -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.
Expand All @@ -175,15 +181,31 @@ 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()

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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 931e467

Please sign in to comment.