Add support for alternate plaintext body for e-mails.

This commit removes the Go html2text lib that would automatically
convert all HTML messages to plaintext and add them as the alt
text body to outgoing e-mails. This lib also had memory leak
issues with certain kinds of HTML templates.

A new UI field for optionally adding an alt plaintext body to
a campaign is added. On enabling, it converts the HTML message in
the campaign editor into plaintext (using the textversionjs lib).

This introduces breaking changes in the campaigns table schema,
model, and template compilation.
This commit is contained in:
Kailash Nadh 2021-01-30 14:59:21 +05:30
parent 535b505404
commit 68afd61024
16 changed files with 180 additions and 75 deletions

View file

@ -14,6 +14,7 @@ import (
"time"
"github.com/gofrs/uuid"
"github.com/jaytaylor/html2text"
"github.com/knadh/listmonk/internal/messenger"
"github.com/knadh/listmonk/internal/subimporter"
"github.com/knadh/listmonk/models"
@ -149,7 +150,7 @@ func handleGetCampaigns(c echo.Context) error {
return c.JSON(http.StatusOK, okResp{out})
}
// handlePreviewTemplate renders the HTML preview of a campaign body.
// handlePreviewCampaign renders the HTML preview of a campaign body.
func handlePreviewCampaign(c echo.Context) error {
var (
app = c.Get("app").(*App)
@ -212,6 +213,17 @@ func handlePreviewCampaign(c echo.Context) error {
return c.HTML(http.StatusOK, string(m.Body()))
}
// handleCampainBodyToText converts an HTML campaign body to plaintext.
func handleCampainBodyToText(c echo.Context) error {
out, err := html2text.FromString(c.FormValue("body"),
html2text.Options{PrettyTables: false})
if err != nil {
return err
}
return c.HTML(http.StatusOK, string(out))
}
// handleCreateCampaign handles campaign creation.
// Newly created campaigns are always drafts.
func handleCreateCampaign(c echo.Context) error {
@ -256,6 +268,7 @@ func handleCreateCampaign(c echo.Context) error {
o.Subject,
o.FromEmail,
o.Body,
o.AltBody,
o.ContentType,
o.SendAt,
pq.StringArray(normalizeTags(o.Tags)),
@ -309,8 +322,10 @@ func handleUpdateCampaign(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("campaigns.cantUpdate"))
}
// Incoming params.
var o campaignReq
// Read the incoming params into the existing campaign fields from the DB.
// This allows updating of values that have been sent where as fields
// that are not in the request retain the old values.
o := campaignReq{Campaign: cm}
if err := c.Bind(&o); err != nil {
return err
}
@ -326,6 +341,7 @@ func handleUpdateCampaign(c echo.Context) error {
o.Subject,
o.FromEmail,
o.Body,
o.AltBody,
o.ContentType,
o.SendAt,
o.SendLater,
@ -557,11 +573,12 @@ func handleTestCampaign(c echo.Context) error {
"name", "{globals.terms.campaign}", "error", pqErrMsg(err)))
}
// Override certain values in the DB with incoming values.
// Override certain values from the DB with incoming values.
camp.Name = req.Name
camp.Subject = req.Subject
camp.FromEmail = req.FromEmail
camp.Body = req.Body
camp.AltBody = req.AltBody
camp.Messenger = req.Messenger
camp.ContentType = req.ContentType
camp.TemplateID = req.TemplateID
@ -601,6 +618,7 @@ func sendTestMessage(sub models.Subscriber, camp *models.Campaign, app *App) err
Subject: m.Subject(),
ContentType: camp.ContentType,
Body: m.Body(),
AltBody: m.AltBody(),
Subscriber: sub,
Campaign: camp,
})

View file

@ -91,6 +91,7 @@ func registerHTTPHandlers(e *echo.Echo) {
g.GET("/api/campaigns/:id", handleGetCampaigns)
g.GET("/api/campaigns/:id/preview", handlePreviewCampaign)
g.POST("/api/campaigns/:id/preview", handlePreviewCampaign)
g.POST("/api/campaigns/:id/text", handlePreviewCampaign)
g.POST("/api/campaigns/:id/test", handleTestCampaign)
g.POST("/api/campaigns", handleCreateCampaign)
g.PUT("/api/campaigns/:id", handleUpdateCampaign)

View file

@ -120,6 +120,7 @@ func install(lastVer string, db *sqlx.DB, fs stuffbin.FileSystem, prompt bool) {
"No Reply <noreply@yoursite.com>",
`<h3>Hi {{ .Subscriber.FirstName }}!</h3>
This is a test e-mail campaign. Your second name is {{ .Subscriber.LastName }} and you are from {{ .Subscriber.Attribs.city }}.`,
nil,
"richtext",
nil,
pq.StringArray{"test-campaign"},

View file

@ -24,6 +24,7 @@
"quill": "^1.3.7",
"quill-delta": "^4.2.2",
"sass-loader": "^8.0.2",
"textversionjs": "^1.1.3",
"vue": "^2.6.11",
"vue-c3": "^1.2.11",
"vue-i18n": "^8.22.2",

View file

@ -224,6 +224,16 @@ section {
}
}
.editor {
margin-bottom: 30px;
}
.plain-editor textarea {
height: 65vh;
}
.alt-body textarea {
height: 30vh;
}
/* Table colors and padding */
.main table {
thead th {

View file

@ -138,7 +138,6 @@
</b-tab-item><!-- campaign -->
<b-tab-item :label="$t('campaigns.content')" icon="text" :disabled="isNew">
<section class="wrap">
<editor
v-model="form.content"
:id="data.id"
@ -147,7 +146,21 @@
:body="data.body"
:disabled="!canEdit"
/>
</section>
<div v-if="canEdit && form.content.contentType !== 'plain'" class="alt-body">
<p class="is-size-6 has-text-grey has-text-right">
<a v-if="form.altbody === null" href="#" @click.prevent="addAltBody">
<b-icon icon="text" size="is-small" /> {{ $t('campaigns.addAltText') }}
</a>
<a v-else href="#" @click.prevent="$utils.confirm(null, removeAltBody)">
<b-icon icon="trash-can-outline" size="is-small" />
{{ $t('campaigns.removeAltText') }}
</a>
</p>
<br />
<b-input v-if="form.altbody !== null" v-model="form.altbody"
type="textarea" :disabled="!canEdit" />
</div>
</b-tab-item><!-- content -->
</b-tabs>
</section>
@ -157,6 +170,8 @@
import Vue from 'vue';
import { mapState } from 'vuex';
import dayjs from 'dayjs';
import htmlToPlainText from 'textversionjs';
import ListSelector from '../components/ListSelector.vue';
import Editor from '../components/Editor.vue';
@ -187,6 +202,7 @@ export default Vue.extend({
tags: [],
sendAt: null,
content: { contentType: 'richtext', body: '' },
altbody: null,
// Parsed Date() version of send_at from the API.
sendAtDate: null,
@ -202,6 +218,22 @@ export default Vue.extend({
return dayjs(s).format('YYYY-MM-DD HH:mm');
},
addAltBody() {
this.form.altbody = htmlToPlainText(this.form.content.body);
},
removeAltBody() {
this.form.altbody = null;
},
onSubmit() {
if (this.isNew) {
this.createCampaign();
} else {
this.updateCampaign();
}
},
getCampaign(id) {
return this.$api.getCampaign(id).then((data) => {
this.data = data;
@ -233,6 +265,7 @@ export default Vue.extend({
template_id: this.form.templateId,
content_type: this.form.content.contentType,
body: this.form.content.body,
altbody: this.form.content.contentType !== 'plain' ? this.form.altbody : null,
subscribers: this.form.testEmails,
};
@ -242,14 +275,6 @@ export default Vue.extend({
return false;
},
onSubmit() {
if (this.isNew) {
this.createCampaign();
} else {
this.updateCampaign();
}
},
createCampaign() {
const data = {
name: this.form.name,
@ -284,6 +309,7 @@ export default Vue.extend({
template_id: this.form.templateId,
content_type: this.form.content.contentType,
body: this.form.content.body,
altbody: this.form.content.contentType !== 'plain' ? this.form.altbody : null,
};
let typMsg = 'globals.messages.updated';

View file

@ -354,6 +354,7 @@ export default Vue.extend({
tags: c.tags,
template_id: c.templateId,
body: c.body,
altbody: c.altbody,
};
this.$api.createCampaign(data).then((d) => {
this.$router.push({ name: 'campaign', params: { id: d.id } });

5
frontend/yarn.lock vendored
View file

@ -8531,6 +8531,11 @@ text-table@^0.2.0:
resolved "https://registry.yarnpkg.com/text-table/-/text-table-0.2.0.tgz#7f5ee823ae805207c00af2df4a84ec3fcfa570b4"
integrity sha1-f17oI66AUgfACvLfSoTsP8+lcLQ=
textversionjs@^1.1.3:
version "1.1.3"
resolved "https://registry.yarnpkg.com/textversionjs/-/textversionjs-1.1.3.tgz#1b700aef780467786882e28ab126f77ca326a1e8"
integrity sha1-G3AK73gEZ3hoguKKsSb3fKMmoeg=
thenify-all@^1.0.0:
version "1.6.0"
resolved "https://registry.yarnpkg.com/thenify-all/-/thenify-all-1.6.0.tgz#1a1918d402d8fc3f98fbf234db0bcc8cc10e9726"

View file

@ -37,6 +37,8 @@
"campaigns.onlyPausedDraft": "Only paused campaigns and drafts can be started.",
"campaigns.onlyScheduledAsDraft": "Only scheduled campaigns can be saved as drafts.",
"campaigns.pause": "Pause",
"campaigns.addAltText": "Add alternate plain text message",
"campaigns.removeAltText": "Remove alternate plain text message",
"campaigns.plainText": "Plain text",
"campaigns.preview": "Preview",
"campaigns.progress": "Progress",

View file

@ -79,6 +79,7 @@ type CampaignMessage struct {
to string
subject string
body []byte
altBody []byte
unsubURL string
}
@ -265,6 +266,7 @@ func (m *Manager) messageWorker() {
Subject: msg.subject,
ContentType: msg.Campaign.ContentType,
Body: msg.body,
AltBody: msg.altBody,
Subscriber: msg.Subscriber,
Campaign: msg.Campaign,
}
@ -299,6 +301,7 @@ func (m *Manager) messageWorker() {
Subject: msg.Subject,
ContentType: msg.ContentType,
Body: msg.Body,
AltBody: msg.AltBody,
Subscriber: msg.Subscriber,
Campaign: msg.Campaign,
})
@ -616,10 +619,25 @@ func (m *CampaignMessage) Render() error {
out.Reset()
}
// Compile the main template.
if err := m.Campaign.Tpl.ExecuteTemplate(&out, models.BaseTpl, m); err != nil {
return err
}
m.body = out.Bytes()
// Is there an alt body?
if m.Campaign.ContentType != models.CampaignContentTypePlain && m.Campaign.AltBody.Valid {
if m.Campaign.AltBodyTpl != nil {
b := bytes.Buffer{}
if err := m.Campaign.AltBodyTpl.ExecuteTemplate(&b, models.ContentTpl, m); err != nil {
return err
}
m.altBody = b.Bytes()
} else {
m.altBody = []byte(m.Campaign.AltBody.String)
}
}
return nil
}
@ -634,3 +652,10 @@ func (m *CampaignMessage) Body() []byte {
copy(out, m.body)
return out
}
// AltBody returns a copy of the message's alt body.
func (m *CampaignMessage) AltBody() []byte {
out := make([]byte, len(m.altBody))
copy(out, m.altBody)
return out
}

View file

@ -7,7 +7,6 @@ import (
"net/smtp"
"net/textproto"
"github.com/jaytaylor/html2text"
"github.com/knadh/listmonk/internal/messenger"
"github.com/knadh/smtppool"
)
@ -19,7 +18,6 @@ type Server struct {
Username string `json:"username"`
Password string `json:"password"`
AuthProtocol string `json:"auth_protocol"`
EmailFormat string `json:"email_format"`
TLSEnabled bool `json:"tls_enabled"`
TLSSkipVerify bool `json:"tls_skip_verify"`
EmailHeaders map[string]string `json:"email_headers"`
@ -114,12 +112,6 @@ func (e *Emailer) Push(m messenger.Message) error {
}
}
mtext, err := html2text.FromString(string(m.Body),
html2text.Options{PrettyTables: true})
if err != nil {
return err
}
em := smtppool.Email{
From: m.From,
To: m.To,
@ -140,14 +132,14 @@ func (e *Emailer) Push(m messenger.Message) error {
}
}
switch srv.EmailFormat {
case "html":
em.HTML = m.Body
switch m.ContentType {
case "plain":
em.Text = []byte(mtext)
em.Text = []byte(m.Body)
default:
em.HTML = m.Body
em.Text = []byte(mtext)
if len(m.AltBody) > 0 {
em.Text = m.AltBody
}
}
return srv.pool.Send(em)

View file

@ -22,6 +22,7 @@ type Message struct {
Subject string
ContentType string
Body []byte
AltBody []byte
Headers textproto.MIMEHeader
Attachments []Attachment

View file

@ -17,6 +17,9 @@ func V0_9_0(db *sqlx.DB, fs stuffbin.FileSystem, ko *koanf.Koanf) error {
('app.message_sliding_window_duration', '"1h"'),
('app.message_sliding_window_rate', '10000')
ON CONFLICT DO NOTHING;
-- Add alternate (plain text) body field on campaigns.
ALTER TABLE campaigns ADD COLUMN IF NOT EXISTS altbody TEXT NULL DEFAULT NULL;
`); err != nil {
return err
}

View file

@ -37,6 +37,9 @@ const (
CampaignStatusCancelled = "cancelled"
CampaignTypeRegular = "regular"
CampaignTypeOptin = "optin"
CampaignContentTypeRichtext = "richtext"
CampaignContentTypeHTML = "html"
CampaignContentTypePlain = "plain"
// List.
ListTypePrivate = "private"
@ -170,6 +173,7 @@ type Campaign struct {
Subject string `db:"subject" json:"subject"`
FromEmail string `db:"from_email" json:"from_email"`
Body string `db:"body" json:"body"`
AltBody null.String `db:"altbody" json:"altbody"`
SendAt null.Time `db:"send_at" json:"send_at"`
Status string `db:"status" json:"status"`
ContentType string `db:"content_type" json:"content_type"`
@ -181,6 +185,7 @@ type Campaign struct {
TemplateBody string `db:"template_body" json:"-"`
Tpl *template.Template `json:"-"`
SubjectTpl *template.Template `json:"-"`
AltBodyTpl *template.Template `json:"-"`
// Pseudofield for getting the total number of subscribers
// in searches and queries.
@ -321,6 +326,7 @@ func (c *Campaign) CompileTemplate(f template.FuncMap) error {
if err != nil {
return fmt.Errorf("error inserting child template: %v", err)
}
c.Tpl = out
// If the subject line has a template string, compile it.
if strings.Contains(c.Subject, "{{") {
@ -335,7 +341,18 @@ func (c *Campaign) CompileTemplate(f template.FuncMap) error {
c.SubjectTpl = subjTpl
}
c.Tpl = out
if strings.Contains(c.AltBody.String, "{{") {
b := c.AltBody.String
for _, r := range regTplFuncs {
b = r.regExp.ReplaceAllString(b, r.replace)
}
bTpl, err := template.New(ContentTpl).Funcs(f).Parse(b)
if err != nil {
return fmt.Errorf("error compiling alt plaintext message: %v", err)
}
c.AltBodyTpl = bTpl
}
return nil
}

View file

@ -354,16 +354,16 @@ WITH campLists AS (
-- Get the list_ids and their optin statuses for the campaigns found in the previous step.
SELECT id AS list_id, campaign_id, optin FROM lists
INNER JOIN campaign_lists ON (campaign_lists.list_id = lists.id)
WHERE id=ANY($12::INT[])
WHERE id=ANY($13::INT[])
),
tpl AS (
-- If there's no template_id given, use the defualt template.
SELECT (CASE WHEN $11 = 0 THEN id ELSE $11 END) AS id FROM templates WHERE is_default IS TRUE
SELECT (CASE WHEN $12 = 0 THEN id ELSE $12 END) AS id FROM templates WHERE is_default IS TRUE
),
counts AS (
SELECT COALESCE(COUNT(id), 0) as to_send, COALESCE(MAX(id), 0) as max_sub_id
FROM subscribers
LEFT JOIN campLists ON (campLists.campaign_id = ANY($12::INT[]))
LEFT JOIN campLists ON (campLists.campaign_id = ANY($13::INT[]))
LEFT JOIN subscriber_lists ON (
subscriber_lists.status != 'unsubscribed' AND
subscribers.id = subscriber_lists.subscriber_id AND
@ -373,16 +373,16 @@ counts AS (
-- any status except for 'unsubscribed' (already excluded above) works.
(CASE WHEN campLists.optin = 'double' THEN subscriber_lists.status = 'confirmed' ELSE true END)
)
WHERE subscriber_lists.list_id=ANY($12::INT[])
WHERE subscriber_lists.list_id=ANY($13::INT[])
AND subscribers.status='enabled'
),
camp AS (
INSERT INTO campaigns (uuid, type, name, subject, from_email, body, content_type, send_at, tags, messenger, template_id, to_send, max_subscriber_id)
SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, (SELECT id FROM tpl), (SELECT to_send FROM counts), (SELECT max_sub_id FROM counts)
INSERT INTO campaigns (uuid, type, name, subject, from_email, body, altbody, content_type, send_at, tags, messenger, template_id, to_send, max_subscriber_id)
SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, (SELECT id FROM tpl), (SELECT to_send FROM counts), (SELECT max_sub_id FROM counts)
RETURNING id
)
INSERT INTO campaign_lists (campaign_id, list_id, list_name)
(SELECT (SELECT id FROM camp), id, name FROM lists WHERE id=ANY($12::INT[]))
(SELECT (SELECT id FROM camp), id, name FROM lists WHERE id=ANY($13::INT[]))
RETURNING (SELECT id FROM camp);
-- name: query-campaigns
@ -392,19 +392,19 @@ INSERT INTO campaign_lists (campaign_id, list_id, list_name)
-- there's a COUNT() OVER() that still returns the total result count
-- for pagination in the frontend, albeit being a field that'll repeat
-- with every resultant row.
SELECT campaigns.id, campaigns.uuid, campaigns.name, campaigns.subject, campaigns.from_email,
campaigns.messenger, campaigns.started_at, campaigns.to_send, campaigns.sent, campaigns.type,
campaigns.body, campaigns.send_at, campaigns.status, campaigns.content_type, campaigns.tags,
campaigns.template_id, campaigns.created_at, campaigns.updated_at,
SELECT c.id, c.uuid, c.name, c.subject, c.from_email,
c.messenger, c.started_at, c.to_send, c.sent, c.type,
c.body, c.altbody, c.send_at, c.status, c.content_type, c.tags,
c.template_id, c.created_at, c.updated_at,
COUNT(*) OVER () AS total,
(
SELECT COALESCE(ARRAY_TO_JSON(ARRAY_AGG(l)), '[]') FROM (
SELECT COALESCE(campaign_lists.list_id, 0) AS id,
campaign_lists.list_name AS name
FROM campaign_lists WHERE campaign_lists.campaign_id = campaigns.id
FROM campaign_lists WHERE campaign_lists.campaign_id = c.id
) l
) AS lists
FROM campaigns
FROM campaigns c
WHERE ($1 = 0 OR id = $1)
AND status=ANY(CASE WHEN ARRAY_LENGTH($2::campaign_status[], 1) != 0 THEN $2::campaign_status[] ELSE ARRAY[status] END)
AND ($3 = '' OR CONCAT(name, subject) ILIKE $3)
@ -580,25 +580,26 @@ ORDER BY RANDOM() LIMIT 1;
-- name: update-campaign
WITH camp AS (
UPDATE campaigns SET
name=(CASE WHEN $2 != '' THEN $2 ELSE name END),
subject=(CASE WHEN $3 != '' THEN $3 ELSE subject END),
from_email=(CASE WHEN $4 != '' THEN $4 ELSE from_email END),
body=(CASE WHEN $5 != '' THEN $5 ELSE body END),
content_type=(CASE WHEN $6 != '' THEN $6::content_type ELSE content_type END),
send_at=(CASE WHEN $8 THEN $7::TIMESTAMP WITH TIME ZONE WHEN NOT $8 THEN NULL ELSE send_at END),
status=(CASE WHEN NOT $8 THEN 'draft' ELSE status END),
tags=$9::VARCHAR(100)[],
messenger=(CASE WHEN $10 != '' THEN $10 ELSE messenger END),
template_id=(CASE WHEN $11 != 0 THEN $11 ELSE template_id END),
name=$2,
subject=$3,
from_email=$4,
body=$5,
altbody=(CASE WHEN $6 = '' THEN NULL ELSE $6 END),
content_type=$7::content_type,
send_at=$8::TIMESTAMP WITH TIME ZONE,
status=(CASE WHEN NOT $9 THEN 'draft' ELSE status END),
tags=$10::VARCHAR(100)[],
messenger=$11,
template_id=$12,
updated_at=NOW()
WHERE id = $1 RETURNING id
),
d AS (
-- Reset list relationships
DELETE FROM campaign_lists WHERE campaign_id = $1 AND NOT(list_id = ANY($12))
DELETE FROM campaign_lists WHERE campaign_id = $1 AND NOT(list_id = ANY($13))
)
INSERT INTO campaign_lists (campaign_id, list_id, list_name)
(SELECT $1 as campaign_id, id, name FROM lists WHERE id=ANY($12::INT[]))
(SELECT $1 as campaign_id, id, name FROM lists WHERE id=ANY($13::INT[]))
ON CONFLICT (campaign_id, list_id) DO UPDATE SET list_name = EXCLUDED.list_name;
-- name: update-campaign-counts

View file

@ -75,6 +75,7 @@ CREATE TABLE campaigns (
subject TEXT NOT NULL,
from_email TEXT NOT NULL,
body TEXT NOT NULL,
altbody TEXT NULL,
content_type content_type NOT NULL DEFAULT 'richtext',
send_at TIMESTAMP WITH TIME ZONE,
status campaign_status NOT NULL DEFAULT 'draft',