From 6b92e3911208511122b041bd4547e0a57237b86e Mon Sep 17 00:00:00 2001 From: Tomas Bures Date: Sat, 6 May 2017 06:35:32 -0400 Subject: [PATCH] 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 | 1 + lib/models/subscriptions.js | 12 +- lib/subscription-mail-helpers.js | 316 +++++++++++++++---------------- routes/api.js | 57 ++++-- routes/lists.js | 2 +- routes/subscription.js | 166 ++++++++-------- 6 files changed, 295 insertions(+), 259 deletions(-) diff --git a/lib/models/confirmations.js b/lib/models/confirmations.js index dbac8497..9cbceee6 100644 --- a/lib/models/confirmations.js +++ b/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. diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js index 34147901..080acb5e 100644 --- a/lib/models/subscriptions.js +++ b/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) => { - // TODO: Once the unsubscription mode is customizable per segment, then this will be a good place to process it. - return list.unsubscriptionMode; -}; - +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. diff --git a/lib/subscription-mail-helpers.js b/lib/subscription-mail-helpers.js index 96beb978..cfbac97e 100644 --- a/lib/subscription-mail-helpers.js +++ b/lib/subscription-mail-helpers.js @@ -1,159 +1,157 @@ -'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'); - - -module.exports = { - sendAlreadySubscribed, - sendConfirmAddressChange, - sendConfirmSubscription, - sendConfirmUnsubscription, - sendSubscriptionConfirmed, - sendUnsubscriptionConfirmed -}; - -function sendSubscriptionConfirmed(list, email, subscription, callback) { - const relativeUrls = { - preferencesUrl: '/subscription/' + list.cid + '/manage/' + subscription.cid, - unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid - }; - - sendMail(list, email, 'subscription-confirmed', _('%s: Subscription Confirmed'), relativeUrls, {}, subscription, callback); -} - -function sendAlreadySubscribed(list, email, subscription, callback) { - const mailOpts = { - ignoreDisableConfirmations: true - }; - const relativeUrls = { - preferencesUrl: '/subscription/' + list.cid + '/manage/' + subscription.cid, - unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid - }; - sendMail(list, email, 'already-subscribed', _('%s: Email Address Already Registered'), relativeUrls, mailOpts, subscription, callback); -} - -function sendConfirmAddressChange(list, email, cid, subscription, callback) { - const mailOpts = { - ignoreDisableConfirmations: true - }; - const relativeUrls = { - confirmUrl: '/subscription/confirm/' + cid - }; - sendMail(list, email, 'confirm-address-change', _('%s: Please Confirm Email Change in Subscription'), relativeUrls, mailOpts, subscription, callback); -} - -function sendConfirmSubscription(list, email, cid, subscription, callback) { - const mailOpts = { - ignoreDisableConfirmations: true - }; - const relativeUrls = { - confirmUrl: '/subscription/confirm/' + cid - }; - sendMail(list, email, 'confirm-subscription', _('%s: Please Confirm Subscription'), relativeUrls, mailOpts, subscription, callback); -} - -function sendConfirmUnsubscription(list, email, cid, subscription, callback) { - const mailOpts = { - ignoreDisableConfirmations: true - }; - const relativeUrls = { - confirmUrl: '/subscription/confirm/' + cid - }; - sendMail(list, email, 'confirm-unsubscription', _('%s: Please Confirm Unsubscription'), relativeUrls, mailOpts, subscription, callback); -} - -function sendUnsubscriptionConfirmed(list, email, subscription, callback) { - const relativeUrls = { - subscribeUrl: '/subscription/' + list.cid + '?cid=' + subscription.cid - }; - sendMail(list, email, 'unsubscription-confirmed', _('%s: Unsubscription Confirmed'), relativeUrls, {}, subscription, callback); -} - - -function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscription, callback) { - fields.list(list.id, (err, fieldList) => { - if (err) { - return callback(err); - } - - let encryptionKeys = []; - fields.getRow(fieldList, subscription).forEach(field => { - if (field.type === 'gpg' && field.value) { - encryptionKeys.push(field.value.trim()); - } - }); - - settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl', 'disableConfirmations'], (err, configItems) => { - if (err) { - return callback(err); - } - - if (!mailOpts.ignoreDisableConfirmations && configItems.disableConfirmations) { - return; - } - - const data = { - title: list.name, - homepage: configItems.defaultHomepage || configItems.serviceUrl, - contactAddress: configItems.defaultAddress, - defaultPostaddress: configItems.defaultPostaddress, - }; - - for (let relativeUrlKey in relativeUrls) { - data[relativeUrlKey] = urllib.resolve(configItems.serviceUrl, relativeUrls[relativeUrlKey]); - } - - function sendMail(html, text) { - mailer.sendMail({ - from: { - name: configItems.defaultFrom, - address: configItems.defaultAddress - }, - to: { - name: [].concat(subscription.firstName || []).concat(subscription.lastName || []).join(' '), - address: email - }, - subject: util.format(subject, list.name), - encryptionKeys - }, { - html, - text, - data - }, err => { - if (err) { - log.error('Subscription', err); - } - }); - } - - let text = { - template: 'subscription/mail-' + template + '-text.hbs' - }; - - let html = { - template: 'subscription/mail-' + template + '-html.mjml.hbs', - layout: 'subscription/layout.mjml.hbs', - type: 'mjml' - }; - - helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { - if (err) { - return sendMail(html, text); - } - - sendMail(tmpl.html, tmpl.text); - }); - - return callback(); - }); - }); -} +'use strict'; + +const log = require('npmlog'); +let fields = require('./models/fields'); +let settings = require('./models/settings'); +let mailer = require('./mailer'); +let urllib = require('url'); +let helpers = require('./helpers'); +let _ = require('./translate')._; +let util = require('util'); + + +module.exports = { + sendAlreadySubscribed, + sendConfirmAddressChange, + sendConfirmSubscription, + sendConfirmUnsubscription, + sendSubscriptionConfirmed, + sendUnsubscriptionConfirmed +}; + +function sendSubscriptionConfirmed(list, email, subscription, callback) { + const relativeUrls = { + preferencesUrl: '/subscription/' + list.cid + '/manage/' + subscription.cid, + unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid + }; + + sendMail(list, email, 'subscription-confirmed', _('%s: Subscription Confirmed'), relativeUrls, {}, subscription, callback); +} + +function sendAlreadySubscribed(list, email, subscription, callback) { + const mailOpts = { + ignoreDisableConfirmations: true + }; + const relativeUrls = { + preferencesUrl: '/subscription/' + list.cid + '/manage/' + subscription.cid, + unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid + }; + sendMail(list, email, 'already-subscribed', _('%s: Email Address Already Registered'), relativeUrls, mailOpts, subscription, callback); +} + +function sendConfirmAddressChange(list, email, cid, subscription, callback) { + const mailOpts = { + ignoreDisableConfirmations: true + }; + const relativeUrls = { + confirmUrl: '/subscription/confirm/change-address/' + cid + }; + sendMail(list, email, 'confirm-address-change', _('%s: Please Confirm Email Change in Subscription'), relativeUrls, mailOpts, subscription, callback); +} + +function sendConfirmSubscription(list, email, cid, subscription, callback) { + const mailOpts = { + ignoreDisableConfirmations: true + }; + const relativeUrls = { + confirmUrl: '/subscription/confirm/subscribe/' + cid + }; + sendMail(list, email, 'confirm-subscription', _('%s: Please Confirm Subscription'), relativeUrls, mailOpts, subscription, callback); +} + +function sendConfirmUnsubscription(list, email, cid, subscription, callback) { + const mailOpts = { + ignoreDisableConfirmations: true + }; + const relativeUrls = { + confirmUrl: '/subscription/confirm/unsubscribe/' + cid + }; + sendMail(list, email, 'confirm-unsubscription', _('%s: Please Confirm Unsubscription'), relativeUrls, mailOpts, subscription, callback); +} + +function sendUnsubscriptionConfirmed(list, email, subscription, callback) { + const relativeUrls = { + subscribeUrl: '/subscription/' + list.cid + '?cid=' + subscription.cid + }; + sendMail(list, email, 'unsubscription-confirmed', _('%s: Unsubscription Confirmed'), relativeUrls, {}, subscription, callback); +} + + +function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscription, callback) { + fields.list(list.id, (err, fieldList) => { + if (err) { + return callback(err); + } + + let encryptionKeys = []; + fields.getRow(fieldList, subscription).forEach(field => { + if (field.type === 'gpg' && field.value) { + encryptionKeys.push(field.value.trim()); + } + }); + + settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl', 'disableConfirmations'], (err, configItems) => { + if (err) { + return callback(err); + } + + if (!mailOpts.ignoreDisableConfirmations && configItems.disableConfirmations) { + return; + } + + const data = { + title: list.name, + homepage: configItems.defaultHomepage || configItems.serviceUrl, + contactAddress: configItems.defaultAddress, + defaultPostaddress: configItems.defaultPostaddress + }; + + for (let relativeUrlKey in relativeUrls) { + data[relativeUrlKey] = urllib.resolve(configItems.serviceUrl, relativeUrls[relativeUrlKey]); + } + + function sendMail(html, text) { + mailer.sendMail({ + from: { + name: configItems.defaultFrom, + address: configItems.defaultAddress + }, + to: { + name: [].concat(subscription.firstName || []).concat(subscription.lastName || []).join(' '), + address: email + }, + subject: util.format(subject, list.name), + encryptionKeys + }, { + html, + text, + data + }, err => { + if (err) { + log.error('Subscription', err); + } + }); + } + + let text = { + template: 'subscription/mail-' + template + '-text.hbs' + }; + + let html = { + template: 'subscription/mail-' + template + '-html.mjml.hbs', + layout: 'subscription/layout.mjml.hbs', + type: 'mjml' + }; + + helpers.injectCustomFormTemplates(list.defaultForm, { text, html }, (err, tmpl) => { + if (err) { + return sendMail(html, text); + } + + sendMail(tmpl.html, tmpl.text); + }); + + return callback(); + }); + }); +} diff --git a/routes/api.js b/routes/api.js index 671bac8f..a44d6fec 100644 --- a/routes/api.js +++ b/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({ - data: { - id: cid + + mailHelpers.sendConfirmSubscription(list, input.EMAIL, confirmCid, subscription, (err) => { + if (err) { + 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({ - data: { - id: subscription.id, - unsubscribed: true + + if (!subscription) { + res.status(404); + return res.json({ + 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 + } + }); }); }); }); diff --git a/routes/lists.js b/routes/lists.js index 402c9095..4c28bf32 100644 --- a/routes/lists.js +++ b/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); diff --git a/routes/subscription.js b/routes/subscription.js index 9790eb72..235a6a3b 100644 --- a/routes/subscription.js +++ b/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) { - err = new Error(_('Selected subscription not found')); + if (!err && (!confirmation || confirmation.action !== action)) { + 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') { - if (!data.subscriptionId) { // Something went terribly wrong and we don't have data that we have originally provided - return next(new Error(_('Subscriber info corrupted or missing'))); +router.get('/confirm/subscribe/:cid', (req, res, next) => { + checkAndExecuteConfirmation(req, 'subscribe', 'Request invalid or already completed. If your subscription request is still pending, please subscribe again.', next, (confirmation, list) => { + 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) => { - 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); - }); - }); + res.redirect('/subscription/' + list.cid + '/subscribed-notice'); }); + }); + }); + }); +}); - } else if (confirmation.action === 'subscribe') { - let optInCountry = geoip.lookupCountry(confirmation.ip) || null; +router.get('/confirm/change-address/:cid', (req, res, next) => { + 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 = { - email: data.email, - optInIp: confirmation.ip, - optInCountry, - status: subscriptions.Status.SUBSCRIBED - }; + if (!data.subscriptionId) { // Something went terribly wrong and we don't have data that we have originally provided + return next(new Error(_('Subscriber info corrupted or missing'))); + } - 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); - } - - 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.updateAddress(list.id, data.subscriptionId, data.emailNew, err => { + if (err) { + return next(err); } + + 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'); + }); + }); }); }); });