Having a concrete Notifiable class is too limiting #1835
Unanswered
alberto-bottarini
asked this question in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
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