laravel-notification-channels / channels

Submit suggestions & pull requests here for new notification channels
https://laravel-notification-channels.com/
200 stars 186 forks source link

Channel interfaces/templates for common patterns #69

Open dwightwatson opened 4 years ago

dwightwatson commented 4 years ago

I wonder if there is a good opportunity to introduce some base channel, or channel interfaces to be used across the board in order to drive consistency.

In looking after apn, facebook-poster and the now deprecated fcm I've come across a few recurring problems that I would love to have a consistent answer for.

atymic commented 4 years ago

I've moved the issue to the channels repo.

I think having guidelines around how the channels work is a great idea.

I'll have a look into this more when I have some spare time :)

irazasyed commented 4 years ago

+1 for this. There were previous discussions regarding handling failed notifications prior to the introduction of the event in Laravel and some chose to use exceptions and some used event method. So if we use multiple channels, there's always inconsistency with failed notifications due to the inconsistency. Perhaps, we should go with the official event method and have all the channels updated besides other changes.

atymic commented 4 years ago

If anyone is interested in a PR feel free to do so and I'll review.

dwightwatson commented 4 years ago

Laravel's Nexmo channel now supports a via method to pass in a custom Nexmo client, which is set on the NexmoMessage. This could set the precedent for customising configurations.

Tend to agree that we should also adopt the provided event rather than throw an exception, however it does have a downside. A failed notification on a queued job should probably be re-attempted - throwing an exception would make that happen, and I'm not sure the event is used to re-attempt.