Add self signed certificate for smtp transport option #40
Conversation
Signed-off-by: Rupal Mahajan <[email protected]>
Signed-off-by: Rupal Mahajan <[email protected]>
Signed-off-by: Rupal Mahajan <[email protected]>
| @@ -94,6 +97,8 @@ async function getEventArguments(event) { | |||
| event['note'] = DEFAULT_EMAIL_NOTE; | |||
| if (event.multitenancy === undefined) | |||
| event['multitenancy'] = DEFAULT_MULTI_TENANCY; | |||
| if (event.selfsignedcerts === undefined) | |||
| event['selfsignedcerts'] = DEFAULT_SELF_SIGNED_CERTIFICATES; | |||
There was a problem hiding this comment.
Could you try this merge strategy :
const defaults = {
auth: DEFAULT_AUTH,
tenant: DEFAULT_TENANT,
... (etc)
}
Object.assign(defaults, ...event)
I think would both clarify the defaults, and make the fallback assignments a one-liner.
| @@ -20,7 +20,7 @@ try { | |||
| // Do not set AWS_SDK_LOAD_CONFIG if aws config file is missing. | |||
| } | |||
|
|
|||
| module.exports = async function sendEmail(filename, url, sender, recipient, transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, subject, note, emailbody) { | |||
| module.exports = async function sendEmail(filename, url, sender, recipient, transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, subject, note, emailbody, selfsignedcerts) { | |||
There was a problem hiding this comment.
Might start to make sense to turn this into an object-reference, with an interface. Naming might be hard, though I could make more sense and less brain-power out of arguments named {emailContent, smtpConfig} with associated interfaces.
This would also mean that further config changes (new options) do not cause change on this method (or at least this method signature
| @@ -20,7 +20,7 @@ try { | |||
| // Do not set AWS_SDK_LOAD_CONFIG if aws config file is missing. | |||
| } | |||
|
|
|||
| module.exports = async function sendEmail(filename, url, sender, recipient, transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, subject, note, emailbody) { | |||
| module.exports = async function sendEmail(filename, url, sender, recipient, transport, smtphost, smtpport, smtpsecure, smtpusername, smtppassword, subject, note, emailbody, selfsignedcerts) { | |||
| if (transport !== undefined && (transport === 'smtp' || ses !== undefined) && sender !== undefined && recipient !== undefined) { | |||
| spinner.start('Sending email...'); | |||
There was a problem hiding this comment.
This feels like making this module to two different things : 1. format/send email. 2. control UI/UX.
I think the only question in splitting this up would be if there is expected some "on-the-fly" UX while the email-sending process is ongoing... In our modern computing environment, there shouldn't be a timing issue... emails never take > 10sec so send.. or else they fail immediately.
| success(info); | ||
| } | ||
| }); | ||
| transporter.sendMail(mailOptions, function (err, info) { |
There was a problem hiding this comment.
This might be a lot more clear using a try..catch.. .and if necessary a throw on self-detected error (err value is not null?)
Description
Issues Resolved
#31
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.