From c927c3056767e955e57a6d5f411591688f7de239 Mon Sep 17 00:00:00 2001 From: dakkar Date: Thu, 3 Jul 2025 12:07:43 +0100 Subject: [PATCH] mark grouped notifs by oldest id - sort-of fix 1139 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Misskey's code does the same, but our groups behave differently enough that this may be not the best choice for example, let's say we have: - notifications 1-5 for reaction to note A - notifications 6-8 for reaction to note B - notifications 9-12 for reaction to note A - notification 13-19 for non-groupable events - notification 20 for reaction to note A and that events happened one every minute (so the last notification is from 20 minutes ago) client requests the most recent 10 notifications; we fetch notifications 1-10, and reply: - grouped id 6 for reactions 6-8 to note B - grouped id 10 for reactions 1-5, 9-10 to note A then the client requests 10 more notifications, untilId=10; we fetch notifications 11-20, and reply: - non-grouped notifications 13-19 - grouped id 20 for reactions 11,12,20 to note A because we sort by id, and also the `createdAt` marks the _newest_ event in each group, the client will then show: 6 reactions to note B, 6 minutes ago 4 reactions to note A, 1 minute ago notifications 13-19, 13 minutes to 19 minutes ago 3 reactions to note A, 11 minutes ago I don't know how to make this work better ☹ --- .../server/api/endpoints/i/notifications-grouped.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts index 444734070f..3821b5a20e 100644 --- a/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts +++ b/packages/backend/src/server/api/endpoints/i/notifications-grouped.ts @@ -116,7 +116,7 @@ export default class extends Endpoint { // eslint- // user has many notifications, the pagination will break the // groups - // scan `notifications` newest-to-oldest + // scan `notifications` newest-to-oldest (unless we have sinceId && !untilId, in which case it's oldest-to-newest) for (let i = 0; i < notifications.length; i++) { const notification = notifications[i]; @@ -135,7 +135,7 @@ export default class extends Endpoint { // eslint- if (prevReaction.type !== 'reaction:grouped') { prevReaction = groupedNotifications[reactionIdx] = { type: 'reaction:grouped', - id: prevReaction.id, // this will be the newest id in this group + id: '', createdAt: prevReaction.createdAt, noteId: prevReaction.noteId!, reactions: [{ @@ -149,6 +149,7 @@ export default class extends Endpoint { // eslint- userId: notification.notifierId!, reaction: notification.reaction!, }); + prevReaction.id = notification.id; // this will be the *oldest* id in this group (newest if sinceId && !untilId) continue; } @@ -167,7 +168,7 @@ export default class extends Endpoint { // eslint- if (prevRenote.type !== 'renote:grouped') { prevRenote = groupedNotifications[renoteIdx] = { type: 'renote:grouped', - id: prevRenote.id, // this will be the newest id in this group + id: '', createdAt: prevRenote.createdAt, noteId: prevRenote.noteId!, userIds: [prevRenote.notifierId!], @@ -175,6 +176,7 @@ export default class extends Endpoint { // eslint- } // add this new renote to the existing group (prevRenote as FilterUnionByProperty).userIds.push(notification.notifierId!); + prevRenote.id = notification.id; // this will be the *oldest* id in this group (newest if sinceId && !untilId) continue; } @@ -182,10 +184,12 @@ export default class extends Endpoint { // eslint- groupedNotifications.push(notification); } - // sort the groups by their id, newest first + // sort the groups by their id groupedNotifications.sort( (a, b) => a.id < b.id ? 1 : a.id > b.id ? -1 : 0, ); + // this matches the logic in NotificationService and it's what MkPagination expects + if (ps.sinceId && !ps.untilId) groupedNotifications.reverse(); return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id); });