Skip to content

Commit 58fe7af

Browse files
damsonndamsonn
committed
Cleanup and improved test coverage for mailer
closes #5489 - Remove unneeded Promise.promisify in mailer - Remove noEmailTransportConfigured error as not relevant anymore (Direct is default) - Clone message argument in mailer.send - Move test from api_mail_spec to mail_spec - Add default mail title test
1 parent 57ae36a commit 58fe7af

File tree

4 files changed

+183
-186
lines changed

4 files changed

+183
-186
lines changed

‎core/server/mail/index.js‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,15 @@ GhostMailer.prototype.getDomain = function () {
6262
// This assumes that api.settings.read('email') was already done on the API level
6363
GhostMailer.prototype.send = function (message) {
6464
var self = this,
65-
to,
66-
sendMail;
65+
to;
6766

68-
message = message || {};
67+
// important to clone message as we modify it
68+
message = _.clone(message) || {};
6969
to = message.to || false;
7070

71-
if (!this.transport) {
72-
return Promise.reject(new Error(i18n.t('errors.mail.noEmailTransportConfigured.error')));
73-
}
7471
if (!(message && message.subject && message.html && message.to)) {
7572
return Promise.reject(new Error(i18n.t('errors.mail.incompleteMessageData.error')));
7673
}
77-
sendMail = Promise.promisify(self.transport.sendMail.bind(self.transport));
7874

7975
message = _.extend(message, {
8076
from: self.from(),
@@ -84,7 +80,7 @@ GhostMailer.prototype.send = function (message) {
8480
});
8581

8682
return new Promise(function (resolve, reject) {
87-
sendMail(message, function (error, response) {
83+
self.transport.sendMail(message, function (error, response) {
8884
if (error) {
8985
return reject(new Error(error));
9086
}

‎core/server/translations/en.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,6 @@
191191
}
192192
},
193193
"mail": {
194-
"noEmailTransportConfigured": {
195-
"error": "Error: No email transport configured."
196-
},
197194
"incompleteMessageData": {
198195
"error": "Error: Incomplete message data."
199196
},

‎core/test/integration/api/api_mail_spec.js‎

Lines changed: 29 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,13 @@ var testUtils = require('../../utils'),
44
should = require('should'),
55
config = require('../../../server/config'),
66
mailer = require('../../../server/mail'),
7-
i18n = require('../../../../core/server/i18n'),
7+
i18n = require('../../../../core/server/i18n'),
88

99
// Stuff we are testing
1010
MailAPI = require('../../../server/api/mail'),
11-
mailDataNoDomain = {
12-
mail: [{
13-
message: {
14-
to: 'joe@doesntexistexample091283zalgo.com',
15-
subject: 'testemail',
16-
html: '<p>This</p>'
17-
},
18-
options: {}
19-
}]
20-
},
21-
mailDataNoServer = {
11+
12+
// test data
13+
mailData = {
2214
mail: [{
2315
message: {
2416
to: 'joe@example.com',
@@ -27,15 +19,6 @@ var testUtils = require('../../utils'),
2719
},
2820
options: {}
2921
}]
30-
},
31-
mailDataIncomplete = {
32-
mail: [{
33-
message: {
34-
subject: 'testemail',
35-
html: '<p>This</p>'
36-
},
37-
options: {}
38-
}]
3922
};
4023
i18n.init();
4124

@@ -46,106 +29,34 @@ describe('Mail API', function () {
4629

4730
should.exist(MailAPI);
4831

49-
describe('Nothing configured', function () {
50-
it('return no email configured', function (done) {
51-
MailAPI.send(mailDataNoServer, testUtils.context.internal).then(function (response) {
52-
/*jshint unused:false */
53-
done();
54-
}).catch(function (error) {
55-
error.message.should.eql('Error: No email transport configured.');
56-
error.errorType.should.eql('EmailError');
57-
done();
58-
}).catch(done);
59-
});
60-
61-
it('return no email configured even when sending incomplete data', function (done) {
62-
MailAPI.send(mailDataIncomplete, testUtils.context.internal).then(function (response) {
63-
/*jshint unused:false */
64-
done();
65-
}).catch(function (error) {
66-
error.message.should.eql('Error: No email transport configured.');
67-
error.errorType.should.eql('EmailError');
68-
done();
69-
}).catch(done);
70-
});
71-
});
72-
73-
describe('Mail API Direct', function () {
74-
before(function (done) {
75-
config.set({mail: {}});
32+
it('returns a success', function (done) {
33+
config.set({mail: {transport: 'stub'}});
7634

77-
mailer.init().then(function () {
78-
done();
79-
});
80-
});
35+
mailer.init().then(function () {
36+
mailer.transport.transportType.should.eql('STUB');
37+
return MailAPI.send(mailData, testUtils.context.internal);
38+
}).then(function (response) {
39+
should.exist(response.mail);
40+
should.exist(response.mail[0].message);
41+
should.exist(response.mail[0].status);
8142

82-
it('return correct failure message for domain doesn\'t exist', function (done) {
83-
mailer.transport.transportType.should.eql('DIRECT');
84-
return MailAPI.send(mailDataNoDomain, testUtils.context.internal).then(function () {
85-
done(new Error('Error message not shown.'));
86-
}, function (error) {
87-
error.message.should.startWith('Error: Failed to send email');
88-
error.errorType.should.eql('EmailError');
89-
done();
90-
}).catch(done);
91-
});
92-
93-
// This test doesn't work properly - it times out locally
94-
it.skip('return correct failure message for no mail server at this address', function (done) {
95-
mailer.transport.transportType.should.eql('DIRECT');
96-
MailAPI.send(mailDataNoServer, testUtils.context.internal).then(function () {
97-
done(new Error('Error message not shown.'));
98-
}, function (error) {
99-
error.message.should.eql('Error: Failed to send email.');
100-
error.errorType.should.eql('EmailError');
101-
done();
102-
}).catch(done);
103-
});
104-
105-
it('return correct failure message for incomplete data', function (done) {
106-
mailer.transport.transportType.should.eql('DIRECT');
107-
108-
MailAPI.send(mailDataIncomplete, testUtils.context.internal).then(function () {
109-
done(new Error('Error message not shown.'));
110-
}, function (error) {
111-
error.message.should.eql('Error: Incomplete message data.');
112-
error.errorType.should.eql('EmailError');
113-
done();
114-
}).catch(done);
115-
});
43+
response.mail[0].message.subject.should.eql('testemail');
44+
done();
45+
}).catch(done);
11646
});
11747

118-
describe.skip('Stub', function () {
119-
it('returns a success', function (done) {
120-
config.set({mail: {transport: 'stub'}});
121-
122-
mailer.init().then(function () {
123-
mailer.transport.transportType.should.eql('STUB');
124-
return MailAPI.send(mailDataNoServer, testUtils.context.internal);
125-
}).then(function (response) {
126-
should.exist(response.mail);
127-
should.exist(response.mail[0].message);
128-
should.exist(response.mail[0].status);
129-
response.mail[0].status.should.eql({message: 'Message Queued'});
130-
response.mail[0].message.subject.should.eql('testemail');
131-
done();
132-
}).catch(done);
133-
});
134-
135-
it('returns a boo boo', function (done) {
136-
config.set({mail: {transport: 'stub', error: 'Stub made a boo boo :('}});
137-
138-
mailer.init().then(function () {
139-
mailer.transport.transportType.should.eql('STUB');
140-
return MailAPI.send(mailDataNoServer, testUtils.context.internal);
141-
}).then(function (response) {
142-
console.log('res', response.mail[0]);
143-
done(new Error('Stub did not error'));
144-
}, function (error) {
145-
error.message.should.startWith('Error: Failed to send email - no mail server found at');
146-
error.errorType.should.eql('EmailError');
147-
done();
148-
}).catch(done);
149-
});
48+
it('returns a boo boo', function (done) {
49+
config.set({mail: {transport: 'stub', options: {error: 'Stub made a boo boo :('}}});
50+
51+
mailer.init().then(function () {
52+
mailer.transport.transportType.should.eql('STUB');
53+
return MailAPI.send(mailData, testUtils.context.internal);
54+
}).then(function () {
55+
done(new Error('Stub did not error'));
56+
}).catch(function (error) {
57+
error.message.should.startWith('Error: Stub made a boo boo :(');
58+
error.errorType.should.eql('EmailError');
59+
done();
60+
}).catch(done);
15061
});
15162
});

0 commit comments

Comments
 (0)