[12.x] Preserve notification state mutations from via() in sendNow()

[alimorgaan]

Summary

Fixes bug where locale set dynamically in via() is lost when sending notifications via sendNow().

Problem

Inconsistency between queueNotification() and sendNow()

Both methods clone notification to $original for sending, but call via() on different objects:

queueNotification() ✅ Correct

protected function queueNotification($notifiables, $notification)
{ 
    $notifiables = $this->formatNotifiables($notifiables);
    $original = clone $notification;

    foreach ($notifiables as $notifiable) {
        foreach ($original->via($notifiable) as $channel) {
        //       ^^^^^^^^^ uses $original

sendNow() ❌ Bug

public function sendNow($notifiables, $notification, array $channels = null)
{
    $original = clone $notification;

    foreach ($notifiables as $notifiable) {
        if (empty($viaChannels = $channels ?: $notification->via($notifiable))) {
        //                                    ^^^^^^^^^^^^^ uses $notification (pre-clone)

        $this->withLocale($this->preferredLocale($notifiable, $notification), ...);
        //                                                    ^^^^^^^^^^^^^ also wrong

When via() mutates state (e.g., sets $this->locale), those changes happen on $notification but $original is sent — losing the mutations.

Real-world use case

Setting notification locale dynamically based on notifiable:

public function via($notifiable)
{
    $this->locale = $notifiable->preferred_language;

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

Works for queued notifications, fails silently for synchronous ones.

Solution

Align sendNow() with queueNotification():

- if (empty($viaChannels = $channels ?: $notification->via($notifiable))) {
+ if (empty($viaChannels = $channels ?: $original->via($notifiable))) {

- $this->withLocale($this->preferredLocale($notifiable, $notification), ...);
+ $this->withLocale($this->preferredLocale($notifiable, $original), ...);

Test Plan

• Added testItPreservesLocaleSetInViaMethod to verify locale set in via() is preserved
• All existing notification tests pass

[rodrigopedra]

+1 on aligning the behavior between sendNow() with queueNotification()

My gripe is with the example choice when the documented way to set a notification's locale based on the notifiable is to make the notifiable implement the Illuminate\Contracts\Translation\HasLocalePreference trait.

Reference: https://laravel.com/docs/12.x/notifications#user-preferred-locales

I am not sure if a notification should be mutated on the via* or with* methods.

I guess any notifiable specific change should be done on the to* methods (toDatabase(), toMail(), etc.).

Anyway, I think it is still worth aligning the behavior between the methods.

[taylorotwell]

Yeah this way of setting the locale is weird and we don't need to test explicitly for that in my opinion. I would at least change the test to be something realistic.

[alimorgaan]

+1 on aligning the behavior between sendNow() with queueNotification()

My gripe is with the example choice when the documented way to set a notification's locale based on the notifiable is to make the notifiable implement the Illuminate\Contracts\Translation\HasLocalePreference trait.

Reference: https://laravel.com/docs/12.x/notifications#user-preferred-locales

I am not sure if a notification should be mutated on the via* or with* methods.

I guess any notifiable specific change should be done on the to* methods (toDatabase(), toMail(), etc.).

Anyway, I think it is still worth aligning the behavior between the methods.

@rodrigopedra
Thanks for the feedback! You're right - using locale as the example was a poor choice since HasLocalePreference is the proper way to handle that. so it's more about preserving the state of the notification object and aligning the behavior

[alimorgaan]

Yeah this way of setting the locale is weird and we don't need to test explicitly for that in my opinion. I would at least change the test to be something realistic.

@taylorotwell

Thanks for the feedback! Agreed - locale was a bad example since HasLocalePreference is the proper way. Updated test to verify generic state mutations in via() are preserved. Renamed PR accordingly.