-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Mailer does not collect profiling #31592
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
:+1 I am also missing the |
@Devristo I'm interested to see your solution for that
|
@tarlepp Your solution is much fancier. I basically have a function which returns a fixed mail address when a specific env is set. Something like |
@Devristo that is what I first tried too, but I didn't like that solution so much so I dig a bit deeper and found that solution - I hope that helps you too. Although not sure if that is the "correct" way to do that, but it works. |
The integration in the bundle is not finished yet... and quite non-existent TBH. We need someone to do the work I suppose. Regarding the profiler, it's not "easy" as most of the time, the email won't be sent right away, but async. So, not sure what we want to do. Regarding If someone wants to work on that, I would be more than happy to help on Slack or here or on a PR. |
If I understand correctly there should be some configuration / extension which configures the parameters of the The profiler will be definitely more complicated due to the varying ways messages can be routed through the system. I think mails can even be sent without using the
Hmm above is flawed by the fact that the Envelope can change later when the |
I think that for that |
It could ease the migration from SwiftMailer considerably though if the functionality is used a lot. For us it has been a quite common use case to deliver mails in a catch-all kind of fashion on non-production servers. |
So seems like current documentation about that
So basically that part could be just replaced with following:
So I don't really see so much difference - just documentation stuff. And also remember that |
The service configuration makes it much more brittle though; as it is tied to the implementation (EnvelopeListener and its constructor's signature). The configuration option gives more flexibility, even when the EnvelopeListener is removed / renamed / refactored everything will still work. But in the end it is not up to us ;). I am willing to dive a bit deeper into the profiling issue if someone could give me some pointers. |
I would definitely use a configuration for that. What about something like this:
|
Would we want the These are the options for the SwiftMailer
|
I think we should make it possible to configure everything related to the Transport in the DSN (that makes it easier to have more than one mailer for instance). For all transports:
And for SMTP transports only:
I would not make |
I see multiple things to do:
@Devristo do you want to work on one of these, maybe we can share the work ;) ? |
I started on the configuration. Just struggling a bit with the tests. If you want to start on the profiler go ahead :). Hopefully I will manage to finish up the configuration this week :) |
@Devristo Do you think you can share your work with us? I'd like to move forward with this one. I can help if needed. |
@garak To view sent messages in profiler, while this feature is implemented for the new This package includes a transport that will use the default swift mailer instance as a transport for emails sent with the new symfony mailer service, so you can view them in swift mailer profiler panel. DISCLAIMER: I'm the author of the package, and I'd like to see this feature implemented in the near future, so I can abandon it. |
My apologies for not finishing it. Could use help with the test case. Would we want an functional test? I struggled to use the private |
I managed to create some tests. Could someone check out my PR and see what else is needed? |
Trying mailer out today for the first time, and I'm also missing the possibility to see any sent mails (even with the NullTransport) in the profiler. |
… from config (Devristo) This PR was squashed before being merged into the 4.4 branch (closes #32081). Discussion ---------- [WIP][Mailer] Overwrite envelope sender and recipients from config | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #31592 #31733 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> # Description This MR adds the following configuration, example: ```yaml # config/packages/mailer.yaml framework: mailer: envelope: sender: 'sender@example.org' recipients: ['redirected@example.org'] ``` In turn the `\Symfony\Component\Mailer\EventListener\EnvelopeListener` will be configured to alter the sender and recipient addresses before the message has been sent. Note: it will only alter the envelope, thus rerouting the message to the correct mailbox. However the message itself will still have the original 'from' and 'to' headers. # Todos - [x] Alter configuration and dependency injection - [x] Create test case - [ ] Update XML config schema? - [ ] Doc PR <!-- 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 ------- 8e0c800 [WIP][Mailer] Overwrite envelope sender and recipients from config
See #32912 for a first version of the web profiler. |
This PR was merged into the 4.4 branch. Discussion ---------- [Mailer] Add message events logger | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | 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 | refs #31592, refs #32409, refs #31947, refs #31747 | License | MIT | Doc PR | n/a To allow testing emails and for the web profiler, we need a way to store all sent/queued messages. Commits ------- 7642178 [Mailer] added message events logger
This PR was merged into the 4.4 branch. Discussion ---------- [Mailer] Add support for the profiler | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | 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 | closes #31592 | License | MIT | Doc PR | n/a Web profiler for the Mailer. Commits ------- f152314 [Mailer] added support for the profiler
…for the Mailer (fabpot) This PR was merged into the 4.4 branch. Discussion ---------- [Mailer][Mime] Add PHPUnit constraints and assertions for the Mailer | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | 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 | refs #31592, closes #32409, closes #31947, closes #31747, closes #30850 | License | MIT | Doc PR | n/a This PR introduces PHPUnit constraints and assertions to ease testing emails in functional tests: ```php <?php namespace App\Tests; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; class MailerAssertionsTraitsTest extends WebTestCase { public function testSomething() { $client = static::createClient(); $client->request('GET', '/'); $this->assertEmailCount(2); $this->assertEmailIsQueued($this->getMailerEvent(0)); $email = $this->getMailerMessage(0); $this->assertEmailHasHeader($email, 'To'); $this->assertEmailHeaderSame($email, 'To', 'fabien@symfony.com'); $this->assertEmailTextBodyContains($email, 'Bar'); $this->assertEmailHtmlBodyContains($email, 'Foo'); $this->assertEmailAttachementCount($email, 1); } } ``` Commits ------- 23f237b added PHPUnit constraints and assertions for the Mailer
@fabpot is there a way currently to call the |
I was not sure if opening this issue, since Mailer component is experimental and 4.3 version is still in beta. Anyway, may is about to end and it looks like an important feature is missing in Mailer. I'm not even sure if this is a bug or a new feature (I opted for last one, being a new component)
My expectation is to replace current combo of Swiftmailer library and Swifmailer bundle with new combo of Mailer and MIME components, and to get more or less the same functionalities.
So, for example, I expect to see some info in the profiler and in the web debug toolbar (you know, the old good envelope icon linked to info about headers, text, rendered message, etc.).
I couldn't find anything in current mailer, for example the NullTransport is doing absolutely nothing
The text was updated successfully, but these errors were encountered: