balderdashy / sails-hook-email

Sails email hook
67 stars 34 forks source link

Upgraded Nodemailer, added new options, moved to Sails View Engine #13

Closed dapriett closed 9 years ago

dapriett commented 9 years ago

I made the following changes to sails-hook-email:

sgress454 commented 9 years ago

@dapriett I ended up merging #8, but feel free to rebase and/or submit another PR.

I'm open to the hook being able to handle more than just .ejs files, but I'd rather not make it dependent on the Sails views hook, since that can be turned off. You could use a solution that incorporates Consolidate.js, like the views hook does.

Re: the global mail option, I would leave that out. The end user can choose to add a global or not to their app which points to sails.hooks.email.sendEmail, but it's not something an option that belongs in the hook itself.

dapriett commented 9 years ago

@sgress454

Cool - I can remove the global option.

Is there a way to disable the sails view hook? It seems like it's currently embedded in the sails module. I figured using that directly would reduce having to configure your template engines in multiple place. I could also just fallback to ejs if for some reason the sails hook is not there.

It looks like in the branch you merged it's passing the options directly to the createTransport method. We may want to make it a separate option - so you can do stuff like this:

var smtpPool = require('nodemailer-smtp-pool');

module.exports.mailer = {
    testMode: false,

    from: "noreply@internet.com",

    transporter: smtpPool({
            host: 'localhost',
            port: 25,
            auth: {
             user: 'username',
              pass: 'password'
           },
           // use up to 5 parallel connections
           maxConnections: 5,
         // do not send more than 10 messages per connection
          maxMessages: 10,
          // no not send more than 5 messages in a second
          rateLimit: 5
        })
}

Otherwise currently in order to set sails-hook-email options (such as from, alwaysSendTo, etc) we would have to create the smtpPool object and then inject the hook options directly into it.

Also currently the old way (just providing service & auth) doesn't do pooling anymore like it used to. You have to explicitly wrap it in an smtpPool transport now with the new version. My branch handles that, so I can try to rebase and merge that in as well.

dapriett commented 9 years ago

I rebased and created a new pull request: https://github.com/balderdashy/sails-hook-email/pull/15 with the following improvements