diff --git a/app/Queries/NotificationGroupsQuery.php b/app/Queries/NotificationGroupsQuery.php index eb12ced25..d267551fc 100644 --- a/app/Queries/NotificationGroupsQuery.php +++ b/app/Queries/NotificationGroupsQuery.php @@ -21,6 +21,8 @@ private const int STACKED_ACTORS = 3; + private const array GROUP_COLUMNS = ['group_id', 'actor_position', 'group_rank', 'event_count', 'actors_count', 'unread_count']; + /** @return CursorPaginator */ public function paginate(User $user, string $type = 'all', ?CarbonImmutable $through = null): CursorPaginator { @@ -30,7 +32,7 @@ public function paginate(User $user, string $type = 'all', ?CarbonImmutable $thr ->selectRaw("NTH_VALUE(actor_id, 2) OVER ({$order}) AS second_actor_id") ->selectRaw("NTH_VALUE(actor_id, 3) OVER ({$order}) AS third_actor_id"); - $query = Notification::query()->fromSub($events, 'notifications') + $query = $this->withNotifications($events, ['second_actor_id', 'third_actor_id']) ->where('group_rank', 1) ->with([ 'actor' => fn (BelongsTo $query): BelongsTo => $query @@ -62,13 +64,13 @@ public function unreadCount(User $user, string $type = 'all'): int $oldestUnread = $this->visible($user)->unread()->oldest()->first(); return $oldestUnread instanceof Notification - ? $this->groupsSince($user, $oldestUnread->created_at, $type)->where('group_rank', 1)->where('unread_count', '>', 0)->count() + ? $this->groupsSince($user, $oldestUnread->created_at, $type)->whereNull('read_at')->distinct()->count('group_id') : 0; } public function members(User $user, Notification $notification, string $type = 'all'): NotificationBuilder { - return $this->group($this->visible($user), $type, $notification) + return $this->withNotifications($this->group($this->visible($user), $type, $notification)) ->with(['actor' => fn (BelongsTo $query): BelongsTo => $query ->withExists(['followers as is_followed' => fn (Builder $query): Builder => $query->whereKey($user->id)])]); } @@ -90,7 +92,25 @@ private function groupsSince(User $user, CarbonInterface $since, string $type = private function visible(User $user): NotificationBuilder { - return Notification::query()->where('recipient_id', $user->id)->relevant()->select('notifications.*'); + return Notification::query()->where('recipient_id', $user->id)->relevant()->select([ + 'notifications.id', 'notifications.recipient_id', 'notifications.actor_id', 'notifications.commit_id', + 'notifications.type', 'notifications.created_at', 'notifications.read_at', + ]); + } + + /** + * Grouping only reads the columns visible() selects. Sorting whole notifications through its windows costs more + * than looking them up by id once the caller has narrowed the groups down. + * + * @param list $columns + */ + private function withNotifications(NotificationBuilder $grouped, array $columns = []): NotificationBuilder + { + $notifications = Notification::query()->select('notifications.*') + ->addSelect(array_map(fn (string $column): string => "grouped.{$column}", [...self::GROUP_COLUMNS, ...$columns])) + ->joinSub($grouped, 'grouped', 'grouped.id', '=', 'notifications.id'); + + return Notification::query()->fromSub($notifications, 'notifications')->withCasts(['event_count' => 'integer']); } private function group(NotificationBuilder $visible, string $type = 'all', ?Notification $member = null): NotificationBuilder @@ -120,7 +140,8 @@ private function group(NotificationBuilder $visible, string $type = 'all', ?Noti // Deleted ancestors supply grouping identity only; content comes from relevant notifications. A commit's // recorded root is always one of its ancestors, so the walk up the thread only continues past it when the // recorded root is itself a reply. Follows never group across days, as a busy account gets followed so often - // that its follows would otherwise never stop joining a single group. + // that its follows would otherwise never stop joining a single group. The windows over the whole history sit in + // their own CTE, so the per-topic windows after them share a single sort. $sql = <<toSql()}), threads AS ( @@ -146,13 +167,17 @@ private function group(NotificationBuilder $visible, string $type = 'all', ?Noti FROM visible LEFT JOIN threads ON threads.notification_id = visible.id LEFT JOIN ancestry ON ancestry.notification_id = visible.id AND ancestry.parent_id IS NULL ), - predecessors AS ( - SELECT id, topic_kind, topic_id, topic_day, created_at, - LAG(created_at) OVER (PARTITION BY topic_kind, topic_id, topic_day ORDER BY created_at, id) AS previous_match_at, + ordered AS ( + SELECT topics.*, LAG(topic_kind) OVER (ORDER BY created_at, id) AS previous_topic_kind, LAG(topic_id) OVER (ORDER BY created_at, id) AS previous_topic_id FROM topics ), + predecessors AS ( + SELECT id, topic_kind, topic_id, topic_day, created_at, previous_topic_kind, previous_topic_id, + LAG(created_at) OVER (PARTITION BY topic_kind, topic_id, topic_day ORDER BY created_at, id) AS previous_match_at + FROM ordered + ), sessions AS ( SELECT id, topic_kind, topic_id, topic_day, created_at, SUM(CASE WHEN topic_kind = 'starred' THEN 0 @@ -184,8 +209,7 @@ private function group(NotificationBuilder $visible, string $type = 'all', ?Noti SQL; return Notification::query()->fromSub($sql, 'notifications')->addBinding($visible->getBindings(), 'from') - ->addBinding($filtered->getBindings(), 'from') - ->withCasts(['event_count' => 'integer']); + ->addBinding($filtered->getBindings(), 'from'); } /** @param array $groups */