TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-01 23:37:43 +03:00
Code Issues Projects Releases Wiki Activity

Deprecated mail.fromaddress, mail.from is Title <email@address>

Browse Source 
Closes #4018

* cleaned up `mail_spec.js`
* deprecated `mail.fromaddress`
* implemented 'Blog title <email@address.com>' format with fallbacks
* added tests to deprecation and from address, made existing ones more robust
* moved domain intuit into its own module: `GhostMailer.getDomain()`
This commit is contained in:
Gabor Javorszky 2014-09-20 22:11:30 +01:00
parent b3820fbb31
commit 1f5a378b4c
6 changed files with 218 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/server/config/index.js
View File

@ -319,7 +319,7 @@ ConfigManager.prototype.isPrivacyDisabled = function (privacyFlag) {
* Check if any of the currently set config items are deprecated, and issues a warning.
*/
ConfigManager.prototype.checkDeprecated = function () {
var deprecatedItems = ['updateCheck'],
var deprecatedItems = ['updateCheck', 'mail.fromaddress'],
self = this;
_.each(deprecatedItems, function (property) {

1
core/server/errors/index.js
View File

@ -73,6 +73,7 @@ errors = {
if ((process.env.NODE_ENV === 'development' ||
process.env.NODE_ENV === 'staging' ||
process.env.NODE_ENV === 'production')) {
warn = warn || 'no message supplied';
var msgs = ['\nWarning:'.yellow, warn.yellow, '\n'];
if (context) {

29
core/server/mail.js
View File

@ -1,6 +1,7 @@
var _ = require('lodash'),
Promise = require('bluebird'),
nodemailer = require('nodemailer'),
validator = require('validator'),
config = require('./config');
function GhostMailer(opts) {
@ -28,22 +29,32 @@ GhostMailer.prototype.createTransport = function () {
this.transport = nodemailer.createTransport(config.mail.transport, _.clone(config.mail.options) || {});
};
GhostMailer.prototype.fromAddress = function () {
var from = config.mail && config.mail.fromaddress,
domain;
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) {
// Extract the domain name from url set in config.js
domain = config.url.match(new RegExp('^https?://([^/:?#]+)(?:[/:?#]|$)', 'i'));
domain = domain && domain[1];
// Default to ghost@[blog.url]
from = 'ghost@' + domain;
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 = 'Ghost at ' + 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 e-mail message enforcing `to` (blog owner) and `from` fields
// This assumes that api.settings.read('email') was aready done on the API level
GhostMailer.prototype.send = function (message) {
@ -63,7 +74,7 @@ GhostMailer.prototype.send = function (message) {
sendMail = Promise.promisify(self.transport.sendMail.bind(self.transport));
message = _.extend(message, {
from: self.fromAddress(),
from: self.from(),
to: to,
generateTextFromHTML: true
});

85
core/test/unit/config_spec.js
View File

@ -12,7 +12,7 @@ var should = require('should'),
// Thing we are testing
defaultConfig = require('../../../config.example')[process.env.NODE_ENV],
config = rewire('../../server/config'),
config = require('../../server/config'),
// storing current environment
currentEnv = process.env.NODE_ENV;
@ -680,7 +680,7 @@ describe('Config', function () {
});
});
describe('Deprecated', function () {
describe('Check for deprecation messages:', function () {
var logStub,
// Can't use afterEach here, because mocha uses console.log to output the checkboxes
// which we've just stubbed, so we need to restore it before the test ends to see ticks.
@ -695,7 +695,7 @@ describe('Config', function () {
});
afterEach(function () {
// logStub.restore();
logStub.restore();
config = rewire('../../server/config');
});
@ -715,14 +715,11 @@ describe('Config', function () {
config.checkDeprecated();
logStub.calledOnce.should.be.true;
logStub.calledWith(
'\nWarning:'.yellow,
('The configuration property [' + 'updateCheck'.bold + '] has been deprecated.').yellow,
'\n',
'This will be removed in a future version, please update your config.js file.'.white,
'\n',
'Please check http://support.ghost.org/config for the most up-to-date example.'.green,
'\n').should.be.true;
logStub.calledWithMatch(null, 'updateCheck').should.be.false;
logStub.calledWithMatch('', 'updateCheck').should.be.true;
logStub.calledWithMatch(sinon.match.string, 'updateCheck').should.be.true;
logStub.calledWithMatch(sinon.match.number, 'updateCheck').should.be.false;
// Future tests: This is important here!
resetEnvironment();
@ -736,14 +733,64 @@ describe('Config', function () {
config.checkDeprecated();
logStub.calledOnce.should.be.true;
logStub.calledWith(
'\nWarning:'.yellow,
('The configuration property [' + 'updateCheck'.bold + '] has been deprecated.').yellow,
'\n',
'This will be removed in a future version, please update your config.js file.'.white,
'\n',
'Please check http://support.ghost.org/config for the most up-to-date example.'.green,
'\n').should.be.true;
logStub.calledWithMatch(null, 'updateCheck').should.be.false;
logStub.calledWithMatch('', 'updateCheck').should.be.true;
logStub.calledWithMatch(sinon.match.string, 'updateCheck').should.be.true;
logStub.calledWithMatch(sinon.match.number, 'updateCheck').should.be.false;
// Future tests: This is important here!
resetEnvironment();
});
it('displays warning when mail.fromaddress exists and is truthy', function () {
config.set({
mail: {
fromaddress: 'foo'
}
});
// Run the test code
config.checkDeprecated();
logStub.calledOnce.should.be.true;
logStub.calledWithMatch(null, 'mail.fromaddress').should.be.false;
logStub.calledWithMatch('', 'mail.fromaddress').should.be.true;
logStub.calledWithMatch(sinon.match.string, 'mail.fromaddress').should.be.true;
logStub.calledWithMatch(sinon.match.number, 'mail.fromaddress').should.be.false;
// Future tests: This is important here!
resetEnvironment();
});
it('displays warning when mail.fromaddress exists and is falsy', function () {
config.set({
mail: {
fromaddress: undefined
}
});
// Run the test code
config.checkDeprecated();
logStub.calledOnce.should.be.true;
logStub.calledWithMatch(null, 'mail.fromaddress').should.be.false;
logStub.calledWithMatch('', 'mail.fromaddress').should.be.true;
logStub.calledWithMatch(sinon.match.string, 'mail.fromaddress').should.be.true;
logStub.calledWithMatch(sinon.match.number, 'mail.fromaddress').should.be.false;
// Future tests: This is important here!
resetEnvironment();
});
it('doesn\'t display warning when only part of a deprecated option is set', function () {
config.set({
mail: {
notfromaddress: 'foo'
}
});
config.checkDeprecated();
logStub.calledOnce.should.be.false;
// Future tests: This is important here!
resetEnvironment();

75
core/test/unit/error_handling_spec.js
View File

@ -50,7 +50,80 @@ describe('Error handling', function () {
});
});
describe('Logging', function () {
describe('Warn Logging', function () {
var logStub,
// Can't use afterEach here, because mocha uses console.log to output the checkboxes
// which we've just stubbed, so we need to restore it before the test ends to see ticks.
resetEnvironment = function () {
logStub.restore();
process.env.NODE_ENV = currentEnv;
};
beforeEach(function () {
logStub = sinon.stub(console, 'log');
process.env.NODE_ENV = 'development';
});
afterEach(function () {
logStub.restore();
});
it('logs default warn with no message supplied', function () {
errors.logWarn();
logStub.calledOnce.should.be.true;
logStub.calledWith(
'\nWarning: no message supplied'.yellow, '\n');
// Future tests: This is important here!
resetEnvironment();
});
it('logs warn with only message', function () {
var errorText = 'Error1';
errors.logWarn(errorText);
logStub.calledOnce.should.be.true;
logStub.calledWith(('\nWarning: ' + errorText).yellow, '\n');
// Future tests: This is important here!
resetEnvironment();
});
it('logs warn with message and context', function () {
var errorText = 'Error1',
contextText = 'Context1';
errors.logWarn(errorText, contextText);
logStub.calledOnce.should.be.true;
logStub.calledWith(
('\nWarning: ' + errorText).yellow, '\n', contextText.white, '\n'
);
// Future tests: This is important here!
resetEnvironment();
});
it('logs warn with message and context and help', function () {
var errorText = 'Error1',
contextText = 'Context1',
helpText = 'Help1';
errors.logWarn(errorText, contextText, helpText);
logStub.calledOnce.should.be.true;
logStub.calledWith(
('\nWarning: ' + errorText).yellow, '\n', contextText.white, '\n', helpText.green, '\n'
);
// Future tests: This is important here!
resetEnvironment();
});
});
describe('Error Logging', function () {
var logStub;
beforeEach(function () {

97
core/test/unit/mail_spec.js
View File

@ -1,18 +1,13 @@
/*globals describe, beforeEach, afterEach, it*/
/*globals describe, afterEach, it*/
/*jshint expr:true*/
var should = require('should'),
sinon = require('sinon'),
Promise = require('bluebird'),
_ = require('lodash'),
rewire = require('rewire'),
// Stuff we are testing
mailer = rewire('../../server/mail'),
defaultConfig = require('../../../config'),
SMTP,
fakeConfig,
fakeSettings,
sandbox = sinon.sandbox.create();
mailer = require('../../server/mail'),
config = require('../../server/config'),
SMTP;
// Mock SMTP config
SMTP = {
@ -27,26 +22,8 @@ SMTP = {
};
describe('Mail', function () {
var overrideConfig = function (newConfig) {
var config = rewire('../../server/config'),
existingConfig = mailer.__get__('config');
config.set(_.extend(existingConfig, newConfig));
};
beforeEach(function () {
// Mock config and settings
fakeConfig = _.extend({}, defaultConfig);
fakeSettings = {
url: 'http://test.tryghost.org',
email: 'ghost-test@localhost'
};
overrideConfig(fakeConfig);
});
afterEach(function () {
sandbox.restore();
config.set({mail: null});
});
it('should attach mail provider to ghost instance', function () {
@ -57,7 +34,7 @@ describe('Mail', function () {
});
it('should setup SMTP transport on initialization', function (done) {
overrideConfig({mail: SMTP});
config.set({mail: SMTP});
mailer.init().then(function () {
mailer.should.have.property('transport');
mailer.transport.transportType.should.eql('SMTP');
@ -67,11 +44,10 @@ describe('Mail', function () {
});
it('should fallback to direct if config is empty', function (done) {
overrideConfig({mail: {}});
config.set({mail: {}});
mailer.init().then(function () {
mailer.should.have.property('transport');
mailer.transport.transportType.should.eql('DIRECT');
done();
}).catch(done);
});
@ -86,27 +62,66 @@ describe('Mail', function () {
descriptors.forEach(function (d) {
d.isRejected().should.be.true;
d.reason().should.be.an.instanceOf(Error);
d.reason().message.should.eql('Email Error: Incomplete message data.');
});
done();
}).catch(done);
});
it('should use from address as configured in config.js', function () {
overrideConfig({mail: {fromaddress: 'static@example.com'}});
mailer.fromAddress().should.equal('static@example.com');
config.set({
mail: {
from: 'Blog Title <static@example.com>'
}
});
mailer.from().should.equal('Blog Title <static@example.com>');
});
it('should fall back to ghost@[blog.url] as from address', function () {
it('should fall back to [blog.title] <ghost@[blog.url]> as from address', function () {
// Standard domain
overrideConfig({url: 'http://default.com', mail: {fromaddress: null}});
mailer.fromAddress().should.equal('ghost@default.com');
config.set({url: 'http://default.com', mail: {from: null}, theme: {title: 'Test'}});
mailer.from().should.equal('Test <ghost@default.com>');
// Trailing slash
overrideConfig({url: 'http://default.com/', mail: {}});
mailer.fromAddress().should.equal('ghost@default.com');
config.set({url: 'http://default.com/', mail: {from: null}, theme: {title: 'Test'}});
mailer.from().should.equal('Test <ghost@default.com>');
// Strip Port
overrideConfig({url: 'http://default.com:2368/', mail: {}});
mailer.fromAddress().should.equal('ghost@default.com');
config.set({url: 'http://default.com:2368/', mail: {from: null}, theme: {title: 'Test'}});
mailer.from().should.equal('Test <ghost@default.com>');
});
it('should use mail.from if both from and fromaddress are present', function () {
// Standard domain
config.set({mail: {from: 'bar <from@default.com>', fromaddress: 'Qux <fa@default.com>'}});
mailer.from().should.equal('bar <from@default.com>');
});
it('should attach blog title if from or fromaddress are only email addresses', function () {
// from and fromaddress are both set
config.set({mail: {from: 'from@default.com', fromaddress: 'fa@default.com'}, theme: {title: 'Test'}});
mailer.from().should.equal('Test <from@default.com>');
// only from set
config.set({mail: {from: 'from@default.com', fromaddress: null}, theme: {title: 'Test'}});
mailer.from().should.equal('Test <from@default.com>');
// only fromaddress set
config.set({mail: {from: null, fromaddress: 'fa@default.com'}, theme: {title: 'Test'}});
mailer.from().should.equal('Test <fa@default.com>');
});
it('should ignore theme title if from address is Title <email@address.com> format', function () {
// from and fromaddress are both set
config.set({mail: {from: 'R2D2 <from@default.com>', fromaddress: 'C3PO <fa@default.com>'}, theme: {title: 'Test'}});
mailer.from().should.equal('R2D2 <from@default.com>');
// only from set
config.set({mail: {from: 'R2D2 <from@default.com>', fromaddress: null}, theme: {title: 'Test'}});
mailer.from().should.equal('R2D2 <from@default.com>');
// only fromaddress set
config.set({mail: {from: null, fromaddress: 'C3PO <fa@default.com>'}, theme: {title: 'Test'}});
mailer.from().should.equal('C3PO <fa@default.com>');
});
});