From e8ad7a9adcbc7a268e0dbba6b217268bb210df46 Mon Sep 17 00:00:00 2001
From: Kailash Nadh <kailash@nadh.in>
Date: Wed, 10 Mar 2021 21:20:26 +0530
Subject: [PATCH] Fix subscriber attribs update API.

Change the behaviour where not passing attribs to the update API
overwrites the attribs with empty values. This commit changes the
behaviour so that in the absence of the attribs field in the
subscriber API, the existing value in the DB is retained.
---
 cmd/subscribers.go                    | 21 +++++++++++++++++++--
 frontend/src/views/SubscriberForm.vue | 20 +++++++++++++-------
 queries.sql                           |  2 +-
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/cmd/subscribers.go b/cmd/subscribers.go
index de9eb62..84d07bf 100644
--- a/cmd/subscribers.go
+++ b/cmd/subscribers.go
@@ -42,6 +42,13 @@ type subsWrap struct {
 	Page    int    `json:"page"`
 }
 
+type subUpdateReq struct {
+	models.Subscriber
+	RawAttribs json.RawMessage `json:"attribs"`
+	Lists      pq.Int64Array   `json:"lists"`
+	ListUUIDs  pq.StringArray  `json:"list_uuids"`
+}
+
 // subProfileData represents a subscriber's collated data in JSON
 // for export.
 type subProfileData struct {
@@ -293,7 +300,7 @@ func handleUpdateSubscriber(c echo.Context) error {
 	var (
 		app   = c.Get("app").(*App)
 		id, _ = strconv.ParseInt(c.Param("id"), 10, 64)
-		req   subimporter.SubReq
+		req   subUpdateReq
 	)
 	// Get and validate fields.
 	if err := c.Bind(&req); err != nil {
@@ -310,11 +317,21 @@ func handleUpdateSubscriber(c echo.Context) error {
 		return echo.NewHTTPError(http.StatusBadRequest, app.i18n.T("subscribers.invalidName"))
 	}
 
+	// If there's an attribs value, validate it.
+	if len(req.RawAttribs) > 0 {
+		var a models.SubscriberAttribs
+		if err := json.Unmarshal(req.RawAttribs, &a); err != nil {
+			return echo.NewHTTPError(http.StatusInternalServerError,
+				app.i18n.Ts("globals.messages.errorUpdating",
+					"name", "{globals.terms.subscriber}", "error", err.Error()))
+		}
+	}
+
 	_, err := app.queries.UpdateSubscriber.Exec(id,
 		strings.ToLower(strings.TrimSpace(req.Email)),
 		strings.TrimSpace(req.Name),
 		req.Status,
-		req.Attribs,
+		req.RawAttribs,
 		req.Lists)
 	if err != nil {
 		app.log.Printf("error updating subscriber: %v", err)
diff --git a/frontend/src/views/SubscriberForm.vue b/frontend/src/views/SubscriberForm.vue
index 7aab45b..8129e8f 100644
--- a/frontend/src/views/SubscriberForm.vue
+++ b/frontend/src/views/SubscriberForm.vue
@@ -97,9 +97,12 @@ export default Vue.extend({
     },
 
     createSubscriber() {
-      const attribs = this.validateAttribs(this.form.strAttribs);
-      if (!attribs) {
-        return;
+      let attribs = {};
+      if (this.form.strAttribs) {
+        attribs = this.validateAttribs(this.form.strAttribs);
+        if (!attribs) {
+          return;
+        }
       }
 
       const data = {
@@ -124,9 +127,12 @@ export default Vue.extend({
     },
 
     updateSubscriber() {
-      const attribs = this.validateAttribs(this.form.strAttribs);
-      if (!attribs) {
-        return;
+      let attribs = {};
+      if (this.form.strAttribs) {
+        attribs = this.validateAttribs(this.form.strAttribs);
+        if (!attribs) {
+          return;
+        }
       }
 
       const data = {
@@ -158,7 +164,7 @@ export default Vue.extend({
         attribs = JSON.parse(str);
       } catch (e) {
         this.$buefy.toast.open({
-          message: `${this.$t('subscribers.invalidJSON')}: e.toString()`,
+          message: `${this.$t('subscribers.invalidJSON')}: ${e.toString()}`,
           type: 'is-danger',
           duration: 3000,
           queue: false,
diff --git a/queries.sql b/queries.sql
index 6290423..285f7a1 100644
--- a/queries.sql
+++ b/queries.sql
@@ -120,7 +120,7 @@ WITH s AS (
         email=(CASE WHEN $2 != '' THEN $2 ELSE email END),
         name=(CASE WHEN $3 != '' THEN $3 ELSE name END),
         status=(CASE WHEN $4 != '' THEN $4::subscriber_status ELSE status END),
-        attribs=(CASE WHEN $5::TEXT != '' THEN $5::JSONB ELSE attribs END),
+        attribs=(CASE WHEN $5 != '' THEN $5::JSONB ELSE attribs END),
         updated_at=NOW()
     WHERE id = $1 RETURNING id
 ),