From 6be3352f52b5afe564128844eb43331680e0bb65 Mon Sep 17 00:00:00 2001
From: Kailash Nadh <kailash@nadh.in>
Date: Sun, 9 Feb 2020 16:25:19 +0530
Subject: [PATCH] Refactor/merge optin-list fetch queries

---
 campaigns.go   |  4 ++--
 public.go      | 20 +++++++-------------
 queries.sql    |  8 +++++++-
 subscribers.go | 13 +++++++------
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/campaigns.go b/campaigns.go
index bf9240d..46c09e2 100644
--- a/campaigns.go
+++ b/campaigns.go
@@ -597,9 +597,9 @@ func makeOptinCampaignMessage(o campaignReq, app *App) (campaignReq, error) {
 	var lists []models.List
 	err := app.Queries.GetListsByOptin.Select(&lists, models.ListOptinDouble, pq.Int64Array(o.ListIDs), nil)
 	if err != nil {
-		app.Logger.Printf("error fetching lists for optin: %s", pqErrMsg(err))
+		app.Logger.Printf("error fetching lists for opt-in: %s", pqErrMsg(err))
 		return o, echo.NewHTTPError(http.StatusInternalServerError,
-			"Error fetching optin lists.")
+			"Error fetching opt-in lists.")
 	}
 
 	// No opt-in lists.
diff --git a/public.go b/public.go
index 2dc595e..802f00d 100644
--- a/public.go
+++ b/public.go
@@ -136,20 +136,14 @@ func handleOptinPage(c echo.Context) error {
 						`One or more UUIDs in the request are invalid.`))
 			}
 		}
+	}
 
-		// Get lists by UUIDs.
-		if err := app.Queries.GetListsByOptin.Select(&out.Lists, models.ListOptinDouble, nil, pq.StringArray(out.ListUUIDs)); err != nil {
-			app.Logger.Printf("error fetching lists for optin: %s", pqErrMsg(err))
-			return c.Render(http.StatusInternalServerError, "message",
-				makeMsgTpl("Error", "", `Error fetching lists. Please retry.`))
-		}
-	} else {
-		// Otherwise, get the list of all unconfirmed lists for the subscriber.
-		if err := app.Queries.GetSubscriberLists.Select(&out.Lists, 0, subUUID, models.SubscriptionStatusUnconfirmed); err != nil {
-			app.Logger.Printf("error fetching lists for optin: %s", pqErrMsg(err))
-			return c.Render(http.StatusInternalServerError, "message",
-				makeMsgTpl("Error", "", `Error fetching lists. Please retry.`))
-		}
+	// Get the list of subscription lists where the subscriber hasn't confirmed.
+	if err := app.Queries.GetSubscriberLists.Select(&out.Lists, 0, subUUID,
+		nil, pq.StringArray(out.ListUUIDs), models.SubscriptionStatusUnconfirmed, nil); err != nil {
+		app.Logger.Printf("error fetching lists for opt-in: %s", pqErrMsg(err))
+		return c.Render(http.StatusInternalServerError, "message",
+			makeMsgTpl("Error", "", `Error fetching lists. Please retry.`))
 	}
 
 	// There are no lists to confirm.
diff --git a/queries.sql b/queries.sql
index f67d4e8..12b71d0 100644
--- a/queries.sql
+++ b/queries.sql
@@ -18,7 +18,13 @@ WITH sub AS (
 SELECT * FROM lists
     LEFT JOIN subscriber_lists ON (lists.id = subscriber_lists.list_id)
     WHERE subscriber_id = (SELECT id FROM sub)
-    AND (CASE WHEN $3 != '' THEN subscriber_lists.status = $3::subscription_status END);
+    -- Optional list IDs or UUIDs to filter.
+    AND (CASE WHEN $3::INT[] IS NOT NULL THEN id = ANY($3::INT[])
+          WHEN $4::UUID[] IS NOT NULL THEN uuid = ANY($4::UUID[])
+          ELSE TRUE
+    END)
+    AND (CASE WHEN $5 != '' THEN subscriber_lists.status = $5::subscription_status END)
+    AND (CASE WHEN $6 != '' THEN lists.optin = $6::list_optin ELSE TRUE END);
 
 -- name: get-subscriber-lists-lazy
 -- Get lists associations of subscribers given a list of subscriber IDs.
diff --git a/subscribers.go b/subscribers.go
index 86b745d..e18c254 100644
--- a/subscribers.go
+++ b/subscribers.go
@@ -167,11 +167,10 @@ func handleCreateSubscriber(c echo.Context) error {
 
 	// Insert and read ID.
 	var (
-		newID int
 		email = strings.ToLower(strings.TrimSpace(req.Email))
 	)
 	req.UUID = uuid.NewV4().String()
-	err := app.Queries.InsertSubscriber.Get(&newID,
+	err := app.Queries.InsertSubscriber.Get(&req.ID,
 		req.UUID,
 		email,
 		strings.TrimSpace(req.Name),
@@ -187,11 +186,12 @@ func handleCreateSubscriber(c echo.Context) error {
 	}
 
 	// If the lists are double-optins, send confirmation e-mails.
+	// Todo: This arbitrary goroutine should be moved to a centralised pool.
 	go sendOptinConfirmation(req.Subscriber, []int64(req.Lists), app)
 
 	// Hand over to the GET handler to return the last insertion.
 	c.SetParamNames("id")
-	c.SetParamValues(fmt.Sprintf("%d", newID))
+	c.SetParamValues(fmt.Sprintf("%d", req.ID))
 	return c.JSON(http.StatusOK, handleGetSubscriber(c))
 }
 
@@ -524,9 +524,10 @@ func sendOptinConfirmation(sub models.Subscriber, listIDs []int64, app *App) err
 	var lists []models.List
 
 	// Fetch double opt-in lists from the given list IDs.
-	err := app.Queries.GetListsByOptin.Select(&lists, models.ListOptinDouble, pq.Int64Array(listIDs))
-	if err != nil {
-		app.Logger.Printf("error fetching lists for optin: %s", pqErrMsg(err))
+	// Get the list of subscription lists where the subscriber hasn't confirmed.
+	if err := app.Queries.GetSubscriberLists.Select(&lists, sub.ID, nil,
+		pq.Int64Array(listIDs), nil, models.SubscriptionStatusUnconfirmed, nil); err != nil {
+		app.Logger.Printf("error fetching lists for opt-in: %s", pqErrMsg(err))
 		return err
 	}