diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index 831a262a4..8081fdf55 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -370,7 +370,7 @@ Sources: [Pastéis setting](resources/js/components/pasteis-mode-setting.tsx), [ ### Notifications -Open **Notifications** to read in-app activity: follows, replies, quotes, mentions, stars, first commits of people you follow, and moderation messages about rejected commits (approvals do not notify). The navigation shows unread activity and checks for more every 30 seconds. While you have unread notifications or messages, the browser tab's icon shows an orange dot; a tab in the background checks about every five minutes. Related activity is grouped, with expandable details: stars per commit, follows per day, and replies and mentions per thread. Filters (Replies, Quotes, Stars, Mentions, Follows, Moderation) are remembered, and the unread count follows them. Opening the inbox marks all notifications read. You can follow back or unfollow from follower notifications. +Open **Notifications** to read in-app activity: follows, replies, quotes, mentions, stars, first commits of people you follow, and moderation messages about rejected commits (approvals do not notify). The navigation shows unread activity and checks for more every 30 seconds. While you have unread notifications or messages, the browser tab's icon shows an orange dot; a tab in the background checks about every five minutes. Related activity is grouped, with expandable details: stars per commit, follows per day, and replies and mentions per thread. Filters (Replies, Quotes, Stars, Mentions, Follows, Moderation) are remembered, and the unread count follows them. Opening the inbox marks all notifications read. You can follow back or unfollow from follower notifications, and star a reply, quote, or mention straight from its notification. When someone you follow makes their first commit, you get a notification for it under **All**, so a newcomer's first post gets seen and answered. It comes once per person, after approval, and never for replies. diff --git a/app/Http/Controllers/NotificationController.php b/app/Http/Controllers/NotificationController.php index ab7c8034f..c58b06199 100644 --- a/app/Http/Controllers/NotificationController.php +++ b/app/Http/Controllers/NotificationController.php @@ -5,11 +5,13 @@ namespace App\Http\Controllers; use App\Http\Requests\ListNotificationsRequest; +use App\Models\Notification; use App\Models\User; use App\Queries\NotificationGroupsQuery; use Carbon\CarbonImmutable; use Illuminate\Container\Attributes\CurrentUser; use Illuminate\Contracts\Pagination\CursorPaginator; +use Illuminate\Pagination\CursorPaginator as Paginator; use Inertia\Inertia; use Inertia\Response; @@ -20,9 +22,34 @@ public function index(ListNotificationsRequest $request, #[CurrentUser] User $us $type = $request->type($user); return Inertia::render('notification/index', [ - 'notifications' => Inertia::scroll(fn (): CursorPaginator => $query->withStarredCommitImages($query->withQuotedCommits($user, $query->paginate($user, $type, $request->through()))))->matchOn('data.group_id'), + 'notifications' => Inertia::scroll(function () use ($request, $user, $type, $query): CursorPaginator { + /** @var Paginator $notifications */ + $notifications = $query->withStarredCommitImages($query->withQuotedCommits($user, $query->paginate($user, $type, $request->through()))); + + $stars = $query->commitStars($user, $notifications->items()); + + // Replies, mentions and quotes can be starred from the inbox, so they say how starred they are. + return $notifications->through(fn (Notification $notification): array => $this->withCommitStars($notification, $stars)); + })->matchOn('data.group_id'), 'filters' => ['type' => $type], 'readAt' => CarbonImmutable::now()->startOfSecond()->toJSON(), ]); } + + /** + * The notification as the page gets it, its commit saying how starred it is. + * + * @param array $stars + * @return array + */ + private function withCommitStars(Notification $notification, array $stars): array + { + $commit = $notification->commit; + + if ($commit === null || ! isset($stars[$commit->id])) { + return $notification->toArray(); + } + + return [...$notification->toArray(), 'commit' => [...$commit->toArray(), ...$stars[$commit->id]]]; + } } diff --git a/app/Http/Controllers/NotificationGroupController.php b/app/Http/Controllers/NotificationGroupController.php index 6f55d5a3b..6fe1073c0 100644 --- a/app/Http/Controllers/NotificationGroupController.php +++ b/app/Http/Controllers/NotificationGroupController.php @@ -15,7 +15,29 @@ { public function show(ShowNotificationGroupRequest $request, #[CurrentUser] User $user, Notification $notification, NotificationGroupsQuery $query): JsonResponse { - return response()->json($query->members($user, $notification, $request->type()) - ->with('commit')->latest()->latest('id')->cursorPaginate(20)->withQueryString()); + $members = $query->members($user, $notification, $request->type()) + ->with('commit')->latest()->latest('id')->cursorPaginate(20)->withQueryString(); + + $stars = $query->commitStars($user, $members->items()); + + // Expanded replies, mentions and quotes can be starred from the inbox too, so they say how starred they are. + return response()->json($members->through(fn (Notification $member): array => $this->withCommitStars($member, $stars))); + } + + /** + * The notification as the inbox gets it, its commit saying how starred it is. + * + * @param array $stars + * @return array + */ + private function withCommitStars(Notification $notification, array $stars): array + { + $commit = $notification->commit; + + if ($commit === null || ! isset($stars[$commit->id])) { + return $notification->toArray(); + } + + return [...$notification->toArray(), 'commit' => [...$commit->toArray(), ...$stars[$commit->id]]]; } } diff --git a/app/Queries/NotificationGroupsQuery.php b/app/Queries/NotificationGroupsQuery.php index 60fb07cdf..56956c1d5 100644 --- a/app/Queries/NotificationGroupsQuery.php +++ b/app/Queries/NotificationGroupsQuery.php @@ -5,6 +5,7 @@ namespace App\Queries; use App\Enums\NotificationType; +use App\Models\Commit; use App\Models\Notification; use App\Models\Profile; use App\Models\User; @@ -74,6 +75,33 @@ public function withQuotedCommits(User $user, CursorPaginator $groups): CursorPa return $groups; } + /** + * How starred the actor's reply, mention or quote is and whether the user starred it, by the commit's ID: the + * inbox lets the user star it from the notification. + * + * @param iterable $notifications + * @return array + */ + public function commitStars(User $user, iterable $notifications): array + { + $commitIds = new Collection($notifications) + ->whereIn('type', [NotificationType::Replied, NotificationType::Mentioned, NotificationType::Quoted]) + ->map(fn (Notification $notification): ?string => $notification->commit?->id) + ->filter() + ->all(); + + return Commit::query() + ->whereKey($commitIds) + ->select('commits.id') + ->withStarsFor($user) + ->get() + ->mapWithKeys(fn (Commit $commit): array => [$commit->id => [ + 'stars_count' => (int) $commit->stars_count, + 'starred' => (bool) $commit->starred, + ]]) + ->all(); + } + /** * A star is always on the recipient's own commit, or the organization's they heard about it for, so its images may * show beside it: an image-only commit has nothing else to show. diff --git a/resources/js/components/commit-star-button.tsx b/resources/js/components/commit-star-button.tsx index 3f80fa66d..1230f2169 100644 --- a/resources/js/components/commit-star-button.tsx +++ b/resources/js/components/commit-star-button.tsx @@ -30,12 +30,20 @@ function starLabel(starred: boolean, count: number): string { return `${verb} commit, ${formatExactCount(count)} ${noun}`; } +/** + * Only what starring reads, so a commit loaded without its full author, like a notification's, can be starred too. + */ +type StarrableCommit = Pick< + Commit, + 'id' | 'status' | 'starred' | 'stars_count' +> & { author: { id: string } }; + export function CommitStarButton({ commit, keyShortcut, className, }: { - commit: Commit; + commit: StarrableCommit; keyShortcut?: string; className?: string; }) { @@ -211,7 +219,7 @@ function StarCount({ className, }: { ref?: Ref; - commit: Commit; + commit: StarrableCommit; count: number; className?: string; }) { diff --git a/resources/js/pages/notification/index.tsx b/resources/js/pages/notification/index.tsx index b2450aa29..1db6e2777 100644 --- a/resources/js/pages/notification/index.tsx +++ b/resources/js/pages/notification/index.tsx @@ -38,6 +38,7 @@ import { AuthorAvatar } from '@/components/author-avatar'; import { CommitBodyExcerpt } from '@/components/commit-body'; import { EmptyFeed } from '@/components/commit-feed'; import { replaceFailedImage } from '@/components/commit-images'; +import { CommitStarButton } from '@/components/commit-star-button'; import { FollowIconButton } from '@/components/follow-button'; import { GraphCanvas } from '@/components/graph-canvas'; import { GraphRail, RailSegment } from '@/components/graph-rail'; @@ -1288,6 +1289,18 @@ export function NotificationContent({ )} {excerpt} {quote} + {isActorsCommit && !grouped && notification.commit && ( + // Above the row's link, so starring doesn't open the commit. +
+ +
+ )} {reason} {notification.type === 'followed' && diff --git a/tests/Feature/Controllers/Api/NotificationControllerTest.php b/tests/Feature/Controllers/Api/NotificationControllerTest.php index 7070d4ca3..7ebea2da3 100644 --- a/tests/Feature/Controllers/Api/NotificationControllerTest.php +++ b/tests/Feature/Controllers/Api/NotificationControllerTest.php @@ -101,6 +101,15 @@ ->assertJsonPath('meta.next_cursor', null); }); +it('leaves the stars of the commits out, as only the web inbox stars from a notification', function (): void { + Notification::factory()->for($this->user, 'recipient')->replied()->create(); + + signedApiRequest($this->keyPair, 'GET', route('api-notification.index')) + ->assertOk() + ->assertJsonMissingPath('data.0.commit.stars_count') + ->assertJsonMissingPath('data.0.commit.starred'); +}); + it('hides notifications that no longer hold', function (): void { $notification = Notification::factory()->for($this->user, 'recipient')->create(); diff --git a/tests/Feature/Controllers/NotificationControllerTest.php b/tests/Feature/Controllers/NotificationControllerTest.php index a3a875e6c..128ac815d 100644 --- a/tests/Feature/Controllers/NotificationControllerTest.php +++ b/tests/Feature/Controllers/NotificationControllerTest.php @@ -64,6 +64,36 @@ ->where('notifications.data.0.commit.id', $mentioned->commit_id)); }); +it('says how starred the reply is and whether the user starred it, so it can be starred from the inbox', function (): void { + $user = User::factory()->create(); + $replied = Notification::factory()->for($user, 'recipient')->replied()->create(); + $reply = Commit::query()->findOrFail($replied->commit_id); + resolve(CreateCommitStar::class)->handle($user, $reply); + resolve(CreateCommitStar::class)->handle(User::factory()->create(), $reply); + + $response = $this->actingAs($user)->get(route('notification.index')); + + $response->assertOk() + ->assertInertia(fn ($page) => $page + ->where('notifications.data.0.commit.id', $reply->id) + ->where('notifications.data.0.commit.stars_count', 2) + ->where('notifications.data.0.commit.starred', true)); +}); + +it('says how starred an expanded reply is and whether the user starred it', function (): void { + $user = User::factory()->create(); + $replied = Notification::factory()->for($user, 'recipient')->replied()->create(); + $reply = Commit::query()->findOrFail($replied->commit_id); + resolve(CreateCommitStar::class)->handle($user, $reply); + resolve(CreateCommitStar::class)->handle(User::factory()->create(), $reply); + + $this->actingAs($user)->getJson(route('notification-group.show', $replied)) + ->assertOk() + ->assertJsonPath('data.0.commit.id', $reply->id) + ->assertJsonPath('data.0.commit.stars_count', 2) + ->assertJsonPath('data.0.commit.starred', true); +}); + it('hides notifications that no longer hold', function (): void { $user = User::factory()->create(); $notification = Notification::factory()->for($user, 'recipient')->create(); diff --git a/tests/Unit/NotificationIndex.test.tsx b/tests/Unit/NotificationIndex.test.tsx index ad6f1db8d..4370a8d1d 100644 --- a/tests/Unit/NotificationIndex.test.tsx +++ b/tests/Unit/NotificationIndex.test.tsx @@ -63,6 +63,7 @@ async function renderPage(page: () => ReactElement): Promise { props: { auth: { user: { id: 'recipient', handle: 'nunomaduro' }, + organizations: [], log: {}, }, errors: {}, @@ -490,3 +491,48 @@ test('shows an organization that replied, quoted or mentioned as itself: square, assert.match(item, /href="\/@laravel"/); assert.doesNotMatch(item, /Follow/); }); + +const starrableReply = { + ...notification, + commit: { + ...notification.commit, + status: 'approved', + starred: false, + stars_count: 2, + }, +} as NotificationGroup; + +void test('lets a single reply be starred from the inbox, outside its link', async () => { + const body = await render([starrableReply]); + + assert.match(body, /]*data-test="star-commit-button"/); + assert.doesNotMatch( + body, + /).)*data-test="star-commit-button"/s, + ); +}); + +for (const type of ['mentioned', 'quoted']) { + void test(`lets a single ${type} commit be starred from the inbox`, async () => { + const body = await render([ + { ...starrableReply, type } as NotificationGroup, + ]); + + assert.match(body, /data-test="star-commit-button"/); + }); +} + +void test('offers no star on stars, groups or commits still in review', async () => { + for (const item of [ + { ...starrableReply, type: 'starred' }, + { ...starrableReply, actors_count: 2, event_count: 2 }, + { + ...starrableReply, + commit: { ...starrableReply.commit, status: 'pending' }, + }, + ]) { + const body = await render([item as NotificationGroup]); + + assert.doesNotMatch(body, /data-test="star-commit-button"/); + } +});