v3 Roadmap [Suggestions Wanted]

[atymic]

Is there anything people suggest for v3? Happy to make BC breaks if required, as long as there is a path for migration for existing users.

Strict types will be the main one.
cc @thomasjohnkane

[madebycaliper]

Hey this package is great but I'd love to see an implementation of a method like notifyAtInterval() that could be used to create a recurring notification until a certain condition is met. If #104 is ready for merge I could see that being a good start.

For example, consider a Project model with a completed_at and invoiced_at timestamp. When the completed_at value is is modified from null to whatever time and Project is considered "completed," the owner of the project should be notified every week to "Invoice the client!" until invoiced_at is no longer null. Some pseudo code:

// In your controller
$project->completed_at = now();


// Then somewhere inside an observer or whatever
public function wasCompleted(Project $project): ScheduledNotification
{
    $user = $project->owner;

    // Send the reminder immediately
    $user->notify(new InvoiceTheClientReminder($project));

    // Also send reminders every two days
    return $user->notifyAtInterval(
        new InvoiceTheClientReminder($project),
        CarbonInterval::days(2),
        fn(Project $project) => !is_null($project->invoiced_at) // A closure to determine when it should stop sending
    );
}

Under the hood I imagine this could make use of shouldInterrupt but that may also be a very dumb idea. I'm new to the whole world of Notifications.

Thanks for considering this!

[brenjt]

A backoff configuration or if someone has an invalid email and the SMTP service fails as an example, it doesn't reattempt it very minute or someway to handle cancelling a notification if an exception is thrown.

Snooze::handleFailedNotifications(function (ScheduledNotification $notification, Throwable $error) {
    // Handle exception
    // 
})

Maybe a max attempts too.

Or just an event when a notification throws an error. event(new NotificationFailed($this));

[npw1982]

A way to defer who the message gets sent to until the time of sending perhaps? Currently, the way we are using it, we are identifying a bunch of users by Role to send the message to. But if those users have been soft deleted at the time the message is sent, they still get the notification (as they have been serialised at the point the notification was scheduled?).

[chatisk]

A way to defer who the message gets sent to until the time of sending perhaps? Currently, the way we are using it, we are identifying a bunch of users by Role to send the message to. But if those users have been soft deleted at the time the message is sent, they still get the notification (as they have been serialised at the point the notification was scheduled?).

You would have to put "watchers" to change the scheduled notification target or even cancel it.

[mateuszwsk]

Hey there, we are using your package and it's working great, however there is one minor improvement suggestion from our team, which came out after analyzing queries not using indexes on MySQL (InnoDB).

The findbyTarget() static method from the Thomasjohnkane\Snooze\ScheduledNotification class is not used in the package's internal code, but for anyone who would like to use it in their projects, there is an improvement potential:

• we could add a composite index on the target_id and target_type columns, to improve query performance.

However it won't work for targets using integers as their PKs, because the target_id column is type of string and the findByTarget() method searches for models via:

$models = $modelClass::query()
            ->whereTargetId($notifiable->getKey())
            ->whereTargetType(get_class($notifiable))
            ->get();

where the $notifiable->getKey() argument passed into the whereTargetId() scope resolves to an int type for integer PKs (tested on Laravel 11.36.1), which with MySQL InnoDB (tested on 8.0.34) the MySQL optimizer won't use the (target_id, target_type) composite index due to target_id column type mismatch – Laravel builds the query in the following way:

select * from `scheduled_notifications` where `target_id` = 123456 and `target_type` = 'App\\Model\\MyTarget';

In order to utilize the index and improve this query's performance, the code would need to check what is returned by $notifiable->getKey() and:

• if it's string – just pass it as string,
• but if it's int – cast the int to string, so that the query would pick up created composite index on the target_id, target_type columns properly.

It is only a tiny improvement, but for those who use this method in their projects on a large scale, having performance issues with tons of scheduled notifications, would be a great improvement – searching for notifications by target_id and target_type is handy in scenarios where you want to filter notifications sent to given targets (e.g. users).

Similarly, an index would be useful when searching by type (notification_type) using the findByType() static method from the Thomasjohnkane\Snooze\ScheduledNotification class and there won't be any more additional steps to make it work.

The indexes could be added also conditionally, e.g. by mention in the config file or docs – does not need to be added by default. The most important part here is to allow the index to be picked up by the MySQL optimizer for the target_id column regardless what's the target's PK type.

Just a suggestion! Great work with the package so far!

[heikokrebs]

Support of morphMaps for the target_type property. See: https://laravel.com/docs/master/eloquent-relationships#custom-polymorphic-types