Skip to content

Commit

Permalink
Granular notification queue jobs (#15681)
Browse files Browse the repository at this point in the history
This tweaks notification queued job to queue a job for each notifiable
/ channel combination. The reason for this is with the prior setup of
using a single job, if one channel fails to send and the job retries
all channels you could deliver duplicate/N number of notifications on a
given channel, possibly costing you money if the channel charges money
per notification.
  • Loading branch information
taylorotwell authored Sep 29, 2016
1 parent 960425a commit 4a4e2d5
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 11 deletions.
26 changes: 18 additions & 8 deletions src/Illuminate/Notifications/ChannelManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function send($notifiables, $notification)
* @param mixed $notification
* @return void
*/
public function sendNow($notifiables, $notification)
public function sendNow($notifiables, $notification, array $channels = null)
{
if (! $notifiables instanceof Collection && ! is_array($notifiables)) {
$notifiables = [$notifiables];
Expand All @@ -61,7 +61,7 @@ public function sendNow($notifiables, $notification)
foreach ($notifiables as $notifiable) {
$notificationId = (string) Uuid::uuid4();

$channels = $notification->via($notifiable);
$channels = $channels ?: $notification->via($notifiable);

if (empty($channels)) {
continue;
Expand Down Expand Up @@ -109,12 +109,22 @@ protected function shouldSendNotification($notifiable, $notification, $channel)
*/
protected function queueNotification($notifiables, $notification)
{
$this->app->make(Bus::class)->dispatch(
(new SendQueuedNotifications($notifiables, $notification))
->onConnection($notification->connection)
->onQueue($notification->queue)
->delay($notification->delay)
);
if (! $notifiables instanceof Collection && ! is_array($notifiables)) {
$notifiables = [$notifiables];
}

$bus = $this->app->make(Bus::class);

foreach ($notifiables as $notifiable) {
foreach ($notification->via($notifiable) as $channel) {
$bus->dispatch(
(new SendQueuedNotifications($notifiables, $notification, [$channel]))
->onConnection($notification->connection)
->onQueue($notification->queue)
->delay($notification->delay)
);
}
}
}

/**
Expand Down
13 changes: 11 additions & 2 deletions src/Illuminate/Notifications/SendQueuedNotifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,24 @@ class SendQueuedNotifications implements ShouldQueue
*/
protected $notification;

/**
* All of the channels to send the notification too.
*
* @var array
*/
protected $channels = null;

/**
* Create a new job instance.
*
* @param \Illuminate\Support\Collection $notifiables
* @param \Illuminate\Notifications\Notification $notification
* @param array $channels
* @return void
*/
public function __construct($notifiables, $notification)
public function __construct($notifiables, $notification, array $channels = null)
{
$this->channels = $channels;
$this->notifiables = $notifiables;
$this->notification = $notification;
}
Expand All @@ -45,6 +54,6 @@ public function __construct($notifiables, $notification)
*/
public function handle(ChannelManager $manager)
{
$manager->sendNow($this->notifiables, $this->notification);
$manager->sendNow($this->notifiables, $this->notification, $this->channels);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public function testNotificationsCanBeSent()
{
$job = new SendQueuedNotifications('notifiables', 'notification');
$manager = Mockery::mock('Illuminate\Notifications\ChannelManager');
$manager->shouldReceive('sendNow')->once()->with('notifiables', 'notification');
$manager->shouldReceive('sendNow')->once()->with('notifiables', 'notification', null);
$job->handle($manager);
}
}

0 comments on commit 4a4e2d5

Please sign in to comment.