From 5754acf2fb9fcb60bb2ffd95bab3b0bcb1b162fd Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 19 Oct 2025 19:10:15 +0100 Subject: [PATCH] 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/Access/Mfa/MfaValue.php | 3 + app/Access/SocialAccount.php | 13 +- app/Activity/Models/Activity.php | 3 + app/Activity/Models/Favourite.php | 3 + app/Activity/Models/Watch.php | 3 + app/Entities/Models/Deletion.php | 3 + app/Entities/Models/PageRevision.php | 3 + app/Users/Models/User.php | 2 - app/Users/UserRepo.php | 65 ++++++--- .../factories/Access/Mfa/MfaValueFactory.php | 28 ++++ .../factories/Access/SocialAccountFactory.php | 29 ++++ .../Activity/Models/ActivityFactory.php | 34 +++++ .../Activity/Models/FavouriteFactory.php | 31 +++++ .../Activity/Models/WatchFactory.php | 33 +++++ .../Entities/Models/DeletionFactory.php | 29 ++++ .../factories/Entities/Models/PageFactory.php | 4 +- .../Entities/Models/PageRevisionFactory.php | 40 ++++++ ..._134751_update_entity_relation_columns.php | 3 +- ..._10_18_163331_clean_user_id_references.php | 80 +++++++++++ tests/User/UserManagementTest.php | 130 +++++++++++++++--- 20 files changed, 495 insertions(+), 44 deletions(-) create mode 100644 database/factories/Access/Mfa/MfaValueFactory.php create mode 100644 database/factories/Access/SocialAccountFactory.php create mode 100644 database/factories/Activity/Models/ActivityFactory.php create mode 100644 database/factories/Activity/Models/FavouriteFactory.php create mode 100644 database/factories/Activity/Models/WatchFactory.php create mode 100644 database/factories/Entities/Models/DeletionFactory.php create mode 100644 database/factories/Entities/Models/PageRevisionFactory.php create mode 100644 database/migrations/2025_10_18_163331_clean_user_id_references.php diff --git a/app/Access/Mfa/MfaValue.php b/app/Access/Mfa/MfaValue.php index 64d20eb18..dd3e04618 100644 --- a/app/Access/Mfa/MfaValue.php +++ b/app/Access/Mfa/MfaValue.php @@ -4,6 +4,7 @@ namespace BookStack\Access\Mfa; use BookStack\Users\Models\User; use Carbon\Carbon; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; /** @@ -16,6 +17,8 @@ use Illuminate\Database\Eloquent\Model; */ class MfaValue extends Model { + use HasFactory; + protected static $unguarded = true; const METHOD_TOTP = 'totp'; diff --git a/app/Access/SocialAccount.php b/app/Access/SocialAccount.php index b76dbb9ec..f52f74cc4 100644 --- a/app/Access/SocialAccount.php +++ b/app/Access/SocialAccount.php @@ -5,18 +5,23 @@ namespace BookStack\Access; use BookStack\Activity\Models\Loggable; use BookStack\App\Model; use BookStack\Users\Models\User; +use Illuminate\Database\Eloquent\Factories\HasFactory; +use Illuminate\Database\Eloquent\Relations\BelongsTo; /** - * Class SocialAccount. - * * @property string $driver * @property User $user */ class SocialAccount extends Model implements Loggable { - protected $fillable = ['user_id', 'driver', 'driver_id', 'timestamps']; + use HasFactory; - public function user() + protected $fillable = ['user_id', 'driver', 'driver_id']; + + /** + * @return BelongsTo + */ + public function user(): BelongsTo { return $this->belongsTo(User::class); } diff --git a/app/Activity/Models/Activity.php b/app/Activity/Models/Activity.php index ac9fec517..898a6c93a 100644 --- a/app/Activity/Models/Activity.php +++ b/app/Activity/Models/Activity.php @@ -6,6 +6,7 @@ use BookStack\App\Model; use BookStack\Entities\Models\Entity; use BookStack\Permissions\Models\JointPermission; use BookStack\Users\Models\User; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; @@ -24,6 +25,8 @@ use Illuminate\Support\Str; */ class Activity extends Model { + use HasFactory; + /** * Get the loggable model related to this activity. * Currently only used for entities (previously entity_[id/type] columns). diff --git a/app/Activity/Models/Favourite.php b/app/Activity/Models/Favourite.php index 6f6079b07..6b5e97dee 100644 --- a/app/Activity/Models/Favourite.php +++ b/app/Activity/Models/Favourite.php @@ -4,11 +4,14 @@ namespace BookStack\Activity\Models; use BookStack\App\Model; use BookStack\Permissions\Models\JointPermission; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; class Favourite extends Model { + use HasFactory; + protected $fillable = ['user_id']; /** diff --git a/app/Activity/Models/Watch.php b/app/Activity/Models/Watch.php index dfb72cc0a..b088bd724 100644 --- a/app/Activity/Models/Watch.php +++ b/app/Activity/Models/Watch.php @@ -5,6 +5,7 @@ namespace BookStack\Activity\Models; use BookStack\Activity\WatchLevels; use BookStack\Permissions\Models\JointPermission; use Carbon\Carbon; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\MorphTo; @@ -20,6 +21,8 @@ use Illuminate\Database\Eloquent\Relations\MorphTo; */ class Watch extends Model { + use HasFactory; + protected $guarded = []; public function watchable(): MorphTo diff --git a/app/Entities/Models/Deletion.php b/app/Entities/Models/Deletion.php index 55589f61e..c24c72d44 100644 --- a/app/Entities/Models/Deletion.php +++ b/app/Entities/Models/Deletion.php @@ -4,6 +4,7 @@ namespace BookStack\Entities\Models; use BookStack\Activity\Models\Loggable; use BookStack\Users\Models\User; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\MorphTo; @@ -17,6 +18,8 @@ use Illuminate\Database\Eloquent\Relations\MorphTo; */ class Deletion extends Model implements Loggable { + use HasFactory; + protected $hidden = []; /** diff --git a/app/Entities/Models/PageRevision.php b/app/Entities/Models/PageRevision.php index 1a6c980e1..4409afdc2 100644 --- a/app/Entities/Models/PageRevision.php +++ b/app/Entities/Models/PageRevision.php @@ -6,6 +6,7 @@ use BookStack\Activity\Models\Loggable; use BookStack\App\Model; use BookStack\Users\Models\User; use Carbon\Carbon; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsTo; /** @@ -30,6 +31,8 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; */ class PageRevision extends Model implements Loggable { + use HasFactory; + protected $fillable = ['name', 'text', 'summary']; protected $hidden = ['html', 'markdown', 'text']; diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index 0017653b5..8bbf11695 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -31,8 +31,6 @@ use Illuminate\Notifications\Notifiable; use Illuminate\Support\Collection; /** - * Class User. - * * @property int $id * @property string $name * @property string $slug diff --git a/app/Users/UserRepo.php b/app/Users/UserRepo.php index 79d9e1b9e..7a4fa5f98 100644 --- a/app/Users/UserRepo.php +++ b/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 { diff --git a/database/factories/Access/Mfa/MfaValueFactory.php b/database/factories/Access/Mfa/MfaValueFactory.php new file mode 100644 index 000000000..03036b608 --- /dev/null +++ b/database/factories/Access/Mfa/MfaValueFactory.php @@ -0,0 +1,28 @@ + + */ +class MfaValueFactory extends Factory +{ + protected $model = \BookStack\Access\Mfa\MfaValue::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + return [ + 'user_id' => User::factory(), + 'method' => 'totp', + 'value' => '123456', + ]; + } +} diff --git a/database/factories/Access/SocialAccountFactory.php b/database/factories/Access/SocialAccountFactory.php new file mode 100644 index 000000000..814f47b58 --- /dev/null +++ b/database/factories/Access/SocialAccountFactory.php @@ -0,0 +1,29 @@ + + */ +class SocialAccountFactory extends Factory +{ + protected $model = \BookStack\Access\SocialAccount::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + return [ + 'user_id' => User::factory(), + 'driver' => 'github', + 'driver_id' => '123456', + 'avatar' => '', + ]; + } +} diff --git a/database/factories/Activity/Models/ActivityFactory.php b/database/factories/Activity/Models/ActivityFactory.php new file mode 100644 index 000000000..06ec07ced --- /dev/null +++ b/database/factories/Activity/Models/ActivityFactory.php @@ -0,0 +1,34 @@ + + */ +class ActivityFactory extends Factory +{ + protected $model = \BookStack\Activity\Models\Activity::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + $activities = ActivityType::all(); + $activity = $activities[array_rand($activities)]; + return [ + 'type' => $activity, + 'detail' => 'Activity detail for ' . $activity, + 'user_id' => User::factory(), + 'ip' => $this->faker->ipv4(), + 'loggable_id' => null, + 'loggable_type' => null, + ]; + } +} diff --git a/database/factories/Activity/Models/FavouriteFactory.php b/database/factories/Activity/Models/FavouriteFactory.php new file mode 100644 index 000000000..75fba86b3 --- /dev/null +++ b/database/factories/Activity/Models/FavouriteFactory.php @@ -0,0 +1,31 @@ + + */ +class FavouriteFactory extends Factory +{ + protected $model = \BookStack\Activity\Models\Favourite::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + $book = Book::query()->first(); + + return [ + 'user_id' => User::factory(), + 'favouritable_id' => $book->id, + 'favouritable_type' => 'book', + ]; + } +} diff --git a/database/factories/Activity/Models/WatchFactory.php b/database/factories/Activity/Models/WatchFactory.php new file mode 100644 index 000000000..0f8b9e6f7 --- /dev/null +++ b/database/factories/Activity/Models/WatchFactory.php @@ -0,0 +1,33 @@ + + */ +class WatchFactory extends Factory +{ + protected $model = \BookStack\Activity\Models\Watch::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + $book = Book::factory()->create(); + + return [ + 'user_id' => User::factory(), + 'watchable_id' => $book->id, + 'watchable_type' => 'book', + 'level' => WatchLevels::NEW, + ]; + } +} diff --git a/database/factories/Entities/Models/DeletionFactory.php b/database/factories/Entities/Models/DeletionFactory.php new file mode 100644 index 000000000..2368447cd --- /dev/null +++ b/database/factories/Entities/Models/DeletionFactory.php @@ -0,0 +1,29 @@ + + */ +class DeletionFactory extends Factory +{ + protected $model = \BookStack\Entities\Models\Deletion::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + return [ + 'deleted_by' => User::factory(), + 'deletable_id' => Page::factory(), + 'deletable_type' => 'page', + ]; + } +} diff --git a/database/factories/Entities/Models/PageFactory.php b/database/factories/Entities/Models/PageFactory.php index 47e5aa5db..538ee5f3e 100644 --- a/database/factories/Entities/Models/PageFactory.php +++ b/database/factories/Entities/Models/PageFactory.php @@ -17,10 +17,8 @@ class PageFactory extends Factory /** * Define the model's default state. - * - * @return array */ - public function definition() + public function definition(): array { $html = '

' . implode('

', $this->faker->paragraphs(5)) . '

'; diff --git a/database/factories/Entities/Models/PageRevisionFactory.php b/database/factories/Entities/Models/PageRevisionFactory.php new file mode 100644 index 000000000..333916931 --- /dev/null +++ b/database/factories/Entities/Models/PageRevisionFactory.php @@ -0,0 +1,40 @@ +' . implode('

', $this->faker->paragraphs(5)) . '

'; + $page = Page::query()->first(); + + return [ + 'page_id' => $page->id, + 'name' => $this->faker->sentence(), + 'html' => $html, + 'text' => strip_tags($html), + 'created_by' => User::factory(), + 'slug' => $page->slug, + 'book_slug' => $page->book->slug, + 'type' => 'version', + 'markdown' => strip_tags($html), + 'summary' => $this->faker->sentence(), + 'revision_number' => rand(1, 4000), + ]; + } +} diff --git a/database/migrations/2025_09_15_134751_update_entity_relation_columns.php b/database/migrations/2025_09_15_134751_update_entity_relation_columns.php index 267cd49f5..6fbeb1dd1 100644 --- a/database/migrations/2025_09_15_134751_update_entity_relation_columns.php +++ b/database/migrations/2025_09_15_134751_update_entity_relation_columns.php @@ -62,8 +62,9 @@ return new class extends Migration }); } - // Convert image zero values to null + // Convert image and activity zero values to null DB::table('images')->where('uploaded_to', '=', 0)->update(['uploaded_to' => null]); + DB::table('activities')->where('loggable_id', '=', 0)->update(['loggable_id' => null]); // Rebuild joint permissions if needed // This was moved here from 2023_01_24_104625_refactor_joint_permissions_storage since the changes diff --git a/database/migrations/2025_10_18_163331_clean_user_id_references.php b/database/migrations/2025_10_18_163331_clean_user_id_references.php new file mode 100644 index 000000000..75ff6af33 --- /dev/null +++ b/database/migrations/2025_10_18_163331_clean_user_id_references.php @@ -0,0 +1,80 @@ + ['user_id'], + 'attachments' => ['created_by', 'updated_by'], + 'comments' => ['created_by', 'updated_by'], + 'deletions' => ['deleted_by'], + 'entities' => ['created_by', 'updated_by', 'owned_by'], + 'images' => ['created_by', 'updated_by'], + 'imports' => ['created_by'], + 'joint_permissions' => ['owner_id'], + 'page_revisions' => ['created_by'], + ]; + + protected static array $toClean = [ + 'api_tokens' => ['user_id'], + 'email_confirmations' => ['user_id'], + 'favourites' => ['user_id'], + 'mfa_values' => ['user_id'], + 'role_user' => ['user_id'], + 'sessions' => ['user_id'], + 'social_accounts' => ['user_id'], + 'user_invites' => ['user_id'], + 'views' => ['user_id'], + 'watches' => ['user_id'], + ]; + + /** + * Run the migrations. + */ + public function up(): void + { + $idSelectQuery = DB::table('users')->select('id'); + + foreach (self::$toNullify as $tableName => $columns) { + Schema::table($tableName, function (Blueprint $table) use ($columns) { + foreach ($columns as $columnName) { + $table->unsignedInteger($columnName)->nullable()->change(); + } + }); + + foreach ($columns as $columnName) { + DB::table($tableName)->where($columnName, '=', 0)->update([$columnName => null]); + DB::table($tableName)->whereNotIn($columnName, $idSelectQuery)->update([$columnName => null]); + } + } + + foreach (self::$toClean as $tableName => $columns) { + foreach ($columns as $columnName) { + 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 + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + foreach (self::$toNullify as $tableName => $columns) { + foreach ($columns as $columnName) { + DB::table($tableName)->whereNull($columnName)->update([$columnName => 0]); + } + + Schema::table($tableName, function (Blueprint $table) use ($columns) { + foreach ($columns as $columnName) { + $table->unsignedInteger($columnName)->nullable(false)->change(); + } + }); + } + } +}; diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php index 6d8b4d75a..a34a9e7f4 100644 --- a/tests/User/UserManagementTest.php +++ b/tests/User/UserManagementTest.php @@ -2,9 +2,21 @@ namespace Tests\User; +use BookStack\Access\Mfa\MfaValue; +use BookStack\Access\SocialAccount; use BookStack\Access\UserInviteException; use BookStack\Access\UserInviteService; use BookStack\Activity\ActivityType; +use BookStack\Activity\Models\Activity; +use BookStack\Activity\Models\Comment; +use BookStack\Activity\Models\Favourite; +use BookStack\Activity\Models\View; +use BookStack\Activity\Models\Watch; +use BookStack\Api\ApiToken; +use BookStack\Entities\Models\Deletion; +use BookStack\Entities\Models\PageRevision; +use BookStack\Exports\Import; +use BookStack\Uploads\Attachment; use BookStack\Uploads\Image; use BookStack\Users\Models\Role; use BookStack\Users\Models\User; @@ -28,10 +40,10 @@ class UserManagementTest extends TestCase $this->withHtml($resp)->assertElementContains('form[action="' . url('/settings/users/create') . '"]', 'Save'); $resp = $this->post('/settings/users/create', [ - 'name' => $user->name, - 'email' => $user->email, - 'password' => $user->password, - 'password-confirm' => $user->password, + 'name' => $user->name, + 'email' => $user->email, + 'password' => $user->password, + 'password-confirm' => $user->password, 'roles[' . $adminRole->id . ']' => 'true', ]); $resp->assertRedirect('/settings/users'); @@ -77,7 +89,7 @@ class UserManagementTest extends TestCase $this->get($userProfilePage)->assertSee('Password confirmation required'); $this->put($userProfilePage, [ - 'password' => 'newpassword', + 'password' => 'newpassword', 'password-confirm' => 'newpassword', ])->assertRedirect('/settings/users'); @@ -167,7 +179,7 @@ class UserManagementTest extends TestCase $this->asAdmin()->delete("settings/users/{$owner->id}", ['new_owner_id' => $newOwner->id])->assertRedirect(); $this->assertDatabaseHasEntityData('page', [ - 'id' => $page->id, + 'id' => $page->id, 'owned_by' => $newOwner->id, ]); } @@ -182,6 +194,90 @@ class UserManagementTest extends TestCase $this->assertSessionHas('success'); } + public function test_delete_with_empty_owner_migration_id_clears_relevant_id_uses() + { + $user = $this->users->editor(); + $page = $this->entities->page(); + $this->actingAs($user); + + // Create relations + $activity = Activity::factory()->create(['user_id' => $user->id]); + $attachment = Attachment::factory()->create(['created_by' => $user->id, 'updated_by' => $user->id]); + $comment = Comment::factory()->create(['created_by' => $user->id, 'updated_by' => $user->id]); + $deletion = Deletion::factory()->create(['deleted_by' => $user->id]); + $page->forceFill(['owned_by' => $user->id, 'created_by' => $user->id, 'updated_by' => $user->id])->save(); + $page->rebuildPermissions(); + $image = Image::factory()->create(['created_by' => $user->id, 'updated_by' => $user->id]); + $import = Import::factory()->create(['created_by' => $user->id]); + $revision = PageRevision::factory()->create(['created_by' => $user->id]); + + $apiToken = ApiToken::factory()->create(['user_id' => $user->id]); + \DB::table('email_confirmations')->insert(['user_id' => $user->id, 'token' => 'abc123']); + $favourite = Favourite::factory()->create(['user_id' => $user->id]); + $mfaValue = MfaValue::factory()->create(['user_id' => $user->id]); + $socialAccount = SocialAccount::factory()->create(['user_id' => $user->id]); + \DB::table('user_invites')->insert(['user_id' => $user->id, 'token' => 'abc123']); + View::incrementFor($page); + $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'], + 'deletions' => ['deleted_by'], + 'email_confirmations' => ['user_id'], + 'entities' => ['created_by', 'updated_by', 'owned_by'], + 'favourites' => ['user_id'], + 'images' => ['created_by', 'updated_by'], + 'imports' => ['created_by'], + 'joint_permissions' => ['owner_id'], + 'mfa_values' => ['user_id'], + 'page_revisions' => ['created_by'], + 'role_user' => ['user_id'], + 'social_accounts' => ['user_id'], + 'user_invites' => ['user_id'], + 'views' => ['user_id'], + 'watches' => ['user_id'], + ]; + + // Ensure columns have user id before deletion + foreach ($userColumnsByTable as $table => $columns) { + foreach ($columns as $column) { + $this->assertDatabaseHas($table, [$column => $user->id]); + } + } + + $resp = $this->asAdmin()->delete("settings/users/{$user->id}", ['new_owner_id' => '']); + $resp->assertRedirect('/settings/users'); + + // Ensure columns missing user id after deletion + foreach ($userColumnsByTable as $table => $columns) { + foreach ($columns as $column) { + $this->assertDatabaseMissing($table, [$column => $user->id]); + } + } + + // 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]); + $this->assertDatabaseHas('entities', ['id' => $page->id, 'created_by' => null, 'updated_by' => null, 'owned_by' => null]); + $this->assertDatabaseHas('images', ['id' => $image->id, 'created_by' => null, 'updated_by' => null]); + $this->assertDatabaseHas('imports', ['id' => $import->id, 'created_by' => null]); + $this->assertDatabaseHas('page_revisions', ['id' => $revision->id, 'created_by' => null]); + + // Check models no longer exist where should have been deleted with the user + $this->assertDatabaseMissing('api_tokens', ['id' => $apiToken->id]); + $this->assertDatabaseMissing('email_confirmations', ['token' => 'abc123']); + $this->assertDatabaseMissing('favourites', ['id' => $favourite->id]); + $this->assertDatabaseMissing('mfa_values', ['id' => $mfaValue->id]); + $this->assertDatabaseMissing('social_accounts', ['id' => $socialAccount->id]); + $this->assertDatabaseMissing('user_invites', ['token' => 'abc123']); + $this->assertDatabaseMissing('watches', ['id' => $watch->id]); + } + public function test_delete_removes_user_preferences() { $editor = $this->users->editor(); @@ -247,9 +343,9 @@ class UserManagementTest extends TestCase }); $this->asAdmin()->post('/settings/users/create', [ - 'name' => $user->name, - 'email' => $user->email, - 'send_invite' => 'true', + 'name' => $user->name, + 'email' => $user->email, + 'send_invite' => 'true', 'roles[' . $adminRole->id . ']' => 'true', ]); @@ -267,9 +363,9 @@ class UserManagementTest extends TestCase }); $this->asAdmin()->post('/settings/users/create', [ - 'name' => $user->name, - 'email' => $user->email, - 'send_invite' => 'true', + 'name' => $user->name, + 'email' => $user->email, + 'send_invite' => 'true', ]); $this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']); @@ -286,9 +382,9 @@ class UserManagementTest extends TestCase }); $resp = $this->asAdmin()->post('/settings/users/create', [ - 'name' => $user->name, - 'email' => $user->email, - 'send_invite' => 'true', + 'name' => $user->name, + 'email' => $user->email, + 'send_invite' => 'true', ]); $resp->assertRedirect('/settings/users/create'); @@ -314,8 +410,8 @@ class UserManagementTest extends TestCase // Both on create $resp = $this->post('/settings/users/create', [ 'language' => 'en 'My name', - 'email' => 'jimmy@example.com', + 'name' => 'My name', + 'email' => 'jimmy@example.com', ]); $resp->assertSessionHasErrors(['language' => 'The language may not be greater than 15 characters.']); $resp->assertSessionHasErrors(['language' => 'The language may only contain letters, numbers, dashes and underscores.']);