webapp/bookstack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2025-10-22 07:52:19 +03:00
Code Activity

DB: Addressed test issues for user ID changes

Browse Source 
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.
This commit is contained in:
Dan Brown
2025-10-19 19:52:15 +01:00
parent 5754acf2fb
commit efff8700d4
7 changed files with 37 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
app/Users/UserRepo.php
View 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'],

4
database/migrations/2025_10_18_163331_clean_user_id_references.php
View 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
}
/**

6
resources/views/settings/audit.blade.php
View File

@@ -88,7 +88,11 @@
@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') }}

11
resources/views/settings/parts/table-user.blade.php
View 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">
<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
</a>

9
resources/views/settings/recycle-bin/parts/recycle-bin-list-item.blade.php
View File

@@ -33,7 +33,14 @@
@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">

16
tests/Activity/AuditLogTest.php
View 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());

5
tests/User/UserManagementTest.php
View 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()