notifme / notifme-sdk

A Node.js library to send all kinds of transactional notifications.
https://notifme.github.io/www/
MIT License
1.94k stars 149 forks source link

Vote for SparkPost Email API #3

Closed aydrian closed 7 years ago

aydrian commented 7 years ago

If you provide some guidance, I'd be happy to create a PR to implement this.

There is a SparkPost transport for nodemailer if that makes it easier.

BDav24 commented 7 years ago

Yes sure, I think the best way is to call SparkPost api without using the nodemailer transport.

You can check Nexmo provider, which also calls an api: https://github.com/notifme/notifme-sdk/search?utf8=%E2%9C%93&q=nexmo&type=

Let me know if you need more information.

BDav24 commented 7 years ago

@aydrian SparkPost API is integrated :)

Note: 5 emails by day in sandbox mode to test is a bit short...

aydrian commented 7 years ago

Thank you. Yes. It's short by design. The deliverability of those emails are not great and were being abused by spammers. Once you create a sending domain, you can send 15K per month for free.

BDav24 commented 7 years ago

@aydrian I'm unsure about cc and bcc in your code, it's not passed the same way in the documentation (https://www.sparkpost.com/docs/faq/cc-bcc-with-rest-api/), can you confirm that it's working this way? https://github.com/notifme/notifme-sdk/blob/master/src/providers/email/sparkpost.js#L44-L45

aydrian commented 7 years ago

The node library has sugar methods to make passing cc & bcc easier. Let me double check.

aydrian commented 7 years ago

Sorry. I'll do a PR to fix those. I need to pass them in an array.

BDav24 commented 7 years ago

It's my mistake in fact, I didn't see that the lib was transforming these fields. I'm changing their type to make things clearer:

export type EmailRequestType = CommonRequestType & {
  ...
  cc?: string[],
  bcc?: string[],
  ...
}
BDav24 commented 7 years ago

I can handle the change, it doesn't seem to be a big change. One question though: if I have multiple cc, is that the right syntax?

  "content": {
    "from": "you@fromyou.com",
    "headers": {
      "CC": "cc1@thatperson.com, cc2@thatperson.com, cc3@thatperson.com"
    },
    "subject": "To and CC",
    "text": "This mail was sent to to@thisperson.com while CCing cc@thatperson.com."
  }
aydrian commented 7 years ago

Here's an example with the sugar. I would just use that, then it's just an array of objects and the lib will transform it for you. The header is only part of it. You also need them in the recipients with the header_to property set. Here's an example.

BDav24 commented 7 years ago

I removed the lib dependency, that's why I need to rewrite "the sugar". So:

Is this right?

_(Edited: switched email and header_to)_

aydrian commented 7 years ago

Yes. The lib is a thin wrapper so the structure would be the same as the second example.

BDav24 commented 7 years ago

verifying your domain: Status Blocked

:/

aydrian commented 7 years ago

Awe... It looks like your domain is already in use in another account. Do you have multiple SparkPost accounts?

BDav24 commented 7 years ago

It seems that @jnoleau added the domain on his test account, it should be alright now. Should I contact support or you can take care of it?

aydrian commented 7 years ago

If he removed it from his account, I would inform support.