Skip to content

Commit efff870

Browse files
committed
DB: Addressed test issues for user ID changes
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.
1 parent 5754acf commit efff870

File tree

7 files changed

+37
-19
lines changed

7 files changed

+37
-19
lines changed

app/Users/UserRepo.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,6 @@ protected function removeUserDependantRelations(User $user): void
198198
protected function nullifyUserNonDependantRelations(User $user): void
199199
{
200200
$toNullify = [
201-
'activities' => ['user_id'],
202201
'attachments' => ['created_by', 'updated_by'],
203202
'comments' => ['created_by', 'updated_by'],
204203
'deletions' => ['deleted_by'],

database/migrations/2025_10_18_163331_clean_user_id_references.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
return new class extends Migration {
88
protected static array $toNullify = [
9-
'activities' => ['user_id'],
109
'attachments' => ['created_by', 'updated_by'],
1110
'comments' => ['created_by', 'updated_by'],
1211
'deletions' => ['deleted_by'],
@@ -55,9 +54,6 @@ public function up(): void
5554
DB::table($tableName)->whereNotIn($columnName, $idSelectQuery)->delete();
5655
}
5756
}
58-
59-
// TODO - Ensure each is fully handled in user delete
60-
// Start by writing tests for each of these columns
6157
}
6258

6359
/**

resources/views/settings/audit.blade.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,11 @@ class="text-item">{{ $type }}</a></li>
8888
@foreach($activities as $activity)
8989
<div class="item-list-row flex-container-row items-center wrap py-xxs">
9090
<div class="flex-2 px-m py-xxs flex-container-row items-center min-width-m">
91-
@include('settings.parts.table-user', ['user' => $activity->user, 'user_id' => $activity->user_id])
91+
@if($activity->user && $activity->user->created_at <= $activity->created_at)
92+
@include('settings.parts.table-user', ['user' => $activity->user])
93+
@else
94+
[ID: {{ $activity->user_id }}] {{ trans('common.deleted_user') }}
95+
@endif
9296
</div>
9397
<div class="flex-2 px-m py-xxs min-width-m"><strong
9498
class="mr-xs hide-over-m">{{ trans('settings.audit_table_event') }}
Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
{{--
2-
$user - User mode to display, Can be null.
3-
$user_id - Id of user to show. Must be provided.
2+
$user - User to display.
43
--}}
5-
@if($user)
6-
<a href="{{ $user->getEditUrl() }}" class="flex-container-row inline gap-s items-center">
7-
<div class="flex-none"><img width="40" height="40" class="avatar block" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></div>
8-
<div class="flex">{{ $user->name }}</div>
9-
</a>
10-
@else
11-
[ID: {{ $user_id }}] {{ trans('common.deleted_user') }}
12-
@endif
4+
<a href="{{ $user->getEditUrl() }}" class="flex-container-row inline gap-s items-center">
5+
<div class="flex-none"><img width="40" height="40" class="avatar block" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></div>
6+
<div class="flex">{{ $user->name }}</div>
7+
</a>

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,14 @@
3333
@endif
3434
</div>
3535
<div class="flex-2 px-m py-xs flex-container-row items-center min-width-m">
36-
<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>
36+
<div>
37+
<strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_by') }}:<br></strong>
38+
@if($deletion->deleter)
39+
@include('settings.parts.table-user', ['user' => $deletion->deleter, 'user_id' => $deletion->deleted_by])
40+
@else
41+
{{ trans('common.deleted_user') }}
42+
@endif
43+
</div>
3744
</div>
3845
<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>
3946
<div class="flex px-m py-xs text-m-right min-width-s">

tests/Activity/AuditLogTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,22 @@ public function test_shows_activity_for_deleted_users()
8383
$resp->assertSeeText("[ID: {$viewer->id}] Deleted User");
8484
}
8585

86+
public function test_deleted_user_shows_if_user_created_date_is_later_than_activity()
87+
{
88+
$viewer = $this->users->viewer();
89+
$this->actingAs($viewer);
90+
$page = $this->entities->page();
91+
$this->activityService->add(ActivityType::PAGE_CREATE, $page);
92+
$viewer->created_at = Carbon::now()->addDay();
93+
$viewer->save();
94+
95+
$this->actingAs($this->users->admin());
96+
97+
$resp = $this->get('settings/audit');
98+
$resp->assertSeeText("[ID: {$viewer->id}] Deleted User");
99+
$resp->assertDontSee($viewer->name);
100+
}
101+
86102
public function test_filters_by_key()
87103
{
88104
$this->actingAs($this->users->admin());

tests/User/UserManagementTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,6 @@ public function test_delete_with_empty_owner_migration_id_clears_relevant_id_use
221221
$watch = Watch::factory()->create(['user_id' => $user->id]);
222222

223223
$userColumnsByTable = [
224-
'activities' => ['user_id'],
225224
'api_tokens' => ['user_id'],
226225
'attachments' => ['created_by', 'updated_by'],
227226
'comments' => ['created_by', 'updated_by'],
@@ -259,7 +258,6 @@ public function test_delete_with_empty_owner_migration_id_clears_relevant_id_use
259258
}
260259

261260
// Check models exist where should be retained
262-
$this->assertDatabaseHas('activities', ['id' => $activity->id, 'user_id' => null]);
263261
$this->assertDatabaseHas('attachments', ['id' => $attachment->id, 'created_by' => null, 'updated_by' => null]);
264262
$this->assertDatabaseHas('comments', ['id' => $comment->id, 'created_by' => null, 'updated_by' => null]);
265263
$this->assertDatabaseHas('deletions', ['id' => $deletion->id, 'deleted_by' => null]);
@@ -276,6 +274,9 @@ public function test_delete_with_empty_owner_migration_id_clears_relevant_id_use
276274
$this->assertDatabaseMissing('social_accounts', ['id' => $socialAccount->id]);
277275
$this->assertDatabaseMissing('user_invites', ['token' => 'abc123']);
278276
$this->assertDatabaseMissing('watches', ['id' => $watch->id]);
277+
278+
// Ensure activity remains using the old ID (Special case for auditing changes)
279+
$this->assertDatabaseHas('activities', ['id' => $activity->id, 'user_id' => $user->id]);
279280
}
280281

281282
public function test_delete_removes_user_preferences()

0 commit comments

Comments
 (0)