diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index de152c881..d54c320c6 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -114,13 +114,15 @@ GitHub organizations, like `@laravel`, have a page at `/@{handle}` too, with the The **Mentions** tab lists the commits and replies that mention the organization, newest first, a reply with the commit it answers. A commit stays listed after the organization is renamed, since it is linked to the organization rather than to the handle. It follows the usual visibility: guests see approved commits only, you also see your own pending ones, and commits across a block or by people you muted are left out. -A member of an organization can **Claim** its page, which lists them as its admin. Only the first member to claim it becomes its admin: once a page has one, it can't be claimed again, and the Claim button is gone. Claiming asks GitHub once for permission to read your organizations, then checks that you are an owner or a member there, not just invited. Organizations that restrict third-party apps only share their members once an owner approved /dev/null: the page then links to GitHub, where an owner grants access and a member requests it. GitHub is only the way in: leaving the organization on GitHub doesn't remove you as an admin on /dev/null. Claims are in your data export and go when you delete your account. +A member of an organization can **Claim** its page, which lists them as its admin. Only the first member to claim it becomes its admin: once a page has one, it can't be claimed again, and the Claim button is gone. Claiming asks GitHub once for permission to read your organizations, then checks that you are an owner or a member there, not just invited. Organizations that restrict third-party apps only share their members once an owner approved /dev/null: the page then links to GitHub, where an owner grants access and a member requests it. GitHub is only the way in: leaving the organization on GitHub doesn't remove you as an admin on /dev/null. -Every organization has a color, /dev/null's blue until its admin chooses another with **Color** on its page: one of the same six lane colors as developers, or a custom one. It rings the organization's avatar, leads its handle, and colors its link preview. +After that, members are managed on /dev/null, not mirrored from GitHub. An organization's members open its **Settings** from its page, where the **Members** tab lists everyone, each an **admin** or a **member**. Admins add people by their /dev/null handle, except people they blocked or who blocked them, make them admins or members, and remove them. Any member can leave. Its last admin can't leave or become a member. Only its members see who its members are; its page lists its admins to everyone. Your organizations and your role in each are in your data export and go when you delete your account. + +Every organization has a color, /dev/null's blue until an admin chooses another in the **Profile** tab of its **Settings**, where its admins land: one of the same six lane colors as developers, or a custom one. It rings the organization's avatar, leads its handle, and colors its link preview. Following organizations is planned, not shipped yet. -Sources: [organization page](resources/js/pages/organization/show.tsx), [mentions](app/Queries/OrganizationMentionsQuery.php), [organization lookups](app/Actions/UpdateCommitTopics.php), [claims](app/Http/Controllers/OrganizationAdminController.php), [color](resources/js/components/organization-lane-dialog.tsx). +Sources: [organization page](resources/js/pages/organization/show.tsx), [mentions](app/Queries/OrganizationMentionsQuery.php), [organization lookups](app/Actions/UpdateCommitTopics.php), [claims](app/Http/Controllers/OrganizationAdminController.php), [color](app/Http/Controllers/OrganizationLaneController.php), [members](app/Http/Controllers/OrganizationMemberController.php). ### Going live diff --git a/app/Actions/CreateExport.php b/app/Actions/CreateExport.php index 079d84250..e04f419d7 100644 --- a/app/Actions/CreateExport.php +++ b/app/Actions/CreateExport.php @@ -44,7 +44,7 @@ * muting: Generator, * blocking: Generator, * dismissed_suggestions: Generator, - * claimed_organizations: array, + * organizations: array, * messages: Generator, * remote_pins: array, * remote_mutes: array, @@ -66,7 +66,7 @@ public function handle(User $user): array { return [ - 'schema_version' => 3, + 'schema_version' => 4, 'exported_at' => now()->toIso8601String(), 'profile' => [ 'github_id' => $user->github_id, @@ -111,7 +111,7 @@ public function handle(User $user): array 'muting' => $this->handles($user->muting()), 'blocking' => $this->handles($user->blocking()), 'dismissed_suggestions' => $this->handles($user->dismissedSuggestions()), - 'claimed_organizations' => $this->claimedOrganizations($user), + 'organizations' => $this->organizations($user), 'messages' => $this->messages($user), 'remote_mutes' => Conversation::query() ->join('conversation_participants as mine', 'mine.conversation_id', '=', 'conversations.id') @@ -293,19 +293,20 @@ private function handles(BelongsToMany $users): Generator } /** - * @return array + * @return array */ - private function claimedOrganizations(User $user): array + private function organizations(User $user): array { - /** @var Collection $claims */ - $claims = $user->claimedOrganizations()->newPivotQuery() - ->join('profiles', 'profiles.id', '=', 'organization_admins.organization_id') - ->oldest('organization_admins.created_at') - ->get(['profiles.handle', 'organization_admins.created_at']); + /** @var Collection $memberships */ + $memberships = $user->organizations()->newPivotQuery() + ->join('profiles', 'profiles.id', '=', 'organization_members.organization_id') + ->oldest('organization_members.created_at') + ->get(['profiles.handle', 'organization_members.role', 'organization_members.created_at']); - return $claims->map(fn (object $claim): array => [ - 'handle' => $claim->handle, - 'claimed_at' => CarbonImmutable::parse($claim->created_at)->toIso8601String(), + return $memberships->map(fn (object $membership): array => [ + 'handle' => $membership->handle, + 'role' => $membership->role, + 'joined_at' => CarbonImmutable::parse($membership->created_at)->toIso8601String(), ])->all(); } diff --git a/app/Actions/CreateOrganizationAdmin.php b/app/Actions/CreateOrganizationAdmin.php index 8d178e0b2..edda93ac3 100644 --- a/app/Actions/CreateOrganizationAdmin.php +++ b/app/Actions/CreateOrganizationAdmin.php @@ -4,6 +4,7 @@ namespace App\Actions; +use App\Enums\OrganizationRole; use App\Models\Profile; use App\Models\User; use Illuminate\Support\Facades\DB; @@ -24,7 +25,8 @@ public function handle(Profile $organization, User $user): bool return false; } - $organization->admins()->attach($user->id); + // A member is left without an admin when the last one erased their account... + $organization->members()->syncWithoutDetaching([$user->id => ['role' => OrganizationRole::Admin]]); return true; }); diff --git a/app/Actions/CreateOrganizationMember.php b/app/Actions/CreateOrganizationMember.php new file mode 100644 index 000000000..aa81c57e8 --- /dev/null +++ b/app/Actions/CreateOrganizationMember.php @@ -0,0 +1,20 @@ +members()->attach($user->id, ['role' => OrganizationRole::Member]); + } +} diff --git a/app/Actions/DeleteOrganization.php b/app/Actions/DeleteOrganization.php index 6354b35eb..75db3006d 100644 --- a/app/Actions/DeleteOrganization.php +++ b/app/Actions/DeleteOrganization.php @@ -11,7 +11,7 @@ final readonly class DeleteOrganization { /** - * Deletes an organization GitHub no longer has, with its admins, and unlinks the commits that mention it, so none + * Deletes an organization GitHub no longer has, with its members, and unlinks the commits that mention it, so none * links to a page that is gone. */ public function handle(Profile $organization): void @@ -22,7 +22,7 @@ public function handle(Profile $organization): void ->where('topic_id', $organization->id) ->delete(); - $organization->admins()->detach(); + $organization->members()->detach(); $organization->delete(); }); } diff --git a/app/Actions/DeleteOrganizationMember.php b/app/Actions/DeleteOrganizationMember.php new file mode 100644 index 000000000..fafce59fd --- /dev/null +++ b/app/Actions/DeleteOrganizationMember.php @@ -0,0 +1,34 @@ +whereKey($organization->id)->lockForUpdate()->first(); + + if ($organization->roleOf($member) === OrganizationRole::Admin && $organization->admins()->whereKeyNot($member->id)->doesntExist()) { + throw new OrganizationMustHaveAnAdmin($organization); + } + + $organization->members()->detach($member->id); + }); + } +} diff --git a/app/Actions/DeleteUser.php b/app/Actions/DeleteUser.php index 56513ed1f..033a8af2f 100644 --- a/app/Actions/DeleteUser.php +++ b/app/Actions/DeleteUser.php @@ -67,7 +67,7 @@ public function handle(User $user): void $this->deleteOrgs->handle($this->emptyOrgs($user)); - $user->claimedOrganizations()->detach(); + $user->organizations()->detach(); $user->delete(); $user->profile->delete(); diff --git a/app/Actions/UpdateOrganizationMemberRole.php b/app/Actions/UpdateOrganizationMemberRole.php new file mode 100644 index 000000000..26155cdee --- /dev/null +++ b/app/Actions/UpdateOrganizationMemberRole.php @@ -0,0 +1,34 @@ +whereKey($organization->id)->lockForUpdate()->first(); + + if ($role === OrganizationRole::Member && $organization->admins()->whereKeyNot($member->id)->doesntExist()) { + throw new OrganizationMustHaveAnAdmin($organization); + } + + $organization->members()->updateExistingPivot($member->id, ['role' => $role]); + }); + } +} diff --git a/app/Enums/OrganizationRole.php b/app/Enums/OrganizationRole.php new file mode 100644 index 000000000..016814abb --- /dev/null +++ b/app/Enums/OrganizationRole.php @@ -0,0 +1,14 @@ + $organization->handle])); + } +} diff --git a/app/Http/Controllers/OrganizationAdminController.php b/app/Http/Controllers/OrganizationAdminController.php index 1f0d8f6a9..935789e9d 100644 --- a/app/Http/Controllers/OrganizationAdminController.php +++ b/app/Http/Controllers/OrganizationAdminController.php @@ -134,7 +134,7 @@ private function github(): GitHubOAuthProvider */ private function claimed(Profile $organization): ?string { - $admin = $organization->admins()->oldest('organization_admins.created_at')->first(); + $admin = $organization->admins()->oldest('organization_members.created_at')->first(); if ($admin === null) { return null; diff --git a/app/Http/Controllers/OrganizationController.php b/app/Http/Controllers/OrganizationController.php index 800b57631..9a800cbc7 100644 --- a/app/Http/Controllers/OrganizationController.php +++ b/app/Http/Controllers/OrganizationController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers; +use App\Enums\OrganizationRole; use App\Http\Requests\ShowOrganizationRequest; use App\Models\Profile; use App\Models\User; @@ -19,8 +20,9 @@ { /** * Organization pages are public, at the "@handle" of users, and show the organization's GitHub profile and the - * commits that mention it, in the lane its admins chose or /dev/null's blue, with the users who claimed it, first - * claim first, and the organization's link preview. Like users' pages, they move for good to the handle's own case. + * commits that mention it, in the lane its admins chose or /dev/null's blue, with its admins, first one first, and + * the organization's link preview, and its members the way to its settings. Like users' pages, they move for good + * to the handle's own case. */ public function show(ShowOrganizationRequest $request, #[CurrentUser] ?User $currentUser, OrganizationMentionsQuery $mentions, PageMetadata $metadata): RedirectResponse|Response { @@ -43,7 +45,13 @@ public function show(ShowOrganizationRequest $request, #[CurrentUser] ?User $cur ->cursorPaginate(20) ->appends(['tab' => $request->tab()])), 'claimUrl' => route('organization-admin.create', $organization), - 'admins' => fn (): Collection => $organization->admins()->oldest('organization_admins.created_at')->get()->makeHidden('pivot'), + // Admins land on the Profile tab, which only they have... + 'settingsUrl' => match ($currentUser instanceof User ? $organization->roleOf($currentUser) : null) { + OrganizationRole::Admin => route('organization-profile.edit', $organization), + OrganizationRole::Member => route('organization-member.index', $organization), + null => null, + }, + 'admins' => fn (): Collection => $organization->admins()->oldest('organization_members.created_at')->get()->makeHidden('pivot'), ])->withViewData(['preview' => $currentUser instanceof User || $request->inertia() ? null : $metadata->organization($organization)]); } } diff --git a/app/Http/Controllers/OrganizationLaneController.php b/app/Http/Controllers/OrganizationLaneController.php index 81a989271..de8be04c7 100644 --- a/app/Http/Controllers/OrganizationLaneController.php +++ b/app/Http/Controllers/OrganizationLaneController.php @@ -10,6 +10,9 @@ use Illuminate\Http\RedirectResponse; use Inertia\Inertia; +/** + * An organization's color, in the Profile tab of its settings. Only its admins choose it. + */ final readonly class OrganizationLaneController { public function update(UpdateOrganizationLaneRequest $request, Profile $organization, UpdateOrganizationLane $action): RedirectResponse diff --git a/app/Http/Controllers/OrganizationMemberController.php b/app/Http/Controllers/OrganizationMemberController.php new file mode 100644 index 000000000..70b2c7ba6 --- /dev/null +++ b/app/Http/Controllers/OrganizationMemberController.php @@ -0,0 +1,97 @@ + $organization->makeVisible('id'), + 'role' => $organization->roleOf($user), + 'members' => $organization->members() + ->oldest('organization_members.created_at') + ->withCasts(['joined_at' => 'datetime']) + ->get(['users.*', 'organization_members.role', 'organization_members.created_at as joined_at']) + ->makeHidden('pivot'), + ]); + } + + public function store(CreateOrganizationMemberRequest $request, Profile $organization, CreateOrganizationMember $action): RedirectResponse + { + $member = $request->member(); + assert($member instanceof User); + + $action->handle($organization, $member); + + return back(); + } + + public function update(UpdateOrganizationMemberRequest $request, Profile $organization, User $member, UpdateOrganizationMemberRole $action): RedirectResponse + { + $role = $request->role(); + assert($role instanceof OrganizationRole); + + try { + $action->handle($organization, $member, $role); + } catch (OrganizationMustHaveAnAdmin $organizationMustHaveAnAdmin) { + return $this->refused($organizationMustHaveAnAdmin); + } + + return back(); + } + + /** + * Leaving takes the user to the organization's page, as its settings are for members only. + */ + public function destroy(DeleteOrganizationMemberRequest $request, #[CurrentUser] User $user, Profile $organization, User $member, DeleteOrganizationMember $action): RedirectResponse + { + try { + $action->handle($organization, $member); + } catch (OrganizationMustHaveAnAdmin $organizationMustHaveAnAdmin) { + return $this->refused($organizationMustHaveAnAdmin); + } + + if ($user->is($member)) { + Inertia::flash('toast', ['type' => 'success', 'message' => __('You left @:handle.', ['handle' => $organization->handle])]); + + return to_route('user.show', $organization->handle); + } + + return back(); + } + + /** + * Another admin was made a member or left meanwhile, so the change would have left no admin. + */ + private function refused(OrganizationMustHaveAnAdmin $exception): RedirectResponse + { + Inertia::flash('toast', ['type' => 'error', 'message' => $exception->getMessage()]); + + return back(); + } +} diff --git a/app/Http/Controllers/OrganizationProfileController.php b/app/Http/Controllers/OrganizationProfileController.php new file mode 100644 index 000000000..7863e3800 --- /dev/null +++ b/app/Http/Controllers/OrganizationProfileController.php @@ -0,0 +1,26 @@ + $organization->makeVisible('id'), + 'role' => OrganizationRole::Admin, + 'lane' => $organization->lane(), + ]); + } +} diff --git a/app/Http/Requests/CreateOrganizationMemberRequest.php b/app/Http/Requests/CreateOrganizationMemberRequest.php new file mode 100644 index 000000000..992850aae --- /dev/null +++ b/app/Http/Requests/CreateOrganizationMemberRequest.php @@ -0,0 +1,85 @@ +user(); + assert($user instanceof User); + + return match ($this->organization()->roleOf($user)) { + null => Response::denyAsNotFound(), + OrganizationRole::Member => Response::deny('Only admins can add members.'), + OrganizationRole::Admin => Response::allow(), + }; + } + + /** + * @return array> + */ + public function rules(): array + { + return [ + 'handle' => ['required', 'string', 'max:40'], + ]; + } + + /** + * @return list + */ + public function after(): array + { + return [function (Validator $validator): void { + if ($validator->errors()->isNotEmpty()) { + return; + } + + $user = $this->user(); + assert($user instanceof User); + + $member = $this->member(); + + if (! $member instanceof User) { + $validator->errors()->add('handle', "There's no @{$this->handle()} on /dev/null."); + } elseif ($user->hasBlockWith($member)) { + $validator->errors()->add('handle', "You can't add {$member->mention()}."); + } elseif ($this->organization()->roleOf($member) instanceof OrganizationRole) { + $validator->errors()->add('handle', "{$member->mention()} is already a member."); + } + }]; + } + + public function organization(): Profile + { + $organization = $this->route('organization'); + assert($organization instanceof Profile); + + return $organization; + } + + /** + * The user at the handle, with or without its "@", resolved like "@handle" URLs, whatever its case. + */ + public function member(): ?User + { + $member = (new User)->resolveRouteBinding($this->handle(), 'handle'); + + return $member instanceof User ? $member : null; + } + + private function handle(): string + { + return $this->string('handle')->trim()->ltrim('@')->value(); + } +} diff --git a/app/Http/Requests/DeleteOrganizationMemberRequest.php b/app/Http/Requests/DeleteOrganizationMemberRequest.php new file mode 100644 index 000000000..9d3611d87 --- /dev/null +++ b/app/Http/Requests/DeleteOrganizationMemberRequest.php @@ -0,0 +1,69 @@ +user(); + assert($user instanceof User); + + return match (true) { + ! $this->organization()->roleOf($user) instanceof OrganizationRole || ! $this->organization()->roleOf($this->member()) instanceof OrganizationRole => Response::denyAsNotFound(), + ! $user->is($this->member()) && $this->organization()->roleOf($user) !== OrganizationRole::Admin => Response::deny('Only admins can remove members.'), + default => Response::allow(), + }; + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } + + /** + * @return list + */ + public function after(): array + { + return [function (Validator $validator): void { + $isLastAdmin = $this->organization()->roleOf($this->member()) === OrganizationRole::Admin + && $this->organization()->admins()->whereKeyNot($this->member()->id)->doesntExist(); + + if ($isLastAdmin) { + $validator->errors()->add('member', "@{$this->organization()->handle} needs at least one admin."); + } + }]; + } + + public function organization(): Profile + { + $organization = $this->route('organization'); + assert($organization instanceof Profile); + + return $organization; + } + + public function member(): User + { + $member = $this->route('member'); + assert($member instanceof User); + + return $member; + } +} diff --git a/app/Http/Requests/ListOrganizationMembersRequest.php b/app/Http/Requests/ListOrganizationMembersRequest.php new file mode 100644 index 000000000..7bd3a48b7 --- /dev/null +++ b/app/Http/Requests/ListOrganizationMembersRequest.php @@ -0,0 +1,36 @@ +user(); + assert($user instanceof User); + + $organization = $this->route('organization'); + assert($organization instanceof Profile); + + return $organization->roleOf($user) instanceof OrganizationRole ? Response::allow() : Response::denyAsNotFound(); + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } +} diff --git a/app/Http/Requests/ShowOrganizationProfileRequest.php b/app/Http/Requests/ShowOrganizationProfileRequest.php new file mode 100644 index 000000000..fe49c2352 --- /dev/null +++ b/app/Http/Requests/ShowOrganizationProfileRequest.php @@ -0,0 +1,36 @@ +user(); + assert($user instanceof User); + + $organization = $this->route('organization'); + assert($organization instanceof Profile); + + return $organization->roleOf($user) === OrganizationRole::Admin ? Response::allow() : Response::denyAsNotFound(); + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } +} diff --git a/app/Http/Requests/UpdateOrganizationMemberRequest.php b/app/Http/Requests/UpdateOrganizationMemberRequest.php new file mode 100644 index 000000000..582cec029 --- /dev/null +++ b/app/Http/Requests/UpdateOrganizationMemberRequest.php @@ -0,0 +1,77 @@ +user(); + assert($user instanceof User); + + return match (true) { + ! $this->organization()->roleOf($user) instanceof OrganizationRole || ! $this->organization()->roleOf($this->member()) instanceof OrganizationRole => Response::denyAsNotFound(), + $this->organization()->roleOf($user) !== OrganizationRole::Admin => Response::deny('Only admins can change roles.'), + $user->is($this->member()) && $this->role() === OrganizationRole::Admin => Response::deny("You're already an admin."), + default => Response::allow(), + }; + } + + /** + * @return array> + */ + public function rules(): array + { + return [ + 'role' => ['required', Rule::enum(OrganizationRole::class)], + ]; + } + + /** + * @return list + */ + public function after(): array + { + return [function (Validator $validator): void { + if ($this->role() === OrganizationRole::Member && $this->organization()->admins()->whereKeyNot($this->member()->id)->doesntExist()) { + $validator->errors()->add('role', "@{$this->organization()->handle} needs at least one admin."); + } + }]; + } + + public function organization(): Profile + { + $organization = $this->route('organization'); + assert($organization instanceof Profile); + + return $organization; + } + + public function member(): User + { + $member = $this->route('member'); + assert($member instanceof User); + + return $member; + } + + public function role(): ?OrganizationRole + { + return $this->enum('role', OrganizationRole::class); + } +} diff --git a/app/Models/Profile.php b/app/Models/Profile.php index aef8c4e14..dc60ab886 100644 --- a/app/Models/Profile.php +++ b/app/Models/Profile.php @@ -4,6 +4,7 @@ namespace App\Models; +use App\Enums\OrganizationRole; use App\Enums\ProfileType; use Carbon\CarbonInterface; use Database\Factories\ProfileFactory; @@ -127,13 +128,33 @@ public function user(): HasOne } /** - * The users who claimed the organization's page. GitHub only lets them in: leaving the organization there doesn't - * remove them here. + * The organization's members on /dev/null, each an admin or a member. GitHub only lets the first admin in, who + * claimed the page: leaving the organization there doesn't remove anyone here. + * + * @return BelongsToMany + */ + public function members(): BelongsToMany + { + return $this->belongsToMany(User::class, 'organization_members', 'organization_id')->withPivot('role')->withTimestamps(updatedAt: false); + } + + /** + * The members who manage the organization's members. Attaching through it makes an admin. * * @return BelongsToMany */ public function admins(): BelongsToMany { - return $this->belongsToMany(User::class, 'organization_admins', 'organization_id')->withTimestamps(updatedAt: false); + return $this->members()->withPivotValue('role', OrganizationRole::Admin->value); + } + + /** + * The user's role in the organization, or null when they aren't a member. + */ + public function roleOf(User $user): ?OrganizationRole + { + $role = $this->members()->newPivotQuery()->where('user_id', $user->id)->value('role'); + + return is_string($role) ? OrganizationRole::from($role) : null; } } diff --git a/app/Models/User.php b/app/Models/User.php index 44679049b..d17824f9e 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -334,13 +334,13 @@ public function isBanned(): bool } /** - * The organizations whose page the user claimed, see `Profile::admins()`. + * The organizations the user is a member of, see `Profile::members()`. * * @return BelongsToMany */ - public function claimedOrganizations(): BelongsToMany + public function organizations(): BelongsToMany { - return $this->belongsToMany(Profile::class, 'organization_admins', 'user_id', 'organization_id')->withTimestamps(updatedAt: false); + return $this->belongsToMany(Profile::class, 'organization_members', 'user_id', 'organization_id')->withPivot('role')->withTimestamps(updatedAt: false); } /** diff --git a/database/migrations/2026_10_29_130000_create_organization_members_table.php b/database/migrations/2026_10_29_130000_create_organization_members_table.php new file mode 100644 index 000000000..83acdd327 --- /dev/null +++ b/database/migrations/2026_10_29_130000_create_organization_members_table.php @@ -0,0 +1,42 @@ +foreignUuid('organization_id')->constrained('profiles'); + $table->foreignUuid('user_id')->constrained(); + $table->string('role'); + $table->timestamp('created_at'); + + $table->primary(['organization_id', 'user_id']); + $table->index('user_id'); + }); + + DB::table('organization_members')->insertUsing( + ['organization_id', 'user_id', 'role', 'created_at'], + DB::table('organization_admins')->select(['organization_id', 'user_id', DB::raw("'admin'"), 'created_at']), + ); + + Schema::drop('organization_admins'); + } +}; diff --git a/resources/js/layouts/organization-settings/layout.tsx b/resources/js/layouts/organization-settings/layout.tsx new file mode 100644 index 000000000..f7d3c92a4 --- /dev/null +++ b/resources/js/layouts/organization-settings/layout.tsx @@ -0,0 +1,69 @@ +import { Link, usePage } from '@inertiajs/react'; +import type { PropsWithChildren } from 'react'; +import OrganizationMemberController from '@/actions/App/Http/Controllers/OrganizationMemberController'; +import OrganizationProfileController from '@/actions/App/Http/Controllers/OrganizationProfileController'; +import { show } from '@/actions/App/Http/Controllers/UserController'; +import { OrganizationAvatar } from '@/components/organization-avatar'; +import { PageTabs } from '@/components/page-tabs'; +import { useCurrentUrl } from '@/hooks/use-current-url'; +import type { Organization } from '@/types'; + +/** + * An organization's settings, like the user's. Only admins have its profile, first; every member sees its members. + */ +export default function OrganizationSettingsLayout({ + children, +}: PropsWithChildren) { + const { organization, role } = usePage<{ + organization: Organization & { id: string }; + role: 'admin' | 'member'; + }>().props; + const { isCurrentUrl } = useCurrentUrl(); + + const tabs = [ + ...(role === 'admin' + ? [ + { + title: 'Profile', + href: OrganizationProfileController.edit(organization.id), + }, + ] + : []), + { + title: 'Members', + href: OrganizationMemberController.index(organization.id), + }, + ]; + + return ( + <> +
+
+

Settings

+ + + @{organization.handle} + +
+ ({ + ...tab, + isActive: isCurrentUrl(tab.href), + }))} + /> +
+ +
+ {children} +
+ + ); +} diff --git a/resources/js/pages/organization-member/index.tsx b/resources/js/pages/organization-member/index.tsx new file mode 100644 index 000000000..feb4e0ef9 --- /dev/null +++ b/resources/js/pages/organization-member/index.tsx @@ -0,0 +1,412 @@ +import { Form, Head, Link, router } from '@inertiajs/react'; +import { useRef, useState } from 'react'; +import { toast } from 'sonner'; +import OrganizationMemberController from '@/actions/App/Http/Controllers/OrganizationMemberController'; +import { show } from '@/actions/App/Http/Controllers/UserController'; +import { Button } from '@/components/ui/button'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select'; +import { UserAvatar } from '@/components/user-avatar'; +import { useNow } from '@/hooks/use-now'; +import { useUser } from '@/hooks/use-user'; +import AppLayout from '@/layouts/app-layout'; +import OrganizationSettingsLayout from '@/layouts/organization-settings/layout'; +import { cn, formatFullDateTime, formatRelativeTime } from '@/lib/utils'; +import type { Organization, User } from '@/types'; + +type Role = 'admin' | 'member'; + +type Member = User & { role: Role; joined_at: string }; + +type OrganizationWithId = Organization & { id: string }; + +export default function Index({ + organization, + role, + members, +}: { + organization: OrganizationWithId; + role: Role; + members: Member[]; +}) { + const headingRef = useRef(null); + const isAdmin = role === 'admin'; + const adminCount = members.filter( + (member) => member.role === 'admin', + ).length; + const isOnlyAdmin = isAdmin && adminCount === 1; + + // A removed row unmounts with the focus inside it, so it moves to the next row, or the heading... + const focusAfter = (handle: string) => { + const index = members.findIndex((member) => member.handle === handle); + const next = members[index + 1] ?? members[index - 1]; + + const target = next + ? document.querySelector( + `[data-member="${next.handle}"] [data-remove]`, + ) + : null; + + (target ?? headingRef.current)?.focus({ preventScroll: true }); + }; + + return ( + <> + + +
+
+

+ Members +

+

+ {isAdmin + ? 'Admins add and remove members, and choose who else is an admin. Only members see this list.' + : 'Only members see this list. Admins add and remove members.'} +

+
+ + {isAdmin && } + +
    + {members.map((member) => ( + focusAfter(member.handle)} + /> + ))} +
+ + {isOnlyAdmin && ( +

+ You're the only admin of @{organization.handle}, so you + can't leave or become a member. Make someone else an + admin first. +

+ )} +
+ + ); +} + +/** + * Admins add people by their handle. They join as members, and can then be made admins. + */ +function AddMemberForm({ organization }: { organization: OrganizationWithId }) { + return ( +
+ {({ errors, processing }) => ( + <> + +
+ + +
+ {/* The line is kept, so an error doesn't push the list down. */} +

+ {errors.handle} +

+ + )} +
+ ); +} + +function MemberRow({ + organization, + member, + isAdmin, + isLastAdmin, + onRemoved, +}: { + organization: OrganizationWithId; + member: Member; + isAdmin: boolean; + isLastAdmin: boolean; + onRemoved: () => void; +}) { + const { user } = useUser(); + const now = useNow(); + const isSelf = member.id === user.id; + const [removal, setRemoval] = useState<'idle' | 'confirming' | 'removing'>( + 'idle', + ); + const [hasCancelled, setHasCancelled] = useState(false); + // The role the select shows until the change is saved, so it doesn't flip back meanwhile. + const [pendingRole, setPendingRole] = useState(null); + const route = { organization: organization.id, member: member.handle }; + const lastAdminReason = `@${organization.handle} needs at least one admin.`; + const joined = formatRelativeTime(member.joined_at, now); + + const changeRole = (role: Role) => { + setPendingRole(role); + + router.visit(OrganizationMemberController.update(route), { + data: { role }, + preserveScroll: true, + // Another admin may have changed the members since the page loaded... + onError: (errors) => toast.error(Object.values(errors)[0]), + onFinish: () => setPendingRole(null), + }); + }; + + const cancel = () => { + setRemoval('idle'); + setHasCancelled(true); + }; + + const remove = () => { + setRemoval('removing'); + + const prefersReducedMotion = window.matchMedia( + '(prefers-reduced-motion: reduce)', + ).matches; + + window.setTimeout( + () => { + let isRemoved = false; + + router.visit(OrganizationMemberController.destroy(route), { + preserveScroll: true, + // A removal the server refused, as another admin left meanwhile, keeps the member listed... + onSuccess: (page) => { + isRemoved = !( + page.props.members as Member[] | undefined + )?.some(({ id }) => id === member.id); + + if (isRemoved && !isSelf) { + onRemoved(); + } + }, + onError: (errors) => toast.error(Object.values(errors)[0]), + // A visit that failed or was cancelled brings the row back, so it can be removed again... + onFinish: () => { + if (!isRemoved) { + setRemoval('idle'); + } + }, + }); + }, + prefersReducedMotion ? 0 : 700, + ); + }; + + const removeLabel = isSelf ? 'Leave' : 'Remove'; + const confirmation = isSelf + ? `Leave @${organization.handle}? An admin would have to add you back.` + : `Remove @${member.handle} from @${organization.handle}?`; + + return ( +
  • +
    + {/* On phones the controls get their own line under the name, which stays readable. */} +
    + + + + +
    +

    + + {member.name} + + {isSelf && ( + + {' '} + (you) + + )} +

    + + {removal === 'idle' && ( +

    + + @{member.handle} + + · + joined{' '} + +

    + )} + + {removal === 'confirming' && ( +

    + {confirmation} +

    + )} + + {removal === 'removing' && ( +

    + rm @{member.handle} → /dev/null +

    + )} +
    + + {removal === 'idle' && ( +
    + {isAdmin ? ( + + ) : ( + + {member.role === 'admin' + ? 'Admin' + : 'Member'} + + )} + + {(isAdmin || isSelf) && ( + + )} +
    + )} + + {removal === 'confirming' && ( +
    { + if (event.key === 'Escape') { + cancel(); + } + }} + className="flex w-full shrink-0 gap-2 pl-12 sm:w-auto sm:pl-0" + > + + +
    + )} +
    +
    +
    • + ); +} + +Index.layout = [AppLayout, OrganizationSettingsLayout]; diff --git a/resources/js/components/organization-lane-dialog.tsx b/resources/js/pages/organization-profile/edit.tsx similarity index 66% rename from resources/js/components/organization-lane-dialog.tsx rename to resources/js/pages/organization-profile/edit.tsx index fc9efcb1b..51d412db5 100644 --- a/resources/js/components/organization-lane-dialog.tsx +++ b/resources/js/pages/organization-profile/edit.tsx @@ -1,52 +1,35 @@ -import { Form, router } from '@inertiajs/react'; +import { Form, Head, router } from '@inertiajs/react'; import type { CSSProperties } from 'react'; import { useState } from 'react'; import OrganizationLaneController from '@/actions/App/Http/Controllers/OrganizationLaneController'; +import Heading from '@/components/heading'; import InputError from '@/components/input-error'; import { LaneOptions } from '@/components/lane-picker'; import { OrganizationAvatar } from '@/components/organization-avatar'; import { Button } from '@/components/ui/button'; -import { - Dialog, - DialogClose, - DialogContent, - DialogDescription, - DialogFooter, - DialogTitle, -} from '@/components/ui/dialog'; +import AppLayout from '@/layouts/app-layout'; +import OrganizationSettingsLayout from '@/layouts/organization-settings/layout'; import { organizationLaneColor, withViewTransition } from '@/lib/utils'; import type { Lane, Organization } from '@/types'; -export function OrganizationLaneDialog({ +export default function Edit({ organization, lane, - open, - onOpenChange, }: { organization: Organization & { id: string }; lane: Lane | null; - open: boolean; - onOpenChange: (open: boolean) => void; }) { const [selected, setSelected] = useState(lane); return ( - { - onOpenChange(isOpen); + <> + - if (!isOpen) { - setSelected(lane); - } - }} - > - - Color - - @{organization.handle}'s lane: the ring around its avatar - and the dot before its handle. - +
    +
    { - onOpenChange(false); - router.flushAll(); - }} + onSuccess={() => router.flushAll()} + className="mt-5" > {({ processing, errors }) => ( <> @@ -89,6 +70,7 @@ export function OrganizationLaneDialog({ selected={selected} automaticLane={null} onSelect={(choice) => { + // Dragging through the custom color picker shouldn't animate every step... if ( typeof choice === 'string' && typeof selected === 'string' @@ -109,22 +91,20 @@ export function OrganizationLaneDialog({ className="mt-2" /> - - - - - - + )} - -
    + + ); } + +Edit.layout = [AppLayout, OrganizationSettingsLayout]; diff --git a/resources/js/pages/organization/show.tsx b/resources/js/pages/organization/show.tsx index d21197f1c..53b5c5398 100644 --- a/resources/js/pages/organization/show.tsx +++ b/resources/js/pages/organization/show.tsx @@ -1,5 +1,5 @@ import { Head, Link, usePage } from '@inertiajs/react'; -import { AlertCircleIcon, Link2, MapPin, Palette } from 'lucide-react'; +import { AlertCircleIcon, Link2, MapPin, Settings } from 'lucide-react'; import type { CSSProperties } from 'react'; import { useState } from 'react'; import { show } from '@/actions/App/Http/Controllers/UserController'; @@ -7,7 +7,6 @@ import { BioText } from '@/components/bio-text'; import { CommitFeed, EmptyFeed } from '@/components/commit-feed'; import { GitHubIcon } from '@/components/github-icon'; import { OrganizationAvatar } from '@/components/organization-avatar'; -import { OrganizationLaneDialog } from '@/components/organization-lane-dialog'; import { PageTabs } from '@/components/page-tabs'; import { ProfileShareButton } from '@/components/profile-share-button'; import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; @@ -29,6 +28,7 @@ export default function Show({ tab, commits, claimUrl, + settingsUrl, admins, }: { organization: Organization & { id: string }; @@ -36,6 +36,7 @@ export default function Show({ tab: 'mentions'; commits: CursorPaginated; claimUrl: string; + settingsUrl: string | null; admins: User[]; }) { const { props, flash } = usePage(); @@ -45,8 +46,6 @@ export default function Show({ // Only the first claim makes an admin. While a claim failed, its alert offers to try again instead. const claimable = auth.user !== null && admins.length === 0; const canClaim = claimable && claimFailed === undefined; - const isAdmin = admins.some((admin) => admin.id === auth.user?.id); - const [isLaneDialogOpen, setIsLaneDialogOpen] = useState(false); return ( <> @@ -75,24 +74,6 @@ export default function Show({ />
    - {isAdmin && ( - <> - - - - )} {canClaim && ( )} + {settingsUrl && ( + + )} , + <> + + From an organization's admins, when they add + you: + {' '} + that you are a member or an admin of that + organization, which its members see. Admins are + also listed on the organization's page. + , <> From Apple, when you sign in with Apple in diff --git a/routes/web.php b/routes/web.php index 141cc5029..7769f5c6f 100644 --- a/routes/web.php +++ b/routes/web.php @@ -80,7 +80,9 @@ use App\Http\Controllers\OrganizationAdminController; use App\Http\Controllers\OrganizationController; use App\Http\Controllers\OrganizationLaneController; +use App\Http\Controllers\OrganizationMemberController; use App\Http\Controllers\OrganizationMentionController; +use App\Http\Controllers\OrganizationProfileController; use App\Http\Controllers\OrganizationSitemapController; use App\Http\Controllers\OrgArchiveController; use App\Http\Controllers\OrgCallController; @@ -339,8 +341,28 @@ ->middleware('throttle:10,1') ->name('organization-admin.store'); - // An organization's color, which its admins choose... - Route::put('organizations/{organization}/lane', [OrganizationLaneController::class, 'update']) + // Organization Members, which admins manage in the organization's settings... + Route::get('organizations/{organization}/settings/members', [OrganizationMemberController::class, 'index']) + ->whereUuid('organization') + ->name('organization-member.index'); + Route::post('organizations/{organization}/settings/members', [OrganizationMemberController::class, 'store']) + ->whereUuid('organization') + ->middleware('throttle:30,1') + ->name('organization-member.store'); + Route::put('organizations/{organization}/settings/members/{member:handle}', [OrganizationMemberController::class, 'update']) + ->whereUuid('organization') + ->withoutScopedBindings() + ->name('organization-member.update'); + Route::delete('organizations/{organization}/settings/members/{member:handle}', [OrganizationMemberController::class, 'destroy']) + ->whereUuid('organization') + ->withoutScopedBindings() + ->name('organization-member.destroy'); + + // An organization's profile, which its admins edit in its settings, starting with its color... + Route::get('organizations/{organization}/settings/profile', [OrganizationProfileController::class, 'edit']) + ->whereUuid('organization') + ->name('organization-profile.edit'); + Route::put('organizations/{organization}/settings/lane', [OrganizationLaneController::class, 'update']) ->whereUuid('organization') ->name('organization-lane.update'); diff --git a/tests/Feature/Controllers/ExportControllerTest.php b/tests/Feature/Controllers/ExportControllerTest.php index b625919e5..49a532cee 100644 --- a/tests/Feature/Controllers/ExportControllerTest.php +++ b/tests/Feature/Controllers/ExportControllerTest.php @@ -8,6 +8,7 @@ use App\Contracts\Services\TranslationProvider; use App\Enums\ModerationReason; use App\Enums\NotificationType; +use App\Enums\OrganizationRole; use App\Models\Commit; use App\Models\CommitImage; use App\Models\Device; @@ -85,7 +86,7 @@ $data = json_decode($response->streamedContent(), true, flags: JSON_THROW_ON_ERROR); expect($data)->toBe([ - 'schema_version' => 3, + 'schema_version' => 4, 'exported_at' => now()->toIso8601String(), 'profile' => [ 'github_id' => $user->github_id, @@ -130,7 +131,7 @@ 'muting' => [], 'blocking' => [], 'dismissed_suggestions' => [], - 'claimed_organizations' => [], + 'organizations' => [], 'messages' => [], 'remote_mutes' => [], 'remote_pins' => [], @@ -441,20 +442,20 @@ ); }); -it('exports the organizations the user claimed, first claim first, without their other admins', function (): void { +it('exports the organizations the user is a member of with their role, first joined first, without their other members', function (): void { $this->freezeSecond(); $user = User::factory()->create(); $laravel = Profile::factory()->organization()->create(['handle' => 'laravel']); $bref = Profile::factory()->organization()->create(['handle' => 'brefphp']); $bref->admins()->attach($user->id, ['created_at' => now()->subDay()]); - $laravel->admins()->attach([$user->id => ['created_at' => now()], User::factory()->create()->id => ['created_at' => now()]]); + $laravel->members()->attach([$user->id => ['role' => OrganizationRole::Member, 'created_at' => now()], User::factory()->create()->id => ['role' => OrganizationRole::Admin, 'created_at' => now()]]); $response = $this->actingAs($user)->get(route('export')); $data = json_decode($response->streamedContent(), true, flags: JSON_THROW_ON_ERROR); - expect($data['claimed_organizations'])->toBe([ - ['handle' => 'brefphp', 'claimed_at' => now()->subDay()->toIso8601String()], - ['handle' => 'laravel', 'claimed_at' => now()->toIso8601String()], + expect($data['organizations'])->toBe([ + ['handle' => 'brefphp', 'role' => 'admin', 'joined_at' => now()->subDay()->toIso8601String()], + ['handle' => 'laravel', 'role' => 'member', 'joined_at' => now()->toIso8601String()], ]); }); diff --git a/tests/Feature/Controllers/OrganizationAdminControllerTest.php b/tests/Feature/Controllers/OrganizationAdminControllerTest.php index 97acac7b5..cda016eed 100644 --- a/tests/Feature/Controllers/OrganizationAdminControllerTest.php +++ b/tests/Feature/Controllers/OrganizationAdminControllerTest.php @@ -9,7 +9,6 @@ use GuzzleHttp\Psr7\Response as Psr7Response; use Illuminate\Http\Client\RequestException; use Illuminate\Http\Client\Response; -use Illuminate\Support\Facades\DB; use Laravel\Socialite\Facades\Socialite; use Laravel\Socialite\Two\GithubProvider as GitHubOAuthProvider; use Laravel\Socialite\Two\InvalidStateException; @@ -115,10 +114,7 @@ function claimingUserAndOrganization(): array [$user, $organization] = claimingUserAndOrganization(); $first = User::factory()->create(); $second = User::factory()->create(); - DB::table('organization_admins')->insert([ - ['organization_id' => $organization->id, 'user_id' => $second->id, 'created_at' => now()], - ['organization_id' => $organization->id, 'user_id' => $first->id, 'created_at' => now()->subDay()], - ]); + $organization->admins()->attach([$second->id => ['created_at' => now()], $first->id => ['created_at' => now()->subDay()]]); $this->actingAs($user) ->withSession(['claiming-organization' => $organization->id]) diff --git a/tests/Feature/Controllers/OrganizationControllerTest.php b/tests/Feature/Controllers/OrganizationControllerTest.php index df56d83f0..7ca0a1309 100644 --- a/tests/Feature/Controllers/OrganizationControllerTest.php +++ b/tests/Feature/Controllers/OrganizationControllerTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); use App\Actions\CreateCommitTopics; +use App\Enums\OrganizationRole; use App\Models\Commit; use App\Models\Profile; use App\Models\User; @@ -44,16 +45,18 @@ 'custom' => [['lane_color' => '#1a2b3c'], '#1a2b3c'], ]); -it('shows who claimed the organization, first claim first, and where to claim it', function (): void { +it('shows the admins of the organization but not its other members, first admin first, and where to claim it', function (): void { $organization = Profile::factory()->organization()->create(['handle' => 'laravel']); $first = User::factory()->create(); $second = User::factory()->create(); $organization->admins()->attach([$second->id => ['created_at' => now()], $first->id => ['created_at' => now()->subDay()]]); + $organization->members()->attach(User::factory()->create()->id, ['role' => OrganizationRole::Member, 'created_at' => now()->subWeek()]); $this->get('/@laravel') ->assertOk() ->assertInertia(fn ($page) => $page ->where('claimUrl', route('organization-admin.create', $organization)) + ->where('settingsUrl', null) ->where('admins.0.id', $first->id) ->where('admins.0.handle', $first->profile->handle) ->where('admins.1.id', $second->id) @@ -61,6 +64,23 @@ ->has('admins', 2)); }); +it('links its members to the settings of the organization, admins to its profile', function (): void { + $organization = Profile::factory()->organization()->create(['handle' => 'laravel']); + $admin = User::factory()->create(); + $member = User::factory()->create(); + $organization->admins()->attach($admin->id); + $organization->members()->attach($member->id, ['role' => OrganizationRole::Member]); + + $this->actingAs($admin)->get('/@laravel') + ->assertInertia(fn ($page) => $page->where('settingsUrl', route('organization-profile.edit', $organization))); + + $this->actingAs($member)->get('/@laravel') + ->assertInertia(fn ($page) => $page->where('settingsUrl', route('organization-member.index', $organization))); + + $this->actingAs(User::factory()->create())->get('/@laravel') + ->assertInertia(fn ($page) => $page->where('settingsUrl', null)); +}); + it('shows guests the link preview of the organization', function (): void { $laravel = Profile::factory()->organization()->create([ 'handle' => 'laravel', diff --git a/tests/Feature/Controllers/OrganizationMemberControllerTest.php b/tests/Feature/Controllers/OrganizationMemberControllerTest.php new file mode 100644 index 000000000..fc8f0250e --- /dev/null +++ b/tests/Feature/Controllers/OrganizationMemberControllerTest.php @@ -0,0 +1,251 @@ +organization()->create(['handle' => 'laravel']); + $admin = User::factory()->create(); + $member = User::factory()->create(); + $organization->admins()->attach($admin->id, ['created_at' => now()->subDay()]); + $organization->members()->attach($member->id, ['role' => OrganizationRole::Member, 'created_at' => now()]); + + return [$organization, $admin, $member]; +} + +/** + * @return array{organization: string, member: string} + */ +function memberRoute(Profile $organization, User $member): array +{ + return ['organization' => $organization->id, 'member' => $member->handle]; +} + +it('lists the members of the organization to its members, first joined first, with their roles', function (): void { + $this->freezeSecond(); + [$organization, $admin, $member] = organizationWithAdminAndMember(); + + $this->actingAs($member) + ->get(route('organization-member.index', $organization)) + ->assertOk() + ->assertInertia(fn ($page) => $page + ->component('organization-member/index') + ->where('organization.id', $organization->id) + ->where('role', 'member') + ->has('members', 2) + ->where('members.0.handle', $admin->handle) + ->where('members.0.role', 'admin') + ->where('members.0.joined_at', now()->subDay()->toJSON()) + ->where('members.1.handle', $member->handle) + ->where('members.1.role', 'member') + ->missing('members.0.pivot') + ->missing('members.0.email')); +}); + +it('hides the members of the organization from everyone else', function (): void { + [$organization] = organizationWithAdminAndMember(); + $user = User::factory()->create(); + + $this->actingAs($user)->get(route('organization-member.index', $organization))->assertNotFound(); + $this->actingAs($user)->get(route('organization-member.index', $user->profile))->assertNotFound(); +}); + +it('lets admins add a member by their handle, whatever its case and with its @', function (): void { + [$organization, $admin] = organizationWithAdminAndMember(); + $user = User::factory()->create(); + + $this->actingAs($admin) + ->from(route('organization-member.index', $organization)) + ->post(route('organization-member.store', $organization), ['handle' => ' @'.mb_strtoupper($user->handle)]) + ->assertRedirect(route('organization-member.index', $organization)) + ->assertSessionHasNoErrors(); + + expect($organization->roleOf($user))->toBe(OrganizationRole::Member); +}); + +it('refuses to add someone who is not on /dev/null or already a member', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + + $this->actingAs($admin) + ->post(route('organization-member.store', $organization), ['handle' => '']) + ->assertSessionHasErrors(['handle' => 'The handle field is required.']); + + $this->actingAs($admin) + ->post(route('organization-member.store', $organization), ['handle' => 'nobody-here']) + ->assertSessionHasErrors(['handle' => "There's no @nobody-here on /dev/null."]); + + $this->actingAs($admin) + ->post(route('organization-member.store', $organization), ['handle' => $organization->handle]) + ->assertSessionHasErrors(['handle' => "There's no @laravel on /dev/null."]); + + $this->actingAs($admin) + ->post(route('organization-member.store', $organization), ['handle' => $member->handle]) + ->assertSessionHasErrors(['handle' => "@{$member->handle} is already a member."]); + + expect($organization->members()->count())->toBe(2); +}); + +it('refuses to add someone across a block, either way', function (): void { + [$organization, $admin] = organizationWithAdminAndMember(); + $blocked = User::factory()->create(); + $blocker = User::factory()->create(); + $admin->blocking()->attach($blocked->id); + $blocker->blocking()->attach($admin->id); + + $this->actingAs($admin) + ->post(route('organization-member.store', $organization), ['handle' => $blocked->handle]) + ->assertSessionHasErrors(['handle' => "You can't add @{$blocked->handle}."]); + + $this->actingAs($admin) + ->post(route('organization-member.store', $organization), ['handle' => $blocker->handle]) + ->assertSessionHasErrors(['handle' => "You can't add @{$blocker->handle}."]); + + expect($organization->members()->count())->toBe(2); +}); + +it('only lets admins add members', function (): void { + [$organization, , $member] = organizationWithAdminAndMember(); + $user = User::factory()->create(); + + $this->actingAs($member)->post(route('organization-member.store', $organization), ['handle' => $user->handle])->assertForbidden(); + $this->actingAs($user)->post(route('organization-member.store', $organization), ['handle' => $user->handle])->assertNotFound(); + + expect($organization->roleOf($user))->toBeNull(); +}); + +it('lets admins make a member an admin, and an admin a member again', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + + $this->actingAs($admin) + ->put(route('organization-member.update', memberRoute($organization, $member)), ['role' => 'admin']) + ->assertSessionHasNoErrors(); + + expect($organization->roleOf($member))->toBe(OrganizationRole::Admin); + + $this->actingAs($member) + ->put(route('organization-member.update', memberRoute($organization, $admin)), ['role' => 'member']) + ->assertSessionHasNoErrors(); + + expect($organization->roleOf($admin))->toBe(OrganizationRole::Member); +}); + +it('keeps the last admin of the organization', function (): void { + [$organization, $admin] = organizationWithAdminAndMember(); + + $this->actingAs($admin) + ->put(route('organization-member.update', memberRoute($organization, $admin)), ['role' => 'member']) + ->assertSessionHasErrors(['role' => '@laravel needs at least one admin.']); + + $this->actingAs($admin) + ->delete(route('organization-member.destroy', memberRoute($organization, $admin))) + ->assertSessionHasErrors(['member' => '@laravel needs at least one admin.']); + + expect($organization->roleOf($admin))->toBe(OrganizationRole::Admin); +}); + +it('keeps the last admin when another admin was made a member after the request was allowed', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + $organization->members()->updateExistingPivot($member->id, ['role' => OrganizationRole::Admin]); + + app()->resolving(UpdateOrganizationMemberRole::class, fn () => $organization->members()->updateExistingPivot($admin->id, ['role' => OrganizationRole::Member])); + + $this->actingAs($admin) + ->from(route('organization-member.index', $organization)) + ->put(route('organization-member.update', memberRoute($organization, $member)), ['role' => 'member']) + ->assertRedirect(route('organization-member.index', $organization)) + ->assertInertiaFlash('toast.type', 'error') + ->assertInertiaFlash('toast.message', '@laravel needs at least one admin.'); + + expect($organization->roleOf($member))->toBe(OrganizationRole::Admin); +}); + +it('keeps the last admin when another admin left after the request was allowed', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + $organization->members()->updateExistingPivot($member->id, ['role' => OrganizationRole::Admin]); + + app()->resolving(DeleteOrganizationMember::class, fn () => $organization->members()->detach($member->id)); + + $this->actingAs($admin) + ->from(route('organization-member.index', $organization)) + ->delete(route('organization-member.destroy', memberRoute($organization, $admin))) + ->assertRedirect(route('organization-member.index', $organization)) + ->assertInertiaFlash('toast.type', 'error') + ->assertInertiaFlash('toast.message', '@laravel needs at least one admin.'); + + expect($organization->roleOf($admin))->toBe(OrganizationRole::Admin); +}); + +it('only lets admins change roles, of members', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + $user = User::factory()->create(); + + $this->actingAs($member)->put(route('organization-member.update', memberRoute($organization, $member)), ['role' => 'admin'])->assertForbidden(); + $this->actingAs($admin)->put(route('organization-member.update', memberRoute($organization, $user)), ['role' => 'admin'])->assertNotFound(); + $this->actingAs($admin)->put(route('organization-member.update', memberRoute($organization, $member)), ['role' => 'owner'])->assertSessionHasErrors('role'); + + expect($organization->roleOf($member))->toBe(OrganizationRole::Member) + ->and($organization->roleOf($user))->toBeNull(); +}); + +it('never lets admins make themselves admins, which would undo another admin making them a member meanwhile', function (): void { + [$organization, $admin] = organizationWithAdminAndMember(); + + $this->actingAs($admin) + ->put(route('organization-member.update', memberRoute($organization, $admin)), ['role' => 'admin']) + ->assertForbidden(); + + expect($organization->roleOf($admin))->toBe(OrganizationRole::Admin); +}); + +it('lets admins remove members', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + + $this->actingAs($admin) + ->from(route('organization-member.index', $organization)) + ->delete(route('organization-member.destroy', memberRoute($organization, $member))) + ->assertRedirect(route('organization-member.index', $organization)); + + expect($organization->roleOf($member))->toBeNull(); +}); + +it('lets members leave, back to the page of the organization', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + + $this->actingAs($member) + ->delete(route('organization-member.destroy', memberRoute($organization, $member))) + ->assertRedirect(route('user.show', 'laravel')) + ->assertInertiaFlash('toast.message', 'You left @laravel.'); + + expect($organization->roleOf($member))->toBeNull() + ->and($organization->roleOf($admin))->toBe(OrganizationRole::Admin); +}); + +it('lets an admin leave while another one stays', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + $organization->members()->updateExistingPivot($member->id, ['role' => OrganizationRole::Admin]); + + $this->actingAs($admin) + ->delete(route('organization-member.destroy', memberRoute($organization, $admin))) + ->assertSessionHasNoErrors(); + + expect($organization->roleOf($admin))->toBeNull(); +}); + +it('only lets admins remove someone else', function (): void { + [$organization, $admin, $member] = organizationWithAdminAndMember(); + $user = User::factory()->create(); + + $this->actingAs($member)->delete(route('organization-member.destroy', memberRoute($organization, $admin)))->assertForbidden(); + $this->actingAs($user)->delete(route('organization-member.destroy', memberRoute($organization, $member)))->assertNotFound(); + + expect($organization->members()->count())->toBe(2); +}); diff --git a/tests/Feature/Controllers/OrganizationProfileControllerTest.php b/tests/Feature/Controllers/OrganizationProfileControllerTest.php new file mode 100644 index 000000000..8c0de932e --- /dev/null +++ b/tests/Feature/Controllers/OrganizationProfileControllerTest.php @@ -0,0 +1,32 @@ +create(); + $organization = Profile::factory()->organization()->create(['lane_color' => '#1a2b3c']); + $organization->admins()->attach($user->id); + + $this->actingAs($user) + ->get(route('organization-profile.edit', $organization)) + ->assertOk() + ->assertInertia(fn ($page) => $page + ->component('organization-profile/edit') + ->where('organization.id', $organization->id) + ->where('role', 'admin') + ->where('lane', '#1a2b3c') + ->missing('organization.lane_color')); +}); + +it('hides the profile settings from members who are not admins, and from everyone else', function (): void { + $organization = Profile::factory()->organization()->create(); + $member = User::factory()->create(); + $organization->members()->attach($member->id, ['role' => OrganizationRole::Member]); + + $this->actingAs($member)->get(route('organization-profile.edit', $organization))->assertNotFound(); + $this->actingAs(User::factory()->create())->get(route('organization-profile.edit', $organization))->assertNotFound(); +}); diff --git a/tests/Unit/Actions/CreateOrganizationAdminTest.php b/tests/Unit/Actions/CreateOrganizationAdminTest.php index c70f63417..d14166209 100644 --- a/tests/Unit/Actions/CreateOrganizationAdminTest.php +++ b/tests/Unit/Actions/CreateOrganizationAdminTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); use App\Actions\CreateOrganizationAdmin; +use App\Enums\OrganizationRole; use App\Models\Profile; use App\Models\User; @@ -14,3 +15,13 @@ ->and(resolve(CreateOrganizationAdmin::class)->handle($organization, $second))->toBeFalse() ->and($organization->admins()->pluck('users.id')->all())->toBe([$first->id]); }); + +it('makes a member the admin of a page left without one', function (): void { + $organization = Profile::factory()->organization()->create(); + $member = User::factory()->create(); + $organization->members()->attach($member->id, ['role' => OrganizationRole::Member]); + + expect(resolve(CreateOrganizationAdmin::class)->handle($organization, $member))->toBeTrue() + ->and($organization->roleOf($member))->toBe(OrganizationRole::Admin) + ->and($organization->members()->count())->toBe(1); +}); diff --git a/tests/Unit/Actions/DeleteOrganizationTest.php b/tests/Unit/Actions/DeleteOrganizationTest.php index 81a730aac..6c6ebeaf8 100644 --- a/tests/Unit/Actions/DeleteOrganizationTest.php +++ b/tests/Unit/Actions/DeleteOrganizationTest.php @@ -3,20 +3,21 @@ declare(strict_types=1); use App\Actions\DeleteOrganization; +use App\Enums\OrganizationRole; use App\Models\Profile; use App\Models\User; use Illuminate\Support\Facades\DB; -it('deletes an organization with its admins, keeping the users', function (): void { +it('deletes an organization with its members, keeping the users', function (): void { $user = User::factory()->create(); $organization = Profile::factory()->organization()->create(); $kept = Profile::factory()->organization()->create(); - $organization->admins()->attach($user->id, ['created_at' => now()]); + $organization->members()->attach($user->id, ['role' => OrganizationRole::Member]); $kept->admins()->attach($user->id, ['created_at' => now()]); resolve(DeleteOrganization::class)->handle($organization); expect($organization->fresh())->toBeNull() ->and($user->fresh())->not->toBeNull() - ->and(DB::table('organization_admins')->pluck('organization_id')->all())->toBe([$kept->id]); + ->and(DB::table('organization_members')->pluck('organization_id')->all())->toBe([$kept->id]); }); diff --git a/tests/Unit/Actions/DeleteUserTest.php b/tests/Unit/Actions/DeleteUserTest.php index 90e6ba65e..8715a3d98 100644 --- a/tests/Unit/Actions/DeleteUserTest.php +++ b/tests/Unit/Actions/DeleteUserTest.php @@ -8,6 +8,7 @@ use App\Actions\UpdateCommitHotScores; use App\Contracts\Services\TranslationProvider; use App\Dto\ThreadEngagement; +use App\Enums\OrganizationRole; use App\Models\Commit; use App\Models\CommitImage; use App\Models\Conversation; @@ -189,15 +190,16 @@ ->and(PollVote::query()->sole()->is($keptVote))->toBeTrue(); }); -it('erases the organizations a deleted user claimed, keeping their other admins', function (): void { +it('erases the organization memberships of a deleted user, keeping their other members', function (): void { $user = User::factory()->create(); $other = User::factory()->create(); $organization = Profile::factory()->organization()->create(); - $organization->admins()->attach([$user->id => ['created_at' => now()], $other->id => ['created_at' => now()]]); + $organization->admins()->attach($user->id); + $organization->members()->attach($other->id, ['role' => OrganizationRole::Member]); resolve(DeleteUser::class)->handle($user); - expect($organization->admins()->pluck('users.id')->all())->toBe([$other->id]); + expect($organization->members()->pluck('users.id')->all())->toBe([$other->id]); }); it('erases the devices of a deleted user', function (): void {