Skip to content

Navigation Menu

Sign in

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 more explicit error if a SMSChannel doesn't have a Recipient #42433

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

[Notifier] Add more explicit error if a SMSChannel doesn't have a Recipient #42433

merged 1 commit into from
Aug 23, 2021

Conversation

ismail1432
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? yes
License MIT

Improve error message if a SmSChannel doesn't have a Recipient. ATM The snippet bellow output the message:
The "sms" channel is not supported. because the SmsChannel::supports return false if you pass an instance of NoRecipient.

This PR improve the error message with : "The "sms" channel needs a Recipient."

<?php
        $notifier = new Notifier(['sms' => new SmsChannel(new NullTransport())]);
        $notifier->send(new Notification("Hello World!", ["sms/twilio"]));

@ismail1432 ismail1432 requested a review from OskarStark as a code owner August 9, 2021 00:16
Nyholm
Nyholm reviewed Aug 9, 2021
View reviewed changes
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you for this PR.

src/Symfony/Component/Notifier/Notifier.php Outdated Show resolved Hide resolved
@chalasr chalasr added this to the 5.4 milestone Aug 9, 2021
@ismail1432 ismail1432 force-pushed the improve-exception-for-sms-without-recipient branch from 81411d6 to 45d577f Compare August 9, 2021 15:05
@OskarStark OskarStark requested a review from fabpot August 10, 2021 07:04
fabpot
fabpot requested changes Aug 12, 2021
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

nvm

@fabpot
Copy link
Member

fabpot commented Aug 23, 2021

Thank you @ismail1432.

@fabpot fabpot merged commit 8220795 into symfony:5.4 Aug 23, 2021
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

@Nyholm Nyholm Nyholm left review comments

@fabpot fabpot fabpot approved these changes

@jderusse jderusse jderusse approved these changes

@OskarStark OskarStark Awaiting requested review from OskarStark OskarStark is a code owner

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

Successfully merging this pull request may close these issues.
6 participants
@ismail1432 @fabpot @jderusse @Nyholm @chalasr @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.