[WiP] Improve gap handling

app/javascript/mastodon/actions/notification_groups.ts

@@ -73,17 +73,15 @@ export const fetchNotifications = createDataLoadingThunk(
           : excludeAllTypesExcept(activeFilter),
     });
   },
-  ({ notifications, links }, { dispatch }) => {
+  ({ notifications }, { dispatch }) => {
     dispatchAssociatedRecords(dispatch, notifications);
-
-    // We ignore the previous link, as it will always be here but we know there are no more
-    // recent notifications when doing the initial load
-    const nextLink = links.refs.find((link) => link.rel === 'next');
-
     const payload: (ApiNotificationGroupJSON | NotificationGap)[] =
       notifications;
 
-    if (nextLink) payload.push({ type: 'gap', loadUrl: nextLink.uri });
+    // TODO: might be worth not using gaps for that…
+    // if (nextLink) payload.push({ type: 'gap', loadUrl: nextLink.uri });
+    if (notifications.length > 1)
+      payload.push({ type: 'gap', maxId: notifications.at(-1)?.page_min_id });
 
     return payload;
     // dispatch(submitMarkers());
@@ -93,14 +91,12 @@ export const fetchNotifications = createDataLoadingThunk(
 export const fetchNotificationsGap = createDataLoadingThunk(
   'notificationGroups/fetchGat',
   async (params: { gap: NotificationGap }) =>
-    apiFetchNotifications({}, params.gap.loadUrl),
+    apiFetchNotifications({ max_id: params.gap.maxId }),
 
-  ({ notifications, links }, { dispatch }) => {
+  ({ notifications }, { dispatch }) => {
     dispatchAssociatedRecords(dispatch, notifications);
 
-    const nextLink = links.refs.find((link) => link.rel === 'next');
-
-    return { notifications, nextLink };
+    return { notifications };
   },
 );
 

app/javascript/mastodon/api/notifications.ts

@@ -1,15 +1,13 @@
 import api, { apiRequest, getLinks } from 'mastodon/api';
 import type { ApiNotificationGroupJSON } from 'mastodon/api_types/notifications';
 
-export const apiFetchNotifications = async (
-  params?: {
-    exclude_types?: string[];
-  },
-  forceUrl?: string,
-) => {
+export const apiFetchNotifications = async (params?: {
+  exclude_types?: string[];
+  max_id?: string;
+}) => {
   const response = await api().request<ApiNotificationGroupJSON[]>({
     method: 'GET',
-    url: forceUrl ?? '/api/v2_alpha/notifications',
+    url: '/api/v2_alpha/notifications',
     params,
   });
 

app/javascript/mastodon/features/notifications_v2/index.tsx

@@ -254,7 +254,7 @@ export const Notifications: React.FC<{
     return notifications.map((item) =>
       item.type === 'gap' ? (
         <LoadGap
-          key={item.loadUrl}
+          key={`${item.maxId}-${item.sinceId}`}
           disabled={isLoading}
           param={item}
           onClick={handleLoadGap}

app/javascript/mastodon/reducers/notifications_groups.ts

@@ -17,6 +17,7 @@ import {
   disconnectTimeline,
   timelineDelete,
 } from 'mastodon/actions/timelines_typed';
+import { compareId } from 'mastodon/compare_id';
 import {
   NOTIFICATIONS_GROUP_MAX_AVATARS,
   createNotificationGroupFromJSON,
@@ -26,7 +27,8 @@ import type { NotificationGroup } from 'mastodon/models/notification_group';
 
 export interface NotificationGap {
   type: 'gap';
-  loadUrl: string;
+  maxId?: string;
+  sinceId?: string;
 }
 
 interface NotificationGroupsState {
@@ -93,32 +95,67 @@ export const notificationsGroupsReducer =
         state.isLoading = false;
       })
       .addCase(fetchNotificationsGap.fulfilled, (state, action) => {
-        const { notifications, nextLink } = action.payload;
+        const { notifications } = action.payload;
 
         // find the gap in the existing notifications
         const gapIndex = state.groups.findIndex(
           (groupOrGap) =>
-            groupOrGap.type === 'gap' && groupOrGap.loadUrl === nextLink?.uri,
+            groupOrGap.type === 'gap' &&
+            groupOrGap.sinceId === action.meta.arg.gap.sinceId &&
+            groupOrGap.maxId === action.meta.arg.gap.maxId,
         );
 
-        if (!gapIndex)
+        if (gapIndex < 0)
           // We do not know where to insert, let's return
           return;
 
+        // Filling a disconnection gap means we're getting historical data
+        // about groups we may know or may not know about.
+
+        // The notifications timeline is split in two by the gap, with
+        // group information newer than the gap, and group information older
+        // than the gap.
+
+        // Filling a gap should not touch anything before the gap, so any
+        // information on groups already appearing before the gap should be
+        // discarded, while any information on groups appearing after the gap
+        // can be updated and re-ordered.
+
+        const oldestPageNotification = notifications.at(-1)?.page_min_id;
+
         // replace the gap with the notifications + a new gap
 
-        const toInsert: NotificationGroupsState['groups'] = notifications.map(
-          (json) => createNotificationGroupFromJSON(json),
-        );
+        const toInsert: NotificationGroupsState['groups'] = notifications
+          .map((json) => createNotificationGroupFromJSON(json))
+          .filter(
+            // TODO: update notification groups instead of just filtering duplicates
+            (notification) =>
+              state.groups.every(
+                (group) =>
+                  group.type === 'gap' ||
+                  group.group_key !== notification.group_key,
+              ),
+          );
 
-        if (nextLink?.uri && notifications.length > 0) {
+        const sinceId = action.meta.arg.gap.sinceId;
+        if (
+          notifications.length > 0 &&
+          !(
+            oldestPageNotification &&
+            sinceId &&
+            compareId(oldestPageNotification, sinceId) <= 0
+          )
+        ) {
           // If we get an empty page, it means we reached the bottom, so we do not need to insert a new gap
+          // Similarly, if we've fetched more than the gap's, this means we have completely filled it
           toInsert.push({
             type: 'gap',
-            loadUrl: nextLink.uri,
+            maxId: notifications.at(-1)?.page_max_id,
+            sinceId,
           } as NotificationGap);
         }
 
+        // TODO: merge gaps made adjacent by moved/deleted groups
         state.groups.splice(gapIndex, 1, ...toInsert);
 
         state.isLoading = false;
@@ -162,13 +199,12 @@ export const notificationsGroupsReducer =
       })
       .addCase(disconnectTimeline, (state, action) => {
         if (action.payload.timeline === 'home') {
-          if (state.groups.length > 0 && state.groups[0]?.type === 'gap')
-            state.groups.shift();
-
-          state.groups.unshift({
-            type: 'gap',
-            loadUrl: 'TODO_LOAD_URL_TOP_OF_TL', // TODO
-          });
+          if (state.groups.length > 0 && state.groups[0]?.type !== 'gap') {
+            state.groups.unshift({
+              type: 'gap',
+              sinceId: state.groups[0]?.page_min_id,
+            });
+          }
         }
       })
       .addCase(timelineDelete, (state, action) => {