From 9d7aa883b871f3eddf55845103d4c0a67b89264a Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 31 Oct 2026 21:42:46 +0100 Subject: [PATCH 1/4] feat: follow the organizations users follow on GitHub, now or once they show up --- KNOWLEDGE_BASE.md | 4 +- app/Actions/CreateExport.php | 13 +++ app/Actions/CreateUserGitHubFollows.php | 51 ++++++++---- app/Actions/UpdateOrCreateOrganization.php | 19 ++++- .../Commands/ImportGitHubFollowsCommand.php | 31 ++++++- app/Contracts/Services/GitHubProvider.php | 5 +- app/Http/Controllers/OnboardingController.php | 7 +- app/Models/User.php | 3 + .../GitHubProviders/HttpGitHubProvider.php | 30 ++++--- .../GitHubProviders/NullGitHubProvider.php | 2 +- database/factories/UserFactory.php | 1 + ...ate_pending_organization_follows_table.php | 27 ++++++ ...ion_follows_imported_at_to_users_table.php | 21 +++++ resources/js/pages/onboarding/show.tsx | 83 ++++++++++++++----- .../Commands/ImportGitHubFollowsTest.php | 52 +++++++++++- .../Controllers/ExportControllerTest.php | 15 ++++ .../Controllers/OnboardingControllerTest.php | 6 ++ .../Actions/CreateUserGitHubFollowsTest.php | 63 +++++++++++--- tests/Unit/Actions/DeleteUserTest.php | 13 +++ .../UpdateOrCreateOrganizationTest.php | 15 ++++ .../Unit/Jobs/ImportGitHubFollowsJobTest.php | 2 +- .../HttpGitHubProviderTest.php | 16 +++- .../NullGitHubProviderTest.php | 2 +- 23 files changed, 404 insertions(+), 77 deletions(-) create mode 100644 database/migrations/2026_11_01_100000_create_pending_organization_follows_table.php create mode 100644 database/migrations/2026_11_01_100001_add_github_organization_follows_imported_at_to_users_table.php diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index 60159b584..fd5aa791c 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -94,7 +94,7 @@ Livestream history: the team agreed on GitHub-only sign-in and importing profile ### First-time setup -The first GitHub sign-in opens a short onboarding flow: review your mirrored profile (after editing it on GitHub, **Sync from GitHub** pulls the changes in without leaving setup), choose a color, see who came over from your GitHub network and follow suggested developers, and learn about the composer, keyboard shortcuts, and terminal commits. Skip setup at any step or finish it to continue to the app. The follow step shows what the one-time GitHub import brought over: the people you follow on GitHub who are already on /dev/null, whom you now follow, and your GitHub followers who are here, who now follow you. Completed or skipped setup does not appear again. New users also start with Tom as a mutual follow; Tom is the platform's fictional welcome bot. Tag @tom in a public commit to ask about the platform or find posts on a topic. +The first GitHub sign-in opens a short onboarding flow: review your mirrored profile (after editing it on GitHub, **Sync from GitHub** pulls the changes in without leaving setup), choose a color, see who came over from your GitHub network and follow suggested developers, and learn about the composer, keyboard shortcuts, and terminal commits. Skip setup at any step or finish it to continue to the app. The follow step shows what the one-time GitHub import brought over: the people and organizations you follow on GitHub that are already on /dev/null, which you now follow, and your GitHub followers who are here, who now follow you. Completed or skipped setup does not appear again. New users also start with Tom as a mutual follow; Tom is the platform's fictional welcome bot. Tag @tom in a public commit to ask about the platform or find posts on a topic. Sources: [onboarding page](resources/js/pages/onboarding/show.tsx), [sign-in tests](tests/Feature/Controllers/SessionControllerTest.php). @@ -166,7 +166,7 @@ Sources: [badge definitions](app/Enums/Achievement.php), [unlock conditions and ### Following developers and suggestions -Follow or unfollow from a developer's profile or a follow control elsewhere in the app. If someone already follows you and you do not follow them yet, their profile, hover card, and connections lists offer **Follow back**. Following puts their commits in your Following log. Following or unfollowing from a log updates the controls, counts, and live-following sidebar without navigating away: Latest and Trending keep their loaded commits and ranking, while Following updates its commits and empty-state suggestions. You cannot follow yourself or follow across a block. GitHub follows are imported once: connections to developers already on /dev/null are added in both directions, with up to 1,000 GitHub connections checked each way. This is not continuous synchronization, so unfollowing someone on /dev/null sticks. +Follow or unfollow from a developer's profile or a follow control elsewhere in the app. If someone already follows you and you do not follow them yet, their profile, hover card, and connections lists offer **Follow back**. Following puts their commits in your Following log. Following or unfollowing from a log updates the controls, counts, and live-following sidebar without navigating away: Latest and Trending keep their loaded commits and ranking, while Following updates its commits and empty-state suggestions. You cannot follow yourself or follow across a block. GitHub follows are imported once: connections to developers already on /dev/null are added in both directions, with up to 1,000 GitHub connections checked each way. The organizations you follow on GitHub come along too: you follow the ones already on /dev/null, and each of the others as soon as it shows up here. This is not continuous synchronization, so unfollowing someone on /dev/null sticks, and following someone on GitHub later does not carry over. **Who to follow** suggests developers in the side column or within the log on smaller screens, each with the first line of their bio and why they are suggested (how many people you follow follow them, or their follower count). Follow from the icon next to each suggestion; for someone who already follows you, it follows back. The X icon marks a suggestion **Not interested**: that developer is never suggested to you again, without muting them, and the next suggestion takes their place. Refresh for different suggestions, or use **… → Don't show again** to hide the block, with an immediate **Undo** option. Show or hide these suggestions in Settings → Appearance. Suggested developers must have a visible top-level commit. On profiles, suggestions prioritize the people that developer follows; on commit pages, they prioritize visible thread participants. Regular suggestions fill any remaining places. You can also follow back from the notifications list. diff --git a/app/Actions/CreateExport.php b/app/Actions/CreateExport.php index 9e84494b4..b88f3bfcf 100644 --- a/app/Actions/CreateExport.php +++ b/app/Actions/CreateExport.php @@ -49,6 +49,7 @@ * dismissed_suggestions: Generator, * organizations: array, * followed_organizations: array, + * pending_organization_follows: list, * messages: Generator, * reports: Traversable>, * remote_pins: array, @@ -119,6 +120,7 @@ public function handle(User $user): array 'dismissed_suggestions' => $this->handles($user->dismissedSuggestions()), 'organizations' => $this->organizations($user), 'followed_organizations' => $this->followedOrganizations($user), + 'pending_organization_follows' => $this->pendingOrganizationFollows($user), 'messages' => $this->messages($user), 'reports' => CommitReport::query()->where('reporter_id', $user->id)->oldest('id')->cursor()->map(fn (CommitReport $report): array => [ 'commit_id' => $report->commit_id, @@ -359,6 +361,17 @@ private function followedOrganizations(User $user): array ])->all(); } + /** + * The GitHub IDs of the organizations the user follows on GitHub, to follow here once they are on /dev/null. + * + * @return list + */ + private function pendingOrganizationFollows(User $user): array + { + /** @var list */ + return DB::table('pending_organization_follows')->where('user_id', $user->id)->orderBy('github_id')->pluck('github_id')->all(); + } + /** * Moderators stay anonymous, as they are on the notifications page, and level-ups are the user's own doing. * diff --git a/app/Actions/CreateUserGitHubFollows.php b/app/Actions/CreateUserGitHubFollows.php index 471f79e05..b63f77cff 100644 --- a/app/Actions/CreateUserGitHubFollows.php +++ b/app/Actions/CreateUserGitHubFollows.php @@ -5,20 +5,24 @@ namespace App\Actions; use App\Contracts\Services\GitHubProvider; +use App\Enums\ProfileType; +use App\Models\Profile; use App\Models\User; use Illuminate\Database\Eloquent\Collection; use Illuminate\Support\Facades\DB; /** - * Brings a user's GitHub network over, once: they follow the people they follow on GitHub, and the people - * following them on GitHub follow them here. It only ever adds follows, and never runs twice, so an unfollow sticks. + * Brings a user's GitHub network over, once: they follow the people and the organizations they follow on GitHub, and + * the people following them on GitHub follow them here. The organizations not on /dev/null yet are kept, so the user + * follows them once they are. It only ever adds follows, and never runs twice, so an unfollow sticks. Organizations are + * marked as imported apart, so the users imported before organizations could be followed get them on another run. */ final readonly class CreateUserGitHubFollows { /** * Up to 1,000 accounts each way, so a popular account can't use up the rate limit on its own. */ - private const int MaxPages = 10; + public const int MaxPages = 10; public function __construct( private GitHubProvider $github, @@ -29,22 +33,20 @@ public function __construct( public function handle(User $user, bool $notify = true): void { - if ($user->github_follows_imported_at !== null) { - return; - } - - $login = $this->github->login($user->github_id); - - if ($login === null) { - $user->update(['github_follows_imported_at' => now()]); + $importsPeople = $user->github_follows_imported_at === null; + $importsOrganizations = $user->github_organization_follows_imported_at === null; + if (! $importsPeople && ! $importsOrganizations) { return; } - $following = $this->users($user, $this->github->followingIds($login, self::MaxPages)); - $followers = $this->users($user, $this->github->followerIds($login, self::MaxPages)); + $login = $this->github->login($user->github_id); + $followingIds = $login === null ? ['users' => [], 'organizations' => []] : $this->github->followingIds($login, self::MaxPages); + $following = $importsPeople ? $this->users($user, $followingIds['users']) : new Collection(); + $followers = $importsPeople && $login !== null ? $this->users($user, $this->github->followerIds($login, self::MaxPages)) : new Collection(); + $organizationIds = collect($followingIds['organizations'])->unique()->sort()->values(); - DB::transaction(function () use ($user, $following, $followers, $notify): void { + DB::transaction(function () use ($user, $notify, $following, $followers, $organizationIds, $importsPeople, $importsOrganizations): void { $followedIds = $user->following()->syncWithoutDetaching($following->modelKeys())['attached']; $followerIds = $user->followers()->syncWithoutDetaching($followers->modelKeys())['attached']; @@ -58,7 +60,26 @@ public function handle(User $user, bool $notify = true): void } } - $user->update(['github_follows_imported_at' => now()]); + if ($importsOrganizations) { + // Waits for an organization being created from GitHub right now, which follows the pending rows it finds + // (UpdateOrCreateOrganization), so the follow is made on one side or the other. Locked in order, so two + // imports never wait on each other. + DB::select('select pg_advisory_xact_lock(hashtextextended(id, 0)) from unnest(?::text[]) as id', ['{'.$organizationIds->map(fn (string $id): string => "github-organization:{$id}")->implode(',').'}']); + + $organizations = Profile::query()->where('type', ProfileType::Organization)->whereIn('github_id', $organizationIds)->get(); + + $user->followedOrganizations()->syncWithoutDetaching($organizations->modelKeys()); + + DB::table('pending_organization_follows')->insertOrIgnore($organizationIds + ->reject(fn (string $id): bool => $organizations->contains('github_id', $id)) + ->map(fn (string $id): array => ['user_id' => $user->id, 'github_id' => $id, 'created_at' => now()]) + ->all()); + } + + $user->update(array_filter([ + 'github_follows_imported_at' => $importsPeople ? now() : null, + 'github_organization_follows_imported_at' => $importsOrganizations ? now() : null, + ])); }); } diff --git a/app/Actions/UpdateOrCreateOrganization.php b/app/Actions/UpdateOrCreateOrganization.php index 6501458bc..c131a9185 100644 --- a/app/Actions/UpdateOrCreateOrganization.php +++ b/app/Actions/UpdateOrCreateOrganization.php @@ -20,7 +20,8 @@ public function __construct(private UpdateStaleHandles $updateStaleHandles) /** * Mirrors a GitHub organization on its profile, from an account of the GitHub REST API, such as - * `GET /users/{username}`. Another profile holding its handle renamed away on GitHub. + * `GET /users/{username}`. Another profile holding its handle renamed away on GitHub. A new organization is + * followed by the users following it on GitHub whose follows were imported before it showed up. * * @param array $account */ @@ -31,16 +32,28 @@ public function handle(array $account): Organization assert(is_int($githubId)); return DB::transaction(function () use ($profile, $githubId): Organization { + // Waits for the GitHub follow imports keeping this organization pending right now (CreateUserGitHubFollows). + DB::select('select pg_advisory_xact_lock(hashtextextended(?, 0))', ["github-organization:{$githubId}"]); + $this->updateStaleHandles->handle($profile->handle, (string) $githubId); // A profile never changes type: a user's account GitHub has since turned into an organization fails on its // GitHub ID, until that case is handled... - $organization = Profile::query()->updateOrCreate(['github_id' => (string) $githubId, 'type' => ProfileType::Organization], [ + $organizationProfile = Profile::query()->updateOrCreate(['github_id' => (string) $githubId, 'type' => ProfileType::Organization], [ ...$profile->attributes(), 'github_synced_at' => now(), ]); - return Organization::query()->firstOrCreate(['id' => $organization->id]); + $organization = Organization::query()->firstOrCreate(['id' => $organizationProfile->id]); + + if ($organization->wasRecentlyCreated) { + $pending = DB::table('pending_organization_follows')->where('github_id', (string) $githubId); + + $organization->followers()->attach($pending->pluck('user_id')->all()); + $pending->delete(); + } + + return $organization; }); } } diff --git a/app/Console/Commands/ImportGitHubFollowsCommand.php b/app/Console/Commands/ImportGitHubFollowsCommand.php index c39dfbb75..b310de786 100644 --- a/app/Console/Commands/ImportGitHubFollowsCommand.php +++ b/app/Console/Commands/ImportGitHubFollowsCommand.php @@ -5,23 +5,40 @@ namespace App\Console\Commands; use App\Actions\CreateUserGitHubFollows; +use App\Contracts\Services\GitHubProvider; use App\Models\User; use Illuminate\Console\Attributes\Description; use Illuminate\Console\Attributes\Signature; use Illuminate\Console\Command; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\Client\ConnectionException; use Illuminate\Http\Client\RequestException; #[Signature('follows:import')] -#[Description('Follow, for every user not imported yet, who they follow and who follows them on GitHub, without notifying anyone')] +#[Description('Follow, for every user not imported yet, who and which organizations they follow and who follows them on GitHub, without notifying anyone')] final class ImportGitHubFollowsCommand extends Command { - public function handle(CreateUserGitHubFollows $createUserGitHubFollows): int + /** + * Below this many requests left, the command stops, leaving the rest to the app until the rate limit resets. + */ + private const int RateLimitFloor = 1000; + + public function handle(CreateUserGitHubFollows $createUserGitHubFollows, GitHubProvider $github): int { $imported = 0; $failed = 0; - foreach (User::query()->whereNull('github_follows_imported_at')->lazyById() as $user) { + foreach ($this->users()->lazyById() as $user) { + $rateLimit = $github->rateLimit(); + + // A user takes one request for their login, and up to the maximum of pages each way. + if ($rateLimit !== null && $rateLimit['remaining'] < self::RateLimitFloor + 1 + 2 * CreateUserGitHubFollows::MaxPages) { + $this->components->warn("Stopped after {$imported} users, to keep ".self::RateLimitFloor." GitHub requests for the rest of the app: {$rateLimit['remaining']} left."); + $this->components->info("{$this->users()->count()} users left. Run the command again after {$rateLimit['resets_at']->utc()->toDateTimeString()} UTC ({$rateLimit['resets_at']->diffForHumans()}), when the GitHub rate limit resets."); + + return self::FAILURE; + } + try { $createUserGitHubFollows->handle($user, notify: false); @@ -48,6 +65,14 @@ public function handle(CreateUserGitHubFollows $createUserGitHubFollows): int return $failed === 0 ? self::SUCCESS : self::FAILURE; } + /** + * @return Builder + */ + private function users(): Builder + { + return User::query()->where(fn (Builder $query): Builder => $query->whereNull('github_follows_imported_at')->orWhereNull('github_organization_follows_imported_at')); + } + private function failed(User $user, RequestException|ConnectionException $exception): void { report($exception); diff --git a/app/Contracts/Services/GitHubProvider.php b/app/Contracts/Services/GitHubProvider.php index 70e18b9b3..c750551c5 100644 --- a/app/Contracts/Services/GitHubProvider.php +++ b/app/Contracts/Services/GitHubProvider.php @@ -56,9 +56,10 @@ public function isPublicMember(string $organizationGithubId, string $githubId): public function hasSponsorsListing(string $token): ?bool; /** - * The IDs of the accounts the user follows, up to the given number of pages. + * The IDs of the accounts the user follows, up to the given number of pages, the people apart from the + * organizations. * - * @return list + * @return array{users: list, organizations: list} */ public function followingIds(string $login, int $maxPages): array; diff --git a/app/Http/Controllers/OnboardingController.php b/app/Http/Controllers/OnboardingController.php index a9880ea1f..ec140f11c 100644 --- a/app/Http/Controllers/OnboardingController.php +++ b/app/Http/Controllers/OnboardingController.php @@ -6,6 +6,7 @@ use App\Actions\UpdateUserOnboardedAt; use App\Http\Requests\ShowOnboardingRequest; +use App\Models\Organization; use App\Models\User; use App\Queries\SuggestedUsersQuery; use Illuminate\Container\Attributes\CurrentUser; @@ -42,7 +43,7 @@ public function update(#[CurrentUser] User $user, UpdateUserOnboardedAt $action) * What the GitHub import brought over: null until the job has run. The follows it made share its timestamp, give * or take the time the job took. Tom's friendship is made at sign-up too, right before, so bots are left out. * - * @return array{following: Collection, followingCount: int, followers: Collection, followersCount: int, pendingCount: int}|null + * @return array{following: Collection, followingCount: int, organizations: Collection, organizationsCount: int, followers: Collection, followersCount: int, pendingCount: int}|null */ private function githubImport(ShowOnboardingRequest $request, User $user): ?array { @@ -55,10 +56,14 @@ private function githubImport(ShowOnboardingRequest $request, User $user): ?arra $imported = fn (BelongsToMany $follows): BelongsToMany => $follows ->wherePivotBetween('created_at', [$importedAt->copy()->subSeconds(10), $importedAt]) ->whereNotIn('users.github_id', User::BOT_GITHUB_IDS); + $organizations = fn (): BelongsToMany => $user->followedOrganizations() + ->wherePivotBetween('created_at', [$importedAt->copy()->subSeconds(10), $importedAt]); return [ 'following' => $imported($user->following())->oldest('users.id')->limit(5)->get(), 'followingCount' => $imported($user->following())->count(), + 'organizations' => $organizations()->oldest('organizations.id')->limit(5)->get(), + 'organizationsCount' => $organizations()->count(), 'followers' => $imported($user->followers())->oldest('users.id')->limit(5)->get(), 'followersCount' => $imported($user->followers())->count(), // SKETCH: pending follows aren't stored yet (see the pending_follows table of feat/unclaimed-profiles). diff --git a/app/Models/User.php b/app/Models/User.php index fe566cd61..7fa15b06c 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -60,6 +60,7 @@ * @property-read Log $log * @property-read CarbonInterface|null $onboarded_at * @property-read CarbonInterface|null $github_follows_imported_at + * @property-read CarbonInterface|null $github_organization_follows_imported_at * @property-read string|null $remember_token * @property-read CarbonInterface|null $github_synced_at * @property-read CarbonInterface|null $last_seen_at @@ -110,6 +111,7 @@ 'log', 'onboarded_at', 'github_follows_imported_at', + 'github_organization_follows_imported_at', 'live_platform', 'live_url', 'live_ends_at', @@ -205,6 +207,7 @@ public function casts(): array 'log' => Log::class, 'onboarded_at' => 'datetime', 'github_follows_imported_at' => 'datetime', + 'github_organization_follows_imported_at' => 'datetime', 'remember_token' => 'string', 'live_platform' => LivePlatform::class, 'live_url' => 'string', diff --git a/app/Services/GitHubProviders/HttpGitHubProvider.php b/app/Services/GitHubProviders/HttpGitHubProvider.php index afa433c25..8c9a9bd68 100644 --- a/app/Services/GitHubProviders/HttpGitHubProvider.php +++ b/app/Services/GitHubProviders/HttpGitHubProvider.php @@ -101,12 +101,18 @@ public function hasSponsorsListing(string $token): ?bool public function followingIds(string $login, int $maxPages): array { - return $this->accountIds("users/{$login}/following", $maxPages); + $accounts = $this->accounts("users/{$login}/following", $maxPages); + $ids = fn (bool $organizations): array => array_values(array_map( + fn (array $account): string => (string) $account['id'], + array_filter($accounts, fn (array $account): bool => ($account['type'] === 'Organization') === $organizations), + )); + + return ['users' => $ids(false), 'organizations' => $ids(true)]; } public function followerIds(string $login, int $maxPages): array { - return $this->accountIds("users/{$login}/followers", $maxPages); + return array_map(fn (array $account): string => (string) $account['id'], $this->accounts("users/{$login}/followers", $maxPages)); } public function sshKeys(string $login): array @@ -274,28 +280,26 @@ private function find(string $path): ?array } /** - * The IDs of the accounts in a paginated list, up to the given number of pages. + * The accounts in a paginated list, up to the given number of pages. * - * @return list + * @return list */ - private function accountIds(string $path, int $maxPages): array + private function accounts(string $path, int $maxPages): array { - $ids = []; + $accounts = []; for ($page = 1; $page <= $maxPages; $page++) { - /** @var list $accounts */ - $accounts = $this->request()->get($path, ['per_page' => self::PerPage, 'page' => $page])->throw()->json(); + /** @var list $pageAccounts */ + $pageAccounts = $this->request()->get($path, ['per_page' => self::PerPage, 'page' => $page])->throw()->json(); - foreach ($accounts as $account) { - $ids[] = (string) $account['id']; - } + array_push($accounts, ...$pageAccounts); - if (count($accounts) < self::PerPage) { + if (count($pageAccounts) < self::PerPage) { break; } } - return $ids; + return $accounts; } /** diff --git a/app/Services/GitHubProviders/NullGitHubProvider.php b/app/Services/GitHubProviders/NullGitHubProvider.php index 8158ef2bb..523d75573 100644 --- a/app/Services/GitHubProviders/NullGitHubProvider.php +++ b/app/Services/GitHubProviders/NullGitHubProvider.php @@ -43,7 +43,7 @@ public function hasSponsorsListing(string $token): ?bool public function followingIds(string $login, int $maxPages): array { - return []; + return ['users' => [], 'organizations' => []]; } public function followerIds(string $login, int $maxPages): array diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index eb4fe351e..698b1db43 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -42,6 +42,7 @@ public function definition(): array 'pinned_commit_id' => null, 'onboarded_at' => now(), 'github_follows_imported_at' => null, + 'github_organization_follows_imported_at' => null, 'commit_rules_accepted_at' => now(), 'remember_token' => Str::random(10), ]; diff --git a/database/migrations/2026_11_01_100000_create_pending_organization_follows_table.php b/database/migrations/2026_11_01_100000_create_pending_organization_follows_table.php new file mode 100644 index 000000000..f0d05f12e --- /dev/null +++ b/database/migrations/2026_11_01_100000_create_pending_organization_follows_table.php @@ -0,0 +1,27 @@ +foreignUuid('user_id')->constrained()->cascadeOnDelete(); + $table->string('github_id'); + $table->timestamp('created_at'); + + $table->primary(['user_id', 'github_id']); + $table->index('github_id'); + }); + } +}; diff --git a/database/migrations/2026_11_01_100001_add_github_organization_follows_imported_at_to_users_table.php b/database/migrations/2026_11_01_100001_add_github_organization_follows_imported_at_to_users_table.php new file mode 100644 index 000000000..e2a36408a --- /dev/null +++ b/database/migrations/2026_11_01_100001_add_github_organization_follows_imported_at_to_users_table.php @@ -0,0 +1,21 @@ +timestamp('github_organization_follows_imported_at')->nullable(); + }); + } +}; diff --git a/resources/js/pages/onboarding/show.tsx b/resources/js/pages/onboarding/show.tsx index e2e8faea2..ea1a8b11c 100644 --- a/resources/js/pages/onboarding/show.tsx +++ b/resources/js/pages/onboarding/show.tsx @@ -19,6 +19,7 @@ import { GitHubProfile } from '@/components/github-profile'; import InputError from '@/components/input-error'; import { KeyboardShortcutsDialog } from '@/components/keyboard-shortcuts-dialog'; import { LaneOptions, LanePreview } from '@/components/lane-picker'; +import { OrganizationAvatar } from '@/components/organization-avatar'; import { Button } from '@/components/ui/button'; import { Spinner } from '@/components/ui/spinner'; import { UserAvatar } from '@/components/user-avatar'; @@ -29,13 +30,15 @@ import { useUser } from '@/hooks/use-user'; import { useKeyLabels } from '@/lib/keyboard'; import { formatCount } from '@/lib/numbers'; import { cn, laneColor } from '@/lib/utils'; -import type { Lane, SuggestedUser, User } from '@/types'; +import type { Lane, Organization, SuggestedUser, User } from '@/types'; const laneNames = ['orange', 'green', 'violet', 'teal', 'rose', 'ochre']; type GitHubImport = { following: User[]; followingCount: number; + organizations: Organization[]; + organizationsCount: number; followers: User[]; followersCount: number; pendingCount: number; @@ -49,6 +52,12 @@ function developers(count: number): string { return count === 1 ? '1 developer' : `${formatCount(count)} developers`; } +function orgs(count: number): string { + return count === 1 + ? '1 organization' + : `${formatCount(count)} organizations`; +} + const countUpDuration = 900; function prefersReducedMotion(): boolean { @@ -93,20 +102,28 @@ function useCountUp(target: number, delay: number): number { */ function ImportStat({ users, + organizations = [], count, label, delay, children, }: { users: User[]; + organizations?: Organization[]; count: number; label: string; delay: number; children: ReactNode; }) { const value = useCountUp(count, delay); - const shown = users.slice(0, 5); - const stagger = countUpDuration / Math.max(shown.length, 1); + // Organizations get at least two of the five places, so they show next to the people. + const shownOrganizations = organizations.slice( + 0, + Math.max(2, 5 - users.length), + ); + const shownUsers = users.slice(0, 5 - shownOrganizations.length); + const shownCount = shownUsers.length + shownOrganizations.length; + const stagger = countUpDuration / Math.max(shownCount, 1); return (
@@ -120,7 +137,7 @@ function ImportStat({ {label}

- {shown.map((user, index) => ( + {shownUsers.map((user, index) => ( ))} - {count > shown.length && ( + {shownOrganizations.map((organization, index) => ( + + + + ))} + {count > shownCount && ( - +{formatCount(count - shown.length)} + +{formatCount(count - shownCount)} )}
@@ -243,14 +275,15 @@ function GitHubImportCard({ const { following, followingCount, followers, followersCount } = githubImport; - const { pendingCount } = githubImport; + const { organizations, organizationsCount, pendingCount } = githubImport; + const allFollowingCount = followingCount + organizationsCount; - if (followingCount === 0 && followersCount === 0 && pendingCount === 0) { + if (allFollowingCount === 0 && followersCount === 0 && pendingCount === 0) { return null; } - const hasImported = followingCount > 0 || followersCount > 0; - const followersDelay = followingCount > 0 ? 450 : 150; + const hasImported = allFollowingCount > 0 || followersCount > 0; + const followersDelay = allFollowingCount > 0 ? 450 : 150; return (
@@ -264,19 +297,26 @@ function GitHubImportCard({
0 && followersCount > 0, + allFollowingCount > 0 && followersCount > 0, })} > - {followingCount > 0 && ( + {allFollowingCount > 0 && ( - {followingCount === 1 - ? 'Someone you follow on GitHub is already here. You follow them now.' - : 'People you follow on GitHub who are already here. You follow them now.'} + {organizationsCount === 0 + ? followingCount === 1 + ? 'Someone you follow on GitHub is already here. You follow them now.' + : 'People you follow on GitHub who are already here. You follow them now.' + : followingCount === 0 + ? organizationsCount === 1 + ? 'An organization you follow on GitHub is already here. You follow it now.' + : 'Organizations you follow on GitHub that are already here. You follow them now.' + : 'Developers and organizations you follow on GitHub who are already here. You follow them now.'} )} {followersCount > 0 && ( @@ -544,6 +584,11 @@ export default function Show({ const headingRef = useRef(null); const importedFollowingCount = githubImport?.followingCount ?? 0; + const importedOrganizationsCount = githubImport?.organizationsCount ?? 0; + const imported = [ + importedFollowingCount > 0 && developers(importedFollowingCount), + importedOrganizationsCount > 0 && orgs(importedOrganizationsCount), + ].filter(Boolean); const isImporting = githubImport === null; const [hasGivenUp, setHasGivenUp] = useState(false); const wasImporting = useRef(isImporting); @@ -741,12 +786,12 @@ export default function Show({ 0 - ? `You already follow ${developers(importedFollowingCount)}.` + imported.length > 0 + ? `You already follow ${imported.join(' and ')}.` : 'Follow a few developers.' } description={ - importedFollowingCount > 0 + imported.length > 0 ? "We brought your GitHub network over, so your log doesn't start empty. Follow a few more to fill it up; unfollowing is one click." : 'Your log is your commits plus the commits of everyone you follow. Here are a few to start with; unfollowing is one click.' } diff --git a/tests/Feature/Commands/ImportGitHubFollowsTest.php b/tests/Feature/Commands/ImportGitHubFollowsTest.php index 1f0a8798a..19c7f4ddb 100644 --- a/tests/Feature/Commands/ImportGitHubFollowsTest.php +++ b/tests/Feature/Commands/ImportGitHubFollowsTest.php @@ -4,7 +4,9 @@ use App\Contracts\Services\GitHubProvider; use App\Models\Notification; +use App\Models\Organization; use App\Models\User; +use Carbon\CarbonImmutable; use GuzzleHttp\Psr7\Response as Psr7Response; use Illuminate\Http\Client\ConnectionException; use Illuminate\Http\Client\RequestException; @@ -14,14 +16,15 @@ it('imports the github follows of every user not imported yet, without notifying anyone', function (): void { $alice = User::factory()->create(); $bob = User::factory()->create(); - $imported = User::factory()->create(['github_follows_imported_at' => now()->subDay()]); + $imported = User::factory()->create(['github_follows_imported_at' => now()->subDay(), 'github_organization_follows_imported_at' => now()->subDay()]); $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('rateLimit')->andReturnNull(); $github->shouldReceive('login')->with($alice->github_id)->andReturn($alice->handle); - $github->shouldReceive('followingIds')->with($alice->handle, 10)->andReturn([$bob->github_id, $imported->github_id]); + $github->shouldReceive('followingIds')->with($alice->handle, 10)->andReturn(['users' => [$bob->github_id, $imported->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($alice->handle, 10)->andReturn([]); $github->shouldReceive('login')->with($bob->github_id)->andReturn($bob->handle); - $github->shouldReceive('followingIds')->with($bob->handle, 10)->andReturn([]); + $github->shouldReceive('followingIds')->with($bob->handle, 10)->andReturn(['users' => [], 'organizations' => []]); $github->shouldReceive('followerIds')->with($bob->handle, 10)->andReturn([$alice->github_id]); $this->artisan('follows:import') @@ -43,9 +46,10 @@ $user = User::factory()->create(); $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('rateLimit')->andReturnNull(); $github->shouldReceive('login')->with($failing->github_id)->andThrow($throwable()); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([]); $this->artisan('follows:import') @@ -67,6 +71,7 @@ User::factory()->create(); $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('rateLimit')->andReturnNull(); $github->shouldReceive('login')->with($user->github_id)->once()->andThrow(new RequestException(new Response(new Psr7Response($status)))); $this->artisan('follows:import') @@ -75,3 +80,42 @@ $github->shouldHaveReceived('login')->once(); })->with([403, 429]); + +it('imports the organizations followed by the users whose people were imported before', function (): void { + $user = User::factory()->create(['github_follows_imported_at' => now()->subDay()]); + $organization = Organization::factory()->create(); + + $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('rateLimit')->andReturnNull(); + $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [], 'organizations' => [$organization->profile->github_id]]); + + $this->artisan('follows:import') + ->expectsOutputToContain('Imported the GitHub follows of 1 users.') + ->assertSuccessful(); + + expect($user->followedOrganizations()->pluck('organizations.id')->all())->toBe([$organization->id]); +}); + +it('stops before using up the github requests the rest of the app needs, and says when to run it again', function (): void { + $this->travelTo(CarbonImmutable::parse('2026-11-01 10:00:00', 'UTC')); + + $imported = User::factory()->create(); + User::factory()->count(2)->create(); + + $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('rateLimit')->andReturn( + ['remaining' => 1021, 'limit' => 5000, 'resets_at' => CarbonImmutable::parse('2026-11-01 10:42:00', 'UTC')], + ['remaining' => 1020, 'limit' => 5000, 'resets_at' => CarbonImmutable::parse('2026-11-01 10:42:00', 'UTC')], + ); + $github->shouldReceive('login')->with($imported->github_id)->once()->andReturn($imported->handle); + $github->shouldReceive('followingIds')->with($imported->handle, 10)->andReturn(['users' => [], 'organizations' => []]); + $github->shouldReceive('followerIds')->with($imported->handle, 10)->andReturn([]); + + $this->artisan('follows:import') + ->expectsOutputToContain('Stopped after 1 users, to keep 1000 GitHub requests for the rest of the app: 1020 left.') + ->expectsOutputToContain('2 users left. Run the command again after 2026-11-01 10:42:00 UTC (42 minutes from now), when the GitHub rate limit resets.') + ->assertFailed(); + + $github->shouldHaveReceived('login')->once(); +}); diff --git a/tests/Feature/Controllers/ExportControllerTest.php b/tests/Feature/Controllers/ExportControllerTest.php index 1fcf74637..d9b12098d 100644 --- a/tests/Feature/Controllers/ExportControllerTest.php +++ b/tests/Feature/Controllers/ExportControllerTest.php @@ -138,6 +138,7 @@ 'dismissed_suggestions' => [], 'organizations' => [], 'followed_organizations' => [], + 'pending_organization_follows' => [], 'messages' => [], 'reports' => [], 'remote_mutes' => [], @@ -509,6 +510,20 @@ ]); }); +it('exports the github organizations the user waits to follow', function (): void { + $user = User::factory()->create(); + DB::table('pending_organization_follows')->insert([ + ['user_id' => $user->id, 'github_id' => '958072', 'created_at' => now()], + ['user_id' => $user->id, 'github_id' => '1', 'created_at' => now()], + ['user_id' => User::factory()->create()->id, 'github_id' => '2', 'created_at' => now()], + ]); + + $response = $this->actingAs($user)->get(route('export')); + $data = json_decode($response->streamedContent(), true, flags: JSON_THROW_ON_ERROR); + + expect($data['pending_organization_follows'])->toBe(['1', '958072']); +}); + it('exports the devices approved for the app, oldest first', function (): void { $user = User::factory()->create(); $older = Device::factory()->for($user)->create(['name' => 'devnull on iPad', 'approved_at' => now()->subWeek(), 'last_used_at' => now()->subDay()]); diff --git a/tests/Feature/Controllers/OnboardingControllerTest.php b/tests/Feature/Controllers/OnboardingControllerTest.php index 2124ae63e..2a3a46f26 100644 --- a/tests/Feature/Controllers/OnboardingControllerTest.php +++ b/tests/Feature/Controllers/OnboardingControllerTest.php @@ -6,6 +6,7 @@ use App\Enums\Log; use App\Http\Middleware\HandleInertiaRequests; use App\Models\Commit; +use App\Models\Organization; use App\Models\User; use Illuminate\Support\Facades\DB; @@ -93,10 +94,13 @@ $followers = User::factory()->count(2)->create(); $user->following()->attach($followed->modelKeys(), ['created_at' => now()->subSeconds(3), 'updated_at' => now()]); $user->followers()->attach($followers->modelKeys(), ['created_at' => now()->subSeconds(3), 'updated_at' => now()]); + $organizations = Organization::factory()->count(2)->create(); + $user->followedOrganizations()->attach($organizations->modelKeys(), ['created_at' => now()->subSeconds(3)]); // Followed after the import, from the onboarding itself... $this->travel(1)->minute(); $user->following()->attach(User::factory()->create(), ['created_at' => now(), 'updated_at' => now()]); + $user->followedOrganizations()->attach(Organization::factory()->create(), ['created_at' => now()]); $response = $this->actingAs($user) ->get(route('onboarding.show')); @@ -106,6 +110,8 @@ ->has('githubImport.following', 5) ->where('githubImport.following', fn ($users): bool => collect($users)->pluck('id')->diff($followed->modelKeys())->isEmpty()) ->where('githubImport.followingCount', 9) + ->where('githubImport.organizations', fn ($imported): bool => collect($imported)->pluck('handle')->sort()->values()->all() === $organizations->pluck('handle')->sort()->values()->all()) + ->where('githubImport.organizationsCount', 2) ->where('githubImport.followers', fn ($users): bool => collect($users)->pluck('id')->sort()->values()->all() === collect($followers->modelKeys())->sort()->values()->all()) ->where('githubImport.followersCount', 2) ->where('githubImport.pendingCount', 0)); diff --git a/tests/Unit/Actions/CreateUserGitHubFollowsTest.php b/tests/Unit/Actions/CreateUserGitHubFollowsTest.php index 4580f87a1..e39ad7835 100644 --- a/tests/Unit/Actions/CreateUserGitHubFollowsTest.php +++ b/tests/Unit/Actions/CreateUserGitHubFollowsTest.php @@ -6,10 +6,12 @@ use App\Contracts\Services\GitHubProvider; use App\Enums\NotificationType; use App\Models\Notification; +use App\Models\Organization; use App\Models\User; use GuzzleHttp\Psr7\Response as Psr7Response; use Illuminate\Http\Client\RequestException; use Illuminate\Http\Client\Response; +use Illuminate\Support\Facades\DB; it('follows the users they follow on github', function (): void { $user = User::factory()->create(); @@ -18,7 +20,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$followed->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$followed->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([]); resolve(CreateUserGitHubFollows::class)->handle($user); @@ -34,7 +36,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([$follower->github_id]); resolve(CreateUserGitHubFollows::class)->handle($user); @@ -49,7 +51,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$stranger->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$stranger->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([$stranger->github_id]); resolve(CreateUserGitHubFollows::class)->handle($user); @@ -65,7 +67,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$followed->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$followed->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([$follower->github_id]); resolve(CreateUserGitHubFollows::class)->handle($user); @@ -85,7 +87,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$followed->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$followed->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([$follower->github_id]); resolve(CreateUserGitHubFollows::class)->handle($user, notify: false); @@ -104,7 +106,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$alreadyFollowed->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$alreadyFollowed->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([]); resolve(CreateUserGitHubFollows::class)->handle($user); @@ -115,7 +117,7 @@ }); it('imports the follows of a user only once', function (): void { - $user = User::factory()->create(['github_follows_imported_at' => now()]); + $user = User::factory()->create(['github_follows_imported_at' => now(), 'github_organization_follows_imported_at' => now()]); $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->never(); @@ -128,7 +130,9 @@ resolve(CreateUserGitHubFollows::class)->handle($user); - expect($user->refresh()->github_follows_imported_at)->not->toBeNull(); + expect($user->refresh()) + ->github_follows_imported_at->not->toBeNull() + ->github_organization_follows_imported_at->not->toBeNull(); }); it('leaves the follows to import again when github fails', function (): void { @@ -139,7 +143,46 @@ $github->shouldReceive('followingIds')->andThrow(new RequestException(new Response(new Psr7Response(500)))); expect(fn () => resolve(CreateUserGitHubFollows::class)->handle($user))->toThrow(RequestException::class) - ->and($user->refresh()->github_follows_imported_at)->toBeNull(); + ->and($user->refresh()->github_follows_imported_at)->toBeNull() + ->and($user->github_organization_follows_imported_at)->toBeNull(); +}); + +it('follows the organizations they follow on github, and keeps the ones not on devnull for when they are', function (): void { + $user = User::factory()->create(); + $organization = Organization::factory()->create(); + Organization::factory()->create(); + + $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [], 'organizations' => [$organization->profile->github_id, '958072']]); + $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([]); + + resolve(CreateUserGitHubFollows::class)->handle($user); + + expect($user->followedOrganizations()->pluck('organizations.id')->all())->toBe([$organization->id]) + ->and(DB::table('pending_organization_follows')->get(['user_id', 'github_id'])->map(fn (object $row): array => (array) $row)->all())->toBe([['user_id' => $user->id, 'github_id' => '958072']]) + ->and($user->refresh()->github_organization_follows_imported_at)->not->toBeNull() + ->and(Notification::query()->count())->toBe(0); +}); + +it('imports only the organizations of a user whose people were imported before, so unfollows stick', function (): void { + $importedAt = now()->subWeek()->startOfSecond(); + $user = User::factory()->create(['github_follows_imported_at' => $importedAt]); + $unfollowed = User::factory()->create(); + $organization = Organization::factory()->create(); + + $github = $this->mock(GitHubProvider::class); + $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$unfollowed->github_id], 'organizations' => [$organization->profile->github_id]]); + + resolve(CreateUserGitHubFollows::class)->handle($user); + + expect($user->isFollowing($unfollowed))->toBeFalse() + ->and($user->followedOrganizations()->pluck('organizations.id')->all())->toBe([$organization->id]) + ->and($user->refresh()->github_follows_imported_at->toDateTimeString())->toBe($importedAt->toDateTimeString()) + ->and($user->github_organization_follows_imported_at)->not->toBeNull(); + + $github->shouldNotHaveReceived('followerIds'); }); it('keeps github imports from following across a block in either direction', function (): void { @@ -153,7 +196,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$blocked->github_id, $blocker->github_id, $allowed->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$blocked->github_id, $blocker->github_id, $allowed->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([$blocked->github_id, $blocker->github_id, $allowed->github_id]); resolve(CreateUserGitHubFollows::class)->handle($user); diff --git a/tests/Unit/Actions/DeleteUserTest.php b/tests/Unit/Actions/DeleteUserTest.php index 89e6d8b45..99bed9e33 100644 --- a/tests/Unit/Actions/DeleteUserTest.php +++ b/tests/Unit/Actions/DeleteUserTest.php @@ -230,6 +230,19 @@ expect($organization->followers()->pluck('users.id')->all())->toBe([$other->id]); }); +it('erases the github organizations a deleted user waited to follow', function (): void { + $user = User::factory()->create(); + $other = User::factory()->create(); + DB::table('pending_organization_follows')->insert([ + ['user_id' => $user->id, 'github_id' => '958072', 'created_at' => now()], + ['user_id' => $other->id, 'github_id' => '958072', 'created_at' => now()], + ]); + + resolve(DeleteUser::class)->handle($user); + + expect(DB::table('pending_organization_follows')->pluck('user_id')->all())->toBe([$other->id]); +}); + it('erases the devices of a deleted user', function (): void { $user = User::factory()->create(); Device::factory()->for($user)->create(); diff --git a/tests/Unit/Actions/UpdateOrCreateOrganizationTest.php b/tests/Unit/Actions/UpdateOrCreateOrganizationTest.php index fda3169b0..eab36df7c 100644 --- a/tests/Unit/Actions/UpdateOrCreateOrganizationTest.php +++ b/tests/Unit/Actions/UpdateOrCreateOrganizationTest.php @@ -6,7 +6,9 @@ use App\Enums\ProfileType; use App\Models\Organization; use App\Models\Profile; +use App\Models\User; use Illuminate\Database\UniqueConstraintViolationException; +use Illuminate\Support\Facades\DB; it('updates the organization with the same GitHub ID when it is renamed', function (): void { $organization = Organization::factory()->create(['github_id' => '958072', 'handle' => 'laravel-old', 'github_synced_at' => now()->subWeek()]); @@ -60,3 +62,16 @@ expect($profile->refresh()->type)->toBe(ProfileType::User) ->and(Organization::query()->exists())->toBeFalse(); }); + +it('is followed, once created, by the users following it on GitHub whose follows were imported before', function (): void { + [$follower, $other] = User::factory()->count(2)->create(); + DB::table('pending_organization_follows')->insert([ + ['user_id' => $follower->id, 'github_id' => '958072', 'created_at' => now()], + ['user_id' => $other->id, 'github_id' => '1', 'created_at' => now()], + ]); + + $organization = resolve(UpdateOrCreateOrganization::class)->handle(['login' => 'laravel', 'id' => 958072]); + + expect($organization->followers()->pluck('users.id')->all())->toBe([$follower->id]) + ->and(DB::table('pending_organization_follows')->pluck('user_id')->all())->toBe([$other->id]); +}); diff --git a/tests/Unit/Jobs/ImportGitHubFollowsJobTest.php b/tests/Unit/Jobs/ImportGitHubFollowsJobTest.php index 49837dcf7..212c660c9 100644 --- a/tests/Unit/Jobs/ImportGitHubFollowsJobTest.php +++ b/tests/Unit/Jobs/ImportGitHubFollowsJobTest.php @@ -13,7 +13,7 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('login')->with($user->github_id)->andReturn($user->handle); - $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn([$followed->github_id]); + $github->shouldReceive('followingIds')->with($user->handle, 10)->andReturn(['users' => [$followed->github_id], 'organizations' => []]); $github->shouldReceive('followerIds')->with($user->handle, 10)->andReturn([]); dispatch_sync(new ImportGitHubFollowsJob($user, $notify)); diff --git a/tests/Unit/Services/GitHubProviders/HttpGitHubProviderTest.php b/tests/Unit/Services/GitHubProviders/HttpGitHubProviderTest.php index 88fc48d1b..ec67519c2 100644 --- a/tests/Unit/Services/GitHubProviders/HttpGitHubProviderTest.php +++ b/tests/Unit/Services/GitHubProviders/HttpGitHubProviderTest.php @@ -136,7 +136,7 @@ ]); it('reads the ids of followed and following accounts a page at a time, up to the given number of pages', function (): void { - $page = fn (int $from, int $count): array => array_map(fn (int $id): array => ['id' => $id, 'login' => "user{$id}"], range($from, $from + $count - 1)); + $page = fn (int $from, int $count): array => array_map(fn (int $id): array => ['id' => $id, 'login' => "user{$id}", 'type' => 'User'], range($from, $from + $count - 1)); Http::fake([ 'api.github.com/users/octocat/following?per_page=100&page=1' => Http::response($page(1, 100)), @@ -147,12 +147,24 @@ $provider = new HttpGitHubProvider(); - expect($provider->followingIds('octocat', 1))->toBe(array_map(strval(...), range(1, 100))) + expect($provider->followingIds('octocat', 1))->toBe(['users' => array_map(strval(...), range(1, 100)), 'organizations' => []]) ->and($provider->followerIds('octocat', 10))->toBe(array_map(strval(...), range(1, 103))); Http::assertSentCount(3); }); +it('tells the organizations the user follows apart from the people', function (): void { + Http::fake([ + 'api.github.com/users/octocat/following?per_page=100&page=1' => Http::response([ + ['id' => 1, 'login' => 'mona', 'type' => 'User'], + ['id' => 958072, 'login' => 'laravel', 'type' => 'Organization'], + ['id' => 2, 'login' => 'hubot', 'type' => 'User'], + ]), + ]); + + expect(new HttpGitHubProvider()->followingIds('octocat', 10))->toBe(['users' => ['1', '2'], 'organizations' => ['958072']]); +}); + it('reads the authentication and signing keys of the account', function (): void { Http::fake([ 'api.github.com/users/octocat/keys*' => Http::response([['id' => 1, 'key' => 'ssh-ed25519 AAAA1']]), diff --git a/tests/Unit/Services/GitHubProviders/NullGitHubProviderTest.php b/tests/Unit/Services/GitHubProviders/NullGitHubProviderTest.php index 34422b21e..942994f13 100644 --- a/tests/Unit/Services/GitHubProviders/NullGitHubProviderTest.php +++ b/tests/Unit/Services/GitHubProviders/NullGitHubProviderTest.php @@ -19,7 +19,7 @@ ->and($provider->login('583231'))->toBeNull() ->and($provider->isPublicMember('45861341', '720328'))->toBeFalse() ->and($provider->hasSponsorsListing('token'))->toBeNull() - ->and($provider->followingIds('octocat', 10))->toBe([]) + ->and($provider->followingIds('octocat', 10))->toBe(['users' => [], 'organizations' => []]) ->and($provider->followerIds('octocat', 10))->toBe([]) ->and($provider->sshKeys('octocat'))->toBe([]) ->and($provider->avatar('https://avatars.githubusercontent.com/u/583231'))->toBeNull() From 88c6e8c3be5a1828fb23909aec2acb4c8778f925 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 31 Oct 2026 21:50:04 +0100 Subject: [PATCH 2/4] fix: keep the log promise to imported people, and lock organization follows once --- app/Actions/CreateUserGitHubFollows.php | 7 +++---- app/Actions/UpdateOrCreateOrganization.php | 14 +++++++++++--- .../Commands/ImportGitHubFollowsCommand.php | 5 +++-- resources/js/pages/onboarding/show.tsx | 2 +- tests/Feature/Commands/ImportGitHubFollowsTest.php | 6 +++--- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/Actions/CreateUserGitHubFollows.php b/app/Actions/CreateUserGitHubFollows.php index b63f77cff..fc5473fec 100644 --- a/app/Actions/CreateUserGitHubFollows.php +++ b/app/Actions/CreateUserGitHubFollows.php @@ -44,7 +44,7 @@ public function handle(User $user, bool $notify = true): void $followingIds = $login === null ? ['users' => [], 'organizations' => []] : $this->github->followingIds($login, self::MaxPages); $following = $importsPeople ? $this->users($user, $followingIds['users']) : new Collection(); $followers = $importsPeople && $login !== null ? $this->users($user, $this->github->followerIds($login, self::MaxPages)) : new Collection(); - $organizationIds = collect($followingIds['organizations'])->unique()->sort()->values(); + $organizationIds = collect($followingIds['organizations'])->unique()->values(); DB::transaction(function () use ($user, $notify, $following, $followers, $organizationIds, $importsPeople, $importsOrganizations): void { $followedIds = $user->following()->syncWithoutDetaching($following->modelKeys())['attached']; @@ -62,9 +62,8 @@ public function handle(User $user, bool $notify = true): void if ($importsOrganizations) { // Waits for an organization being created from GitHub right now, which follows the pending rows it finds - // (UpdateOrCreateOrganization), so the follow is made on one side or the other. Locked in order, so two - // imports never wait on each other. - DB::select('select pg_advisory_xact_lock(hashtextextended(id, 0)) from unnest(?::text[]) as id', ['{'.$organizationIds->map(fn (string $id): string => "github-organization:{$id}")->implode(',').'}']); + // (UpdateOrCreateOrganization), so the follow is made on one side or the other. + DB::select('select pg_advisory_xact_lock(hashtextextended(?, 0))', [UpdateOrCreateOrganization::FollowsLock]); $organizations = Profile::query()->where('type', ProfileType::Organization)->whereIn('github_id', $organizationIds)->get(); diff --git a/app/Actions/UpdateOrCreateOrganization.php b/app/Actions/UpdateOrCreateOrganization.php index c131a9185..4f247ca76 100644 --- a/app/Actions/UpdateOrCreateOrganization.php +++ b/app/Actions/UpdateOrCreateOrganization.php @@ -13,6 +13,12 @@ final readonly class UpdateOrCreateOrganization { + /** + * Held while an organization is created and while a GitHub import keeps the organizations it doesn't find pending, + * so neither misses the other's rows. Both are short and rare, so one lock for every organization is enough. + */ + public const string FollowsLock = 'github-organization-follows'; + public function __construct(private UpdateStaleHandles $updateStaleHandles) { // @@ -32,8 +38,7 @@ public function handle(array $account): Organization assert(is_int($githubId)); return DB::transaction(function () use ($profile, $githubId): Organization { - // Waits for the GitHub follow imports keeping this organization pending right now (CreateUserGitHubFollows). - DB::select('select pg_advisory_xact_lock(hashtextextended(?, 0))', ["github-organization:{$githubId}"]); + DB::select('select pg_advisory_xact_lock(hashtextextended(?, 0))', [self::FollowsLock]); $this->updateStaleHandles->handle($profile->handle, (string) $githubId); @@ -49,7 +54,10 @@ public function handle(array $account): Organization if ($organization->wasRecentlyCreated) { $pending = DB::table('pending_organization_follows')->where('github_id', (string) $githubId); - $organization->followers()->attach($pending->pluck('user_id')->all()); + DB::table('organization_follows')->insertUsing( + ['user_id', 'organization_id', 'created_at'], + $pending->clone()->select('user_id')->selectRaw('?, ?', [$organization->id, now()]), + ); $pending->delete(); } diff --git a/app/Console/Commands/ImportGitHubFollowsCommand.php b/app/Console/Commands/ImportGitHubFollowsCommand.php index b310de786..225446192 100644 --- a/app/Console/Commands/ImportGitHubFollowsCommand.php +++ b/app/Console/Commands/ImportGitHubFollowsCommand.php @@ -19,9 +19,10 @@ final class ImportGitHubFollowsCommand extends Command { /** - * Below this many requests left, the command stops, leaving the rest to the app until the rate limit resets. + * Below this many requests left, the command stops, leaving the rest to the app until the rate limit resets: above + * the floor where looking up mentioned organizations pauses, so they carry on meanwhile. */ - private const int RateLimitFloor = 1000; + private const int RateLimitFloor = 1500; public function handle(CreateUserGitHubFollows $createUserGitHubFollows, GitHubProvider $github): int { diff --git a/resources/js/pages/onboarding/show.tsx b/resources/js/pages/onboarding/show.tsx index ea1a8b11c..da4ff9fde 100644 --- a/resources/js/pages/onboarding/show.tsx +++ b/resources/js/pages/onboarding/show.tsx @@ -791,7 +791,7 @@ export default function Show({ : 'Follow a few developers.' } description={ - imported.length > 0 + importedFollowingCount > 0 ? "We brought your GitHub network over, so your log doesn't start empty. Follow a few more to fill it up; unfollowing is one click." : 'Your log is your commits plus the commits of everyone you follow. Here are a few to start with; unfollowing is one click.' } diff --git a/tests/Feature/Commands/ImportGitHubFollowsTest.php b/tests/Feature/Commands/ImportGitHubFollowsTest.php index 19c7f4ddb..f9b6a1051 100644 --- a/tests/Feature/Commands/ImportGitHubFollowsTest.php +++ b/tests/Feature/Commands/ImportGitHubFollowsTest.php @@ -105,15 +105,15 @@ $github = $this->mock(GitHubProvider::class); $github->shouldReceive('rateLimit')->andReturn( - ['remaining' => 1021, 'limit' => 5000, 'resets_at' => CarbonImmutable::parse('2026-11-01 10:42:00', 'UTC')], - ['remaining' => 1020, 'limit' => 5000, 'resets_at' => CarbonImmutable::parse('2026-11-01 10:42:00', 'UTC')], + ['remaining' => 1521, 'limit' => 5000, 'resets_at' => CarbonImmutable::parse('2026-11-01 10:42:00', 'UTC')], + ['remaining' => 1520, 'limit' => 5000, 'resets_at' => CarbonImmutable::parse('2026-11-01 10:42:00', 'UTC')], ); $github->shouldReceive('login')->with($imported->github_id)->once()->andReturn($imported->handle); $github->shouldReceive('followingIds')->with($imported->handle, 10)->andReturn(['users' => [], 'organizations' => []]); $github->shouldReceive('followerIds')->with($imported->handle, 10)->andReturn([]); $this->artisan('follows:import') - ->expectsOutputToContain('Stopped after 1 users, to keep 1000 GitHub requests for the rest of the app: 1020 left.') + ->expectsOutputToContain('Stopped after 1 users, to keep 1500 GitHub requests for the rest of the app: 1520 left.') ->expectsOutputToContain('2 users left. Run the command again after 2026-11-01 10:42:00 UTC (42 minutes from now), when the GitHub rate limit resets.') ->assertFailed(); From 3df484cdb4d825c1c34341124d2d971312e9708b Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 31 Oct 2026 23:43:59 +0100 Subject: [PATCH 3/4] fix: draw organizations in the onboarding with their lane, like people --- app/Http/Controllers/OnboardingController.php | 6 ++++-- resources/js/pages/onboarding/show.tsx | 21 ++++++++++++------- .../Controllers/OnboardingControllerTest.php | 1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/OnboardingController.php b/app/Http/Controllers/OnboardingController.php index ec140f11c..643ada514 100644 --- a/app/Http/Controllers/OnboardingController.php +++ b/app/Http/Controllers/OnboardingController.php @@ -43,7 +43,7 @@ public function update(#[CurrentUser] User $user, UpdateUserOnboardedAt $action) * What the GitHub import brought over: null until the job has run. The follows it made share its timestamp, give * or take the time the job took. Tom's friendship is made at sign-up too, right before, so bots are left out. * - * @return array{following: Collection, followingCount: int, organizations: Collection, organizationsCount: int, followers: Collection, followersCount: int, pendingCount: int}|null + * @return array{following: Collection, followingCount: int, organizations: Collection>, organizationsCount: int, followers: Collection, followersCount: int, pendingCount: int}|null */ private function githubImport(ShowOnboardingRequest $request, User $user): ?array { @@ -62,7 +62,9 @@ private function githubImport(ShowOnboardingRequest $request, User $user): ?arra return [ 'following' => $imported($user->following())->oldest('users.id')->limit(5)->get(), 'followingCount' => $imported($user->following())->count(), - 'organizations' => $organizations()->oldest('organizations.id')->limit(5)->get(), + // With their lane, drawn around their avatar like people's. + 'organizations' => $organizations()->oldest('organizations.id')->limit(5)->get() + ->map(fn (Organization $organization): array => [...$organization->toArray(), 'lane' => $organization->lane()]), 'organizationsCount' => $organizations()->count(), 'followers' => $imported($user->followers())->oldest('users.id')->limit(5)->get(), 'followersCount' => $imported($user->followers())->count(), diff --git a/resources/js/pages/onboarding/show.tsx b/resources/js/pages/onboarding/show.tsx index da4ff9fde..f2b0dcb1c 100644 --- a/resources/js/pages/onboarding/show.tsx +++ b/resources/js/pages/onboarding/show.tsx @@ -29,15 +29,17 @@ import { useClipboard } from '@/hooks/use-clipboard'; import { useUser } from '@/hooks/use-user'; import { useKeyLabels } from '@/lib/keyboard'; import { formatCount } from '@/lib/numbers'; -import { cn, laneColor } from '@/lib/utils'; +import { cn, laneColor, organizationLaneColor } from '@/lib/utils'; import type { Lane, Organization, SuggestedUser, User } from '@/types'; const laneNames = ['orange', 'green', 'violet', 'teal', 'rose', 'ochre']; +type OrganizationWithLane = Organization & { lane: Lane | null }; + type GitHubImport = { following: User[]; followingCount: number; - organizations: Organization[]; + organizations: OrganizationWithLane[]; organizationsCount: number; followers: User[]; followersCount: number; @@ -109,7 +111,7 @@ function ImportStat({ children, }: { users: User[]; - organizations?: Organization[]; + organizations?: OrganizationWithLane[]; count: number; label: string; delay: number; @@ -159,13 +161,18 @@ function ImportStat({ diff --git a/tests/Feature/Controllers/OnboardingControllerTest.php b/tests/Feature/Controllers/OnboardingControllerTest.php index 2a3a46f26..6ce0bb50c 100644 --- a/tests/Feature/Controllers/OnboardingControllerTest.php +++ b/tests/Feature/Controllers/OnboardingControllerTest.php @@ -111,6 +111,7 @@ ->where('githubImport.following', fn ($users): bool => collect($users)->pluck('id')->diff($followed->modelKeys())->isEmpty()) ->where('githubImport.followingCount', 9) ->where('githubImport.organizations', fn ($imported): bool => collect($imported)->pluck('handle')->sort()->values()->all() === $organizations->pluck('handle')->sort()->values()->all()) + ->where('githubImport.organizations.0.lane', null) ->where('githubImport.organizationsCount', 2) ->where('githubImport.followers', fn ($users): bool => collect($users)->pluck('id')->sort()->values()->all() === collect($followers->modelKeys())->sort()->values()->all()) ->where('githubImport.followersCount', 2) From 81598a8c88cb6bbb77c8a76a0e5743026f956cc5 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sun, 1 Nov 2026 00:11:04 +0100 Subject: [PATCH 4/4] fix: show the organizations imported after the people in the onboarding --- app/Http/Controllers/OnboardingController.php | 4 +++- .../Controllers/OnboardingControllerTest.php | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/OnboardingController.php b/app/Http/Controllers/OnboardingController.php index 643ada514..5706985c6 100644 --- a/app/Http/Controllers/OnboardingController.php +++ b/app/Http/Controllers/OnboardingController.php @@ -56,8 +56,10 @@ private function githubImport(ShowOnboardingRequest $request, User $user): ?arra $imported = fn (BelongsToMany $follows): BelongsToMany => $follows ->wherePivotBetween('created_at', [$importedAt->copy()->subSeconds(10), $importedAt]) ->whereNotIn('users.github_id', User::BOT_GITHUB_IDS); + // Organizations are imported on their own for the users who signed up before they could be followed... + $organizationsImportedAt = $user->github_organization_follows_imported_at ?? $importedAt; $organizations = fn (): BelongsToMany => $user->followedOrganizations() - ->wherePivotBetween('created_at', [$importedAt->copy()->subSeconds(10), $importedAt]); + ->wherePivotBetween('created_at', [$organizationsImportedAt->copy()->subSeconds(10), $organizationsImportedAt]); return [ 'following' => $imported($user->following())->oldest('users.id')->limit(5)->get(), diff --git a/tests/Feature/Controllers/OnboardingControllerTest.php b/tests/Feature/Controllers/OnboardingControllerTest.php index 6ce0bb50c..b621dfc13 100644 --- a/tests/Feature/Controllers/OnboardingControllerTest.php +++ b/tests/Feature/Controllers/OnboardingControllerTest.php @@ -118,6 +118,22 @@ ->where('githubImport.pendingCount', 0)); }); +it('shows the organizations imported after the people, for users who signed up before organizations could be followed', function (): void { + $this->travelTo(now()->startOfSecond()); + + $user = User::factory()->notOnboarded()->create(['github_follows_imported_at' => now()->subDay(), 'github_organization_follows_imported_at' => now()]); + $organization = Organization::factory()->create(); + $user->followedOrganizations()->attach($organization, ['created_at' => now()->subSeconds(3)]); + + $response = $this->actingAs($user) + ->get(route('onboarding.show')); + + $response->assertOk() + ->assertInertia(fn ($page) => $page + ->where('githubImport.organizations.0.handle', $organization->handle) + ->where('githubImport.organizationsCount', 1)); +}); + it('is not found once the user has onboarded', function (): void { $user = User::factory()->create();