diff --git a/languages/de_DE.po b/languages/de_DE.po index 69020cc7..73774df1 100644 --- a/languages/de_DE.po +++ b/languages/de_DE.po @@ -4030,7 +4030,7 @@ msgstr "" "Bestätigung" #: routes/subscription.js:730 -msgid "%s: Unsubscribe Confirmed" +msgid "%s: Unsubscription Confirmed" msgstr "%s: Abmeldungen Bestätigt" #: routes/subscription.js:777 routes/subscription.js:793 diff --git a/lib/helpers.js b/lib/helpers.js index 8324a811..70a9b232 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -98,7 +98,7 @@ function filterCustomFields(customFieldsIn = [], fieldIds = [], method = 'includ id: 'email', name: 'Email Address', type: 'Email', - typeSubsciptionEmail: true + typeSubscriptionEmail: true }, { id: 'firstname', name: 'First Name', diff --git a/lib/models/campaigns.js b/lib/models/campaigns.js index c25c57ee..bec512bb 100644 --- a/lib/models/campaigns.js +++ b/lib/models/campaigns.js @@ -1054,13 +1054,13 @@ module.exports.updateMessage = (message, status, updateSubscription, callback) = let statusCode; if (status === 'unsubscribed') { - statusCode = 2; - } - if (status === 'bounced') { - statusCode = 3; - } - if (status === 'complained') { - statusCode = 4; + statusCode = subscriptions.Status.UNSUBSCRIBED; + } else if (status === 'bounced') { + statusCode = subscriptions.Status.BOUNCED; + } else if (status === 'complained') { + statusCode = subscriptions.Status.COMPLAINED; + } else { + return callback(new Error(_('Unrecognized message status'))); } let query = 'UPDATE `campaigns` SET `' + status + '`=`' + status + '`+1 WHERE id=? LIMIT 1'; @@ -1074,7 +1074,7 @@ module.exports.updateMessage = (message, status, updateSubscription, callback) = } if (updateSubscription) { - subscriptions.changeStatus(message.list, message.subscription, statusCode === 2 ? message.campaign : false, statusCode, callback); + subscriptions.changeStatus(message.list, message.subscription, statusCode === subscriptions.Status.UNSUBSCRIBED ? message.campaign : false, statusCode, callback); } else { return callback(null, true); } diff --git a/lib/models/confirmations.js b/lib/models/confirmations.js index 66025ed1..dbac8497 100644 --- a/lib/models/confirmations.js +++ b/lib/models/confirmations.js @@ -16,14 +16,14 @@ module.exports.addConfirmation = (listId, action, ip, data, callback) => { } let query = 'INSERT INTO confirmations (cid, list, action, ip, data) VALUES (?,?,?,?,?)'; - connection.query(query, [cid, list, action, ip, JSON.stringify(data || {})], (err, result) => { + connection.query(query, [cid, listId, action, ip, JSON.stringify(data || {})], (err, result) => { connection.release(); if (err) { return callback(err); } if (!result || !result.affectedRows) { - return callback(null, false); + return callback(new Error(_('Could not store confirmation data'))); } return callback(null, cid); diff --git a/lib/models/reports.js b/lib/models/reports.js index aac2d967..edfe665f 100644 --- a/lib/models/reports.js +++ b/lib/models/reports.js @@ -13,7 +13,8 @@ const ReportState = { SCHEDULED: 0, PROCESSING: 1, FINISHED: 2, - FAILED: 3 + FAILED: 3, + MAX: 4 }; module.exports.ReportState = ReportState; diff --git a/lib/models/settings.js b/lib/models/settings.js index 77b9f391..9655ebd7 100644 --- a/lib/models/settings.js +++ b/lib/models/settings.js @@ -15,6 +15,8 @@ function listValues(filter, callback) { filter = false; } + // TODO: It would be good to cache the settings. It feels awkward to always go to DB to retrieve something what is essentially a constant + filter = [].concat(filter || []).map(key => tools.toDbKey(key)); db.getConnection((err, connection) => { diff --git a/lib/models/subscriptions.js b/lib/models/subscriptions.js index 08f2cbac..34147901 100644 --- a/lib/models/subscriptions.js +++ b/lib/models/subscriptions.js @@ -12,6 +12,16 @@ let _ = require('../translate')._; let util = require('util'); let tableHelpers = require('../table-helpers'); +const Status = { + SUBSCRIBED: 1, + UNSUBSCRIBED: 2, + BOUNCED: 3, + COMPLAINED: 4, + MAX: 5 +}; + +module.exports.Status = Status; + module.exports.list = (listId, start, limit, callback) => { listId = Number(listId) || 0; if (!listId) { @@ -152,19 +162,19 @@ module.exports.insert = (listId, meta, subscriptionData, callback) => { let entryId = existing ? existing.id : false; meta.cid = existing ? rows[0].cid : meta.cid; - meta.status = meta.status || (existing ? existing.status : 1); + meta.status = meta.status || (existing ? existing.status : Status.SUBSCRIBED); let statusChange = !existing || existing.status !== meta.status; let statusDirection; - if (existing && !meta.partial) { - return helpers.rollbackAndReleaseConnection(connection, new Error(_('Email address already registered')), callback); + if (existing && existing.status === Status.SUBSCRIBED && !meta.partial) { + return helpers.rollbackAndReleaseConnection(connection, () => callback(new Error(_('Email address already registered')))); } if (statusChange) { keys.push('status', 'status_change'); values.push(meta.status, new Date()); - statusDirection = !existing ? (meta.status === 1 ? '+' : false) : (existing.status === 1 ? '-' : '+'); + statusDirection = !existing ? (meta.status === Status.SUBSCRIBED ? '+' : false) : (existing.status === Status.SUBSCRIBED ? '-' : '+'); } if (!keys.length) { @@ -458,8 +468,8 @@ module.exports.changeStatus = (listId, id, campaignId, status, callback) => { return helpers.rollbackAndReleaseConnection(connection, () => callback(null, true)); } - if (statusChange && oldStatus === 1 || status === 1) { - statusDirection = status === 1 ? '+' : '-'; + if (statusChange && oldStatus === Status.SUBSCRIBED || status === Status.SUBSCRIBED) { + statusDirection = status === Status.SUBSCRIBED ? '+' : '-'; } connection.query('UPDATE `subscription__' + listId + '` SET `status`=?, `status_change`=NOW() WHERE id=? LIMIT 1', [status, id], err => { @@ -483,7 +493,7 @@ module.exports.changeStatus = (listId, id, campaignId, status, callback) => { } // status change is not related to a campaign or it marks message as bounced etc. - if (!campaignId || status > 2) { + if (!campaignId || status !== Status.SUBSCRIBED) { return connection.commit(err => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); @@ -512,7 +522,7 @@ module.exports.changeStatus = (listId, id, campaignId, status, callback) => { } // 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 => { + connection.query('UPDATE `campaigns` SET `unsubscribed`=`unsubscribed`' + (status === Status.UNSUBSCRIBED ? '+' : '-') + '1 WHERE `cid`=? LIMIT 1', [campaignId], err => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); } @@ -583,7 +593,7 @@ module.exports.delete = (listId, cid, callback) => { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); } - if (subscription.status !== 1) { + if (subscription.status !== Status.SUBSCRIBED) { return connection.commit(err => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); @@ -679,11 +689,10 @@ module.exports.updateImport = (listId, importId, data, callback) => { connection.release(); return callback(null, affected); }); - return; + } else { + connection.release(); + return callback(null, affected); } - - connection.release(); - return callback(null, affected); }); }); }; @@ -816,7 +825,7 @@ module.exports.updateAddressCheck = (list, cid, emailNew, ip, callback) => { return callback(err); } - let query = 'SELECT * FROM `subscription__' + list.id + '` WHERE `cid`=? LIMIT 1'; + let query = 'SELECT * FROM `subscription__' + list.id + '` WHERE `cid`=? AND `status`=' + Status.SUBSCRIBED + ' LIMIT 1'; let args = [cid]; connection.query(query, args, (err, rows) => { if (err) { @@ -835,7 +844,7 @@ module.exports.updateAddressCheck = (list, cid, emailNew, ip, callback) => { let old = rows[0]; - let query = 'SELECT `id` FROM `subscription__' + list.id + '` WHERE `email`=? AND `cid`<>? AND `status`=1 LIMIT 1'; + let query = 'SELECT `id` FROM `subscription__' + list.id + '` WHERE `email`=? AND `cid`<>? AND `status`=' + Status.SUBSCRIBED + ' LIMIT 1'; let args = [emailNew, cid]; connection.query(query, args, (err, rows) => { connection.release(); @@ -870,7 +879,7 @@ module.exports.updateAddress = (listId, subscriptionId, emailNew, callback) => { return callback(err); } - let query = 'SELECT `id` FROM `subscription__' + listId + '` WHERE `email`=? AND `id`<>? AND `status`=1 LIMIT 1'; + let query = 'SELECT `id` FROM `subscription__' + listId + '` WHERE `email`=? AND `id`<>? AND `status`=' + Status.SUBSCRIBED + ' LIMIT 1'; let args = [emailNew, subscriptionId]; connection.query(query, args, (err, rows) => { if (err) { @@ -878,7 +887,7 @@ module.exports.updateAddress = (listId, subscriptionId, emailNew, callback) => { } if (rows && rows.length > 0) { - return helpers.rollbackAndReleaseConnection(connection, new Error(_('Email address already registered')), callback); + return helpers.rollbackAndReleaseConnection(connection, () => callback(new Error(_('Email address already registered')))); } let query = 'DELETE FROM `subscription__' + listId + '` WHERE `email`=? AND `id`<>?'; @@ -888,13 +897,17 @@ module.exports.updateAddress = (listId, subscriptionId, emailNew, callback) => { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); } - let query = 'UPDATE `subscription__' + listId + '` SET `email`=? WHERE `id`=? LIMIT 1'; + let query = 'UPDATE `subscription__' + listId + '` SET `email`=? WHERE `id`=? AND `status`=' + Status.SUBSCRIBED + ' LIMIT 1'; let args = [emailNew, subscriptionId]; - connection.query(query, args, err => { + connection.query(query, args, (err, result) => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); } + if (!result || !result.affectedRows) { + return helpers.rollbackAndReleaseConnection(connection, () => callback(new Error(_('Subscription not found in this list')))); + } + return connection.commit(err => { if (err) { return helpers.rollbackAndReleaseConnection(connection, () => callback(err)); diff --git a/lib/subscription-mail-helpers.js b/lib/subscription-mail-helpers.js index 3211a6b9..96beb978 100644 --- a/lib/subscription-mail-helpers.js +++ b/lib/subscription-mail-helpers.js @@ -2,7 +2,6 @@ const log = require('npmlog'); const config = require('config'); -let db = require('./db'); let fields = require('./models/fields'); let settings = require('./models/settings'); let mailer = require('./mailer'); @@ -28,7 +27,7 @@ function sendSubscriptionConfirmed(list, email, subscription, callback) { unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid }; - subscriptions.sendMail(list, email, 'subscription-confirmed', _('%s: Subscription Confirmed'), relativeUrls, {}, data.subscriptionData, callback); + sendMail(list, email, 'subscription-confirmed', _('%s: Subscription Confirmed'), relativeUrls, {}, subscription, callback); } function sendAlreadySubscribed(list, email, subscription, callback) { @@ -39,7 +38,7 @@ function sendAlreadySubscribed(list, email, subscription, callback) { preferencesUrl: '/subscription/' + list.cid + '/manage/' + subscription.cid, unsubscribeUrl: '/subscription/' + list.cid + '/unsubscribe/' + subscription.cid }; - module.exports.sendMail(list, email, 'already-subscribed', _('%s: Email Address Already Registered'), relativeUrls, mailOpts, subscription, callback); + sendMail(list, email, 'already-subscribed', _('%s: Email Address Already Registered'), relativeUrls, mailOpts, subscription, callback); } function sendConfirmAddressChange(list, email, cid, subscription, callback) { @@ -49,7 +48,7 @@ function sendConfirmAddressChange(list, email, cid, subscription, callback) { const relativeUrls = { confirmUrl: '/subscription/confirm/' + cid }; - module.exports.sendMail(list, email, 'confirm-address-change', _('%s: Please Confirm Email Change in Subscription'), relativeUrls, mailOpts, subscription, callback); + sendMail(list, email, 'confirm-address-change', _('%s: Please Confirm Email Change in Subscription'), relativeUrls, mailOpts, subscription, callback); } function sendConfirmSubscription(list, email, cid, subscription, callback) { @@ -59,7 +58,7 @@ function sendConfirmSubscription(list, email, cid, subscription, callback) { const relativeUrls = { confirmUrl: '/subscription/confirm/' + cid }; - module.exports.sendMail(list, email, 'confirm-subscription', _('%s: Please Confirm Subscription'), relativeUrls, mailOpts, subscription, callback); + sendMail(list, email, 'confirm-subscription', _('%s: Please Confirm Subscription'), relativeUrls, mailOpts, subscription, callback); } function sendConfirmUnsubscription(list, email, cid, subscription, callback) { @@ -69,98 +68,92 @@ function sendConfirmUnsubscription(list, email, cid, subscription, callback) { const relativeUrls = { confirmUrl: '/subscription/confirm/' + cid }; - module.exports.sendMail(list, email, 'confirm-unsubscription', _('%s: Please Confirm Unsubscription'), relativeUrls, mailOpts, subscription, callback); + 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 }; - subscriptions.sendMail(list, email, 'unsubscription-confirmed', _('%s: Unsubscribe Confirmed'), relativeUrls, {}, subscription, callback); + sendMail(list, email, 'unsubscription-confirmed', _('%s: Unsubscription Confirmed'), relativeUrls, {}, subscription, callback); } function sendMail(list, email, template, subject, relativeUrls, mailOpts, subscription, callback) { - db.getConnection((err, connection) => { + fields.list(list.id, (err, fieldList) => { if (err) { return callback(err); } - fields.list(list.id, (err, fieldList) => { + 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); } - let encryptionKeys = []; - fields.getRow(fieldList, subscription).forEach(field => { - if (field.type === 'gpg' && field.value) { - encryptionKeys.push(field.value.trim()); - } - }); + if (!mailOpts.ignoreDisableConfirmations && configItems.disableConfirmations) { + return; + } - settings.list(['defaultHomepage', 'defaultFrom', 'defaultAddress', 'defaultPostaddress', 'serviceUrl', 'disableConfirmations'], (err, configItems) => { - if (err) { - return callback(err); - } + const data = { + title: list.name, + homepage: configItems.defaultHomepage || configItems.serviceUrl, + contactAddress: configItems.defaultAddress, + defaultPostaddress: configItems.defaultPostaddress, + }; - if (!mailOpts.ignoreDisableConfirmations && configItems.disableConfirmations) { - return; - } + for (let relativeUrlKey in relativeUrls) { + data[relativeUrlKey] = urllib.resolve(configItems.serviceUrl, relativeUrls[relativeUrlKey]); + } - 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) => { + 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) { - return sendMail(html, text); + log.error('Subscription', err); } - - sendMail(tmpl.html, tmpl.text); }); + } - return callback(); + 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/subscription.js b/routes/subscription.js index d3e712e9..9790eb72 100644 --- a/routes/subscription.js +++ b/routes/subscription.js @@ -47,7 +47,7 @@ let corsOrCsrfProtection = (req, res, next) => { router.get('/confirm/:cid', (req, res, next) => { confirmations.takeConfirmation(req.params.cid, (err, confirmation) => { - if (!err && !subscription) { + if (!err && !confirmation) { err = new Error(_('Selected subscription not found')); err.status = 404; } @@ -79,7 +79,7 @@ router.get('/confirm/:cid', (req, res, next) => { return next(err); } - subscriptions.getById(list.id, subscriptionId, (err, subscription) => { + subscriptions.getById(list.id, data.subscriptionId, (err, subscription) => { if (err) { return next(err); } @@ -89,7 +89,8 @@ router.get('/confirm/:cid', (req, res, next) => { return next(err); } - res.redirect('/subscription/' + list.cid + '/manage-address/' + subscription.cid); + req.flash('info', _('Email address changed')); + res.redirect('/subscription/' + list.cid + '/manage/' + subscription.cid); }); }); }); @@ -101,7 +102,7 @@ router.get('/confirm/:cid', (req, res, next) => { email: data.email, optInIp: confirmation.ip, optInCountry, - status: 1 + status: subscriptions.Status.SUBSCRIBED }; subscriptions.insert(list.id, meta, data.subscriptionData, (err, result) => { @@ -129,7 +130,7 @@ router.get('/confirm/:cid', (req, res, next) => { }); } else if (confirmation.action === 'unsubscribe') { - subscriptions.changeStatus(list.id, confirmation.data.subscriptionId, confirmation.data.campaignId, 2, (err, found) => { + subscriptions.changeStatus(list.id, confirmation.data.subscriptionId, confirmation.data.campaignId, subscriptions.Status.UNSUBSCRIBED, (err, found) => { if (err) { return next(err); } @@ -146,7 +147,7 @@ router.get('/confirm/:cid', (req, res, next) => { return next(err); } - res.redirect('/subscription/' + req.params.lcid + '/unsubscribed-notice'); + res.redirect('/subscription/' + list.cid + '/unsubscribed-notice'); }); }); }); @@ -171,7 +172,7 @@ router.get('/:cid', passport.csrfProtection, (req, res, next) => { return next(err); } - // FIXME: process subscriber cid param for resubscription requests + // TODO: process subscriber cid param for resubscription requests let data = tools.convertKeys(req.query, { skip: ['layout'] @@ -181,51 +182,74 @@ router.get('/:cid', passport.csrfProtection, (req, res, next) => { data.cid = list.cid; data.csrfToken = req.csrfToken(); - fields.list(list.id, (err, fieldList) => { - if (err && !fieldList) { - fieldList = []; - } - data.customFields = fields.getRow(fieldList, data); - data.useEditor = true; - - settings.list(['pgpPrivateKey', 'defaultAddress', 'defaultPostaddress'], (err, configItems) => { - if (err) { - return next(err); + function nextStep() { + fields.list(list.id, (err, fieldList) => { + if (err && !fieldList) { + fieldList = []; } - data.hasPubkey = !!configItems.pgpPrivateKey; - data.defaultAddress = configItems.defaultAddress; - data.defaultPostaddress = configItems.defaultPostaddress; - data.template = { - template: 'subscription/web-subscribe.mjml.hbs', - layout: 'subscription/layout.mjml.hbs' - }; + data.customFields = fields.getRow(fieldList, data); + data.useEditor = true; - helpers.injectCustomFormData(req.query.fid || list.defaultForm, 'subscription/web-subscribe', data, (err, data) => { + settings.list(['pgpPrivateKey', 'defaultAddress', 'defaultPostaddress'], (err, configItems) => { if (err) { return next(err); } + data.hasPubkey = !!configItems.pgpPrivateKey; + data.defaultAddress = configItems.defaultAddress; + data.defaultPostaddress = configItems.defaultPostaddress; - helpers.getMjmlTemplate(data.template, (err, htmlRenderer) => { + data.template = { + template: 'subscription/web-subscribe.mjml.hbs', + layout: 'subscription/layout.mjml.hbs' + }; + + helpers.injectCustomFormData(req.query.fid || list.defaultForm, 'subscription/web-subscribe', data, (err, data) => { if (err) { return next(err); } - helpers.captureFlashMessages(req, res, (err, flash) => { + helpers.getMjmlTemplate(data.template, (err, htmlRenderer) => { if (err) { return next(err); } - data.isWeb = true; - data.needsJsWarning = true; - data.flashMessages = flash; - res.send(htmlRenderer(data)); + helpers.captureFlashMessages(req, res, (err, flash) => { + if (err) { + return next(err); + } + + data.isWeb = true; + data.needsJsWarning = true; + data.flashMessages = flash; + res.send(htmlRenderer(data)); + }); }); }); }); }); - }); + } + + + const ucid = req.query.cid; + if (ucid) { + subscriptions.get(list.id, ucid, (err, subscription) => { + if (err) { + return next(err); + } + + for (let key in subscription) { + if (!(key in data)) { + data[key] = subscription[key]; + } + } + + nextStep(); + }); + } else { + nextStep(); + } }); }); @@ -360,14 +384,14 @@ router.post('/:cid/subscribe', passport.parseForm, corsOrCsrfProtection, (req, r subscriptionData[key] = (req.body[key] || '').toString().trim(); } }); - subscriptionData = tools.convertKeys(data); + subscriptionData = tools.convertKeys(subscriptionData); subscriptions.getByEmail(list.id, email, (err, subscription) => { if (err) { return req.xhr ? sendJsonError(err) : next(err); } - if (subscription) { + if (subscription && subscription.status === subscriptions.Status.SUBSCRIBED) { mailHelpers.sendAlreadySubscribed(list, email, subscription, (err) => { if (err) { return req.xhr ? sendJsonError(err) : next(err); @@ -381,10 +405,6 @@ router.post('/:cid/subscribe', passport.parseForm, corsOrCsrfProtection, (req, r }; confirmations.addConfirmation(list.id, 'subscribe', req.ip, data, (err, confirmCid) => { - if (!err && !confirmCid) { - err = new Error(_('Could not store confirmation data')); - } - if (err) { if (req.xhr) { return sendJsonError(err); @@ -406,7 +426,7 @@ router.post('/:cid/subscribe', passport.parseForm, corsOrCsrfProtection, (req, r log.info('Subscription', 'Confirmation message for %s marked to be skipped (%s)', email, JSON.stringify(data)); sendWebResponse(); } else { - sendConfirmSubscription(list, email, confirmCid, data, (err) => { + mailHelpers.sendConfirmSubscription(list, email, confirmCid, data, (err) => { if (err) { return req.xhr ? sendJsonError(err) : sendWebResponse(err); } @@ -436,8 +456,8 @@ router.get('/:lcid/manage/:ucid', passport.csrfProtection, (req, res, next) => { return next(err); } subscriptions.get(list.id, req.params.ucid, (err, subscription) => { - if (!err && !subscription) { - err = new Error(_('Subscription not found from this list')); + if (!err && (!subscription || subscription.status !== subscriptions.Status.SUBSCRIBED)) { + err = new Error(_('Subscription not found in this list')); err.status = 404; } @@ -507,13 +527,22 @@ router.post('/:lcid/manage', passport.parseForm, passport.csrfProtection, (req, return next(err); } - 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)); + subscriptions.get(list.id, req.body.cid, (err, subscription) => { + if (!err && (!subscription || subscription.status !== subscriptions.Status.SUBSCRIBED)) { + err = new Error(_('Subscription not found in this list')); + err.status = 404; } - res.redirect('/subscription/' + req.params.lcid + '/updated-notice'); + + if (err) { + return next(err); + } + + subscriptions.update(list.id, subscription.cid, req.body, false, err => { + if (err) { + return next(err); + } + res.redirect('/subscription/' + req.params.lcid + '/updated-notice'); + }); }); }); }); @@ -535,8 +564,8 @@ router.get('/:lcid/manage-address/:ucid', passport.csrfProtection, (req, res, ne } subscriptions.get(list.id, req.params.ucid, (err, subscription) => { - if (!err && !subscription) { - err = new Error(_('Subscription not found from this list')); + if (!err && (!subscription || subscription.status !== subscriptions.Status.SUBSCRIBED)) { + err = new Error(_('Subscription not found in this list')); err.status = 404; } @@ -589,48 +618,52 @@ router.post('/:lcid/manage-address', passport.parseForm, passport.csrfProtection return next(err); } - const emailNew = (req.body.emailNew || '').toString().trim(); + let bodyData = tools.convertKeys(req.body); // This is here to convert "email-new" to "emailNew" + const emailOld = (bodyData.email || '').toString().trim(); + const emailNew = (bodyData.emailNew || '').toString().trim(); - subscriptions.updateAddressCheck(list, req.body.cid, emailNew, req.ip, (err, subscription, newEmailAvailable) => { - if (err) { - req.flash('danger', err.message || err); - log.error('Subscription', err); - return res.redirect('/subscription/' + encodeURIComponent(req.params.lcid) + '/manage-address/' + encodeURIComponent(req.body.cid) + '?' + tools.queryParams(req.body)); - } + if (emailOld === emailNew) { + req.flash('info', _('Nothing seems to be changed')); + res.redirect('/subscription/' + req.params.lcid + '/manage/' + req.body.cid); - function sendWebResponse(err) { + } else { + subscriptions.updateAddressCheck(list, req.body.cid, emailNew, req.ip, (err, subscription, newEmailAvailable) => { if (err) { return next(err); } - req.flash('info', _('An email with further instructions has been sent to the provided address')); - res.redirect('/subscription/' + req.params.lcid + '/manage/' + req.body.cid); - } - - if (newEmailAvailable) { - const data = { - subscriptionId: subscription.id, - emailNew - }; - - confirmations.addConfirmation(list.id, 'change-address', req.ip, data, err => { + function sendWebResponse(err) { if (err) { return next(err); } - mailHelpers.sendConfirmAddressChange(list, emailNew, subscription, sendWebResponse); - }); + req.flash('info', _('An email with further instructions has been sent to the provided address')); + res.redirect('/subscription/' + req.params.lcid + '/manage/' + req.body.cid); + } - } else { - mailHelpers.sendAlreadySubscribed(list, emailNew, subscription, sendWebResponse); - } - }); + if (newEmailAvailable) { + const data = { + subscriptionId: subscription.id, + emailNew + }; + + confirmations.addConfirmation(list.id, 'change-address', req.ip, data, (err, confirmCid) => { + if (err) { + return next(err); + } + + mailHelpers.sendConfirmAddressChange(list, emailNew, confirmCid, subscription, sendWebResponse); + }); + + } else { + mailHelpers.sendAlreadySubscribed(list, emailNew, subscription, sendWebResponse); + } + }); + } }); }); router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next) => { - // FIXME: handle different subscription options. The one below is currently "One-step with unsubscribe form" - lists.getByCid(req.params.lcid, (err, list) => { if (!err && !list) { err = new Error(_('Selected list not found')); @@ -647,7 +680,7 @@ router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next) } subscriptions.get(list.id, req.params.ucid, (err, subscription) => { - if (!err && !subscription) { + if (!err && (!subscription || subscription.status !== subscriptions.Status.SUBSCRIBED)) { err = new Error(_('Subscription not found in this list')); err.status = 404; } @@ -657,7 +690,8 @@ router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next) } - if (list.unsubscriptionMode === lists.UnsubscriptionMode.ONE_STEP_WITH_FORM || + if (req.query.formTest || + list.unsubscriptionMode === lists.UnsubscriptionMode.ONE_STEP_WITH_FORM || list.unsubscriptionMode === lists.UnsubscriptionMode.TWO_STEP_WITH_FORM) { subscription.lcid = req.params.lcid; @@ -695,7 +729,7 @@ router.get('/:lcid/unsubscribe/:ucid', passport.csrfProtection, (req, res, next) }); }); } else { // UnsubscriptionMode.ONE_STEP || UnsubscriptionMode.TWO_STEP || UnsubscriptionMode.MANUAL - handleUnsubscribe(list, subscription, res); + handleUnsubscribe(list, subscription, req.query.c, req.ip, res, next); } }); }); @@ -716,7 +750,7 @@ router.post('/:lcid/unsubscribe', passport.parseForm, passport.csrfProtection, ( const campaignId = (req.body.campaign || '').toString().trim() || false; subscriptions.get(list.id, req.body.ucid, (err, subscription) => { - if (!err && !subscription) { + if (!err && (!subscription || subscription.status !== subscriptions.Status.SUBSCRIBED)) { err = new Error(_('Subscription not found in this list')); err.status = 404; } @@ -725,12 +759,12 @@ router.post('/:lcid/unsubscribe', passport.parseForm, passport.csrfProtection, ( return next(err); } - handleUnsubscribe(list, subscription, res); + handleUnsubscribe(list, subscription, campaignId, req.ip, res, next); }); }); }); -function handleUnsubscribe(list, subscription, res) { +function handleUnsubscribe(list, subscription, campaignId, ip, res, next) { if (list.unsubscriptionMode === lists.UnsubscriptionMode.TWO_STEP || list.unsubscriptionMode === lists.UnsubscriptionMode.TWO_STEP_WITH_FORM) { @@ -739,28 +773,24 @@ function handleUnsubscribe(list, subscription, res) { campaignId }; - confirmations.addConfirmation(list.id, 'unsubscribe', req.ip, data, (err, confirmCid) => { - if (!err && !confirmCid) { - err = new Error(_('Could not store confirmation data')); - } - + confirmations.addConfirmation(list.id, 'unsubscribe', ip, data, (err, confirmCid) => { if (err) { return next(err); } - mailHelpers.sendConfirmUnsubscription(list, subscription.email, subscription, err => { + mailHelpers.sendConfirmUnsubscription(list, subscription.email, confirmCid, subscription, err => { if (err) { return next(err); } - res.redirect('/subscription/' + list.cid + '/unsubscribed-notice'); + res.redirect('/subscription/' + list.cid + '/confirm-unsubscription-notice'); }); }); } else if (list.unsubscriptionMode === lists.UnsubscriptionMode.ONE_STEP || list.unsubscriptionMode === lists.UnsubscriptionMode.ONE_STEP_WITH_FORM) { - subscriptions.changeStatus(subscription.id, list.id, campaignId, 2, (err, found) => { + subscriptions.changeStatus(list.id, subscription.id, campaignId, subscriptions.Status.UNSUBSCRIBED, (err, found) => { if (err) { return next(err); } diff --git a/views/lists/forms/edit.hbs b/views/lists/forms/edit.hbs index 5b91dcfb..da199096 100644 --- a/views/lists/forms/edit.hbs +++ b/views/lists/forms/edit.hbs @@ -55,7 +55,11 @@ {{#translate}}Updated Notice{{/translate}} | {{#translate}}Unsubscribed Notice{{/translate}} + | + {{#translate}}Manual Unsubscribe Notice{{/translate}} {{#if testUsers}} + | + {{#translate}}Unsubscribe{{/translate}} | {{#translate}}Manage{{/translate}} | diff --git a/views/subscription/partials/subscription-custom-fields.hbs b/views/subscription/partials/subscription-custom-fields.hbs index 847a169e..9e679b03 100644 --- a/views/subscription/partials/subscription-custom-fields.hbs +++ b/views/subscription/partials/subscription-custom-fields.hbs @@ -1,6 +1,6 @@ {{#each customFields}} - {{#if typeSubsciptionEmail}} + {{#if typeSubscriptionEmail}}
{{#if ../isManagePreferences}} diff --git a/views/subscription/partials/subscription-subscribe-form.hbs b/views/subscription/partials/subscription-subscribe-form.hbs index 92b40c71..46bd7708 100644 --- a/views/subscription/partials/subscription-subscribe-form.hbs +++ b/views/subscription/partials/subscription-subscribe-form.hbs @@ -10,6 +10,7 @@ + {{> subscription_custom_fields}}