Some fixes in lists and apis to reflect the changes in subscriptions.

Confirmation URLs split per action type. This allows more specific error reporting.

lib/models/confirmations.js

@@ -3,6 +3,7 @@
 let db = require('../db');
 let shortid = require('shortid');
 let helpers = require('../helpers');
+let _ = require('../translate')._;
 
 /*
     Adds new entry to the confirmations tables. Generates confirmation cid, which it returns.

lib/models/subscriptions.js

@@ -6,10 +6,7 @@ let tools = require('../tools');
 let helpers = require('../helpers');
 let fields = require('./fields');
 let segments = require('./segments');
-let settings = require('./settings');
-let mailer = require('../mailer');
 let _ = require('../translate')._;
-let util = require('util');
 let tableHelpers = require('../table-helpers');
 
 const Status = {
@@ -892,7 +889,7 @@ module.exports.updateAddress = (listId, subscriptionId, emailNew, callback) => {
 
                 let query = 'DELETE FROM `subscription__' + listId + '` WHERE `email`=? AND `id`<>?';
                 let args = [emailNew, subscriptionId];
-                connection.query(query, args, (err, rows) => {
+                connection.query(query, args, err => {
                     if (err) {
                         return helpers.rollbackAndReleaseConnection(connection, () => callback(err));
                     }
@@ -924,8 +921,5 @@ module.exports.updateAddress = (listId, subscriptionId, emailNew, callback) => {
 
 };
 
-module.exports.getUnsubscriptionMode = (list, subscriptionId) => {
+module.exports.getUnsubscriptionMode = (list, subscriptionId) => list.unsubscriptionMode; // eslint-disable-line no-unused-vars
-    // TODO: Once the unsubscription mode is customizable per segment, then this will be a good place to process it.
+// TODO: Once the unsubscription mode is customizable per segment, then this will be a good place to process it.
-    return list.unsubscriptionMode;
-};
-

lib/subscription-mail-helpers.js

@@ -1,13 +1,11 @@
 'use strict';
 
 const log = require('npmlog');
-const config = require('config');
 let fields = require('./models/fields');
 let settings = require('./models/settings');
 let mailer = require('./mailer');
 let urllib = require('url');
 let helpers = require('./helpers');
-let tools = require('./tools');
 let _ = require('./translate')._;
 let util = require('util');
 
@@ -46,7 +44,7 @@ function sendConfirmAddressChange(list, email, cid, subscription, callback) {
         ignoreDisableConfirmations: true
     };
     const relativeUrls = {
-        confirmUrl: '/subscription/confirm/' + cid
+        confirmUrl: '/subscription/confirm/change-address/' + cid
     };
     sendMail(list, email, 'confirm-address-change', _('%s: Please Confirm Email Change in Subscription'), relativeUrls, mailOpts, subscription, callback);
 }
@@ -56,7 +54,7 @@ function sendConfirmSubscription(list, email, cid, subscription, callback) {
         ignoreDisableConfirmations: true
     };
     const relativeUrls = {
-        confirmUrl: '/subscription/confirm/' + cid
+        confirmUrl: '/subscription/confirm/subscribe/' + cid
     };
     sendMail(list, email, 'confirm-subscription', _('%s: Please Confirm Subscription'), relativeUrls, mailOpts, subscription, callback);
 }
@@ -66,7 +64,7 @@ function sendConfirmUnsubscription(list, email, cid, subscription, callback) {
         ignoreDisableConfirmations: true
     };
     const relativeUrls = {
-        confirmUrl: '/subscription/confirm/' + cid
+        confirmUrl: '/subscription/confirm/unsubscribe/' + cid
     };
     sendMail(list, email, 'confirm-unsubscription', _('%s: Please Confirm Unsubscription'), relativeUrls, mailOpts, subscription, callback);
 }
@@ -105,7 +103,7 @@ function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscr
                 title: list.name,
                 homepage: configItems.defaultHomepage || configItems.serviceUrl,
                 contactAddress: configItems.defaultAddress,
-                defaultPostaddress: configItems.defaultPostaddress,
+                defaultPostaddress: configItems.defaultPostaddress
             };
 
             for (let relativeUrlKey in relativeUrls) {

routes/api.js

@@ -5,10 +5,12 @@ let lists = require('../lib/models/lists');
 let fields = require('../lib/models/fields');
 let blacklist = require('../lib/models/blacklist');
 let subscriptions = require('../lib/models/subscriptions');
+let confirmations = require('../lib/models/confirmations');
 let tools = require('../lib/tools');
 let express = require('express');
 let log = require('npmlog');
 let router = new express.Router();
+let mailHelpers = require('../lib/subscription-mail-helpers');
 
 router.all('/*', (req, res, next) => {
     if (!req.query.access_token) {
@@ -93,8 +95,6 @@ router.post('/subscribe/:listId', (req, res) => {
                 subscription.tz = (input.TIMEZONE || '').toString().trim();
             }
 
-            subscription._action = 'subscribe';
-
             fields.list(list.id, (err, fieldList) => {
                 if (err && !fieldList) {
                     fieldList = [];
@@ -125,7 +125,7 @@ router.post('/subscribe/:listId', (req, res) => {
                 }
 
                 if (/^(yes|true|1)$/i.test(input.REQUIRE_CONFIRMATION)) {
-                    subscriptions.addConfirmation(list, input.EMAIL, req.ip, subscription, (err, cid) => {
+                    confirmations.addConfirmation(list.id, 'subscribe', req.ip, subscription, (err, confirmCid) => {
                         if (err) {
                             log.error('API', err);
                             res.status(500);
@@ -134,11 +134,23 @@ router.post('/subscribe/:listId', (req, res) => {
                                 data: []
                             });
                         }
-                        res.status(200);
+
-                        res.json({
+                        mailHelpers.sendConfirmSubscription(list, input.EMAIL, confirmCid, subscription, (err) => {
-                            data: {
+                            if (err) {
-                                id: cid
+                                log.error('API', err);
+                                res.status(500);
+                                return res.json({
+                                    error: err.message || err,
+                                    data: []
+                                });
                             }
+
+                            res.status(200);
+                            res.json({
+                                data: {
+                                    id: confirmCid
+                                }
+                            });
                         });
                     });
                 } else {
@@ -191,7 +203,8 @@ router.post('/unsubscribe/:listId', (req, res) => {
                 data: []
             });
         }
-        subscriptions.unsubscribe(list.id, input.EMAIL, false, (err, subscription) => {
+
+        subscriptions.getByEmail(list.id, input.EMAIL, (err, subscription) => {
             if (err) {
                 res.status(500);
                 return res.json({
@@ -199,12 +212,30 @@ router.post('/unsubscribe/:listId', (req, res) => {
                     data: []
                 });
             }
-            res.status(200);
+
-            res.json({
+            if (!subscription) {
-                data: {
+                res.status(404);
-                    id: subscription.id,
+                return res.json({
-                    unsubscribed: true
+                    error: 'Subscription with given email not found',
+                    data: []
+                });
+            }
+
+            subscriptions.changeStatus(list.id, subscription.id, false, subscriptions.Status.UNSUBSCRIBED, (err, found) => {
+                if (err) {
+                    res.status(500);
+                    return res.json({
+                        error: err.message || err,
+                        data: []
+                    });
                 }
+                res.status(200);
+                res.json({
+                    data: {
+                        id: subscription.id,
+                        unsubscribed: true
+                    }
+                });
             });
         });
     });

routes/lists.js

@@ -451,7 +451,7 @@ router.post('/subscription/unsubscribe', passport.parseForm, passport.csrfProtec
                 return res.redirect('/lists/view/' + list.id);
             }
 
-            subscriptions.unsubscribe(list.id, subscription.email, false, err => {
+            subscriptions.changeStatus(list.id, subscription.id, false, subscriptions.Status.UNSUBSCRIBED, (err, found) => {
                 if (err) {
                     req.flash('danger', err && err.message || err || _('Could not unsubscribe user'));
                     return res.redirect('/lists/subscription/' + list.id + '/edit/' + subscription.cid);

routes/subscription.js

@@ -45,10 +45,10 @@ let corsOrCsrfProtection = (req, res, next) => {
     }
 };
 
-router.get('/confirm/:cid', (req, res, next) => {
+function checkAndExecuteConfirmation(req, action, errorMsg, next, exec) {
     confirmations.takeConfirmation(req.params.cid, (err, confirmation) => {
-        if (!err && !confirmation) {
+        if (!err && (!confirmation || confirmation.action !== action)) {
-            err = new Error(_('Selected subscription not found'));
+            err = new Error(_(errorMsg));
             err.status = 404;
         }
 
@@ -56,8 +56,6 @@ router.get('/confirm/:cid', (req, res, next) => {
             return next(err);
         }
 
-        const data = confirmation.data;
-
         lists.get(confirmation.listId, (err, list) => {
             if (!err && !list) {
                 err = new Error(_('Selected list not found'));
@@ -68,90 +66,104 @@ router.get('/confirm/:cid', (req, res, next) => {
                 return next(err);
             }
 
+            exec(confirmation, list);
+        });
+    });
+}
 
-            if (confirmation.action === 'change-address') {
+router.get('/confirm/subscribe/:cid', (req, res, next) => {
-                if (!data.subscriptionId) { // Something went terribly wrong and we don't have data that we have originally provided
+    checkAndExecuteConfirmation(req, 'subscribe', 'Request invalid or already completed. If your subscription request is still pending, please subscribe again.', next, (confirmation, list) => {
-                    return next(new Error(_('Subscriber info corrupted or missing')));
+        const data = confirmation.data;
+        let optInCountry = geoip.lookupCountry(confirmation.ip) || null;
+
+        const meta = {
+            email: data.email,
+            optInIp: confirmation.ip,
+            optInCountry,
+            status: subscriptions.Status.SUBSCRIBED
+        };
+
+        subscriptions.insert(list.id, meta, data.subscriptionData, (err, result) => {
+            if (err) {
+                return next(err);
+            }
+
+            if (!result.entryId) {
+                return next(new Error(_('Could not save subscription')));
+            }
+
+            subscriptions.getById(list.id, result.entryId, (err, subscription) => {
+                if (err) {
+                    return next(err);
                 }
 
-                subscriptions.updateAddress(list.id, data.subscriptionId, data.emailNew, err => {
+                mailHelpers.sendSubscriptionConfirmed(list, data.email, subscription, err => {
                     if (err) {
                         return next(err);
                     }
 
-                    subscriptions.getById(list.id, data.subscriptionId, (err, subscription) => {
+                    res.redirect('/subscription/' + list.cid + '/subscribed-notice');
-                        if (err) {
-                            return next(err);
-                        }
-
-                        mailHelpers.sendSubscriptionConfirmed(list, data.emailNew, subscription, err => {
-                            if (err) {
-                                return next(err);
-                            }
-
-                            req.flash('info', _('Email address changed'));
-                            res.redirect('/subscription/' + list.cid + '/manage/' + subscription.cid);
-                        });
-                    });
                 });
+            });
+        });
+    });
+});
 
-            } else if (confirmation.action === 'subscribe') {
+router.get('/confirm/change-address/:cid', (req, res, next) => {
-                let optInCountry = geoip.lookupCountry(confirmation.ip) || null;
+    checkAndExecuteConfirmation(req, 'change-address', 'Request invalid or already completed. If your address change request is still pending, please change the address again.', next, (confirmation, list) => {
+        const data = confirmation.data;
 
-                const meta = {
+        if (!data.subscriptionId) { // Something went terribly wrong and we don't have data that we have originally provided
-                    email: data.email,
+            return next(new Error(_('Subscriber info corrupted or missing')));
-                    optInIp: confirmation.ip,
+        }
-                    optInCountry,
-                    status: subscriptions.Status.SUBSCRIBED
-                };
 
-                subscriptions.insert(list.id, meta, data.subscriptionData, (err, result) => {
+        subscriptions.updateAddress(list.id, data.subscriptionId, data.emailNew, err => {
-                    if (err) {
+            if (err) {
-                        return next(err);
+                return next(err);
-                    }
-
-                    if (!result.entryId) {
-                        return next(new Error(_('Could not save subscription')));
-                    }
-
-                    subscriptions.getById(list.id, result.entryId, (err, subscription) => {
-                        if (err) {
-                            return next(err);
-                        }
-
-                        mailHelpers.sendSubscriptionConfirmed(list, data.email, subscription, err => {
-                            if (err) {
-                                return next(err);
-                            }
-
-                            res.redirect('/subscription/' + list.cid + '/subscribed-notice');
-                        });
-                    });
-                });
-
-            } else if (confirmation.action === 'unsubscribe') {
-                subscriptions.changeStatus(list.id, confirmation.data.subscriptionId, confirmation.data.campaignId, subscriptions.Status.UNSUBSCRIBED, (err, found) => {
-                    if (err) {
-                        return next(err);
-                    }
-
-                    // TODO: Shall we do anything with "found"?
-
-                    subscriptions.getById(list.id, confirmation.data.subscriptionId, (err, subscription) => {
-                        if (err) {
-                            return next(err);
-                        }
-
-                        mailHelpers.sendUnsubscriptionConfirmed(list, subscription.email, subscription, err => {
-                            if (err) {
-                                return next(err);
-                            }
-
-                            res.redirect('/subscription/' + list.cid + '/unsubscribed-notice');
-                        });
-                    });
-                });
             }
+
+            subscriptions.getById(list.id, data.subscriptionId, (err, subscription) => {
+                if (err) {
+                    return next(err);
+                }
+
+                mailHelpers.sendSubscriptionConfirmed(list, data.emailNew, subscription, err => {
+                    if (err) {
+                        return next(err);
+                    }
+
+                    req.flash('info', _('Email address changed'));
+                    res.redirect('/subscription/' + list.cid + '/manage/' + subscription.cid);
+                });
+            });
+        });
+    });
+});
+
+router.get('/confirm/unsubscribe/:cid', (req, res, next) => {
+    checkAndExecuteConfirmation(req, 'unsubscribe', 'Request invalid or already completed. If your unsubscription request is still pending, please unsubscribe again.', next, (confirmation, list) => {
+        const data = confirmation.data;
+
+        subscriptions.changeStatus(list.id, confirmation.data.subscriptionId, confirmation.data.campaignId, subscriptions.Status.UNSUBSCRIBED, (err, found) => {
+            if (err) {
+                return next(err);
+            }
+
+            // TODO: Shall we do anything with "found"?
+
+            subscriptions.getById(list.id, confirmation.data.subscriptionId, (err, subscription) => {
+                if (err) {
+                    return next(err);
+                }
+
+                mailHelpers.sendUnsubscriptionConfirmed(list, subscription.email, subscription, err => {
+                    if (err) {
+                        return next(err);
+                    }
+
+                    res.redirect('/subscription/' + list.cid + '/unsubscribed-notice');
+                });
+            });
         });
     });
 });