Merge pull request #6335 from damsonn/fix-mailer-specs

Improve test coverage for mailer

core/server/mail/index.js

@@ -62,19 +62,15 @@ GhostMailer.prototype.getDomain = function () {
 // This assumes that api.settings.read('email') was already done on the API level
 GhostMailer.prototype.send = function (message) {
     var self = this,
-        to,
+        to;
-        sendMail;
 
-    message = message || {};
+    // important to clone message as we modify it
+    message = _.clone(message) || {};
     to = message.to || false;
 
-    if (!this.transport) {
-        return Promise.reject(new Error(i18n.t('errors.mail.noEmailTransportConfigured.error')));
-    }
     if (!(message && message.subject && message.html && message.to)) {
         return Promise.reject(new Error(i18n.t('errors.mail.incompleteMessageData.error')));
     }
-    sendMail = Promise.promisify(self.transport.sendMail.bind(self.transport));
 
     message = _.extend(message, {
         from: self.from(),
@@ -84,7 +80,7 @@ GhostMailer.prototype.send = function (message) {
     });
 
     return new Promise(function (resolve, reject) {
-        sendMail(message, function (error, response) {
+        self.transport.sendMail(message, function (error, response) {
             if (error) {
                 return reject(new Error(error));
             }

core/server/translations/en.json

@@ -191,9 +191,6 @@
             }
         },
         "mail": {
-            "noEmailTransportConfigured": {
-                "error": "Error: No email transport configured."
-            },
             "incompleteMessageData": {
                 "error": "Error: Incomplete message data."
             },

core/test/integration/api/api_mail_spec.js

@@ -8,17 +8,9 @@ var testUtils       = require('../../utils'),
 
     // Stuff we are testing
     MailAPI         = require('../../../server/api/mail'),
-    mailDataNoDomain = {
+
-        mail: [{
+    // test data
-            message: {
+    mailData = {
-                to: 'joe@doesntexistexample091283zalgo.com',
-                subject: 'testemail',
-                html: '<p>This</p>'
-            },
-            options: {}
-        }]
-    },
-    mailDataNoServer = {
         mail: [{
             message: {
                 to: 'joe@example.com',
@@ -27,15 +19,6 @@ var testUtils       = require('../../utils'),
             },
             options: {}
         }]
-    },
-    mailDataIncomplete = {
-        mail: [{
-            message: {
-                subject: 'testemail',
-                html: '<p>This</p>'
-            },
-            options: {}
-        }]
     };
 i18n.init();
 
@@ -46,106 +29,34 @@ describe('Mail API', function () {
 
     should.exist(MailAPI);
 
-    describe('Nothing configured', function () {
-        it('return no email configured', function (done) {
-            MailAPI.send(mailDataNoServer, testUtils.context.internal).then(function (response) {
-                /*jshint unused:false */
-                done();
-            }).catch(function (error) {
-                error.message.should.eql('Error: No email transport configured.');
-                error.errorType.should.eql('EmailError');
-                done();
-            }).catch(done);
-        });
-
-        it('return no email configured even when sending incomplete data', function (done) {
-            MailAPI.send(mailDataIncomplete, testUtils.context.internal).then(function (response) {
-                /*jshint unused:false */
-                done();
-            }).catch(function (error) {
-                error.message.should.eql('Error: No email transport configured.');
-                error.errorType.should.eql('EmailError');
-                done();
-            }).catch(done);
-        });
-    });
-
-    describe('Mail API Direct', function () {
-        before(function (done) {
-            config.set({mail: {}});
-
-            mailer.init().then(function () {
-                done();
-            });
-        });
-
-        it('return correct failure message for domain doesn\'t exist', function (done) {
-            mailer.transport.transportType.should.eql('DIRECT');
-            return MailAPI.send(mailDataNoDomain, testUtils.context.internal).then(function () {
-                done(new Error('Error message not shown.'));
-            }, function (error) {
-                error.message.should.startWith('Error: Failed to send email');
-                error.errorType.should.eql('EmailError');
-                done();
-            }).catch(done);
-        });
-
-        // This test doesn't work properly - it times out locally
-        it.skip('return correct failure message for no mail server at this address', function (done) {
-            mailer.transport.transportType.should.eql('DIRECT');
-            MailAPI.send(mailDataNoServer, testUtils.context.internal).then(function () {
-                done(new Error('Error message not shown.'));
-            }, function (error) {
-                error.message.should.eql('Error: Failed to send email.');
-                error.errorType.should.eql('EmailError');
-                done();
-            }).catch(done);
-        });
-
-        it('return correct failure message for incomplete data', function (done) {
-            mailer.transport.transportType.should.eql('DIRECT');
-
-            MailAPI.send(mailDataIncomplete, testUtils.context.internal).then(function () {
-                done(new Error('Error message not shown.'));
-            }, function (error) {
-                error.message.should.eql('Error: Incomplete message data.');
-                error.errorType.should.eql('EmailError');
-                done();
-            }).catch(done);
-        });
-    });
-
-    describe.skip('Stub', function () {
     it('returns a success', function (done) {
         config.set({mail: {transport: 'stub'}});
 
         mailer.init().then(function () {
             mailer.transport.transportType.should.eql('STUB');
-                return MailAPI.send(mailDataNoServer, testUtils.context.internal);
+            return MailAPI.send(mailData, testUtils.context.internal);
         }).then(function (response) {
             should.exist(response.mail);
             should.exist(response.mail[0].message);
             should.exist(response.mail[0].status);
-                response.mail[0].status.should.eql({message: 'Message Queued'});
+
             response.mail[0].message.subject.should.eql('testemail');
             done();
         }).catch(done);
     });
 
     it('returns a boo boo', function (done) {
-            config.set({mail: {transport: 'stub', error: 'Stub made a boo boo :('}});
+        config.set({mail: {transport: 'stub', options: {error: 'Stub made a boo boo :('}}});
 
         mailer.init().then(function () {
             mailer.transport.transportType.should.eql('STUB');
-                return MailAPI.send(mailDataNoServer, testUtils.context.internal);
+            return MailAPI.send(mailData, testUtils.context.internal);
-            }).then(function (response) {
+        }).then(function () {
-                console.log('res', response.mail[0]);
             done(new Error('Stub did not error'));
-            }, function (error) {
+        }).catch(function (error) {
-                error.message.should.startWith('Error: Failed to send email - no mail server found at');
+            error.message.should.startWith('Error: Stub made a boo boo :(');
             error.errorType.should.eql('EmailError');
             done();
         }).catch(done);
     });
-    });
 });

core/test/unit/mail_spec.js

@@ -1,4 +1,4 @@
-/*globals describe, afterEach, it*/
+/*globals describe, afterEach, beforeEach, it*/
 /*jshint expr:true*/
 var should          = require('should'),
     Promise         = require('bluebird'),
@@ -8,11 +8,8 @@ var should          = require('should'),
     configUtils     = require('../utils/configUtils'),
     i18n            = require('../../server/i18n'),
 
-    SMTP;
+    // Mock SMTP config
-i18n.init();
+    SMTP = {
-
-// Mock SMTP config
-SMTP = {
         transport: 'SMTP',
         options: {
             service: 'Gmail',
@@ -21,7 +18,25 @@ SMTP = {
                 pass: '123'
             }
         }
-};
+    },
+
+    // test data
+    mailDataNoDomain = {
+        to: 'joe@doesntexistexample091283zalgo.com',
+        subject: 'testemail',
+        html: '<p>This</p>'
+    },
+    mailDataNoServer = {
+        to: 'joe@example.com',
+        subject: 'testemail',
+        html: '<p>This</p>'
+    },
+    mailDataIncomplete = {
+        subject: 'testemail',
+        html: '<p>This</p>'
+    };
+
+i18n.init();
 
 describe('Mail', function () {
     afterEach(function () {
@@ -54,6 +69,34 @@ describe('Mail', function () {
         }).catch(done);
     });
 
+    it('sends valid message successfully ', function (done) {
+        configUtils.set({mail: {transport: 'stub'}});
+
+        mailer.init().then(function () {
+            mailer.transport.transportType.should.eql('STUB');
+            return mailer.send(mailDataNoServer);
+        }).then(function (response) {
+            should.exist(response.message);
+            should.exist(response.envelope);
+            response.envelope.to.should.containEql('joe@example.com');
+            done();
+        }).catch(done);
+    });
+
+    it('handles failure', function (done) {
+        configUtils.set({mail: {transport: 'stub', options: {error: 'Stub made a boo boo :('}}});
+
+        mailer.init().then(function () {
+            mailer.transport.transportType.should.eql('STUB');
+            return mailer.send(mailDataNoServer);
+        }).then(function () {
+            done(new Error('Stub did not error'));
+        }).catch(function (error) {
+            error.message.should.eql('Error: Stub made a boo boo :(');
+            done();
+        }).catch(done);
+    });
+
     it('should fail to send messages when given insufficient data', function (done) {
         Promise.settle([
             mailer.send(),
@@ -70,7 +113,51 @@ describe('Mail', function () {
         }).catch(done);
     });
 
-    it('should use from address as configured in config.js', function () {
+    describe('Direct', function () {
+        beforeEach(function (done) {
+            configUtils.set({mail: {}});
+
+            mailer.init().then(function () {
+                done();
+            });
+        });
+
+        it('return correct failure message for domain doesn\'t exist', function (done) {
+            mailer.transport.transportType.should.eql('DIRECT');
+
+            mailer.send(mailDataNoDomain).then(function () {
+                done(new Error('Error message not shown.'));
+            }, function (error) {
+                error.message.should.startWith('Error: Failed to send email');
+                done();
+            }).catch(done);
+        });
+
+        it('return correct failure message for no mail server at this address', function (done) {
+            mailer.transport.transportType.should.eql('DIRECT');
+
+            mailer.send(mailDataNoServer).then(function () {
+                done(new Error('Error message not shown.'));
+            }, function (error) {
+                error.message.should.eql('Error: Failed to send email.');
+                done();
+            }).catch(done);
+        });
+
+        it('return correct failure message for incomplete data', function (done) {
+            mailer.transport.transportType.should.eql('DIRECT');
+
+            mailer.send(mailDataIncomplete).then(function () {
+                done(new Error('Error message not shown.'));
+            }, function (error) {
+                error.message.should.eql('Error: Incomplete message data.');
+                done();
+            }).catch(done);
+        });
+    });
+
+    describe('From address', function () {
+        it('should use the config', function () {
             configUtils.set({
                 mail: {
                     from: '"Blog Title" <static@example.com>'
@@ -79,7 +166,7 @@ describe('Mail', function () {
             mailer.from().should.equal('"Blog Title" <static@example.com>');
         });
 
-    it('should fall back to [blog.title] <ghost@[blog.url]> as from address', function () {
+        it('should fall back to [blog.title] <ghost@[blog.url]>', function () {
             // Standard domain
             configUtils.set({url: 'http://default.com', mail: {from: null}, theme: {title: 'Test'}});
             mailer.from().should.equal('"Test" <ghost@default.com>');
@@ -126,4 +213,10 @@ describe('Mail', function () {
             configUtils.set({mail: {from: null, fromaddress: '"C3PO" <fa@default.com>'}, theme: {title: 'Test'}});
             mailer.from().should.equal('"C3PO" <fa@default.com>');
         });
+
+        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.from().should.equal('"Ghost at default.com" <ghost@default.com>');
+        });
+    });
 });