merge: mark grouped notifs by oldest id - sort-of fix 1139 (!1152)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/1152

Approved-by: Hazelnoot <acomputerdog@gmail.com>
Approved-by: Marie <github@yuugi.dev>
This commit is contained in:
dakkar 2025-07-27 14:42:29 +00:00
commit b5034f673c
2 changed files with 23 additions and 5 deletions

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);
}); });

View file

@ -23,7 +23,7 @@ SPDX-License-Identifier: AGPL-3.0-only
:moveClass=" $style.transition_x_move" :moveClass=" $style.transition_x_move"
tag="div" tag="div"
> >
<div v-for="(notification, i) in notifications" :key="notification.id"> <div v-for="(notification, i) in sortedByTime(notifications)" :key="notification.id">
<DynamicNote v-if="['reply', 'quote', 'mention'].includes(notification.type)" :class="$style.item" :note="notification.note" :withHardMute="true" :data-scroll-anchor="notification.id"/> <DynamicNote v-if="['reply', 'quote', 'mention'].includes(notification.type)" :class="$style.item" :note="notification.note" :withHardMute="true" :data-scroll-anchor="notification.id"/>
<XNotification v-else :class="$style.item" :notification="notification" :withTime="true" :full="true" :data-scroll-anchor="notification.id"/> <XNotification v-else :class="$style.item" :notification="notification" :withTime="true" :full="true" :data-scroll-anchor="notification.id"/>
</div> </div>
@ -67,6 +67,20 @@ const pagination = computed(() => prefer.r.useGroupedNotifications.value ? {
})), })),
}); });
// for pagination reasons, each notification group needs to have the
// id of the oldest notification inside it, but we want to show the
// groups sorted by the time of the *newest* notification; so we re-sort
// them here
function sortedByTime(notifications) {
return notifications.toSorted(
(a, b) => {
if (a.createdAt < b.createdAt) return 1;
if (a.createdAt > b.createdAt) return -1;
return 0;
}
);
}
function onNotification(notification) { function onNotification(notification) {
const isMuted = props.excludeTypes ? props.excludeTypes.includes(notification.type) : false; const isMuted = props.excludeTypes ? props.excludeTypes.includes(notification.type) : false;
if (isMuted || window.document.visibilityState === 'visible') { if (isMuted || window.document.visibilityState === 'visible') {