From 31171147c7cd419a056f5da00c559f2e3197f436 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Tue, 27 Oct 2026 18:42:13 +0000 Subject: [PATCH 1/2] Manage an org's members and leave it from its thread An org thread's menu now has Members, which opens the web's org settings as a screen: everyone in the org with their role, Rename and Add people for maintainers, a sheet per member to change their role or remove them, and Leave org for anyone. Changes show at once and go to devnull in the background; one devnull turns down goes back with a toast. Add people reuses New org's search, chips and candidate rows, asks devnull who could join this org, and sends the share history choice. New org's picking moved into a PicksPeople concern so both screens share it. Leaving drops the org from the kept inbox, and the thread under the members screen steps back to the inbox when it shows again. Co-Authored-By: Claude Opus 5.5 --- app/Devnull/Client.php | 19 +- app/Devnull/ResponseCache.php | 34 ++ app/NativeComponents/AddOrgMembers.php | 97 ++++ app/NativeComponents/Concerns/PicksPeople.php | 98 ++++ .../Concerns/SearchesPeople.php | 7 +- app/NativeComponents/Conversation.php | 24 +- app/NativeComponents/NewOrg.php | 71 +-- app/NativeComponents/OrgMembers.php | 444 ++++++++++++++++++ .../views/native/add-org-members.blade.php | 50 ++ resources/views/native/new-org.blade.php | 36 +- resources/views/native/org-members.blade.php | 77 +++ .../native/partials/org-member-row.blade.php | 42 ++ .../partials/org-member-sheet.blade.php | 70 +++ .../native/partials/people-results.blade.php | 17 + .../native/partials/picked-people.blade.php | 20 + routes/mobile.php | 4 + tests/Feature/Screens/OrgMembersTest.php | 381 +++++++++++++++ 17 files changed, 1391 insertions(+), 100 deletions(-) create mode 100644 app/NativeComponents/AddOrgMembers.php create mode 100644 app/NativeComponents/Concerns/PicksPeople.php create mode 100644 app/NativeComponents/OrgMembers.php create mode 100644 resources/views/native/add-org-members.blade.php create mode 100644 resources/views/native/org-members.blade.php create mode 100644 resources/views/native/partials/org-member-row.blade.php create mode 100644 resources/views/native/partials/org-member-sheet.blade.php create mode 100644 resources/views/native/partials/people-results.blade.php create mode 100644 resources/views/native/partials/picked-people.blade.php create mode 100644 tests/Feature/Screens/OrgMembersTest.php diff --git a/app/Devnull/Client.php b/app/Devnull/Client.php index 606cfeb2..0b62c5ae 100644 --- a/app/Devnull/Client.php +++ b/app/Devnull/Client.php @@ -338,14 +338,16 @@ public function remotes(?string $tab = null, ?string $before = null): array /** * People matching a term, each with the web's reason you can't message - * them, or add them to a new org, when there is one. Never kept: who you - * can reach changes as people follow, block and turn Remotes off. + * them, or add them to a new org or the org $orgId, when there is one. + * Never kept: who you can reach changes as people follow, block and turn + * Remotes off. * * @return list */ - public function remoteCandidates(string $term, string $for): array + public function remoteCandidates(string $term, string $for, ?string $orgId = null): array { - $candidates = $this->json($this->send('GET', self::RemoteCandidatesPath, ['q' => $term, 'for' => $for]))['candidates'] ?? null; + $query = $orgId === null ? ['q' => $term, 'for' => $for] : ['q' => $term, 'org' => $orgId]; + $candidates = $this->json($this->send('GET', self::RemoteCandidatesPath, $query))['candidates'] ?? null; return array_values(array_map( fn (array $candidate): array => [ @@ -369,6 +371,15 @@ public function createOrg(?string $name, array $members): string return is_array($org) && is_string($org['id'] ?? null) ? $org['id'] : ''; } + /** + * Where an org's members are added, and where one of them is given a role or taken out, which is how you leave + * it too, by your own username. + */ + public static function orgMembersPath(string $orgId, ?string $username = null): string + { + return self::threadPath(orgId: $orgId).'/members'.($username === null ? '' : '/'.rawurlencode($username)); + } + /** * The messages of a Remotes thread before one, as the web loads them when * you scroll back. diff --git a/app/Devnull/ResponseCache.php b/app/Devnull/ResponseCache.php index 472875af..1c26a6f2 100644 --- a/app/Devnull/ResponseCache.php +++ b/app/Devnull/ResponseCache.php @@ -692,6 +692,40 @@ private function rememberDeletion(string $id): void } } + /** + * You left an org: devnull no longer shows it to you, so it leaves every + * kept page of your inbox, and its thread goes. The thread's screen, + * still under the one you left from, steps back to the inbox when it + * next shows (see takeLeftOrg()). + */ + public function orgLeft(string $orgId): void + { + $userId = $this->userId(); + + if ($userId === null) { + return; + } + + foreach ($this->index($userId) as $meta) { + if ($meta['path'] === Client::RemotesPath) { + $this->patch($meta['path'], $meta['query'], fn (array $json): array => [...$json, 'data' => self::without($json['data'] ?? [], $orgId)]); + } + } + + $this->forget(Client::threadPath(orgId: $orgId).'/messages'); + $this->store()->forever("{$userId} left {$orgId}", true); + } + + /** + * Whether you left the org since its thread last asked, which asking forgets. + */ + public function takeLeftOrg(string $orgId): bool + { + $userId = $this->userId(); + + return $userId !== null && $this->store()->pull("{$userId} left {$orgId}") === true; + } + public function appleLinkChanged(bool $linked): void { $this->patch('me', [], fn (array $json): array => [...$json, 'apple_linked' => $linked]); diff --git a/app/NativeComponents/AddOrgMembers.php b/app/NativeComponents/AddOrgMembers.php new file mode 100644 index 00000000..ddd2d09b --- /dev/null +++ b/app/NativeComponents/AddOrgMembers.php @@ -0,0 +1,97 @@ +data('members'); + $this->memberCount = is_int($members) && $members > 0 ? $members : 1; + } + + public function navTitle(): string + { + return 'Add people'; + } + + public function updatedQuery(): void + { + $this->searchPeople('org', $this->org); + } + + public function toggleSharesHistory(): void + { + $this->sharesHistory = ! $this->sharesHistory; + } + + public function add(): void + { + if ($this->picked === [] || $this->adding) { + return; + } + + $this->adding = true; + $this->pickError = null; + + $this->writeInBackground('POST', Client::orgMembersPath($this->org), ['members' => $this->pickedUsernames(), 'share_history' => $this->sharesHistory], function (): void { + $this->back(); + }, function (string $message): void { + $this->pickError = $message; + $this->adding = false; + }); + } + + public function render(): View + { + return view('native.add-org-members', [ + 'term' => $this->term(), + 'total' => $this->total(), + 'isFull' => $this->isFull(), + 'myColour' => $this->myColour(), + 'rows' => $this->candidateRows(), + ]); + } + + protected function alreadyIn(): int + { + return $this->memberCount; + } +} diff --git a/app/NativeComponents/Concerns/PicksPeople.php b/app/NativeComponents/Concerns/PicksPeople.php new file mode 100644 index 00000000..579d6849 --- /dev/null +++ b/app/NativeComponents/Concerns/PicksPeople.php @@ -0,0 +1,98 @@ + */ + public array $picked = []; + + /** + * Why devnull wouldn't take them, in its words, beside the button as on the web. + */ + public ?string $pickError = null; + + /** + * How many are in the org before anyone picked: you, for a new one. + */ + abstract protected function alreadyIn(): int; + + /** + * Picks someone, or takes them back out. + */ + public function toggle(string $username): void + { + $this->pickError = null; + + if ($this->isPicked($username)) { + $this->picked = array_values(array_filter($this->picked, fn (User $person): bool => $person->username !== $username)); + + return; + } + + $candidate = $this->candidate($username); + + if ($candidate === null || $candidate['reason'] !== null || $this->isFull()) { + return; + } + + $this->picked = [...$this->picked, $candidate['user']]; + } + + /** + * Everyone the org would hold with the people picked. + */ + protected function total(): int + { + return $this->alreadyIn() + count($this->picked); + } + + protected function isFull(): bool + { + return $this->total() >= NewOrg::MaxMembers; + } + + /** + * @return list + */ + protected function pickedUsernames(): array + { + return array_map(fn (User $person): string => $person->username, $this->picked); + } + + /** + * Who the search found, picked or not, and why each one who can't be picked can't. + * + * @return list + */ + protected function candidateRows(): array + { + $isFull = $this->isFull(); + + return array_map(fn (array $candidate): array => [ + 'user' => $candidate['user'], + 'isPicked' => $this->isPicked($candidate['user']->username), + 'reason' => $candidate['reason'] ?? ($isFull && ! $this->isPicked($candidate['user']->username) ? 'the org is full ('.NewOrg::MaxMembers.')' : null), + ], $this->candidates); + } + + private function isPicked(string $username): bool + { + foreach ($this->picked as $person) { + if ($person->username === $username) { + return true; + } + } + + return false; + } +} diff --git a/app/NativeComponents/Concerns/SearchesPeople.php b/app/NativeComponents/Concerns/SearchesPeople.php index 85a938d1..b45b753b 100644 --- a/app/NativeComponents/Concerns/SearchesPeople.php +++ b/app/NativeComponents/Concerns/SearchesPeople.php @@ -31,9 +31,10 @@ protected function term(): string } /** - * Who could start a remote with you, or join a new org: `$for` is "remote" or "org". + * Who could start a remote with you, or join a new org: `$for` is "remote" or "org". With `$orgId`, who could + * join that org. */ - protected function searchPeople(string $for): void + protected function searchPeople(string $for, ?string $orgId = null): void { $term = $this->term(); @@ -44,7 +45,7 @@ protected function searchPeople(string $for): void return; } - $found = $this->attempt(fn (Client $devnull): array => $devnull->remoteCandidates($term, $for)); + $found = $this->attempt(fn (Client $devnull): array => $devnull->remoteCandidates($term, $for, $orgId)); if ($found === null) { return; diff --git a/app/NativeComponents/Conversation.php b/app/NativeComponents/Conversation.php index 284a684d..ab268289 100644 --- a/app/NativeComponents/Conversation.php +++ b/app/NativeComponents/Conversation.php @@ -9,6 +9,7 @@ use App\Devnull\Data\User; use App\Devnull\Display; use App\Devnull\Outbox; +use App\Devnull\ResponseCache; use App\Devnull\SiteLink; use App\Icons\Lucide; use App\Models\PendingWrite; @@ -208,6 +209,13 @@ public function mount(): void public function onResume(): void { + // You left the org from its members: back to the inbox, which no longer lists it. + if ($this->isOrg() && app(ResponseCache::class)->takeLeftOrg($this->org)) { + $this->back(); + + return; + } + $this->forgetRefreshes(); $this->showKept(); $this->showPending(); @@ -229,9 +237,9 @@ public function navigationOptions(): ?NavBarOptions $items = []; - if (! $this->isOrg()) { - $items[] = NavAction::make('view-profile')->label('View profile')->icon(ios: Lucide::UserRound->ios(), android: Lucide::UserRound->android())->press('viewProfile'); - } + $items[] = $this->isOrg() + ? NavAction::make('members')->label('Members')->icon(ios: Lucide::UsersRound->ios(), android: Lucide::UsersRound->android())->press('viewMembers') + : NavAction::make('view-profile')->label('View profile')->icon(ios: Lucide::UserRound->ios(), android: Lucide::UserRound->android())->press('viewProfile'); $items[] = $this->thread->isArchived ? NavAction::make('pop')->label('Pop')->icon(ios: Lucide::ArchiveRestore->ios(), android: Lucide::ArchiveRestore->android())->press('pop') @@ -494,6 +502,16 @@ public function viewProfile(): void } } + /** + * The org's members, where a maintainer manages them and anyone can leave. + */ + public function viewMembers(): void + { + if ($this->isOrg()) { + $this->openInTab('/orgs/'.rawurlencode($this->org).'/members'); + } + } + public function openProfile(string $username): void { $this->openInTab('/users/'.rawurlencode($username)); diff --git a/app/NativeComponents/NewOrg.php b/app/NativeComponents/NewOrg.php index dfda2d95..c481b2fd 100644 --- a/app/NativeComponents/NewOrg.php +++ b/app/NativeComponents/NewOrg.php @@ -3,11 +3,11 @@ namespace App\NativeComponents; use App\Devnull\Client; -use App\Devnull\Data\User; use App\Devnull\Exceptions\ApiException; use App\Devnull\Exceptions\KeychainUnavailable; use App\Devnull\Exceptions\Unauthenticated; use App\NativeComponents\Concerns\OpensInTab; +use App\NativeComponents\Concerns\PicksPeople; use App\NativeComponents\Concerns\SearchesPeople; use App\NativeComponents\Concerns\SyncsOutbox; use App\NativeComponents\Concerns\TalksToDevnull; @@ -23,6 +23,7 @@ class NewOrg extends NativeComponent { use OpensInTab; + use PicksPeople; use SearchesPeople; use SyncsOutbox; use TalksToDevnull; @@ -36,16 +37,8 @@ class NewOrg extends NativeComponent public string $name = ''; - /** @var list */ - public array $picked = []; - public bool $creating = false; - /** - * Why devnull wouldn't create it, in its words, beside the button as on the web. - */ - public ?string $createError = null; - public function navTitle(): string { return 'New org'; @@ -56,28 +49,6 @@ public function updatedQuery(): void $this->searchPeople('org'); } - /** - * Picks someone, or takes them back out. Someone who can't join, or anyone once the org is full, can't be picked. - */ - public function toggle(string $username): void - { - $this->createError = null; - - if ($this->isPicked($username)) { - $this->picked = array_values(array_filter($this->picked, fn (User $person): bool => $person->username !== $username)); - - return; - } - - $candidate = $this->candidate($username); - - if ($candidate === null || $candidate['reason'] !== null || $this->isFull()) { - return; - } - - $this->picked = [...$this->picked, $candidate['user']]; - } - public function create(): void { if ($this->picked === [] || $this->creating) { @@ -85,24 +56,24 @@ public function create(): void } $this->creating = true; - $this->createError = null; + $this->pickError = null; try { $id = app(Client::class)->createOrg( trim($this->name) === '' ? null : trim($this->name), - array_map(fn (User $person): string => $person->username, $this->picked), + $this->pickedUsernames(), ); } catch (Unauthenticated $exception) { $this->signOut($exception); return; } catch (ApiException $exception) { - $this->createError = $exception->firstError(); + $this->pickError = $exception->firstError(); $this->creating = false; return; } catch (KeychainUnavailable $exception) { - $this->createError = $exception->getMessage(); + $this->pickError = $exception->getMessage(); $this->creating = false; return; @@ -113,34 +84,20 @@ public function create(): void public function render(): View { - $isFull = $this->isFull(); - return view('native.new-org', [ 'term' => $this->term(), - 'total' => 1 + count($this->picked), - 'isFull' => $isFull, + 'total' => $this->total(), + 'isFull' => $this->isFull(), 'myColour' => $this->myColour(), - 'rows' => array_map(fn (array $candidate): array => [ - 'user' => $candidate['user'], - 'isPicked' => $this->isPicked($candidate['user']->username), - 'reason' => $candidate['reason'] ?? ($isFull && ! $this->isPicked($candidate['user']->username) ? 'the org is full ('.self::MaxMembers.')' : null), - ], $this->candidates), + 'rows' => $this->candidateRows(), ]); } - private function isPicked(string $username): bool - { - foreach ($this->picked as $person) { - if ($person->username === $username) { - return true; - } - } - - return false; - } - - private function isFull(): bool + /** + * You, since you maintain it. + */ + protected function alreadyIn(): int { - return 1 + count($this->picked) >= self::MaxMembers; + return 1; } } diff --git a/app/NativeComponents/OrgMembers.php b/app/NativeComponents/OrgMembers.php new file mode 100644 index 00000000..9db14e10 --- /dev/null +++ b/app/NativeComponents/OrgMembers.php @@ -0,0 +1,444 @@ + */ + public array $members = []; + + /** + * Your role in the org: a maintainer, or a contributor. + */ + public ?string $role = null; + + public bool $loaded = false; + + /** + * The name in the rename field. + */ + public string $name = ''; + + public bool $renaming = false; + + /** + * Whose sheet is open, by username. + */ + public ?string $managing = null; + + /** + * The sheet is asking whether to remove them. + */ + public bool $confirmingRemoval = false; + + /** + * The members whose role or place devnull hasn't answered for yet, which a refresh leaves alone. + * + * @var array + */ + public array $changing = []; + + public bool $leaving = false; + + public function mount(): void + { + $this->showKept(); + $this->revalidate(); + } + + /** + * Back from Add people or a profile: take what's kept, then ask devnull. + */ + public function onResume(): void + { + $this->forgetRefreshes(); + $this->showKept(); + $this->revalidate(); + } + + public function navTitle(): string + { + return $this->orgName ?? 'Org'; + } + + public function navigationOptions(): ?NavBarOptions + { + return NavBarOptions::make()->subtitle($this->loaded ? count($this->members).'/'.NewOrg::MaxMembers.' members' : null); + } + + protected function revalidate(): void + { + $this->refresh($this->messagesPath(), [], fn (array $json) => $this->take($json)); + } + + protected function showsContent(): bool + { + return $this->loaded; + } + + /** + * Renames the org, or takes its name away when the field is empty, as the web's Rename does. + */ + public function rename(?string $submitted = null): void + { + $name = trim($submitted ?? $this->name); + + if (! $this->isMaintainer() || $this->renaming || $name === ($this->orgName ?? '')) { + return; + } + + $before = $this->orgName; + $this->orgName = $name === '' ? null : $name; + $this->name = $name; + $this->renaming = true; + + $this->writeInBackground('PUT', $this->threadPath(), ['name' => $name], function (): void { + $this->renaming = false; + }, function (string $message) use ($before): void { + $this->orgName = $before; + $this->name = $before ?? ''; + $this->renaming = false; + Toasts::failed($message); + }); + } + + /** + * Opens a member's sheet, for a maintainer, on anyone but themselves. + */ + public function manage(string $username): void + { + if ($this->canManage($username)) { + $this->managing = $username; + $this->confirmingRemoval = false; + } + } + + public function sheetClosed(): void + { + $this->managing = null; + $this->confirmingRemoval = false; + } + + public function makeMaintainer(string $username): void + { + $this->changeRole($username, self::Maintainer); + } + + public function makeContributor(string $username): void + { + $this->changeRole($username, self::Contributor); + } + + public function viewProfile(string $username): void + { + $this->sheetClosed(); + $this->openInTab('/users/'.rawurlencode($username)); + } + + /** + * Remove from org, from a member's sheet, which then asks whether to, in the sheet itself: iOS drops an alert + * raised over a sheet. + */ + public function confirmRemove(string $username): void + { + if ($this->managing === $username && $this->canManage($username)) { + $this->confirmingRemoval = true; + } + } + + public function cancelRemove(): void + { + $this->confirmingRemoval = false; + } + + /** + * Leave org, in the words of the web's confirmation. + */ + public function confirmLeave(): void + { + if ($this->leaving || $this->me() === null) { + return; + } + + Dialog::alert("Leave {$this->title()}?", 'You stop seeing the org and its history. A maintainer would have to add you back. If you are its only maintainer, the longest-standing member takes over.', [ + ['label' => 'Cancel', 'style' => 'cancel'], + ['label' => 'Leave', 'style' => 'destructive'], + ])->id(self::LeaveAlert.':'.$this->org); + } + + #[On(ButtonPressed::class)] + public function onAlertButton(string $label = '', ?string $id = null): void + { + if ($id === self::LeaveAlert.':'.$this->org && $label === 'Leave') { + $this->leave(); + } + } + + /** + * Takes someone out of the org straight away, and puts them back where they were if devnull says no. + */ + public function remove(string $username): void + { + $index = $this->memberIndex($username); + + $this->sheetClosed(); + + if ($index === null || ! $this->canManage($username)) { + return; + } + + $member = $this->members[$index]; + $this->members = array_values(array_filter($this->members, fn (array $kept): bool => $kept['user']->username !== $username)); + $this->changing[$username] = true; + + $this->writeInBackground('DELETE', Client::orgMembersPath($this->org, $username), null, function () use ($username): void { + unset($this->changing[$username]); + $this->revalidate(); + }, function (string $message) use ($username, $member, $index): void { + unset($this->changing[$username]); + array_splice($this->members, min($index, count($this->members)), 0, [$member]); + Toasts::failed($message); + }); + } + + /** + * Leaves the org once devnull has taken it, then goes back to the inbox, which no longer lists it. + */ + public function leave(): void + { + $me = $this->me(); + + if ($me === null || $this->leaving) { + return; + } + + $this->leaving = true; + + $this->writeInBackground('DELETE', Client::orgMembersPath($this->org, $me->username), null, function (): void { + app(ResponseCache::class)->orgLeft($this->org); + $this->back(); + }, function (string $message): void { + $this->leaving = false; + Toasts::failed($message); + }); + } + + /** + * Add people, for a maintainer while the org has room. + */ + public function addPeople(): void + { + if ($this->isMaintainer() && count($this->members) < NewOrg::MaxMembers) { + $this->openInTab('/orgs/'.rawurlencode($this->org).'/members/add', ['members' => count($this->members)]); + } + } + + public function render(): View + { + $me = $this->me(); + $isMaintainer = $this->isMaintainer(); + $managed = $this->managing === null ? null : $this->memberIndex($this->managing); + + return view('native.org-members', [ + 'isMaintainer' => $isMaintainer, + 'isFull' => count($this->members) >= NewOrg::MaxMembers, + 'canRename' => $isMaintainer && ! $this->renaming && trim($this->name) !== ($this->orgName ?? ''), + 'myColour' => $this->myColour(), + 'rows' => array_map(fn (array $member): array => [ + 'user' => $member['user'], + 'role' => $member['role'] ?? self::Contributor, + 'isMe' => $member['user']->id === $me?->id, + 'canManage' => $this->canManage($member['user']->username), + 'isChanging' => isset($this->changing[$member['user']->username]), + ], $this->members), + 'managed' => $managed === null ? null : $this->members[$managed], + 'isLastMaintainer' => $managed !== null && ($this->members[$managed]['role'] ?? null) === self::Maintainer && $this->maintainers() === 1, + ]); + } + + /** + * The org as devnull sent its thread. A member whose change is still on its way stays as the change left them. + * + * @param array $json + */ + private function take(array $json): void + { + $thread = Thread::fromArray($json); + + if (! $thread->isOrg()) { + return; + } + + if (! $this->loaded || trim($this->name) === ($this->orgName ?? '')) { + $this->name = $thread->orgName ?? ''; + } + + $this->orgName = $this->renaming ? $this->orgName : $thread->orgName; + $this->role = $thread->role; + $this->loaded = true; + + if ($this->changing === []) { + $this->members = array_map(fn (array $member): array => ['user' => $member['user'], 'role' => $member['role']], $thread->members); + } + + if ($this->managing !== null && ! $this->canManage($this->managing)) { + $this->sheetClosed(); + } + } + + /** + * Gives someone a role straight away, and puts their old one back if devnull says no. An org keeps at least one + * maintainer. + */ + private function changeRole(string $username, string $role): void + { + $this->sheetClosed(); + $index = $this->memberIndex($username); + + if ($index === null || ! $this->canManage($username) || ($this->members[$index]['role'] ?? self::Contributor) === $role) { + return; + } + + if ($role === self::Contributor && $this->maintainers() === 1) { + Toasts::failed('An org needs at least one maintainer.'); + + return; + } + + $before = $this->members[$index]['role']; + $this->setRoleOf($username, $role); + $this->changing[$username] = true; + + $this->writeInBackground('PUT', Client::orgMembersPath($this->org, $username), ['role' => $role], function () use ($username): void { + unset($this->changing[$username]); + $this->revalidate(); + }, function (string $message) use ($username, $before): void { + unset($this->changing[$username]); + $this->setRoleOf($username, $before); + Toasts::failed($message); + }); + } + + private function setRoleOf(string $username, ?string $role): void + { + $this->members = array_map( + fn (array $member): array => $member['user']->username === $username ? [...$member, 'role' => $role] : $member, + $this->members, + ); + } + + private function showKept(): void + { + if (($json = $this->cached($this->messagesPath())) !== null) { + $this->take($json); + } + } + + private function isMaintainer(): bool + { + return $this->role === self::Maintainer; + } + + /** + * Whether you can change someone's role or remove them: you maintain the org, and they're someone else in it. + */ + private function canManage(string $username): bool + { + return $this->isMaintainer() && $username !== $this->me()?->username && $this->memberIndex($username) !== null; + } + + private function memberIndex(string $username): ?int + { + foreach ($this->members as $index => $member) { + if ($member['user']->username === $username) { + return $index; + } + } + + return null; + } + + private function maintainers(): int + { + return count(array_filter($this->members, fn (array $member): bool => $member['role'] === self::Maintainer)); + } + + /** + * What the confirmations call the org: its name, or else its other members' first names, as its thread is titled. + */ + private function title(): string + { + if ($this->orgName !== null) { + return $this->orgName; + } + + $viewerId = $this->me()?->id; + $others = array_filter($this->members, fn (array $member): bool => $member['user']->id !== $viewerId); + $names = array_map(fn (array $member): string => (string) strtok($member['user']->name, ' '), $others); + + return $names === [] ? 'the org' : implode(', ', $names); + } + + private function threadPath(): string + { + return Client::threadPath(orgId: $this->org); + } + + private function messagesPath(): string + { + return $this->threadPath().'/messages'; + } +} diff --git a/resources/views/native/add-org-members.blade.php b/resources/views/native/add-org-members.blade.php new file mode 100644 index 00000000..2d174091 --- /dev/null +++ b/resources/views/native/add-org-members.blade.php @@ -0,0 +1,50 @@ +{{-- + The web's Add people dialog as a screen, laid out as New org: how full the org would be, a field to find people, + who you've picked as chips you tap to take back out, then who matches. Share history and Add ride above the + keyboard in the bottom bar, with devnull's reason beside Add when it says no. +--}} + + @include('native.partials.error') + + + + They join as contributors. + {{ $total }}/{{ \App\NativeComponents\NewOrg::MaxMembers }} + + @include('native.partials.people-search', ['placeholder' => 'Find people by name or @handle', 'autofocus' => true]) + @include('native.partials.picked-people') + + + + + @include('native.partials.people-results') + {{-- The bottom bar sits over the end of the list, so the list clears it and the home indicator. --}} + + + + + + + {{-- The web's checkbox, drawn, since a system switch in the bottom bar doesn't take taps on iOS. --}} + + + + @if ($sharesHistory) + + @endif + + + Share history + They can read everything pushed before they joined. + + + + + {{ $pickError ?? '' }} + + {{ trim('Add '.($picked === [] ? '' : count($picked))) }} + + + + + diff --git a/resources/views/native/new-org.blade.php b/resources/views/native/new-org.blade.php index 1a19086e..44fc6974 100644 --- a/resources/views/native/new-org.blade.php +++ b/resources/views/native/new-org.blade.php @@ -22,42 +22,12 @@ class="w-full text-theme-ink" /> @include('native.partials.people-search', ['placeholder' => 'Find people by name or @handle', 'autofocus' => true]) - - @if ($picked === []) - nobody picked yet - @else - - - @foreach ($picked as $person) - - - @include('native.partials.avatar', ['user' => $person, 'size' => 20]) - {{ $person->handle() }} - - - - @endforeach - - - @endif - + @include('native.partials.picked-people') - @if ($term === '') - - Type a name or handle - - @elseif ($searched === $term && $rows === []) - - fatal: nothing matches '{{ $term }}' - - @elseif ($searched === $term) - @foreach ($rows as $row) - @include('native.partials.candidate-row', $row + ['press' => 'toggle']) - @endforeach - @endif + @include('native.partials.people-results') {{-- The bottom bar sits over the end of the list, so the list clears it and the home indicator. --}} @@ -66,7 +36,7 @@ class="w-full text-theme-ink" - {{ $createError ?? '' }} + {{ $pickError ?? '' }} Create org diff --git a/resources/views/native/org-members.blade.php b/resources/views/native/org-members.blade.php new file mode 100644 index 00000000..89fbecd4 --- /dev/null +++ b/resources/views/native/org-members.blade.php @@ -0,0 +1,77 @@ +{{-- + The web's org members dialog as a screen, laid out like the Safety settings: for a maintainer, the org's name with + Rename beside it, then everyone in a hairline box, with Add people at its top, then Leave org. The name and how full + the org is sit in the top bar. A member's sheet holds what a maintainer can do to them, as the web's menu does. +--}} + + + + @include('native.partials.error') + @include('native.partials.refresh-failed') + + @if ($isMaintainer) + + + + + + Rename + + + @endif + + @if ($loaded) + + @if ($isMaintainer) + + + + + + + Add people + {{ $isFull ? 'the org is full ('.\App\NativeComponents\NewOrg::MaxMembers.')' : 'they join as contributors' }} + + + + @endif + @foreach ($rows as $row) + @if ($isMaintainer || ! $loop->first) + + @endif + @include('native.partials.org-member-row', $row + ['showsSlot' => $isMaintainer]) + @endforeach + + + + Leave org + + @elseif ($error === null) + + @foreach (range(1, 3) as $skeleton) + @if (! $loop->first) + + @endif + + + + + + + + @endforeach + + @endif + + + + @include('native.partials.org-member-sheet') + diff --git a/resources/views/native/partials/org-member-row.blade.php b/resources/views/native/partials/org-member-row.blade.php new file mode 100644 index 00000000..f9f790da --- /dev/null +++ b/resources/views/native/partials/org-member-row.blade.php @@ -0,0 +1,42 @@ +{{-- + Someone in an org, as the web's members dialog lists them: their avatar ringed in their lane, their name, "(you)" + on your own row, their handle, and their role. A tap opens their profile. For a maintainer, a slot on the right + holds the button that opens their sheet, or a spinner while a change to them is on its way, and stays empty on + your own row so the roles line up. +--}} + + + + + + @include('native.partials.avatar', ['user' => $user, 'size' => 32]) + + + + + {{ $user->name }} + @if ($isMe) + (you) + @endif + + {{ $user->handle() }} + + + {{ $role }} + + + + @if ($showsSlot) + + @if ($isChanging) + + @elseif ($canManage) + + + + + + @endif + + @endif + diff --git a/resources/views/native/partials/org-member-sheet.blade.php b/resources/views/native/partials/org-member-sheet.blade.php new file mode 100644 index 00000000..eecae2a6 --- /dev/null +++ b/resources/views/native/partials/org-member-sheet.blade.php @@ -0,0 +1,70 @@ +{{-- + What a maintainer can do to someone else in the org, as the web's member menu offers it: make them a maintainer or + a contributor, then remove them. The sheet names who it's for, and can open their profile. Make contributor is off + for the last maintainer, since an org needs one. Remove from org asks first, in the sheet itself, since iOS drops + an alert raised over a sheet. +--}} + + @if ($managed) + @php($user = $managed['user']) + @php($isMaintainer = $managed['role'] === \App\NativeComponents\OrgMembers::Maintainer) + + + + @include('native.partials.avatar', ['user' => $user, 'size' => 44]) + + {{ $user->name }} + {{ $user->handle() }} · {{ $managed['role'] ?? \App\NativeComponents\OrgMembers::Contributor }} + + + + + + + @if ($confirmingRemoval) + + + Remove {{ $user->handle() }}? + They stop seeing the org and its history. A maintainer would have to add them back. + + + + Cancel + + + Remove + + + + @else + @if ($isMaintainer) + @if ($isLastMaintainer) + + Make contributor + An org needs at least one maintainer. + + @else + + + Make contributor + + + @endif + @else + + + Make maintainer + + + @endif + + + + + Remove from org + + + @endif + + @endif + diff --git a/resources/views/native/partials/people-results.blade.php b/resources/views/native/partials/people-results.blade.php new file mode 100644 index 00000000..00ab930a --- /dev/null +++ b/resources/views/native/partials/people-results.blade.php @@ -0,0 +1,17 @@ +{{-- + What an org picker's search found, inside its list: a hint before anything is typed, the web's line when nothing + matches, or the people, picked or not, with why anyone who can't be picked can't. +--}} +@if ($term === '') + + Type a name or handle + +@elseif ($searched === $term && $rows === []) + + fatal: nothing matches '{{ $term }}' + +@elseif ($searched === $term) + @foreach ($rows as $row) + @include('native.partials.candidate-row', $row + ['press' => 'toggle']) + @endforeach +@endif diff --git a/resources/views/native/partials/picked-people.blade.php b/resources/views/native/partials/picked-people.blade.php new file mode 100644 index 00000000..fd9705ac --- /dev/null +++ b/resources/views/native/partials/picked-people.blade.php @@ -0,0 +1,20 @@ +{{-- The people an org picker has picked, as chips you tap to take back out, or a line saying nobody is yet. --}} + + @if ($picked === []) + nobody picked yet + @else + + + @foreach ($picked as $person) + + + @include('native.partials.avatar', ['user' => $person, 'size' => 20]) + {{ $person->handle() }} + + + + @endforeach + + + @endif + diff --git a/routes/mobile.php b/routes/mobile.php index f4a63deb..1a5a0ec5 100644 --- a/routes/mobile.php +++ b/routes/mobile.php @@ -1,5 +1,6 @@ 'http://devnull.test']); + signIn(); +}); + +/** + * `GET remotes/orgs/org-1/messages` for dev/null core, with you (local_dev) in it as $role, Ada contributing and + * Grace maintaining. + * + * @param list>|null $members + * @return array + */ +function orgJson(string $role = 'maintainer', ?string $name = 'dev/null core', ?array $members = null): array +{ + return ['data' => [ + 'org' => ['id' => 'org-1', 'name' => $name], + 'messages' => [remoteMessageJson(['id' => 1, 'body' => 'hello org', 'user' => userJson(['id' => 'u2', 'handle' => 'ada', 'name' => 'Ada Lovelace'])])], + 'has_older' => false, + 'reactions_version' => 0, + 'reaction_changes' => [], + 'read_through_id' => 1, + 'members' => $members ?? [ + ['user' => userJson(), 'role' => $role, 'read_through_id' => 1], + ['user' => userJson(['id' => 'u2', 'handle' => 'ada', 'name' => 'Ada Lovelace', 'lane' => 1]), 'role' => 'contributor', 'read_through_id' => 1], + ['user' => userJson(['id' => 'u3', 'handle' => 'grace', 'name' => 'Grace Hopper', 'lane' => 2]), 'role' => 'maintainer', 'read_through_id' => 1], + ], + 'typing' => [], + 'is_archived' => false, + 'role' => $role, + 'access' => 'open', + ]]; +} + +/** + * devnull answering the org's thread with $thread, people searches with $candidates, and every write with the + * status $writes gives its "METHOD path", 204 otherwise. + * + * @param array $thread + * @param array $writes + * @param list> $candidates + */ +function serveOrg(array $thread, array $writes = [], array $candidates = []): void +{ + serveApi(function (Request $request) use ($thread, $writes, $candidates) { + $path = substr((string) parse_url($request->url(), PHP_URL_PATH), strlen('/api/v1/')); + + return match (true) { + $request->method() === 'GET' && $path === 'remotes/orgs/org-1/messages' => Http::response($thread), + $request->method() === 'GET' && $path === 'remotes/candidates' => Http::response(['candidates' => $candidates]), + $request->method() === 'GET' => Http::response(currentUserJson()), + ($writes["{$request->method()} {$path}"] ?? 204) >= 300 => Http::response(['message' => 'Only maintainers can do that.'], $writes["{$request->method()} {$path}"]), + default => Http::response(null, 204), + }; + }); +} + +function writesSent(): array +{ + return collect(Http::recorded()) + ->filter(fn (array $sent): bool => $sent[0]->method() !== 'GET') + ->map(fn (array $sent): string => $sent[0]->method().' '.substr((string) parse_url($sent[0]->url(), PHP_URL_PATH), strlen('/api/v1/'))) + ->values() + ->all(); +} + +it('offers Members from an org thread’s menu, and View profile from a person’s', function () { + serveOrg(orgJson()); + + finishRefreshes(Native::visit('/remotes/orgs/org-1')) + ->assertElement('top_bar_action', fn (array $node): bool => ($node['props']['id'] ?? null) === 'members' && ($node['props']['label'] ?? null) === 'Members') + ->assertMissingElement('top_bar_action', fn (array $node): bool => ($node['props']['id'] ?? null) === 'view-profile') + ->call('viewMembers') + ->assertNavigatedTo('/remotes/orgs/org-1/members'); + + serveApi(fn (Request $request) => Http::response(str_contains($request->url(), 'remotes/@ada/messages') ? threadJson([remoteMessageJson()]) : ['usage' => []])); + + finishRefreshes(Native::visit('/remotes/conversations/ada')) + ->assertElement('top_bar_action', fn (array $node): bool => ($node['props']['id'] ?? null) === 'view-profile') + ->assertMissingElement('top_bar_action', fn (array $node): bool => ($node['props']['id'] ?? null) === 'members'); +}); + +it('shows everyone in the org with their role, from what the thread kept, before devnull answers', function () { + keep('remotes/orgs/org-1/messages', orgJson()); + holdRefreshes(); + + Native::visit('/remotes/orgs/org-1/members') + ->assertNavTitle('dev/null core') + ->assertSee('Local Dev') + ->assertSee('(you)') + ->assertSee('@ada') + ->assertSee('Grace Hopper') + ->assertSee('contributor') + ->assertSee('maintainer') + ->assertSee('Leave org'); +}); + +it('calls an org without a name Org, and counts its members against the web’s limit', function () { + serveOrg(orgJson(name: null)); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members'))->assertNavTitle('Org'); + + expect(findNode($screen->tree(), fn (array $node): bool => ($node['props']['nav_subtitle'] ?? null) === '3/'.NewOrg::MaxMembers.' members'))->not->toBeNull(); +}); + +it('lets a maintainer rename, add people, and manage everyone but themselves', function () { + serveOrg(orgJson('maintainer')); + + finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->assertSee('Rename') + ->assertSee('Add people') + ->assertElement('pressable', fn (array $node): bool => ($node['ref'] ?? null) === 'manage-ada') + ->assertElement('pressable', fn (array $node): bool => ($node['ref'] ?? null) === 'manage-grace') + ->assertMissingElement('pressable', fn (array $node): bool => ($node['ref'] ?? null) === 'manage-local_dev'); +}); + +it('leaves a contributor to see who is in the org and leave, and nothing else', function () { + serveOrg(orgJson('contributor')); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->assertDontSee('Rename') + ->assertDontSee('Add people') + ->assertMissingElement('pressable', fn (array $node): bool => str_starts_with((string) ($node['ref'] ?? ''), 'manage-')) + ->assertSee('Leave org'); + + $screen->call('manage', 'ada') + ->assertSet('managing', null) + ->call('rename', 'mine now') + ->call('makeMaintainer', 'ada') + ->call('confirmRemove', 'ada') + ->assertNativeNotCalled('Dialog.Alert') + ->call('remove', 'ada') + ->call('addPeople') + ->assertNoNavigation(); + + expect(writesSent())->toBe([]); +}); + +it('renames the org at once, and takes the name away when the field is empty', function () { + serveOrg(orgJson()); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->set('name', ' core team ') + ->tap('rename-org') + ->assertNavTitle('core team'); + + finishRefreshes($screen)->assertSet('renaming', false); + + expect(json_decode(sentTo('PUT', 'remotes/orgs/org-1')[0], true))->toBe(['name' => 'core team']); + + finishRefreshes($screen->call('rename', ''))->assertNavTitle('Org'); + + expect(json_decode(sentTo('PUT', 'remotes/orgs/org-1')[1], true))->toBe(['name' => '']); +}); + +it('puts the name back with devnull’s reason when it won’t rename the org', function () { + serveOrg(orgJson(), ['PUT remotes/orgs/org-1' => 403]); + + finishRefreshes(finishRefreshes(Native::visit('/remotes/orgs/org-1/members'))->call('rename', 'core team')) + ->assertNavTitle('dev/null core') + ->assertSet('name', 'dev/null core') + ->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'Only maintainers can do that.'); +}); + +it('makes a contributor a maintainer from their sheet', function () { + serveOrg(orgJson()); + holdRefreshes(); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->tap('manage-ada') + ->assertSet('managing', 'ada') + ->assertSee('Make maintainer') + ->assertSee('Remove from org') + ->tap('sheet-make-maintainer') + ->assertSet('managing', null); + + expect($screen->instance()->members[1]['role'])->toBe('maintainer'); + + serveOrg(orgJson(members: [ + ['user' => userJson(), 'role' => 'maintainer', 'read_through_id' => 1], + ['user' => userJson(['id' => 'u2', 'handle' => 'ada', 'name' => 'Ada Lovelace']), 'role' => 'maintainer', 'read_through_id' => 1], + ])); + finishRefreshes($screen); + + expect($screen->instance()->members[1]['role'])->toBe('maintainer') + ->and(writesSent())->toBe(['PUT remotes/orgs/org-1/members/ada']) + ->and(json_decode(sentTo('PUT', 'remotes/orgs/org-1/members/ada')[0], true))->toBe(['role' => 'maintainer']); +}); + +it('offers Make contributor for a maintainer, and puts their role back when devnull says no', function () { + serveOrg(orgJson(), ['PUT remotes/orgs/org-1/members/grace' => 422]); + holdRefreshes(); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->tap('manage-grace') + ->assertSee('Make contributor') + ->tap('sheet-make-contributor'); + + expect($screen->instance()->members[2]['role'])->toBe('contributor'); + + finishRefreshes($screen)->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'Only maintainers can do that.'); + + expect($screen->instance()->members[2]['role'])->toBe('maintainer'); +}); + +it('never leaves an org without a maintainer', function () { + // devnull says you maintain it, but its list has Grace as the only maintainer. + serveOrg(orgJson('maintainer', members: [ + ['user' => userJson(), 'role' => 'contributor', 'read_through_id' => 1], + ['user' => userJson(['id' => 'u3', 'handle' => 'grace', 'name' => 'Grace Hopper']), 'role' => 'maintainer', 'read_through_id' => 1], + ])); + + finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->tap('manage-grace') + ->assertSee('An org needs at least one maintainer.') + ->call('makeContributor', 'grace') + ->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'An org needs at least one maintainer.'); + + expect(writesSent())->toBe([]); +}); + +it('asks in the sheet before removing someone, and removes them only once you say so', function () { + serveOrg(orgJson()); + holdRefreshes(); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->tap('manage-ada') + ->tap('sheet-remove') + ->assertSet('managing', 'ada') + ->assertSee('Remove @ada?') + ->assertSee('They stop seeing the org and its history. A maintainer would have to add them back.') + ->tap('cancel-remove') + ->assertSet('managing', 'ada') + ->assertDontSee('Remove @ada?') + ->assertSee('Ada Lovelace'); + + expect(writesSent())->toBe([]); + + $screen->tap('sheet-remove') + ->tap('confirm-remove') + ->assertSet('managing', null) + ->assertDontSee('Ada Lovelace'); + + serveOrg(orgJson(members: [orgJson()['data']['members'][0], orgJson()['data']['members'][2]])); + finishRefreshes($screen)->assertDontSee('Ada Lovelace'); + + expect(writesSent())->toBe(['DELETE remotes/orgs/org-1/members/ada']); +}); + +it('puts someone back where they were when devnull won’t remove them', function () { + serveOrg(orgJson(), ['DELETE remotes/orgs/org-1/members/ada' => 403]); + + $screen = finishRefreshes(finishRefreshes(Native::visit('/remotes/orgs/org-1/members'))->call('remove', 'ada')) + ->assertSee('Ada Lovelace') + ->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'Only maintainers can do that.'); + + expect(array_map(fn (array $member): string => $member['user']->username, $screen->instance()->members))->toBe(['local_dev', 'ada', 'grace']); +}); + +it('asks before you leave, in the web’s words', function () { + serveOrg(orgJson('contributor')); + + finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) + ->tap('leave-org') + ->assertNativeCalled('Dialog.Alert', fn (array $params): bool => $params['title'] === 'Leave dev/null core?' + && $params['message'] === 'You stop seeing the org and its history. A maintainer would have to add you back. If you are its only maintainer, the longest-standing member takes over.') + ->emitNative(ButtonPressed::class, ['index' => 0, 'label' => 'Cancel', 'id' => 'leave-org:org-1']) + ->assertNoNavigation(); + + expect(writesSent())->toBe([]); +}); + +it('leaves the org, takes it out of the kept inbox, and goes back past its thread to the inbox', function () { + serveOrg(orgJson('contributor')); + keep('remotes', remotesInboxJson([conversationJson(['id' => 'org-1', 'kind' => 'org', 'name' => 'dev/null core']), conversationJson()])); + + $thread = finishRefreshes(Native::visit('/remotes/orgs/org-1')); + $members = $thread->call('viewMembers')->follow(); + + finishRefreshes($members)->emitNative(ButtonPressed::class, ['index' => 1, 'label' => 'Leave', 'id' => 'leave-org:org-1']); + finishRefreshes($members)->assertWentBack(); + + expect(writesSent())->toContain('DELETE remotes/orgs/org-1/members/local_dev') + ->and(array_column(kept('remotes')['data'], 'id'))->toBe(['conv-1']) + ->and(kept('remotes/orgs/org-1/messages'))->toBeNull(); + + $members->goBack()->assertWentBack(); + + // Only the once: the thread opened again later stays. + expect(app(ResponseCache::class)->takeLeftOrg('org-1'))->toBeFalse(); +}); + +it('stays in the org with devnull’s reason when it won’t let you leave', function () { + serveOrg(orgJson('contributor'), ['DELETE remotes/orgs/org-1/members/local_dev' => 422]); + + finishRefreshes(finishRefreshes(Native::visit('/remotes/orgs/org-1/members'))->call('leave')) + ->assertNoNavigation() + ->assertSet('leaving', false) + ->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'Only maintainers can do that.'); + + expect(kept('remotes/orgs/org-1/messages'))->not->toBeNull(); +}); + +it('opens Add people with how many the org holds, and opens a member’s profile from their row', function () { + serveOrg(orgJson()); + + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')); + + $screen->call('addPeople')->assertNavigatedTo('/remotes/orgs/org-1/members/add'); + + expect($screen->navigationIntent()->data)->toBe(['members' => 3]); + + finishRefreshes(Native::visit('/remotes/orgs/org-1/members'))->call('viewProfile', 'ada')->assertNavigatedTo('/remotes/users/ada'); +}); + +it('finds people who could join this org, and adds the ones picked with or without the history', function () { + serveOrg(orgJson(), candidates: [ + ['user' => userJson(['id' => 'u4', 'handle' => 'linus', 'name' => 'Linus Torvalds']), 'reason' => null], + ['user' => userJson(['id' => 'u2', 'handle' => 'ada', 'name' => 'Ada Lovelace']), 'reason' => 'already in the org'], + ]); + + $screen = Native::visit('/remotes/orgs/org-1/members/add', ['members' => 3]) + ->assertNavTitle('Add people') + ->assertSee('They join as contributors.') + ->assertSee('3/'.NewOrg::MaxMembers) + ->set('query', 'a') + ->assertSee('@ada · already in the org') + ->tap('candidate-ada') + ->tap('candidate-linus') + ->assertSee('4/'.NewOrg::MaxMembers) + ->assertSee('Add 1') + ->tap('share-history') + ->assertSet('sharesHistory', true) + ->tap('add-people'); + + $search = collect(Http::recorded())->first(fn (array $sent): bool => str_contains($sent[0]->url(), 'remotes/candidates'))[0]; + + expect($search->url())->toContain('org=org-1')->not->toContain('for='); + + finishRefreshes($screen)->assertWentBack(); + + expect(json_decode(sentTo('POST', 'remotes/orgs/org-1/members')[0], true))->toBe(['members' => ['linus'], 'share_history' => true]); +}); + +it("shows devnull's reason beside Add when it won't add them, and stays", function () { + serveOrg(orgJson(), ['POST remotes/orgs/org-1/members' => 422], [ + ['user' => userJson(['id' => 'u4', 'handle' => 'linus', 'name' => 'Linus Torvalds']), 'reason' => null], + ]); + + $screen = Native::visit('/remotes/orgs/org-1/members/add', ['members' => 3]) + ->set('query', 'linus') + ->tap('candidate-linus') + ->tap('add-people'); + + finishRefreshes($screen) + ->assertSee('Only maintainers can do that.') + ->assertSet('adding', false) + ->assertNoNavigation(); + + expect(json_decode(sentTo('POST', 'remotes/orgs/org-1/members')[0], true))->toBe(['members' => ['linus'], 'share_history' => false]); +}); + +it('says the org is full beside everyone once the people picked would fill it', function () { + serveOrg(orgJson(), candidates: [ + ['user' => userJson(['id' => 'u4', 'handle' => 'linus', 'name' => 'Linus Torvalds']), 'reason' => null], + ]); + + Native::visit('/remotes/orgs/org-1/members/add', ['members' => NewOrg::MaxMembers]) + ->set('query', 'linus') + ->assertSee('@linus · the org is full (100)') + ->tap('candidate-linus') + ->assertSet('picked', []); +}); From 245c91d53c093ec7fca6ca9a79ccb27ab20d2f3d Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Wed, 28 Oct 2026 12:55:40 +0000 Subject: [PATCH 2/2] Open a member's actions from a menu, and confirm removal with an alert More on a member now opens the platform's own menu, as the web's member menu does: Make maintainer or Make contributor (off for the last maintainer), then Remove from org, which asks in an alert. The sheet is gone, and the new user-minus icon draws Remove. Share history on Add people is the system switch again, on the right. Co-Authored-By: Claude Opus 5.5 --- app/Devnull/OrgMemberMenu.php | 45 +++++++++ app/Icons/Lucide.php | 2 + app/NativeComponents/AddOrgMembers.php | 4 +- app/NativeComponents/OrgMembers.php | 93 ++++++++----------- resources/icons/user-minus.svg | 1 + .../views/native/add-org-members.blade.php | 21 ++--- resources/views/native/org-members.blade.php | 4 +- .../native/partials/org-member-row.blade.php | 8 +- .../partials/org-member-sheet.blade.php | 70 -------------- tests/Feature/Screens/OrgMembersTest.php | 62 ++++++------- 10 files changed, 129 insertions(+), 181 deletions(-) create mode 100644 app/Devnull/OrgMemberMenu.php create mode 100644 resources/icons/user-minus.svg delete mode 100644 resources/views/native/partials/org-member-sheet.blade.php diff --git a/app/Devnull/OrgMemberMenu.php b/app/Devnull/OrgMemberMenu.php new file mode 100644 index 00000000..d5d43fd6 --- /dev/null +++ b/app/Devnull/OrgMemberMenu.php @@ -0,0 +1,45 @@ + + */ + public static function items(User $member, ?string $role, bool $isLastMaintainer): array + { + return [ + $role === OrgMembers::Maintainer + ? self::item($member, 'make-contributor', 'Make contributor', Lucide::UserRound, 'makeContributor', disabled: $isLastMaintainer) + : self::item($member, 'make-maintainer', 'Make maintainer', Lucide::Shield, 'makeMaintainer'), + NavAction::divider(), + self::item($member, 'remove', 'Remove from org', Lucide::UserMinus, 'confirmRemove', destructive: true), + ]; + } + + private static function item(User $member, string $action, string $label, Lucide $icon, string $method, bool $destructive = false, bool $disabled = false): TopBarAction + { + return NavAction::make($action) + ->label($label) + ->icon(ios: $icon->ios(), android: $icon->android()) + ->destructive($destructive) + ->disabled($disabled) + ->press("{$method}('{$member->username}')") + ->toElement() + ->ref("{$action}-{$member->username}"); + } +} diff --git a/app/Icons/Lucide.php b/app/Icons/Lucide.php index 92a46513..f9eefb0d 100644 --- a/app/Icons/Lucide.php +++ b/app/Icons/Lucide.php @@ -91,6 +91,7 @@ enum Lucide case PinOff; case BellOff; case LogOut; + case UserMinus; /** * The icon the web's inbox draws for a kind of notification. @@ -194,6 +195,7 @@ public function custom(bool $filled = false): ?string self::PinOff => 'pin-off', self::BellOff => 'bell-off', self::LogOut => 'log-out', + self::UserMinus => 'user-minus', default => null, }; diff --git a/app/NativeComponents/AddOrgMembers.php b/app/NativeComponents/AddOrgMembers.php index ddd2d09b..a04eda74 100644 --- a/app/NativeComponents/AddOrgMembers.php +++ b/app/NativeComponents/AddOrgMembers.php @@ -57,9 +57,9 @@ public function updatedQuery(): void $this->searchPeople('org', $this->org); } - public function toggleSharesHistory(): void + public function setSharesHistory(bool $sharesHistory): void { - $this->sharesHistory = ! $this->sharesHistory; + $this->sharesHistory = $sharesHistory; } public function add(): void diff --git a/app/NativeComponents/OrgMembers.php b/app/NativeComponents/OrgMembers.php index 9db14e10..e680d36a 100644 --- a/app/NativeComponents/OrgMembers.php +++ b/app/NativeComponents/OrgMembers.php @@ -5,6 +5,7 @@ use App\Devnull\Client; use App\Devnull\Data\Thread; use App\Devnull\Data\User; +use App\Devnull\OrgMemberMenu; use App\Devnull\ResponseCache; use App\Devnull\Toasts; use App\NativeComponents\Concerns\OpensInTab; @@ -22,16 +23,16 @@ /** * The web's org members dialog as a screen, opened from an org thread's menu: * everyone in the org with their role, and Leave org for anyone. A maintainer - * also renames the org, adds people, and from a member's sheet makes them a - * maintainer or a contributor, or removes them, as on the web. A contributor - * sees who's in it and can leave, and nothing else. + * also renames the org, adds people, and from More on someone else makes them + * a maintainer or a contributor, or removes them, as the web's member menu + * does (see OrgMemberMenu). A contributor sees who's in it and can leave, and + * nothing else. * * It shows the org as its thread last kept it, then asks devnull behind it. * Each change shows at once and goes to devnull in the background; one that * devnull turns down goes back as it was, with a toast in devnull's words. - * Removing someone asks first, in their sheet, and leaving asks in the web's - * words. Once you've left, the inbox no longer lists the org, and back goes - * past its thread to the inbox. + * Removing someone and leaving ask first. Once you've left, the inbox no + * longer lists the org, and back goes past its thread to the inbox. */ class OrgMembers extends NativeComponent { @@ -46,8 +47,11 @@ class OrgMembers extends NativeComponent public const string Contributor = 'contributor'; /** - * The alert Leave org confirms with. Its id names the org, so an answer only reaches the screen that asked. + * The alerts Remove from org and Leave org confirm with. Each one's id names the org, and the member, so an + * answer only reaches what asked. */ + public const string RemoveAlert = 'remove-member'; + public const string LeaveAlert = 'leave-org'; /** @@ -74,16 +78,6 @@ class OrgMembers extends NativeComponent public bool $renaming = false; - /** - * Whose sheet is open, by username. - */ - public ?string $managing = null; - - /** - * The sheet is asking whether to remove them. - */ - public bool $confirmingRemoval = false; - /** * The members whose role or place devnull hasn't answered for yet, which a refresh leaves alone. * @@ -155,23 +149,6 @@ public function rename(?string $submitted = null): void }); } - /** - * Opens a member's sheet, for a maintainer, on anyone but themselves. - */ - public function manage(string $username): void - { - if ($this->canManage($username)) { - $this->managing = $username; - $this->confirmingRemoval = false; - } - } - - public function sheetClosed(): void - { - $this->managing = null; - $this->confirmingRemoval = false; - } - public function makeMaintainer(string $username): void { $this->changeRole($username, self::Maintainer); @@ -184,24 +161,23 @@ public function makeContributor(string $username): void public function viewProfile(string $username): void { - $this->sheetClosed(); $this->openInTab('/users/'.rawurlencode($username)); } /** - * Remove from org, from a member's sheet, which then asks whether to, in the sheet itself: iOS drops an alert - * raised over a sheet. + * Remove from org, from someone's menu, which asks first. The web doesn't, but taking someone out of the org is + * as hard to undo as leaving it. */ public function confirmRemove(string $username): void { - if ($this->managing === $username && $this->canManage($username)) { - $this->confirmingRemoval = true; + if (! $this->canManage($username)) { + return; } - } - public function cancelRemove(): void - { - $this->confirmingRemoval = false; + Dialog::alert("Remove @{$username}?", 'They stop seeing the org and its history. A maintainer would have to add them back.', [ + ['label' => 'Cancel', 'style' => 'cancel'], + ['label' => 'Remove', 'style' => 'destructive'], + ])->id($this->alertId(self::RemoveAlert, $username)); } /** @@ -216,14 +192,22 @@ public function confirmLeave(): void Dialog::alert("Leave {$this->title()}?", 'You stop seeing the org and its history. A maintainer would have to add you back. If you are its only maintainer, the longest-standing member takes over.', [ ['label' => 'Cancel', 'style' => 'cancel'], ['label' => 'Leave', 'style' => 'destructive'], - ])->id(self::LeaveAlert.':'.$this->org); + ])->id($this->alertId(self::LeaveAlert)); } #[On(ButtonPressed::class)] public function onAlertButton(string $label = '', ?string $id = null): void { - if ($id === self::LeaveAlert.':'.$this->org && $label === 'Leave') { + if ($id === $this->alertId(self::LeaveAlert) && $label === 'Leave') { $this->leave(); + + return; + } + + $prefix = $this->alertId(self::RemoveAlert, ''); + + if ($id !== null && str_starts_with($id, $prefix) && $label === 'Remove') { + $this->remove(substr($id, strlen($prefix))); } } @@ -234,8 +218,6 @@ public function remove(string $username): void { $index = $this->memberIndex($username); - $this->sheetClosed(); - if ($index === null || ! $this->canManage($username)) { return; } @@ -290,7 +272,7 @@ public function render(): View { $me = $this->me(); $isMaintainer = $this->isMaintainer(); - $managed = $this->managing === null ? null : $this->memberIndex($this->managing); + $maintainers = $this->maintainers(); return view('native.org-members', [ 'isMaintainer' => $isMaintainer, @@ -301,11 +283,11 @@ public function render(): View 'user' => $member['user'], 'role' => $member['role'] ?? self::Contributor, 'isMe' => $member['user']->id === $me?->id, - 'canManage' => $this->canManage($member['user']->username), + 'menu' => $this->canManage($member['user']->username) + ? OrgMemberMenu::items($member['user'], $member['role'], $member['role'] === self::Maintainer && $maintainers === 1) + : [], 'isChanging' => isset($this->changing[$member['user']->username]), ], $this->members), - 'managed' => $managed === null ? null : $this->members[$managed], - 'isLastMaintainer' => $managed !== null && ($this->members[$managed]['role'] ?? null) === self::Maintainer && $this->maintainers() === 1, ]); } @@ -334,9 +316,6 @@ private function take(array $json): void $this->members = array_map(fn (array $member): array => ['user' => $member['user'], 'role' => $member['role']], $thread->members); } - if ($this->managing !== null && ! $this->canManage($this->managing)) { - $this->sheetClosed(); - } } /** @@ -345,7 +324,6 @@ private function take(array $json): void */ private function changeRole(string $username, string $role): void { - $this->sheetClosed(); $index = $this->memberIndex($username); if ($index === null || ! $this->canManage($username) || ($this->members[$index]['role'] ?? self::Contributor) === $role) { @@ -432,6 +410,11 @@ private function title(): string return $names === [] ? 'the org' : implode(', ', $names); } + private function alertId(string $alert, ?string $username = null): string + { + return "{$alert}:{$this->org}".($username === null ? '' : ":{$username}"); + } + private function threadPath(): string { return Client::threadPath(orgId: $this->org); diff --git a/resources/icons/user-minus.svg b/resources/icons/user-minus.svg new file mode 100644 index 00000000..236ec538 --- /dev/null +++ b/resources/icons/user-minus.svg @@ -0,0 +1 @@ + diff --git a/resources/views/native/add-org-members.blade.php b/resources/views/native/add-org-members.blade.php index 2d174091..384211e7 100644 --- a/resources/views/native/add-org-members.blade.php +++ b/resources/views/native/add-org-members.blade.php @@ -25,20 +25,13 @@ - {{-- The web's checkbox, drawn, since a system switch in the bottom bar doesn't take taps on iOS. --}} - - - - @if ($sharesHistory) - - @endif - - - Share history - They can read everything pushed before they joined. - - - + + + Share history + They can read everything pushed before they joined. + + + {{ $pickError ?? '' }} diff --git a/resources/views/native/org-members.blade.php b/resources/views/native/org-members.blade.php index 89fbecd4..7f8cbf4b 100644 --- a/resources/views/native/org-members.blade.php +++ b/resources/views/native/org-members.blade.php @@ -1,7 +1,7 @@ {{-- The web's org members dialog as a screen, laid out like the Safety settings: for a maintainer, the org's name with Rename beside it, then everyone in a hairline box, with Add people at its top, then Leave org. The name and how full - the org is sit in the top bar. A member's sheet holds what a maintainer can do to them, as the web's menu does. + the org is sit in the top bar. More on a member opens what a maintainer can do to them, as the web's menu does. --}} @@ -72,6 +72,4 @@ class="w-full text-theme-ink" @endif - - @include('native.partials.org-member-sheet') diff --git a/resources/views/native/partials/org-member-row.blade.php b/resources/views/native/partials/org-member-row.blade.php index f9f790da..1ef0dc4a 100644 --- a/resources/views/native/partials/org-member-row.blade.php +++ b/resources/views/native/partials/org-member-row.blade.php @@ -1,8 +1,8 @@ {{-- Someone in an org, as the web's members dialog lists them: their avatar ringed in their lane, their name, "(you)" on your own row, their handle, and their role. A tap opens their profile. For a maintainer, a slot on the right - holds the button that opens their sheet, or a spinner while a change to them is on its way, and stays empty on - your own row so the roles line up. + holds More, which opens the platform's own menu of what they can do to them (see OrgMemberMenu), or a spinner + while a change to them is on its way, and stays empty on your own row so the roles line up. --}} @@ -30,8 +30,8 @@ @if ($isChanging) - @elseif ($canManage) - + @elseif ($menu !== []) + diff --git a/resources/views/native/partials/org-member-sheet.blade.php b/resources/views/native/partials/org-member-sheet.blade.php deleted file mode 100644 index eecae2a6..00000000 --- a/resources/views/native/partials/org-member-sheet.blade.php +++ /dev/null @@ -1,70 +0,0 @@ -{{-- - What a maintainer can do to someone else in the org, as the web's member menu offers it: make them a maintainer or - a contributor, then remove them. The sheet names who it's for, and can open their profile. Make contributor is off - for the last maintainer, since an org needs one. Remove from org asks first, in the sheet itself, since iOS drops - an alert raised over a sheet. ---}} - - @if ($managed) - @php($user = $managed['user']) - @php($isMaintainer = $managed['role'] === \App\NativeComponents\OrgMembers::Maintainer) - - - - @include('native.partials.avatar', ['user' => $user, 'size' => 44]) - - {{ $user->name }} - {{ $user->handle() }} · {{ $managed['role'] ?? \App\NativeComponents\OrgMembers::Contributor }} - - - - - - - @if ($confirmingRemoval) - - - Remove {{ $user->handle() }}? - They stop seeing the org and its history. A maintainer would have to add them back. - - - - Cancel - - - Remove - - - - @else - @if ($isMaintainer) - @if ($isLastMaintainer) - - Make contributor - An org needs at least one maintainer. - - @else - - - Make contributor - - - @endif - @else - - - Make maintainer - - - @endif - - - - - Remove from org - - - @endif - - @endif - diff --git a/tests/Feature/Screens/OrgMembersTest.php b/tests/Feature/Screens/OrgMembersTest.php index 2b487533..dc5b5cc9 100644 --- a/tests/Feature/Screens/OrgMembersTest.php +++ b/tests/Feature/Screens/OrgMembersTest.php @@ -131,9 +131,7 @@ function writesSent(): array ->assertMissingElement('pressable', fn (array $node): bool => str_starts_with((string) ($node['ref'] ?? ''), 'manage-')) ->assertSee('Leave org'); - $screen->call('manage', 'ada') - ->assertSet('managing', null) - ->call('rename', 'mine now') + $screen->call('rename', 'mine now') ->call('makeMaintainer', 'ada') ->call('confirmRemove', 'ada') ->assertNativeNotCalled('Dialog.Alert') @@ -170,17 +168,23 @@ function writesSent(): array ->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'Only maintainers can do that.'); }); -it('makes a contributor a maintainer from their sheet', function () { +it('offers a maintainer the web’s menu on everyone else, and makes a contributor a maintainer from it', function (string $platform) { serveOrg(orgJson()); holdRefreshes(); - $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) - ->tap('manage-ada') - ->assertSet('managing', 'ada') - ->assertSee('Make maintainer') - ->assertSee('Remove from org') - ->tap('sheet-make-maintainer') - ->assertSet('managing', null); + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members', platform: $platform)); + + // More only opens its menu, which has what the web's member menu has. + expect(nodeByRef($screen, 'manage-ada')['props']['has_menu'] ?? null)->toBeTrue() + ->and(menuItems($screen, 'manage-ada'))->toBe([ + ['make-maintainer-ada', 'Make maintainer', 'custom:shield'], + 'divider', + ['remove-ada', 'Remove from org', 'custom:user-minus'], + ]) + ->and(menuItems($screen, 'manage-grace')[0])->toBe(['make-contributor-grace', 'Make contributor', 'custom:user-round']) + ->and(nodeByRef($screen, 'remove-ada')['props']['destructive'] ?? null)->toBeTrue(); + + $screen->tap('make-maintainer-ada'); expect($screen->instance()->members[1]['role'])->toBe('maintainer'); @@ -193,16 +197,13 @@ function writesSent(): array expect($screen->instance()->members[1]['role'])->toBe('maintainer') ->and(writesSent())->toBe(['PUT remotes/orgs/org-1/members/ada']) ->and(json_decode(sentTo('PUT', 'remotes/orgs/org-1/members/ada')[0], true))->toBe(['role' => 'maintainer']); -}); +})->with(['ios', 'android']); it('offers Make contributor for a maintainer, and puts their role back when devnull says no', function () { serveOrg(orgJson(), ['PUT remotes/orgs/org-1/members/grace' => 422]); holdRefreshes(); - $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) - ->tap('manage-grace') - ->assertSee('Make contributor') - ->tap('sheet-make-contributor'); + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members'))->tap('make-contributor-grace'); expect($screen->instance()->members[2]['role'])->toBe('contributor'); @@ -218,35 +219,30 @@ function writesSent(): array ['user' => userJson(['id' => 'u3', 'handle' => 'grace', 'name' => 'Grace Hopper']), 'role' => 'maintainer', 'read_through_id' => 1], ])); - finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) - ->tap('manage-grace') - ->assertSee('An org needs at least one maintainer.') - ->call('makeContributor', 'grace') + $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')); + + expect(nodeByRef($screen, 'make-contributor-grace')['props']['disabled'] ?? null)->toBeTrue(); + + $screen->call('makeContributor', 'grace') ->assertNativeCalled('ToastKit.Show', fn (array $params): bool => $params['message'] === 'An org needs at least one maintainer.'); expect(writesSent())->toBe([]); }); -it('asks in the sheet before removing someone, and removes them only once you say so', function () { +it('asks before removing someone, and removes them only once you say so', function () { serveOrg(orgJson()); holdRefreshes(); $screen = finishRefreshes(Native::visit('/remotes/orgs/org-1/members')) - ->tap('manage-ada') - ->tap('sheet-remove') - ->assertSet('managing', 'ada') - ->assertSee('Remove @ada?') - ->assertSee('They stop seeing the org and its history. A maintainer would have to add them back.') - ->tap('cancel-remove') - ->assertSet('managing', 'ada') - ->assertDontSee('Remove @ada?') + ->tap('remove-ada') + ->assertNativeCalled('Dialog.Alert', fn (array $params): bool => $params['title'] === 'Remove @ada?' + && $params['message'] === 'They stop seeing the org and its history. A maintainer would have to add them back.') + ->emitNative(ButtonPressed::class, ['index' => 0, 'label' => 'Cancel', 'id' => 'remove-member:org-1:ada']) ->assertSee('Ada Lovelace'); expect(writesSent())->toBe([]); - $screen->tap('sheet-remove') - ->tap('confirm-remove') - ->assertSet('managing', null) + $screen->emitNative(ButtonPressed::class, ['index' => 1, 'label' => 'Remove', 'id' => 'remove-member:org-1:ada']) ->assertDontSee('Ada Lovelace'); serveOrg(orgJson(members: [orgJson()['data']['members'][0], orgJson()['data']['members'][2]])); @@ -337,7 +333,7 @@ function writesSent(): array ->tap('candidate-linus') ->assertSee('4/'.NewOrg::MaxMembers) ->assertSee('Add 1') - ->tap('share-history') + ->toggle('share-history-switch', true) ->assertSet('sharesHistory', true) ->tap('add-people');