hangar-tech
/
listmonk
3
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix the optin-in, form re-subscribe behaviour. 

If a user is already subscribed to an optin list but hasn't
confirmed, subscribing using the same e-mail id from the public
form now re-sends the optin e-mail while also showing an
appropriate message on the frontend rather than just saying
"subscribed successfully".

https://github.com/knadh/listmonk/issues/266
https://github.com/knadh/listmonk/issues/264
This commit is contained in:
Kailash Nadh 2021-02-13 17:55:10 +05:30
parent 2f05d4ef5b
commit 2f6bd05ca0
4 changed files with 60 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
cmd/public.go
View File

@ -324,14 +324,18 @@ func handleSubscriptionForm(c echo.Context) error {
// Insert the subscriber into the DB.
req.Status = models.SubscriberStatusEnabled
req.ListUUIDs = pq.StringArray(req.SubListUUIDs)
if _, err := insertSubscriber(req.SubReq, app); err != nil && err != errSubscriberExists {
_, _, hasOptin, err := insertSubscriber(req.SubReq, app)
if err != nil {
return c.Render(http.StatusInternalServerError, tplMessage,
makeMsgTpl(app.i18n.T("public.errorTitle"), "", fmt.Sprintf("%s", err.(*echo.HTTPError).Message)))
}
return c.Render(http.StatusOK, tplMessage,
makeMsgTpl(app.i18n.T("public.subTitle"), "",
app.i18n.Ts("public.subConfirmed")))
msg := "public.subConfirmed"
if hasOptin {
msg = "public.subOptinPending"
}
return c.Render(http.StatusOK, tplMessage, makeMsgTpl(app.i18n.T("public.subTitle"), "", app.i18n.Ts(msg)))
}
// handleLinkRedirect redirects a link UUID to its original underlying link

87
cmd/subscribers.go
View File

@ -79,7 +79,11 @@ func handleGetSubscriber(c echo.Context) error {
id, _ = strconv.Atoi(c.Param("id"))
)
sub, err := getSubscriber(id, app)
if id < 1 {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID"))
}
sub, err := getSubscriber(id, "", "", app)
if err != nil {
return err
}
@ -273,14 +277,13 @@ func handleCreateSubscriber(c echo.Context) error {
}
// Insert the subscriber into the DB.
sub, err := insertSubscriber(req, app)
sub, isNew, _, err := insertSubscriber(req, app)
if err != nil {
if err == errSubscriberExists {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.emailExists"))
}
return err
}
if !isNew {
return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.emailExists"))
}
return c.JSON(http.StatusOK, okResp{sub})
}
@ -321,11 +324,11 @@ func handleUpdateSubscriber(c echo.Context) error {
}
// Send a confirmation e-mail (if there are any double opt-in lists).
sub, err := getSubscriber(int(id), app)
sub, err := getSubscriber(int(id), "", "", app)
if err != nil {
return err
}
_ = sendOptinConfirmation(sub, []int64(req.Lists), app)
_, _ = sendOptinConfirmation(sub, []int64(req.Lists), app)
return c.JSON(http.StatusOK, okResp{sub})
}
@ -355,7 +358,7 @@ func handleSubscriberSendOptin(c echo.Context) error {
app.i18n.Ts("globals.messages.notFound", "name", "{globals.terms.subscriber}"))
}
if err := sendOptinConfirmation(out[0], nil, app); err != nil {
if _, err := sendOptinConfirmation(out[0], nil, app); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError,
app.i18n.T("subscribers.errorSendingOptin"))
}
@ -619,56 +622,53 @@ func handleExportSubscriberData(c echo.Context) error {
return c.Blob(http.StatusOK, "application/json", b)
}
// insertSubscriber inserts a subscriber and returns the ID.
func insertSubscriber(req subimporter.SubReq, app *App) (models.Subscriber, error) {
// insertSubscriber inserts a subscriber and returns the ID. The first bool indicates if
// it was a new subscriber, and the second bool indicates if the subscriber was sent an optin confirmation.
func insertSubscriber(req subimporter.SubReq, app *App) (models.Subscriber, bool, bool, error) {
uu, err := uuid.NewV4()
if err != nil {
return req.Subscriber, err
return req.Subscriber, false, false, err
}
req.UUID = uu.String()
err = app.queries.InsertSubscriber.Get(&req.ID,
isNew := true
if err = app.queries.InsertSubscriber.Get(&req.ID,
req.UUID,
req.Email,
strings.TrimSpace(req.Name),
req.Status,
req.Attribs,
req.Lists,
req.ListUUIDs)
if err != nil {
req.ListUUIDs); err != nil {
if pqErr, ok := err.(*pq.Error); ok && pqErr.Constraint == "subscribers_email_key" {
return req.Subscriber, errSubscriberExists
isNew = false
} else {
// return req.Subscriber, errSubscriberExists
app.log.Printf("error inserting subscriber: %v", err)
return req.Subscriber, false, false, echo.NewHTTPError(http.StatusInternalServerError,
app.i18n.Ts("globals.messages.errorCreating",
"name", "{globals.terms.subscriber}", "error", pqErrMsg(err)))
}
app.log.Printf("error inserting subscriber: %v", err)
return req.Subscriber, echo.NewHTTPError(http.StatusInternalServerError,
app.i18n.Ts("globals.messages.errorCreating",
"name", "{globals.terms.subscriber}", "error", pqErrMsg(err)))
}
// Fetch the subscriber's full data.
sub, err := getSubscriber(req.ID, app)
// Fetch the subscriber's full data. If the subscriber already existed and wasn't
// created, the id will be empty. Fetch the details by e-mail then.
sub, err := getSubscriber(req.ID, "", strings.ToLower(req.Email), app)
if err != nil {
return sub, err
return sub, false, false, err
}
// Send a confirmation e-mail (if there are any double opt-in lists).
_ = sendOptinConfirmation(sub, []int64(req.Lists), app)
return sub, nil
num, _ := sendOptinConfirmation(sub, []int64(req.Lists), app)
return sub, isNew, num > 0, nil
}
// getSubscriber gets a single subscriber by ID.
func getSubscriber(id int, app *App) (models.Subscriber, error) {
var (
out models.Subscribers
)
// getSubscriber gets a single subscriber by ID, uuid, or e-mail in that order.
// Only one of these params should have a value.
func getSubscriber(id int, uuid, email string, app *App) (models.Subscriber, error) {
var out models.Subscribers
if id < 1 {
return models.Subscriber{},
echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("globals.messages.invalidID"))
}
if err := app.queries.GetSubscriber.Select(&out, id, nil); err != nil {
if err := app.queries.GetSubscriber.Select(&out, id, uuid, email); err != nil {
app.log.Printf("error fetching subscriber: %v", err)
return models.Subscriber{}, echo.NewHTTPError(http.StatusInternalServerError,
app.i18n.Ts("globals.messages.errorFetching",
@ -733,8 +733,9 @@ func exportSubscriberData(id int64, subUUID string, exportables map[string]bool,
}
// sendOptinConfirmation sends a double opt-in confirmation e-mail to a subscriber
// if at least one of the given listIDs is set to optin=double
func sendOptinConfirmation(sub models.Subscriber, listIDs []int64, app *App) error {
// if at least one of the given listIDs is set to optin=double. It returns the number of
// opt-in lists that were found.
func sendOptinConfirmation(sub models.Subscriber, listIDs []int64, app *App) (int, error) {
var lists []models.List
// Fetch double opt-in lists from the given list IDs.
@ -742,12 +743,12 @@ func sendOptinConfirmation(sub models.Subscriber, listIDs []int64, app *App) err
if err := app.queries.GetSubscriberLists.Select(&lists, sub.ID, nil,
pq.Int64Array(listIDs), nil, models.SubscriptionStatusUnconfirmed, models.ListOptinDouble); err != nil {
app.log.Printf("error fetching lists for opt-in: %s", pqErrMsg(err))
return err
return 0, err
}
// None.
if len(lists) == 0 {
return nil
return 0, nil
}
var (
@ -764,9 +765,9 @@ func sendOptinConfirmation(sub models.Subscriber, listIDs []int64, app *App) err
if err := app.sendNotification([]string{sub.Email},
app.i18n.T("subscribers.optinSubject"), notifSubscriberOptin, out); err != nil {
app.log.Printf("error sending opt-in e-mail: %s", err)
return err
return 0, err
}
return nil
return len(lists), nil
}
// sanitizeSQLExp does basic sanitisation on arbitrary

1
i18n/en.json
View File

@ -259,6 +259,7 @@
"public.privacyWipeHelp": "Delete all your subscriptions and related data from the database permanently.",
"public.sub": "Subscribe",
"public.subConfirmed": "Subscribed successfully.",
"public.subOptinPending": "An e-mail has been sent to you to confirm your subscription(s).",
"public.subConfirmedTitle": "Confirmed",
"public.subName": "Name (optional)",
"public.subNotFound": "Subscription not found.",

9
queries.sql
View File

@ -1,8 +1,13 @@
-- subscribers
-- name: get-subscriber
-- Get a single subscriber by id or UUID.
SELECT * FROM subscribers WHERE CASE WHEN $1 > 0 THEN id = $1 ELSE uuid = $2 END;
-- Get a single subscriber by id or UUID or email.
SELECT * FROM subscribers WHERE
CASE
WHEN $1 > 0 THEN id = $1
WHEN $2 != '' THEN uuid = $2::UUID
WHEN $3 != '' THEN email = $3
END;
-- name: subscriber-exists
-- Check if a subscriber exists by id or UUID.