Skip to content
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

[9.x] Add Symfony Mailer to Upgrade Guide #7454

Closed
wants to merge 18 commits into from
Closed

[9.x] Add Symfony Mailer to Upgrade Guide #7454

wants to merge 18 commits into from

Conversation

Jubeki
Copy link
Contributor

@Jubeki Jubeki commented Nov 22, 2021

There are probably lots of things to edit in this PR. I mostly copied and formatted the content of the laravel/framework#38481 PR description.

I also added some small code snippets and changed the order of some things.

I also was not sure at what level the headings need to be.

upgrade.md Outdated
});

##### `Illuminate\Mail\Message`
The Illuminate\Mail\Message class now contains an instance of Symfony\Component\Mime\Email instead of Swift_Message so all forwarding calls in userland will need to be updated.
Copy link
Member

Choose a reason for hiding this comment

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

We will perhaps need an example of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were only Methods which were not documented by Laravel because, Laravel already used from, subject, cc` like the Symfony Mailer. There should probably not be many changes needed.

upgrade.md Outdated
It's no longer possible to get a list of failed recipients. Instead, if an email is failed to send, and exception will be thrown.

##### Stream Options for SMTP transport
Setting stream options for the SMTP transport can no longer be done through an smtp key. Instead you need to set the options directly in the config. For example, when disabling TLS peer verification you can do
Copy link
Member

Choose a reason for hiding this comment

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

Will likely need an actual code example of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only have the verify_peer option in mind. Anything else which should be added as a code example?

upgrade.md Outdated

##### Renamed Swift methods to Symfony

All Swift methods were renamed to Symfony. For example:
Copy link
Member

Choose a reason for hiding this comment

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

May need a list of these, plus link to the Symfony Mailer documentation where they can read about the public methods available on a Symfony Mail message, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at the PR for the framework and searched for all methods which where renamed.

Do you want to keep methods in the upgrade guide which are probably not used?

  • Mailer::getSymfonyTransport()
  • Mailer::setSymfonyTransport(TransportInterface $transport)
  • MailManager::createSymfonyTransport($config)

@taylorotwell taylorotwell marked this pull request as draft November 22, 2021 20:43
@Jubeki Jubeki marked this pull request as ready for review November 22, 2021 23:37
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Thanks @Jubeki. Some minor changes are needed here.

Jubeki and others added 13 commits November 25, 2021 14:22
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
Co-authored-by: Dries Vints <[email protected]>
@Jubeki
Copy link
Contributor Author

Jubeki commented Nov 25, 2021

Thanks @driesvints I reviewed and committed everything you wanted to change.

@taylorotwell taylorotwell marked this pull request as draft December 2, 2021 13:56
@taylorotwell
Copy link
Member

Marking this as draft until I get closer to writing 9.0 upgrade guide.


##### Failed recipients

It's no longer possible to get a list of failed recipients. Instead, if an email is failed to send, and exception will be thrown.
Copy link
Member

Choose a reason for hiding this comment

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

What kind of exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@taylorotwell
A TransportExceptionInterface will be thrown if the email could not be send.
https://symfony.com/doc/current/mailer.html#handling-sending-failures

The failed recipients are an artifact of Swift_RfcComplianceException, thus holds the invalid address (you can validate this yourself as well, before sending)
symfony/symfony#33812

It would probably be better to word it something like this:

Suggested change
It's no longer possible to get a list of failed recipients. Instead, if an email is failed to send, and exception will be thrown.
It's no longer possible to get a list of failed recipients. Instead, you need to validate the emails yourself before sending.
Furthermore to validate if an email was really send you need to check this with your E-Mail Provider. For example through webhooks.

@taylorotwell
Copy link
Member

I have rewritten this and merged it to the current master branch of the documentation. Thank you for your help.

@driesvints
Copy link
Member

Thanks @Jubeki !

@Jubeki Jubeki deleted the add-symfony-mailer-to-upgrade-guide branch January 5, 2022 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants