Resolve #596 : add support for enabling sendgrid click tracking #618
Conversation
|
Thanks for opening this PR. Our organization is also hoping this gets merged soon! We opened #596. |
There was a problem hiding this comment.
@germsvel - Hey! I don't know if you're still maintaining this library, but I have looked over @sreecodeslayer's implementation here and it's basically identical to Joydrive's implementation on our forked branch. I can attest that this strategy works in production, although ours is slightly different, in that we break out the click_tracking_enabled_text and allow that to be passed-in also. I don't think this is critical. I approve!
Ours:
defp put_click_tracking(body, %Email{
private: %{click_tracking_enabled: enabled, click_tracking_enabled_text: enabled_text}
}) do
click_tracking = %{enable: enabled, enabled_text: enabled_text}
tracking_settings =
body
|> Map.get(:tracking_settings, %{})
|> Map.put(:click_tracking, click_tracking)
body
|> Map.put(:tracking_settings, tracking_settings)
end
defp put_click_tracking(body, _), do: body def with_click_tracking(email, enabled, enabled_text)
when is_boolean(enabled) and is_boolean(enabled_text) do
email
|> Email.put_private(@click_tracking_enabled, enabled)
|> Email.put_private(@click_tracking_enabled_text, enabled_text)
end
def with_click_tracking(_email, _enabled, _enabled_text) do
raise "expected with_click_tracking enabled/enabled_text parameters to be booleans"
end
|
@danadaldos unfortunately, I haven't had much time to maintain Bamboo since leaving thoughtbot. But I just gave this a look, and it looks good to me! Thanks for contributing @sreecodeslayer! |
|
@germsvel Thank you! |
# Problem [A recent PR](#618) added click tracking support for the SendGrid adapter. Unfortunately, we didn't catch a typo that sends "enabled" as the param instead of "enable" as is recognized by the API. This results in the following error: ``` "{\"errors\":[{\"message\":\"The click_tracking enable parameter is required.\",\"field\":\"tracking_settings.click_tracking.enable\",\"help\":\"http://sendgrid.com/docs/API_Reference/Web_API_v3/Mail/errors.html#message.tracking_settings.click_tracking.enable\"}]}" ``` with these params: ``` "tracking_settings" => %{"click_tracking" => %{"enable_text" => false, "enabled" => false} ``` Where it says `"enabled" => false` in the params, it should say `"enable" => false` # Solution Fix the typo and update the tests.
Close #596