Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#175168739] Common utilities for mailing #112

Merged
merged 8 commits into from
Oct 15, 2020
Merged

[#175168739] Common utilities for mailing #112

merged 8 commits into from
Oct 15, 2020

Conversation

balanza
Copy link
Contributor

@balanza balanza commented Oct 9, 2020

Collect mail utilities and groups them under a unique submodule of io-functons-commons. The idea is to have a single, shared entrypoint applications can call and setup by just pour their configuration in it.

The PR is in draft as I'd like to make some integration tests.

Design

Everything regarding email is now under the /mailer folder. There are no outbound dependency references, so the folder may be even moved in a module on its own.
The folder has a index.ts entrypoint which is ideally the only file users want to refer. Inner references worth to the user are proxied by the index module itself.
Every reference to nodemailer has been confined into the mailer/transports file.
Mailer's env configuration decoder is exposed and user applications may want to extend their config with that.

Usage

import { sendMail, getMailerTransporter, MailerConfig } from "io-functions-commons/dist/mailer";

// config is expected to be decoded already
const config = MailerConfig.decode(process.env).getOrElseL(_=> throw);
// create a transporter
const transporter = getMailerTransporter(config);
// use sendMail util to wrap the request into a TaskEither
sendMail(transporter, { from, to, subject /* etc etc */ }).run();

Breaking changes

When upgrading to a version of io-functions-commons which introduces this changes, please take note of the following:

  • Transporter options used to be composed inside the application, now it's getMailerTransporter's duty
  • module utils/nodemailer is removed, most of its functionalities are now in mailer/transports
  • modules utils/email is removed, its only function (sendMail) has been included into mailer/transports
  • function sendMail used to return a Promise<Either<Error, MailInfo>>, now it's a TaskEither<Error, MailInfo>
  • modules utils/multi_transport_connection and utils/mailup have both been moved into /mailer

@@ -23,7 +23,7 @@
"@types/helmet": "^0.0.40",
"@types/jest": "^24.0.13",
"@types/node": "10.14.1",
"@types/nodemailer": "^4.6.2",
"@types/nodemailer": "^6.4.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine as we are already using this version in our applications

@pagopa-github-bot
Copy link
Contributor

pagopa-github-bot commented Oct 9, 2020

Affected stories

  • 🌟 #175168739: Come DEV voglio che l'invio email sia una utility comune alle functions

New dependencies added: nodemailer-sendgrid, @types/node-fetch and @types/nodemailer-sendgrid.

nodemailer-sendgrid

Author: Andris Reinman

Description: SendGrid transport object for Nodemailer.

Homepage: http://npmjs.com/package/nodemailer-sendgrid

Createdalmost 3 years ago
Last Updatedover 2 years ago
LicenseMIT
Maintainers1
Releases4
Direct Dependencies@sendgrid/mail
Keywordsnodemailer and sendgrid
README

nodemailer-sendgrid

SendGrid transport object for Nodemailer.

Warning, vendor lock-in ahead!

Using provider APIs like SendGrid might result in a vendor lock-in, especially if you are using provider specific features. So always consider if you would
prefer to use SMTP based services instead where vendor lock-ins do not happen.

Switching from a SMTP based provider to another is much easier (you do need to edit some DNS settings at least) than switching API based providers where you
probably have a lot of custom code targeting your existing provider.

This module is specially designed to be as much compatible with Nodemailer as possible, so if you do not touch the Sendgrid specific configuration options then
switching from SendGrid API to any other provider should be just as easy as switching from SMTP.

Usage

Requires Nodemailer v4.3.0+

This module is mostly meant to demonstrate the usage of mail.normalize(cb) method in Nodemailer v4.3. This allows creating HTTP API based transports for
Nodemailer much easier.

Install from NPM

npm install nodemailer nodemailer-sendgrid

Create Nodemailer transport

const nodemailer = require('nodemailer');
const nodemailerSendgrid = require('nodemailer-sendgrid');
const transport = nodemailer.createTransport(
    nodemailerSendgrid({
        apiKey: process.env.SENDGRID_API_KEY
    })
);

See full example.

Send a message

Message objects support the entire Nodemailer API. In addition you can provide SendGrid specific keys like templateId or sendAt.

transport.sendMail({
    from: 'sender@example.com',
    to: 'Receiver Name <receiver@example.com>, someother@example.com',
    subject: 'hello world',
    html: '<h1>Hello world!</h1>'
});

License

MIT

@types/node-fetch

Author: Unknown

Description: TypeScript definitions for node-fetch

Homepage: http://npmjs.com/package/@types/node-fetch

Createdabout 4 years ago
Last Updatedabout 2 months ago
LicenseMIT
Maintainers1
Releases33
Direct Dependencies@types/node and form-data
README

Installation

npm install --save @types/node-fetch

Summary

This package contains type definitions for node-fetch (https://github.com/bitinn/node-fetch).

Details

Files were exported from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node-fetch.

Additional Details

Credits

These definitions were written by Torsten Werner, Niklas Lindgren, Vinay Bedre, Antonio Román, Andrew Leedham, Jason Li, Brandon Wilson, Steve Faulkner, ExE Boss, Alex Savin, Alexis Tyler, and Jakub Kisielewski.

@types/nodemailer-sendgrid

Author: Unknown

Description: TypeScript definitions for nodemailer-sendgrid

Homepage: http://npmjs.com/package/@types/nodemailer-sendgrid

Created4 months ago
Last Updatedabout 2 months ago
LicenseMIT
Maintainers1
Releases1
Direct Dependencies@types/nodemailer
README

Installation

npm install --save @types/nodemailer-sendgrid

Summary

This package contains type definitions for nodemailer-sendgrid (https://github.com/nodemailer/nodemailer-sendgrid).

Details

Files were exported from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/nodemailer-sendgrid.

Additional Details

  • Last updated: Tue, 23 Jun 2020 19:07:37 GMT
  • Dependencies: @types/nodemailer
  • Global values: none

Credits

These definitions were written by Luke Jones.

Generated by 🚫 dangerJS against 48a197a

Comment on lines 43 to 48
const abortableFetch = AbortableFetch(agent.getHttpsFetch(process.env));
const fetchWithTimeout = setFetchTimeout(
DEFAULT_EMAIL_REQUEST_TIMEOUT_MS as Millisecond,
abortableFetch
);
const fetchAgent = toFetch(fetchWithTimeout);
Copy link
Contributor

@gunzip gunzip Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, as this we're accessing process.env here, this (fetch) should be configured in the caller and removed from this module but passed as a parameter to getMailerTransporter

@codecov-io
Copy link

codecov-io commented Oct 9, 2020

Codecov Report

Merging #112 into master will increase coverage by 0.06%.
The diff coverage is 83.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
+ Coverage   86.50%   86.56%   +0.06%     
==========================================
  Files          32       70      +38     
  Lines        1000     1020      +20     
  Branches       92       86       -6     
==========================================
+ Hits          865      883      +18     
- Misses        133      134       +1     
- Partials        2        3       +1     
Impacted Files Coverage Δ
src/utils/response.ts 50.00% <50.00%> (+8.82%) ⬆️
src/utils/async.ts 63.46% <63.46%> (ø)
src/mailer/transports.ts 71.42% <71.42%> (ø)
src/models/message.ts 86.15% <77.14%> (-3.56%) ⬇️
src/models/message_status.ts 84.21% <77.77%> (-1.09%) ⬇️
src/models/notification_status.ts 80.64% <81.81%> (-1.58%) ⬇️
src/mailer/mailup.ts 90.47% <85.71%> (ø)
src/utils/cosmosdb_model.ts 85.71% <85.71%> (ø)
src/utils/types.ts 90.90% <90.90%> (ø)
src/mailer/config.ts 91.66% <91.66%> (ø)
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae0c2e9...07f4963. Read the comment docs.

// exclude a specific value from a type
// as strict equality is performed, allowed input types are constrained to be values not references (object, arrays, etc)
// tslint:disable-next-line max-union-size
export const AnyBut = <A extends string | number | boolean | symbol, O = A>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this belongs to io-ts-commons, but at least, can we move it into ustils/types.ts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not do it because I didn't stress AnyBut enough to make it a general-purpose utility. It is performing good for this specific case, but I'm afraid it might not be the same in other contexts.

@gunzip
Copy link
Contributor

gunzip commented Oct 12, 2020

why draft?

@balanza balanza marked this pull request as ready for review October 14, 2020 14:59
@gunzip gunzip self-requested a review October 15, 2020 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants