-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Mailer] New Brevo mailer bridge (formerly Sendinblue) #50302
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
8cf7fda
to
1e6df84
Compare
src/Symfony/Component/Mailer/Bridge/Brevo/Transport/BrevoApiTransport.php
Outdated
Show resolved
Hide resolved
} | ||
|
||
/** | ||
* @return array<int, string> |
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.
the return type is actually list<array{email: string, name?: string}>
/** | ||
* @return array<int, string> | ||
*/ | ||
protected function stringifyAddresses(array $addresses): array |
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.
However the protected method of the AbstractTransport with a different return type is a bad idea. This should use a different method name for the needs of this class
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.
formatAddresses() ?
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.
this looks like a good name
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.
@PEtanguy you haven't replaced this method
@@ -76,7 +76,7 @@ protected function doSendApi(SentMessage $sentMessage, Email $email, Envelope $e | |||
/** | |||
* @return list<array{email: string, name?: string}> | |||
*/ | |||
protected function stringifyAddresses(array $addresses): array | |||
protected function formatAddresses(array $addresses): array |
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.
should be a private method as it does not try to overwrite the parent method anymore.
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.
Good point
a3a2944
to
72a6287
Compare
I will make the documentation once this is merged. I also noticed an error in the doc of another transport. |
73582e0
to
59e14a0
Compare
6.3 | ||
--- | ||
|
||
* Added the bridge as a replacement of the deprecated Sendinblue one. |
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.
* Added the bridge as a replacement of the deprecated Sendinblue one. | |
* Add the bridge |
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.
@PEtanguy you forgot this change when fixing requested things
I would not touch the other bridge in this PR; let's just add this as a new bridge. No need to deprecate the code IMHO, we can abandon the whole package via packagist. WDYT @fabpot ? |
Well, triggering a deprecation when someone configures a mailer DSN using the If we only abandon the package, projects removing the old bridge without updating their env variables in prod would get a broken project due to using an unsupported scheme. |
…anguy) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Notifier] Add Brevo bridge (formerly Sendinblue) | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | TODO Hello, This PR is aimed at updating the config for the sendinblue notifier. As you might have seen, Sendinblue has rebranded to [Brevo](https://developers.brevo.com/) and also rewrote their apis. This change ensure compatibility with the new endpoints and removes any reference to Sendinblue. This is the mailer PR: #50302 Commits ------- 731f9b0 [Notifier] Add Brevo bridge (formerly Sendinblue)
…anguy) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Notifier] Add Brevo bridge (formerly Sendinblue) | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | TODO Hello, This PR is aimed at updating the config for the sendinblue notifier. As you might have seen, Sendinblue has rebranded to [Brevo](https://developers.brevo.com/) and also rewrote their apis. This change ensure compatibility with the new endpoints and removes any reference to Sendinblue. This is the mailer PR: symfony/symfony#50302 Commits ------- 731f9b02ef [Notifier] Add Brevo bridge (formerly Sendinblue)
…anguy) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Notifier] Add Brevo bridge (formerly Sendinblue) | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | TODO Hello, This PR is aimed at updating the config for the sendinblue notifier. As you might have seen, Sendinblue has rebranded to [Brevo](https://developers.brevo.com/) and also rewrote their apis. This change ensure compatibility with the new endpoints and removes any reference to Sendinblue. This is the mailer PR: symfony/symfony#50302 Commits ------- 731f9b02ef [Notifier] Add Brevo bridge (formerly Sendinblue)
66fa84c
to
f537358
Compare
Thank you @PEtanguy. |
This PR was merged into the 6.4 branch. Discussion ---------- [Mailer] Rename Sendinlue to Brevo Sendinblue mailer transport was deprecated in Symfony 6.4, new transport Brevo was created due to the service name change. https://www.brevo.com/fr/blog/pourquoi-brevo/ PR : symfony/symfony#50302 Commits ------- 10ef904 [Mailer] Rename Sendinlue to Brevo
…anguy) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Notifier] Add Brevo bridge (formerly Sendinblue) | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | N/A | License | MIT | Doc PR | TODO Hello, This PR is aimed at updating the config for the sendinblue notifier. As you might have seen, Sendinblue has rebranded to [Brevo](https://developers.brevo.com/) and also rewrote their apis. This change ensure compatibility with the new endpoints and removes any reference to Sendinblue. This is the mailer PR: symfony/symfony#50302 Commits ------- 731f9b02ef [Notifier] Add Brevo bridge (formerly Sendinblue)
#51295 should be enough, right? |
Yes, thank you :) |
Hello,
This PR is aimed at updating the config for the sendinblue mailer.
As you might have seen, Sendinblue has rebranded to Brevo and also rewrote their apis.
This change ensure compatibility with the new endpoints and removes any reference to Sendinblue.
This is the notifier PR: #50296