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

Add Brevo types/config and deprecate usage of Sendinblue #374

Open
wants to merge 4 commits into
base: 2.5
Choose a base branch
from

Conversation

gruberro
Copy link

@gruberro gruberro commented Feb 11, 2024

Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? yes
Fixed tickets fixes #384
Related issues/PRs
License MIT

What's in this PR?

This PR aims to replace the deprecated naming/package SendinBlue with Brevo.

Why?

Get rid of deprecation warnings and keep naming aligned with official third-party naming.

BC Breaks/Deprecations

Configuration field sendinblue_api_key is renamed to brevo_api_key. Also all internal fields are renamed. This for sure requires existing form configurations to be updated. Not sure how you would like to proceed with that kind of changes. LMK if you desire another way of refactoring.

@alexander-schranz
Copy link
Member

@gruberro Thank you for your contribution. This currently would be a bc break so I will keep this open for the next major release so we can merge it then.

@alexander-schranz
Copy link
Member

I think the best option without a bc break would be to add new config and a new type with called brevo and provide documentation how to migrate from sendinblue configuration to brevo. Then we can keep this 2 types and remove in 3.0 the sendinblue type. The new type can then already use the new sdk.

@steeven-th @gruberro what do you think?

@steeven-th
Copy link

Sendinblue has become Brevo.
If the Sendinblue API still works, it's a good idea to keep it until version 3.0.
But if it's no longer working, it's better to remove it.

@gruberro
Copy link
Author

gruberro commented Jun 5, 2024

Sounds absolut valid. Sendinblue package is abandoned, but is still working. Do you want me to rework this PR @alexander-schranz?

@alexander-schranz
Copy link
Member

alexander-schranz commented Jun 5, 2024

If you want to work on it 👍 . In the config it will be something like:

-    ->scalarNode('sendinblue_api_key')->defaultValue(null)->end()
+    ->scalarNode('sendinblue_api_key')->setDeprecated(/*...*/)->defaultValue(null)->end()
+    ->scalarNode('brevo_api_key')->defaultValue(null)->end()

And then keep the sendinblue part as it is and create the new brevo files. We can then add a line to the UPGRADE.md how to change from the sendinblue to brevo sdk. So people can start new projects or migrate projects already to brevo and don't get a composer deprecation message if they alread did it.

@gruberro
Copy link
Author

gruberro commented Jun 7, 2024

Ok, will take care of over the next days…

@gruberro gruberro force-pushed the replace-deprecated-sendinblue-with-brevo branch from a5ec05f to 85d2bd3 Compare June 10, 2024 11:17
@gruberro gruberro changed the title Replace depreated SendinBlue naming and package with Brevo Add Brevo types/config and deprecate usage of Sendinblue Jun 10, 2024
@gruberro
Copy link
Author

@alexander-schranz I've reworked the PR by adding Brevo and deprecating all Sendinblue thingies 😄

@alexander-schranz alexander-schranz force-pushed the replace-deprecated-sendinblue-with-brevo branch from 85d2bd3 to 4f1f543 Compare August 16, 2024 13:26
@alexander-schranz
Copy link
Member

@gruberro Can you also add a description in the UPGRADE.md how to switch from SendinBlue to Brevo including SQL update query for existing form with sending blue.

@gruberro
Copy link
Author

@gruberro Can you also add a description in the UPGRADE.md how to switch from SendinBlue to Brevo including SQL update query for existing form with sending blue.

@alexander-schranz I've done that, but I would really appreciate a review of the query since I don't have a good knowledge of Sulu Forms' data management. I guess the form field id should remain as it is?

@@ -1,5 +1,25 @@
# Upgrade

## 2.5.x
Copy link
Author

Choose a reason for hiding this comment

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

tbd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sendinblue/api-v3-sdk is abandoned - replace by getbrevo/brevo-php
3 participants