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

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

Conversation

mgsmus
Copy link
Contributor

@mgsmus mgsmus commented Jun 9, 2022

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.

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

please merge this PR.

@mgsmus
Copy link
Contributor Author

mgsmus commented Jun 10, 2022

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

@veneliniliev
Copy link

@atymic please merge this PR

@ZedanLab
Copy link

@atymic kindly merge this PR

@veneliniliev
Copy link

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

@hrep-john
Copy link

up

1 similar comment
@gabrieldebem
Copy link

up

@mtawil
Copy link

mtawil commented Jun 30, 2022

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

@masterix21
Copy link

up

@NViktors
Copy link

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

@masterix21
Copy link

@atymic, could you take a look here? Thanks

@Saifallak
Copy link

Saifallak commented Jul 26, 2022

up @atymic

@la5digital
Copy link

👍

@Rignesh-EWW
Copy link

any solutions found for this ?

@veneliniliev
Copy link

any solutions found for this ?

#116 (comment)

@Rignesh-EWW
Copy link

Rignesh-EWW commented Aug 1, 2022

any solutions found for this ?

#116 (comment)

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

@masterix21
Copy link

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

Rignesh-EWW commented Aug 1, 2022

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

again the same issue generates.

@mwllgr
Copy link

mwllgr commented Aug 13, 2022

Any news here?

@iml885203
Copy link

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

atymic commented Aug 22, 2022

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.

@atymic atymic merged commit f252588 into laravel-notification-channels:master Aug 22, 2022
@dwightwatson
Copy link
Collaborator

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

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

Successfully merging this pull request may close these issues.