From: Dan Brown <redacted>
Date: Sun, 10 Jan 2021 22:43:22 +0000 (+0000)
Subject: Improved some query efficiencies on user list
X-Git-Tag: v0.31.3~1^2~2
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/d0a7a8b890cfe570b273addbf8d9cba120a5bc9e

Improved some query efficiencies on user list
---

diff --git a/app/Auth/User.php b/app/Auth/User.php
index fdfd9e616..9d7eaa72e 100644
--- a/app/Auth/User.php
+++ b/app/Auth/User.php
@@ -1,19 +1,20 @@
 <?php namespace BookStack\Auth;
 
-use BookStack\Actions\Activity;
 use BookStack\Api\ApiToken;
 use BookStack\Interfaces\Loggable;
 use BookStack\Model;
 use BookStack\Notifications\ResetPassword;
 use BookStack\Uploads\Image;
 use Carbon\Carbon;
+use Exception;
 use Illuminate\Auth\Authenticatable;
 use Illuminate\Auth\Passwords\CanResetPassword;
 use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract;
 use Illuminate\Contracts\Auth\CanResetPassword as CanResetPasswordContract;
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
 use Illuminate\Database\Eloquent\Relations\HasMany;
-use Illuminate\Database\Eloquent\Relations\HasOne;
 use Illuminate\Notifications\Notifiable;
 use Illuminate\Support\Collection;
 
@@ -46,6 +47,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
      */
     protected $fillable = ['name', 'email'];
 
+    protected $casts = ['last_activity_at' => 'datetime'];
+
     /**
      * The attributes excluded from the model's JSON form.
      * @var array
@@ -181,7 +184,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
 
     /**
      * Get the social account associated with this user.
-     * @return \Illuminate\Database\Eloquent\Relations\HasMany
+     * @return HasMany
      */
     public function socialAccounts()
     {
@@ -218,7 +221,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
 
         try {
             $avatar = $this->avatar ? url($this->avatar->getThumb($size, $size, false)) : $default;
-        } catch (\Exception $err) {
+        } catch (Exception $err) {
             $avatar = $default;
         }
         return $avatar;
@@ -226,7 +229,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
 
     /**
      * Get the avatar for the user.
-     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo
+     * @return BelongsTo
      */
     public function avatar()
     {
@@ -242,11 +245,16 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
     }
 
     /**
-     * Get the latest activity instance for this user.
+     * Get the last activity time for this user.
      */
-    public function latestActivity(): HasOne
+    public function scopeWithLastActivityAt(Builder $query)
     {
-        return $this->hasOne(Activity::class)->latest();
+        $query->addSelect(['activities.created_at as last_activity_at'])
+            ->leftJoinSub(function (\Illuminate\Database\Query\Builder $query) {
+                $query->from('activities')->select('user_id')
+                    ->selectRaw('max(created_at) as created_at')
+                    ->groupBy('user_id');
+            }, 'activities', 'users.id', '=', 'activities.user_id');
     }
 
     /**
diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index 6fb5dfa0f..29a0ebc14 100644
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@@ -59,14 +59,10 @@ class UserRepo
     public function getAllUsersPaginatedAndSorted(int $count, array $sortData): LengthAwarePaginator
     {
         $sort = $sortData['sort'];
-        if ($sort === 'latest_activity') {
-            $sort = \BookStack\Actions\Activity::query()->select('created_at')
-                ->whereColumn('activities.user_id', 'users.id')
-                ->latest()
-                ->take(1);
-        }
 
-        $query = User::query()->with(['roles', 'avatar', 'latestActivity'])
+        $query = User::query()->select(['*'])
+            ->withLastActivityAt()
+            ->with(['roles', 'avatar'])
             ->orderBy($sort, $sortData['order']);
 
         if ($sortData['search']) {
diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php
index 852d507c1..92e1cd8b7 100644
--- a/app/Http/Controllers/UserController.php
+++ b/app/Http/Controllers/UserController.php
@@ -41,6 +41,7 @@ class UserController extends Controller
             'sort' => $request->get('sort', 'name'),
         ];
         $users = $this->userRepo->getAllUsersPaginatedAndSorted(20, $listDetails);
+
         $this->setPageTitle(trans('settings.users'));
         $users->appends($listDetails);
         return view('users.index', ['users' => $users, 'listDetails' => $listDetails]);
diff --git a/resources/views/users/index.blade.php b/resources/views/users/index.blade.php
index 4b5bad0fd..68641ca64 100644
--- a/resources/views/users/index.blade.php
+++ b/resources/views/users/index.blade.php
@@ -37,7 +37,7 @@
                     </th>
                     <th>{{ trans('settings.role_user_roles') }}</th>
                     <th class="text-right">
-                        <a href="{{ sortUrl('/settings/users', $listDetails, ['sort' => 'latest_activity']) }}">{{ trans('settings.users_latest_activity') }}</a>
+                        <a href="{{ sortUrl('/settings/users', $listDetails, ['sort' => 'last_activity_at']) }}">{{ trans('settings.users_latest_activity') }}</a>
                     </th>
                 </tr>
                 @foreach($users as $user)
@@ -58,8 +58,8 @@
                             @endforeach
                         </td>
                         <td class="text-right text-muted">
-                            @if($user->latestActivity)
-                                <small title="{{ $user->latestActivity->created_at->format('Y-m-d H:i:s') }}">{{ $user->latestActivity->created_at->diffForHumans() }}</small>
+                            @if($user->last_activity_at)
+                                <small title="{{ $user->last_activity_at->format('Y-m-d H:i:s') }}">{{ $user->last_activity_at->diffForHumans() }}</small>
                             @endif
                         </td>
                     </tr>