Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Notifier] Add Sweego bridge #58322

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 18, 2024
Merged

[Notifier] Add Sweego bridge #58322

merged 1 commit into from
Oct 18, 2024

Conversation

welcoMattic
Copy link
Member

@welcoMattic welcoMattic commented Sep 19, 2024 •
edited
Loading

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues
Docs PR symfony/symfony-docs#20338
Recipes PR symfony/recipes#1348
License MIT

This PR folllows

This Bridge brings Webhook support for sent event for now. Others will follow in the future.
from parameter is handled on Sweego side (AFAIK, it means one from per API Key. @pydubreucq can you confirm my words?).

@carsonbot carsonbot added this to the 7.2 milestone Sep 19, 2024
@welcoMattic welcoMattic changed the title Add Sweego Notifier bridge [Notifier] Add Sweego Notifier bridge Sep 19, 2024
@welcoMattic welcoMattic changed the title [Notifier] Add Sweego Notifier bridge [Notifier] Add Sweego bridge Sep 19, 2024
OskarStark
OskarStark requested changes Sep 19, 2024
View reviewed changes
src/Symfony/Component/Notifier/Bridge/Sweego/CHANGELOG.md Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/Sweego/SweegoTransport.php.orig Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/Sweego/Webhook/SweegoRequestParser.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/Sweego/Webhook/SweegoRequestParser.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/Sweego/Webhook/SweegoRequestParser.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Bridge/Sweego/composer.json Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Exception/UnsupportedSchemeException.php Outdated Show resolved Hide resolved
src/Symfony/Component/Notifier/Tests/Exception/UnsupportedSchemeExceptionTest.php Outdated Show resolved Hide resolved
@carsonbot carsonbot changed the title [Notifier] Add Sweego bridge Add Sweego bridge Sep 19, 2024
@carsonbot carsonbot changed the title Add Sweego bridge [Notifier] Add Sweego bridge Sep 19, 2024
@Wasta-Geek
Copy link

from parameter is handled on Sweego side (AFAIK, it means one from per API Key. @pydubreucq can you confirm my words?).

Hi, I'm working on Sweego project, to answer about this, indeed the from is managed on our side but it's not different for each API Key. An optional sender ID (alphanumeric sender) can be configured on Sweego App and when sending an SMS, the sender ID will be taken as sender; if missing, a short number will be used as sender

@welcoMattic
Copy link
Member Author

Hi, I'm working on Sweego project, to answer about this, indeed the from is managed on our side but it's not different for each API Key. An optional sender ID (alphanumeric sender) can be configured on Sweego App and when sending an SMS, the sender ID will be taken as sender; if missing, a short number will be used as sender

Hi @Wasta-Geek! Thanks for your advice about the Sender ID. Can you help me to implement it? I didn't find how/where to set this Sender ID in your documentation about SMS sending. We are close to finish this PR, let's do it together!

welcoMattic
welcoMattic commented Sep 25, 2024
View reviewed changes
src/Symfony/Component/Notifier/Bridge/Sweego/SweegoTransport.php Outdated Show resolved Hide resolved
@Wasta-Geek
Copy link

Hi, I'm working on Sweego project, to answer about this, indeed the from is managed on our side but it's not different for each API Key. An optional sender ID (alphanumeric sender) can be configured on Sweego App and when sending an SMS, the sender ID will be taken as sender; if missing, a short number will be used as sender

Hi @Wasta-Geek! Thanks for your advice about the Sender ID. Can you help me to implement it? I didn't find how/where to set this Sender ID in your documentation about SMS sending. We are close to finish this PR, let's do it together!

Oops, I didn't see your question last time I checked the PR.
I wasn't clear enough maybe, sorry; but a client can create a sender ID on Sweego App page and it will be used for SMS sending automatically for countries supporting it but it's not something that can be configured on API side when sending a message so you have nothing to add on your side.

@welcoMattic
Copy link
Member Author

region and campaign_type parameter has been added to the DSN to let users set them.

@welcoMattic welcoMattic added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 3, 2024
Wasta-Geek
Wasta-Geek reviewed Oct 4, 2024
View reviewed changes
src/Symfony/Component/Notifier/Bridge/Sweego/SweegoTransport.php Outdated Show resolved Hide resolved
OskarStark
OskarStark reviewed Oct 13, 2024
View reviewed changes
src/Symfony/Component/Notifier/Bridge/Sweego/SweegoTransport.php Outdated Show resolved Hide resolved
@welcoMattic welcoMattic requested review from stof, Wasta-Geek and nicolas-grekas and removed request for Wasta-Geek October 16, 2024 20:27
@welcoMattic
Copy link
Member Author

I've been informed that webhook signature is available from Sweego. I'm working on the implementation on both Bridge Mailer and Notifier. For Notifier, would you prefer update this PR or a new one?

@fabpot
Copy link
Member

fabpot commented Oct 18, 2024

I've been informed that webhook signature is available from Sweego. I'm working on the implementation on both Bridge Mailer and Notifier. For Notifier, would you prefer update this PR or a new one?

Let's do it in a new PR.

@fabpot
Copy link
Member

fabpot commented Oct 18, 2024

Thank you @welcoMattic.

@fabpot fabpot merged commit 6deaace into symfony:7.2 Oct 18, 2024
9 of 10 checks passed
@fabpot fabpot mentioned this pull request Oct 18, 2024
Closed
@welcoMattic welcoMattic deleted the sweego-notifier branch October 18, 2024 07:38
@fabpot
Copy link
Member

fabpot commented Oct 18, 2024

@welcoMattic Can you create a recipe?

@welcoMattic
Copy link
Member Author

@welcoMattic Can you create a recipe?

Sure, I'll create it this weekend with the doc PR

This was referenced Oct 19, 2024
Merged
Merged
@fabpot fabpot mentioned this pull request Oct 27, 2024
Merged
@nicolas-grekas nicolas-grekas mentioned this pull request Nov 15, 2024
Merged
fabpot added a commit that referenced this pull request Nov 18, 2024
@fabpot
bug #58888 [Mailer][Notifier] Sweego is backing their bridges, thanks…
ebcab14 
… to them! (nicolas-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[Mailer][Notifier] Sweego is backing their bridges, thanks to them!

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

After sponsoring `@welcoMattic` for their bridges (see #58322 and #57431), Sweego doubles down and is now also sponsoring the bridges themselves.

Thanks to them! (and give Sweego a try ;) )

/cc `@Wasta`-Geek `@pydubreucq`

Commits
-------

600d49e [Mailer][Notifier] Sweego is backing their bridges, thanks to them!
fabpot added a commit that referenced this pull request Dec 6, 2024
@fabpot
feature #58651 [Mailer][Notifier] Add webhooks signature verification…
3a804f5 
… on Sweego bridges (welcoMattic)

This PR was merged into the 7.3 branch.

Discussion
----------

[Mailer][Notifier] Add webhooks signature verification on Sweego bridges

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

This PR follows the 2 previous ones (#57431, and #58322). It brings webhook signature verification to improve security.

Sweego documentation about webhook is here https://learn.sweego.io/docs/webhooks/webhook_signature

Commits
-------

9c59199 Add webhooks signature verification on Sweego bridges
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark approved these changes

@xabbuh xabbuh xabbuh approved these changes

@Wasta-Geek Wasta-Geek Wasta-Geek approved these changes

@fabpot fabpot Awaiting requested review from fabpot

@stof stof Awaiting requested review from stof

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

Assignees
No one assigned
Labels
Feature Notifier ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.
7 participants
@welcoMattic @Wasta-Geek @fabpot @stof @OskarStark @xabbuh @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.