-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Sending notification to the topic without FCM registration token. #192
Comments
No, not at this stage. Happy to review any ideas or PRs that implement this. |
I had the same problem and fixed by overriding public function send(mixed $notifiable, Notification $notification): ?Collection
{
// Firstly we create the message class
/** @var \NotificationChannels\Fcm\FcmMessage */
$fcmMessage = $notification->toFcm($notifiable);
// If message has a topic then send it directly
if (!is_null($fcmMessage->topic)) {
return collect(($fcmMessage->client ?? $this->client)->send($fcmMessage));
}
// Otherwise collect tokens from the notifiable model
$tokens = Arr::wrap($notifiable->routeNotificationFor('fcm', $notification));
if (empty($tokens)) {
return null;
}
return Collection::make($tokens)
->chunk(self::TOKENS_PER_REQUEST)
->map(fn ($tokens) => ($fcmMessage->client ?? $this->client)->sendMulticast($fcmMessage, $tokens->all()))
->map(fn (MulticastSendReport $report) => $this->checkReportForFailures($notifiable, $notification, $report));
} @dwightwatson Is this behaviour correct? |
I'm personally not familiar with sending notifications to topics without a token. If it's as simple if checking if Like I said above, happy to look at any PRs that introduce this behaviour. I think we should still do |
Checking if We need to notify an indefinite number of users with the bare minimum load on the server. Another way to implement this could be: if I can write a PR, just let me know how do you want to integrate it. |
Would it only ever return a single Or does it make sense to have a separate channel for this - so you use A few people liked your earlier comment - perhaps they can share their 2 cents/use cases so we can work out what a good implementation would look like. |
Hi, I solved this by creating a new channel in the form of the FcmTopicChannel class namespace Our\Project\Namespace\Notifications;
use Illuminate\Notifications\Notification;
use Kreait\Firebase\Exception\MessagingException;
use Kreait\Firebase\Messaging\Message;
use NotificationChannels\Fcm\Exceptions\CouldNotSendNotification;
use NotificationChannels\Fcm\FcmChannel;
/**
* Class FcmTopicChannel
*
* @package Our\Project\Namespace\Notifications
*/
final class FcmTopicChannel extends FcmChannel
{
/**
* @param $notifiable
* @param \Illuminate\Notifications\Notification $notification
*
* @return array
* @throws \Kreait\Firebase\Exception\FirebaseException
* @throws \NotificationChannels\Fcm\Exceptions\CouldNotSendNotification
*/
public function send($notifiable, Notification $notification)
{
$fcmMessage = $notification->toFcm($notifiable);
if (! $fcmMessage instanceof Message) {
throw CouldNotSendNotification::invalidMessage();
}
$responses = [];
try {
$responses[] = $this->messaging()->send($fcmMessage);
} catch (MessagingException $exception) {
$this->failedNotification($notifiable, $notification, $exception);
throw CouldNotSendNotification::serviceRespondedWithAnError($exception);
}
return $responses;
}
} Sending by AnonymousNotifiable resolve(AnonymousNotifiable::class)->notify(new OurCustomTopicNotificationClass($attribute)); |
I believe that for topics or tokens, a single transmission is sufficient. public function send($notifiable, Notification $notification)
{
// Get the message from the notification class
$fcmMessage = $notification->toFcm($notifiable);
if (! $fcmMessage instanceof Message) {
throw CouldNotSendNotification::invalidMessage();
}
// send to topic or device
$topic = $fcmMessage->getTopic();
$token = empty($topic) ? $notifiable->routeNotificationFor('fcm', $notification) : null;
try {
// The target of the transmission must be a topic or a device.'
if (empty($topic) && empty($token))
throw new Exception('The target of the transmission must be a topic or a device.');
$this->fcmProject = null;
if (method_exists($notification, 'fcmProject')) {
$this->fcmProject = $notification->fcmProject($notifiable, $fcmMessage);
}
$responses = [];
if (is_array($token)) {
// Use multicast when there are multiple recipients
$partialTokens = array_chunk($token, self::MAX_TOKEN_PER_REQUEST, false);
foreach ($partialTokens as $tokens) {
$responses[] = $this->sendToFcmMulticast($fcmMessage, $tokens);
}
} else {
$responses[] = $this->sendToFcm($fcmMessage, $token);
}
} catch (MessagingException $exception) {
$this->failedNotification($notifiable, $notification, $exception, $token);
throw CouldNotSendNotification::serviceRespondedWithAnError($exception);
}
return $responses;
} |
Thanks @mrcrg! that saved my day. I did the same, but using composer patches to apply the changes. Therefore I think this should be supported by the channel package itself. |
As I mentioned earlier I'm happy to review any PRs for this functionality. The above example would probably need to be updated to me similar to the current |
Hello,
Is it possible to send a notification to topics only? Currently, when I register
routeNotificationForFcm
in aNotifiable
model and return empty string, it throws an error:The text was updated successfully, but these errors were encountered: