Fix List-Unsubscribe merge vars #318
Conversation
| vars: [ | ||
| %{ | ||
| name: "subscription_id", | ||
| content: subscription_for(announcement, user).token |
There was a problem hiding this comment.
Thoughts on token_for? if subscription_for isn't used anywhere else?
|
LGTM |
| @@ -5,6 +5,25 @@ defmodule Constable.TestWithEcto do | |||
| quote do | |||
| import Constable.Factory | |||
| alias Constable.Repo | |||
|
|
|||
| defp merge_vars_for(user, announcement) do | |||
There was a problem hiding this comment.
Functions generally should not be written directly inside quotes because it makes stack traces super messed up since it's copying this function in the context of every module that uses it. Instead it would probably be good to extract a module that you alias or import here like alias EmailHelper or something. And since this isn't used in that many context I would probably not alias or import it in TestWithEcto and instead alias or import it in the test that uses it
|
I thought the issue was that the token wasn't being interpolated in the header. If that's the case, does this fix that? |
kenyonj
force-pushed
the
jk-fix-unsubscribe-header
branch
from
762e06d to
6e65db0
Compare
|
I think this is going to be subsumed by #339 |
|
@kenyonj I think you said that this is not possible after all so I'm going to close this. If I'm mistaken please reopen! |
This makes sure that the merge vars are set for mandrill to fill in when
sending the email.
closes: #251