Skip to content
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

Open
dwightwatson opened this issue Oct 4, 2019 · 4 comments
Open

Channel interfaces/templates for common patterns #69

dwightwatson opened this issue Oct 4, 2019 · 4 comments

Comments

@dwightwatson
Copy link

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 atymic transferred this issue from laravel-notification-channels/laravel-notification-channels.github.io Oct 22, 2019
@atymic
Copy link
Member

atymic commented Oct 22, 2019

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
Copy link
Member

+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
Copy link
Member

atymic commented Oct 30, 2019

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

@dwightwatson
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants