[Mailer] Fix the possibility to set a From header from MessageListener #31774
Conversation
fabpot
commented
May 31, 2019
| Q | A |
|---|---|
| Branch? | 4.3 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #31733 |
| License | MIT |
| Doc PR | n/a |
| @@ -40,14 +37,7 @@ public function __construct(Address $sender, array $recipients) | ||
|
|
||
| public static function create(RawMessage $message): self |
There was a problem hiding this comment.
Not sure if we should keep this method.
|
@fabpot I'm getting a new error:
I think it relates to the difference between "Sender" and "From" that I was mentioning (#31733 (comment)) - though I'm far from an expert on this stuff. Should the |
|
"From" is what you write on the letter, "Sender" is what you write on the envelope. If you don't provide an envelope, we create one for you based on what you wrote on the letter. So, you must always provide a "From" (or register a MessageListener to do it for you -- will be part of the semantic configuration when available of course). |
146f260 to
f4254e6
Compare
|
|
||
| public function __construct(RawMessage $message) | ||
| { | ||
| if (!$message instanceof Message) { |
There was a problem hiding this comment.
Wouldn't it be better to change the type-hint instead? If we want to support any other message in the future we can still change the type then.
There was a problem hiding this comment.
I don't think so. I've done it that way to get a "better" error message.
There was a problem hiding this comment.
But it means that the user will only get an error at runtime if they didn't discover before that Message instances are required here. With a proper type hint they can be warned before.
There was a problem hiding this comment.
How would they be warned before? I suppose you're thinking about static analysis. Not sure what to do.
There was a problem hiding this comment.
In any case, that's independent from this PR as I'm doing the same at several other places (and this code is copy/pasted from SmtpEnvelope).
…sageListener (fabpot) This PR was merged into the 4.3 branch. Discussion ---------- [Mailer] Fix the possibility to set a From header from MessageListener | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #31733 | License | MIT | Doc PR | n/a <!-- Replace this notice by a short README for your feature/bugfix. This will help people understand your PR and can be used as a start for the documentation. Additionally (see https://symfony.com/roadmap): - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against branch 4.4. - Legacy code removals go to the master branch. --> Commits ------- f4254e6 [Mailer] fixed the possibility to set a From header from MessageListener
This PR was merged into the 4.3 branch. Discussion ---------- Change type hints | Q | A | ------------- | --- | Branch? | 4.3 for features / 3.4, 4.2 or 4.3 for bug fixes <!-- see below --> | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | n/a | License | MIT | Doc PR | n/a see #31774 (comment) <!-- Replace this notice by a short README for your feature/bugfix. This will help people understand your PR and can be used as a start for the documentation. Additionally (see https://symfony.com/roadmap): - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against branch 4.4. - Legacy code removals go to the master branch. --> Commits ------- d56ae06 changed type hints