From 33df9e49aba82cd67157a1006af6b5fc3f54b50a Mon Sep 17 00:00:00 2001 From: Twyco Date: Thu, 24 Sep 2026 15:02:05 +0200 Subject: [PATCH] Keep guest rooms moderated and restore owner moderation --- app/Jobs/RemoveAbsentPlayers.php | 41 +++++++- .../Rooms/RoomParticipationService.php | 25 ++++- ...ncile_guest_rooms_after_player_cleanup.php | 48 ++++++++++ resources/js/composables/useRoomRealtime.ts | 11 +++ tests/Feature/AbsentPlayerRemovalTest.php | 94 ++++++++++++++++++- tests/Feature/EmbeddedRoomTest.php | 59 +++++++++++- 6 files changed, 267 insertions(+), 11 deletions(-) create mode 100644 database/migrations/2026_09_24_125529_reconcile_guest_rooms_after_player_cleanup.php diff --git a/app/Jobs/RemoveAbsentPlayers.php b/app/Jobs/RemoveAbsentPlayers.php index 7ae5fdb..f7f4d1c 100644 --- a/app/Jobs/RemoveAbsentPlayers.php +++ b/app/Jobs/RemoveAbsentPlayers.php @@ -30,7 +30,7 @@ class RemoveAbsentPlayers implements ShouldQueue ->select(['id', 'room_id']) ->chunkById(100, function ($players) use ($cutoff, $realtimeState): void { foreach ($players as $player) { - $room = DB::transaction(function () use ($player, $cutoff): ?Room { + $result = DB::transaction(function () use ($player, $cutoff): ?array { $room = Room::withTrashed() ->lockForUpdate() ->find($player->room_id); @@ -54,14 +54,49 @@ class RemoveAbsentPlayers implements ShouldQueue } $currentPlayer->delete(); + $roomDeleted = false; + + if ($room->owner_id === null && ! $room->trashed()) { + if (! $room->players()->exists()) { + $room->delete(); + $roomDeleted = true; + } elseif (! $room->players()->where('is_moderator', true)->exists()) { + $successor = $room->players() + ->where(function ($query) use ($cutoff): void { + $query->where('last_seen_at', '>', $cutoff) + ->orWhere(function ($query) use ($cutoff): void { + $query->whereNull('last_seen_at') + ->where('created_at', '>', $cutoff); + }); + }) + ->orderBy('created_at') + ->orderBy('id') + ->first(); + + $successor?->update(['is_moderator' => true]); + } + } + $room->update([ 'state_version' => $room->state_version + 1, ]); - return $room; + return [$room, $roomDeleted]; }); - if ($room !== null && ! $room->trashed()) { + if ($result === null) { + continue; + } + + [$room, $roomDeleted] = $result; + + if ($roomDeleted) { + RoomStateChanged::dispatch( + $room->id, + $room->state_version, + RoomStateChange::RoomDeleted, + ); + } elseif (! $room->trashed()) { RoomStateChanged::dispatch( $room->id, $room->state_version, diff --git a/app/Services/Rooms/RoomParticipationService.php b/app/Services/Rooms/RoomParticipationService.php index f00d80f..7c53ad6 100644 --- a/app/Services/Rooms/RoomParticipationService.php +++ b/app/Services/Rooms/RoomParticipationService.php @@ -69,17 +69,19 @@ class RoomParticipationService $user, $sessionPlayerIds[$room->id] ?? null, ); + $shouldModerate = $this->shouldModerate($room, $user); if ($player === null) { $player = $room->players()->create([ 'user_id' => $user?->id, 'name' => $data['player_name'], - 'is_moderator' => false, + 'is_moderator' => $shouldModerate, 'last_seen_at' => now(), ]); } else { $player->update([ 'last_seen_at' => now(), + 'is_moderator' => $player->is_moderator || $shouldModerate, ]); } @@ -123,6 +125,7 @@ class RoomParticipationService $user, $sessionPlayerId, ); + $shouldModerate = $this->shouldModerate($lockedRoom, $user); $nameIsTaken = $lockedRoom->players() ->where('name', $playerName) @@ -144,18 +147,19 @@ class RoomParticipationService $player = $lockedRoom->players()->create([ 'user_id' => $user?->id, 'name' => $playerName, - 'is_moderator' => false, + 'is_moderator' => $shouldModerate, 'last_seen_at' => now(), ]); $change = RoomStateChange::PlayerJoined; } else { - if ($player->name !== $playerName) { + if ($player->name !== $playerName || ($shouldModerate && ! $player->is_moderator)) { $change = RoomStateChange::PlayerUpdated; } $updates = [ 'name' => $playerName, 'last_seen_at' => now(), + 'is_moderator' => $player->is_moderator || $shouldModerate, ]; if ( @@ -196,6 +200,16 @@ class RoomParticipationService return $player; } + private function shouldModerate(Room $room, ?User $user): bool + { + if ($user !== null && $room->owner_id === $user->id) { + return true; + } + + return $room->owner_id === null + && ! $room->players()->where('is_moderator', true)->exists(); + } + private function findExistingEmbeddedPlayer( Room $room, ?User $user, @@ -218,7 +232,10 @@ class RoomParticipationService ->lockForUpdate() ->first(); - if ($sessionPlayer !== null) { + if ( + $sessionPlayer !== null + && ($user === null || $sessionPlayer->user_id === null || $sessionPlayer->user_id === $user->id) + ) { return $sessionPlayer; } } diff --git a/database/migrations/2026_09_24_125529_reconcile_guest_rooms_after_player_cleanup.php b/database/migrations/2026_09_24_125529_reconcile_guest_rooms_after_player_cleanup.php new file mode 100644 index 0000000..f42ddc4 --- /dev/null +++ b/database/migrations/2026_09_24_125529_reconcile_guest_rooms_after_player_cleanup.php @@ -0,0 +1,48 @@ +whereNull('owner_id') + ->whereNull('deleted_at') + ->whereNotIn('id', DB::table('players')->select('room_id')) + ->update(['deleted_at' => now()]); + + DB::table('rooms') + ->whereNull('owner_id') + ->whereNull('deleted_at') + ->whereNotIn('id', DB::table('players')->where('is_moderator', true)->select('room_id')) + ->select('id') + ->chunkById(100, function ($rooms): void { + foreach ($rooms as $room) { + $playerId = DB::table('players') + ->where('room_id', $room->id) + ->orderBy('created_at') + ->orderBy('id') + ->value('id'); + + if ($playerId !== null) { + DB::table('players') + ->where('id', $playerId) + ->update(['is_moderator' => true]); + } + } + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + // Deleted rooms and former moderator assignments cannot be reconstructed. + } +}; diff --git a/resources/js/composables/useRoomRealtime.ts b/resources/js/composables/useRoomRealtime.ts index 5760342..b7739e4 100644 --- a/resources/js/composables/useRoomRealtime.ts +++ b/resources/js/composables/useRoomRealtime.ts @@ -70,6 +70,17 @@ export function useRoomRealtime( ...payload.state, }; + const currentRoomPlayer = room.value.players.find( + (player) => player.id === currentPlayer.value.id, + ); + + if (currentRoomPlayer !== undefined) { + currentPlayer.value = { + ...currentPlayer.value, + is_moderator: currentRoomPlayer.is_moderator, + }; + } + if ( payload.change === 'votes_reset' || payload.change === 'next_round' diff --git a/tests/Feature/AbsentPlayerRemovalTest.php b/tests/Feature/AbsentPlayerRemovalTest.php index acfcc81..64f1fa6 100644 --- a/tests/Feature/AbsentPlayerRemovalTest.php +++ b/tests/Feature/AbsentPlayerRemovalTest.php @@ -4,6 +4,7 @@ use App\Enums\RoomStateChange; use App\Events\RoomStateChanged; use App\Jobs\RemoveAbsentPlayers; use App\Models\Room; +use App\Models\User; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; @@ -13,7 +14,7 @@ test('the player table no longer stores an inactive status', function () { expect(Schema::hasColumn('players', 'inactive_at'))->toBeFalse(); }); -test('the migration removes old inactive and absent players while keeping present players', function () { +test('migrations remove absent players and reconcile guest rooms', function () { Schema::table('players', function (Blueprint $table): void { $table->timestamp('inactive_at')->nullable(); }); @@ -34,15 +35,31 @@ test('the migration removes old inactive and absent players while keeping presen 'name' => 'Anna', 'last_seen_at' => now(), ]); + $emptyGuestRoom = Room::create([ + 'title' => 'Empty guest room', + 'deck_config' => ['name' => 'Fibonacci', 'cards' => []], + ]); + $owner = User::factory()->create(); + $emptyAccountRoom = Room::create([ + 'owner_id' => $owner->id, + 'is_persistent' => true, + 'title' => 'Persistent account room', + 'deck_config' => ['name' => 'Fibonacci', 'cards' => []], + ]); DB::table('players')->where('id', $previouslyInactive->id)->update(['inactive_at' => now()]); $migration = require database_path('migrations/2026_09_24_123048_remove_previously_inactive_players.php'); $migration->up(); + $guestRoomMigration = require database_path('migrations/2026_09_24_125529_reconcile_guest_rooms_after_player_cleanup.php'); + $guestRoomMigration->up(); $this->assertModelMissing($previouslyInactive); $this->assertModelMissing($absent); $this->assertModelExists($present); + $this->assertSoftDeleted($emptyGuestRoom); + $this->assertModelExists($emptyAccountRoom); + expect($present->fresh()->is_moderator)->toBeTrue(); expect(Schema::hasColumn('players', 'inactive_at'))->toBeFalse(); }); @@ -54,6 +71,7 @@ test('the timeout removes absent players and their votes but keeps recent player ]); $absentPlayer = $room->players()->create([ 'name' => 'Nico', + 'is_moderator' => true, 'last_seen_at' => now()->subHour(), ]); $recentPlayer = $room->players()->create([ @@ -76,12 +94,14 @@ test('the timeout removes absent players and their votes but keeps recent player $this->assertModelMissing($vote); $this->assertModelExists($recentPlayer); expect($room->fresh()->state_version)->toBe($previousVersion + 1); + expect($recentPlayer->fresh()->is_moderator)->toBeTrue(); Event::assertDispatched( RoomStateChanged::class, fn (RoomStateChanged $event): bool => $event->change === RoomStateChange::PlayerKicked && $event->affectedPlayerId === $absentPlayer->id - && collect($event->state['players'])->doesntContain('id', $absentPlayer->id), + && collect($event->state['players'])->doesntContain('id', $absentPlayer->id) + && collect($event->state['players'])->firstWhere('id', $recentPlayer->id)['is_moderator'] === true, ); Event::assertDispatchedTimes(RoomStateChanged::class, 1); @@ -92,3 +112,73 @@ test('the timeout removes absent players and their votes but keeps recent player expect($room->players()->where('name', 'Nico')->sole()->id)->not->toBe($absentPlayer->id); }); + +test('the timeout deletes a guest room when its last player is removed', function () { + $room = Room::create([ + 'title' => 'Guest planning', + 'deck_config' => ['name' => 'Fibonacci', 'cards' => []], + ]); + $moderator = $room->players()->create([ + 'name' => 'Nico', + 'is_moderator' => true, + 'last_seen_at' => now()->subHour(), + ]); + + Event::fake([RoomStateChanged::class]); + + app()->call([new RemoveAbsentPlayers, 'handle']); + app()->call([new RemoveAbsentPlayers, 'handle']); + + $this->assertModelMissing($moderator); + $this->assertSoftDeleted($room); + Event::assertDispatched( + RoomStateChanged::class, + fn (RoomStateChanged $event): bool => $event->roomId === $room->id + && $event->change === RoomStateChange::RoomDeleted, + ); + Event::assertDispatchedTimes(RoomStateChanged::class, 1); + + $this->post(route('room.code'), [ + 'join_code' => $room->join_code, + 'player_name' => 'Nico', + ])->assertSessionHasErrors('join_code'); +}); + +test('the owner regains moderation when joining a persistent room after a timeout', function () { + $owner = User::factory()->create(); + $room = Room::create([ + 'owner_id' => $owner->id, + 'is_persistent' => true, + 'title' => 'Permanent planning', + 'deck_config' => ['name' => 'Fibonacci', 'cards' => []], + ]); + $oldModerator = $room->players()->create([ + 'user_id' => $owner->id, + 'name' => 'Nico', + 'is_moderator' => true, + 'last_seen_at' => now()->subHour(), + ]); + $guest = $room->players()->create([ + 'name' => 'Anna', + 'last_seen_at' => now(), + ]); + + app()->call([new RemoveAbsentPlayers, 'handle']); + + $this->assertModelMissing($oldModerator); + $this->assertModelExists($room); + expect($guest->fresh()->is_moderator)->toBeFalse(); + + $this->actingAs($owner) + ->withSession(["room_players.{$room->id}" => $oldModerator->id]) + ->post(route('room.code'), [ + 'join_code' => $room->join_code, + 'player_name' => 'Nico', + ]) + ->assertRedirect(route('room.show', $room)); + + $newModerator = $room->players()->where('user_id', $owner->id)->sole(); + + expect($newModerator->id)->not->toBe($oldModerator->id) + ->and($newModerator->is_moderator)->toBeTrue(); +}); diff --git a/tests/Feature/EmbeddedRoomTest.php b/tests/Feature/EmbeddedRoomTest.php index d74355d..4925b2c 100644 --- a/tests/Feature/EmbeddedRoomTest.php +++ b/tests/Feature/EmbeddedRoomTest.php @@ -2,6 +2,7 @@ use App\Enums\RoomStateChange; use App\Events\RoomStateChanged; +use App\Jobs\RemoveAbsentPlayers; use App\Models\Room; use App\Models\User; use Illuminate\Support\Facades\Event; @@ -37,7 +38,7 @@ test('a guest automatically joins the embedded room', function () { $player = $room->players()->sole(); expect($player->user_id)->toBeNull() - ->and($player->is_moderator)->toBeFalse(); + ->and($player->is_moderator)->toBeTrue(); $response->assertSessionHas("room_players.{$room->id}", $player->id); }); @@ -80,10 +81,64 @@ test('an authenticated user automatically joins the embedded room', function () 'room_id' => $room->id, 'user_id' => $user->id, 'name' => 'Nico', - 'is_moderator' => false, + 'is_moderator' => true, ]); }); +test('a persistent room owner regains moderation through the embedded room', function () { + $owner = User::factory()->create(); + $room = createEmbeddedRoom([ + 'owner_id' => $owner->id, + 'is_persistent' => true, + ]); + $oldModerator = $room->players()->create([ + 'user_id' => $owner->id, + 'name' => 'Nico', + 'is_moderator' => true, + 'last_seen_at' => now()->subHour(), + ]); + + app()->call([new RemoveAbsentPlayers, 'handle']); + + $this->actingAs($owner) + ->withSession(["room_players.{$room->id}" => $oldModerator->id]) + ->get(route('embedded.room', [ + 'join_code' => $room->join_code, + 'player_name' => 'Nico', + ])) + ->assertOk() + ->assertInertia(fn (Assert $page) => $page + ->where('currentPlayer.is_moderator', true)); + + expect($room->players()->where('user_id', $owner->id)->sole()->id)->not->toBe($oldModerator->id); +}); + +test('a persistent room owner does not take over another account session when becoming moderator', function () { + $owner = User::factory()->create(); + $otherUser = User::factory()->create(); + $room = createEmbeddedRoom([ + 'owner_id' => $owner->id, + 'is_persistent' => true, + ]); + $otherPlayer = $room->players()->create([ + 'user_id' => $otherUser->id, + 'name' => 'Anna', + ]); + + $this->actingAs($owner) + ->withSession(["room_players.{$room->id}" => $otherPlayer->id]) + ->get(route('embedded.room', [ + 'join_code' => $room->join_code, + 'player_name' => 'Nico', + ])) + ->assertOk() + ->assertInertia(fn (Assert $page) => $page + ->where('currentPlayer.is_moderator', true)); + + expect($otherPlayer->fresh()->is_moderator)->toBeFalse() + ->and($room->players()->where('user_id', $owner->id)->sole()->is_moderator)->toBeTrue(); +}); + test('embedded access reuses the user player and applies a changed display name', function () { $user = User::factory()->create(); $room = createEmbeddedRoom();