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

feat(mailer): Resend handler #9175

Merged
merged 6 commits into from
Sep 18, 2023
Merged

Conversation

Josh-Walker-GM
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM commented Sep 15, 2023

Description
This is intended to be a mail handler which specifically uses the resend node sdk.

Changes

  1. Adds a new resend mail handler which uses the resend node sdk to send mail.
  2. Removes an unneeded uuid dependency from the in-memory mail handler. I noticed this when copying over the handler as a template for this new one.

Notes

  1. I had to add a transformation of the attachment contents when they were passed as a string. If I did not transform them into a buffer then the attachment content would be malformed during my testing.
  2. I have tested this manually but for the moment there is no specific testing suite/strategy in place to test these mail handlers/renderers.

@Josh-Walker-GM Josh-Walker-GM added the release:feature This PR introduces a new feature label Sep 15, 2023
@Josh-Walker-GM Josh-Walker-GM added this to the next-release milestone Sep 15, 2023
@Josh-Walker-GM Josh-Walker-GM self-assigned this Sep 15, 2023
@jtoar jtoar modified the milestones: next-release, v6.3.0 Sep 16, 2023
@Josh-Walker-GM Josh-Walker-GM marked this pull request as ready for review September 16, 2023 19:40
Copy link
Contributor

@dthyresson dthyresson left a comment

Choose a reason for hiding this comment

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

See comments. Otherwise LGTM!

// I was not having success at passing attachment contents as strings directly
// to the Resend client, so I'm going to transform them to Buffers if they are
// strings.
const transformedAttachments = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think other mail handlers will have a similar need? If so, break out into a util function? Can do that refactor later perhaps, unless easier to test individual function to convert.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure. In theory resend allows the string type but passing them in directly was confusing when it was malformed in my basic tests.

Happy to either extract it out or even change the interface (not user facing but handler facing) to adapt to what we find more convenient as we add more handlers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok. Let's refactor/revisit when implementing other handlers like Postmark, etc etc

packages/mailer/handlers/resend/README.md Outdated Show resolved Hide resolved
@dthyresson
Copy link
Contributor

@Josh-Walker-GM approved and good to merge

@Josh-Walker-GM Josh-Walker-GM enabled auto-merge (squash) September 18, 2023 23:37
@Josh-Walker-GM Josh-Walker-GM merged commit 74d36b7 into main Sep 18, 2023
29 checks passed
@Josh-Walker-GM Josh-Walker-GM deleted the jgmw-mailer/resend-handler branch September 18, 2023 23:59
jtoar pushed a commit that referenced this pull request Sep 19, 2023
**Description**
This is intended to be a mail handler which specifically uses the
[resend node sdk](https://github.com/resendlabs/resend-node/tree/main).

**Changes**
1. Adds a new resend mail handler which uses the resend node sdk to send
mail.
2. Removes an unneeded `uuid` dependency from the in-memory mail
handler. I noticed this when copying over the handler as a template for
this new one.

**Notes**
1. I had to add a transformation of the attachment contents when they
were passed as a string. If I did not transform them into a buffer then
the attachment content would be malformed during my testing.
2. I have tested this manually but for the moment there is no specific
testing suite/strategy in place to test these mail handlers/renderers.
@jtoar jtoar modified the milestones: next-release, v6.3.0 Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants