Z3d0X / filament-logger

Extensible activity logger for filament that works out-of-the-box.
MIT License
276 stars 40 forks source link

fix auto discover events in Laravel 11 #97

Closed julio-cavallari closed 4 months ago

julio-cavallari commented 4 months ago

This PR aims to override the original shouldDiscoverEvents method in FilamentLoggerServiceProvider to return the correct value, since get_class($this) === __CLASS__⁠ always returns false when $this is an extension of __CLASS__, causing Laravel to not load events automatically

Z3d0X commented 4 months ago

This would be a breaking change for 10.x earlier versions of 11.x.

Could you please explain more, what issue is being solved here, possibly with an example?

julio-cavallari commented 4 months ago

In Laravel 11, there is no longer an EventServiceProvider available by default so that the user can activate or deactivate the automatic discovery of events by returning true in the shouldDiscoverEvents method code

This is now done by calling the withEvents method directly from the ApplicationBuilder in bootstrap/app.php code (2)

But the check done in the original Provider that your provider extends will always return false because of the way it is done in the original provider

Executing the method through your provider, the expression get_class($this) === __CLASS__ will always be false, and events will never be discovered automatically in Laravel 11 code (1)

Z3d0X commented 4 months ago

@julio-cavallari thanks for the detailed explaination.

I'd like to keep this simple. We could get rid of FilamentLoggerEventServiceProvider so EventServiceProvider is no longer extended by this package.

Use Event:listen() in FilamentLoggerServiceProvider

julio-cavallari commented 4 months ago

Done on 4143416 commit @Z3d0X

Z3d0X commented 4 months ago

replaced by #99