[9.x] Added ability to set delay per channel based on notifiable instance

[ehamrin]

We have a lot of users who don't want to receive Notification emails during the night, and some that does.
After some digging this is a solution to bypass the current behaviour:

public function via($notifiable)
{
	$this->delay = [
		'mail' => $notifiable->likes_uninterrupted_sleep ? $notifiable->nextWakeUpTime() : null
	];

       return ['mail', 'database'];
}

Trying to set a delay based on a notifiable instance currently required the delay to be set in $notification->via() as it is the last method called where the notifiable is passed through. It is however not optimal as it is set on the $original instance in NotificationSender::186, so it must be overwritten in every via-call on every notifiable.

My proposal is a new method call withDelay that accepts the notifiable and channel as arguments which provides:

public function withDelay($notifiable, $channel)
{
	return match ($channel) {
		'mail' => $notifiable->likes_uninterrupted_sleep ? $notifiable->nextWakeUpTime() : null,
		default => null,
	};


}

//OR

public function withDelay($notifiable)
{
	return [
		'mail' => $notifiable->likes_uninterrupted_sleep ? $notifiable->nextWakeUpTime() : null
	];
}

To me there are only upsides, if there are database queries performed or heavy logic to resolve the delay it's only calculated once if the developer choose to match against the channel name provided. The code looks cleaner as well, the via method doesn't become cluttered with delay data.

It does not break the existing implementation by setting the delay prop.
Only breaking change would be if someone has implemented a withDelay function in a notification to solve something else.

[taylorotwell]

Was there a reason you named the method withDelay instead of just delay?

[ehamrin]

@taylorotwell Illuminate\Bus\Queueable already defines delay as a fluent setter. Open for suggestions on the method name, I thought delay/withDelay() would be similiar to queue/viaQueues()

[robert-sykes]

@taylorotwell Illuminate\Bus\Queueable already defines delay as a fluent setter. Open for suggestions on the method name, I thought delay/withDelay() would be similiar to queue/viaQueues()

I think delay reads easier and a common interface i.e. similar to Queueable fluent setter might be a good idea. 🤔

[ehamrin]

The reason I mentioned Illuminate\Bus\Queueable is that artisan make:notification {name} command uses the Illuminate\Bus\Queueable-trait per default on all notifications. So there would be a duplicate declaration of methods, or needed to be handled frequently by developers to avoid duplication if it was simply named delay

It would also break any current use of notifications that has the Queueable-trait as it would be calling a setter-method instead of getting the desired delays per channel

[robert-sykes]

The reason I mentioned Illuminate\Bus\Queueable is that artisan make:notification {name} command uses the Illuminate\Bus\Queueable-trait per default on all notifications. So there would be a duplicate declaration of methods, or needed to be handled frequently by developers to avoid duplication if it was simply named delay

It would also break any current use of notifications that has the Queueable-trait as it would be calling a setter-method instead of getting the desired delays per channel

Ah - I didn't consider that. 👍

[taylorotwell]

@ehamrin got it