Update to Pusher Beams #42
Conversation
There was a problem hiding this comment.
Thanks for the PR @jessarcher 🇦🇺
Maybe worth updating the package title/readme top as well?
We'll tag the release v2.
Let me know if you're keen to maintain this package as well, as I don't actually use pusher (I just stepped in as the old maintainer is no longer available) :)
src/PusherBeams.php
Outdated
|
|
||
| class PusherChannel | ||
| class PusherBeams |
There was a problem hiding this comment.
Any thoughts on naming this PusherBeamsChannel instead of PusherChannel?
There was a problem hiding this comment.
That's a good point! I was so caught up in switching from Pusher Channels to Pusher Beams that I forgot this is a Notification Channel, so the channel name is still valid. Now that I think of it, this probably wasn't even named after Pusher Channels in the first place!
I'll rename this tomorrow when I'm back on my computer, and maybe add a note to the readme to hopefully save others the confusion between the different terminology of Pusher and Laravel.
There was a problem hiding this comment.
On reflection after realising that this wasn't named after the Pusher "Channels" service, I decided to just rename it back to PusherChannel to keep symmetry with PusherMessage and other notification channels that I've since looked at. Hopefully the note added to the README makes it clear that the "Channel" refers to the notification channel, and not the Pusher Channels service.
|
Good call on updating the readme title, i'll sort it out tomorrow :) I'm happy to take on maintenance, although I am only using Pusher on a client project so I probably won't be using it long term either. |
|
No worries. I'll add you as a maintainer :) |
|
It should be noted that I haven't fully been able to confirm this works with Android/FCM as my current project is iOS-only for the time being. I did test sending the FCM payload and it was accepted by Pusher, so hopefully that's a good indication. |
|
Thanks @jessarcher I'll merge now and tag the new version :) Thanks for your work on this. |
|
Thanks for you help in getting this merged and tagged so quickly @atymic! My client will be very happy :) |
Fixes #30, #38, and #40.
Implements the same changes as #41 and #39, as well as continues what @wanghanlin started in #27 by updating the plugin to use the Pusher Beams SDK, which seems to have replaced the notification API of Pusher Channels.
This is a breaking change because I have renamed
PusherChanneltoPusherBeamsto reflect the change in service, and because the configuration requirements are now completely different. Pusher Beams doesn't use the same configuration as Pusher Channels, and it didn't feel appropriate to store these new credentials inconfig/broadcast.phpbecause they are unrelated to Laravel's event broadcasting.Let me know if there's anything else that needs to be done on this @atymic!