]> BookStack Code Mirror - bookstack/commitdiff
DB: Addressed test issues for user ID changes 5844/head
authorDan Brown <redacted>
Sun, 19 Oct 2025 18:52:15 +0000 (19:52 +0100)
committerDan Brown <redacted>
Sun, 19 Oct 2025 18:52:15 +0000 (19:52 +0100)
Reverted change for activities table so that a record is retained of
past activity, and added a check where the ID may be displayed to ensure
it does not mislead and accidentially reference other, newer users.

app/Users/UserRepo.php
database/migrations/2025_10_18_163331_clean_user_id_references.php
resources/views/settings/audit.blade.php
resources/views/settings/parts/table-user.blade.php
resources/views/settings/recycle-bin/parts/recycle-bin-list-item.blade.php
tests/Activity/AuditLogTest.php
tests/User/UserManagementTest.php

index 7a4fa5f98d268d56340788f97c21856668d35df3..894d7c01f7d00e42efb1cab25aa4aa1a5fbefa2c 100644 (file)
@@ -198,7 +198,6 @@ class UserRepo
     protected function nullifyUserNonDependantRelations(User $user): void
     {
         $toNullify = [
-            'activities' => ['user_id'],
             'attachments' => ['created_by', 'updated_by'],
             'comments' => ['created_by', 'updated_by'],
             'deletions' => ['deleted_by'],
index 75ff6af33cb2495359ea767815d95989cb43edba..42e67013900ac2c5e47475df7e2cd6178b85bb92 100644 (file)
@@ -6,7 +6,6 @@ use Illuminate\Support\Facades\Schema;
 
 return new class extends Migration {
     protected static array $toNullify = [
-        'activities' => ['user_id'],
         'attachments' => ['created_by', 'updated_by'],
         'comments' => ['created_by', 'updated_by'],
         'deletions' => ['deleted_by'],
@@ -55,9 +54,6 @@ return new class extends Migration {
                 DB::table($tableName)->whereNotIn($columnName, $idSelectQuery)->delete();
             }
         }
-
-        // TODO - Ensure each is fully handled in user delete
-        //    Start by writing tests for each of these columns
     }
 
     /**
index 8e47766804094086d2dfb67a25747c3c86292aec..0407275e059d98f585e189f7fbabac6208326e7f 100644 (file)
                 @foreach($activities as $activity)
                     <div class="item-list-row flex-container-row items-center wrap py-xxs">
                         <div class="flex-2 px-m py-xxs flex-container-row items-center min-width-m">
-                            @include('settings.parts.table-user', ['user' => $activity->user, 'user_id' => $activity->user_id])
+                            @if($activity->user && $activity->user->created_at <= $activity->created_at)
+                                @include('settings.parts.table-user', ['user' => $activity->user])
+                            @else
+                                [ID: {{ $activity->user_id }}] {{ trans('common.deleted_user') }}
+                            @endif
                         </div>
                         <div class="flex-2 px-m py-xxs min-width-m"><strong
                                     class="mr-xs hide-over-m">{{ trans('settings.audit_table_event') }}
index d29ad1979a0fafb95c5e8612d447a7cc89494522..affc7b6c4c4f6045f8cb299f36d95452213b0fed 100644 (file)
@@ -1,12 +1,7 @@
 {{--
-$user - User mode to display, Can be null.
-$user_id - Id of user to show. Must be provided.
+$user - User to display.
 --}}
-@if($user)
-    <a href="{{ $user->getEditUrl() }}" class="flex-container-row inline gap-s items-center">
-        <div class="flex-none"><img width="40" height="40" class="avatar block" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></div>
-        <div class="flex">{{ $user->name }}</div>
-    </a>
-@else
-    [ID: {{ $user_id }}] {{ trans('common.deleted_user') }}
-@endif
\ No newline at end of file
+<a href="{{ $user->getEditUrl() }}" class="flex-container-row inline gap-s items-center">
+    <div class="flex-none"><img width="40" height="40" class="avatar block" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></div>
+    <div class="flex">{{ $user->name }}</div>
+</a>
\ No newline at end of file
index b18f9cbe08c963cb3c3045475e79cd72cea8b804..2dad617dcade78f4dadc24588e51896d08d0f659 100644 (file)
         @endif
     </div>
     <div class="flex-2 px-m py-xs flex-container-row items-center min-width-m">
-        <div><strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_by') }}:<br></strong>@include('settings.parts.table-user', ['user' => $deletion->deleter, 'user_id' => $deletion->deleted_by])</div>
+        <div>
+            <strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_by') }}:<br></strong>
+            @if($deletion->deleter)
+                @include('settings.parts.table-user', ['user' => $deletion->deleter, 'user_id' => $deletion->deleted_by])
+            @else
+                {{ trans('common.deleted_user') }}
+            @endif
+        </div>
     </div>
     <div class="flex px-m py-xs min-width-s"><strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_at') }}:<br></strong>{{ $deletion->created_at }}</div>
     <div class="flex px-m py-xs text-m-right min-width-s">
index 6b435544df78f278a2da48b0b208ca5700bdb004..a6ba6be9f6259177af67fb405808355afe360f12 100644 (file)
@@ -83,6 +83,22 @@ class AuditLogTest extends TestCase
         $resp->assertSeeText("[ID: {$viewer->id}] Deleted User");
     }
 
+    public function test_deleted_user_shows_if_user_created_date_is_later_than_activity()
+    {
+        $viewer = $this->users->viewer();
+        $this->actingAs($viewer);
+        $page = $this->entities->page();
+        $this->activityService->add(ActivityType::PAGE_CREATE, $page);
+        $viewer->created_at = Carbon::now()->addDay();
+        $viewer->save();
+
+        $this->actingAs($this->users->admin());
+
+        $resp = $this->get('settings/audit');
+        $resp->assertSeeText("[ID: {$viewer->id}] Deleted User");
+        $resp->assertDontSee($viewer->name);
+    }
+
     public function test_filters_by_key()
     {
         $this->actingAs($this->users->admin());
index a34a9e7f4c4f16259a386262cf2897c2cf18bbb0..d50ac2087919507d737be51c2429496cb2c19675 100644 (file)
@@ -221,7 +221,6 @@ class UserManagementTest extends TestCase
         $watch = Watch::factory()->create(['user_id' => $user->id]);
 
         $userColumnsByTable = [
-            'activities' => ['user_id'],
             'api_tokens' => ['user_id'],
             'attachments' => ['created_by', 'updated_by'],
             'comments' => ['created_by', 'updated_by'],
@@ -259,7 +258,6 @@ class UserManagementTest extends TestCase
         }
 
         // Check models exist where should be retained
-        $this->assertDatabaseHas('activities', ['id' => $activity->id, 'user_id' => null]);
         $this->assertDatabaseHas('attachments', ['id' => $attachment->id, 'created_by' => null, 'updated_by' => null]);
         $this->assertDatabaseHas('comments', ['id' => $comment->id, 'created_by' => null, 'updated_by' => null]);
         $this->assertDatabaseHas('deletions', ['id' => $deletion->id, 'deleted_by' => null]);
@@ -276,6 +274,9 @@ class UserManagementTest extends TestCase
         $this->assertDatabaseMissing('social_accounts', ['id' => $socialAccount->id]);
         $this->assertDatabaseMissing('user_invites', ['token' => 'abc123']);
         $this->assertDatabaseMissing('watches', ['id' => $watch->id]);
+
+        // Ensure activity remains using the old ID (Special case for auditing changes)
+        $this->assertDatabaseHas('activities', ['id' => $activity->id, 'user_id' => $user->id]);
     }
 
     public function test_delete_removes_user_preferences()