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

Added Sendinblue as a provider #111

Merged
merged 32 commits into from Nov 17, 2021
Merged

Added Sendinblue as a provider #111

merged 32 commits into from Nov 17, 2021

Conversation

galezra
Copy link
Contributor

@galezra galezra commented Nov 12, 2021

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Added SendinBlue as a provider.

  • What is the current behavior? (You can also link to an open issue here)
    Add new SendinBlue provider #79

  • What is the new behavior (if this is a feature change)?
    Use SendinBlue as a email provider

  • Other information:
    Used the SendinBlue typescript repo to create the provider

@vercel
Copy link

vercel bot commented Nov 12, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/dimagrossman/docs/HNGPNtZXDhRJBRXa28eAqMKd3R2i
✅ Preview: https://docs-git-fork-galezra-sendinblue-provider-dimagrossman.vercel.app

[Deployment for e9b2d01 canceled]

@scopsy
Copy link
Contributor

scopsy commented Nov 12, 2021

From a quick pick look amazing @galezra ! I'll take a closer look in the near couple of days. Do you want maybe to already add the attachments support? Just merge from v0.3.0 and move your pr to point to it instead of master.

@galezra
Copy link
Contributor Author

galezra commented Nov 12, 2021

Sure, I'll add the attachment support and resolve the conflicts

@scopsy scopsy self-requested a review November 13, 2021 14:48
@scopsy
Copy link
Contributor

scopsy commented Nov 13, 2021

@galezra This looks great just waiting on the attachments and we can merge :)
Let me know if you need any help with it

* Add a direct message interface

* Fix lint error for short hand property

* Update content.engine.ts

* Update direct.handler.ts
@galezra
Copy link
Contributor Author

galezra commented Nov 14, 2021

@galezra This looks great just waiting on the attachments and we can merge :) Let me know if you need any help with it

Done

Copy link
Contributor

@scopsy scopsy left a comment

Choose a reason for hiding this comment

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

This looks fantastic and I think we are ready to merge. Just a small questions about the utf8 there

email.textContent = options.text;
email.attachment = options.attachments?.map((attachment) => ({
name: attachment?.name,
content: attachment?.file?.toString('utf8'),
Copy link
Contributor

Choose a reason for hiding this comment

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

@galezra Have you tried doing a test case of sending it here? I'm not sure about the utf8 string here, in the docs looks like they say it should be either a relative path or a base64 encoding. ( https://github.com/sendinblue/APIv3-nodejs-library/blob/master/docs/SendSmtpEmail.md )

If you haven't tried it our I can maybe create a free account there later to try and see if the attachments are sent and can be opened on the other side.

@galezra
Copy link
Contributor Author

galezra commented Nov 16, 2021

@scopsy You're right, utf8 encoding wasn't necessary. I fixed it and pushed the changes (to clarify, it worked before as well)

@scopsy scopsy linked an issue Nov 17, 2021 that may be closed by this pull request
@scopsy scopsy merged commit bcc957a into novuhq:v0.3.0 Nov 17, 2021
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.

Add new SendinBlue provider
5 participants