From cebf58bc50e05c2d317c844e9dfa9b558cb36388 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 14 Nov 2024 19:23:45 +0900 Subject: [PATCH 01/10] Rely on model validator to reset country attribute And use singleton for looking up while at it. --- app/Http/Controllers/UsersController.php | 10 ++++++---- app/Models/User.php | 7 ++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/UsersController.php b/app/Http/Controllers/UsersController.php index 605f995b851..cd72c10642a 100644 --- a/app/Http/Controllers/UsersController.php +++ b/app/Http/Controllers/UsersController.php @@ -17,7 +17,6 @@ use App\Libraries\UserRegistration; use App\Models\Beatmap; use App\Models\BeatmapDiscussion; -use App\Models\Country; use App\Models\IpBan; use App\Models\Log; use App\Models\User; @@ -983,9 +982,8 @@ private function storeUser(array $rawParams) 'username', ], ['null_missing' => true]); $countryCode = request_country(); - $country = Country::find($countryCode); $params['user_ip'] = $ip; - $params['country_acronym'] = $country === null ? '' : $country->getKey(); + $params['country_acronym'] = $countryCode; $params['user_lang'] = \App::getLocale(); $registration = new UserRegistration($params); @@ -1009,7 +1007,11 @@ private function storeUser(array $rawParams) $user = $registration->user(); // report unknown country code but ignore non-country from cloudflare - if ($countryCode !== null && $country === null && $countryCode !== 'T1') { + if ( + $countryCode !== null + && $countryCode !== 'T1' + && app('countries')->byCode($countryCode) === null + ) { app('sentry')->getClient()->captureMessage( 'User registered from unknown country', null, diff --git a/app/Models/User.php b/app/Models/User.php index 8f51d3e2739..336742f95ae 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -2320,11 +2320,12 @@ public function isValid() if ($this->isDirty('country_acronym')) { if (present($this->country_acronym)) { - if (($country = Country::find($this->country_acronym)) !== null) { + $country = app('countries')->byCode($this->country_acronym); + if ($country === null) { + $this->validationErrors()->add('country', '.invalid_country'); + } else { // ensure matching case $this->country_acronym = $country->getKey(); - } else { - $this->validationErrors()->add('country', '.invalid_country'); } } else { $this->country_acronym = Country::UNKNOWN; From 546f90c3f946f2d663a4ceea6bb298747676c451 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 14 Nov 2024 19:24:30 +0900 Subject: [PATCH 02/10] Use singleton for country lookup --- app/Libraries/User/CountryChange.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Libraries/User/CountryChange.php b/app/Libraries/User/CountryChange.php index d8304baab04..f28943cc94b 100644 --- a/app/Libraries/User/CountryChange.php +++ b/app/Libraries/User/CountryChange.php @@ -9,7 +9,6 @@ use App\Exceptions\InvariantException; use App\Models\Beatmap; -use App\Models\Country; use App\Models\User; use App\Models\UserAccountHistory; @@ -18,7 +17,7 @@ class CountryChange public static function handle(User $user, string $newCountry, string $reason): void { // Assert valid country acronym - $country = Country::find($newCountry); + $country = app('countries')->byCode($newCountry); if ($country === null) { throw new InvariantException('invalid country specified'); } From 77ff38a089c5058327beab2de686b36289931284 Mon Sep 17 00:00:00 2001 From: nanaya Date: Fri, 15 Nov 2024 16:31:16 +0900 Subject: [PATCH 03/10] Reset singleton after creating --- database/factories/CountryFactory.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/database/factories/CountryFactory.php b/database/factories/CountryFactory.php index e3ca111fd94..a3ac82f6c6c 100644 --- a/database/factories/CountryFactory.php +++ b/database/factories/CountryFactory.php @@ -13,6 +13,11 @@ class CountryFactory extends Factory { protected $model = Country::class; + public function configure(): static + { + return $this->afterCreating(fn () => app('countries')->resetMemoized()); + } + public function definition(): array { return [ From 5df41aae31164aac9c45126c6b205379a756e448 Mon Sep 17 00:00:00 2001 From: nanaya Date: Sat, 16 Nov 2024 02:53:53 +0900 Subject: [PATCH 04/10] Revert "workaround not redirecting" This reverts commit c8dfa67a6ad039da0fb907dd314e618f40d81887. --- resources/views/layout/ujs-redirect.blade.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/views/layout/ujs-redirect.blade.php b/resources/views/layout/ujs-redirect.blade.php index 84052e14c6a..85c2d06ba6e 100644 --- a/resources/views/layout/ujs-redirect.blade.php +++ b/resources/views/layout/ujs-redirect.blade.php @@ -3,6 +3,6 @@ See the LICENCE file in the repository root for full licence text. --}} ;(function() { - $(document).off(".ujsHideLoadingOverlay"); - window.setTimeout(() => Turbo.visit({!! json_encode($url) !!}), 0); + $(document).off(".ujsHideLoadingOverlay") + Turbo.visit({!! json_encode($url) !!}) }).call(this); From 1a164e4cbe8c218759375f14c6296682561b3b41 Mon Sep 17 00:00:00 2001 From: nanaya Date: Sat, 16 Nov 2024 02:53:57 +0900 Subject: [PATCH 05/10] Fix incorrect password reset form parameter --- resources/views/password_reset/index.blade.php | 1 - 1 file changed, 1 deletion(-) diff --git a/resources/views/password_reset/index.blade.php b/resources/views/password_reset/index.blade.php index d8235e796cb..e4af3c8658a 100644 --- a/resources/views/password_reset/index.blade.php +++ b/resources/views/password_reset/index.blade.php @@ -22,7 +22,6 @@
Date: Sat, 16 Nov 2024 03:07:12 +0900 Subject: [PATCH 06/10] Fix handling cover preset update --- .../UserCoverPresetsController.php | 2 +- resources/js/setup-turbo.ts | 6 +++ resources/views/layout/ujs-redirect.blade.php | 1 + .../views/user_cover_presets/index.blade.php | 37 +++++++++++-------- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/app/Http/Controllers/UserCoverPresetsController.php b/app/Http/Controllers/UserCoverPresetsController.php index 87bdfa70122..2a8302b0cbe 100644 --- a/app/Http/Controllers/UserCoverPresetsController.php +++ b/app/Http/Controllers/UserCoverPresetsController.php @@ -82,6 +82,6 @@ public function update(string $id): Response $item->update(['active' => $params['active']]); } - return ujs_redirect(route('user-cover-presets.index').'#cover-'.$item->getKey()); + return response(null, 204); } } diff --git a/resources/js/setup-turbo.ts b/resources/js/setup-turbo.ts index f6c0e566b4d..b2f7bb2eb67 100644 --- a/resources/js/setup-turbo.ts +++ b/resources/js/setup-turbo.ts @@ -3,6 +3,7 @@ import '@hotwired/turbo'; import { hideLoadingOverlay, showLoadingOverlay } from 'utils/loading-overlay'; +import { reloadPage } from 'utils/turbolinks'; Turbo.config.drive.progressBarDelay = 0; @@ -17,6 +18,11 @@ document.addEventListener('turbo:submit-start', (e) => { } }); document.addEventListener('turbo:submit-end', hideLoadingOverlay); +document.addEventListener('turbo:submit-end', (e) => { + if (e.detail.success && e.detail.formSubmission.formElement.dataset.reloadOnSuccess === '1') { + reloadPage(); + } +}); // disable turbo navigation for old webs document.addEventListener('turbo:click', (event) => { diff --git a/resources/views/layout/ujs-redirect.blade.php b/resources/views/layout/ujs-redirect.blade.php index 85c2d06ba6e..2aa99b18884 100644 --- a/resources/views/layout/ujs-redirect.blade.php +++ b/resources/views/layout/ujs-redirect.blade.php @@ -4,5 +4,6 @@ --}} ;(function() { $(document).off(".ujsHideLoadingOverlay") + Turbo.cache.clear(); Turbo.visit({!! json_encode($url) !!}) }).call(this); diff --git a/resources/views/user_cover_presets/index.blade.php b/resources/views/user_cover_presets/index.blade.php index bbdd09dca0d..8558caee5db 100644 --- a/resources/views/user_cover_presets/index.blade.php +++ b/resources/views/user_cover_presets/index.blade.php @@ -89,27 +89,31 @@ class="u-contents js-user-cover-preset-batch-enable"

- + + +

@csrf From ca8ce96c11fe42838ab628e6d3d17df983189fa6 Mon Sep 17 00:00:00 2001 From: nanaya Date: Tue, 19 Nov 2024 18:43:08 +0900 Subject: [PATCH 07/10] Remove wrapper No variables etc used so there isn't much point. --- resources/views/layout/ujs-redirect.blade.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/resources/views/layout/ujs-redirect.blade.php b/resources/views/layout/ujs-redirect.blade.php index 2aa99b18884..fe651fb8e7a 100644 --- a/resources/views/layout/ujs-redirect.blade.php +++ b/resources/views/layout/ujs-redirect.blade.php @@ -2,8 +2,6 @@ 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. --}} -;(function() { - $(document).off(".ujsHideLoadingOverlay") - Turbo.cache.clear(); - Turbo.visit({!! json_encode($url) !!}) -}).call(this); +$(document).off('.ujsHideLoadingOverlay'); +Turbo.cache.clear(); +Turbo.visit({!! json_encode($url) !!}); From 6c8ee9384c5a6d1f2fc38123f52ca289134130a0 Mon Sep 17 00:00:00 2001 From: nanaya Date: Wed, 20 Nov 2024 15:38:37 +0900 Subject: [PATCH 08/10] Fix cache key for ip2asn --- .github/workflows/tests.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5e697737ab3..885e854a895 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -137,11 +137,16 @@ jobs: key: ${{ runner.os }}-composer-${{ hashFiles('composer.lock') }} restore-keys: ${{ runner.os }}-composer- + - name: Get current date + id: current-date + run: echo "today=$(date '+%Y%m%d')" >> "$GITHUB_OUTPUT" + - name: Cache ip2asn uses: actions/cache@v4 with: path: database/ip2asn/ - key: ip2asn + key: ip2asn-${{ steps.current-date.outputs.today }} + restore-keys: ip2asn- - run: ./build.sh From d45e987508b7a355bdaa8d8de0789fe76bdbba84 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 21 Nov 2024 20:44:58 +0900 Subject: [PATCH 09/10] Make test user session/token helpers chainable --- tests/TestCase.php | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index 9a95cea322a..8a70dd92270 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -152,24 +152,22 @@ protected function tearDown(): void /** * Act as a User with OAuth scope permissions. */ - protected function actAsScopedUser(?User $user, ?array $scopes = ['*'], ?Client $client = null): void + protected function actAsScopedUser(?User $user, ?array $scopes = ['*'], ?Client $client = null): static { - $this->actingWithToken($this->createToken( + return $this->actingWithToken($this->createToken( $user, $scopes, $client ?? Client::factory()->create(), )); } - protected function actAsUser(?User $user, bool $verified = false, $driver = null) + protected function actAsUser(?User $user, bool $verified = false, $driver = null): static { - if ($user === null) { - return; + if ($user !== null) { + $this->be($user, $driver)->withSession(['verified' => $verified]); } - $this->be($user, $driver); - - $this->withSession(['verified' => $verified]); + return $this; } /** @@ -179,7 +177,7 @@ protected function actAsUser(?User $user, bool $verified = false, $driver = null * @param string $driver Auth driver to use. * @return void */ - protected function actAsUserWithToken(Token $token, $driver = null) + protected function actAsUserWithToken(Token $token, $driver = null): static { $guard = app('auth')->guard($driver); $user = $token->getResourceOwner(); @@ -196,24 +194,21 @@ protected function actAsUserWithToken(Token $token, $driver = null) request()->attributes->set(AuthApi::REQUEST_OAUTH_TOKEN_KEY, $token); app('auth')->shouldUse($driver); - } - - protected function actingAsVerified($user) - { - $this->actAsUser($user, true); return $this; } - protected function actingWithToken($token) + protected function actingAsVerified($user): static { - $this->actAsUserWithToken($token); + return $this->actAsUser($user, true); + } + protected function actingWithToken($token): static + { $encodedToken = EncodeToken::encodeAccessToken($token); - return $this->withHeaders([ - 'Authorization' => "Bearer {$encodedToken}", - ]); + return $this->actAsUserWithToken($token) + ->withHeaders(['Authorization' => "Bearer {$encodedToken}"]); } protected function assertEqualsUpToOneSecond(CarbonInterface $expected, CarbonInterface $actual): void From 79a433f926de76761155c074fcbebffc68098a24 Mon Sep 17 00:00:00 2001 From: nanaya Date: Thu, 21 Nov 2024 22:24:23 +0900 Subject: [PATCH 10/10] There's withToken helper --- tests/TestCase.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/TestCase.php b/tests/TestCase.php index 8a70dd92270..5dbb8b506ad 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -205,10 +205,8 @@ protected function actingAsVerified($user): static protected function actingWithToken($token): static { - $encodedToken = EncodeToken::encodeAccessToken($token); - return $this->actAsUserWithToken($token) - ->withHeaders(['Authorization' => "Bearer {$encodedToken}"]); + ->withToken(EncodeToken::encodeAccessToken($token)); } protected function assertEqualsUpToOneSecond(CarbonInterface $expected, CarbonInterface $actual): void