-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Mailer][Mailgun] Support more headers #35776
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
Comments
That works for me. |
Hm. I’m doing something like this: $email->getHeaders()->addTextHeader('o:deliverytime', $sendAt->format(\DateTimeInterface::RFC2822)); But I get a payload like: “ |
symfony/src/Symfony/Component/Mailer/Bridge/Mailgun/Transport/MailgunApiTransport.php Lines 114 to 132 in 269c4a2
$email->getHeaders()->add(new TagHeader('tag'));
$email->getHeaders()->add(new MetadataHeader('my-var', 'value'));
$email->getHeaders()->addTextHeader('oX-My-Header', 'value'); |
Oh. I see. Thank you. I was just looking at 5.0. But there is still no way to set o:deliverytime |
It was added here: #35047 That was done for Postmark. To fully support Mailgun, it should be done something similar. |
Regarding these:
@fabpot, can we implement a |
@greedyivan Yes, I think that is a good idea. The Do you mind submitting a PR? |
I did some research and found out that the only place where it is needed is Thus, I suggest break BC and removing the addition of The upgrade will consist of the need to explicitly set the full name of the header. |
Thank you! Im all for making things consistent. But I really don't want to break BC. |
It is much more complicated than just adding prefix support. Currently,
Just add support for I understand that the Mailer component is not a full substitution for the original API library, but we must decide what part of that API it should support. It looks good that it supports arbitrary custom headers using the |
Let’s not complicate it to much, it’s a BC break but it’s still experimental, so it should be no problem to change this behavior if it makes sense. We did this quite often in messenger component |
What @OskarStark said. The goal of a component being experimental is precisely to allow us to break BC whenever it makes sense. |
This PR was squashed before being merged into the 5.1-dev branch. Discussion ---------- [Mailer][Mailgun] Support more headers | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | Fix #35776 | License | MIT | Doc PR | Im not sure if this should be classified as a bug since "setting headers are broken". Ie, you cannot use some Mailgun API features. Or, this may be a feature because now you may specify any supported custom header you want. Commits ------- 537c8b8 [Mailer][Mailgun] Support more headers
Mailgun has a number of headers available. Example is
o:deliverytime
andv:my-var
. There is no way of setting these headers with the current implementation. All headers get prefixed withh:
. SeeMailgunApiTransport::getPayload()
It would be a good idea to allow user to set these headers. See here for a complete list: https://documentation.mailgun.com/en/latest/api-sending.html#sending
I suggest to only allow headers that begins with
h:
,t:
,o:
,v:
and then fallback to prefix withh:
. That will allow us to add this in a BC compatible way.The text was updated successfully, but these errors were encountered: