node-red / node-red-nodes

Extra nodes for Node-RED
Other
1k stars 595 forks source link

extending e-mail node's usage of nodemailer createTransport #123

Open cowchimp opened 9 years ago

cowchimp commented 9 years ago

Hi.

The outgoing email node uses nodemailer to send email. AFAIK, node-red doesn't let you change the options passed to nodemailer's createTransport method, which is crucial for use with some SMTP providers.

Some of the most well-known email providers require configuring the transport with more options other than just server and port, so it needs to be extendable in some way.

Would be more than happy to send a PR with a fix and tests, but wanted to ask about how you want to approach this.

Here's 2 cents: Adding extra transport fields via the UI is probably not a good idea because it's too much of an edge-case, but maybe this is the sort of thing that should be extendable by passing in a msg with an extra property which contains an object that should be merged with the default configuration object.

It's not just a matter of enabling adding properties though, because right now the transport object is configured with secure: true by default, and this causes errors in some mail providers. Removing this now would break existing flows, so if avoiding a breaking change is a priority, we could either (a) Expose this as a checkbox in the UI which defaults to checked. (b) Allow to override the default true by passing secure: false in the aforementioned new msg configuration property.

WDYT?

For reference:

Thanks!

knolleary commented 9 years ago

Given mailinator's list of well-known configs, it would appear to make sense to offer those as predefined server configs (which hides the details) - but also have an option that allows the finer details to be provided. I suspect most users would be able to pick something from that list, so if the finer details are hidden, it wouldn't be overwhelming.

I know @dceejay looked at some of this in the early days, so would want to get his view on it.

@cowchimp if you did want to follow-up with a PR once we're agreed on an approach then please take note of our contribution guidelines regarding CLAs.

dceejay commented 9 years ago

I don't have a problem with it... if you look at the html code I did have the long list of pre-done hosts already there (very old nodemailer code). The main reason for not enabling it being I didn't want to over complicate the UI with options that confuse beginners (and some of those mail hosts looked a bit iffy).... so yes - quite happy to accept a PR to address this... (as long as CLA in place)... and as long as UI doesn't go mad.