[Notifier] Custom notifications with dependencies

[Padam87]

Hi,

This is my first foray into the notifier component, and I'm a bit conflicted. On one hand, this is exactly what I need. Notifications on multiple channels, and many integrations out of the box.

On the other hand it is an uphill battle, let me show what I would like to achieve, hopefully someone can help me with coming up with something.

namespace App\Notifier\Notification\Task;

use App\Entity\Embeddables\EntityReference;
use App\Entity\Task;
use App\Entity\User;
use App\Message\DoctrineNotificationMessage;
use App\Notifier\Notification\DoctrineNotificationInterface;
use App\Notifier\Notification\Notifications;
use App\Notifier\Recipient\UserRecipientInterface;
use Symfony\Component\Notifier\Bridge\Slack\Block\SlackActionsBlock;
use Symfony\Component\Notifier\Bridge\Slack\Block\SlackDividerBlock;
use Symfony\Component\Notifier\Bridge\Slack\SlackOptions;
use Symfony\Component\Notifier\Message\ChatMessage;
use Symfony\Component\Notifier\Notification\ChatNotificationInterface;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

class TaskCreatedNotification extends Notification implements DoctrineNotificationInterface, ChatNotificationInterface
{
    private Task $task;

    public function __construct(Task $task)
    {
        $this->task = $task;
    }

    public function asNotificationMessage(UserRecipientInterface $recipient, string $transport = null): ?DoctrineNotificationMessage
    {
        $message = new DoctrineNotificationMessage();

        $message
            ->setUser(EntityReference::create(User::class, $recipient->getId()))
            ->setType(Notifications::TYPE_TASK_CREATED)
            ->setTarget(new EntityReference($this->task))
        ;

        return $message;
    }

    public function asChatMessage(RecipientInterface $recipient, string $transport = null): ?ChatMessage
    {
        $subject = sprintf('...'); // I would like to access the translator here

        $message = new ChatMessage($subject);

        $actionBlock = (new SlackActionsBlock())
            ->button(
                'Open task', // translator again
                'https://...', // I would like to generate an url here with the router
                'primary'
            )
        ;

        $slackOptions = (new SlackOptions())
            ->block(new SlackDividerBlock())
            ->block($actionBlock)
        ;

        $message->options($slackOptions);

        return $message;
    }

    public function getChannels(RecipientInterface $recipient): array
    {
        return ['doctrine', 'chat/slack'];
    }

So as you could probably guess, the client wanted an internal task system for their production management.

The doctrine channel is just a custom channel I wrote to save a notification entity, not really important here.

But when it comes to the slack integration I would like to rely on dependencies, such as the router and the translator, and who knows what else down the line.

At first I thought these interfaces are really amazing, but now I find myself lured into a bit of a trap :)

What are the community's thoughts on this one? What would you do?

Inject each manually?
Factory (wouldn't work well IMO, but no better idea for now)?
Something better? (I hope so :))

Thaks,
Adam

[norkunas]

We just added another listener in same way as for email messages, and then use twig to render slack messages, so in twig you can access to translator, generate routes and more :)

[michaoj]

Another solution would be o use a service locator in your Task class. This way you can inject the desired interfaces only when needed

See https://symfony.com/doc/current/service_container/service_subscribers_locators.html#defining-a-service-locator for an example on how to implement it

[Padam87]

Neither ansswer is really on point unfortunately.

@michaoj Task is a VO, no place for services.

@norkunas That would make the notifier component kind of pointless though... My goal here is to understand how the notifier is supposed to help us :)

[norkunas]

Notification is also VO.. Use custom service locator, and then inject into notification then..

[Padam87]

@norkunas You are right about Notification being a VO, and that is what makes this interface based message generation is too limited.

I really don't get how is this design supposed to work. At that point you don't have access to services, and for example if you do the translation before creating a notification, the interface based customizations are not helping...

I think I will go with a custom solution instead.

[norkunas]

Yes, I also had to come up with a custom solution to workaround when I need to generate route links in slack messages..

[Padam87]

In case someone finds this and needs something similar, here is what I'm endend up doing.

<?php

namespace App\Notifier\Notification;

use App\Notifier\Message\DoctrineNotificationMessage;
use App\Notifier\Recipient\UserRecipientInterface;
use Symfony\Component\Notifier\Message\ChatMessage;
use Symfony\Component\Notifier\Message\EmailMessage;
use Symfony\Component\Notifier\Message\MessageInterface;
use Symfony\Component\Notifier\Notification\ChatNotificationInterface;
use Symfony\Component\Notifier\Notification\EmailNotificationInterface;
use Symfony\Component\Notifier\Notification\Notification as BaseNotification;
use Symfony\Component\Notifier\Recipient\EmailRecipientInterface;
use Symfony\Component\Notifier\Recipient\RecipientInterface;

class Notification extends BaseNotification implements ChatNotificationInterface, DoctrineNotificationInterface, EmailNotificationInterface
{
    private const CHANNEL_MAP = [
        ChatMessage::class => 'chat/slack',
        DoctrineNotificationMessage::class => 'doctrine',
        EmailMessage::class => 'email',
    ];

    private array $messages = [
        ChatMessage::class => null,
        DoctrineNotificationMessage::class => null,
        EmailMessage::class => null,
    ];

    public function __construct(MessageInterface ...$messages)
    {
        foreach ($messages as $message) {
            $class = get_class($message);

            if (!array_key_exists($class, $this->messages)) {
                throw new \LogicException(sprintf(
                    'This Notificaiton currently only supports %s messages, %s given.',
                    implode(', ', array_keys($this->messages)),
                    $class
                ));
            }

            $this->messages[$class] = $message;
        }
    }

    public function asChatMessage(RecipientInterface $recipient, string $transport = null): ?ChatMessage
    {
        return $this->messages[ChatMessage::class];
    }

    public function asNotificationMessage(UserRecipientInterface $recipient, string $transport = null): ?DoctrineNotificationMessage
    {
        return $this->messages[DoctrineNotificationMessage::class];
    }

    public function asEmailMessage(EmailRecipientInterface $recipient, string $transport = null): ?EmailMessage
    {
        return $this->messages[EmailMessage::class];
    }

    public function getChannels(RecipientInterface $recipient): array
    {
        $channels = [];

        foreach ($this->messages as $message) {
            if (null === $message) {
                continue;
            }

            $channels[] = self::CHANNEL_MAP[get_class($message)];
        }

        return $channels;
    }
}

This allows me to assemble messages before the notification is created in dedicated generators, where I have access to the DIC.

Calling it

$this->notifier->send(new Notification(...$this->generator->__invoke($task)), $recipient);

And the generator:

class TaskCreatedNotificationGenerator implements GeneratorInterface
{
    private TranslatorInterface $translator;
    private UrlGeneratorInterface $router;

    public function __construct(TranslatorInterface $translator, UrlGeneratorInterface $router)
    {
        $this->translator = $translator;
        $this->router = $router;
    }

    public function __invoke(Task $task): \Generator
    {
        yield $this->doctrine($task);
        yield $this->slack($task);
    }

    private function doctrine(Task $task): ?DoctrineNotificationMessage
    {
        // ...
    }

    private function slack(Task $task): ?ChatMessage
    {
        // ...
    }
}

[janklan]

Same issue here. It's a good idea to make the Notification a object simple DTO, but it does fall short when it needs to access anything else than itself.

I briefly considered just providing the translator and URL generator dependencies myself to each notification (I can autowire them and then pass them on), but I rejected that idea shortly after.

I ended up with an autowired factory service that does the repetitive work for me with just a few minor downsides (see below the code):

<?php

namespace App\Notifier;

use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;

class NotificationFactory
{
    public function __construct(
        private readonly TranslatorInterface $translator,
        private readonly UrlGeneratorInterface $urlGenerator
    ) {
        // Void
    }

    /**
     * @template T of Notification
     * @psalm-param class-string<T> $className
     *
     * @return T
     */
    public function build(string $className, ...$rest)
    {
        $notification = new $className(...$rest);

        // Sanity check - the above is way too powerful
        if (!$notification instanceof Notification) {
            throw new \InvalidArgumentException('This factory can only be used to build Notifications.');
        }

        if ($notification instanceof TranslatedNotificationInterface) {
            $notification->setTranslator($this->translator);
        }

        if ($notification instanceof UrlGeneratingNotificationInterface) {
            $notification->setUrlGenerator($this->urlGenerator);
        }

        return $notification;
    }
}

Together with two interfaces

<?php

namespace App\Notifier;

use Symfony\Contracts\Translation\TranslatorInterface;

interface TranslatedNotificationInterface
{
    public function setTranslator(TranslatorInterface $translator): void;
}

<?php 

namespace App\Notifier;

use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

interface UrlGeneratingNotificationInterface
{
    public function setUrlGenerator(UrlGeneratorInterface $urlGenerator): void;
}

And two traits

<?php

namespace App\Notifier\Notification;

use Symfony\Contracts\Translation\TranslatableInterface;
use Symfony\Contracts\Translation\TranslatorInterface;

trait TranslatedNotificationTrait
{
    private readonly TranslatorInterface $translator;

    public function setTranslator(TranslatorInterface $translator): void
    {
        $this->translator = $translator;
    }

    public function content(string|TranslatableInterface $content): static
    {
        return parent::content(is_string($content) ? $content : $content->trans($this->translator));
    }

    public function subject(string|TranslatableInterface $subject): static
    {
        return parent::subject(is_string($subject) ? $subject : $subject->trans($this->translator));
    }
}

<?php

namespace App\Notifier\Notification;

use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

trait UrlGeneratingNotificationTrait
{
    private readonly UrlGeneratorInterface $urlGenerator;

    public function setUrlGenerator(UrlGeneratorInterface $urlGenerator): void
    {
        $this->urlGenerator = $urlGenerator;
    }
}

Now I'm able to build a custom Notification, for example:

<?php

namespace App\Notifier\Notification;

use App\Entity\OrganisationUser;
use App\Notifier\NotificationEmail;
use App\Notifier\TranslatedNotificationInterface;
use App\Notifier\UrlGeneratingNotificationInterface;
use Symfony\Component\Mime\Address;
use Symfony\Component\Notifier\Message\EmailMessage;
use Symfony\Component\Notifier\Notification\EmailNotificationInterface;
use Symfony\Component\Notifier\Notification\Notification;
use Symfony\Component\Notifier\Recipient\EmailRecipientInterface;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Translation\TranslatableMessage;

class OrganisationInviteNotification extends Notification implements EmailNotificationInterface, TranslatedNotificationInterface, UrlGeneratingNotificationInterface
{
    use TranslatedNotificationTrait;
    use UrlGeneratingNotificationTrait;

    private OrganisationUser $organisationUser;

    public function setOrganisationUser(OrganisationUser $organisationUser): self
    {
        $this->organisationUser = $organisationUser;

        $this->subject(new TranslatableMessage('notification.organisation_invite.subject', [
            'organisation' => $this->organisationUser->getOrganisation(),
        ]));

        $this->content(new TranslatableMessage('notification.organisation_invite.content', [
            'organisation' => $this->organisationUser->getOrganisation(),
            'invitedBy' => $this->organisationUser->getInvitedBy() ?: 'colleague',
        ]));

        return $this;
    }

    public function asEmailMessage(EmailRecipientInterface $recipient, string $transport = null): ?EmailMessage
    {
        $email = NotificationEmail::asPublicEmail()
            ->to($recipient->getEmail())
            ->subject($this->getSubject())
            ->content($this->getContent())
            ->action(
                (new TranslatableMessage('notification.organisation_invite.respond'))->trans($this->translator),
                $this->urlGenerator->generate('security_login', referenceType: UrlGeneratorInterface::ABSOLUTE_URL)
            )
        ;

        if ($this->organisationUser->getInvitedBy()) {
            $email->addFrom(new Address(
                $this->organisationUser->getInvitedBy()->getEmail(),
                $this->organisationUser->getInvitedBy()
            ));
        }

        return new EmailMessage($email);
    }
}

And I can get hold of that notification with semi-autowired translation about like this:

<?php

// pseudo code

class SomeController {
    public function someAction(NotificationFactory $notificationFactory, NotifierInterface $notifier) {
        $this->notifier->send(
            $this->notificationFactory->build(OrganisationInviteNotification::class)->setOrganisationUser($organisationUser),
            new Recipient($organisationUser),
        );
    }
}

---

It works OK. The only downside is that because the translator and URL generator are not passed to the constructor directly, I can't set the content and subject in the constructor - the services become available after the constructor has finished.

The second downside is that I'm now setting readonly properties outside of a constructor, which I think is not the best practice.

Previously I was passing the OrganisationUser object directly to my notification's constructor and was setting all the things from there. The problem with this approach is that I expect various notifications to have various constructor arguments, which makes the factory approach a bit wobbly - do I always inject both dependencies? How do I provide the custom arguments? It become too complicated, so I ended up simply not using the constructor for anything, and adding a dedicated setter for the payload needed to generate the notification. Sending of the notification will fail if I don't call this setter, but I think it's quite close to having to call $notification->subject()->content() manually anyway, so I'm OK with it. Using fluent interface in the setter makes it effortless.

The Notifier component is heavily opinionated, which is out of the ordinary for Symfony, but let's roll with it. It's a good idea in a good direction.

I hope this helps someone.

[janklan]

I tripped over the same problem a year later. This time I was trying to pass a TranslatableMessage to e-mail subject only to have it coerced into an untranslated string.

[florentdestremau]

Hi, I have the very same issue (email subject needs translation) and I'm not really convinced with the solution (although I know it works). Did you have any better suggestions in the meantime ?

[janklan]

Nope, but feel free to share yours when you find it.

[florentdestremau]

for now i went the pragmatic way and use the translator on the spot when creating the notification so that the subject is already translated. It feels a bit heavy usage but it's the simplest way yet. In my case the notification is created in only one spot (+ test) so it's very manageable