laravel-notification-channels / fcm

Firebase Cloud Messaging (FCM) notifications channel for Laravel
https://laravel-notification-channels.com/
MIT License
495 stars 127 forks source link

Fix for "A message can only have one of the following targets" error #116

Closed mgsmus closed 2 years ago

mgsmus commented 2 years ago

https://github.com/kreait/firebase-php/blob/6.x/src/Firebase/Messaging/CloudMessage.php#L75

It checks array keys but does not check if they are empty or not, so it causes "A message can only have one of the following targets: condition, token, topic, unknown" error.

HAstaShakiba commented 2 years ago

please merge this PR.

mgsmus commented 2 years ago

@HAstaShakiba Meanwhile you can use https://github.com/symplify/vendor-patches to patch the error

veneliniliev commented 2 years ago

@atymic please merge this PR

ZedanLab commented 2 years ago

@atymic kindly merge this PR

veneliniliev commented 2 years ago

temporary resolution of the problem... add "kreait/firebase-php": "6.3.1" to composer.json

hrep-john commented 2 years ago

up

gabrieldebem commented 2 years ago

up

mtawil commented 2 years ago

@atymic Why is this PR taking so long time to merge?

masterix21 commented 2 years ago

up

NViktors commented 2 years ago

For real..? PR with such tiny changes takes forever to merge?

masterix21 commented 2 years ago

@atymic, could you take a look here? Thanks

Saifallak commented 2 years ago

up @atymic

la5digital commented 2 years ago

👍

Rignesh-EWW commented 2 years ago

any solutions found for this ?

veneliniliev commented 2 years ago

any solutions found for this ?

https://github.com/laravel-notification-channels/fcm/pull/116#issuecomment-1156475473

Rignesh-EWW commented 2 years ago

any solutions found for this ?

#116 (comment)

"kreait/firebase-php": "6.3.1" => add composer file but not working create the same issue

masterix21 commented 2 years ago

If the maintainer doesn't merge the PR, you could use your FcmMessage class to extend the original ones from the last version of the package.

namespace App\Notifications\Concerns;

class FcmMessage extends \NotificationChannels\Fcm\FcmMessage
{
    public function toArray(): array
    {
        $data = [
            'name' => $this->getName(),
            'data' => $this->getData(),
            'notification' => ! is_null($this->getNotification()) ? $this->getNotification()->toArray() : null,
            'android' => ! is_null($this->getAndroid()) ? $this->getAndroid()->toArray() : null,
            'webpush' => ! is_null($this->getWebpush()) ? $this->getWebpush()->toArray() : null,
            'apns' => ! is_null($this->getApns()) ? $this->getApns()->toArray() : null,
            'fcm_options' => ! is_null($this->getFcmOptions()) ? $this->getFcmOptions()->toArray() : null,
        ];

        if ($token = $this->getToken()) {
            $data['token'] = $token;
        }

        if ($topic = $this->getTopic()) {
            $data['topic'] = $topic;
        }

        if ($condition = $this->getCondition()) {
            $data['condition'] = $condition;
        }

        return $data;
    }
}

Then, replace your notifications to use your own FcmMessage like so:

class PushNotification extends Notification
{
    // ...

    public function toFcm($notifiable)
    {
        return \App\Notifications\Concerns\FcmMessage::create();
    }

    // ...
}
Rignesh-EWW commented 2 years ago

=> "A message can only have one of the following targets: condition, token, topic, unknown"

again the same issue generates.

mwllgr commented 2 years ago

Any news here?

iml885203 commented 2 years ago

@masterix21 The create function needs to be overridden as well.

namespace App\Notifications\Concerns;

class FcmMessage extends \NotificationChannels\Fcm\FcmMessage
{
    public static function create(): self
    {
        return new self;
    }

    public function toArray(): Array
    {
        $data = [
            'name' => $this->getName(),
            'data' => $this->getData(),
            'notification' => ! is_null($this->getNotification()) ? $this->getNotification()->toArray() : null,
            'android' => ! is_null($this->getAndroid()) ? $this->getAndroid()->toArray() : null,
            'webpush' => ! is_null($this->getWebpush()) ? $this->getWebpush()->toArray() : null,
            'apns' => ! is_null($this->getApns()) ? $this->getApns()->toArray() : null,
            'fcm_options' => ! is_null($this->getFcmOptions()) ? $this->getFcmOptions()->toArray() : null,
        ];

        if ($token = $this->getToken()) {
            $data['token'] = $token;
        }

        if ($topic = $this->getTopic()) {
            $data['topic'] = $topic;
        }

        if ($condition = $this->getCondition()) {
            $data['condition'] = $condition;
        }

        return $data;
    }
}
class PushNotification extends Notification
{
    // ...

    public function toFcm($notifiable)
    {
        return \App\Notifications\Concerns\FcmMessage::create();
    }

    // ...
}
atymic commented 2 years ago

Apologies for the delay, I'm not the maintainer of this specific package and had notifications off. I'll merge shortly, if anyone is interested in joining as maintainer please email me.

dwightwatson commented 2 years ago

@atymic - happy to help out with maintenance. I already look after the APN and Facebook channels as well.