diff --git a/app/Queries/ContributorsQuery.php b/app/Queries/ContributorsQuery.php index 1e9b64159..fce4c9959 100644 --- a/app/Queries/ContributorsQuery.php +++ b/app/Queries/ContributorsQuery.php @@ -4,13 +4,15 @@ namespace App\Queries; +use App\Models\User; use App\Services\GitHub; use Illuminate\Http\Client\HttpClientException; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Fluent; /** - * @phpstan-type Contributor array{login: string, commits: int} + * @phpstan-type Account array{login: string, github_id: string, commits: int} + * @phpstan-type Contributor array{login: string, commits: int, username: string|null} */ final readonly class ContributorsQuery { @@ -33,17 +35,43 @@ public function __construct(private GitHub $github) } /** - * The repository contributors, most commits first, as GitHub counts them. A failed read is kept for a - * minute, so an outage doesn't send every visitor to GitHub. + * The repository contributors, most commits first, as GitHub counts them, each with their /dev/null username, or + * null when they aren't on it. They are found by their GitHub account, since a username can change on either side. * * @return list|null */ public function get(): ?array + { + $accounts = $this->accounts(); + + if ($accounts === null) { + return null; + } + + $usernames = User::query() + ->whereIn('github_id', array_column($accounts, 'github_id')) + ->get() + ->mapWithKeys(fn (User $user): array => [$user->github_id => $user->username]); + + return array_map(fn (array $account): array => [ + 'login' => $account['login'], + 'commits' => $account['commits'], + 'username' => $usernames->get($account['github_id']), + ], $accounts); + } + + /** + * The GitHub accounts behind the contributors. A failed read is kept for a minute, so an outage doesn't send every + * visitor to GitHub. + * + * @return list|null + */ + private function accounts(): ?array { $repository = config()->string('services.github.repository'); - $key = "about:contributors:{$repository}"; + $key = "about:contributor-accounts:{$repository}"; - /** @var list|false|null $contributors */ + /** @var list|false|null $contributors */ $contributors = Cache::get($key); if ($contributors === null) { @@ -65,10 +93,12 @@ public function get(): ?array $contributors = [...$contributors, ...$accounts->where('type', 'User') ->mapInto(Fluent::class) ->filter(fn (Fluent $account): bool => $account->string('login')->isNotEmpty() + && is_int($account->get('id')) && is_int($account->get('contributions')) && ! in_array($account->string('login')->lower()->value(), self::Agents, true)) ->map(fn (Fluent $account): array => [ 'login' => $account->string('login')->value(), + 'github_id' => (string) $account->integer('id'), 'commits' => $account->integer('contributions'), ]) ->values() diff --git a/resources/js/lib/about.ts b/resources/js/lib/about.ts index 067c5475d..903985d2e 100644 --- a/resources/js/lib/about.ts +++ b/resources/js/lib/about.ts @@ -35,9 +35,13 @@ export type StoryPerson = { name: string; }; +/** + * Someone with a commit on production. The username is theirs on /dev/null, or null when they aren't on it. + */ export type StoryContributor = { login: string; commits: number; + username: string | null; }; export const storyBefore: StoryEvent[] = [ diff --git a/resources/js/pages/about/show.tsx b/resources/js/pages/about/show.tsx index b52abf33f..be1b5f2dc 100644 --- a/resources/js/pages/about/show.tsx +++ b/resources/js/pages/about/show.tsx @@ -13,6 +13,7 @@ import { DocumentPage, DocumentSection, } from '@/components/document-page'; +import { UserPreview } from '@/components/user-preview'; import { WordmarkText } from '@/components/wordmark-text'; import AppLayout from '@/layouts/app-layout'; import type { @@ -30,6 +31,7 @@ import { storyStreams, } from '@/lib/about'; import { cn, laneColor } from '@/lib/utils'; +import { show } from '@/routes/user'; import type { Lane } from '@/types'; const kindIcons: Record = { @@ -265,37 +267,64 @@ function Stream({ stream }: { stream: StoryStream }) { ); } +function ContributorCard({ contributor }: { contributor: StoryContributor }) { + return ( + <> + + + + @{contributor.login} + + + {contributor.commits}{' '} + {contributor.commits === 1 ? 'commit' : 'commits'} + + + + ); +} + +/** + * Contributors on /dev/null link to their profile, with their profile card on hover. Everyone else links to GitHub. + */ function Contributors({ contributors }: { contributors: StoryContributor[] }) { + const className = + 'flex items-center gap-2.5 rounded-md border px-2.5 py-2 no-underline! transition-colors hover:bg-muted'; + return ( diff --git a/tests/Feature/Controllers/AboutControllerTest.php b/tests/Feature/Controllers/AboutControllerTest.php index d6b342b53..868e0b0c5 100644 --- a/tests/Feature/Controllers/AboutControllerTest.php +++ b/tests/Feature/Controllers/AboutControllerTest.php @@ -40,7 +40,18 @@ ->assertOk() ->assertInertia(fn ($page) => $page ->has('contributors', 1) - ->where('contributors.0', ['login' => 'xHeaven', 'commits' => 187])); + ->where('contributors.0', ['login' => 'xHeaven', 'commits' => 187, 'username' => null])); +}); + +it('links each contributor who is on /dev/null to their profile, found by their GitHub account', function (): void { + fakeGitHubContributors([gitHubContributor('xHeaven', 217, id: 14284867), gitHubContributor('nunomaduro', 189)]); + User::factory()->create(['github_id' => '14284867', 'username' => 'renamed-on-devnull']); + + $this->get(route('about.show')) + ->assertOk() + ->assertInertia(fn ($page) => $page + ->where('contributors.0', ['login' => 'xHeaven', 'commits' => 217, 'username' => 'renamed-on-devnull']) + ->where('contributors.1', ['login' => 'nunomaduro', 'commits' => 189, 'username' => null])); }); it('leaves the contributors out while GitHub is unreachable', function (): void { diff --git a/tests/Pest.php b/tests/Pest.php index 837b50b2b..72a5a7da4 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -138,9 +138,9 @@ function fakeGitHubAccount(User $user, array $account = []): void /** * @return array */ -function gitHubContributor(string $login, int $commits, string $type = 'User'): array +function gitHubContributor(string $login, int $commits, string $type = 'User', ?int $id = null): array { - return ['login' => $login, 'contributions' => $commits, 'type' => $type]; + return ['login' => $login, 'id' => $id ?? crc32($login), 'contributions' => $commits, 'type' => $type]; } /** diff --git a/tests/Unit/Queries/ContributorsQueryTest.php b/tests/Unit/Queries/ContributorsQueryTest.php index 2b48148dd..87bb535fd 100644 --- a/tests/Unit/Queries/ContributorsQueryTest.php +++ b/tests/Unit/Queries/ContributorsQueryTest.php @@ -11,8 +11,8 @@ fakeGitHubContributors([gitHubContributor('xHeaven', 187), gitHubContributor('jubianchi', 1)]); expect(resolve(ContributorsQuery::class)->get())->toBe([ - ['login' => 'xHeaven', 'commits' => 187], - ['login' => 'jubianchi', 'commits' => 1], + ['login' => 'xHeaven', 'commits' => 187, 'username' => null], + ['login' => 'jubianchi', 'commits' => 1, 'username' => null], ]); }); @@ -24,7 +24,7 @@ gitHubContributor('dependabot', 4, 'Bot'), ]); - expect(resolve(ContributorsQuery::class)->get())->toBe([['login' => 'xHeaven', 'commits' => 187]]); + expect(resolve(ContributorsQuery::class)->get())->toBe([['login' => 'xHeaven', 'commits' => 187, 'username' => null]]); }); it('reads every page GitHub has, up to a thousand people', function (int $contributors, int $expected, int $requests): void { @@ -49,10 +49,15 @@ Http::assertSentCount(2); }); -it('skips what GitHub sends without a login or a count', function (): void { - fakeGitHubContributors([['login' => 'ghost', 'type' => 'User'], ['contributions' => 3, 'type' => 'User'], gitHubContributor('xHeaven', 187)]); +it('skips what GitHub sends without a login, an account or a count', function (): void { + fakeGitHubContributors([ + ['login' => 'ghost', 'id' => 1, 'type' => 'User'], + ['id' => 2, 'contributions' => 3, 'type' => 'User'], + ['login' => 'nobody', 'contributions' => 3, 'type' => 'User'], + gitHubContributor('xHeaven', 187), + ]); - expect(resolve(ContributorsQuery::class)->get())->toBe([['login' => 'xHeaven', 'commits' => 187]]); + expect(resolve(ContributorsQuery::class)->get())->toBe([['login' => 'xHeaven', 'commits' => 187, 'username' => null]]); }); it('asks GitHub again an hour later, not before', function (): void { @@ -74,7 +79,7 @@ fakeGitHubContributors([gitHubContributor('claude', 2)]); expect(resolve(ContributorsQuery::class)->get())->toBe([]) - ->and(Cache::get('about:contributors:'.config()->string('services.github.repository')))->toBe([]); + ->and(Cache::get('about:contributor-accounts:'.config()->string('services.github.repository')))->toBe([]); $this->travel(2)->minutes(); resolve(ContributorsQuery::class)->get(); @@ -123,6 +128,6 @@ $this->travel(61)->seconds(); - expect(resolve(ContributorsQuery::class)->get())->toBe([['login' => 'recovered', 'commits' => 12]]); + expect(resolve(ContributorsQuery::class)->get())->toBe([['login' => 'recovered', 'commits' => 12, 'username' => null]]); Http::assertSentCount(3); });