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

Add replyTo field to SendGrid provider and Email type #2045

Open
wants to merge 4 commits into
base: release
Choose a base branch
from

Conversation

hannesortmeier
Copy link

Description

This PR introduces the following changes:

Add optional replyTo field to Email type (commit 7ddc034): This commit adds a new optional field replyTo to the Email type. This allows for specifying a different email address that recipients will reply to.

Add optional replyTo field to SendGrid provider (commit adae95b): This commit extends the SendGrid provider to support the new replyTo field. If provided, the SendGrid provider will use this field as the reply-to address in the emails it sends.

Document replyTo field for SendGrid provider (commit 1120fae): This commit adds documentation for the new replyTo field in the SendGrid provider. This helps to ensure that the usage of this new field is clear to other developers.

These changes provide more flexibility in handling replies to emails sent by the application since without the replyTo field set, replying to an E-Mail sent by SendGrid will result in a 550 Mailbox not found.

Select what type of change this PR introduces:

  1. Just code/docs improvement (no functional change).
  2. Bug fix (non-breaking change which fixes an issue).
  3. New feature (non-breaking change which adds functionality).
  4. Breaking change (fix or feature that would cause existing functionality to not work as expected).

Update Waspc ChangeLog and version if needed

If you did a bug fix, new feature, or breaking change, that affects waspc, make sure you satisfy the following:

  1. I updated ChangeLog.md with description of the change this PR introduces.
  2. I bumped waspc version in waspc.cabal to reflect changes I introduced, with regards to the version of the latest wasp release, if the bump was needed.

@Martinsos Martinsos requested a review from infomiho May 21, 2024 08:37
@@ -1,5 +1,11 @@
# Changelog

## 0.13.3 (TBD)
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see that your forked main branch is 33 commits behind the main in our repo.

We are now working on 0.14.0 and you'll see the entries in the Changelog once you sync your branch with the main repo.

@@ -384,6 +384,10 @@ The `send` method accepts an object with the following fields:

The email address of the sender.

- `replyTo?: string`

The email address to which the recipient can reply to. Only available for SendGrid provider.
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you analyze the other providers to see if they also provide options for replyTo field? We need to consider the SMTP and the Mailgun providers to make sure Wasp works for as many people as possible.

Copy link
Author

Choose a reason for hiding this comment

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

I only checked the Mailgun provider which does not offer a reply_to option. I'm pretty sure, that it can be implemented for the SMTP provider as well. I will have a look at it.

@@ -4,7 +4,7 @@ import type { SendGridProvider, EmailSender } from "../types";

// PRIVATE API
export function initSendGridEmailSender(
provider: SendGridProvider
provider: SendGridProvider
Copy link
Contributor

@infomiho infomiho May 22, 2024

Choose a reason for hiding this comment

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

Let's remove this indent change to keep the changeset minimal.

Copy link
Author

Choose a reason for hiding this comment

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

Whoops, formatters at work I guess.

@infomiho
Copy link
Contributor

infomiho commented May 22, 2024

Nice job! Thanks for the contribution.

There are couple of things we need to do before we can merge this:

  • make sure your fork has the latest changes from main https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork#syncing-a-fork-branch-from-the-web-ui
  • update the Changelog not to mention 0.13.3 since this is not a version we'll be releasing. Instead, you should append your changes under the 0.14.0
  • you should investigate if the Mailgun or SMTP providers can also use the replyTo option and use it for them as well
  • update the docs accordingly (related to the previous point)
  • update the e2e tests (this might be tricky for you if you didn't set up the Haskell toolchain locally)
  • ensure the feature works by testing it locally with the examples/todoApp (again, this might be tricky for you if you didn't set up the project locally)

@infomiho infomiho deleted the branch wasp-lang:release August 26, 2024 12:49
@infomiho infomiho closed this Aug 26, 2024
@infomiho
Copy link
Contributor

Closed by accident!

@infomiho infomiho reopened this Aug 30, 2024
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.

2 participants