From 9a5d723663d2937cabefb69f7c3c6788d4a07ec6 Mon Sep 17 00:00:00 2001
From: Andris Reinman <andris@kreata.ee>
Date: Wed, 25 May 2016 13:47:14 +0300
Subject: [PATCH] Fixed unsubscription bug #49

---
 lib/models/subscriptions.js        | 51 +++++++++++++++++++++++++++---
 routes/subscription.js             |  4 ++-
 views/campaigns/delivered.hbs      |  2 +-
 views/subscription/unsubscribe.hbs |  1 +
 4 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js
index 3666be24..addd0972 100644
--- a/lib/models/subscriptions.js
+++ b/lib/models/subscriptions.js
@@ -703,7 +703,8 @@ module.exports.changeStatus = (id, listId, campaignId, status, callback) => {
                             });
                         }
 
-                        if (!campaignId) {
+                        // status change is not related to a campaign or it marks message as bounced etc.
+                        if (!campaignId || status > 2) {
                             return connection.commit(err => {
                                 if (err) {
                                     return connection.rollback(() => {
@@ -716,22 +717,62 @@ module.exports.changeStatus = (id, listId, campaignId, status, callback) => {
                             });
                         }
 
-                        connection.query('UPDATE `campaigns` SET `unsubscribed`=`unsubscribed`+1 WHERE `cid`=? LIMIT 1', [campaignId], err => {
+                        connection.query('SELECT `id` FROM `campaigns` WHERE `cid`=? LIMIT 1', [campaignId], (err, rows) => {
                             if (err) {
                                 return connection.rollback(() => {
                                     connection.release();
                                     return callback(err);
                                 });
                             }
-                            return connection.commit(err => {
+
+                            let campaign = rows && rows[0] || false;
+
+                            if (!campaign) {
+                                // should not happend
+                                return connection.commit(err => {
+                                    if (err) {
+                                        return connection.rollback(() => {
+                                            connection.release();
+                                            return callback(err);
+                                        });
+                                    }
+                                    connection.release();
+                                    return callback(null, true);
+                                });
+                            }
+
+                            // we should see only unsubscribe events here but you never know
+                            connection.query('UPDATE `campaigns` SET `unsubscribed`=`unsubscribed`' + (status === 2 ? '+' : '-') + '1 WHERE `cid`=? LIMIT 1', [campaignId], err => {
                                 if (err) {
                                     return connection.rollback(() => {
                                         connection.release();
                                         return callback(err);
                                     });
                                 }
-                                connection.release();
-                                return callback(null, true);
+
+                                let query = 'UPDATE `campaign__' + campaign.id + '` SET `status`=? WHERE `list`=? AND `subscription`=? LIMIT 1';
+                                let values = [status, listId, id];
+
+                                // Updated tracker status
+                                connection.query(query, values, err => {
+                                    if (err) {
+                                        return connection.rollback(() => {
+                                            connection.release();
+                                            return callback(err);
+                                        });
+                                    }
+
+                                    return connection.commit(err => {
+                                        if (err) {
+                                            return connection.rollback(() => {
+                                                connection.release();
+                                                return callback(err);
+                                            });
+                                        }
+                                        connection.release();
+                                        return callback(null, true);
+                                    });
+                                });
                             });
                         });
                     });
diff --git a/routes/subscription.js b/routes/subscription.js
index afb36bd1..7424eeef 100644
--- a/routes/subscription.js
+++ b/routes/subscription.js
@@ -305,6 +305,7 @@ router.post('/:lcid/manage', passport.parseForm, passport.csrfProtection, (req,
         subscriptions.update(list.id, req.body.cid, req.body, false, err => {
             if (err) {
                 req.flash('danger', err.message || err);
+                log.error('Subscription', err);
                 return res.redirect('/subscription/' + encodeURIComponent(req.params.lcid) + '/manage/' + encodeURIComponent(req.body.cid) + '?' + tools.queryParams(req.body));
             }
             res.redirect('/subscription/' + req.params.lcid + '/updated-notice');
@@ -324,7 +325,7 @@ router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next)
         }
 
         subscriptions.get(list.id, req.params.ucid, (err, subscription) => {
-            if (!err && !list) {
+            if (!err && !subscription) {
                 err = new Error('Subscription not found from this list');
                 err.status = 404;
             }
@@ -360,6 +361,7 @@ router.post('/:lcid/unsubscribe', passport.parseForm, passport.csrfProtection, (
         subscriptions.unsubscribe(list.id, email, req.body.campaign, (err, subscription) => {
             if (err) {
                 req.flash('danger', err.message || err);
+                log.error('Subscription', err);
                 return res.redirect('/subscription/' + encodeURIComponent(req.params.lcid) + '/unsubscribe/' + encodeURIComponent(req.body.cid) + '?' + tools.queryParams(req.body));
             }
             res.redirect('/subscription/' + req.params.lcid + '/unsubscribe-notice');
diff --git a/views/campaigns/delivered.hbs b/views/campaigns/delivered.hbs
index fec89f6d..89bc8194 100644
--- a/views/campaigns/delivered.hbs
+++ b/views/campaigns/delivered.hbs
@@ -20,7 +20,7 @@
 
     <div class="panel panel-info">
         <!-- Default panel contents -->
-        <div class="panel-heading">Subscribers who received the message:</div>
+        <div class="panel-heading">Subscribers who received the message and did not bounce/unsubscribe:</div>
         <div class="panel-body">
             <div class="table-responsive">
                 <table data-topic-url="/campaigns/status" data-topic-id="{{id}}/1" data-sort-column="1" data-sort-order="asc" class="table table-bordered table-hover data-table-ajax display nowrap" width="100%" data-row-sort="0,1,1,1,0,1,0">
diff --git a/views/subscription/unsubscribe.hbs b/views/subscription/unsubscribe.hbs
index ce626782..1c230df5 100644
--- a/views/subscription/unsubscribe.hbs
+++ b/views/subscription/unsubscribe.hbs
@@ -8,6 +8,7 @@
 
     <input type="hidden" name="_csrf" value="{{csrfToken}}">
     <input type="hidden" name="campaign" value="{{campaign}}">
+    <input type="hidden" name="cid" value="{{cid}}">
 
     <div class="form-group">
         <label for="email">Email address</label>