-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Notifier] Rename/Change AdminRecipient class #35558
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
Labels
Comments
Let's close the issue and continue the discussion on the PR instead. |
fabpot
added a commit
that referenced
this issue
Aug 7, 2020
This PR was squashed before being merged into the 5.2-dev branch. Discussion ---------- [Notifier] Change notifier recipient handling | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #35558 <!-- prefix each issue number with "Fix #", if any --> | License | MIT | Doc PR | tbd. According to @wouterj's reasoning in #35558 I did the following to merge the `AdminRecipient` with the `Recipient` class: - introduced a `EmailRecipientInterface` with `getEmail(): string` methods - introduced a `RecipientInterface` that is extended by `EmailRecipientInterface` and `SmsRecipientInterface` - changed `Recipient` class which now holds an email and a phone number property and implements the `EmailRecipientInterface` and the `SmsRecipientInterface` - changed `NoRecipient` class which now implements the `RecipientInterface` - removed the `AdminRecipient` and fixed all related type-hints - introduced an `EmailRecipient` and `SmsRecipient` - changed the type-hint of the `$recipient` argument in `NotifierInterface::send()`, `Notifier::getChannels()`, `ChannelInterface::notifiy()` and `ChannelInterface::supports()` to `RecipientInterface` - changed the type hint of the `$recipient` argument in the `as*Message()` of the `EmailNotificationInterface` and `SmsNotificationInterface` which now uses the introduced interfaces - changed the type hint of the `$recipient` argument in the static `fromNotification()` factory methods in `EmailMessage` and `SmsMessage` which now uses the introduced interfaces - changed `EmailChannel` to only support recipients which implement the `EmailRecipientInterface` - changed `SmsChannel` only supports recipients which implement the `SmsRecipientInterface` Commits ------- 588607b [Notifier] Change notifier recipient handling
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While testing out the new Notifier component to write some docs about it (ref symfony/symfony-docs#13025), I've found the
AdminRecipient
classname a bit confusing. It makes sense when purely looking at it from an error notification perspective.However, I think the Notifier component shines in being able to sent any kind of notification to any kind of transport.
I would propose to merge this with
Recipient
, so the base class can have both email or phone:The text was updated successfully, but these errors were encountered: