-
-
Notifications
You must be signed in to change notification settings - Fork 166
Replace symfony/swiftmailer-bundle with symfony/mailer #1829
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great for a start!
Nice! Imo |
Regarding
|
I have added the fallback for Note: the default SMTP mailer of the Symfony Mailer Component does not allow as many options as Swiftmailer did. The following options in a DSN will be ignored:
I deprecated |
Please revert it. The Contao 3 framework is already deprecated, therefore we do not need to deprecate every class separately. |
Done. One problem I now noticed is, that I can make a PR for |
a43f722
to
c905ddc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, excellent work!
In #1830 we briefly discussed, whether Contao should use its own mailer, rather than using the one from the framework bundle. This would of course have to implemented in this PR first. However, we decided against it, as I think it would still make more sense to use the existing mailer service from the framework bundle. Otherwise Contao would have to implement a lot of the mailer component on its own, which I think defeats the purpose. |
I think so, too. Using our own mailer instead of the default one is not a good idea IMHO. |
Note to self: check whether
|
I created a PR for the missing options: symfony/symfony#37432 |
composer.json
Outdated
@@ -99,14 +99,14 @@ | |||
"symfony/http-foundation": "4.4.*", | |||
"symfony/http-kernel": "4.4.*", | |||
"symfony/lock": "4.4.*", | |||
"symfony/mailer": "^4.4 || 5.0.* || 5.1.*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"symfony/mailer": "^4.4 || 5.0.* || 5.1.*", | |
6D40 | "symfony/mailer": "4.4.*", |
Because that is how you have requested the package in the core-bundle/composer.json
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the dependencies now. Since Symfony 5.0 is EoL, I have changed it to ^4.4 || 5.1.*
. Note: I also updated the extra.symfony.require
accordingly. This is something we have to change for contao/managed-edition
as well.
composer.json
Outdated
@@ -99,14 +99,14 @@ | |||
"symfony/http-foundation": "4.4.*", | |||
"symfony/http-kernel": "4.4.*", | |||
"symfony/lock": "4.4.*", | |||
"symfony/mailer": "^4.4 || 5.1.*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"symfony/mailer": "^4.4 || 5.1.*", | |
"symfony/mailer": "4.4.*", |
This is still wrong. We are not yet supporting Symfony 5, therefore please use the same version constraint as we do for all other Symfony packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we could support individual components of Symfony 5, in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. We can leave the PR open until we have discussed it or we can adjust it and merge it now. 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 💪
Thank you @fritzmg. |
Description ----------- | Q | A | -----------------| --- | Fixed issues | Fixes contao/core-bundle#1613 | Docs PR or issue | contao/docs#465 _Note:_ this PR depends and is based on #1829. It will be rebased, once #1829 got merged. This is the alternative version of #1469, based on the [Symfony Mailer Component](https://symfony.com/doc/4.4/mailer.html) instead of the Swiftmailer Bundle. It makes the configured `framework.mailer.transports` selectable in the back end. Example: ```yml # config/config.yml framework: mailer: transports: app: smtps://app@example.org:foobar@example.org:465 page: smtps://page@example.org:foobar@example.org:465 forms: smtps://forms@example.org:foobar@example.org:465 newsletter: smtps://newsletter@example.org:foobar@example.org:465 contao: mailer: transports: page: ~ forms: ~ newsletter: ~ ``` <img src="https://user-images.githubusercontent.com/4970961/84607561-e3a06d00-aea5-11ea-9f89-daac495b8a85.png" alt="mailer_transport_01" width="576"> _Note:_ only the transports configured in `contao.mailer.transports` will be available for selection. You can also provide translations: ```yml # translations/mailer_transports.en.yml page: 'Page' forms: 'Forms' newsletter: 'Newsletters' ``` <img src="https://user-images.githubusercontent.com/4970961/84607577-f7e46a00-aea5-11ea-9cd1-59271843609b.png" alt="mailer_transport_02" width="576"> And you can override the `From` address for each transport in the Contao configuration: ```yml contao: mailer: transports: page: from: Contao Page <page@example.org> forms: from: Contao Forms <forms@example.org> newsletter: from: Contao Newsletter <newsletter@example.org> ``` <img src="https://user-images.githubusercontent.com/4970961/84607478-670d8e80-aea5-11ea-9bdd-1cb2b090fd5a.png" alt="mailer_transport_03" width="576"> _Note:_ only the transports configured in `contao.mailer.transports` will be available for selection. Using the Symfony Mailer Component for this seems more elegant to me, since it requires no change whatsoever in the `\Contao\Email` class ([see the comparison](fritzmg/contao@feature/symfony-mailer...feature/available-symfony-mailers)). With the Symfony Mailer Component, the transport to be used is simply chosen with an `X-Transport` header in the email message itself. This PR decorates the `mailer` service and automatically sets an `X-Transport` header based on the website root settings - and automatically overrides the `From` address based on the chosen transport. Commits ------- fd3da56 switch to symfony/mailer 6c21d67 change MAILER_DSN back to MAILER_URL eeaea32 dynamically add default mailer 6ad300c add \Swift_Mailer fallback 71a5553 use MAILER_DSN with MAILER_URL fallback cc34824 remove Email deprecation 8c2480a increase symfony/mailer dependency 3926766 switch to symfony/mailer 3fd2799 dynamically add default mailer 43aa11c provide mailer transport selection and from override c38e864 add missing model property 8cee6db fix AvailableTransportsTest 08c9b20 fix code style a8d9b59 fix yml style ca47801 only show configured mailer transports within Contao e4f774f change translation domain 1722e81 restore previous version requirement 21b55d2 code style fix 1abfc38 rename mailer_transport DCA field to mailerTransport af8563c use Annotations for mailerTransport options callback 058da89 implement some early outs 4585e9d add missing model methods a2da52c fix code style f802003 Merge remote-tracking branch 'origin/master' into feature/available-symfony-mailers a5d7c74 add more unit tests 94203d7 use assertSame 38c3de9 improve testAnnotatedCallbacks test 3ca8e02 Rearrange the form fields in the back end 33f38c4 add missing methods b7159fe change wording to 'mailer transport' ba4f423 merge with master b8ffb36 Apply suggestions from code review Co-authored-by: Leo Feyer <github@contao.org>
Description ----------- This PR replaces the usage of `symfony/swiftmailer-bundle` with `symfony/mailer`. It serves as a basis for a follow-up PR for the available mailer selection based on the [Symfony Mailer Component](https://symfony.com/doc/4.4/mailer.html) (which would replace the existing draft contao#1469 based on the Swiftmailer Bundle). The switch is pretty straight forward actually. However there are some things to consider which I will comment on in the files view. Commits ------- 38ae891 switch to symfony/mailer f7ac5f4 make port optional d3243e2 change MAILER_DSN back to MAILER_URL 73a7fd5 dynamically add default mailer 6ddec29 fix code style d820da6 add \Swift_Mailer fallback c793448 fix automatic embedding of images 525dfd6 add comment db5e1ba use MAILER_DSN with MAILER_URL fallback 193821a remove Email deprecation 44536cd allow query options from MAILER_URL c905ddc increase symfony/mailer dependency 8906865 add comment about why we add default mailer dynamically 399d6b5 add comment about the gmail transport 21983dc add invalid argument exception 5821447 code style fix 3981e8c update dependencies 181a9c3 update dependencies c8b8f42 update dependencies aa46267 only allow Symfony 4.4
…tao#1830) Description ----------- | Q | A | -----------------| --- | Fixed issues | Fixes contao/core-bundle#1613 | Docs PR or issue | contao/docs#465 _Note:_ this PR depends and is based on contao#1829. It will be rebased, once contao#1829 got merged. This is the alternative version of contao#1469, based on the [Symfony Mailer Component](https://symfony.com/doc/4.4/mailer.html) instead of the Swiftmailer Bundle. It makes the configured `framework.mailer.transports` selectable in the back end. Example: ```yml # config/config.yml framework: mailer: transports: app: smtps://app@example.org:foobar@example.org:465 page: smtps://page@example.org:foobar@example.org:465 forms: smtps://forms@example.org:foobar@example.org:465 newsletter: smtps://newsletter@example.org:foobar@example.org:465 contao: mailer: transports: page: ~ forms: ~ newsletter: ~ ``` <img src="https://user-images.githubusercontent.com/4970961/84607561-e3a06d00-aea5-11ea-9f89-daac495b8a85.png" alt="mailer_transport_01" width="576"> _Note:_ only the transports configured in `contao.mailer.transports` will be available for selection. You can also provide translations: ```yml # translations/mailer_transports.en.yml page: 'Page' forms: 'Forms' newsletter: 'Newsletters' ``` <img src="https://user-images.githubusercontent.com/4970961/84607577-f7e46a00-aea5-11ea-9cd1-59271843609b.png" alt="mailer_transport_02" width="576"> And you can override the `From` address for each transport in the Contao configuration: ```yml contao: mailer: transports: page: from: Contao Page <page@example.org> forms: from: Contao Forms <forms@example.org> newsletter: from: Contao Newsletter <newsletter@example.org> ``` <img src="https://user-images.githubusercontent.com/4970961/84607478-670d8e80-aea5-11ea-9bdd-1cb2b090fd5a.png" alt="mailer_transport_03" width="576"> _Note:_ only the transports configured in `contao.mailer.transports` will be available for selection. Using the Symfony Mailer Component for this seems more elegant to me, since it requires no change whatsoever in the `\Contao\Email` class ([see the comparison](fritzmg/contao@feature/symfony-mailer...feature/available-symfony-mailers)). With the Symfony Mailer Component, the transport to be used is simply chosen with an `X-Transport` header in the email message itself. This PR decorates the `mailer` service and automatically sets an `X-Transport` header based on the website root settings - and automatically overrides the `From` address based on the chosen transport. Commits ------- fd3da56 switch to symfony/mailer 6c21d67 change MAILER_DSN back to MAILER_URL eeaea32 dynamically add default mailer 6ad300c add \Swift_Mailer fallback 71a5553 use MAILER_DSN with MAILER_URL fallback cc34824 remove Email deprecation 8c2480a increase symfony/mailer dependency 3926766 switch to symfony/mailer 3fd2799 dynamically add default mailer 43aa11c provide mailer transport selection and from override c38e864 add missing model property 8cee6db fix AvailableTransportsTest 08c9b20 fix code style a8d9b59 fix yml style ca47801 only show configured mailer transports within Contao e4f774f change translation domain 1722e81 restore previous version requirement 21b55d2 code style fix 1abfc38 rename mailer_transport DCA field to mailerTransport af8563c use Annotations for mailerTransport options callback 058da89 implement some early outs 4585e9d add missing model methods a2da52c fix code style f802003 Merge remote-tracking branch 'origin/master' into feature/available-symfony-mailers a5d7c74 add more unit tests 94203d7 use assertSame 38c3de9 improve testAnnotatedCallbacks test 3ca8e02 Rearrange the form fields in the back end 33f38c4 add missing methods b7159fe change wording to 'mailer transport' ba4f423 merge with master b8ffb36 Apply suggestions from code review Co-authored-by: Leo Feyer <github@contao.org>
This PR replaces the usage of
symfony/swiftmailer-bundle
withsymfony/mailer
. It serves as a basis for a follow-up PR for the available mailer selection based on the Symfony Mailer Component (which would replace the existing draft #1469 based on the Swiftmailer Bundle).The switch is pretty straight forward actually. However there are some things to consider which I will comment on in the files view.