From d0be2dc1c67aca7e5b7b8d1c98cb3db3069725e4 Mon Sep 17 00:00:00 2001 From: nanaya Date: Fri, 10 Jan 2025 22:27:57 +0900 Subject: [PATCH 01/14] Add interop endpoint to create and join multiplayer room --- .../InterOp/Multiplayer/RoomsController.php | 35 ++++++ .../Multiplayer/RoomsController.php | 47 ++------ app/Models/Multiplayer/Room.php | 12 +++ .../Multiplayer/RoomTransformer.php | 20 ++++ app/helpers.php | 8 +- routes/web.php | 5 + .../Multiplayer/RoomsControllerTest.php | 101 ++++++++++++++++++ tests/TestCase.php | 15 ++- 8 files changed, 200 insertions(+), 43 deletions(-) create mode 100644 app/Http/Controllers/InterOp/Multiplayer/RoomsController.php create mode 100644 tests/Controllers/InterOp/Multiplayer/RoomsControllerTest.php diff --git a/app/Http/Controllers/InterOp/Multiplayer/RoomsController.php b/app/Http/Controllers/InterOp/Multiplayer/RoomsController.php new file mode 100644 index 00000000000..27e08dfbb9c --- /dev/null +++ b/app/Http/Controllers/InterOp/Multiplayer/RoomsController.php @@ -0,0 +1,35 @@ +. Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +namespace App\Http\Controllers\InterOp\Multiplayer; + +use App\Http\Controllers\Controller; +use App\Models\Multiplayer\Room; +use App\Models\User; +use App\Transformers\Multiplayer\RoomTransformer; + +class RoomsController extends Controller +{ + public function join(string $id, string $userId) + { + $user = User::findOrFail($userId); + $room = Room::findOrFail($id); + + $room->assertCorrectPassword(get_string(request('password'))); + $room->join($user); + + return RoomTransformer::createShowResponse($room); + } + + public function store() + { + $params = \Request::all(); + $user = User::findOrFail(get_int($params['user_id'] ?? null)); + + $room = (new Room())->startGame($user, $params); + + return RoomTransformer::createShowResponse($room); + } +} diff --git a/app/Http/Controllers/Multiplayer/RoomsController.php b/app/Http/Controllers/Multiplayer/RoomsController.php index 97c6e7fd555..00d0bdd7c13 100644 --- a/app/Http/Controllers/Multiplayer/RoomsController.php +++ b/app/Http/Controllers/Multiplayer/RoomsController.php @@ -5,7 +5,6 @@ namespace App\Http\Controllers\Multiplayer; -use App\Exceptions\InvariantException; use App\Http\Controllers\Controller; use App\Http\Controllers\Ranking\DailyChallengeController; use App\Models\Model; @@ -101,24 +100,18 @@ public function index() public function join($roomId, $userId) { + $currentUser = \Auth::user(); // this allows admins/whatever to add users to games in the future - if (get_int($userId) !== auth()->user()->user_id) { + if (get_int($userId) !== $currentUser->getKey()) { abort(403); } $room = Room::findOrFail($roomId); + $room->assertCorrectPassword(get_string(request('password'))); - if ($room->password !== null) { - $password = get_param_value(request('password'), null); - - if ($password === null || !hash_equals(hash('sha256', $room->password), hash('sha256', $password))) { - abort(403, osu_trans('multiplayer.room.invalid_password')); - } - } - - $room->join(auth()->user()); + $room->join($currentUser); - return $this->createJoinedRoomResponse($room); + return RoomTransformer::createShowResponse($room); } public function leaderboard($roomId) @@ -168,7 +161,7 @@ public function show($id) } if (is_api_request()) { - return $this->createJoinedRoomResponse($room); + return RoomTransformer::createShowResponse($room); } if ($room->category === 'daily_challenge') { @@ -200,32 +193,8 @@ public function show($id) public function store() { - try { - $room = (new Room())->startGame(auth()->user(), request()->all()); - - return $this->createJoinedRoomResponse($room); - } catch (InvariantException $e) { - return error_popup($e->getMessage(), $e->getStatusCode()); - } - } + $room = (new Room())->startGame(\Auth::user(), \Request::all()); - private function createJoinedRoomResponse($room) - { - return json_item( - $room->loadMissing([ - 'host', - 'playlist.beatmap.beatmapset', - 'playlist.beatmap.baseMaxCombo', - ]), - 'Multiplayer\Room', - [ - 'current_user_score.playlist_item_attempts', - 'host.country', - 'playlist.beatmap.beatmapset', - 'playlist.beatmap.checksum', - 'playlist.beatmap.max_combo', - 'recent_participants', - ] - ); + return RoomTransformer::createShowResponse($room); } } diff --git a/app/Models/Multiplayer/Room.php b/app/Models/Multiplayer/Room.php index c50ed5d63e3..a4aaedb1f82 100644 --- a/app/Models/Multiplayer/Room.php +++ b/app/Models/Multiplayer/Room.php @@ -6,6 +6,7 @@ namespace App\Models\Multiplayer; use App\Casts\PresentString; +use App\Exceptions\AuthorizationException; use App\Exceptions\InvariantException; use App\Models\Beatmap; use App\Models\Chat\Channel; @@ -332,6 +333,17 @@ public function scopeWithRecentParticipantIds($query, ?int $limit = null) ", 'recent_participant_ids'); } + public function assertCorrectPassword(?string $password): void + { + if ($this->password === null) { + return; + } + + if ($password === null || !hash_equals(hash('sha256', $this->password), hash('sha256', $password))) { + throw new AuthorizationException(osu_trans('multiplayer.room.invalid_password')); + } + } + public function difficultyRange() { $extraQuery = true; diff --git a/app/Transformers/Multiplayer/RoomTransformer.php b/app/Transformers/Multiplayer/RoomTransformer.php index c96cae14b56..cc3d0cf93c1 100644 --- a/app/Transformers/Multiplayer/RoomTransformer.php +++ b/app/Transformers/Multiplayer/RoomTransformer.php @@ -23,6 +23,26 @@ class RoomTransformer extends TransformerAbstract 'recent_participants', ]; + public static function createShowResponse(Room $room): array + { + return json_item( + $room->loadMissing([ + 'host', + 'playlist.beatmap.baseMaxCombo', + 'playlist.beatmap.beatmapset', + ]), + new static(), + [ + 'current_user_score.playlist_item_attempts', + 'host.country', + 'playlist.beatmap.beatmapset', + 'playlist.beatmap.checksum', + 'playlist.beatmap.max_combo', + 'recent_participants', + ], + ); + } + public function transform(Room $room) { return [ diff --git a/app/helpers.php b/app/helpers.php index 81685d9c2bc..2a808374167 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -840,7 +840,9 @@ function forum_user_link(int $id, string $username, string|null $colour, int|nul function is_api_request(): bool { - return str_starts_with(rawurldecode(Request::getPathInfo()), '/api/'); + $url = rawurldecode(Request::getPathInfo()); + return str_starts_with($url, '/api/') + || str_starts_with($url, '/_lio/'); } function is_http(string $url): bool @@ -1718,6 +1720,10 @@ function parse_time_to_carbon($value) if ($value instanceof DateTime) { return Carbon\Carbon::instance($value); } + + if ($value instanceof Carbon\CarbonImmutable) { + return $value->toMutable(); + } } function format_duration_for_display(int $seconds) diff --git a/routes/web.php b/routes/web.php index 2dc8ccb6689..6437bcae2d1 100644 --- a/routes/web.php +++ b/routes/web.php @@ -602,6 +602,11 @@ Route::apiResource('bulk', 'Indexing\BulkController', ['only' => ['store']]); }); + Route::group(['as' => 'multiplayer.', 'namespace' => 'Multiplayer', 'prefix' => 'multiplayer'], function () { + Route::put('rooms/{room}/users/{user}', 'RoomsController@join')->name('rooms.join'); + Route::apiResource('rooms', 'RoomsController', ['only' => ['store']]); + }); + Route::post('user-achievement/{user}/{achievement}/{beatmap?}', 'UsersController@achievement')->name('users.achievement'); Route::group(['as' => 'user-group.'], function () { diff --git a/tests/Controllers/InterOp/Multiplayer/RoomsControllerTest.php b/tests/Controllers/InterOp/Multiplayer/RoomsControllerTest.php new file mode 100644 index 00000000000..353e1902eea --- /dev/null +++ b/tests/Controllers/InterOp/Multiplayer/RoomsControllerTest.php @@ -0,0 +1,101 @@ +. Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +namespace Tests\Controllers\InterOp\Multiplayer; + +use App\Models\Beatmap; +use App\Models\Chat\UserChannel; +use App\Models\Multiplayer\Room; +use App\Models\User; +use Carbon\CarbonImmutable; +use Tests\TestCase; + +class RoomsControllerTest extends TestCase +{ + private static function startRoomParams(): array + { + $beatmap = Beatmap::factory()->create(); + + return [ + 'ends_at' => CarbonImmutable::now()->addHours(1), + 'name' => 'test room', + 'type' => Room::REALTIME_DEFAULT_TYPE, + 'playlist' => [[ + 'beatmap_id' => $beatmap->getKey(), + 'ruleset_id' => $beatmap->playmode, + ]], + ]; + } + + public function testJoin(): void + { + $room = (new Room())->startGame(User::factory()->create(), static::startRoomParams()); + $user = User::factory()->create(); + + $this->expectCountChange(fn () => UserChannel::count(), 1); + + $this->withInterOpHeader( + route('interop.multiplayer.rooms.join', [ + 'room' => $room->getKey(), + 'user' => $user->getKey(), + ]), + fn ($url) => $this->put($url), + )->assertSuccessful(); + } + + public function testJoinWithPassword(): void + { + $room = (new Room())->startGame(User::factory()->create(), [ + ...static::startRoomParams(), + 'password' => 'hunter2', + ]); + $user = User::factory()->create(); + + $this->expectCountChange(fn () => UserChannel::count(), 1); + + $this->withInterOpHeader( + route('interop.multiplayer.rooms.join', [ + 'room' => $room->getKey(), + 'user' => $user->getKey(), + ]), + fn ($url) => $this->put($url, ['password' => 'hunter2']), + )->assertSuccessful(); + } + + public function testJoinWithPasswordInvalid(): void + { + $room = (new Room())->startGame(User::factory()->create(), [ + ...static::startRoomParams(), + 'password' => 'hunter2', + ]); + $user = User::factory()->create(); + + $this->expectCountChange(fn () => UserChannel::count(), 0); + + $this->withInterOpHeader( + route('interop.multiplayer.rooms.join', [ + 'room' => $room->getKey(), + 'user' => $user->getKey(), + ]), + fn ($url) => $this->put($url, ['password' => '*******']), + )->assertStatus(403); + } + + public function testStore(): void + { + $beatmap = Beatmap::factory()->create(); + $params = [ + ...static::startRoomParams(), + 'user_id' => User::factory()->create()->getKey(), + ]; + + $this->expectCountChange(fn () => Room::count(), 1); + + $this->withInterOpHeader( + route('interop.multiplayer.rooms.store'), + fn ($url) => $this->post($url, $params), + )->assertSuccessful(); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 3617fbd4993..f706a8427aa 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -365,11 +365,20 @@ protected function runFakeQueue() $this->invokeSetProperty(app('queue'), 'jobs', []); } - protected function withInterOpHeader($url) + protected function withInterOpHeader($url, ?callable $callback = null) { - return $this->withHeaders([ - 'X-LIO-Signature' => hash_hmac('sha1', $url, $GLOBALS['cfg']['osu']['legacy']['shared_interop_secret']), + if ($callback === null) { + $timestampedUrl = $url; + } else { + $connector = strpos($url, '?') === false ? '?' : '&'; + $timestampedUrl = $url.$connector.'timestamp='.time(); + } + + $this->withHeaders([ + 'X-LIO-Signature' => hash_hmac('sha1', $timestampedUrl, $GLOBALS['cfg']['osu']['legacy']['shared_interop_secret']), ]); + + return $callback === null ? $this : $callback($timestampedUrl); } protected function withPersistentSession(SessionStore $session): static From 82fc1122df27ef1b39d19375f44c71fe049883a7 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 16 Jan 2025 19:55:18 +0900 Subject: [PATCH 02/14] Remove proxying option of blade background image --- app/helpers.php | 12 ++++-------- resources/views/follows/modding.blade.php | 2 +- resources/views/forum/topics/_post_info.blade.php | 2 +- resources/views/layout/_header_user.blade.php | 2 +- resources/views/layout/_page_header_v4.blade.php | 2 +- resources/views/layout/_popup_user.blade.php | 2 +- resources/views/objects/_flag_team.blade.php | 2 +- 7 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/helpers.php b/app/helpers.php index 81685d9c2bc..b375c176c4c 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -55,15 +55,11 @@ function atom_id(string $namespace, $id = null): string return 'tag:'.request()->getHttpHost().',2019:'.$namespace.($id === null ? '' : "/{$id}"); } -function background_image($url, $proxy = true) +function background_image($url): string { - if (!present($url)) { - return ''; - } - - $url = $proxy ? proxy_media($url) : $url; - - return sprintf(' style="background-image:url(\'%s\');" ', e($url)); + return present($url) + ? sprintf(' style="background-image:url(\'%s\');" ', e($url)) + : ''; } function beatmap_timestamp_format($ms) diff --git a/resources/views/follows/modding.blade.php b/resources/views/follows/modding.blade.php index 18f677dec3b..210c646fdc2 100644 --- a/resources/views/follows/modding.blade.php +++ b/resources/views/follows/modding.blade.php @@ -60,7 +60,7 @@
beatmapset->coverURL('list'), false) !!} + {!! background_image($watch->beatmapset->coverURL('list')) !!} class="beatmapset-watches__cover" >
 diff --git a/resources/views/forum/topics/_post_info.blade.php b/resources/views/forum/topics/_post_info.blade.php index db4bd562cbd..0b2cc37e46e 100644 --- a/resources/views/forum/topics/_post_info.blade.php +++ b/resources/views/forum/topics/_post_info.blade.php @@ -76,7 +76,7 @@ class="forum-post-info__row forum-post-info__row--title" logo()->url(), false) !!} + {!! background_image($team->logo()->url()) !!} > diff --git a/resources/views/layout/_header_user.blade.php b/resources/views/layout/_header_user.blade.php index 1905100230f..7e7d2338157 100644 --- a/resources/views/layout/_header_user.blade.php +++ b/resources/views/layout/_header_user.blade.php @@ -21,6 +21,6 @@ class="{{ $class }} avatar--guest" class="{{ $class }} {{ Auth::user()->isRestricted() ? 'avatar--restricted' : '' }}" data-click-menu-target="nav2-user-popup" href="{{ route('users.show', Auth::user()) }}" - {!! background_image(Auth::user()->user_avatar, false) !!} + {!! background_image(Auth::user()->user_avatar) !!} > @endif diff --git a/resources/views/layout/_page_header_v4.blade.php b/resources/views/layout/_page_header_v4.blade.php index 963d1093f22..36d78c726da 100644 --- a/resources/views/layout/_page_header_v4.blade.php +++ b/resources/views/layout/_page_header_v4.blade.php @@ -24,7 +24,7 @@ ">
-
+
+ {user.team != null && +
+ +
+ } + {user.username} diff --git a/resources/views/home/_search_result_user.blade.php b/resources/views/home/_search_result_user.blade.php index 57c4e093add..efeb19a0731 100644 --- a/resources/views/home/_search_result_user.blade.php +++ b/resources/views/home/_search_result_user.blade.php @@ -2,7 +2,14 @@ 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. --}} +@php + use App\Transformers\UserCompactTransformer; +@endphp
-
+ data-modifiers="{{ json_encode(['search']) }}" + data-users="{{ json_encode(json_collection( + $search->data(), + new UserCompactTransformer(), + UserCompactTransformer::CARD_INCLUDES, + )) }}" +>
From ed2770035c0c464a4b53c7a08c5f4ec8ce8546c8 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 20 Jan 2025 16:21:16 +0900 Subject: [PATCH 04/14] always wrap even if no breakpoints --- resources/css/bem/beatmap-discussions-header-top.less | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/css/bem/beatmap-discussions-header-top.less b/resources/css/bem/beatmap-discussions-header-top.less index 3ccef7ae580..32c2a199f74 100644 --- a/resources/css/bem/beatmap-discussions-header-top.less +++ b/resources/css/bem/beatmap-discussions-header-top.less @@ -90,6 +90,7 @@ grid-area: owners; font-size: @font-size--title-small; padding: 10px 10px 0; + overflow-wrap: anywhere; } &__stats { From 88913738d6054fe95d36864d9731634e3036b5f0 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 20 Jan 2025 16:25:19 +0900 Subject: [PATCH 05/14] unneeded? --- resources/js/beatmap-discussions/header.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/js/beatmap-discussions/header.tsx b/resources/js/beatmap-discussions/header.tsx index 45e7fdb7d80..7abe08c9356 100644 --- a/resources/js/beatmap-discussions/header.tsx +++ b/resources/js/beatmap-discussions/header.tsx @@ -177,14 +177,14 @@ export class Header extends React.Component {
{hasGuestOwners(this.currentBeatmap, this.beatmapset) && ( - + <> , }} pattern={trans('beatmaps.discussions.guest')} /> - + )}
From 44135a660a8f9141b18b8317dda95ea90db46e2d Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 20 Jan 2025 16:30:32 +0900 Subject: [PATCH 06/14] Flip order and only store display message for changelog entry --- app/Models/ChangelogEntry.php | 57 +++++++++++-------- .../ChangelogEntryTransformer.php | 4 +- tests/Models/ChangelogEntryTest.php | 31 +++++----- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/app/Models/ChangelogEntry.php b/app/Models/ChangelogEntry.php index 194a87238c7..4a377b2b8ef 100644 --- a/app/Models/ChangelogEntry.php +++ b/app/Models/ChangelogEntry.php @@ -39,9 +39,9 @@ class ChangelogEntry extends Model public static function convertLegacy($changelog) { - $message = $changelog->message; - $splitMessage = static::splitMessage($message); - $title = $splitMessage[0]; + $splitMessage = static::splitMessage($changelog->message); + $title = presence($splitMessage[0]); + $message = $splitMessage[1]; if ($title === null) { $title = $splitMessage[1]; @@ -65,6 +65,13 @@ public static function convertLegacy($changelog) ]); } + public static function getDisplayMessage(?string $origMessage): string + { + $split = static::splitMessage($origMessage); + + return $split[1] === null ? '' : $split[0]; + } + public static function guessCategory($data) { static $ignored = [ @@ -125,7 +132,7 @@ public static function importFromGithub($data) 'category' => static::guessCategory($data), 'created_at' => Carbon::parse($data['pull_request']['merged_at']), 'github_pull_request_id' => $data['pull_request']['number'], - 'message' => $data['pull_request']['body'], + 'message' => static::getDisplayMessage($data['pull_request']['body']), 'private' => static::isPrivate($data), 'title' => $data['pull_request']['title'], 'type' => static::guessType($data), @@ -180,21 +187,30 @@ public static function placeholder() ]); } - public static function splitMessage($message) + /** + * Returns array of message split by thematic break (`---`) + * + * The array length is always two. + * If the message is empty, both values will be null. + * If there's no thematic break, the second value will be null. + */ + public static function splitMessage($message): array { if (!present($message)) { return [null, null]; } static $separator = "\n\n---\n"; - // prepended with \n\n just in case the message starts with ---\n (blank first part). - $message = "\n\n".trim(str_replace("\r\n", "\n", $message)); - $splitPos = null_if_false(strpos($message, $separator)) ?? strlen($message); - - return [ - presence(trim(substr($message, 0, $splitPos))), - presence(trim(substr($message, $splitPos + strlen($separator)))), - ]; + // Surround with newlines to handle separator at the start/end. + $message = "\n\n".trim(strtr($message, ["\r\n" => "\n"]))."\n"; + $splitPos = strpos($message, $separator); + + return $splitPos === false + ? [trim($message), null] + : [ + trim(substr($message, 0, $splitPos)), + trim(substr($message, $splitPos + strlen($separator))), + ]; } public function builds() @@ -253,21 +269,12 @@ public function githubUrl() } } - public function publicMessageHtml() + public function messageHtml(): ?string { return $this->memoize(__FUNCTION__, function () { - $message = $this->publicMessage(); + $message = $this->message; - if ($message !== null) { - return markdown($message, 'changelog_entry'); - } - }); - } - - public function publicMessage() - { - return $this->memoize(__FUNCTION__, function () { - return static::splitMessage($this->message)[1]; + return present($message) ? markdown($message, 'changelog_entry') : null; }); } } diff --git a/app/Transformers/ChangelogEntryTransformer.php b/app/Transformers/ChangelogEntryTransformer.php index f11f9091f63..0d190c36b24 100644 --- a/app/Transformers/ChangelogEntryTransformer.php +++ b/app/Transformers/ChangelogEntryTransformer.php @@ -39,11 +39,11 @@ public function includeGithubUser(ChangelogEntry $entry) public function includeMessage(ChangelogEntry $entry) { - return $this->primitive($entry->publicMessage()); + return $this->primitive($entry->message); } public function includeMessageHtml(ChangelogEntry $entry) { - return $this->primitive($entry->publicMessageHtml()); + return $this->primitive($entry->messageHtml()); } } diff --git a/tests/Models/ChangelogEntryTest.php b/tests/Models/ChangelogEntryTest.php index 367da68468b..e4e31beec7c 100644 --- a/tests/Models/ChangelogEntryTest.php +++ b/tests/Models/ChangelogEntryTest.php @@ -12,12 +12,11 @@ class ChangelogEntryTest extends TestCase { /** - * @dataProvider dataForPublicMessageHtmlVisibility + * @dataProvider dataForGetDisplayMessage */ - public function testPublicMessageHtmlVisibility($message, $html) + public function testGetDisplayMessage($message, $html) { - $entry = new ChangelogEntry(compact('message')); - $this->assertSame($html, $entry->publicMessageHtml()); + $this->assertSame($html, ChangelogEntry::getDisplayMessage($message)); } public function testConvertLegacyChangelogWithTitle() @@ -26,7 +25,7 @@ public function testConvertLegacyChangelogWithTitle() $legacy = new Changelog(['message' => $title]); $converted = ChangelogEntry::convertLegacy($legacy); $this->assertSame($title, $converted->title); - $this->assertNull($converted->publicMessageHtml()); + $this->assertNull($converted->messageHtml()); } public function testConvertLegacyChangelogWithTitleAndMessage() @@ -36,7 +35,7 @@ public function testConvertLegacyChangelogWithTitleAndMessage() $legacy = new Changelog(['message' => "{$title}\n\n---\n{$message}"]); $converted = ChangelogEntry::convertLegacy($legacy); $this->assertSame($title, $converted->title); - $this->assertSame("

{$message}

\n
", $converted->publicMessageHtml()); + $this->assertSame("

{$message}

\n
", $converted->messageHtml()); } public function testConvertLegacyChangelogWithMessage() @@ -45,7 +44,7 @@ public function testConvertLegacyChangelogWithMessage() $legacy = new Changelog(['message' => "---\n{$message}"]); $converted = ChangelogEntry::convertLegacy($legacy); $this->assertSame($message, $converted->title); - $this->assertNull($converted->publicMessageHtml()); + $this->assertNull($converted->messageHtml()); } public function testGuessCategoryCapitalise() @@ -131,17 +130,17 @@ public function testIsPrivate() $this->assertTrue(ChangelogEntry::isPrivate($data)); } - public static function dataForPublicMessageHtmlVisibility() + public static function dataForGetDisplayMessage() { return [ - ['Hidden', null], - ["---\nVisible", "

Visible

\n
"], - ["Hidden\n---", null], - ["Hidden\n\n---", null], - ["Hidden\n\n---Still hidden", null], - ["Hidden\n---\n\nStill hidden", null], - ["Hidden\n---\nStill hidden", null], - ["Hidden\n\n---\nVisible", "

Visible

\n
"], + ['Hidden', ''], + ["Visible\n\n---", 'Visible'], + ["---\nHidden", ''], + ['---Hidden', ''], + ['Still hidden---', ''], + ["Hidden\n---\n\nStill hidden", ''], + ["Hidden\n---\nStill hidden", ''], + ["Visible\n\n---\nHidden", 'Visible'], ]; } } From 28b5d4e2b14ec821666d51e1d1009498ef2eed65 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 20 Jan 2025 17:57:23 +0900 Subject: [PATCH 07/14] there's only one element --- resources/js/beatmap-discussions/header.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/resources/js/beatmap-discussions/header.tsx b/resources/js/beatmap-discussions/header.tsx index 7abe08c9356..7bb4de86829 100644 --- a/resources/js/beatmap-discussions/header.tsx +++ b/resources/js/beatmap-discussions/header.tsx @@ -177,14 +177,12 @@ export class Header extends React.Component {
{hasGuestOwners(this.currentBeatmap, this.beatmapset) && ( - <> - , - }} - pattern={trans('beatmaps.discussions.guest')} - /> - + , + }} + pattern={trans('beatmaps.discussions.guest')} + /> )}
From 312363c10debdc6e5c88411808cda59b1e556c14 Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 21 Jan 2025 21:13:27 +0900 Subject: [PATCH 08/14] Show user card when hovering user avatar on team member listing --- resources/css/bem/team-members-manage.less | 9 ++++++- resources/views/teams/members/index.blade.php | 24 +++++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/resources/css/bem/team-members-manage.less b/resources/css/bem/team-members-manage.less index 9ddbb0f6544..250e0ddc726 100644 --- a/resources/css/bem/team-members-manage.less +++ b/resources/css/bem/team-members-manage.less @@ -8,7 +8,7 @@ font-size: @font-size--title-small; display: grid; gap: 2px 10px; - grid-template-columns: auto 1fr auto auto auto; + grid-template-columns: 1fr auto auto auto; &__avatar { .default-border-radius(); @@ -38,4 +38,11 @@ font-weight: 600; } } + + &__username { + display: flex; + align-items: center; + width: max-content; + gap: 10px; + } } diff --git a/resources/views/teams/members/index.blade.php b/resources/views/teams/members/index.blade.php index cc549d03c58..2befc8268da 100644 --- a/resources/views/teams/members/index.blade.php +++ b/resources/views/teams/members/index.blade.php @@ -18,7 +18,6 @@
  • - {{ osu_trans('teams.members.index.table.status') }} {{ osu_trans('teams.members.index.table.joined_at') }} @@ -28,15 +27,20 @@ $user = $member->userOrDeleted(); @endphp
  • - - user_avatar) !!} - > - - - {!! link_to_user($user, null, '', []) !!} - + + + user_avatar) !!} + > + + + {{ $user->username }} + {{ osu_trans('teams.members.index.status.status_'.(int) $user->isActive()) }} @if ($user->isOnline()) From f396d040fedac7a99c8488337ad24545b48317e6 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Mon, 20 Jan 2025 22:43:50 +0900 Subject: [PATCH 09/14] update username change cost --- app/Models/User.php | 31 ++++++++++++++-------------- app/Models/UsernameChangeHistory.php | 5 +++++ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/Models/User.php b/app/Models/User.php index 53088b6d1a3..3c9f96e8fa4 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -317,24 +317,23 @@ public function getAuthPassword() public function usernameChangeCost() { - $changesToDate = $this->usernameChangeHistory() - ->whereIn('type', ['support', 'paid']) - ->count(); + $tier = min($this->usernameChangeHistory()->paid()->count(), 5); - switch ($changesToDate) { - case 0: - return 0; - case 1: - return 8; - case 2: - return 16; - case 3: - return 32; - case 4: - return 64; - default: - return 100; + if ($tier > 1) { + $lastChange = $this->usernameChangeHistory()->paid()->last()?->timestamp; + if ($lastChange !== null) { + $tier = max($tier - $lastChange->diffInYears(Carbon::now(), false), 1); + } } + + return match ($tier) { + 0 => 0, + 1 => 8, + 2 => 16, + 3 => 32, + 4 => 64, + default => 100, + }; } public function revertUsername($type = 'revert'): UsernameChangeHistory diff --git a/app/Models/UsernameChangeHistory.php b/app/Models/UsernameChangeHistory.php index 2711b5bf326..abcdc26a5fe 100644 --- a/app/Models/UsernameChangeHistory.php +++ b/app/Models/UsernameChangeHistory.php @@ -21,6 +21,11 @@ class UsernameChangeHistory extends Model protected $table = 'osu_username_change_history'; protected $primaryKey = 'change_id'; + public function scopePaid($query) + { + $query->whereIn('type', ['support', 'paid']); // changed by support counts as paid. + } + public function scopeVisible($query) { $query->whereIn('type', ['support', 'paid', 'admin']); From 50124cdeac46cbb39a4865e3b7f2553631fc7e4c Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 22 Jan 2025 14:25:39 +0900 Subject: [PATCH 10/14] add test --- .../UsernameChangeHistoryFactory.php | 31 +++++++++++++++++++ tests/Models/UserTest.php | 27 ++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 database/factories/UsernameChangeHistoryFactory.php diff --git a/database/factories/UsernameChangeHistoryFactory.php b/database/factories/UsernameChangeHistoryFactory.php new file mode 100644 index 00000000000..7359c941b6c --- /dev/null +++ b/database/factories/UsernameChangeHistoryFactory.php @@ -0,0 +1,31 @@ +. 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\User; +use App\Models\UsernameChangeHistory; +use Carbon\Carbon; + +class UsernameChangeHistoryFactory extends Factory +{ + protected $model = UsernameChangeHistory::class; + + public function definition(): array + { + return [ + 'timestamp' => Carbon::now(), + 'type' => 'paid', + 'user_id' => User::factory(), + + // depend on user_id; the username will be incorrect when factorying multiple names at once, + // so they should be handled separately if realistic name changes are wanted. + 'username' => fn (array $attr) => User::find($attr['user_id'])->username, + 'username_last' => fn (array $attr) => "{$attr['username']}_prev", + ]; + } +} diff --git a/tests/Models/UserTest.php b/tests/Models/UserTest.php index 0367dd3a709..f2f4ff62c00 100644 --- a/tests/Models/UserTest.php +++ b/tests/Models/UserTest.php @@ -10,11 +10,26 @@ use App\Libraries\Session\Store; use App\Models\OAuth\Token; use App\Models\User; +use App\Models\UsernameChangeHistory; use Database\Factories\OAuth\RefreshTokenFactory; use Tests\TestCase; class UserTest extends TestCase { + public static function dataProviderForUsernameChangeCost() + { + return [ + [0, 0], + [1, 8], + [2, 16], + [3, 32], + [4, 64], + [5, 100], + [6, 100], + [10, 100], + ]; + } + /** * @dataProvider dataProviderForAttributeTwitter */ @@ -25,6 +40,18 @@ public function testAttributeTwitter($setValue, $getValue) $this->assertSame($getValue, $user->user_twitter); } + /** + * @dataProvider dataProviderForUsernameChangeCost + */ + public function testChangeUsernameChangeCost(int $changes, int $cost) + { + $user = User::factory() + ->has(UsernameChangeHistory::factory()->count($changes)->state(['type' => 'paid'])) + ->create(); + + $this->assertSame($user->usernameChangeCost(), $cost); + } + public function testEmailLoginDisabled() { config_set('osu.user.allow_email_login', false); From 8b04bbf0586ecc96d43774c5802386ee9401fa65 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 22 Jan 2025 17:42:31 +0900 Subject: [PATCH 11/14] add test for cost reduction --- tests/Models/UserTest.php | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tests/Models/UserTest.php b/tests/Models/UserTest.php index f2f4ff62c00..778fc5b54d7 100644 --- a/tests/Models/UserTest.php +++ b/tests/Models/UserTest.php @@ -11,6 +11,7 @@ use App\Models\OAuth\Token; use App\Models\User; use App\Models\UsernameChangeHistory; +use Carbon\CarbonImmutable; use Database\Factories\OAuth\RefreshTokenFactory; use Tests\TestCase; @@ -30,6 +31,21 @@ public static function dataProviderForUsernameChangeCost() ]; } + public static function dataProviderForUsernameChangeCostLastChange() + { + // assume there are 6 changes (max tier + 1) + return [ + [0, 100], + [1, 64], + [2, 32], + [3, 16], + [4, 8], + [5, 8], + [6, 8], + [10, 8], + ]; + } + /** * @dataProvider dataProviderForAttributeTwitter */ @@ -43,12 +59,28 @@ public function testAttributeTwitter($setValue, $getValue) /** * @dataProvider dataProviderForUsernameChangeCost */ - public function testChangeUsernameChangeCost(int $changes, int $cost) + public function testUsernameChangeCost(int $changes, int $cost) + { + $user = User::factory() + ->has(UsernameChangeHistory::factory()->count($changes)) + ->create(); + + $this->assertSame($user->usernameChangeCost(), $cost); + } + + /** + * @dataProvider dataProviderForUsernameChangeCostLastChange + */ + public function testUsernameChangeCostLastChange(int $years, int $cost) { + $this->travelTo(CarbonImmutable::now()->subYears($years)); + $user = User::factory() - ->has(UsernameChangeHistory::factory()->count($changes)->state(['type' => 'paid'])) + ->has(UsernameChangeHistory::factory()->count(6)) // 6 = max tier + 1 ->create(); + $this->travelBack(); + $this->assertSame($user->usernameChangeCost(), $cost); } From d54218a1c8253e941c0fa2023bc25a10fcd5a37b Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 22 Jan 2025 18:36:11 +0900 Subject: [PATCH 12/14] more tests --- tests/Models/UserTest.php | 54 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/Models/UserTest.php b/tests/Models/UserTest.php index 778fc5b54d7..963340f5556 100644 --- a/tests/Models/UserTest.php +++ b/tests/Models/UserTest.php @@ -46,6 +46,28 @@ public static function dataProviderForUsernameChangeCostLastChange() ]; } + public static function dataProviderForUsernameChangeCostType() + { + return [ + ['admin', 0], + ['inactive', 0], + ['paid', 8], + ['revert', 0], + ['support', 8], + ]; + } + + public static function dataProviderForUsernameChangeCostTypeLastChange() + { + return [ + ['admin', 8], + ['inactive', 8], + ['paid', 32], + ['revert', 8], + ['support', 32], + ]; + } + /** * @dataProvider dataProviderForAttributeTwitter */ @@ -84,6 +106,38 @@ public function testUsernameChangeCostLastChange(int $years, int $cost) $this->assertSame($user->usernameChangeCost(), $cost); } + /** + * @dataProvider dataProviderForUsernameChangeCostType + */ + public function testUsernameChangeCostType(string $type, int $cost) + { + $user = User::factory() + ->has(UsernameChangeHistory::factory()->state(['type' => $type])) + ->create(); + + $this->assertSame($user->usernameChangeCost(), $cost); + } + + /** + * This tests the correct last UsernameChangeHistory is used when applying the cost changes. + * + * @dataProvider dataProviderForUsernameChangeCostTypeLastChange + */ + public function testUsernameChangeCostTypeLastChange(string $type, int $cost) + { + $this->travelTo(CarbonImmutable::now()->subYears(1)); + + $user = User::factory() + ->has(UsernameChangeHistory::factory()->count(2)) + ->create(); + + $this->travelBack(); + + UsernameChangeHistory::factory()->state(['type' => $type, 'user_id' => $user])->create(); + + $this->assertSame($user->usernameChangeCost(), $cost); + } + public function testEmailLoginDisabled() { config_set('osu.user.allow_email_login', false); From 965310b7676c64005f13cd2a98b782a8aa38d136 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Wed, 22 Jan 2025 18:37:41 +0900 Subject: [PATCH 13/14] reorder tests and dataproviders --- tests/Models/UserTest.php | 190 +++++++++++++++++++------------------- 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/tests/Models/UserTest.php b/tests/Models/UserTest.php index 963340f5556..e9d06d1d606 100644 --- a/tests/Models/UserTest.php +++ b/tests/Models/UserTest.php @@ -17,6 +17,17 @@ class UserTest extends TestCase { + public static function dataProviderForAttributeTwitter(): array + { + return [ + ['@hello', 'hello'], + ['hello', 'hello'], + ['@', null], + ['', null], + [null, null], + ]; + } + public static function dataProviderForUsernameChangeCost() { return [ @@ -68,6 +79,30 @@ public static function dataProviderForUsernameChangeCostTypeLastChange() ]; } + public static function dataProviderValidDiscordUsername(): array + { + return [ + ['username', true], + ['user_name', true], + ['user.name', true], + ['user2name', true], + ['u_sernam.e1337', true], + ['username#', false], + ['u', false], + ['morethan32characterinthisusername', false], // 33 characters + + // old format + ['username#1337', true], + ['ユーザー名#1337', true], + ['username#1', false], + ['username#13bb', false], + ['username#abcd', false], + ['user@name#1337', false], + ['user#name#1337', false], + ['user:name#1337', false], + ]; + } + /** * @dataProvider dataProviderForAttributeTwitter */ @@ -78,6 +113,66 @@ public function testAttributeTwitter($setValue, $getValue) $this->assertSame($getValue, $user->user_twitter); } + public function testEmailLoginDisabled() + { + config_set('osu.user.allow_email_login', false); + User::factory()->create([ + 'username' => 'test', + 'user_email' => 'test@example.org', + ]); + + $this->assertNull(User::findForLogin('test@example.org')); + } + + public function testEmailLoginEnabled() + { + config_set('osu.user.allow_email_login', true); + $user = User::factory()->create([ + 'username' => 'test', + 'user_email' => 'test@example.org', + ]); + + $this->assertTrue($user->is(User::findForLogin('test@example.org'))); + } + + public function testResetSessions(): void + { + $user = User::factory()->create(); + + // create session + $this->post(route('login'), ['username' => $user->username, 'password' => User::factory()::DEFAULT_PASSWORD]); + // sanity check + $this->assertNotEmpty(Store::ids($user->getKey())); + + // create token + $token = Token::factory()->create(['user_id' => $user, 'revoked' => false]); + $refreshToken = (new RefreshTokenFactory())->create(['access_token_id' => $token, 'revoked' => false]); + + $user->resetSessions(); + + $this->assertEmpty(Store::ids($user->getKey())); + $this->assertTrue($token->fresh()->revoked); + $this->assertTrue($refreshToken->fresh()->revoked); + } + + public function testUsernameAvailableAtForDefaultGroup() + { + config_set('osu.user.allowed_rename_groups', ['default']); + $allowedAtUpTo = now()->addYears(5); + $user = User::factory()->withGroup('default')->create(); + + $this->assertLessThanOrEqual($allowedAtUpTo, $user->getUsernameAvailableAt()); + } + + public function testUsernameAvailableAtForNonDefaultGroup() + { + config_set('osu.user.allowed_rename_groups', ['default']); + $allowedAt = now()->addYears(10); + $user = User::factory()->withGroup('gmt')->create(['group_id' => app('groups')->byIdentifier('default')]); + + $this->assertGreaterThanOrEqual($allowedAt, $user->getUsernameAvailableAt()); + } + /** * @dataProvider dataProviderForUsernameChangeCost */ @@ -138,66 +233,6 @@ public function testUsernameChangeCostTypeLastChange(string $type, int $cost) $this->assertSame($user->usernameChangeCost(), $cost); } - public function testEmailLoginDisabled() - { - config_set('osu.user.allow_email_login', false); - User::factory()->create([ - 'username' => 'test', - 'user_email' => 'test@example.org', - ]); - - $this->assertNull(User::findForLogin('test@example.org')); - } - - public function testEmailLoginEnabled() - { - config_set('osu.user.allow_email_login', true); - $user = User::factory()->create([ - 'username' => 'test', - 'user_email' => 'test@example.org', - ]); - - $this->assertTrue($user->is(User::findForLogin('test@example.org'))); - } - - public function testUsernameAvailableAtForDefaultGroup() - { - config_set('osu.user.allowed_rename_groups', ['default']); - $allowedAtUpTo = now()->addYears(5); - $user = User::factory()->withGroup('default')->create(); - - $this->assertLessThanOrEqual($allowedAtUpTo, $user->getUsernameAvailableAt()); - } - - public function testUsernameAvailableAtForNonDefaultGroup() - { - config_set('osu.user.allowed_rename_groups', ['default']); - $allowedAt = now()->addYears(10); - $user = User::factory()->withGroup('gmt')->create(['group_id' => app('groups')->byIdentifier('default')]); - - $this->assertGreaterThanOrEqual($allowedAt, $user->getUsernameAvailableAt()); - } - - public function testResetSessions(): void - { - $user = User::factory()->create(); - - // create session - $this->post(route('login'), ['username' => $user->username, 'password' => User::factory()::DEFAULT_PASSWORD]); - // sanity check - $this->assertNotEmpty(Store::ids($user->getKey())); - - // create token - $token = Token::factory()->create(['user_id' => $user, 'revoked' => false]); - $refreshToken = (new RefreshTokenFactory())->create(['access_token_id' => $token, 'revoked' => false]); - - $user->resetSessions(); - - $this->assertEmpty(Store::ids($user->getKey())); - $this->assertTrue($token->fresh()->revoked); - $this->assertTrue($refreshToken->fresh()->revoked); - } - /** * @dataProvider dataProviderValidDiscordUsername */ @@ -212,39 +247,4 @@ public function testValidDiscordUsername(string $username, bool $valid) $this->assertArrayHasKey('user_discord', $user->validationErrors()->all()); } } - - public static function dataProviderForAttributeTwitter(): array - { - return [ - ['@hello', 'hello'], - ['hello', 'hello'], - ['@', null], - ['', null], - [null, null], - ]; - } - - public static function dataProviderValidDiscordUsername(): array - { - return [ - ['username', true], - ['user_name', true], - ['user.name', true], - ['user2name', true], - ['u_sernam.e1337', true], - ['username#', false], - ['u', false], - ['morethan32characterinthisusername', false], // 33 characters - - // old format - ['username#1337', true], - ['ユーザー名#1337', true], - ['username#1', false], - ['username#13bb', false], - ['username#abcd', false], - ['user@name#1337', false], - ['user#name#1337', false], - ['user:name#1337', false], - ]; - } } From a68d5e04f2bd2c1a1a971484a73e9b59491eb5e3 Mon Sep 17 00:00:00 2001 From: bakaneko Date: Thu, 23 Jan 2025 03:30:18 +0900 Subject: [PATCH 14/14] whoops, flipped order --- tests/Models/UserTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Models/UserTest.php b/tests/Models/UserTest.php index e9d06d1d606..1a623a2d57a 100644 --- a/tests/Models/UserTest.php +++ b/tests/Models/UserTest.php @@ -182,7 +182,7 @@ public function testUsernameChangeCost(int $changes, int $cost) ->has(UsernameChangeHistory::factory()->count($changes)) ->create(); - $this->assertSame($user->usernameChangeCost(), $cost); + $this->assertSame($cost, $user->usernameChangeCost()); } /** @@ -198,7 +198,7 @@ public function testUsernameChangeCostLastChange(int $years, int $cost) $this->travelBack(); - $this->assertSame($user->usernameChangeCost(), $cost); + $this->assertSame($cost, $user->usernameChangeCost()); } /** @@ -210,7 +210,7 @@ public function testUsernameChangeCostType(string $type, int $cost) ->has(UsernameChangeHistory::factory()->state(['type' => $type])) ->create(); - $this->assertSame($user->usernameChangeCost(), $cost); + $this->assertSame($cost, $user->usernameChangeCost()); } /** @@ -230,7 +230,7 @@ public function testUsernameChangeCostTypeLastChange(string $type, int $cost) UsernameChangeHistory::factory()->state(['type' => $type, 'user_id' => $user])->create(); - $this->assertSame($user->usernameChangeCost(), $cost); + $this->assertSame($cost, $user->usernameChangeCost()); } /**