DB: Updated handling of deleted user ID handling in DB

Updated uses of user ID to nullify on delete.
Added testing to cover deletion of user relations.
Added model factories to support changes and potential other tests.
Cleans existing ID references in the DB via migration.

app/Users/UserRepo.php

@@ -5,8 +5,6 @@ namespace BookStack\Users;
 use BookStack\Access\UserInviteException;
 use BookStack\Access\UserInviteService;
 use BookStack\Activity\ActivityType;
-use BookStack\Entities\EntityProvider;
-use BookStack\Entities\Models\Entity;
 use BookStack\Exceptions\NotifyException;
 use BookStack\Exceptions\UserUpdateException;
 use BookStack\Facades\Activity;
@@ -27,7 +25,6 @@ class UserRepo
     ) {
     }
 
-
     /**
      * Get a user by their email address.
      */
@@ -161,15 +158,12 @@ class UserRepo
      *
      * @throws Exception
      */
-    public function destroy(User $user, ?int $newOwnerId = null)
+    public function destroy(User $user, ?int $newOwnerId = null): void
     {
         $this->ensureDeletable($user);
 
-        $user->socialAccounts()->delete();
-        $user->apiTokens()->delete();
-        $user->favourites()->delete();
-        $user->mfaValues()->delete();
-        $user->watches()->delete();
+        $this->removeUserDependantRelations($user);
+        $this->nullifyUserNonDependantRelations($user);
         $user->delete();
 
         // Delete user profile images
@@ -178,17 +172,53 @@ class UserRepo
         // Delete related activities
         setting()->deleteUserSettings($user->id);
 
+        // Migrate or nullify ownership
+        $newOwner = null;
         if (!empty($newOwnerId)) {
             $newOwner = User::query()->find($newOwnerId);
-            if (!is_null($newOwner)) {
-                $this->migrateOwnership($user, $newOwner);
-            }
-            // TODO - Should be be nullifying ownership instead?
         }
+        $this->migrateOwnership($user, $newOwner);
 
         Activity::add(ActivityType::USER_DELETE, $user);
     }
 
+    protected function removeUserDependantRelations(User $user): void
+    {
+        $user->apiTokens()->delete();
+        $user->socialAccounts()->delete();
+        $user->favourites()->delete();
+        $user->mfaValues()->delete();
+        $user->watches()->delete();
+
+        $tables = ['email_confirmations', 'user_invites', 'views'];
+        foreach ($tables as $table) {
+            DB::table($table)->where('user_id', '=', $user->id)->delete();
+        }
+    }
+    protected function nullifyUserNonDependantRelations(User $user): void
+    {
+        $toNullify = [
+            'activities' => ['user_id'],
+            'attachments' => ['created_by', 'updated_by'],
+            'comments' => ['created_by', 'updated_by'],
+            'deletions' => ['deleted_by'],
+            'entities' => ['created_by', 'updated_by'],
+            'images' => ['created_by', 'updated_by'],
+            'imports' => ['created_by'],
+            'joint_permissions' => ['owner_id'],
+            'page_revisions' => ['created_by'],
+            'sessions' => ['user_id'],
+        ];
+
+        foreach ($toNullify as $table => $columns) {
+            foreach ($columns as $column) {
+                DB::table($table)
+                    ->where($column, '=', $user->id)
+                    ->update([$column => null]);
+            }
+        }
+    }
+
     /**
      * @throws NotifyException
      */
@@ -206,11 +236,12 @@ class UserRepo
     /**
      * Migrate ownership of items in the system from one user to another.
      */
-    protected function migrateOwnership(User $fromUser, User $toUser): void
+    protected function migrateOwnership(User $fromUser, User|null $toUser): void
     {
+        $newOwnerValue = $toUser ? $toUser->id : null;
         DB::table('entities')
             ->where('owned_by', '=', $fromUser->id)
-            ->update(['owned_by' => $toUser->id]);
+            ->update(['owned_by' => $newOwnerValue]);
     }
 
     /**
@@ -248,7 +279,7 @@ class UserRepo
      *
      * @throws UserUpdateException
      */
-    protected function setUserRoles(User $user, array $roles)
+    protected function setUserRoles(User $user, array $roles): void
     {
         $roles = array_filter(array_values($roles));
 
@@ -261,7 +292,7 @@ class UserRepo
 
     /**
      * Check if the given user is the last admin and their new roles no longer
-     * contains the admin role.
+     * contain the admin role.
      */
     protected function demotingLastAdmin(User $user, array $newRoles): bool
     {