Skip to content

Commit

Permalink
feature #35773 [Notifier] Change notifier recipient handling (jschaedl)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fabpot committed Aug 7, 2020
2 parents 669b3df + 588607b commit 7520884
Show file tree
Hide file tree
Showing 28 changed files with 330 additions and 111 deletions.
Expand Up @@ -107,7 +107,7 @@
use Symfony\Component\Notifier\Bridge\Twilio\TwilioTransportFactory;
use Symfony\Component\Notifier\Bridge\Zulip\ZulipTransportFactory;
use Symfony\Component\Notifier\Notifier;
use Symfony\Component\Notifier\Recipient\AdminRecipient;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\PropertyAccess\PropertyAccessor;
use Symfony\Component\PropertyInfo\PropertyAccessExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyDescriptionExtractorInterface;
Expand Down Expand Up @@ -2098,7 +2098,7 @@ private function registerNotifierConfiguration(array $config, ContainerBuilder $
$notifier = $container->getDefinition('notifier');
foreach ($config['admin_recipients'] as $i => $recipient) {
$id = 'notifier.admin_recipient.'.$i;
$container->setDefinition($id, new Definition(AdminRecipient::class, [$recipient['email'], $recipient['phone']]));
$container->setDefinition($id, new Definition(Recipient::class, [$recipient['email'], $recipient['phone']]));
$notifier->addMethodCall('addAdminRecipient', [new Reference($id)]);
}
}
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Component/Notifier/CHANGELOG.md
Expand Up @@ -6,6 +6,20 @@ CHANGELOG

* [BC BREAK] The `TransportInterface::send()` and `AbstractTransport::doSend()` methods changed to return a `?SentMessage` instance instead of `void`.
* Added the Zulip notifier bridge
* The `EmailRecipientInterface` and `RecipientInterface` were introduced.
* Added `email` and and `phone` properties to `Recipient`.
* [BC BREAK] Changed the type-hint of the `$recipient` argument in the `as*Message()`
of the `EmailNotificationInterface` and `SmsNotificationInterface` to `EmailRecipientInterface`
and `SmsRecipientInterface`.
* [BC BREAK] Removed the `AdminRecipient`.
* The `EmailRecipientInterface` and `SmsRecipientInterface` now extend the `RecipientInterface`.
* The `EmailRecipient` and `SmsRecipient` were introduced.
* [BC BREAK] Changed the type-hint of the `$recipient` argument in `NotifierInterface::send()`,
`Notifier::getChannels()`, `ChannelInterface::notifiy()` and `ChannelInterface::supports()` to
`RecipientInterface`.
* Changed `EmailChannel` to only support recipients which implement the `EmailRecipientInterface`.
* Changed `SmsChannel` to only support recipients which implement the `SmsRecipientInterface`.


5.1.0
-----
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Notifier/Channel/BrowserChannel.php
Expand Up @@ -13,7 +13,7 @@

use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -29,7 +29,7 @@ public function __construct(RequestStack $stack)
$this->stack = $stack;
}

public function notify(Notification $notification, Recipient $recipient, string $transportName = null): void
public function notify(Notification $notification, RecipientInterface $recipient, string $transportName = null): void
{
if (null === $request = $this->stack->getCurrentRequest()) {
return;
Expand All @@ -42,7 +42,7 @@ public function notify(Notification $notification, Recipient $recipient, string
$request->getSession()->getFlashBag()->add('notification', $message);
}

public function supports(Notification $notification, Recipient $recipient): bool
public function supports(Notification $notification, RecipientInterface $recipient): bool
{
return true;
}
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Notifier/Channel/ChannelInterface.php
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Notifier\Channel;

use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -21,7 +21,7 @@
*/
interface ChannelInterface
{
public function notify(Notification $notification, Recipient $recipient, string $transportName = null): void;
public function notify(Notification $notification, RecipientInterface $recipient, string $transportName = null): void;

public function supports(Notification $notification, Recipient $recipient): bool;
public function supports(Notification $notification, RecipientInterface $recipient): bool;
}
6 changes: 3 additions & 3 deletions src/Symfony/Component/Notifier/Channel/ChatChannel.php
Expand Up @@ -14,7 +14,7 @@
use Symfony\Component\Notifier\Message\ChatMessage;
use Symfony\Component\Notifier\Notification\ChatNotificationInterface;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -23,7 +23,7 @@
*/
class ChatChannel extends AbstractChannel
{
public function notify(Notification $notification, Recipient $recipient, string $transportName = null): void
public function notify(Notification $notification, RecipientInterface $recipient, string $transportName = null): void
{
$message = null;
if ($notification instanceof ChatNotificationInterface) {
Expand All @@ -45,7 +45,7 @@ public function notify(Notification $notification, Recipient $recipient, string
}
}

public function supports(Notification $notification, Recipient $recipient): bool
public function supports(Notification $notification, RecipientInterface $recipient): bool
{
return true;
}
Expand Down
9 changes: 5 additions & 4 deletions src/Symfony/Component/Notifier/Channel/EmailChannel.php
Expand Up @@ -20,7 +20,8 @@
use Symfony\Component\Notifier\Message\EmailMessage;
use Symfony\Component\Notifier\Notification\EmailNotificationInterface;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\EmailRecipientInterface;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -46,7 +47,7 @@ public function __construct(TransportInterface $transport = null, MessageBusInte
$this->envelope = $envelope;
}

public function notify(Notification $notification, Recipient $recipient, string $transportName = null): void
public function notify(Notification $notification, RecipientInterface $recipient, string $transportName = null): void
{
$message = null;
if ($notification instanceof EmailNotificationInterface) {
Expand Down Expand Up @@ -84,8 +85,8 @@ public function notify(Notification $notification, Recipient $recipient, string
}
}

public function supports(Notification $notification, Recipient $recipient): bool
public function supports(Notification $notification, RecipientInterface $recipient): bool
{
return '' !== $recipient->getEmail();
return $recipient instanceof EmailRecipientInterface;
}
}
8 changes: 4 additions & 4 deletions src/Symfony/Component/Notifier/Channel/SmsChannel.php
Expand Up @@ -14,7 +14,7 @@
use Symfony\Component\Notifier\Message\SmsMessage;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Notification\SmsNotificationInterface;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;
use Symfony\Component\Notifier\Recipient\SmsRecipientInterface;

/**
Expand All @@ -24,7 +24,7 @@
*/
class SmsChannel extends AbstractChannel
{
public function notify(Notification $notification, Recipient $recipient, string $transportName = null): void
public function notify(Notification $notification, RecipientInterface $recipient, string $transportName = null): void
{
$message = null;
if ($notification instanceof SmsNotificationInterface) {
Expand All @@ -46,8 +46,8 @@ public function notify(Notification $notification, Recipient $recipient, string
}
}

public function supports(Notification $notification, Recipient $recipient): bool
public function supports(Notification $notification, RecipientInterface $recipient): bool
{
return $recipient instanceof SmsRecipientInterface && '' !== $recipient->getPhone();
return $recipient instanceof SmsRecipientInterface;
}
}
9 changes: 7 additions & 2 deletions src/Symfony/Component/Notifier/Message/EmailMessage.php
Expand Up @@ -15,9 +15,10 @@
use Symfony\Component\Mailer\Envelope;
use Symfony\Component\Mime\Email;
use Symfony\Component\Mime\RawMessage;
use Symfony\Component\Notifier\Exception\InvalidArgumentException;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\EmailRecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -35,8 +36,12 @@ public function __construct(RawMessage $message, Envelope $envelope = null)
$this->envelope = $envelope;
}

public static function fromNotification(Notification $notification, Recipient $recipient): self
public static function fromNotification(Notification $notification, EmailRecipientInterface $recipient): self
{
if ('' === $recipient->getEmail()) {
throw new InvalidArgumentException(sprintf('"%s" needs an email, it cannot be empty.', static::class));
}

if (!class_exists(NotificationEmail::class)) {
$email = (new Email())
->to($recipient->getEmail())
Expand Down
18 changes: 10 additions & 8 deletions src/Symfony/Component/Notifier/Message/SmsMessage.php
Expand Up @@ -11,10 +11,8 @@

namespace Symfony\Component\Notifier\Message;

use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Exception\InvalidArgumentException;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Notification\SmsNotificationInterface;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\SmsRecipientInterface;

/**
Expand All @@ -30,16 +28,16 @@ final class SmsMessage implements MessageInterface

public function __construct(string $phone, string $subject)
{
if ('' === $phone) {
throw new InvalidArgumentException(sprintf('"%s" needs a phone number, it cannot be empty.', static::class));
}

$this->subject = $subject;
$this->phone = $phone;
}

public static function fromNotification(Notification $notification, Recipient $recipient): self
public static function fromNotification(Notification $notification, SmsRecipientInterface $recipient): self
{
if (!$recipient instanceof SmsRecipientInterface) {
throw new LogicException(sprintf('To send a SMS message, "%s" should implement "%s" or the recipient should implement "%s".', get_debug_type($notification), SmsNotificationInterface::class, SmsRecipientInterface::class));
}

return new self($recipient->getPhone(), $notification->getSubject());
}

Expand All @@ -48,6 +46,10 @@ public static function fromNotification(Notification $notification, Recipient $r
*/
public function phone(string $phone): self
{
if ('' === $phone) {
throw new InvalidArgumentException(sprintf('"%s" needs a phone number, it cannot be empty.', static::class));
}

$this->phone = $phone;

return $this;
Expand Down
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Notifier\Notification;

use Symfony\Component\Notifier\Message\ChatMessage;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -21,5 +21,5 @@
*/
interface ChatNotificationInterface
{
public function asChatMessage(Recipient $recipient, string $transport = null): ?ChatMessage;
public function asChatMessage(RecipientInterface $recipient, string $transport = null): ?ChatMessage;
}
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Notifier\Notification;

use Symfony\Component\Notifier\Message\EmailMessage;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\EmailRecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -21,5 +21,5 @@
*/
interface EmailNotificationInterface
{
public function asEmailMessage(Recipient $recipient, string $transport = null): ?EmailMessage;
public function asEmailMessage(EmailRecipientInterface $recipient, string $transport = null): ?EmailMessage;
}
4 changes: 2 additions & 2 deletions src/Symfony/Component/Notifier/Notification/Notification.php
Expand Up @@ -13,7 +13,7 @@

use Psr\Log\LogLevel;
use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand Down Expand Up @@ -158,7 +158,7 @@ public function channels(array $channels): self
return $this;
}

public function getChannels(Recipient $recipient): array
public function getChannels(RecipientInterface $recipient): array
{
return $this->channels;
}
Expand Down
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Notifier\Notification;

use Symfony\Component\Notifier\Message\SmsMessage;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\SmsRecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -21,5 +21,5 @@
*/
interface SmsNotificationInterface
{
public function asSmsMessage(Recipient $recipient, string $transport = null): ?SmsMessage;
public function asSmsMessage(SmsRecipientInterface $recipient, string $transport = null): ?SmsMessage;
}
11 changes: 5 additions & 6 deletions src/Symfony/Component/Notifier/Notifier.php
Expand Up @@ -17,9 +17,8 @@
use Symfony\Component\Notifier\Channel\ChannelPolicyInterface;
use Symfony\Component\Notifier\Exception\LogicException;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\AdminRecipient;
use Symfony\Component\Notifier\Recipient\NoRecipient;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -41,7 +40,7 @@ public function __construct($channels, ChannelPolicyInterface $policy = null)
$this->policy = $policy;
}

public function send(Notification $notification, Recipient ...$recipients): void
public function send(Notification $notification, RecipientInterface ...$recipients): void
{
if (!$recipients) {
$recipients = [new NoRecipient()];
Expand All @@ -54,20 +53,20 @@ public function send(Notification $notification, Recipient ...$recipients): void
}
}

public function addAdminRecipient(AdminRecipient $recipient): void
public function addAdminRecipient(RecipientInterface $recipient): void
{
$this->adminRecipients[] = $recipient;
}

/**
* @return AdminRecipient[]
* @return RecipientInterface[]
*/
public function getAdminRecipients(): array
{
return $this->adminRecipients;
}

private function getChannels(Notification $notification, Recipient $recipient): iterable
private function getChannels(Notification $notification, RecipientInterface $recipient): iterable
{
$channels = $notification->getChannels($recipient);
if (!$channels) {
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Notifier/NotifierInterface.php
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Notifier;

use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\Recipient;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

/**
* Interface for the Notifier system.
Expand All @@ -23,5 +23,5 @@
*/
interface NotifierInterface
{
public function send(Notification $notification, Recipient ...$recipients): void;
public function send(Notification $notification, RecipientInterface ...$recipients): void;
}

0 comments on commit 7520884

Please sign in to comment.