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

Convert send_grid_adapter to SendGrid API v3 #293

Merged
merged 1 commit into from
Nov 8, 2017

Conversation

randycoulman
Copy link
Contributor

The existing SendGridAdapter uses v2 of the SendGrid API. In https://sendgrid.com/docs/API_Reference/Web_API/mail.html, it recommends migrating to the new v3 API:

To current v2 users: We encourage migrating to the new endpoint to take advantage of upcoming features.

I was looking at adding attachment support to the existing adapter, and found that it would be much easier to do with the v3 API instead. So, I'm submitting this PR first, and will then add attachment support on top of it.

For reference, the v3 API docs are at https://sendgrid.com/docs/API_Reference/api_v3.html.

I've done my best to preserve the existing API for both the adapter and the helper, but I've changed the internal representation of the private data that the helper adds to the e-mail. The new format is simpler and less coupled to the resulting API format:

%{
  send_grid_template: %{
    template_id: "YOUR ID HERE",
    substitutions: %{
      "%foo%" => "bar"
    }
  }
}

I've tested this adapter in an existing application and it works perfectly, though that app doesn't use all of the available features.

I'm still relatively new to Elixir, so any suggestions for cleaning up the code or making it more idiomatic are most welcome.

@randycoulman
Copy link
Contributor Author

I just rebased this on the latest master.

@javierjulio
Copy link

javierjulio commented Aug 16, 2017

@randycoulman thanks for doing this! I've been using this in a small Elixir app in production and it works great. Its been very reliable without issue. Let me know if I can be of any help in continuing to test this PR as it gets reviewed.

@devshane
Copy link

Hi, any update on merging support for v3? I wonder if this is the cause of a mysterious "Unexpected error" mentioned in #280.

@Schultzer
Copy link

@paulcsmith what is the status of this and #294?

@paulcsmith paulcsmith closed this Nov 8, 2017
@paulcsmith paulcsmith reopened this Nov 8, 2017
Copy link
Contributor

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

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

@randycoulman Could you rebase this? Once that's done I'll go ahead and merge this in!

@randycoulman
Copy link
Contributor Author

@paulcsmith Done

@paulcsmith paulcsmith merged commit e8e9816 into beam-community:master Nov 8, 2017
@paulcsmith
Copy link
Contributor

Thanks

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.

5 participants