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 support for SendGrid's asm_group_id #368

Closed
wants to merge 3 commits into from

Conversation

richeterre
Copy link

This is useful for letting recipients unsubscribe from certain types of email communication. By assigning an ASM group id to the email, hitting "Unsubscribe" in the footer will unsubscribe the recipient only from this email group, not all emails.

This is useful for letting recipients unsubscribe from certain types of email communication. By assigning an ASM group id to the email, hitting "Unsubscribe" in the footer will unsubscribe the recipient only from this email group, not all communication.
@mootpointer
Copy link

This change looks good (and is something we need to get unsubscribe links to work properly with SendGrid). Is there any chance of cleaning up the conflicts and getting this merged in?

@richeterre
Copy link
Author

I've cleaned up the "conflict" (it was just some indenting issue), so maintainers are welcome to merge this if they so wish!

@tomtaylor
Copy link
Contributor

It'd be great to get this merged! Can I help?

Copy link
Contributor

@maymillerricci maymillerricci left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this. This looks great! There's just a tiny typo to be fixed, and merge conflicts to be resolved. Then we can merge!

@@ -83,6 +84,23 @@ defmodule Bamboo.SendGridHelper do
raise "expected a list of category strings"
end

@doc """
An integer id for an ASM (Advanced Suppression Manager) group that this email should belong to.
This can be used to let receipients unsubscribe from only a certain type of communication.
Copy link
Contributor

Choose a reason for hiding this comment

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

"receipients" -> "recipients"

@tomtaylor
Copy link
Contributor

To help getting this merged I've made the changes suggested in a new pull request: #457.

@maymillerricci
Copy link
Contributor

Closing this since the changes were incorporated in #457. Thanks for your work on this!

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.

4 participants