From ec1732fab287b828eb97c02cf21e954a204ca1e8 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 31 Oct 2026 00:30:20 +0100 Subject: [PATCH 1/3] fix: keep each GitHub handle to one profile, whatever its case --- app/Actions/UpdateMentionedOrganizations.php | 3 +- app/Actions/UpdateStaleHandles.php | 2 +- .../Requests/CreateApiOrgMemberRequest.php | 4 +- app/Http/Requests/CreateApiOrgRequest.php | 4 +- app/Http/Requests/CreateOrgMemberRequest.php | 4 +- app/Http/Requests/CreateOrgRequest.php | 4 +- app/Http/Requests/ShowOrganizationRequest.php | 11 +---- app/Models/Profile.php | 17 -------- app/Models/User.php | 7 +--- app/Queries/BioProfilesQuery.php | 3 +- app/Queries/MentionedOrganizationsQuery.php | 3 +- app/Queries/MentionedUsersQuery.php | 3 +- ..._make_profile_handles_case_insensitive.php | 42 +++++++++++++++++++ .../Api/OrganizationControllerTest.php | 7 ++++ .../Api/OrganizationFollowControllerTest.php | 12 ++++++ .../Feature/Controllers/OrgControllerTest.php | 16 +++++++ .../OrganizationControllerTest.php | 14 ------- .../Controllers/UserControllerTest.php | 28 ------------- tests/Feature/OrganizationMentionsTest.php | 5 +-- tests/Feature/ProfileHandlesMigrationTest.php | 32 ++++++++++++++ tests/Unit/Models/ProfileTest.php | 21 ++++++++++ 21 files changed, 149 insertions(+), 93 deletions(-) create mode 100644 database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php create mode 100644 tests/Feature/ProfileHandlesMigrationTest.php create mode 100644 tests/Unit/Models/ProfileTest.php diff --git a/app/Actions/UpdateMentionedOrganizations.php b/app/Actions/UpdateMentionedOrganizations.php index a2d8a4d3fa..acd26dd596 100644 --- a/app/Actions/UpdateMentionedOrganizations.php +++ b/app/Actions/UpdateMentionedOrganizations.php @@ -12,7 +12,6 @@ use Closure; use Illuminate\Http\Client\RequestException; use Illuminate\Support\Facades\Cache; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; final readonly class UpdateMentionedOrganizations @@ -49,7 +48,7 @@ public function __construct( */ public function handle(array $handles): ?CarbonImmutable { - $profiles = Profile::query()->whereIn(DB::raw('LOWER(handle)'), $handles)->get(); + $profiles = Profile::query()->whereIn('handle', $handles)->get(); $known = Organization::query()->whereKey($profiles->where('type', ProfileType::Organization)->modelKeys())->get(); /** @var list|null), Closure(): mixed}> $lookups */ diff --git a/app/Actions/UpdateStaleHandles.php b/app/Actions/UpdateStaleHandles.php index 86b1585ea6..c2ecf8da5a 100644 --- a/app/Actions/UpdateStaleHandles.php +++ b/app/Actions/UpdateStaleHandles.php @@ -16,7 +16,7 @@ public function handle(string $handle, string $githubId): void { Profile::query() - ->whereRaw('LOWER(handle) = ?', [mb_strtolower($handle)]) + ->where('handle', $handle) ->where('github_id', '!=', $githubId) ->each(function (Profile $profile): void { $profile->update(['handle' => match ($profile->github_id) { diff --git a/app/Http/Requests/CreateApiOrgMemberRequest.php b/app/Http/Requests/CreateApiOrgMemberRequest.php index f61d3d88d6..b9bc91ca14 100644 --- a/app/Http/Requests/CreateApiOrgMemberRequest.php +++ b/app/Http/Requests/CreateApiOrgMemberRequest.php @@ -38,7 +38,7 @@ public function rules(): array { return [ 'members' => ['required', 'array', 'min:1', 'max:'.(Conversation::MAX_MEMBERS - 1)], - 'members.*' => ['required', 'string', 'distinct', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], + 'members.*' => ['required', 'string', 'distinct:ignore_case', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], 'share_history' => ['required', 'boolean'], ]; } @@ -86,6 +86,6 @@ public function members(): Collection /** @var list $handles */ $handles = $this->array('members'); - return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search($user->handle, $handles, true))->values()->toBase(); + return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search(mb_strtolower($user->handle), array_map(mb_strtolower(...), $handles), true))->values()->toBase(); } } diff --git a/app/Http/Requests/CreateApiOrgRequest.php b/app/Http/Requests/CreateApiOrgRequest.php index 1df4380f55..201009b6c7 100644 --- a/app/Http/Requests/CreateApiOrgRequest.php +++ b/app/Http/Requests/CreateApiOrgRequest.php @@ -35,7 +35,7 @@ public function rules(): array return [ 'name' => ['nullable', 'string', 'max:60'], 'members' => ['required', 'array', 'min:1', 'max:'.(Conversation::MAX_MEMBERS - 1)], - 'members.*' => ['required', 'string', 'distinct', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], + 'members.*' => ['required', 'string', 'distinct:ignore_case', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], ]; } @@ -72,6 +72,6 @@ public function members(): Collection /** @var list $handles */ $handles = $this->array('members'); - return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search($user->handle, $handles, true))->values()->toBase(); + return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search(mb_strtolower($user->handle), array_map(mb_strtolower(...), $handles), true))->values()->toBase(); } } diff --git a/app/Http/Requests/CreateOrgMemberRequest.php b/app/Http/Requests/CreateOrgMemberRequest.php index 93af109132..c963912def 100644 --- a/app/Http/Requests/CreateOrgMemberRequest.php +++ b/app/Http/Requests/CreateOrgMemberRequest.php @@ -38,7 +38,7 @@ public function rules(): array { return [ 'members' => ['required', 'array', 'min:1', 'max:'.(Conversation::MAX_MEMBERS - 1)], - 'members.*' => ['required', 'string', 'distinct', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], + 'members.*' => ['required', 'string', 'distinct:ignore_case', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], 'share_history' => ['required', 'boolean'], ]; } @@ -86,6 +86,6 @@ public function members(): Collection /** @var list $handles */ $handles = $this->array('members'); - return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search($user->handle, $handles, true))->values()->toBase(); + return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search(mb_strtolower($user->handle), array_map(mb_strtolower(...), $handles), true))->values()->toBase(); } } diff --git a/app/Http/Requests/CreateOrgRequest.php b/app/Http/Requests/CreateOrgRequest.php index bea03d7514..86f66d2dde 100644 --- a/app/Http/Requests/CreateOrgRequest.php +++ b/app/Http/Requests/CreateOrgRequest.php @@ -35,7 +35,7 @@ public function rules(): array return [ 'name' => ['nullable', 'string', 'max:60'], 'members' => ['required', 'array', 'min:1', 'max:'.(Conversation::MAX_MEMBERS - 1)], - 'members.*' => ['required', 'string', 'distinct', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], + 'members.*' => ['required', 'string', 'distinct:ignore_case', Rule::exists('profiles', 'handle')->where('type', ProfileType::User->value)], ]; } @@ -70,6 +70,6 @@ public function members(): Collection /** @var list $handles */ $handles = $this->array('members'); - return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search($user->handle, $handles, true))->values()->toBase(); + return User::query()->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles))->get()->sortBy(fn (User $user): int|false => array_search(mb_strtolower($user->handle), array_map(mb_strtolower(...), $handles), true))->values()->toBase(); } } diff --git a/app/Http/Requests/ShowOrganizationRequest.php b/app/Http/Requests/ShowOrganizationRequest.php index ee07ccc82a..a641a2a25c 100644 --- a/app/Http/Requests/ShowOrganizationRequest.php +++ b/app/Http/Requests/ShowOrganizationRequest.php @@ -36,20 +36,13 @@ public function tab(): string } /** - * Organizations share the "@handle" pages with users, so this is the handle no user has, whatever its case, - * the exact case first. + * Organizations share the "@handle" pages with users, so this is the handle no user has, whatever its case. */ public function organization(): ?Organization { $handle = $this->route('user'); assert(is_string($handle)); - return $this->organization ??= Organization::query() - ->select('organizations.*') - ->join('profiles', 'profiles.id', '=', 'organizations.id') - ->whereRaw('LOWER(profiles.handle) = LOWER(?)', [$handle]) - ->orderByRaw('profiles.handle = ? desc', [$handle]) - ->orderBy('profiles.handle') - ->first(); + return $this->organization ??= Organization::query()->whereRelation('profile', 'handle', $handle)->first(); } } diff --git a/app/Models/Profile.php b/app/Models/Profile.php index cb606ac3b8..edff9be5f8 100644 --- a/app/Models/Profile.php +++ b/app/Models/Profile.php @@ -7,14 +7,12 @@ use App\Enums\ProfileType; use Carbon\CarbonInterface; use Database\Factories\ProfileFactory; -use Illuminate\Contracts\Database\Eloquent\Builder as BuilderContract; use Illuminate\Database\Eloquent\Attributes\Touches; use Illuminate\Database\Eloquent\Attributes\Visible; use Illuminate\Database\Eloquent\Concerns\HasUuids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\HasOne; -use Illuminate\Database\Eloquent\Relations\Relation; /** * A GitHub account at its "@handle", mirrored from GitHub: what GitHub calls a profile owner. The handle is GitHub's @@ -78,21 +76,6 @@ public function casts(): array ]; } - /** - * Resolves "@handle" URLs whatever the handle's case, like GitHub. A stale profile may still hold the same handle - * in another case, so the exact case wins, then a user's, then always the same one, like users' pages. - * - * @param Model|BuilderContract|Relation $query - * @param mixed $value - * @param string|null $field - */ - public function resolveRouteBindingQuery($query, $value, $field = null): BuilderContract - { - return $field === 'handle' - ? $query->whereRaw('LOWER(handle) = LOWER(?)', [$value])->orderByRaw('handle = ? desc', [$value])->orderByRaw('type = ? desc', [ProfileType::User])->orderBy('handle') - : parent::resolveRouteBindingQuery($query, $value, $field); - } - /** * @return HasOne */ diff --git a/app/Models/User.php b/app/Models/User.php index 1dd40b6244..ae1553e456 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -9,7 +9,6 @@ use App\Dto\LiveStream; use App\Enums\LivePlatform; use App\Enums\Log; -use App\Enums\ProfileType; use App\Enums\RemotesFrom; use Carbon\CarbonInterface; use Database\Factories\UserFactory; @@ -231,9 +230,7 @@ public function profile(): BelongsTo } /** - * Resolves "@handle" URLs through the profile, which holds the handle, whatever its case, like GitHub. A stale - * profile may still hold the same handle in another case, so the exact case wins, then a user's, then always the - * same one, like Profile::resolveRouteBindingQuery() for the link preview images. + * Resolves "@handle" URLs through the profile, which holds the handle, whatever its case, like GitHub. * * @param Model|BuilderContract|Relation $query * @param mixed $value @@ -242,7 +239,7 @@ public function profile(): BelongsTo public function resolveRouteBindingQuery($query, $value, $field = null): BuilderContract { return $field === 'handle' - ? parent::resolveRouteBindingQuery($query, Profile::query()->select('github_id')->whereRaw('LOWER(handle) = LOWER(?)', [$value])->orderByRaw('handle = ? desc', [$value])->orderByRaw('type = ? desc', [ProfileType::User])->orderBy('handle')->limit(1), 'github_id') + ? parent::resolveRouteBindingQuery($query, Profile::query()->select('github_id')->where('handle', $value), 'github_id') : parent::resolveRouteBindingQuery($query, $value, $field); } diff --git a/app/Queries/BioProfilesQuery.php b/app/Queries/BioProfilesQuery.php index bbb6656c40..3289cb518a 100644 --- a/app/Queries/BioProfilesQuery.php +++ b/app/Queries/BioProfilesQuery.php @@ -7,7 +7,6 @@ use App\Models\Profile; use App\Models\User; use App\Services\Mentions; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; final readonly class BioProfilesQuery @@ -32,7 +31,7 @@ public function get(User $user): array } return Profile::query() - ->whereIn(DB::raw('LOWER(handle)'), $handles) + ->whereIn('handle', $handles) ->get(['handle']) ->mapWithKeys(fn (Profile $profile): array => [Str::lower($profile->handle) => $profile->handle]) ->all(); diff --git a/app/Queries/MentionedOrganizationsQuery.php b/app/Queries/MentionedOrganizationsQuery.php index a147191e0c..1d8778ade9 100644 --- a/app/Queries/MentionedOrganizationsQuery.php +++ b/app/Queries/MentionedOrganizationsQuery.php @@ -7,7 +7,6 @@ use App\Models\Organization; use App\Services\Mentions; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; final readonly class MentionedOrganizationsQuery @@ -33,7 +32,7 @@ public function get(string $body, array $mentionedUsers): array } return Organization::query() - ->whereHas('profile', fn (Builder $query): Builder => $query->whereIn(DB::raw('LOWER(handle)'), $handles)) + ->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles)) ->get() ->mapWithKeys(fn (Organization $organization): array => [Str::lower($organization->handle) => $organization->id]) ->all(); diff --git a/app/Queries/MentionedUsersQuery.php b/app/Queries/MentionedUsersQuery.php index 28d2d4182f..4270bb4d3d 100644 --- a/app/Queries/MentionedUsersQuery.php +++ b/app/Queries/MentionedUsersQuery.php @@ -7,7 +7,6 @@ use App\Models\User; use App\Services\Mentions; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; final readonly class MentionedUsersQuery @@ -29,7 +28,7 @@ public function get(string $body): array } return User::query() - ->whereHas('profile', fn (Builder $query): Builder => $query->whereIn(DB::raw('LOWER(handle)'), $handles)) + ->whereHas('profile', fn (Builder $query): Builder => $query->whereIn('handle', $handles)) ->get() ->mapWithKeys(fn (User $user): array => [Str::lower($user->handle) => $user->id]) ->all(); diff --git a/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php b/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php new file mode 100644 index 0000000000..596397cf57 --- /dev/null +++ b/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php @@ -0,0 +1,42 @@ +dropUnique(['handle']); + $table->dropIndex('profiles_lower_handle_index'); + }); + + DB::statement('ALTER TABLE profiles ALTER COLUMN handle TYPE citext'); + + Profile::query() + ->whereIn('handle', Profile::query()->select('handle')->groupBy('handle')->havingRaw('COUNT(*) > 1')) + ->orderByRaw('github_synced_at DESC NULLS LAST') + ->latest('updated_at') + ->get() + ->unique(fn (Profile $profile): string => mb_strtolower($profile->handle)) + ->each(fn (Profile $profile) => resolve(UpdateStaleHandles::class)->handle($profile->handle, $profile->github_id)); + + Schema::table('profiles', function (Blueprint $table): void { + $table->unique('handle'); + }); + } +}; diff --git a/tests/Feature/Controllers/Api/OrganizationControllerTest.php b/tests/Feature/Controllers/Api/OrganizationControllerTest.php index 0a8cbe4835..a2016d28c7 100644 --- a/tests/Feature/Controllers/Api/OrganizationControllerTest.php +++ b/tests/Feature/Controllers/Api/OrganizationControllerTest.php @@ -35,3 +35,10 @@ $this->getJson('/api/v1/organizations/octocat')->assertNotFound(); $this->getJson('/api/v1/organizations/missing')->assertNotFound(); }); + +it('shows an organization whatever the case of its handle', function (): void { + Organization::factory()->create(['handle' => 'Laravel']); + + $this->getJson('/api/v1/organizations/LARAVEL')->assertOk()->assertJsonPath('data.handle', 'Laravel'); + $this->getJson('/api/v1/organizations/laravel/mentions')->assertOk(); +}); diff --git a/tests/Feature/Controllers/Api/OrganizationFollowControllerTest.php b/tests/Feature/Controllers/Api/OrganizationFollowControllerTest.php index 8ead56d2fa..36f0865dd6 100644 --- a/tests/Feature/Controllers/Api/OrganizationFollowControllerTest.php +++ b/tests/Feature/Controllers/Api/OrganizationFollowControllerTest.php @@ -22,6 +22,18 @@ expect($this->user->followedOrganizations()->count())->toBe(0); }); +it('may follow and unfollow an organization whatever the case of its handle', function (): void { + $organization = Organization::factory()->create(['handle' => 'Laravel']); + + signedApiRequest($this->keyPair, 'POST', route('api-organization-follow.store', ['handle' => 'laravel']))->assertNoContent(); + + expect($this->user->followedOrganizations()->pluck('organizations.id')->all())->toBe([$organization->id]); + + signedApiRequest($this->keyPair, 'DELETE', route('api-organization-follow.destroy', ['handle' => 'LARAVEL']))->assertNoContent(); + + expect($this->user->followedOrganizations()->count())->toBe(0); +}); + it('returns not found for handles without an organization page', function (string $method, string $route): void { User::factory()->create(['handle' => 'octocat']); diff --git a/tests/Feature/Controllers/OrgControllerTest.php b/tests/Feature/Controllers/OrgControllerTest.php index ab41e2d7f0..c610be9a67 100644 --- a/tests/Feature/Controllers/OrgControllerTest.php +++ b/tests/Feature/Controllers/OrgControllerTest.php @@ -26,6 +26,22 @@ ->assertJsonPath('conversations.0.members.0.role', 'maintainer'); }); +it('adds people whatever the case of their handles, but each only once', function (): void { + $mark = User::factory()->create(); + [$nuno] = followersOf($mark, 1); + + $this->actingAs($mark) + ->postJson(route('org.store'), ['members' => [mb_strtoupper($nuno->handle), $nuno->handle]]) + ->assertJsonValidationErrors(['members.0', 'members.1']); + + $org = $this->actingAs($mark) + ->postJson(route('org.store'), ['members' => [mb_strtoupper($nuno->handle)]]) + ->assertCreated() + ->json('org.id'); + + expect(Conversation::query()->findOrFail($org)->roleOf($nuno)?->value)->toBe('contributor'); +}); + it('identifies orgs by uuids', function (): void { $mark = User::factory()->create(); [$nuno] = followersOf($mark, 1); diff --git a/tests/Feature/Controllers/OrganizationControllerTest.php b/tests/Feature/Controllers/OrganizationControllerTest.php index 5e08d020c8..ee84e6910f 100644 --- a/tests/Feature/Controllers/OrganizationControllerTest.php +++ b/tests/Feature/Controllers/OrganizationControllerTest.php @@ -244,17 +244,3 @@ $this->get('/@laravel?user[]=a&user[]=b')->assertRedirect('/@Laravel?user[]=a&user[]=b'); $this->get('/@laravel?tag=one&tag=two')->assertRedirect('/@Laravel?tag=one&tag=two'); }); - -it('opens each organization at its own case when a stale one holds the handle in another case', function (): void { - $lower = Organization::factory()->create(['handle' => 'laravel']); - $mixed = Organization::factory()->create(['handle' => 'Laravel']); - - $this->get('/@laravel')->assertOk()->assertInertia(fn ($page) => $page->where('organization.id', $lower->id)); - $this->get('/@Laravel')->assertOk()->assertInertia(fn ($page) => $page->where('organization.id', $mixed->id)); - - $location = (string) $this->get('/@LARAVEL')->assertMovedPermanently()->headers->get('Location'); - - expect($location)->toBeIn([url('/@laravel'), url('/@Laravel')]); - - $this->get($location)->assertOk(); -}); diff --git a/tests/Feature/Controllers/UserControllerTest.php b/tests/Feature/Controllers/UserControllerTest.php index 1a60bde07a..5baa3dd332 100644 --- a/tests/Feature/Controllers/UserControllerTest.php +++ b/tests/Feature/Controllers/UserControllerTest.php @@ -8,7 +8,6 @@ use App\Models\Commit; use App\Models\LinkPreview; use App\Models\Organization; -use App\Models\Profile; use App\Models\User; use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Facades\DB; @@ -837,30 +836,3 @@ ->assertMovedPermanently() ->assertRedirect("/@OctoCat/{$list}"); })->with(['following', 'followers', 'mutuals']); - -it('opens each profile at its own case when a stale profile holds the handle in another case', function (): void { - $lower = User::factory()->create(['handle' => 'octocat']); - $mixed = User::factory()->create(['handle' => 'OctoCat']); - - $this->get('/@octocat')->assertOk()->assertInertia(fn ($page) => $page->where('user.id', $lower->id)); - $this->get('/@OctoCat')->assertOk()->assertInertia(fn ($page) => $page->where('user.id', $mixed->id)); - - $redirect = $this->get('/@OCTOCAT')->assertMovedPermanently(); - - expect($redirect->headers->get('Location'))->toBeIn([url('/@octocat'), url('/@OctoCat')]) - ->and($this->get('/@OCTOCAT')->headers->get('Location'))->toBe($redirect->headers->get('Location')); - - $this->get((string) $redirect->headers->get('Location'))->assertOk(); -}); - -it('opens the user rather than an organization holding the handle in another case, unless the case is exact', function (): void { - $user = User::factory()->create(['handle' => 'Laravel']); - $organization = Organization::factory()->create(['handle' => 'laravel']); - - $this->get('/@LARAVEL')->assertRedirect('/@Laravel')->assertMovedPermanently(); - $this->get('/@Laravel')->assertOk()->assertInertia(fn ($page) => $page->component('user/show')->where('user.id', $user->id)); - $this->get('/@laravel')->assertOk()->assertInertia(fn ($page) => $page->component('organization/show')->where('organization.id', $organization->id)); - - expect(new Profile()->resolveRouteBinding('LARAVEL', 'handle')?->getKey())->toBe($user->profile->getKey()) - ->and(new Profile()->resolveRouteBinding('laravel', 'handle')?->getKey())->toBe($organization->getKey()); -}); diff --git a/tests/Feature/OrganizationMentionsTest.php b/tests/Feature/OrganizationMentionsTest.php index f9c786ef46..c3f580d431 100644 --- a/tests/Feature/OrganizationMentionsTest.php +++ b/tests/Feature/OrganizationMentionsTest.php @@ -9,13 +9,12 @@ use App\Models\User; use Illuminate\Support\Facades\Queue; -it('links known organizations right away, a user of the same handle first', function (): void { +it('links known organizations right away, apart from users, whatever the case they are written in', function (): void { Queue::fake(); $author = User::factory()->create(); - $tom = User::factory()->create(['handle' => 'tom']); + $tom = User::factory()->create(['handle' => 'Tom']); $laravel = Organization::factory()->create(['handle' => 'laravel']); - Organization::factory()->create(['handle' => 'Tom']); $this->actingAs($author)->post('/commits', ['body' => 'Thanks @Laravel and @tom'])->assertRedirect(); diff --git a/tests/Feature/ProfileHandlesMigrationTest.php b/tests/Feature/ProfileHandlesMigrationTest.php new file mode 100644 index 0000000000..28112da0ce --- /dev/null +++ b/tests/Feature/ProfileHandlesMigrationTest.php @@ -0,0 +1,32 @@ +create(['handle' => 'octocat', 'github_synced_at' => now()->subYear()]); + $current = User::factory()->create(['handle' => 'OctoCat', 'github_synced_at' => now()]); + $organization = Organization::factory()->create(['handle' => 'OCTOCAT', 'github_synced_at' => null]); + $tom = User::factory()->create(['github_id' => CreateTomFriendship::GITHUB_ID, 'handle' => 'tom', 'github_synced_at' => null]); + $person = User::factory()->create(['handle' => 'Tom', 'github_synced_at' => now()]); + $alone = User::factory()->create(['handle' => 'Laravel']); + + $migration = require database_path('migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php'); + $migration->up(); + + expect(Profile::query()->pluck('handle', 'github_id')->all())->toEqual([ + $stale->github_id => "_renamed-{$stale->github_id}", + $current->github_id => 'OctoCat', + $organization->profile->github_id => "_renamed-{$organization->profile->github_id}", + $tom->github_id => CreateTomFriendship::FALLBACK_HANDLE, + $person->github_id => 'Tom', + $alone->github_id => 'Laravel', + ])->and(Profile::query()->where('handle', 'octocat')->sole()->github_id)->toBe($current->github_id); +}); diff --git a/tests/Unit/Models/ProfileTest.php b/tests/Unit/Models/ProfileTest.php new file mode 100644 index 0000000000..0f876f38d7 --- /dev/null +++ b/tests/Unit/Models/ProfileTest.php @@ -0,0 +1,21 @@ +create(['handle' => 'OctoCat']); + + expect(Profile::query()->where('handle', 'octocat')->sole()->is($user->profile))->toBeTrue() + ->and(Profile::query()->whereIn('handle', ['OCTOCAT'])->value('handle'))->toBe('OctoCat'); +}); + +it('keeps each handle to one profile whatever its case, like GitHub', function (): void { + User::factory()->create(['handle' => 'laravel']); + + expect(fn () => Organization::factory()->create(['handle' => 'Laravel']))->toThrow(UniqueConstraintViolationException::class); +}); From 0436e453af9da20a6d1122616e958a5eec5871ae Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 31 Oct 2026 10:29:22 +0100 Subject: [PATCH 2/3] refactor: convert handles without a cleanup production doesn't need, and say they ignore case where agents look --- app/Models/Profile.php | 4 +++ app/Queries/MentionSuggestionsQuery.php | 2 +- app/Queries/OrganizationSuggestionsQuery.php | 2 +- app/Queries/UserSearchQuery.php | 2 +- ..._make_profile_handles_case_insensitive.php | 19 ++--------- tests/Feature/ProfileHandlesMigrationTest.php | 32 ------------------- 6 files changed, 9 insertions(+), 52 deletions(-) delete mode 100644 tests/Feature/ProfileHandlesMigrationTest.php diff --git a/app/Models/Profile.php b/app/Models/Profile.php index edff9be5f8..cdef7e99bb 100644 --- a/app/Models/Profile.php +++ b/app/Models/Profile.php @@ -19,6 +19,10 @@ * login, which can be renamed, so the account is matched by its GitHub ID. Organizations are profiles too, and what * only they have lives on `Organization`. * + * Like on GitHub, a handle is one whatever its case: the column is `citext`, so a plain `where('handle', ...)` matches + * any case and its unique index refuses the same handle in another case. Never lowercase it in a query to compare + * it. Only searches lowercase it, as `LOWER(handle) LIKE ...`, which the trigram index serves. + * * @property-read string $id * @property-read ProfileType $type * @property-read string $github_id diff --git a/app/Queries/MentionSuggestionsQuery.php b/app/Queries/MentionSuggestionsQuery.php index 46130e1aa3..82ba3ff13a 100644 --- a/app/Queries/MentionSuggestionsQuery.php +++ b/app/Queries/MentionSuggestionsQuery.php @@ -71,7 +71,7 @@ public function get(User $user, string $term, ?Conversation $org = null, int $li ->when($contributors, fn (Builder $query, QueryBuilder $contributors): Builder => $query ->orderByDesc((clone $contributors)->selectRaw('COUNT(*) > 0')->whereColumn('commits.user_id', 'users.id'))) ->orderByRaw( - "CASE WHEN LOWER(profiles.handle) = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", + "CASE WHEN profiles.handle = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", [$term, "{$escaped}%", "{$escaped}%"], ) ->orderByDesc(fn (QueryBuilder $query): QueryBuilder => $query diff --git a/app/Queries/OrganizationSuggestionsQuery.php b/app/Queries/OrganizationSuggestionsQuery.php index 66d17e8c49..593ce4fa62 100644 --- a/app/Queries/OrganizationSuggestionsQuery.php +++ b/app/Queries/OrganizationSuggestionsQuery.php @@ -38,7 +38,7 @@ public function get(string $term, int $limit = 3): Collection ->whereRaw("LOWER(profiles.handle) LIKE ? ESCAPE '!'", ["%{$escaped}%"]) ->orWhereRaw("LOWER(profiles.name) LIKE ? ESCAPE '!'", ["%{$escaped}%"])) ->orderByRaw( - "CASE WHEN LOWER(profiles.handle) = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", + "CASE WHEN profiles.handle = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", [$term, "{$escaped}%", "{$escaped}%"], ) ->orderBy('profiles.handle') diff --git a/app/Queries/UserSearchQuery.php b/app/Queries/UserSearchQuery.php index 2e734938bd..e4135c3ce1 100644 --- a/app/Queries/UserSearchQuery.php +++ b/app/Queries/UserSearchQuery.php @@ -38,7 +38,7 @@ public function get(string $term, ?User $viewer, int $limit = 8): Collection ->whereNotIn('users.id', $viewer->mutedIds()) ->whereNotIn('users.id', $viewer->idsAcrossBlocks())) ->orderByRaw( - "CASE WHEN LOWER(profiles.handle) = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", + "CASE WHEN profiles.handle = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", [$term, "{$escaped}%", "{$escaped}%"], ) ->orderBy('profiles.handle') diff --git a/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php b/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php index 596397cf57..ee0cc2d4cc 100644 --- a/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php +++ b/database/migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php @@ -2,8 +2,6 @@ declare(strict_types=1); -use App\Actions\UpdateStaleHandles; -use App\Models\Profile; use Illuminate\Database\Migrations\Migration; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\DB; @@ -13,30 +11,17 @@ { /** * GitHub handles are unique whatever their case, so profiles' are too: comparing them ignores case, so looking one - * up needs no lowercasing, and a stale profile can't keep a handle in another case. Profiles already holding one - * in another case leave it to the one GitHub synced last and get the placeholder a sync gives them. + * up needs no lowercasing, and the unique index, rebuilt on the new type, keeps a stale profile from holding a + * handle in another case. */ public function up(): void { Schema::ensureExtensionExists('citext'); Schema::table('profiles', function (Blueprint $table): void { - $table->dropUnique(['handle']); $table->dropIndex('profiles_lower_handle_index'); }); DB::statement('ALTER TABLE profiles ALTER COLUMN handle TYPE citext'); - - Profile::query() - ->whereIn('handle', Profile::query()->select('handle')->groupBy('handle')->havingRaw('COUNT(*) > 1')) - ->orderByRaw('github_synced_at DESC NULLS LAST') - ->latest('updated_at') - ->get() - ->unique(fn (Profile $profile): string => mb_strtolower($profile->handle)) - ->each(fn (Profile $profile) => resolve(UpdateStaleHandles::class)->handle($profile->handle, $profile->github_id)); - - Schema::table('profiles', function (Blueprint $table): void { - $table->unique('handle'); - }); } }; diff --git a/tests/Feature/ProfileHandlesMigrationTest.php b/tests/Feature/ProfileHandlesMigrationTest.php deleted file mode 100644 index 28112da0ce..0000000000 --- a/tests/Feature/ProfileHandlesMigrationTest.php +++ /dev/null @@ -1,32 +0,0 @@ -create(['handle' => 'octocat', 'github_synced_at' => now()->subYear()]); - $current = User::factory()->create(['handle' => 'OctoCat', 'github_synced_at' => now()]); - $organization = Organization::factory()->create(['handle' => 'OCTOCAT', 'github_synced_at' => null]); - $tom = User::factory()->create(['github_id' => CreateTomFriendship::GITHUB_ID, 'handle' => 'tom', 'github_synced_at' => null]); - $person = User::factory()->create(['handle' => 'Tom', 'github_synced_at' => now()]); - $alone = User::factory()->create(['handle' => 'Laravel']); - - $migration = require database_path('migrations/2026_10_31_120000_make_profile_handles_case_insensitive.php'); - $migration->up(); - - expect(Profile::query()->pluck('handle', 'github_id')->all())->toEqual([ - $stale->github_id => "_renamed-{$stale->github_id}", - $current->github_id => 'OctoCat', - $organization->profile->github_id => "_renamed-{$organization->profile->github_id}", - $tom->github_id => CreateTomFriendship::FALLBACK_HANDLE, - $person->github_id => 'Tom', - $alone->github_id => 'Laravel', - ])->and(Profile::query()->where('handle', 'octocat')->sole()->github_id)->toBe($current->github_id); -}); From a962010b0726a34160b9d484c7e3a8460e3bba86 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 31 Oct 2026 10:42:28 +0100 Subject: [PATCH 3/3] refactor: rank handle matches without lowercasing, as only search filters need it for their index --- app/Models/Profile.php | 2 +- app/Queries/MentionSuggestionsQuery.php | 2 +- app/Queries/OrganizationSuggestionsQuery.php | 2 +- app/Queries/UserSearchQuery.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Models/Profile.php b/app/Models/Profile.php index cdef7e99bb..356bea7b12 100644 --- a/app/Models/Profile.php +++ b/app/Models/Profile.php @@ -21,7 +21,7 @@ * * Like on GitHub, a handle is one whatever its case: the column is `citext`, so a plain `where('handle', ...)` matches * any case and its unique index refuses the same handle in another case. Never lowercase it in a query to compare - * it. Only searches lowercase it, as `LOWER(handle) LIKE ...`, which the trigram index serves. + * it. Only search filters lowercase it, as `LOWER(handle) LIKE ...`, which the trigram index serves. * * @property-read string $id * @property-read ProfileType $type diff --git a/app/Queries/MentionSuggestionsQuery.php b/app/Queries/MentionSuggestionsQuery.php index 82ba3ff13a..71bdf66c07 100644 --- a/app/Queries/MentionSuggestionsQuery.php +++ b/app/Queries/MentionSuggestionsQuery.php @@ -71,7 +71,7 @@ public function get(User $user, string $term, ?Conversation $org = null, int $li ->when($contributors, fn (Builder $query, QueryBuilder $contributors): Builder => $query ->orderByDesc((clone $contributors)->selectRaw('COUNT(*) > 0')->whereColumn('commits.user_id', 'users.id'))) ->orderByRaw( - "CASE WHEN profiles.handle = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", + "CASE WHEN profiles.handle = ? THEN 0 WHEN profiles.handle LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", [$term, "{$escaped}%", "{$escaped}%"], ) ->orderByDesc(fn (QueryBuilder $query): QueryBuilder => $query diff --git a/app/Queries/OrganizationSuggestionsQuery.php b/app/Queries/OrganizationSuggestionsQuery.php index 593ce4fa62..b388e45e60 100644 --- a/app/Queries/OrganizationSuggestionsQuery.php +++ b/app/Queries/OrganizationSuggestionsQuery.php @@ -38,7 +38,7 @@ public function get(string $term, int $limit = 3): Collection ->whereRaw("LOWER(profiles.handle) LIKE ? ESCAPE '!'", ["%{$escaped}%"]) ->orWhereRaw("LOWER(profiles.name) LIKE ? ESCAPE '!'", ["%{$escaped}%"])) ->orderByRaw( - "CASE WHEN profiles.handle = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", + "CASE WHEN profiles.handle = ? THEN 0 WHEN profiles.handle LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", [$term, "{$escaped}%", "{$escaped}%"], ) ->orderBy('profiles.handle') diff --git a/app/Queries/UserSearchQuery.php b/app/Queries/UserSearchQuery.php index e4135c3ce1..5dfc2d2c88 100644 --- a/app/Queries/UserSearchQuery.php +++ b/app/Queries/UserSearchQuery.php @@ -38,7 +38,7 @@ public function get(string $term, ?User $viewer, int $limit = 8): Collection ->whereNotIn('users.id', $viewer->mutedIds()) ->whereNotIn('users.id', $viewer->idsAcrossBlocks())) ->orderByRaw( - "CASE WHEN profiles.handle = ? THEN 0 WHEN LOWER(profiles.handle) LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", + "CASE WHEN profiles.handle = ? THEN 0 WHEN profiles.handle LIKE ? ESCAPE '!' THEN 1 WHEN LOWER(profiles.name) LIKE ? ESCAPE '!' THEN 2 ELSE 3 END", [$term, "{$escaped}%", "{$escaped}%"], ) ->orderBy('profiles.handle')