benwilkins / laravel-fcm-notification

Laravel FCM (Firebase Cloud Messaging) Notification Channel
MIT License
213 stars 91 forks source link

Split recipients array into chunks to respect FCM limits #50

Closed chimit closed 5 years ago

chimit commented 5 years ago

PR to address this issue https://github.com/benwilkins/laravel-fcm-notification/issues/48

chimit commented 5 years ago

There is one problem with this approach. In case of one request the send() method returns FCM response body. For example,

{"multicast_id":7512246268530429272,"success":0,"failure":2,"canonical_ids":0,"results":[{"error":"InvalidRegistration"},{"error":"InvalidRegistration"}]}

But in case of multiple requests this PR returns only the latest response.

The above request can be split into two separate and the responses would be:

{"multicast_id":7306578091349811732,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}
{"multicast_id":4783702789326450040,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}

We could merge multiple response objects into one by adding success, failure, canonical_ids numbers and merging results arrays, but there is also a multicast_id which is unique and can't be just transformed into array. At least if we care about backward compatibility.

What do you guys think?

benwilkins commented 5 years ago

Seems like the best option would be to release a major version (due to breaking changes) that returned an array of responses:

[
    {"multicast_id":7306578091349811732,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]},
    {"multicast_id":4783702789326450040,"success":0,"failure":1,"canonical_ids":0,"results":[{"error":"InvalidRegistration"}]}
]

You could then loop over the responses, even if there's only one, and get whatever you need from them.

chimit commented 5 years ago

@benwilkins, please, check.