Merge pull request #16709 from snipe/#16699-fix-email-locales-when-none-set-on-user

Fixed #16699 - Better handle user locales in mailables

app/Http/Controllers/Api/UsersController.php

@@ -339,6 +339,7 @@ class UsersController extends Controller
             $users = $users->where(function ($query) use ($request) {
                 $query->SimpleNameSearch($request->get('search'))
                     ->orWhere('username', 'LIKE', '%'.$request->get('search').'%')
+                    ->orWhere('email', 'LIKE', '%'.$request->get('search').'%')
                     ->orWhere('employee_num', 'LIKE', '%'.$request->get('search').'%');
             });
         }

app/Http/Controllers/ProfileController.php

@@ -53,7 +53,7 @@ class ProfileController extends Controller
         $user->enable_confetti = $request->input('enable_confetti', false);
 
         if (! config('app.lock_passwords')) {
-            $user->locale = $request->input('locale', 'en-US');
+            $user->locale = $request->input('locale');
         }
 
         if ((Gate::allows('self.two_factor')) && ((Setting::getSettings()->two_factor_enabled == '1') && (! config('app.lock_passwords')))) {

app/Listeners/CheckoutableListener.php

@@ -63,11 +63,9 @@ class CheckoutableListener
         }
         $ccEmails = array_filter($adminCcEmailsArray);
         $mailable = $this->getCheckoutMailType($event, $acceptance);
-        $notifiable = $this->getNotifiables($event);
+        $notifiable = $this->getNotifiableUsers($event);
+
 
-        if ($event->checkedOutTo->locale) {
-            $mailable->locale($event->checkedOutTo->locale);
-        }
         // Send email notifications
         try {
             /**
@@ -79,7 +77,7 @@ class CheckoutableListener
 
             if ($event->checkoutable->requireAcceptance() || $event->checkoutable->getEula() ||
                 $this->checkoutableShouldSendEmail($event)) {
-                Log::info('Sending checkout email, Locale: ' . ($event->checkedOutTo->locale ?? 'default'));
+                //Log::info('Sending checkout email, Locale: ' . ($event->checkedOutTo->locale ?? 'default'));
                 if (!empty($notifiable)) {
                     Mail::to($notifiable)->cc($ccEmails)->send($mailable);
                 } elseif (!empty($ccEmails)) {
@@ -161,10 +159,8 @@ class CheckoutableListener
         }
         $ccEmails = array_filter($adminCcEmailsArray);
         $mailable =  $this->getCheckinMailType($event);
-        $notifiable = $this->getNotifiables($event);
-        if ($event->checkedOutTo?->locale) {
-            $mailable->locale($event->checkedOutTo->locale);
-        }
+        $notifiable = $this->getNotifiableUsers($event);
+
         // Send email notifications
         try {
             /**
@@ -175,7 +171,6 @@ class CheckoutableListener
              */
                 if ($event->checkoutable->requireAcceptance() || $event->checkoutable->getEula() ||
                     $this->checkoutableShouldSendEmail($event)) {
-                    Log::info('Sending checkin email, Locale: ' . ($event->checkedOutTo->locale ?? 'default'));
                     if (!empty($notifiable)) {
                         Mail::to($notifiable)->cc($ccEmails)->send($mailable);
                     } elseif (!empty($ccEmails)){
@@ -324,17 +319,26 @@ class CheckoutableListener
         return new $mailable($event->checkoutable, $event->checkedOutTo, $event->checkedInBy, $event->note);
 
     }
-    private function getNotifiables($event){
+
+    /**
+     * This gets the recipient objects based on the type of checkoutable.
+     * The 'name' property for users is set in the boot method in the User model.
+     *
+     * @see \App\Models\User::boot()
+     * @param $event
+     * @return mixed
+     */
+    private function getNotifiableUsers($event){
 
         if($event->checkedOutTo instanceof Asset){
             $event->checkedOutTo->load('assignedTo');
-            return $event->checkedOutTo->assignedto?->email ?? '';
+            return $event->checkedOutTo->assignedto;
         }
         else if($event->checkedOutTo instanceof Location) {
-            return $event->checkedOutTo->manager?->email ?? '';
+            return $event->checkedOutTo->manager;
         }
         else{
-            return $event->checkedOutTo?->email ?? '';
+            return $event->checkedOutTo;
         }
     }
     private function webhookSelected(){

app/Models/User.php

@@ -20,6 +20,7 @@ use Illuminate\Notifications\Notifiable;
 use Illuminate\Support\Facades\Gate;
 use Laravel\Passport\HasApiTokens;
 use Watson\Validating\ValidatingTrait;
+use Illuminate\Database\Eloquent\Casts\Attribute;
 
 class User extends SnipeModel implements AuthenticatableContract, AuthorizableContract, CanResetPasswordContract, HasLocalePreference
 {
@@ -139,6 +140,29 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         'manager'    => ['first_name', 'last_name', 'username'],
     ];
 
+
+    /**
+     * This sets the name property on the user. It's not a real field in the database
+     * (since we use first_name and last_name), but the Laravel mailable method
+     * uses this to determine the name of the user to send emails to.
+     *
+     * We only have to do this on the User model and no other models because other
+     * first-class objects have a name field.
+     * @return void
+     */
+
+    public $name;
+
+    protected static function boot()
+    {
+        parent::boot();
+
+        static::retrieved(function($user){
+            $user->name = $user->getFullNameAttribute();
+        });
+    }
+
+
     /**
      * Internally check the user permission for the given section
      *
@@ -279,6 +303,7 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         return $this->activated == 1;
     }
 
+
     /**
      * Returns the full name attribute
      *
@@ -844,10 +869,22 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         return $query->leftJoin('companies as companies_user', 'users.company_id', '=', 'companies_user.id')->orderBy('companies_user.name', $order);
     }
 
-    public function preferredLocale()
+
+
+
+    /**
+     * Get the preferred locale for the user.
+     *
+     * This uses the HasLocalePreference contract to determine the user's preferred locale,
+     * used by Laravel's mail system to determine the locale for sending emails.
+     * https://laravel.com/docs/11.x/mail#user-preferred-locales
+     *
+     */
+    public function preferredLocale(): string
     {
-        return $this->locale;
+        return $this->locale ?? Setting::getSettings()->locale ?? config('app.locale');
     }
+
     public function getUserTotalCost(){
         $asset_cost= 0;
         $license_cost= 0;

tests/Feature/Users/Api/UsersForSelectListTest.php

@@ -40,6 +40,19 @@ class UsersForSelectListTest extends TestCase
         $this->assertTrue($results->pluck('text')->contains(fn($text) => str_contains($text, 'Luke')));
     }
 
+    public function testUsersCanBeSearchedByEmail()
+    {
+        User::factory()->create(['first_name' => 'Luke', 'last_name' => 'Skywalker', 'email' => 'luke@jedis.org']);
+
+        Passport::actingAs(User::factory()->create());
+        $response = $this->getJson(route('api.users.selectlist', ['search' => 'luke@jedis']))->assertOk();
+
+        $results = collect($response->json('results'));
+
+        $this->assertEquals(1, $results->count());
+        $this->assertTrue($results->pluck('text')->contains(fn($text) => str_contains($text, 'Luke')));
+    }
+
     public function testUsersScopedToCompanyWhenMultipleFullCompanySupportEnabled()
     {
         $this->settings->enableMultipleFullCompanySupport();