[6.x] Fix notifications database channel for anonymous notifiables

[driesvints]

This PR prevents on-demand notifications from using the database channel by throwing an informative exception when anyone tries it. At the moment you'd receive a fatal error when trying to resolve the routed notification.

It also will skip sending to the database channel of a notification if the notifiable is anonymous.

Fixes #33408

[illambo]

Hi, I ask you information only to clarify how works on-demand notification because from documentation it does not seem clear to me.
If the via array of the notification is populated with multiple channels (ex. ['slack', 'mail']) using on-demand notification (ex Notification::route('mail', '[email protected]')) is the notification sent only for the requested route channel (in the example email) or through all channels returned by notification via method ?

Thank you very much!

[driesvints]

is the notification sent only for the requested route channel

@illambo yes. I think the example in the docs makes this clear enough because it's listing multiple routes for different channels. If you feel anything should be clarified feel free to attempt a pr to the docs.

[illambo]

Hi, yes the docs seems clear but in the issue #33408 when I set via method to mail and database and try with on-demand notification only for mail the notification was attempted to ship on both channels and not only for mail.
So it seems not to respect "sent only for the requested route channel".
What do you think about it ?

[driesvints]

@illambo that's exactly what this pr solves.

[illambo]

Ok, sorry!

[bobbybouwmann]

@driesvints This is actually breaking our code

Notification::route('database', $user->notifications())
    ->notifyNow(new CardActivationReminder($user, $card);

I now get the The database channel does not support on-demand notifications error back. It obviously happens for anonymous notifications like this as well

Notification::route('messagebird', (string) $phoneNumber)
    ->route('database', auth()->user()->notifications())
    ->notifyNow($notification);

What is the correct approach to also store a notification in the database for this? We use the database notifications to display all send notifications to a user in Nova.

As I understand it, it's not possible to create a database notification anymore for anonymous users. However, this code doesn't create one for an anonymous user, right?

Notification::route('database', auth()->user()->notifications())
    ->notifyNow($notification);

We are allowed to send in the query builder object that points to the current user.

I think we need a fix for this. What do you think?

[driesvints]

Hey @bobbybouwmann. That's odd. I'm wondering why you're using it like that? Seems to me you need:

$user->notifyNow(new CardActivationReminder($user, $card);

The database channel for anonymous notifiables makes no sense since we can't store any record in the database (because we have no context of who the notifiable is (no morph column values).

You posted:

Notification::route('database', $user->notifications())
    ->notifyNow(new CardActivationReminder($user, $card);

But that has no user context when sending the notification. So I'm wondering: does the CardActivationReminder class itself send the notification?

[driesvints]

Just to clarify the above. The problem was that a conditional check needed to be made in the via method:

    public function via($notifiable): array
    {
        if ($notifiable->email) {
            return ['email', 'database'];
        }

        return ['database'];
    }

[bobbybouwmann]

Thanks @driesvints for debugging this together and come up with a solution. We will work on a refactor for it in our codebase 👍