Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.7] Mail recipient and notifiable preferred locale #25752

Merged
merged 2 commits into from
Sep 27, 2018

Conversation

derekmd
Copy link
Contributor

@derekmd derekmd commented Sep 24, 2018

This wraps up localized notifications introduced by #23178, #24451, and #24919.

Users can have a preferred locale (likely stored in the users database table) which the Mail and Notification services will use when queuing jobs.

Why store a user locale preference?

  1. To allow admin-triggered or automated scheduled notifications in the expected language.
  2. Clean up excessive need to call Mail::locale() and Notification::locale(). A typical localized scenario:
    • Default app locale: 'en'
    • http://mydomain.com/de/ requests have middleware call App::setLocale('de') for German translations
    • When queued, this mailable is actually delivered using the app's default 'en' instead of 'de'
      User::create(request()->all());
      Mail::to($user)->queue(new OrderConfirmation($order));
    • This call will translate the mailable as expected:
      Mail::to($user)->locale(app()->getLocale())->queue(new OrderConfirmation($order))
    • However this means every translated mailable sent in a codebase needs to call Mail::to()->locale()
      • likewise for Notification::locale()

Example User model with stored locale preference

use Illuminate\Contracts\Translation\HasLocalePreference;

class User extends Model implements HasLocalePreference
{
    public function preferredLocale()
    {
        return $this->locale;
    }
}
$user = User::create([
    'email' => '[email protected]',
    'locale' => 'de',
]);

These four calls will send the notification in German:

  • $user->notify(new OrderConfirmation($order))
  • Notification::send($user, new OrderConfirmation($order))
  • Mail::to($user)->queue(new OrderConfirmation($order)
  • Mail::to($user)->send(new OrderConfirmation($order)

Notifying multiple recipients with mixed locale preferences

$users = collect([
    User::create([
        'email' => '[email protected]',
        'locale' => 'en',
    ]),
    User::create([
        'email' => '[email protected]',
        'locale' => 'ar',
    ]),
]);

This call will notify Taylor in English and Mohamed in Arabic:

  • Notification::send($users, new OrderConfirmation($order))

This call will send the email in the application's default locale:

  • Mail::to($users)->queue(new OrderConfirmation($order))

Back in February I wrote a rambling explanation here: #23178 (comment) In summary, different message bodies would deviate from the email spec. If user-land wishes to breakup mailables into multiple translated emails being sent out, Illuminate\Mail\Mailer is macroable. e.g.,

Illuminate\Mail\Mailer::macro('queueTranslated', function ($users, $mailable) {
    return collect()->wrap($users)->groupBy->preferredLocale()->map(function ($users, $locale) use ($mailable) {
        return Mail::to($users)->locale($locale)->queue($mailable);
    });
});

Mail::queueTranslated(
    $users, new Newsletter(request('body'))
);

Overriding user preferred locale

If admins sending email wish to choose the locale, the locale() method call is used instead of the recipient's preference. These four calls will send the notification in English even if $user has a German preference:

  • $user->notify((new OrderConfirmation($order))->locale('en'))
  • Notification::locale('en')->send($user, new OrderConfirmation($order))
  • Mail::to($user)->locale('en')->queue(new OrderConfirmation($order))
  • Mail::to($user)->locale('en')->send(new OrderConfirmation($order))

Auth scaffolding User model class

It might be worth introducing an empty implementation of this contract to https://github.com/laravel/laravel/blob/master/app/User.php?

class User extends Authenticatable implements HasLocalePreference
{
    use Notifiable;

    // ...

    public function preferredLocale()
    {
    }
}

Returning null has no effect on the Mail and Notification services (the app default locale is used instead) so new projects are aware they can implement this feature. However I imagine a small percentage of Laravel projects actually use translations so it could be considered more scaffolding noise to strip out. Maybe just some laravel.com docs will do.

@taylorotwell taylorotwell merged commit aeef1b0 into laravel:5.7 Sep 27, 2018
@themsaid
Copy link
Member

Thank you @derekmd :)

@snipe
Copy link

snipe commented Sep 27, 2018

This is fantastic - thank you so much!

@derekmd derekmd deleted the mail-notification-preferred-locale branch September 27, 2018 20:31
@ali-syria
Copy link

Greate Work! thanks

@twf-nikhila
Copy link

This is amazing!! Thank you

@cayaner
Copy link

cayaner commented Sep 28, 2018

Great work,
Can the same result be achieved for broadcasting events?

@derekmd
Copy link
Contributor Author

derekmd commented Sep 28, 2018

I noticed this actually allows route-based queued notification translations even without storing locale in the database:

use Illuminate\Contracts\Translation\HasLocalePreference;

class User extends Model implements HasLocalePreference
{
    /**
     * Use the application's locale when it's secondary.
     *
     * @return string|null
     */
    public function preferredLocale()
    {
        return collect(config('app.supported_locales'))
            ->keys()
            ->intersect(app()->getLocale())
            ->first();
    }
}

So if you're not worried about the app sending cron-job or admin-triggered notifications, this will always include the application's non-default locale in the queued payload. Useful for a translated landing page (w/ email capture) or marketing-only apps without much of a PHP backend.

@gultyayev
Copy link

Nice, but what about standard mails like email verification? Will they be translated?

@0arifurrahman0
Copy link

Great work,

@mhetreramesh
Copy link

I'm getting error Call to a member function locale() on null but email is actually getting sent in the local language.

@thijsw
Copy link

thijsw commented Jan 13, 2019

This update removes the need to call the locale() method every time a notification is being created, which is great!

However, I'm using the $this->locale property in my notifications to generate URLs and discovered that it isn't set automatically (at least not in queued notifications?). Is this expected behaviour?

I could just add the locale() call again, but I was hoping to be done with it thanks to this PR.

@derekmd
Copy link
Contributor Author

derekmd commented Jan 13, 2019

Is this expected behaviour?

Yes, there can be multiple locales sent in one call when many users have different settings. That (temporary) locale isn't copied into the Notification object due to the locale precedence that pulls the locale from one of three sources. The framework switches the application container (and translations) into the new locale while building the notification and sending the API request.

Instead of $this->locale use app()->getLocale() inside toNexmo(), toMail(), etc. to build a unique URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.