Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Mailer][Mailgun] Support more headers#36148

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromNyholm:mailgun-headers
Mar 23, 2020

Conversation

Nyholm
Copy link
Member

@NyholmNyholm commentedMar 20, 2020
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?yes
TicketsFix#35776
LicenseMIT
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.

@greedyivan
Copy link
Contributor

How to set header without prefix?

As it was mentioned in the original issue,t:version andt:text mean nothing without thetemplate header.

@Nyholm
Copy link
MemberAuthor

Ooops. I forgot those.

Is “template” and “amp-html” the only two ‘non-prefixed’ headers?

I think I’ll make an exception for those two then.

@nicolas-grekasnicolas-grekas added this to thenext milestoneMar 20, 2020
@greedyivan
Copy link
Contributor

Is “template” and “amp-html” the only two ‘non-prefixed’ headers?

recipient-variables, as well.

The current solution requires update every time new headers are added to Mailgun, as the current history of Mailgun tells us that this happens after a while.

It is just a matter of time before the same issue reappears.

@Nyholm
Copy link
MemberAuthor

Thank you@greedyivan for the review.

I've deprecated passing a header without the "h:" prefix. So we need to "keep up to date" with Mailguns API until Symfony 6.0.

@fabpot
Copy link
Member

Thank you@Nyholm.

@fabpotfabpot merged commit0c22ab8 intosymfony:masterMar 23, 2020
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
@akalineskou
Copy link
Contributor

I'm usingSymfony\Component\Mime\Email to create the email:

$email    ->from($from)    ->replyTo($replyTo)    ->to($to);

And I'm gettingUser Deprecated: Not prefixing the Mailgun header name with "h:" is deprecated since Symfony 5.1. Use header name "h:reply-to" instead.
I was wondering where should theh: addition to thereply-to happen.

@Nyholm
Copy link
MemberAuthor

Excellent question!

I suggest to open up a new issue for that. That way it wont get lost on a closed PR.

akalineskou reacted with thumbs up emoji

} else {
// fallback to prefix with "h:" to not break BC
$headerName = 'h:'.$name;
@trigger_error(sprintf('Not prefixing the Mailgun header name with "h:" is deprecated since Symfony 5.1. Use header name "%s" instead.', $headerName), E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

This seems weird to me. Theh: prefix is a mailgun specific feature. Requiring the Email object to set it means writing Mailgun-specific code in a place that may not even be aware of which transport is used.

dmaicher and apfelbox reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

@Nyholm I suggest removing the deprecation warning

nicolas-grekas added a commit that referenced this pull requestMay 23, 2021
This PR was merged into the 5.2 branch.Discussion----------Make Mailgun Header compatible with other Bridges| Q             | A| ------------- | ---| Branch?       | 5.2| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This revert deprecating passing a header without the required Mailgun `h:` prefix.And makes the bridge compatible with other bridges.See:-#36148 (comment)-#41308 (comment)Commits-------82f1f9c Make Mailgun Header compatible with other Bridges
symfony-splitter pushed a commit to symfony/mailer that referenced this pull requestSep 28, 2021
This PR was merged into the 5.2 branch.Discussion----------Make Mailgun Header compatible with other Bridges| Q             | A| ------------- | ---| Branch?       | 5.2| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This revert deprecating passing a header without the required Mailgun `h:` prefix.And makes the bridge compatible with other bridges.See:-symfony/symfony#36148 (comment)-symfony/symfony#41308 (comment)Commits-------82f1f9c618 Make Mailgun Header compatible with other Bridges
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stofstofstof left review comments

@fabpotfabpotfabpot approved these changes

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

[Mailer][Mailgun] Support more headers
8 participants
@Nyholm@greedyivan@fabpot@akalineskou@javiereguiluz@stof@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp