From e91e9eadacd81e1b64f1ee635074aa5b76311228 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Tue, 28 Jun 2016 20:13:01 +0200 Subject: [PATCH] improvement: mail structure (#7033) no issue - in preparation for subscribers V2 - do not implement code in index.js - create mail utils --- core/server/api/authentication.js | 11 +- core/server/api/mail.js | 71 ++---------- core/server/api/users.js | 7 +- core/server/mail/GhostMailer.js | 105 +++++++++++++++++ core/server/mail/index.js | 107 +----------------- core/server/mail/utils.js | 40 +++++++ .../GhostMailer_spec.js} | 34 +++--- core/test/unit/mail/utils_spec.js | 45 ++++++++ 8 files changed, 231 insertions(+), 189 deletions(-) create mode 100644 core/server/mail/GhostMailer.js create mode 100644 core/server/mail/utils.js rename core/test/unit/{mail_spec.js => mail/GhostMailer_spec.js} (91%) create mode 100644 core/test/unit/mail/utils_spec.js diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index 99f91a2364..a32d2e806a 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -3,7 +3,8 @@ var _ = require('lodash'), pipeline = require('../utils/pipeline'), dataProvider = require('../models'), settings = require('./settings'), - mail = require('./mail'), + mail = require('./../mail'), + apiMail = require('./mail'), globalUtils = require('../utils'), utils = require('./utils'), errors = require('../errors'), @@ -171,7 +172,7 @@ authentication = { '/ghost/reset/' + globalUtils.encodeBase64URLsafe(data.resetToken) + '/'; - return mail.generateContent({ + return mail.utils.generateContent({ data: { resetUrl: resetUrl }, @@ -189,7 +190,7 @@ authentication = { }] }; - return mail.send(payload, {context: {internal: true}}); + return apiMail.send(payload, {context: {internal: true}}); }); } @@ -413,7 +414,7 @@ authentication = { ownerEmail: setupUser.email }; - return mail.generateContent({data: data, template: 'welcome'}) + return mail.utils.generateContent({data: data, template: 'welcome'}) .then(function then(content) { var message = { to: setupUser.email, @@ -428,7 +429,7 @@ authentication = { }] }; - mail.send(payload, {context: {internal: true}}).catch(function (error) { + apiMail.send(payload, {context: {internal: true}}).catch(function (error) { errors.logError( error.message, i18n.t( diff --git a/core/server/api/mail.js b/core/server/api/mail.js index b9f24890c7..fd3bc1a70b 100644 --- a/core/server/api/mail.js +++ b/core/server/api/mail.js @@ -1,35 +1,26 @@ // # Mail API // API for sending Mail -var _ = require('lodash').runInContext(), - Promise = require('bluebird'), + +var Promise = require('bluebird'), pipeline = require('../utils/pipeline'), - config = require('../config'), errors = require('../errors'), - GhostMail = require('../mail'), + mail = require('../mail'), Models = require('../models'), utils = require('./utils'), notifications = require('./notifications'), - path = require('path'), - fs = require('fs'), - templatesDir = path.resolve(__dirname, '..', 'mail', 'templates'), - htmlToText = require('html-to-text'), - readFile = Promise.promisify(fs.readFile), docName = 'mail', i18n = require('../i18n'), mode = process.env.NODE_ENV, testing = mode !== 'production' && mode !== 'development', mailer, - mail; - -_.templateSettings.interpolate = /{{([\s\S]+?)}}/g; + apiMail; /** * Send mail helper */ - function sendMail(object) { - if (!(mailer instanceof GhostMail) || testing) { - mailer = new GhostMail(); + if (!(mailer instanceof mail.GhostMailer) || testing) { + mailer = new mail.GhostMailer(); } return mailer.send(object.mail[0].message).catch(function (err) { @@ -58,7 +49,7 @@ function sendMail(object) { * @typedef Mail * @param mail */ -mail = { +apiMail = { /** * ### Send * Send an email @@ -95,9 +86,9 @@ mail = { } tasks = [ - utils.handlePermissions(docName, 'send'), - send, - formatResponse + utils.handlePermissions(docName, 'send'), + send, + formatResponse ]; return pipeline(tasks, options || {}); @@ -127,7 +118,7 @@ mail = { */ function generateContent(result) { - return mail.generateContent({template: 'test'}).then(function (content) { + return mail.utils.generateContent({template: 'test'}).then(function (content) { var payload = { mail: [{ message: { @@ -158,45 +149,7 @@ mail = { ]; return pipeline(tasks); - }, - - /** - * - * @param {Object} options { - * data: JSON object representing the data that will go into the email - * template: which email template to load (files are stored in /core/server/mail/templates/) - * } - * @returns {*} - */ - generateContent: function (options) { - var defaults, - data; - - defaults = { - siteUrl: config.forceAdminSSL ? (config.urlSSL || config.url) : config.url - }; - - data = _.defaults(defaults, options.data); - - // read the proper email body template - return readFile(path.join(templatesDir, options.template + '.html'), 'utf8').then(function (content) { - var compiled, - htmlContent, - textContent; - - // insert user-specific data into the email - compiled = _.template(content); - htmlContent = compiled(data); - - // generate a plain-text version of the same email - textContent = htmlToText.fromString(htmlContent); - - return { - html: htmlContent, - text: textContent - }; - }); } }; -module.exports = mail; +module.exports = apiMail; diff --git a/core/server/api/users.js b/core/server/api/users.js index 69990ef9af..e6be9861ca 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -9,7 +9,8 @@ var Promise = require('bluebird'), utils = require('./utils'), globalUtils = require('../utils'), config = require('../config'), - mail = require('./mail'), + mail = require('./../mail'), + apiMail = require('./mail'), pipeline = require('../utils/pipeline'), i18n = require('../i18n'), @@ -44,7 +45,7 @@ sendInviteEmail = function sendInviteEmail(user) { emailData.resetLink = baseUrl.replace(/\/$/, '') + '/ghost/signup/' + globalUtils.encodeBase64URLsafe(resetToken) + '/'; - return mail.generateContent({data: emailData, template: 'invite-user'}); + return mail.utils.generateContent({data: emailData, template: 'invite-user'}); }).then(function (emailContent) { var payload = { mail: [{ @@ -58,7 +59,7 @@ sendInviteEmail = function sendInviteEmail(user) { }] }; - return mail.send(payload, {context: {internal: true}}); + return apiMail.send(payload, {context: {internal: true}}); }); }; /** diff --git a/core/server/mail/GhostMailer.js b/core/server/mail/GhostMailer.js new file mode 100644 index 0000000000..4d9efb4452 --- /dev/null +++ b/core/server/mail/GhostMailer.js @@ -0,0 +1,105 @@ +// # Mail +// Handles sending email for Ghost +var _ = require('lodash'), + Promise = require('bluebird'), + nodemailer = require('nodemailer'), + validator = require('validator'), + config = require('../config'), + i18n = require('../i18n'); + +function GhostMailer() { + var transport = config.mail && config.mail.transport || 'direct', + options = config.mail && _.clone(config.mail.options) || {}; + + this.state = {}; + + this.transport = nodemailer.createTransport(transport, options); + + this.state.usingDirect = transport === 'direct'; +} + +GhostMailer.prototype.from = function () { + var from = config.mail && (config.mail.from || config.mail.fromaddress); + + // If we don't have a from address at all + if (!from) { + // Default to ghost@[blog.url] + from = 'ghost@' + this.getDomain(); + } + + // If we do have a from address, and it's just an email + if (validator.isEmail(from)) { + if (!config.theme.title) { + config.theme.title = i18n.t('common.mail.title', {domain: this.getDomain()}); + } + from = '"' + config.theme.title + '" <' + from + '>'; + } + + return from; +}; + +// Moved it to its own module +GhostMailer.prototype.getDomain = function () { + var domain = config.url.match(new RegExp('^https?://([^/:?#]+)(?:[/:?#]|$)', 'i')); + return domain && domain[1]; +}; + +// Sends an email message enforcing `to` (blog owner) and `from` fields +// This assumes that api.settings.read('email') was already done on the API level +GhostMailer.prototype.send = function (message) { + var self = this, + to; + + // important to clone message as we modify it + message = _.clone(message) || {}; + to = message.to || false; + + if (!(message && message.subject && message.html && message.to)) { + return Promise.reject(new Error(i18n.t('errors.mail.incompleteMessageData.error'))); + } + + message = _.extend(message, { + from: self.from(), + to: to, + generateTextFromHTML: true, + encoding: 'base64' + }); + + return new Promise(function (resolve, reject) { + self.transport.sendMail(message, function (error, response) { + if (error) { + return reject(new Error(error)); + } + + if (self.transport.transportType !== 'DIRECT') { + return resolve(response); + } + + response.statusHandler.once('failed', function (data) { + var reason = i18n.t('errors.mail.failedSendingEmail.error'); + + if (data.error && data.error.errno === 'ENOTFOUND') { + reason += i18n.t('errors.mail.noMailServerAtAddress.error', {domain: data.domain}); + } + reason += '.'; + return reject(new Error(reason)); + }); + + response.statusHandler.once('requeue', function (data) { + var errorMessage = i18n.t('errors.mail.messageNotSent.error'); + + if (data.error && data.error.message) { + errorMessage += i18n.t('errors.general.moreInfo', {info: data.error.message}); + } + + return reject(new Error(errorMessage)); + }); + + response.statusHandler.once('sent', function () { + return resolve(i18n.t('notices.mail.messageSent')); + }); + }); + }); +}; + +module.exports = GhostMailer; diff --git a/core/server/mail/index.js b/core/server/mail/index.js index 4d9efb4452..e365a0a573 100644 --- a/core/server/mail/index.js +++ b/core/server/mail/index.js @@ -1,105 +1,2 @@ -// # Mail -// Handles sending email for Ghost -var _ = require('lodash'), - Promise = require('bluebird'), - nodemailer = require('nodemailer'), - validator = require('validator'), - config = require('../config'), - i18n = require('../i18n'); - -function GhostMailer() { - var transport = config.mail && config.mail.transport || 'direct', - options = config.mail && _.clone(config.mail.options) || {}; - - this.state = {}; - - this.transport = nodemailer.createTransport(transport, options); - - this.state.usingDirect = transport === 'direct'; -} - -GhostMailer.prototype.from = function () { - var from = config.mail && (config.mail.from || config.mail.fromaddress); - - // If we don't have a from address at all - if (!from) { - // Default to ghost@[blog.url] - from = 'ghost@' + this.getDomain(); - } - - // If we do have a from address, and it's just an email - if (validator.isEmail(from)) { - if (!config.theme.title) { - config.theme.title = i18n.t('common.mail.title', {domain: this.getDomain()}); - } - from = '"' + config.theme.title + '" <' + from + '>'; - } - - return from; -}; - -// Moved it to its own module -GhostMailer.prototype.getDomain = function () { - var domain = config.url.match(new RegExp('^https?://([^/:?#]+)(?:[/:?#]|$)', 'i')); - return domain && domain[1]; -}; - -// Sends an email message enforcing `to` (blog owner) and `from` fields -// This assumes that api.settings.read('email') was already done on the API level -GhostMailer.prototype.send = function (message) { - var self = this, - to; - - // important to clone message as we modify it - message = _.clone(message) || {}; - to = message.to || false; - - if (!(message && message.subject && message.html && message.to)) { - return Promise.reject(new Error(i18n.t('errors.mail.incompleteMessageData.error'))); - } - - message = _.extend(message, { - from: self.from(), - to: to, - generateTextFromHTML: true, - encoding: 'base64' - }); - - return new Promise(function (resolve, reject) { - self.transport.sendMail(message, function (error, response) { - if (error) { - return reject(new Error(error)); - } - - if (self.transport.transportType !== 'DIRECT') { - return resolve(response); - } - - response.statusHandler.once('failed', function (data) { - var reason = i18n.t('errors.mail.failedSendingEmail.error'); - - if (data.error && data.error.errno === 'ENOTFOUND') { - reason += i18n.t('errors.mail.noMailServerAtAddress.error', {domain: data.domain}); - } - reason += '.'; - return reject(new Error(reason)); - }); - - response.statusHandler.once('requeue', function (data) { - var errorMessage = i18n.t('errors.mail.messageNotSent.error'); - - if (data.error && data.error.message) { - errorMessage += i18n.t('errors.general.moreInfo', {info: data.error.message}); - } - - return reject(new Error(errorMessage)); - }); - - response.statusHandler.once('sent', function () { - return resolve(i18n.t('notices.mail.messageSent')); - }); - }); - }); -}; - -module.exports = GhostMailer; +exports.GhostMailer = require('./GhostMailer'); +exports.utils = require('./utils'); diff --git a/core/server/mail/utils.js b/core/server/mail/utils.js new file mode 100644 index 0000000000..e84d4d5a4d --- /dev/null +++ b/core/server/mail/utils.js @@ -0,0 +1,40 @@ +var _ = require('lodash').runInContext(), + fs = require('fs'), + Promise = require('bluebird'), + path = require('path'), + htmlToText = require('html-to-text'), + config = require('../config'), + templatesDir = path.resolve(__dirname, '..', 'mail', 'templates'); + +_.templateSettings.interpolate = /{{([\s\S]+?)}}/g; + +exports.generateContent = function generateContent(options) { + var defaults, + data; + + defaults = { + siteUrl: config.forceAdminSSL ? (config.urlSSL || config.url) : config.url + }; + + data = _.defaults(defaults, options.data); + + // read the proper email body template + return Promise.promisify(fs.readFile)(path.join(templatesDir, options.template + '.html'), 'utf8') + .then(function (content) { + var compiled, + htmlContent, + textContent; + + // insert user-specific data into the email + compiled = _.template(content); + htmlContent = compiled(data); + + // generate a plain-text version of the same email + textContent = htmlToText.fromString(htmlContent); + + return { + html: htmlContent, + text: textContent + }; + }); +}; diff --git a/core/test/unit/mail_spec.js b/core/test/unit/mail/GhostMailer_spec.js similarity index 91% rename from core/test/unit/mail_spec.js rename to core/test/unit/mail/GhostMailer_spec.js index d456fc88ec..15ae8a91f0 100644 --- a/core/test/unit/mail_spec.js +++ b/core/test/unit/mail/GhostMailer_spec.js @@ -2,9 +2,9 @@ var should = require('should'), Promise = require('bluebird'), // Stuff we are testing - GhostMail = require('../../server/mail'), - configUtils = require('../utils/configUtils'), - i18n = require('../../server/i18n'), + mail = require('../../../server/mail'), + configUtils = require('../../utils/configUtils'), + i18n = require('../../../server/i18n'), mailer, // Mock SMTP config @@ -37,7 +37,7 @@ var should = require('should'), i18n.init(); -describe('Mail', function () { +describe('Mail: Ghostmailer', function () { afterEach(function () { mailer = null; @@ -45,7 +45,7 @@ describe('Mail', function () { }); it('should attach mail provider to ghost instance', function () { - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); should.exist(mailer); mailer.should.have.property('send').and.be.a.Function(); @@ -53,7 +53,7 @@ describe('Mail', function () { it('should setup SMTP transport on initialization', function () { configUtils.set({mail: SMTP}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.should.have.property('transport'); mailer.transport.transportType.should.eql('SMTP'); @@ -63,7 +63,7 @@ describe('Mail', function () { it('should fallback to direct if config is empty', function () { configUtils.set({mail: {}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.should.have.property('transport'); mailer.transport.transportType.should.eql('DIRECT'); @@ -72,7 +72,7 @@ describe('Mail', function () { it('sends valid message successfully ', function (done) { configUtils.set({mail: {transport: 'stub'}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.transport.transportType.should.eql('STUB'); @@ -88,7 +88,7 @@ describe('Mail', function () { it('handles failure', function (done) { configUtils.set({mail: {transport: 'stub', options: {error: 'Stub made a boo boo :('}}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.transport.transportType.should.eql('STUB'); @@ -101,7 +101,7 @@ describe('Mail', function () { }); it('should fail to send messages when given insufficient data', function (done) { - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); Promise.all([ mailer.send().reflect(), @@ -122,7 +122,7 @@ describe('Mail', function () { beforeEach(function () { configUtils.set({mail: {}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); }); afterEach(function () { @@ -171,7 +171,7 @@ describe('Mail', function () { } }); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.from().should.equal('"Blog Title" '); }); @@ -180,7 +180,7 @@ describe('Mail', function () { // Standard domain configUtils.set({url: 'http://default.com', mail: {from: null}, theme: {title: 'Test'}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.from().should.equal('"Test" '); @@ -197,7 +197,7 @@ describe('Mail', function () { // Standard domain configUtils.set({mail: {from: '"bar" ', fromaddress: '"Qux" '}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.from().should.equal('"bar" '); }); @@ -206,7 +206,7 @@ describe('Mail', function () { // from and fromaddress are both set configUtils.set({mail: {from: 'from@default.com', fromaddress: 'fa@default.com'}, theme: {title: 'Test'}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.from().should.equal('"Test" '); @@ -223,7 +223,7 @@ describe('Mail', function () { // from and fromaddress are both set configUtils.set({mail: {from: '"R2D2" ', fromaddress: '"C3PO" '}, theme: {title: 'Test'}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.from().should.equal('"R2D2" '); @@ -239,7 +239,7 @@ describe('Mail', function () { it('should use default title if not theme title is provided', function () { configUtils.set({url: 'http://default.com:2368/', mail: {from: null}, theme: {title: null}}); - mailer = new GhostMail(); + mailer = new mail.GhostMailer(); mailer.from().should.equal('"Ghost at default.com" '); }); diff --git a/core/test/unit/mail/utils_spec.js b/core/test/unit/mail/utils_spec.js new file mode 100644 index 0000000000..f433a2a6d8 --- /dev/null +++ b/core/test/unit/mail/utils_spec.js @@ -0,0 +1,45 @@ +var sinon = require('sinon'), + mail = require(__dirname + '../../../../server/mail'), + sandbox = sinon.sandbox.create(); + +describe('Mail: Utils', function () { + var scope = {ghostMailer: null}; + + before(function () { + scope.ghostMailer = new mail.GhostMailer(); + + sandbox.stub(scope.ghostMailer.transport, 'sendMail', function (message, sendMailDone) { + sendMailDone(null, { + statusHandler: { + once: function (eventName, eventDone) { + if (eventName === 'sent') { + eventDone(); + } + } + } + }); + }); + }); + + after(function () { + sandbox.restore(); + }); + + it('generate welcome', function (done) { + mail.utils.generateContent({ + template: 'welcome', + data: { + ownerEmail: 'kate@ghost.org' + } + }).then(function (result) { + return scope.ghostMailer.send({ + to: 'kate@ghost.org', + subject: 'lol', + html: result.html, + text: result.text + }); + }).then(function () { + done(); + }).catch(done); + }); +});