filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
18.86k stars 2.93k forks source link

Cannot send notifications on the boot method of a service provider #4682

Closed realslimsutton closed 2 years ago

realslimsutton commented 2 years ago

Package

filament/notifications

Package Version

2.16.36

Laravel Version

9.36.4

Livewire Version

2.10.7

PHP Version

8.1.11

Problem description

When sending a notification on a boot method in a service provider, the notification is not always displayed for the user. I have confirmed that the notification is in fact pushed to the filament.notifications session array.

One workaround I've found is if I wrap sending the notification with the Filament::serving callback, however this only works when I have the additional admin package installed.

When I go directly to the page it'll display but if it's just a refresh then the notification will not be displayed.

Expected behavior

The notification to always be displayed to the user on the request that triggered the notification.

Steps to reproduce

Add the following code to your boot method inside app/providers/AppServiceProvider.php:

$someCondition = false;
if (!$someCondition) {
    //Filament::serving(static function () {
        Notification::make()
            ->danger()
            ->title('Error title')
            ->body('Error message')
            ->send();
    //});
}

Run php artisan serve (or use a web-server) and go to the login page (such as http://localhost:8000/login). The notification will be displayed, refresh, and the notification will not be displayed. Uncomment the Filament::serving callback and the notification is always displayed.

Reproduction repository

https://github.com/realslimsutton/filament-notifications-test

Relevant log output

No response

EDIT: Updated to include details that the notification does in fact inconsistently get displayed, but wrapping sending the notification with the Filament::serving callback, makes the notification consistently displayed to the user.

awcodes commented 2 years ago

Would this not be better executed in middleware? It just sounds like a system level thing that is not dependent on filament, as I understand you're explanation. Otherwise it works in the context of filament admin. Boot in service provider is not the right place to execute such logic.

realslimsutton commented 2 years ago

Middleware would be an ideal place but in this instance it's a modular system to separate the code, which boots various service providers. Loading the service providers in the middleware would be too late in the request lifecycle. In the scenario that an error occurs during the boot of a service provider (or module) it'd send a notification. Which is fine as long as they're on the admin panel but if the notification is sent on any page that isn't the admin panel, then it won't be displayed.

awcodes commented 2 years ago

So you need to display a notification without filament, because there is no guarantee, AFAIK, in Laravel to guarantee boot order. So, should the notification be handled at run time or pushed to something like the db notifications? I'm just not sure this is an issue with Filament. I could be wrong though.

realslimsutton commented 2 years ago

From what I could tell from the code, sending a notification just pushes the notification into the session. Then they're loaded when a notification livewire component is loaded, so loading order shouldn't be an issue here imo. I wasn't sure if this was an actual issue or intentional or not when I made the issue but I figure worst case someone could clarify for me.

DB notifications could be a solution for me, but I was just surprised it works inconsistently rather than not at all (and looking from at the code it appears it should at least work).

zepfietje commented 2 years ago

When I go directly to the page it'll display but if it's just a refresh then the notification will not be displayed.

@realslimsutton could you clarify this?

caendesilva commented 2 years ago

DB notifications could be a solution for me, but I was just surprised it works inconsistently rather than not at all (and looking from at the code it appears it should at least work).

Yeah, like awcodes said, I don't think you can rely on the service providers to boot in a certain order. Without knowing for sure, I would guess that the order is random since you are getting unpredictable results. Some Filament provider required by the notification service doesn't always get booted before yours.

Like it's also been said, I don't think a service provider is what you want for this, commonly things like these are put in middlewares. Though from your use case regarding error handling maybe you'd want to use to use email/database notifications instead.

If you are sure using a service provider is what you really want, you could also try registering a callback like this in your register method of a service provider:

$this->app->booted(function () {
    // Do something after the application has booted
});
zepfietje commented 2 years ago

Thanks for commenting, @caendesilva. I wanted to comment something similar, but was confused since @realslimsutton stated the notification was always added to the session (even when not visible on the page). If that's the case, it must be related to something like a page reload / redirect resulting in the notification being removed from the session.

Again, I'm quite sure this is not an issue with Filament.

Feel free to comment if you've found any evidence that it actually is a Filament issue, @realslimsutton.