Collection of tokens causes exception

[kohenkatz]

I was trying to do something similar to https://github.com/laravel-notification-channels/fcm/issues/11#issuecomment-592995457 to get user tokens from the database. Here's what my version looks like (in my User model):

    public function routeNotificationForFcm($notification = null)
    {
        return $this->phones()
            ->where('device_os', 'ANDROID')
            ->whereNotNull('device_push_token')
            ->pluck('device_push_token');
    }

I got this error in my logs when I tried to send:

production.ERROR: The registration token is not a valid FCM registration token {"exception":"[object] (NotificationChannels\\Fcm\\Exceptions\\CouldNotSendNotification(code: 0): The registration token is not a valid FCM registration token at /srv/app/vendor/laravel-notification-channels/fcm/src/Exceptions/CouldNotSendNotification.php:13)

Looking at the code in this repository, I see this line, which checks if the $token is an array:

https://github.com/laravel-notification-channels/fcm/blob/3607d38942a671ff33874cfe1a7885c6a5505b6d/src/FcmChannel.php#L73

This check fails for my code because pluck() returns a Collection.

If I add ->toArray() after pluck(...), then the notifications are sent as expected.

Before version 2.0 was released, the check for whether there was a single token or multiple tokens was done like this:

https://github.com/laravel-notification-channels/fcm/blob/1739984d01ef8b9e87d8966145f1d9f6b9f7981f/src/FcmChannel.php#L51

This worked because count() works on any class that implements the Countable interface, while is_array() only checks for actual arrays.

Can we go back to the old way that supports Collections directly?

[dwightwatson]

It's a shame that was failing silently.

Yes - we should support single tokens, arrays and Collections, I think that would be the expected behaviour.

Happy to review a PR with tests that fixes this.

[kohenkatz]

It's a shame that was failing silently.

Yep. The only way I found it was local testing on my machine with the SYNC queue driver, I was getting 500 responses.

I will see if I can do a PR today.

[dwightwatson]

Okay - turns out we don't have a test suite on this package, and FcmChannel is actually a bit of a mess of dependencies that it isn't the easiest thing to test. Have a review of this PR and see if it will resolve your use-case. I'd like to drop supporting a heap of older PHPs and Laravel: https://github.com/laravel-notification-channels/fcm/pull/130