-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Channel interfaces/templates for common patterns #69
Comments
I've moved the issue to the 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 :) |
+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. |
If anyone is interested in a PR feel free to do so and I'll review. |
Laravel's Nexmo channel now supports a 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. |
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 deprecatedfcm
I've come across a few recurring problems that I would love to have a consistent answer for.How do we handle a channel having multiple configurations? For example, sending notifications to different iOS apps, Android apps or Facebook accounts.
How should we handle a failed send? iOS throws an exception, but there is also event that's provided in Laravel.
The text was updated successfully, but these errors were encountered: