Add SendGrid template support. #163
Conversation
|
Outstanding issue is getting the raw JSON |
|
This looks pretty cool and I can see how it would be useful for people. Can you confirm that you've tried this out and it works? I have a few style comments and questions that I'll try to leave in the next few days :D |
|
I can confirm that the template functionality works. Looking forward to the feedback! |
| @@ -107,6 +107,7 @@ defmodule Bamboo.SendgridAdapter do | |||
| |> put_subject(email) | |||
| |> put_html_body(email) | |||
| |> put_text_body(email) | |||
| |> put_x_smtp_api(email) | |||
There was a problem hiding this comment.
How about maybe_put_x_smtp_api so it's clear that it may or may not happen?
|
Hey @rafroehlich2 Thanks a ton for this. I left some comments on it. Let me know if you have any questions. I'm excited to get this in :D |
| """ | ||
| def substitute(email, tag, value) do | ||
| case is_binary(tag) do | ||
| false -> |
There was a problem hiding this comment.
Since this is a boolean I think an if might make more sense here
if is_binary(tag) do
# send it...
else
# raise...
end
|
I think this looks great! I (hopefully) answered your question and left a couple comments. I'll merge once the error is added. Thanks so much for this (and sorry for the late response. Been a bit busy this summer :D) |
|
No worries. The error in the tests are a result of the ExMachina issue #145. The individual tests for SendgridAdapter and SendgridHelper all test green. I will change the dependency version once it is updated with the fix. |
|
@rafroehlich2 I believe you can rebase against the latest on master and it should pass on CI! If it does I'll go ahead and merge this in. Thanks for your work on it 🎉 |
|
One more fix coming down the pipe so hold off merging. |
|
I also think this might need to be rebased because there are a bunch of commits in this PR |
…al helpful error message.
This allows a person to substitute these fields and not have to supply an empty string for each.
|
This is ready to be merged. Let me know if you want this further rebased. |
|
Status on merge? |
|
Thank you so much @rafroehlich2! I think this will be very helpful to SendGrid users |
The current implementation only allows using substitution tags on a specified template. This can be used as a jumping off point for future use of the
x-smtpapifield.