mark grouped notifs by oldest id - sort-of fix 1139

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 ☹
This commit is contained in:
dakkar 2025-07-03 12:07:43 +01:00
parent a77c32b17d
commit c927c30567

View file

@ -116,7 +116,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
// user has many notifications, the pagination will break the // user has many notifications, the pagination will break the
// groups // 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++) { for (let i = 0; i < notifications.length; i++) {
const notification = notifications[i]; const notification = notifications[i];
@ -135,7 +135,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
if (prevReaction.type !== 'reaction:grouped') { if (prevReaction.type !== 'reaction:grouped') {
prevReaction = groupedNotifications[reactionIdx] = { prevReaction = groupedNotifications[reactionIdx] = {
type: 'reaction:grouped', type: 'reaction:grouped',
id: prevReaction.id, // this will be the newest id in this group id: '',
createdAt: prevReaction.createdAt, createdAt: prevReaction.createdAt,
noteId: prevReaction.noteId!, noteId: prevReaction.noteId!,
reactions: [{ reactions: [{
@ -149,6 +149,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
userId: notification.notifierId!, userId: notification.notifierId!,
reaction: notification.reaction!, reaction: notification.reaction!,
}); });
prevReaction.id = notification.id; // this will be the *oldest* id in this group (newest if sinceId && !untilId)
continue; continue;
} }
@ -167,7 +168,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
if (prevRenote.type !== 'renote:grouped') { if (prevRenote.type !== 'renote:grouped') {
prevRenote = groupedNotifications[renoteIdx] = { prevRenote = groupedNotifications[renoteIdx] = {
type: 'renote:grouped', type: 'renote:grouped',
id: prevRenote.id, // this will be the newest id in this group id: '',
createdAt: prevRenote.createdAt, createdAt: prevRenote.createdAt,
noteId: prevRenote.noteId!, noteId: prevRenote.noteId!,
userIds: [prevRenote.notifierId!], userIds: [prevRenote.notifierId!],
@ -175,6 +176,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
} }
// add this new renote to the existing group // add this new renote to the existing group
(prevRenote as FilterUnionByProperty<MiGroupedNotification, 'type', 'renote:grouped'>).userIds.push(notification.notifierId!); (prevRenote as FilterUnionByProperty<MiGroupedNotification, 'type', 'renote:grouped'>).userIds.push(notification.notifierId!);
prevRenote.id = notification.id; // this will be the *oldest* id in this group (newest if sinceId && !untilId)
continue; continue;
} }
@ -182,10 +184,12 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
groupedNotifications.push(notification); groupedNotifications.push(notification);
} }
// sort the groups by their id, newest first // sort the groups by their id
groupedNotifications.sort( groupedNotifications.sort(
(a, b) => a.id < b.id ? 1 : a.id > b.id ? -1 : 0, (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); return await this.notificationEntityService.packGroupedMany(groupedNotifications, me.id);
}); });