You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I really like this plugin and the idea to have notifications that warn you about what happened but I think that the implementation of this idea is too strict if your application is already using the Laravel notification framework.
My scenario: an application that already sends notifications, using User as standard notifiable class, with some routeNotificationFor methods: a standard pattern Laravel-way.
I would use all of these logics to send the backup notification, picking admin users as recipient of these alerts and using their standard notification channels.
Having a concrete Notifiable class referenced in the code does not allow me to do, forcing me to "reinvent the wheel" only for backup alerts.
In my scenario, I would like to have a way to picking the recipient users performing a query on User model and returning a collection of these and passing them to laravel-backup EventHandler.
A proposal to solve this issue, keeping the actual logic, could be this:
instead of having a fixed Notifiable class in the config ('notifiable' => \Spatie\Backup\Notifications\Notifiable::class) we could have a notifiableResolver class, that could return a collection of classes that have the Illuminate\Notifications\Notifiable. This resolver could be set, as default, to a standard resolver that would return a single \Spatie\Backup\Notifications\Notifiable class, keeping the behaviour the same as today
upgrade the determineNotifiable method in order to add the resolver to get the list of recipient
remove/improve the return type of protected function determineNotifiable(): Notifiable that limits the configurability of recipient
looping through the returned collection in order to send a notification for each items
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
I really like this plugin and the idea to have notifications that warn you about what happened but I think that the implementation of this idea is too strict if your application is already using the Laravel notification framework.
My scenario: an application that already sends notifications, using User as standard notifiable class, with some routeNotificationFor methods: a standard pattern Laravel-way.
I would use all of these logics to send the backup notification, picking admin users as recipient of these alerts and using their standard notification channels.
Having a concrete Notifiable class referenced in the code does not allow me to do, forcing me to "reinvent the wheel" only for backup alerts.
In my scenario, I would like to have a way to picking the recipient users performing a query on User model and returning a collection of these and passing them to laravel-backup EventHandler.
A proposal to solve this issue, keeping the actual logic, could be this:
'notifiable' => \Spatie\Backup\Notifications\Notifiable::class
) we could have anotifiableResolver
class, that could return a collection of classes that have the Illuminate\Notifications\Notifiable. This resolver could be set, as default, to a standard resolver that would return a single \Spatie\Backup\Notifications\Notifiable class, keeping the behaviour the same as todaydetermineNotifiable
method in order to add the resolver to get the list of recipientprotected function determineNotifiable(): Notifiable
that limits the configurability of recipientWhat do you think?
Beta Was this translation helpful? Give feedback.
All reactions