[#174824788] subscription feed on user delete #81
Conversation
balanza
requested review from
gunzip
and removed request for
cloudify and
francescopersico
Affected stories
New dependencies added: html-to-textAuthor: Malte Legenhausen Description: Advanced html to plain text converter Homepage: https://github.com/werk85/node-html-to-text
nodemailerAuthor: Andris Reinman Description: Easy as cake e-mail sending from your Node.js applications Homepage: https://nodemailer.com/
|
| Created | almost 3 years ago |
| Last Updated | over 2 years ago |
| License | MIT |
| Maintainers | 1 |
| Releases | 4 |
| Direct Dependencies | @sendgrid/mail |
| Keywords | nodemailer 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: '[email protected]',
to: 'Receiver Name <[email protected]>, [email protected]',
subject: 'hello world',
html: '<h1>Hello world!</h1>'
});License
MIT
Generated by 🚫 dangerJS
| // When the function starts, attempt to create the table if it does not exist | ||
| // Note that we cannot log anything just yet since we don't have a Context | ||
| tableService.createTableIfNotExists(subscriptionsFeedTable, () => 0); |
There was a problem hiding this comment.
remove these lines (they're useless even in io-fn-app)
env.example
Outdated
| @@ -53,3 +53,6 @@ NODE_TLS_REJECT_UNAUTHORIZED=0 | |||
| languageWorkers__node__arguments="--inspect=0.0.0.0:5861" | |||
|
|
|||
| # AzureWebJobs.UpdateVisibleServicesCache.Disabled=true | |||
|
|
|||
| QueueStorageConnection=<STORAGE_CONNECTION_STRING> | |||
There was a problem hiding this comment.
can we rename QueueStorageConnection to SubscriptionFeedStorageConnection ?
There was a problem hiding this comment.
Of course, just note that it's the same value used here https://github.com/pagopa/io-functions-app/blob/master/UpdateSubscriptionsFeedActivity/index.ts#L18
Codecov Report
@@ Coverage Diff @@
## master #81 +/- ##
==========================================
- Coverage 86.62% 83.94% -2.69%
==========================================
Files 39 44 +5
Lines 1249 1489 +240
Branches 88 124 +36
==========================================
+ Hits 1082 1250 +168
- Misses 166 234 +68
- Partials 1 5 +4
Continue to review full report at Codecov.
|
Co-authored-by: Danilo Spinelli <[email protected]>
| @@ -0,0 +1,176 @@ | |||
| import * as crypto from "crypto"; | |||
There was a problem hiding this comment.
Do we really want to maintain all this logic when we just want to add an UNSUBSCRIBE event in the table?
There was a problem hiding this comment.
May you mark the parts that you think are redundant? I've asked the same question myself, then I noticed that
- the intent here was to have a general purpose activity (as the name is indeed 'UpdateSubscriptionsFeedActivity' and not 'UnsubscribeUser')
- If we switch this to
UnsubscribeUserwe need to keep 80% of the logic anyway and you only gain about 10 lines less getting rid of the 'subscribe' branch.
a better way, as @balanza suggested, is to have this code inside io-functions-commons and use that here and in io-functions-app
There was a problem hiding this comment.
The risk of duplicating the "subscribe" code here is that you must maintain it in sync if changes happens.
About sharing the code inside io-functions-commons i am more convinced that subscriptions must be managed by a specific/separate service.
There was a problem hiding this comment.
what do you suggest in the meantime?
There was a problem hiding this comment.
It is ok for me to merge as is.
Co-authored-by: Danilo Spinelli <[email protected]>
…om:pagopa/io-functions-admin into 174824788-subscription-feed-on-user-delete
Add a record of type
UNSUBSCRIBEto the subscription feed when a user is deleted in his/her claim to erasure.A connection to the SubscriptionsFeed storage table is added to the current service.
Env variables
The following environment variables have been added: