Fix subscriber create query to not ignore duplicate e-mail error.

Trying to insert a pre-existing e-mail on POST /api/subscribers
now return a 409 Conflict error.

Closes #718

cmd/public.go

@@ -320,16 +320,21 @@ func handleSubscriptionForm(c echo.Context) error {
 			makeMsgTpl(app.i18n.T("public.errorTitle"), "", app.i18n.T("subscribers.invalidName")))
 	}
 
+	msg := "public.subConfirmed"
+
 	// Insert the subscriber into the DB.
 	req.Status = models.SubscriberStatusEnabled
 	req.ListUUIDs = pq.StringArray(req.SubListUUIDs)
-	_, _, hasOptin, err := app.core.CreateSubscriber(req.SubReq.Subscriber, nil, req.ListUUIDs, false)
+	_, hasOptin, err := app.core.CreateSubscriber(req.SubReq.Subscriber, nil, req.ListUUIDs, false)
 	if err != nil {
+		if e, ok := err.(*echo.HTTPError); ok && e.Code == http.StatusConflict {
+			return c.Render(http.StatusOK, tplMessage, makeMsgTpl(app.i18n.T("public.subTitle"), "", app.i18n.Ts(msg)))
+		}
+
 		return c.Render(http.StatusInternalServerError, tplMessage,
 			makeMsgTpl(app.i18n.T("public.errorTitle"), "", fmt.Sprintf("%s", err.(*echo.HTTPError).Message)))
 	}
 
-	msg := "public.subConfirmed"
 	if hasOptin {
 		msg = "public.subOptinPending"
 	}

cmd/subscribers.go

@@ -212,13 +212,10 @@ func handleCreateSubscriber(c echo.Context) error {
 	}
 
 	// Insert the subscriber into the DB.
-	sub, isNew, _, err := app.core.CreateSubscriber(req.Subscriber, req.Lists, req.ListUUIDs, req.PreconfirmSubs)
+	sub, _, err := app.core.CreateSubscriber(req.Subscriber, req.Lists, req.ListUUIDs, req.PreconfirmSubs)
 	if err != nil {
 		return err
 	}
-	if !isNew {
-		return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.emailExists"))
-	}
 
 	return c.JSON(http.StatusOK, okResp{sub})
 }

internal/core/subscribers.go

@@ -237,21 +237,17 @@ func (c *Core) ExportSubscribers(query string, subIDs, listIDs []int, batchSize
 
 // 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.
-// 1st bool = isNew?, 2nd bool = optinSent?
-func (c *Core) CreateSubscriber(sub models.Subscriber, listIDs []int, listUUIDs []string, preconfirm bool) (models.Subscriber, bool, bool, error) {
+// bool = optinSent?
+func (c *Core) CreateSubscriber(sub models.Subscriber, listIDs []int, listUUIDs []string, preconfirm bool) (models.Subscriber, bool, error) {
 	uu, err := uuid.NewV4()
 	if err != nil {
 		c.log.Printf("error generating UUID: %v", err)
-		return models.Subscriber{}, false, false, echo.NewHTTPError(http.StatusInternalServerError,
+		return models.Subscriber{}, false, echo.NewHTTPError(http.StatusInternalServerError,
 			c.i18n.Ts("globals.messages.errorUUID", "error", err.Error()))
 	}
 	sub.UUID = uu.String()
 
-	var (
-		isNew     = true
-		subStatus = models.SubscriptionStatusUnconfirmed
-	)
-
+	subStatus := models.SubscriptionStatusUnconfirmed
 	if preconfirm {
 		subStatus = models.SubscriptionStatusConfirmed
 	}
@@ -277,11 +273,12 @@ func (c *Core) CreateSubscriber(sub models.Subscriber, listIDs []int, listUUIDs
 		pq.Array(listUUIDs),
 		subStatus); err != nil {
 		if pqErr, ok := err.(*pq.Error); ok && pqErr.Constraint == "subscribers_email_key" {
-			isNew = false
+			return models.Subscriber{}, false, echo.NewHTTPError(http.StatusConflict,
+				c.i18n.T("subscribers.emailExists"))
 		} else {
 			// return sub.Subscriber, errSubscriberExists
 			c.log.Printf("error inserting subscriber: %v", err)
-			return models.Subscriber{}, false, false, echo.NewHTTPError(http.StatusInternalServerError,
+			return models.Subscriber{}, false, echo.NewHTTPError(http.StatusInternalServerError,
 				c.i18n.Ts("globals.messages.errorCreating",
 					"name", "{globals.terms.subscriber}", "error", pqErrMsg(err)))
 		}
@@ -291,7 +288,7 @@ func (c *Core) CreateSubscriber(sub models.Subscriber, listIDs []int, listUUIDs
 	// created, the id will be empty. Fetch the details by e-mail then.
 	out, err := c.GetSubscriber(sub.ID, "", sub.Email)
 	if err != nil {
-		return models.Subscriber{}, false, false, err
+		return models.Subscriber{}, false, err
 	}
 
 	hasOptin := false
@@ -301,7 +298,7 @@ func (c *Core) CreateSubscriber(sub models.Subscriber, listIDs []int, listUUIDs
 		hasOptin = num > 0
 	}
 
-	return out, isNew, hasOptin, nil
+	return out, hasOptin, nil
 }
 
 // UpdateSubscriber updates a subscriber's properties.

queries.sql

@@ -55,7 +55,6 @@ SELECT id as subscriber_id,
 WITH sub AS (
     INSERT INTO subscribers (uuid, email, name, status, attribs)
     VALUES($1, $2, $3, $4, $5)
-    ON CONFLICT(email) DO UPDATE SET updated_at=NOW()
     returning id, status
 ),
 listIDs AS (