Skip to content

[Notifier] Add Telnyx notifier bridge #41172

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
Jun 23, 2021
Merged

[Notifier] Add Telnyx notifier bridge #41172

merged 1 commit into from
Jun 23, 2021

Conversation

StaffNowa
Copy link
Contributor

@StaffNowa StaffNowa commented May 11, 2021 •
edited
Loading

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
License MIT
Doc PR symfony/symfony-docs#15377
Recipe PR symfony/recipes#951

Telnyx notifier https://developers.telnyx.com/docs/v2/messaging/quickstarts/sending-sms-and-mms

@StaffNowa StaffNowa requested a review from OskarStark as a code owner May 11, 2021 11:01
@carsonbot carsonbot changed the title [notifier] Add Telnyx notifier bridge [Notifier] Add Telnyx notifier bridge May 11, 2021
@nicolas-grekas nicolas-grekas added this to the 5.x milestone May 11, 2021
OskarStark
OskarStark reviewed May 11, 2021
View reviewed changes
src/Symfony/Component/Notifier/Bridge/Telnyx/README.md Outdated

where:
- `API_KEY` is your telnyx API key
- `FROM` is your sender. The phone number should start with a plus "%2B"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should validate this and throw a proper exception instead of documenting it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Asked company about to have ability to test number + custom sender name, then I will make additional validation :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

If people wants to use sender name they need to have this option. Where better put this option? into env like sub option? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean you can add a messageProfileId and from there the company name is used as sender?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OskarStark provider allow to have sender name like Symfony (not phone number) if this option filled and correct :) Than I will need to put this option into .env file to access it :) messageProfileId unique by their account and can be regenerated :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@StaffNowa StaffNowa requested a review from OskarStark May 26, 2021 10:51
@OskarStark OskarStark requested a review from Nyholm May 26, 2021 11:20
@StaffNowa
Copy link
Contributor Author

Added docs + recipes

@OskarStark OskarStark requested review from chalasr and jderusse May 27, 2021 14:28
@OskarStark OskarStark requested review from fabpot, chalasr, jderusse and Nyholm and removed request for jderusse, Nyholm and chalasr June 17, 2021 09:18
@StaffNowa
Copy link
Contributor Author

Forgot about me? :))))

@OskarStark
Copy link
Contributor

We need another review and approval please

@fabpot
Copy link
Member

fabpot commented Jun 23, 2021

Thank you @StaffNowa.

@fabpot fabpot merged commit 2fb272e into symfony:5.4 Jun 23, 2021
@StaffNowa StaffNowa deleted the feature/telnyx branch June 23, 2021 12:05
@wouterj wouterj mentioned this pull request Jun 26, 2021
Merged
fabpot added a commit that referenced this pull request Jun 27, 2021
@fabpot
bug #41868 [FrameworkBundle] Fix Telnyx service registration (fixes b…
2cc6956 
…uild) (wouterj)

This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] Fix Telnyx service registration (fixes build)

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This was missed in #41172 and caused the build to fail in 5.4 and 6.0

Commits
-------

1044087 [FrameworkBundle] Fix Telnyx service registration
This was referenced Nov 5, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@welcoMattic welcoMattic welcoMattic left review comments

@noniagriconomie noniagriconomie noniagriconomie left review comments

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark OskarStark approved these changes

@chalasr chalasr Awaiting requested review from chalasr

@jderusse jderusse Awaiting requested review from jderusse

@Nyholm Nyholm Awaiting requested review from Nyholm

Assignees
No one assigned
Labels
Feature Notifier Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
7 participants
@StaffNowa @OskarStark @fabpot @welcoMattic @noniagriconomie @nicolas-grekas @carsonbot