From 9a5608b967815680fc819af64ce07204ebeb533b Mon Sep 17 00:00:00 2001 From: nanaya Date: Fri, 7 Apr 2023 10:39:52 +0900 Subject: [PATCH 1/8] Add class based factory for Multiplayer models --- .../Multiplayer/PlaylistItemFactory.php | 44 ++++++---- .../factories/Multiplayer/RoomFactory.php | 58 +++++++----- .../factories/Multiplayer/ScoreFactory.php | 88 +++++++++---------- .../ModelSeeders/MultiplayerSeeder.php | 9 +- tests/Browser/SanityTest.php | 2 +- .../Chat/ChannelsControllerTest.php | 8 +- .../Rooms/Playlist/ScoresControllerTest.php | 6 +- .../Multiplayer/RoomsControllerTest.php | 12 +-- tests/Models/ContestTest.php | 12 +-- tests/Models/Multiplayer/RoomTest.php | 26 +++--- .../Multiplayer/UserScoreAggregateTest.php | 82 ++++++++--------- 11 files changed, 182 insertions(+), 165 deletions(-) diff --git a/database/factories/Multiplayer/PlaylistItemFactory.php b/database/factories/Multiplayer/PlaylistItemFactory.php index c73a17e1a02..01ef8fdf95c 100644 --- a/database/factories/Multiplayer/PlaylistItemFactory.php +++ b/database/factories/Multiplayer/PlaylistItemFactory.php @@ -3,21 +3,29 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. -$factory->define(App\Models\Multiplayer\PlaylistItem::class, function (Faker\Generator $faker) { - return [ - 'beatmap_id' => function () { - return App\Models\Beatmap::factory()->create()->getKey(); - }, - 'room_id' => function () { - return factory(App\Models\Multiplayer\Room::class)->create()->getKey(); - }, - 'ruleset_id' => function (array $self) { - return App\Models\Beatmap::find($self['beatmap_id'])->playmode; - }, - 'allowed_mods' => [], - 'required_mods' => [], - 'owner_id' => function () { - return App\Models\User::factory()->create()->getKey(); - }, - ]; -}); +declare(strict_types=1); + +namespace Database\Factories\Multiplayer; + +use App\Models\Beatmap; +use App\Models\Multiplayer\PlaylistItem; +use App\Models\Multiplayer\Room; +use App\Models\User; +use Database\Factories\Factory; + +class PlaylistItemFactory extends Factory +{ + protected $model = PlaylistItem::class; + + public function definition(): array + { + return [ + 'beatmap_id' => Beatmap::factory(), + 'room_id' => Room::factory(), + 'ruleset_id' => fn(array $attributes) => Beatmap::find($attributes['beatmap_id'])->playmode, + 'allowed_mods' => [], + 'required_mods' => [], + 'owner_id' => User::factory(), + ]; + } +} diff --git a/database/factories/Multiplayer/RoomFactory.php b/database/factories/Multiplayer/RoomFactory.php index 9820be8ab02..03edf0d650f 100644 --- a/database/factories/Multiplayer/RoomFactory.php +++ b/database/factories/Multiplayer/RoomFactory.php @@ -3,29 +3,41 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + +namespace Database\Factories\Multiplayer; + use App\Models\Chat\Channel; use App\Models\Multiplayer\Room; +use App\Models\User; +use Carbon\Carbon; +use Database\Factories\Factory; + +class RoomFactory extends Factory +{ + protected $model = Room::class; + + public function configure(): static + { + return $this->afterCreating(function (Room $room) { + $channel = Channel::createMultiplayer($room); + + $room->update(['channel_id' => $channel->getKey()]); + }); + } + + public function definition(): array + { + return [ + 'user_id' => User::factory(), + 'name' => fn() => $this->faker->realText(20), + 'starts_at' => fn() => Carbon::now()->subHours(1), + 'ends_at' => fn() => Carbon::now()->addHours(1), + ]; + } -$factory->define(Room::class, function (Faker\Generator $faker) { - return [ - 'user_id' => function (array $self) { - return App\Models\User::factory()->create()->getKey(); - }, - 'name' => function () use ($faker) { - return $faker->realText(20); - }, - 'starts_at' => Carbon\Carbon::now()->subHours(1), - 'ends_at' => Carbon\Carbon::now()->addHours(1), - ]; -}); - -$factory->state(Room::class, 'ended', function (Faker\Generator $faker) { - return [ - 'ends_at' => Carbon\Carbon::now()->subMinutes(1), - ]; -}); - -$factory->afterCreating(Room::class, function (Room $room, $faker) { - $channel = Channel::createMultiplayer($room); - $room->update(['channel_id' => $channel->getKey()]); -}); + public function ended(): static + { + return $this->state(['ends_at' => Carbon::now()->subMinutes(1)]); + } +} diff --git a/database/factories/Multiplayer/ScoreFactory.php b/database/factories/Multiplayer/ScoreFactory.php index d19d9e4971f..e72320d3713 100644 --- a/database/factories/Multiplayer/ScoreFactory.php +++ b/database/factories/Multiplayer/ScoreFactory.php @@ -3,51 +3,51 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + +namespace Database\Factories\Multiplayer; + use App\Models\Multiplayer\PlaylistItem; use App\Models\Multiplayer\Score; use App\Models\User; +use Carbon\Carbon; +use Database\Factories\Factory; + +class ScoreFactory extends Factory +{ + protected $model = Score::class; + + public function completed(): static + { + return $this->state(['ended_at' => Carbon::now()->subMinutes(1)]); + } + + public function definition(): array + { + return [ + 'playlist_item_id' => PlaylistItem::factory(), + 'beatmap_id' => fn(array $attributes) => PlaylistItem::find($attributes['playlist_item_id'])->beatmap_id, + 'room_id' => fn(array $attributes) => PlaylistItem::find($attributes['playlist_item_id'])->room_id, + 'user_id' => User::factory(), + 'total_score' => 1, + 'started_at' => fn() => Carbon::now()->subMinutes(5), + 'accuracy' => 0.5, + 'pp' => 1, + ]; + } + + public function failed(): static + { + return $this->completed()->state(['passed' => false]); + } + + public function passed(): static + { + return $this->completed()->state(['passed' => true]); + } -$factory->define(Score::class, function (Faker\Generator $faker) { - return [ - 'playlist_item_id' => function () { - return factory(PlaylistItem::class)->create()->getKey(); - }, - 'beatmap_id' => function (array $self) { - return PlaylistItem::find($self['playlist_item_id'])->beatmap_id; - }, - 'room_id' => function (array $self) { - return PlaylistItem::find($self['playlist_item_id'])->room_id; - }, - 'user_id' => function () { - return User::factory()->create()->getKey(); - }, - 'total_score' => 1, - 'started_at' => now()->subMinutes(5), - 'accuracy' => 0.5, - 'pp' => 1, - ]; -}); - -$factory->state(Score::class, 'completed', function (Faker\Generator $faker) { - return [ - 'ended_at' => now(), - ]; -}); - -$factory->state(Score::class, 'passed', function (Faker\Generator $faker) { - return [ - 'passed' => true, - ]; -}); - -$factory->state(Score::class, 'failed', function (Faker\Generator $faker) { - return [ - 'passed' => false, - ]; -}); - -$factory->state(Score::class, 'scoreless', function (Faker\Generator $faker) { - return [ - 'total_score' => 0, - ]; -}); + public function scoreless(): static + { + return $this->state(['total_score' => 0]); + } +} diff --git a/database/seeders/ModelSeeders/MultiplayerSeeder.php b/database/seeders/ModelSeeders/MultiplayerSeeder.php index 988f9ffafd3..89562aea728 100644 --- a/database/seeders/ModelSeeders/MultiplayerSeeder.php +++ b/database/seeders/ModelSeeders/MultiplayerSeeder.php @@ -3,6 +3,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + namespace Database\Seeders\ModelSeeders; use App\Models\Beatmap; @@ -22,15 +24,14 @@ class MultiplayerSeeder extends Seeder */ public function run() { - $rooms = factory(Room::class, 10)->create(); + $rooms = Room::factory()->count(10)->create(); foreach ($rooms as $room) { $beatmaps = Beatmap::orderByRaw('RAND()')->limit(rand(4, 10))->get(); foreach ($beatmaps as $beatmap) { - $playlistItem = factory(PlaylistItem::class)->create([ + $playlistItem = PlaylistItem::factory()->create([ 'room_id' => $room->getKey(), 'beatmap_id' => $beatmap->getKey(), - 'ruleset_id' => $beatmap->playmode, ]); $users = User::orderByRaw('RAND()')->limit(rand(4, 10))->get(); @@ -38,7 +39,7 @@ public function run() $attempts = rand(1, 10); for ($i = 0; $i < $attempts; $i++) { $completed = rand(0, 100) > 20; - factory(Score::class)->create([ + Score::factory()->create([ 'playlist_item_id' => $playlistItem->getKey(), 'user_id' => $user->getKey(), 'beatmap_id' => $beatmap->getKey(), diff --git a/tests/Browser/SanityTest.php b/tests/Browser/SanityTest.php index ddba9a1116e..ff67be3c89f 100644 --- a/tests/Browser/SanityTest.php +++ b/tests/Browser/SanityTest.php @@ -253,7 +253,7 @@ private static function createScaffolding() // score factory self::$scaffolding['score'] = Score\Best\Osu::factory()->withReplay()->create(); - self::$scaffolding['room'] = factory(Room::class)->create(['category' => 'spotlight']); + self::$scaffolding['room'] = Room::factory()->create(['category' => 'spotlight']); app('groups')->resetMemoized(); } diff --git a/tests/Controllers/Chat/ChannelsControllerTest.php b/tests/Controllers/Chat/ChannelsControllerTest.php index fc70efd2eba..0bbb8ae439a 100644 --- a/tests/Controllers/Chat/ChannelsControllerTest.php +++ b/tests/Controllers/Chat/ChannelsControllerTest.php @@ -189,7 +189,7 @@ public function testChannelJoinPM() // fail public function testChannelJoinMultiplayerWhenNotParticipated() { - $score = factory(Score::class)->create(); + $score = Score::factory()->create(); $this->actAsScopedUser($this->user, ['*']); $request = $this->json('PUT', route('api.chat.channels.join', [ @@ -202,7 +202,7 @@ public function testChannelJoinMultiplayerWhenNotParticipated() public function testChannelJoinMultiplayerWhenParticipated() { - $score = factory(Score::class)->create(['user_id' => $this->user->getKey()]); + $score = Score::factory()->create(['user_id' => $this->user]); $this->actAsScopedUser($this->user, ['*']); $request = $this->json('PUT', route('api.chat.channels.join', [ @@ -269,7 +269,7 @@ public function testChannelMarkAsReadWhenJoined() // success public function testChannelMarkAsReadBackwards() // success (with no change) { - $newerPublicMessage = Message::factory()->create(['channel_id' => $this->publicChannel->channel_id]); + $newerPublicMessage = Message::factory()->create(['channel_id' => $this->publicChannel]); $this->actAsScopedUser($this->user, ['*']); $this->json('PUT', route('api.chat.channels.join', [ @@ -401,7 +401,7 @@ protected function setUp(): void $this->publicChannel = Channel::factory()->type('public')->create(); $this->privateChannel = Channel::factory()->type('private')->create(); $this->pmChannel = Channel::factory()->type('pm')->create(); - $this->publicMessage = Message::factory()->create(['channel_id' => $this->publicChannel->channel_id]); + $this->publicMessage = Message::factory()->create(['channel_id' => $this->publicChannel]); } private function getAssertableChannelList(User $user): AssertableJsonString diff --git a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php index 9457dea175f..dc4e6ceec0f 100644 --- a/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php +++ b/tests/Controllers/Multiplayer/Rooms/Playlist/ScoresControllerTest.php @@ -15,7 +15,7 @@ class ScoresControllerTest extends TestCase { public function testShow() { - $score = factory(Score::class)->create(); + $score = Score::factory()->create(); $user = User::factory()->create(); $this->actAsScopedUser($user, ['*']); @@ -33,7 +33,7 @@ public function testShow() public function testStore($allowRanking, $hashParam, $status) { $user = User::factory()->create(); - $playlistItem = factory(PlaylistItem::class)->create(); + $playlistItem = PlaylistItem::factory()->create(); $build = Build::factory()->create(['allow_ranking' => $allowRanking]); $initialScoresCount = Score::count(); @@ -60,7 +60,7 @@ public function testStore($allowRanking, $hashParam, $status) public function testUpdate($bodyParams, $status) { $user = User::factory()->create(); - $playlistItem = factory(PlaylistItem::class)->create(); + $playlistItem = PlaylistItem::factory()->create(); $room = $playlistItem->room; $build = Build::factory()->create(['allow_ranking' => true]); $score = $room->startPlay($user, $playlistItem); diff --git a/tests/Controllers/Multiplayer/RoomsControllerTest.php b/tests/Controllers/Multiplayer/RoomsControllerTest.php index e4926dd433b..1bcfde01f86 100644 --- a/tests/Controllers/Multiplayer/RoomsControllerTest.php +++ b/tests/Controllers/Multiplayer/RoomsControllerTest.php @@ -18,7 +18,7 @@ class RoomsControllerTest extends TestCase { public function testIndex() { - $room = factory(Room::class)->create(); + $room = Room::factory()->create(); $user = User::factory()->create(); $this->actAsScopedUser($user, ['*']); @@ -284,7 +284,7 @@ public function testStorePlaylistsAllowance() $user = $token->user; for ($i = 0; $i < $user->maxMultiplayerRooms(); $i++) { - factory(Room::class)->create(['user_id' => $user]); + Room::factory()->create(['user_id' => $user]); } $roomsCountInitial = Room::count(); @@ -305,7 +305,7 @@ public function testStorePlaylistsAllowanceSeparateFromRealtime() { $token = Token::factory()->create(['scopes' => ['*']]); $user = $token->user; - factory(Room::class)->create(['user_id' => $user, 'type' => Room::REALTIME_DEFAULT_TYPE]); + Room::factory()->create(['user_id' => $user, 'type' => Room::REALTIME_DEFAULT_TYPE]); $roomsCountInitial = Room::count(); $playlistItemsCountInitial = PlaylistItem::count(); @@ -327,7 +327,7 @@ public function testStoreRealtimeAllowance() $user = $token->user; - factory(Room::class)->create(['user_id' => $user, 'type' => Room::REALTIME_DEFAULT_TYPE]); + Room::factory()->create(['user_id' => $user, 'type' => Room::REALTIME_DEFAULT_TYPE]); $roomsCountInitial = Room::count(); $playlistItemsCountInitial = PlaylistItem::count(); @@ -350,7 +350,7 @@ public function testStoreRealtimeAllowanceSeparateFromPlaylists() $user = $token->user; for ($i = 0; $i < $user->maxMultiplayerRooms(); $i++) { - factory(Room::class)->create(['user_id' => $user]); + Room::factory()->create(['user_id' => $user]); } $roomsCountInitial = Room::count(); @@ -371,7 +371,7 @@ public function testJoinWithPassword() { $token = Token::factory()->create(['scopes' => ['*']]); $password = 'hunter2'; - $room = factory(Room::class)->create(compact('password')); + $room = Room::factory()->create(compact('password')); $initialUserChannelCount = UserChannel::count(); $url = route('api.rooms.join', ['room' => $room, 'user' => $token->user]); diff --git a/tests/Models/ContestTest.php b/tests/Models/ContestTest.php index b5a6359cee3..788f3b3043f 100644 --- a/tests/Models/ContestTest.php +++ b/tests/Models/ContestTest.php @@ -36,12 +36,12 @@ public function testAssertVoteRequirementPlaylistBeatmapsets(bool $loggedIn, boo // extra beatmap Beatmap::factory()->create(); - $rooms = factory(Room::class, 2)->create(); + $rooms = Room::factory()->count(2)->create(); foreach ($rooms as $i => $room) { foreach ($beatmapsets as $beatmapset) { - $playlistItems[] = factory(PlaylistItem::class)->create([ - 'room_id' => $room->getKey(), - 'beatmap_id' => $beatmapset->beatmaps[$i]->getKey(), + $playlistItems[] = PlaylistItem::factory()->create([ + 'room_id' => $room, + 'beatmap_id' => $beatmapset->beatmaps[$i], ]); } } @@ -71,10 +71,10 @@ public function testAssertVoteRequirementPlaylistBeatmapsets(bool $loggedIn, boo ->playlist() ->whereIn('beatmap_id', array_column($beatmapset->beatmaps->all(), 'beatmap_id')) ->first(); - factory(MultiplayerScore::class)->create([ + MultiplayerScore::factory()->create([ 'ended_at' => $endedAt, 'passed' => $passed, - 'playlist_item_id' => $playlistItem->getKey(), + 'playlist_item_id' => $playlistItem, 'user_id' => $userId, ]); } diff --git a/tests/Models/Multiplayer/RoomTest.php b/tests/Models/Multiplayer/RoomTest.php index 9f4382b656b..0a174ded628 100644 --- a/tests/Models/Multiplayer/RoomTest.php +++ b/tests/Models/Multiplayer/RoomTest.php @@ -94,9 +94,9 @@ public function testStartGameWithDeletedBeatmap() public function testRoomHasEnded() { $user = User::factory()->create(); - $room = factory(Room::class)->states('ended')->create(); - $playlistItem = factory(PlaylistItem::class)->create([ - 'room_id' => $room->getKey(), + $room = Room::factory()->ended()->create(); + $playlistItem = PlaylistItem::factory()->create([ + 'room_id' => $room, ]); $this->expectException(InvariantException::class); @@ -106,13 +106,9 @@ public function testRoomHasEnded() public function testMaxAttemptsReached() { $user = User::factory()->create(); - $room = factory(Room::class)->create(['max_attempts' => 2]); - $playlistItem1 = factory(PlaylistItem::class)->create([ - 'room_id' => $room->getKey(), - ]); - $playlistItem2 = factory(PlaylistItem::class)->create([ - 'room_id' => $room->getKey(), - ]); + $room = Room::factory()->create(['max_attempts' => 2]); + $playlistItem1 = PlaylistItem::factory()->create(['room_id' => $room]); + $playlistItem2 = PlaylistItem::factory()->create(['room_id' => $room]); $room->startPlay($user, $playlistItem1); $this->assertTrue(true); @@ -127,13 +123,13 @@ public function testMaxAttemptsReached() public function testMaxAttemptsForItemReached() { $user = User::factory()->create(); - $room = factory(Room::class)->create(); - $playlistItem1 = factory(PlaylistItem::class)->create([ - 'room_id' => $room->getKey(), + $room = Room::factory()->create(); + $playlistItem1 = PlaylistItem::factory()->create([ + 'room_id' => $room, 'max_attempts' => 1, ]); - $playlistItem2 = factory(PlaylistItem::class)->create([ - 'room_id' => $room->getKey(), + $playlistItem2 = PlaylistItem::factory()->create([ + 'room_id' => $room, 'max_attempts' => 1, ]); diff --git a/tests/Models/Multiplayer/UserScoreAggregateTest.php b/tests/Models/Multiplayer/UserScoreAggregateTest.php index 0c7e2016876..0992b270597 100644 --- a/tests/Models/Multiplayer/UserScoreAggregateTest.php +++ b/tests/Models/Multiplayer/UserScoreAggregateTest.php @@ -34,11 +34,11 @@ public function testInCompleteScoresAreNotCounted() $playlistItem = $this->playlistItem(); $agg = UserScoreAggregate::new($user, $this->room); - $score = factory(Score::class) + $score = Score::factory() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, ]); $agg->addScore($score); @@ -55,22 +55,22 @@ public function testFailedScoresAreAttemptsOnly() $agg = UserScoreAggregate::new($user, $this->room); $agg->addScore( - factory(Score::class) - ->states('completed', 'failed') + Score::factory() + ->failed() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, ]) ); $agg->addScore( - factory(Score::class) - ->states('completed', 'passed') + Score::factory() + ->passed() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, ]) ); @@ -87,12 +87,12 @@ public function testPassedScoresIncrementsCompletedCount() $agg = UserScoreAggregate::new($user, $this->room); $agg->addScore( - factory(Score::class) - ->states('completed', 'passed') + Score::factory() + ->passed() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, ]) ); @@ -110,11 +110,11 @@ public function testPassedScoresAreAveraged() $agg = UserScoreAggregate::new($user, $this->room); $agg->addScore( - factory(Score::class) + Score::factory() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, 'total_score' => 1, 'pp' => 0.2, 'pp' => 0.2, @@ -122,12 +122,12 @@ public function testPassedScoresAreAveraged() ); $agg->addScore( - factory(Score::class) - ->states('completed', 'failed') + Score::factory() + ->failed() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, 'total_score' => 1, 'accuracy' => 0.3, 'pp' => 0.3, @@ -135,12 +135,12 @@ public function testPassedScoresAreAveraged() ); $agg->addScore( - factory(Score::class) - ->states('completed', 'passed') + Score::factory() + ->passed() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem, + 'user_id' => $user, 'total_score' => 1, 'accuracy' => 0.5, 'pp' => 0.5, @@ -148,12 +148,12 @@ public function testPassedScoresAreAveraged() ); $agg->addScore( - factory(Score::class) - ->states('completed', 'passed') + Score::factory() + ->passed() ->create([ - 'room_id' => $this->room->getKey(), - 'playlist_item_id' => $playlistItem2->getKey(), - 'user_id' => $user->getKey(), + 'room_id' => $this->room, + 'playlist_item_id' => $playlistItem2, + 'user_id' => $user, 'total_score' => 1, 'accuracy' => 0.8, 'pp' => 0.8, @@ -170,13 +170,13 @@ protected function setUp(): void { parent::setUp(); - $this->room = factory(Room::class)->create(); + $this->room = Room::factory()->create(); } private function playlistItem() { - return factory(PlaylistItem::class)->create([ - 'room_id' => $this->room->getKey(), + return PlaylistItem::factory()->create([ + 'room_id' => $this->room, ]); } } From 015f5abddaa5bf58ee1666a2307ab361e6ff3974 Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 10 Apr 2023 14:16:12 +0900 Subject: [PATCH 2/8] Add class based factory for Notification --- database/factories/NotificationFactory.php | 26 +++++--- .../factories/UserNotificationFactory.php | 26 ++++++-- tests/Models/UserNotificationTest.php | 64 +++++++++---------- 3 files changed, 69 insertions(+), 47 deletions(-) diff --git a/database/factories/NotificationFactory.php b/database/factories/NotificationFactory.php index 70ec5cfd327..e328dafe198 100644 --- a/database/factories/NotificationFactory.php +++ b/database/factories/NotificationFactory.php @@ -3,14 +3,24 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + +namespace Database\Factories; + use App\Libraries\MorphMap; use App\Models\Notification; -$factory->define(Notification::class, function (Faker\Generator $faker) { - return [ - 'notifiable_type' => array_rand_val(MorphMap::MAP), - 'notifiable_id' => rand(), - 'name' => array_rand(Notification::NAME_TO_CATEGORY), - 'details' => [], - ]; -}); +class NotificationFactory extends Factory +{ + protected $model = Notification::class; + + public function definition(): array + { + return [ + 'notifiable_type' => array_rand_val(MorphMap::MAP), + 'notifiable_id' => rand(), + 'name' => array_rand(Notification::NAME_TO_CATEGORY), + 'details' => [], + ]; + } +} diff --git a/database/factories/UserNotificationFactory.php b/database/factories/UserNotificationFactory.php index ae973b61034..ee248e1b88d 100644 --- a/database/factories/UserNotificationFactory.php +++ b/database/factories/UserNotificationFactory.php @@ -3,12 +3,24 @@ // Copyright (c) ppy Pty Ltd . Licensed under the GNU Affero General Public License v3.0. // See the LICENCE file in the repository root for full licence text. +declare(strict_types=1); + +namespace Database\Factories; + +use App\Models\Notification; +use App\Models\User; use App\Models\UserNotification; -$factory->define(UserNotification::class, function (Faker\Generator $faker) { - return [ - 'delivery' => UserNotification::deliveryMask(array_rand(UserNotification::DELIVERY_OFFSETS)), - 'notification_id' => rand(), - 'user_id' => rand(), - ]; -}); +class UserNotificationFactory extends Factory +{ + protected $model = UserNotification::class; + + public function definition(): array + { + return [ + 'delivery' => UserNotification::deliveryMask(array_rand(UserNotification::DELIVERY_OFFSETS)), + 'notification_id' => Notification::factory(), + 'user_id' => User::factory(), + ]; + } +} diff --git a/tests/Models/UserNotificationTest.php b/tests/Models/UserNotificationTest.php index c21d9c8712e..d99ef17a9e7 100644 --- a/tests/Models/UserNotificationTest.php +++ b/tests/Models/UserNotificationTest.php @@ -17,17 +17,17 @@ public function testBatchDestroyByIds() { $user = User::factory()->create(); - $notificationA = factory(Notification::class)->create(); - $notificationB = factory(Notification::class)->create(); + $notificationA = Notification::factory()->create(); + $notificationB = Notification::factory()->create(); - $userNotificationA = factory(UserNotification::class)->create([ - 'notification_id' => $notificationA->getKey(), - 'user_id' => $user->getKey(), + $userNotificationA = UserNotification::factory()->create([ + 'notification_id' => $notificationA, + 'user_id' => $user, ]); - $userNotificationB = factory(UserNotification::class)->create([ - 'notification_id' => $notificationB->getKey(), - 'user_id' => $user->getKey(), + $userNotificationB = UserNotification::factory()->create([ + 'notification_id' => $notificationB, + 'user_id' => $user, ]); $initialUserNotificationCount = UserNotification::count(); @@ -47,35 +47,35 @@ public function testBatchDestroyByNotificationIdentyByStack() { $user = User::factory()->create(); - $notificationA = factory(Notification::class)->create([ + $notificationA = Notification::factory()->create([ 'name' => Notification::BEATMAPSET_DISCUSSION_LOCK, 'notifiable_id' => 1, 'notifiable_type' => 'beatmapset', ]); - $notificationB = factory(Notification::class)->create([ + $notificationB = Notification::factory()->create([ 'name' => $notificationA->name, 'notifiable_id' => $notificationA->notifiable_id, 'notifiable_type' => $notificationA->notifiable_type, ]); - $notificationC = factory(Notification::class)->create([ + $notificationC = Notification::factory()->create([ 'name' => $notificationA->name, 'notifiable_id' => 2, 'notifiable_type' => $notificationA->notifiable_type, ]); - $userNotificationA = factory(UserNotification::class)->create([ - 'notification_id' => $notificationA->getKey(), - 'user_id' => $user->getKey(), + $userNotificationA = UserNotification::factory()->create([ + 'notification_id' => $notificationA, + 'user_id' => $user, ]); - $userNotificationB = factory(UserNotification::class)->create([ - 'notification_id' => $notificationB->getKey(), - 'user_id' => $user->getKey(), + $userNotificationB = UserNotification::factory()->create([ + 'notification_id' => $notificationB, + 'user_id' => $user, ]); - $userNotificationC = factory(UserNotification::class)->create([ - 'notification_id' => $notificationC->getKey(), - 'user_id' => $user->getKey(), + $userNotificationC = UserNotification::factory()->create([ + 'notification_id' => $notificationC, + 'user_id' => $user, ]); $initialUserNotificationCount = UserNotification::count(); @@ -100,32 +100,32 @@ public function testBatchDestroyByNotificationIdentityByObjectType() { $user = User::factory()->create(); - $notificationA = factory(Notification::class)->create([ + $notificationA = Notification::factory()->create([ 'name' => Notification::BEATMAPSET_DISCUSSION_LOCK, 'notifiable_type' => 'beatmapset', ]); - $notificationB = factory(Notification::class)->create([ + $notificationB = Notification::factory()->create([ 'name' => $notificationA->name, 'notifiable_type' => 'beatmapset', ]); - $notificationC = factory(Notification::class)->create([ + $notificationC = Notification::factory()->create([ 'name' => Notification::COMMENT_NEW, 'notifiable_type' => 'news_post', ]); - $userNotificationA = factory(UserNotification::class)->create([ - 'notification_id' => $notificationA->getKey(), - 'user_id' => $user->getKey(), + $userNotificationA = UserNotification::factory()->create([ + 'notification_id' => $notificationA, + 'user_id' => $user, ]); - $userNotificationB = factory(UserNotification::class)->create([ - 'notification_id' => $notificationB->getKey(), - 'user_id' => $user->getKey(), + $userNotificationB = UserNotification::factory()->create([ + 'notification_id' => $notificationB, + 'user_id' => $user, ]); - $userNotificationC = factory(UserNotification::class)->create([ - 'notification_id' => $notificationC->getKey(), - 'user_id' => $user->getKey(), + $userNotificationC = UserNotification::factory()->create([ + 'notification_id' => $notificationC, + 'user_id' => $user, ]); $initialUserNotificationCount = UserNotification::count(); From 7c4a5b83ff1cdf41a12cb6b42a65fcf1281563f9 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 10 Apr 2023 15:15:28 +0900 Subject: [PATCH 3/8] children actually resolves to ReactNode[] & ReactNode which means it can be null; use a factory method --- .../discussion-message.tsx | 10 +++++----- .../plain-text-preview.tsx | 2 +- .../js/beatmap-discussions/renderers.tsx | 19 ++++--------------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/resources/js/beatmap-discussions/discussion-message.tsx b/resources/js/beatmap-discussions/discussion-message.tsx index c399bff6f51..8ac732cdaca 100644 --- a/resources/js/beatmap-discussions/discussion-message.tsx +++ b/resources/js/beatmap-discussions/discussion-message.tsx @@ -7,7 +7,7 @@ import remarkBreaks from 'remark-breaks'; import autolink from 'remark-plugins/autolink'; import disableConstructs, { DisabledType } from 'remark-plugins/disable-constructs'; import ImageLink from './image-link'; -import { emphasisRenderer, linkRenderer, listItemRenderer, paragraphRenderer, strongRenderer, transformLinkUri } from './renderers'; +import { createRenderer, linkRenderer, transformLinkUri } from './renderers'; interface Props { markdown: string; @@ -21,11 +21,11 @@ export default class DiscussionMessage extends React.Component { className='osu-md osu-md--discussions' components={{ a: linkRenderer, - em: emphasisRenderer, + em: createRenderer('em'), img: ImageLink, - li: listItemRenderer, - p: paragraphRenderer, - strong: strongRenderer, + li: createRenderer('li'), + p: createRenderer('p'), + strong: createRenderer('strong'), }} remarkPlugins={[autolink, [disableConstructs, { type: this.props.type }], remarkBreaks]} transformLinkUri={transformLinkUri} diff --git a/resources/js/beatmap-discussions/plain-text-preview.tsx b/resources/js/beatmap-discussions/plain-text-preview.tsx index a436fd9a685..8db2385259d 100644 --- a/resources/js/beatmap-discussions/plain-text-preview.tsx +++ b/resources/js/beatmap-discussions/plain-text-preview.tsx @@ -29,7 +29,7 @@ export function linkRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLPr } function textRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { - return <>{astProps.children.map(timestampDecorator)}; + return <>{astProps.children?.map(timestampDecorator)}; } export default class PlainTextPreview extends React.Component { diff --git a/resources/js/beatmap-discussions/renderers.tsx b/resources/js/beatmap-discussions/renderers.tsx index fff59ded971..8a0e84962d6 100644 --- a/resources/js/beatmap-discussions/renderers.tsx +++ b/resources/js/beatmap-discussions/renderers.tsx @@ -9,9 +9,10 @@ import { openBeatmapEditor } from 'utils/url'; export const LinkContext = React.createContext({ inLink: false }); -// FIXME: use a factory -export function emphasisRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { - return {astProps.children.map(timestampDecorator)}; +export function createRenderer(ElementType: React.ElementType) { + return function defaultRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { + return {astProps.children?.map(timestampDecorator)}; + }; } export function linkRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLAnchorElement>) { @@ -26,18 +27,6 @@ export function linkRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLPr ); } -export function listItemRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { - return
  • {astProps.children?.map(timestampDecorator)}
    • ; -} - -export function paragraphRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLParagraphElement>) { - return

    {astProps.children.map(timestampDecorator)}

    ; -} - -export function strongRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { - return {astProps.children.map(timestampDecorator)}; -} - export function timestampDecorator(reactNode: React.ReactNode) { if (typeof reactNode === 'string') { const matches = [...reactNode.matchAll(timestampRegexGlobal)]; From 55d4a9d195980338cc0b7a0c12ac3a301059c4d7 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 10 Apr 2023 19:47:08 +0900 Subject: [PATCH 4/8] can be just readonly constants --- .../discussion-message.tsx | 18 +++++++++-------- .../plain-text-preview.tsx | 20 ++++++++++--------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/resources/js/beatmap-discussions/discussion-message.tsx b/resources/js/beatmap-discussions/discussion-message.tsx index 8ac732cdaca..2a00b4b9aef 100644 --- a/resources/js/beatmap-discussions/discussion-message.tsx +++ b/resources/js/beatmap-discussions/discussion-message.tsx @@ -9,6 +9,15 @@ import disableConstructs, { DisabledType } from 'remark-plugins/disable-construc import ImageLink from './image-link'; import { createRenderer, linkRenderer, transformLinkUri } from './renderers'; +const components = Object.freeze({ + a: linkRenderer, + em: createRenderer('em'), + img: ImageLink, + li: createRenderer('li'), + p: createRenderer('p'), + strong: createRenderer('strong'), +}); + interface Props { markdown: string; type?: DisabledType; @@ -19,14 +28,7 @@ export default class DiscussionMessage extends React.Component { return ( { return ( Date: Mon, 10 Apr 2023 22:10:13 +0900 Subject: [PATCH 5/8] more conveient timestampDecorator? Also, more accurate type, maybe; ReactNode includes Iterable as part of ReactFragment so the ReactNode[] declared by react-markdown isn't quite right... --- resources/js/beatmap-discussions/image-link.tsx | 2 +- .../js/beatmap-discussions/plain-text-preview.tsx | 9 ++++----- resources/js/beatmap-discussions/renderers.tsx | 11 ++++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/resources/js/beatmap-discussions/image-link.tsx b/resources/js/beatmap-discussions/image-link.tsx index 6aa4f4c1a8d..cbf005a5baa 100644 --- a/resources/js/beatmap-discussions/image-link.tsx +++ b/resources/js/beatmap-discussions/image-link.tsx @@ -9,7 +9,7 @@ import React from 'react'; import { ReactMarkdownProps } from 'react-markdown/lib/complex-types'; import { LinkContext } from './renderers'; -type Props = ReactMarkdownProps & React.DetailedHTMLProps, HTMLImageElement>; +type Props = ReactMarkdownProps & JSX.IntrinsicElements['img']; @observer export default class ImageLink extends React.Component { diff --git a/resources/js/beatmap-discussions/plain-text-preview.tsx b/resources/js/beatmap-discussions/plain-text-preview.tsx index 22fcae58155..955e5609dc9 100644 --- a/resources/js/beatmap-discussions/plain-text-preview.tsx +++ b/resources/js/beatmap-discussions/plain-text-preview.tsx @@ -3,7 +3,6 @@ import React from 'react'; import ReactMarkdown from 'react-markdown'; -import { ReactMarkdownProps } from 'react-markdown/lib/complex-types'; import rehypeTruncate from 'rehype-truncate'; import autolink from 'remark-plugins/autolink'; import disableConstructs, { DisabledType } from 'remark-plugins/disable-constructs'; @@ -27,19 +26,19 @@ interface Props { type?: DisabledType; } -function imageRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLImageElement>) { +function imageRenderer(astProps: JSX.IntrinsicElements['img']) { // render something besides image url. return <>{presence(astProps.alt) ?? '[image]'}; } -export function linkRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLAnchorElement>) { +export function linkRenderer(astProps: JSX.IntrinsicElements['a']) { const props = propsFromHref(astProps.href); return props.children != null ? : <>{astProps.children}; } -function textRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { - return <>{astProps.children?.map(timestampDecorator)}; +function textRenderer(astProps: JSX.IntrinsicElements[keyof JSX.IntrinsicElements]) { + return <>{timestampDecorator(astProps.children)}; } export default class PlainTextPreview extends React.Component { diff --git a/resources/js/beatmap-discussions/renderers.tsx b/resources/js/beatmap-discussions/renderers.tsx index 8a0e84962d6..8fd3c253582 100644 --- a/resources/js/beatmap-discussions/renderers.tsx +++ b/resources/js/beatmap-discussions/renderers.tsx @@ -3,19 +3,18 @@ import * as React from 'react'; import { uriTransformer } from 'react-markdown'; -import { ReactMarkdownProps } from 'react-markdown/lib/complex-types'; import { propsFromHref, timestampRegexGlobal } from 'utils/beatmapset-discussion-helper'; import { openBeatmapEditor } from 'utils/url'; export const LinkContext = React.createContext({ inLink: false }); export function createRenderer(ElementType: React.ElementType) { - return function defaultRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLElement>) { - return {astProps.children?.map(timestampDecorator)}; + return function defaultRenderer(astProps: JSX.IntrinsicElements[keyof JSX.IntrinsicElements]) { + return {timestampDecorator(astProps.children)}; }; } -export function linkRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLProps, HTMLAnchorElement>) { +export function linkRenderer(astProps: JSX.IntrinsicElements['a']) { const props = propsFromHref(astProps.href); return ( @@ -27,7 +26,7 @@ export function linkRenderer(astProps: ReactMarkdownProps & React.DetailedHTMLPr ); } -export function timestampDecorator(reactNode: React.ReactNode) { +export function timestampDecorator(reactNode: React.ReactNode): React.ReactNode { if (typeof reactNode === 'string') { const matches = [...reactNode.matchAll(timestampRegexGlobal)]; @@ -57,6 +56,8 @@ export function timestampDecorator(reactNode: React.ReactNode) { return nodes; } + } else if (Array.isArray(reactNode)) { + return reactNode.map(timestampDecorator); } return reactNode; From f4f88ac7bc5a4cea7a23fdfd89caad07c5555ffa Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 10 Apr 2023 23:09:06 +0900 Subject: [PATCH 6/8] actually, the type is kind of useless, we'd likely make a different function if other props need to be handled, anyway. --- resources/js/beatmap-discussions/renderers.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/js/beatmap-discussions/renderers.tsx b/resources/js/beatmap-discussions/renderers.tsx index 8fd3c253582..7718b414ae2 100644 --- a/resources/js/beatmap-discussions/renderers.tsx +++ b/resources/js/beatmap-discussions/renderers.tsx @@ -9,7 +9,7 @@ import { openBeatmapEditor } from 'utils/url'; export const LinkContext = React.createContext({ inLink: false }); export function createRenderer(ElementType: React.ElementType) { - return function defaultRenderer(astProps: JSX.IntrinsicElements[keyof JSX.IntrinsicElements]) { + return function defaultRenderer(astProps: { children: React.ReactNode }) { return {timestampDecorator(astProps.children)}; }; } From 490cf7abb66cc867d18dceff0bc6af98150aaf2e Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 11 Apr 2023 12:40:47 +0900 Subject: [PATCH 7/8] Sort keys --- database/factories/NotificationFactory.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/database/factories/NotificationFactory.php b/database/factories/NotificationFactory.php index e328dafe198..f1e023d59b0 100644 --- a/database/factories/NotificationFactory.php +++ b/database/factories/NotificationFactory.php @@ -17,10 +17,10 @@ class NotificationFactory extends Factory public function definition(): array { return [ - 'notifiable_type' => array_rand_val(MorphMap::MAP), - 'notifiable_id' => rand(), - 'name' => array_rand(Notification::NAME_TO_CATEGORY), 'details' => [], + 'name' => array_rand(Notification::NAME_TO_CATEGORY), + 'notifiable_id' => rand(), + 'notifiable_type' => array_rand_val(MorphMap::MAP), ]; } } From 5482382f03030a3e8a5cb032d00e896a014ac88b Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 11 Apr 2023 12:46:55 +0900 Subject: [PATCH 8/8] Sort keys --- database/factories/LegacyMatch/EventFactory.php | 8 ++++---- database/factories/LegacyMatch/GameFactory.php | 5 +++-- database/factories/LegacyMatch/LegacyMatchFactory.php | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/database/factories/LegacyMatch/EventFactory.php b/database/factories/LegacyMatch/EventFactory.php index 55347fd6d95..8f06ff1e31d 100644 --- a/database/factories/LegacyMatch/EventFactory.php +++ b/database/factories/LegacyMatch/EventFactory.php @@ -22,24 +22,24 @@ public function definition(): array { return [ 'match_id' => LegacyMatch::factory(), - 'user_id' => User::factory(), 'timestamp' => fn() => Carbon::now(), + 'user_id' => User::factory(), ]; } public function stateCreate(): static { return $this->state([ - 'user_id' => null, 'text' => 'CREATE', + 'user_id' => null, ]); } public function disband(): static { return $this->state([ - 'user_id' => null, 'text' => 'DISBAND', + 'user_id' => null, ]); } @@ -56,9 +56,9 @@ public function part(): static public function game(): static { return $this->state([ + 'game_id' => Game::factory()->inProgress(), 'text' => 'test game', 'user_id' => null, - 'game_id' => Game::factory()->inProgress(), ]); } } diff --git a/database/factories/LegacyMatch/GameFactory.php b/database/factories/LegacyMatch/GameFactory.php index 389ceb9546b..e0784483c64 100644 --- a/database/factories/LegacyMatch/GameFactory.php +++ b/database/factories/LegacyMatch/GameFactory.php @@ -20,10 +20,11 @@ public function definition(): array { return [ 'beatmap_id' => Beatmap::factory(), - 'start_time' => fn(array $attributes) => Carbon::now()->subSeconds(Beatmap::find($attributes['beatmap_id'])->total_length), - 'play_mode' => fn(array $attributes) => Beatmap::find($attributes['beatmap_id'])->playmode, 'scoring_type' => fn() => $this->faker->numberBetween(0, 3), 'team_type' => fn() => $this->faker->numberBetween(0, 3), + + 'play_mode' => fn(array $attributes) => Beatmap::find($attributes['beatmap_id'])->playmode, + 'start_time' => fn(array $attributes) => Carbon::now()->subSeconds(Beatmap::find($attributes['beatmap_id'])->total_length), ]; } diff --git a/database/factories/LegacyMatch/LegacyMatchFactory.php b/database/factories/LegacyMatch/LegacyMatchFactory.php index 3f457e03fd9..dd64815b030 100644 --- a/database/factories/LegacyMatch/LegacyMatchFactory.php +++ b/database/factories/LegacyMatch/LegacyMatchFactory.php @@ -17,8 +17,8 @@ public function definition(): array { return [ 'name' => fn() => $this->faker->sentence(), - 'start_time' => fn() => Carbon::now(), 'private' => 0, + 'start_time' => fn() => Carbon::now(), ]; }